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

Add session_cookie to tenant #220

Merged
merged 6 commits into from
Jul 12, 2022
Merged

Add session_cookie to tenant #220

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c7a0190
Adding support for session_cookie management including documentation …
Jul 6, 2022
efa5671
Adding to example
Jul 6, 2022
89e2de0
Simplifying flattenTenantSessionCookie logic
Jul 8, 2022
541f4db
Removing extra Computed property, adding test to ensure that clearing…
Jul 8, 2022
6fa3663
Updating to use Go SDK v0.9.0
Jul 12, 2022
63c54ee
Merge branch 'main' into add-tenant-session-cookie
willvedd Jul 12, 2022
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
21 changes: 21 additions & 0 deletions auth0/resource_auth0_tenant.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,25 @@ func newTenant() *schema.Resource {
validation.IsURLWithScheme([]string{"https"}),
),
},
"session_cookie": {
Type: schema.TypeList,
Optional: true,
Computed: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need computed both here and on the mode? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know, do we? Frankly, it's a confusing property that possesses unintuitive behavior; I'm seeing that we set many things to Computed: true so I'm just following a precedent.

Copy link
Contributor

Choose a reason for hiding this comment

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

What we can do is try first the parameters without the computed property. The computed is usually needed if there are weird diffs constantly showing whenever terraform performs a refresh, that have no explanation (my personal heuristic on when to use computed or not).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out the computed property only needs to be enabled on the root object, not necessarily the children, or at least for this example. However, enabling for the child too did not alter behavior.

I'd prefer to not require manual verification testing for these types of additions, so I added a third stage for the TestAccTenant test. It attempts to apply an empty tenant resource block to see if it triggers any odd behavior from Terraform or the Management API. Specifically for this case, it caught a 400 error that occurred when Terrafrom attempted to set the session_cookie to an empty object, something that the Management API prohibits.

Copy link
Contributor

Choose a reason for hiding this comment

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

We're sending an empty object because the expand func is flawed. We can remove the Computed property and refactor the expand as follows:

func expandTenantSessionCookie(d ResourceData) *management.TenantSessionCookie {
	var sessionCookie *management.TenantSessionCookie

	List(d, "session_cookie").Elem(func(d ResourceData) {
		sessionCookie = &management.TenantSessionCookie{Mode: String(d, "mode")}
	})

	return sessionCookie
}

This will make it so that sessionCookie is nil in case it's not present in the config so it will get omitted from the payload because of the omitempty tag.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
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.

The suggested change to refactor expandTenantSessionCookie works, but it does not enable us to remove the Computed property because it fails the "reset" test stage. The plan is not empty afterwards. I'm going to opt to keep it as-is because there is no discernible change in code. Further, the code for that expand function matches the existing precedent set by expandTenantChangePassword, expandTenantGuardianMFAPage, expandTenantErrorPage and expandTenantUniversalLogin.

MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"mode": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: validation.StringInSlice([]string{
"persistent",
"non-persistent",
}, false),
Description: "Behavior of tenant session cookie. Accepts either \"persistent\" or \"non-persistent\"",
},
},
},
},
},
}
}
Expand Down Expand Up @@ -339,6 +358,7 @@ func readTenant(ctx context.Context, d *schema.ResourceData, m interface{}) diag
d.Set("error_page", flattenTenantErrorPage(tenant.ErrorPage)),
d.Set("flags", flattenTenantFlags(tenant.Flags)),
d.Set("universal_login", flattenTenantUniversalLogin(tenant.UniversalLogin)),
d.Set("session_cookie", flattenTenantSessionCookie(tenant.SessionCookie)),
)

return diag.FromErr(result.ErrorOrNil())
Expand Down Expand Up @@ -378,6 +398,7 @@ func expandTenant(d *schema.ResourceData) *management.Tenant {
ErrorPage: expandTenantErrorPage(d),
Flags: expandTenantFlags(d.GetRawConfig().GetAttr("flags")),
UniversalLogin: expandTenantUniversalLogin(d),
SessionCookie: expandTenantSessionCookie(d),
}

return tenant
Expand Down
14 changes: 14 additions & 0 deletions auth0/resource_auth0_tenant_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ func TestAccTenant(t *testing.T) {
resource.TestCheckResourceAttr("auth0_tenant.my_tenant", "universal_login.0.colors.0.primary", "#0059d6"),
resource.TestCheckResourceAttr("auth0_tenant.my_tenant", "universal_login.0.colors.0.page_background", "#000000"),
resource.TestCheckResourceAttr("auth0_tenant.my_tenant", "default_redirection_uri", "https://example.com/login"),
resource.TestCheckResourceAttr("auth0_tenant.my_tenant", "session_cookie.0.mode", "non-persistent"),
),
},
{
Expand All @@ -53,6 +54,13 @@ func TestAccTenant(t *testing.T) {
resource.TestCheckResourceAttr("auth0_tenant.my_tenant", "flags.0.disable_clickjack_protection_headers", "false"),
resource.TestCheckResourceAttr("auth0_tenant.my_tenant", "flags.0.enable_public_signup_user_exists_error", "true"),
resource.TestCheckResourceAttr("auth0_tenant.my_tenant", "flags.0.use_scope_descriptions_for_consent", "false"),
resource.TestCheckResourceAttr("auth0_tenant.my_tenant", "session_cookie.0.mode", "persistent"),
),
},
{
Config: `resource "auth0_tenant" "my_tenant" {}`,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("auth0_tenant.my_tenant", "session_cookie.0.mode", "persistent"),
),
},
},
Expand Down Expand Up @@ -103,6 +111,9 @@ resource "auth0_tenant" "my_tenant" {
}
}
default_redirection_uri = "https://example.com/login"
session_cookie {
mode = "non-persistent"
}
}
`

Expand Down Expand Up @@ -150,6 +161,9 @@ resource "auth0_tenant" "my_tenant" {
}
}
default_redirection_uri = "https://example.com/login"
session_cookie {
mode = "persistent"
}
}
`

Expand Down
17 changes: 17 additions & 0 deletions auth0/structure_auth0_tenant.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,13 @@ func flattenTenantUniversalLogin(universalLogin *management.TenantUniversalLogin
return []interface{}{m}
}

func flattenTenantSessionCookie(sessionCookie *management.TenantSessionCookie) []interface{} {
m := make(map[string]interface{})
m["mode"] = sessionCookie.GetMode()

return []interface{}{m}
}

func expandTenantChangePassword(d ResourceData) *management.TenantChangePassword {
var changePassword management.TenantChangePassword

Expand Down Expand Up @@ -176,3 +183,13 @@ func expandTenantUniversalLogin(d ResourceData) *management.TenantUniversalLogin

return &universalLogin
}

func expandTenantSessionCookie(d ResourceData) *management.TenantSessionCookie {
var sessionCookie management.TenantSessionCookie

List(d, "session_cookie").Elem(func(d ResourceData) {
sessionCookie.Mode = String(d, "mode")
})

return &sessionCookie
}
Comment on lines +187 to +195
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func expandTenantSessionCookie(d ResourceData) *management.TenantSessionCookie {
var sessionCookie management.TenantSessionCookie
List(d, "session_cookie").Elem(func(d ResourceData) {
sessionCookie.Mode = String(d, "mode")
})
return &sessionCookie
}
func expandTenantSessionCookie(d ResourceData) *management.TenantSessionCookie {
var sessionCookie *management.TenantSessionCookie
List(d, "session_cookie").Elem(func(d ResourceData) {
sessionCookie = &management.TenantSessionCookie{Mode: String(d, "mode")}
})
return sessionCookie
}

148 changes: 118 additions & 30 deletions auth0/testdata/recordings/TestAccTenant.yaml
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions docs/resources/tenant.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ resource "auth0_tenant" "tenant" {
]
session_lifetime = 46000
sandbox_version = "8"
session_cookie {
mode = "non-persistent"
}
}
```

Expand All @@ -73,6 +76,7 @@ Arguments accepted by this resource include:
* `flags` - (Optional) List(Resource). Configuration settings for tenant flags. For details, see [Flags](#flags).
* `universal_login` - (Optional) List(Resource). Configuration settings for Universal Login. For details, see [Universal Login](#universal-login).
* `default_redirection_uri` - (Optional) String. The default absolute redirection uri, must be https and cannot contain a fragment.
* `session_cookie` - (Optional) List(Resource). Alters behavior of tenant's session cookie. Contains a single `mode` property that accepts two values: `"persistent"` or `"non-persistent"`.

### Change Password Page

Expand Down
4 changes: 4 additions & 0 deletions example/tenant/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,8 @@ resource "auth0_tenant" "tenant" {
session_lifetime = 8760
sandbox_version = "8"
enabled_locales = ["en"]

session_cookie {
mode = "non-persistent"
}
}
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module github.com/auth0/terraform-provider-auth0
go 1.18

require (
github.com/auth0/go-auth0 v0.8.0
github.com/auth0/go-auth0 v0.0.0-20220706202431-cce36896b06c
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's take @Widcket's suggestion to keep these kind of PRs in Draft mode till we make a release in go-auth0 SDK, wdyt? So we can easily differentiate between the ones that can be immediately merged and these that shouldn't yet.

github.com/dnaeon/go-vcr/v2 v2.0.1
github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320
github.com/hashicorp/go-multierror v1.1.1
Expand Down
8 changes: 2 additions & 6 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ github.com/apparentlymart/go-textseg/v12 v12.0.0/go.mod h1:S/4uRK2UtaQttw1GenVJE
github.com/apparentlymart/go-textseg/v13 v13.0.0 h1:Y+KvPE1NYz0xl601PVImeQfFyEy6iT90AvPUL1NNfNw=
github.com/apparentlymart/go-textseg/v13 v13.0.0/go.mod h1:ZK2fH7c4NqDTLtiYLvIkEghdlcqw7yxLeM89kiTRPUo=
github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs=
github.com/auth0/go-auth0 v0.8.0 h1:x5Z6q++NUtdaSyzoHMaMRjH/SpiWS5rLTQ8ZX0AsRRE=
github.com/auth0/go-auth0 v0.8.0/go.mod h1:kxxGNgF592VPvWSNPVWNbmWHs+R9c0MZCTuW+T3NgZw=
github.com/auth0/go-auth0 v0.0.0-20220706202431-cce36896b06c h1:Yen1xN1GTlL55p5T526dUpOlKoo9jteXCg2dh4GsX70=
github.com/auth0/go-auth0 v0.0.0-20220706202431-cce36896b06c/go.mod h1:kxxGNgF592VPvWSNPVWNbmWHs+R9c0MZCTuW+T3NgZw=
github.com/aybabtme/iocontrol v0.0.0-20150809002002-ad15bcfc95a0 h1:0NmehRCgyk5rljDQLKUO+cRJCnduDyn11+zGZIc9Z48=
github.com/aybabtme/iocontrol v0.0.0-20150809002002-ad15bcfc95a0/go.mod h1:6L7zgvqo0idzI7IO8de6ZC051AfXb5ipkIJ7bIA2tGA=
github.com/benbjohnson/clock v1.1.0 h1:Q92kusRqC1XV2MjkWETPvjJVqKetz1OzxZB7mHJLju8=
Expand Down Expand Up @@ -323,8 +323,6 @@ golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPh
golang.org/x/crypto v0.0.0-20210322153248-0c34fe9e7dc2/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4=
golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4=
golang.org/x/crypto v0.0.0-20210616213533-5ff15b29337e/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
golang.org/x/crypto v0.0.0-20220518034528-6f7dac969898 h1:SLP7Q4Di66FONjDJbCYrCRrh97focO6sLogHO7/g8F0=
golang.org/x/crypto v0.0.0-20220518034528-6f7dac969898/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/crypto v0.0.0-20220622213112-05595931fe9d h1:sK3txAijHtOK88l68nt020reeT1ZdKLIYetKl95FzVY=
golang.org/x/crypto v0.0.0-20220622213112-05595931fe9d/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
Expand Down Expand Up @@ -395,8 +393,6 @@ golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96b
golang.org/x/net v0.0.0-20210428140749-89ef3d95e781/go.mod h1:OJAsFXCWl8Ukc7SiCT/9KSuxbyM7479/AVlXFRxuMCk=
golang.org/x/net v0.0.0-20210510120150-4163338589ed/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
golang.org/x/net v0.0.0-20220520000938-2e3eb7b945c2 h1:NWy5+hlRbC7HK+PmcXVUmW1IMyFce7to56IUvhUFm7Y=
golang.org/x/net v0.0.0-20220520000938-2e3eb7b945c2/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
golang.org/x/net v0.0.0-20220630215102-69896b714898 h1:K7wO6V1IrczY9QOQ2WkVpw4JQSwCd52UsxVEirZUfiw=
golang.org/x/net v0.0.0-20220630215102-69896b714898/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
Expand Down