Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DXCDT-482: Fix deleting the universal login template within auth0_branding #695

Merged
merged 2 commits into from
Jul 10, 2023
Merged

DXCDT-482: Fix deleting the universal login template within auth0_branding #695

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bc8b8c2
Fix deleting the universal login template within auth0_branding
sergiught Jul 7, 2023
9877a28
Fix typo
sergiught Jul 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/resources/branding.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ Optional:
<a id="nestedblock--universal_login"></a>
### Nested Schema for `universal_login`

Optional:
Required:

- `body` (String) The body of login pages.
- `body` (String) The html template for the New Universal Login Experience.

## Import

Expand Down
79 changes: 37 additions & 42 deletions internal/auth0/branding/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,17 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/id"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"

"github.com/auth0/terraform-provider-auth0/internal/config"
"github.com/auth0/terraform-provider-auth0/internal/value"
)

var errNoCustomDomain = fmt.Errorf(
"managing the Universal Login body through the 'auth0_branding' resource requires at least one custom domain " +
"to be configured for the tenant.\n\nUse the 'auth0_custom_domain' resource to set one up",
)

// NewResource will return a new auth0_branding resource.
func NewResource() *schema.Resource {
return &schema.Resource{
Expand All @@ -32,6 +38,7 @@ func NewResource() *schema.Resource {
"colors": {
Type: schema.TypeList,
Optional: true,
Computed: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marking this now as computed in order to not require any custom logic within the read func that only reads the colors details if this is present within the config as it goes against best practices.

MaxItems: 1,
Description: "Configuration settings for colors for branding.",
Elem: &schema.Resource{
Expand Down Expand Up @@ -66,6 +73,7 @@ func NewResource() *schema.Resource {
"font": {
Type: schema.TypeList,
Optional: true,
Computed: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marking this now as computed in order to not require any custom logic within the read func that only reads the font details if this is present within the config as it goes against best practices.

MaxItems: 1,
Description: "Configuration settings to customize the font.",
Elem: &schema.Resource{
Expand All @@ -87,10 +95,10 @@ func NewResource() *schema.Resource {
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"body": {
Type: schema.TypeString,
Optional: true,
Computed: true,
Description: "The body of login pages.",
Type: schema.TypeString,
Required: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marking this as required to prevent instances where the font block is present with no attributes assigned.

ValidateFunc: validation.StringIsNotEmpty,
Description: "The html template for the New Universal Login Experience.",
},
},
},
Expand All @@ -115,18 +123,12 @@ func readBranding(_ context.Context, d *schema.ResourceData, m interface{}) diag
result := multierror.Append(
d.Set("favicon_url", branding.GetFaviconURL()),
d.Set("logo_url", branding.GetLogoURL()),
d.Set("colors", flattenBrandingColors(branding.GetColors())),
d.Set("font", flattenBrandingFont(branding.GetFont())),
d.Set("universal_login", nil),
)
if _, ok := d.GetOk("colors"); ok {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a very weird custom logic we had within the read func that was only assigning the API data if the config blocks were present.

result = multierror.Append(result, d.Set("colors", flattenBrandingColors(branding.GetColors())))
}
if _, ok := d.GetOk("font"); ok {
result = multierror.Append(result, d.Set("font", flattenBrandingFont(branding.GetFont())))
}
if _, ok := d.GetOk("universal_login"); ok {
if err := checkForCustomDomains(api); err != nil {
return diag.FromErr(err)
}

if err := checkForCustomDomains(api); err == nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're only fetching template data if there's a custom domain set.

brandingUniversalLogin, err := flattenBrandingUniversalLogin(api)
if err != nil {
return diag.FromErr(err)
Expand All @@ -147,6 +149,19 @@ func updateBranding(ctx context.Context, d *schema.ResourceData, m interface{})
}
}

oldUL, newUL := d.GetChange("universal_login")
oldUniversalLogin := oldUL.([]interface{})
newUniversalLogin := newUL.([]interface{})

// This indicates that a removal of the block happened, and we need to delete the template.
if len(newUniversalLogin) == 0 && len(oldUniversalLogin) != 0 {
if err := api.Branding.DeleteUniversalLogin(); err != nil {
return diag.FromErr(err)
}

return readBranding(ctx, d, m)
}
Comment on lines +152 to +163
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the part that actually fixes the GitHub issue around the deletion of the template.


if universalLogin := expandBrandingUniversalLogin(d.GetRawConfig()); universalLogin.GetBody() != "" {
if err := checkForCustomDomains(api); err != nil {
return diag.FromErr(err)
Expand All @@ -160,44 +175,31 @@ func updateBranding(ctx context.Context, d *schema.ResourceData, m interface{})
return readBranding(ctx, d, m)
}

func deleteBranding(_ context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
func deleteBranding(_ context.Context, _ *schema.ResourceData, m interface{}) diag.Diagnostics {
api := m.(*config.Config).GetAPI()

if _, ok := d.GetOk("universal_login"); !ok {
d.SetId("")
return nil
}

if err := checkForCustomDomains(api); err != nil {
d.SetId("")
return diag.Diagnostics{
{
Severity: diag.Warning,
Summary: "No custom domains configured",
Detail: "Failed to properly destroy the 'auth0_branding' resource " +
"because no custom domains are available on the tenant.",
AttributePath: cty.Path{cty.GetAttrStep{Name: "universal_login"}},
},
if err == errNoCustomDomain {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The delete func was also improved to directly allow the deletion of the resource if there are no custom domains set.

return nil
}

return diag.FromErr(err)
}

if err := api.Branding.DeleteUniversalLogin(); err != nil {
return diag.FromErr(err)
}

d.SetId("")
return nil
}

func expandBranding(config cty.Value) *management.Branding {
branding := &management.Branding{
return &management.Branding{
FaviconURL: value.String(config.GetAttr("favicon_url")),
LogoURL: value.String(config.GetAttr("logo_url")),
Colors: expandBrandingColors(config.GetAttr("colors")),
Font: expandBrandingFont(config.GetAttr("font")),
}

return branding
}

func expandBrandingColors(config cty.Value) *management.BrandingColors {
Expand Down Expand Up @@ -267,10 +269,6 @@ func flattenBrandingUniversalLogin(api *management.Management) ([]interface{}, e
return nil, err
}

if universalLogin == nil {
return nil, nil
}
Comment on lines -270 to -272
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was unnecessary.


flattenedUniversalLogin := []interface{}{
map[string]interface{}{
"body": universalLogin.GetBody(),
Expand All @@ -284,6 +282,7 @@ func flattenBrandingFont(brandingFont *management.BrandingFont) []interface{} {
if brandingFont == nil {
return nil
}

return []interface{}{
map[string]interface{}{
"url": brandingFont.GetURL(),
Expand All @@ -298,11 +297,7 @@ func checkForCustomDomains(api *management.Management) error {
}

if len(customDomains) < 1 {
return fmt.Errorf(
"managing the universal login body through the 'auth0_branding' resource requires at least " +
"one custom domain to be configured for the tenant.\n\n" +
"Use the 'auth0_custom_domain' resource to set one up.",
)
return errNoCustomDomain
}

return nil
Expand Down
89 changes: 43 additions & 46 deletions internal/auth0/branding/resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,16 @@ import (
"github.com/auth0/terraform-provider-auth0/internal/acctest"
)

const testAccTenantAllowsUniversalLoginCustomization = `
const testAccGivenACustomDomain = `
resource "auth0_custom_domain" "my_custom_domain" {
domain = "auth.terraform-provider-auth0.com"
type = "auth0_managed_certs"
type = "auth0_managed_certs"
}

`

const testAccTenantDisallowsUniversalLoginCustomization = `
const testAccTenantDisallowsUniversalLoginCustomizationWhenNoCustomDomainSet = `
resource "auth0_branding" "my_custom_domain" {
logo_url = "https://mycompany.org/v1/logo.png"
logo_url = "https://mycompany.org/v1/logo.png"
favicon_url = "https://mycompany.org/favicon.ico"

universal_login {
Expand All @@ -28,20 +27,20 @@ resource "auth0_branding" "my_custom_domain" {
}
`

const testAccBrandingConfigCreate = `
const testAccBrandingConfigCreate = testAccGivenACustomDomain + `
resource "auth0_branding" "my_brand" {
depends_on = [ auth0_custom_domain.my_custom_domain ]

logo_url = "https://mycompany.org/v1/logo.png"
logo_url = "https://mycompany.org/v1/logo.png"
favicon_url = "https://mycompany.org/favicon.ico"
}
`

const testAccBrandingConfigUpdateAllFields = `
const testAccBrandingConfigUpdateAllFields = testAccGivenACustomDomain + `
resource "auth0_branding" "my_brand" {
depends_on = [ auth0_custom_domain.my_custom_domain ]

logo_url = "https://mycompany.org/v2/logo.png"
logo_url = "https://mycompany.org/v2/logo.png"
favicon_url = "https://mycompany.org/favicon.ico"

colors {
Expand All @@ -59,11 +58,11 @@ resource "auth0_branding" "my_brand" {
}
`

const testAccBrandingConfigUpdateAgain = `
const testAccBrandingConfigThrowsAValidationErrorIfUniversalLoginBodyIsEmpty = testAccGivenACustomDomain + `
resource "auth0_branding" "my_brand" {
depends_on = [ auth0_custom_domain.my_custom_domain ]

logo_url = "https://mycompany.org/v3/logo.png"
logo_url = "https://mycompany.org/v3/logo.png"
favicon_url = "https://mycompany.org/favicon.ico"

colors {
Expand All @@ -75,24 +74,23 @@ resource "auth0_branding" "my_brand" {
}

universal_login {
# Setting this to an empty string should
# not trigger any API call, so the value
# stays the same as the previous scenario.
# Setting this to an empty string should trigger
# a validation error as the API doesn't allow it.
body = ""
}
}
`

const testAccBrandingConfigReset = `
const testAccBrandingConfigRemovesUniversalLoginTemplate = testAccGivenACustomDomain + `
resource "auth0_branding" "my_brand" {
depends_on = [ auth0_custom_domain.my_custom_domain ]

logo_url = "https://mycompany.org/v1/logo.png"
logo_url = "https://mycompany.org/v1/logo.png"
favicon_url = "https://mycompany.org/favicon.ico"
}
`

const testAccBrandingConfigWithOnlyUniversalLogin = `
const testAccBrandingConfigWithOnlyUniversalLogin = testAccGivenACustomDomain + `
resource "auth0_branding" "my_brand" {
depends_on = [ auth0_custom_domain.my_custom_domain ]

Expand All @@ -102,35 +100,34 @@ resource "auth0_branding" "my_brand" {
}
`

func TestAccBranding_WithNoCustomDomainsSet(t *testing.T) {
const testAccBrandingConfigReset = testAccGivenACustomDomain + `
resource "auth0_branding" "my_brand" {
depends_on = [ auth0_custom_domain.my_custom_domain ]
}
`

func TestAccBranding(t *testing.T) {
acctest.Test(t, resource.TestCase{
Steps: []resource.TestStep{
{
Config: testAccTenantDisallowsUniversalLoginCustomization,
Config: testAccTenantDisallowsUniversalLoginCustomizationWhenNoCustomDomainSet,
ExpectError: regexp.MustCompile(
"managing the universal login body through the 'auth0_branding' resource " +
"managing the Universal Login body through the 'auth0_branding' resource " +
"requires at least one custom domain to be configured for the tenant",
),
},
},
})
}

func TestAccBranding(t *testing.T) {
acctest.Test(t, resource.TestCase{
Steps: []resource.TestStep{
{
Config: testAccTenantAllowsUniversalLoginCustomization + testAccBrandingConfigCreate,
Config: testAccBrandingConfigCreate,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("auth0_branding.my_brand", "logo_url", "https://mycompany.org/v1/logo.png"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "favicon_url", "https://mycompany.org/favicon.ico"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "colors.#", "0"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "font.#", "0"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "colors.#", "1"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "font.#", "1"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "universal_login.#", "0"),
),
},
{
Config: testAccTenantAllowsUniversalLoginCustomization + testAccBrandingConfigUpdateAllFields,
Config: testAccBrandingConfigUpdateAllFields,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("auth0_branding.my_brand", "logo_url", "https://mycompany.org/v2/logo.png"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "favicon_url", "https://mycompany.org/favicon.ico"),
Expand All @@ -144,36 +141,36 @@ func TestAccBranding(t *testing.T) {
),
},
{
Config: testAccTenantAllowsUniversalLoginCustomization + testAccBrandingConfigUpdateAgain,
Config: testAccBrandingConfigThrowsAValidationErrorIfUniversalLoginBodyIsEmpty,
ExpectError: regexp.MustCompile("expected \"universal_login.0.body\" to not be an empty string"),
},
{
Config: testAccBrandingConfigRemovesUniversalLoginTemplate,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("auth0_branding.my_brand", "logo_url", "https://mycompany.org/v3/logo.png"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "logo_url", "https://mycompany.org/v1/logo.png"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "favicon_url", "https://mycompany.org/favicon.ico"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "colors.#", "1"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "colors.0.primary", "#0059d6"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "colors.0.page_background", "#000000"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "font.#", "1"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "font.0.url", "https://mycompany.org/font/myfont.ttf"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "universal_login.#", "0"),
),
},
{
Config: testAccBrandingConfigWithOnlyUniversalLogin,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("auth0_branding.my_brand", "universal_login.#", "1"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "universal_login.0.body", "<!DOCTYPE html><html><head>{%- auth0:head -%}</head><body>{%- auth0:widget -%}</body></html>"),
),
},
{
Config: testAccTenantAllowsUniversalLoginCustomization + testAccBrandingConfigReset,
Config: testAccBrandingConfigReset,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("auth0_branding.my_brand", "logo_url", "https://mycompany.org/v1/logo.png"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "favicon_url", "https://mycompany.org/favicon.ico"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "colors.#", "0"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "font.#", "0"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "colors.#", "1"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "font.#", "1"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "universal_login.#", "0"),
),
},
{
Config: testAccTenantAllowsUniversalLoginCustomization + testAccBrandingConfigWithOnlyUniversalLogin,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("auth0_branding.my_brand", "universal_login.#", "1"),
resource.TestCheckResourceAttr("auth0_branding.my_brand", "universal_login.0.body", "<!DOCTYPE html><html><head>{%- auth0:head -%}</head><body>{%- auth0:widget -%}</body></html>"),
),
},
},
})
}
Loading