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

fix: Allow users to set OIDC maxAge value to 0 to require immediate reauth #364

Merged
merged 3 commits into from
Mar 22, 2023
Merged

fix: Allow users to set OIDC maxAge value to 0 to require immediate reauth #364

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
542e5eb
fix: Allow users to set OIDC maxAge value to 0 to require immediate r…
mikemountain Mar 17, 2023
3114cc1
Update doc strings, fix imports, and run go generate
mikemountain Mar 21, 2023
b89e404
add changes to CHANGELOG
mikemountain Mar 22, 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/auth_method_oidc.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ The OIDC auth method resource allows you to configure a Boundary auth_method_oid
- `description` (String) The auth method description.
- `disable_discovered_config_validation` (Boolean) Disables validation logic ensuring that the OIDC provider's information from its discovery endpoint matches the information here. The validation is only performed at create or update time.
- `idp_ca_certs` (List of String) A list of CA certificates to trust when validating the IdP's token signatures.
- `is_primary_for_scope` (Boolean) When true, makes this auth method the primary auth method for the scope in which it resides. The primary auth method for a scope means the the user will be automatically created when they login using an OIDC account.
- `is_primary_for_scope` (Boolean) When true, makes this auth method the primary auth method for the scope in which it resides. The primary auth method for a scope means the user will be automatically created when they login using an OIDC account.
- `issuer` (String) The issuer corresponding to the provider, which must match the issuer field in generated tokens.
- `max_age` (Number) The max age to provide to the provider, indicating how much time is allowed to have passed since the last authentication before the user is challenged again.
- `max_age` (Number) The max age to provide to the provider, indicating how much time is allowed to have passed since the last authentication before the user is challenged again. A value of 0 sets an immediate requirement for all users to reauthenticate, and an unset maxAge results in a Terraform value of -1 and the default TTL of the chosen OIDC will be used.
- `name` (String) The auth method name. Defaults to the resource name.
- `signing_algorithms` (List of String) Allowed signing algorithms for the provider's issued tokens.
- `state` (String) Can be one of 'inactive', 'active-private', or 'active-public'. Currently automatically set to active-public.
Expand Down
22 changes: 16 additions & 6 deletions internal/provider/resource_auth_method_oidc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,11 @@ func resourceAuthMethodOidc() *schema.Resource {
Optional: true,
},
authmethodOidcMaxAgeKey: {
Description: "The max age to provide to the provider, indicating how much time is allowed to have passed since the last authentication before the user is challenged again.",
Type: schema.TypeInt,
Optional: true,
Description: `The max age to provide to the provider, indicating how much time is allowed to have passed since the last authentication before the user is challenged again. ` +
`A value of 0 sets an immediate requirement for all users to reauthenticate, and an unset maxAge results in a Terraform value of -1 and the default TTL of the chosen OIDC will be used.`,
Type: schema.TypeInt,
Optional: true,
Default: -1,
},
authmethodOidcSigningAlgorithmsKey: {
Description: "Allowed signing algorithms for the provider's issued tokens.",
Expand Down Expand Up @@ -166,7 +168,7 @@ func resourceAuthMethodOidc() *schema.Resource {
Computed: true,
},
authmethodOidcIsPrimaryAuthMethodForScope: {
Description: "When true, makes this auth method the primary auth method for the scope in which it resides. The primary auth method for a scope means the the user will be automatically created when they login using an OIDC account.",
Description: "When true, makes this auth method the primary auth method for the scope in which it resides. The primary auth method for a scope means the user will be automatically created when they login using an OIDC account.",
Type: schema.TypeBool,
Optional: true,
},
Expand Down Expand Up @@ -267,8 +269,12 @@ func resourceAuthMethodOidcCreate(ctx context.Context, d *schema.ResourceData, m
if clientSecret, ok := d.GetOk(authmethodOidcClientSecretKey); ok {
opts = append(opts, authmethods.WithOidcAuthMethodClientSecret(clientSecret.(string)))
}
if maxAge, ok := d.GetOk(authmethodOidcMaxAgeKey); ok {
// null values are not correctly recognized by the Terraform SDK, so we instead check here for maxAge value
johanbrandhorst marked this conversation as resolved.
Show resolved Hide resolved
// if maxAge is unset it will default and set the terraform state to -1 and clear the maxAge param in Boundary
if maxAge, _ := d.GetOk(authmethodOidcMaxAgeKey); maxAge.(int) >= 0 {
opts = append(opts, authmethods.WithOidcAuthMethodMaxAge(uint32(maxAge.(int))))
} else {
opts = append(opts, authmethods.DefaultOidcAuthMethodMaxAge())
}
if prefix, ok := d.GetOk(authmethodOidcApiUrlPrefixKey); ok {
opts = append(opts, authmethods.WithOidcAuthMethodApiUrlPrefix(prefix.(string)))
Expand Down Expand Up @@ -471,8 +477,12 @@ func resourceAuthMethodOidcUpdate(ctx context.Context, d *schema.ResourceData, m
}
}
if d.HasChange(authmethodOidcMaxAgeKey) {
if maxAge, ok := d.GetOk(authmethodOidcMaxAgeKey); ok {
// null values are not correctly recognized by the Terraform SDK, so we instead check here for maxAge value
// if maxAge is unset it will default and set the terraform state to -1 and clear the maxAge param in Boundary
if maxAge, _ := d.GetOk(authmethodOidcMaxAgeKey); maxAge.(int) >= 0 {
opts = append(opts, authmethods.WithOidcAuthMethodMaxAge(uint32(maxAge.(int))))
} else {
opts = append(opts, authmethods.DefaultOidcAuthMethodMaxAge())
}
}
if d.HasChange(authmethodOidcSigningAlgorithmsKey) {
Expand Down
11 changes: 5 additions & 6 deletions internal/provider/resource_auth_method_oidc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ resource "boundary_auth_method_oidc" "foo" {
issuer = "%s"
client_id = "foo_id"
client_secret = "foo_secret"
max_age = 10
max_age = 0
api_url_prefix = "http://localhost:9200"
idp_ca_certs = [
<<EOT
Expand All @@ -80,7 +80,6 @@ resource "boundary_auth_method_oidc" "foo" {
issuer = "https://test-update.com"
client_id = "foo_id_update"
client_secret = "foo_secret_update"
max_age = 1
api_url_prefix = "http://localhost:9200"
idp_ca_certs = [
<<EOT
Expand All @@ -92,7 +91,7 @@ EOT
account_claim_maps = ["oid=sub"]
claims_scopes = ["profile"]

// we need to disable this validatin, since the updated issuer isn't discoverable
// we need to disable this validation, since the updated issuer isn't discoverable
disable_discovered_config_validation = true
}`
)
Expand Down Expand Up @@ -120,12 +119,12 @@ func TestAccAuthMethodOidc(t *testing.T) {
resource.TestCheckResourceAttr("boundary_auth_method_oidc.foo", "name", "test"),
resource.TestCheckResourceAttr("boundary_auth_method_oidc.foo", authmethodOidcIssuerKey, tp.Addr()),
resource.TestCheckResourceAttr("boundary_auth_method_oidc.foo", authmethodOidcClientIdKey, "foo_id"),
resource.TestCheckResourceAttr("boundary_auth_method_oidc.foo", authmethodOidcMaxAgeKey, "0"),
testAccCheckAuthMethodAttrAryValueSet(provider, "boundary_auth_method_oidc.foo", authmethodOidcIdpCaCertsKey, []string{tpCert}),
testAccCheckAuthMethodAttrAryValueSet(provider, "boundary_auth_method_oidc.foo", authmethodOidcAllowedAudiencesKey, []string{"foo_aud"}),
testAccCheckAuthMethodAttrAryValueSet(provider, "boundary_auth_method_oidc.foo", authmethodOidcSigningAlgorithmsKey, []string{"ES256"}),
testAccCheckAuthMethodAttrAryValueSet(provider, "boundary_auth_method_oidc.foo", authmethodOidcAccountClaimMapsKey, []string{"oid=sub"}),
testAccCheckAuthMethodAttrAryValueSet(provider, "boundary_auth_method_oidc.foo", authmethodOidcClaimsScopesKey, []string{"profile"}),
resource.TestCheckResourceAttr("boundary_auth_method_oidc.foo", authmethodOidcMaxAgeKey, "10"),
testAccCheckAuthMethodResourceExists(provider, "boundary_auth_method_oidc.foo"),
testAccIsPrimaryForScope(provider, "boundary_auth_method_oidc.foo", false),
),
Expand All @@ -139,15 +138,15 @@ func TestAccAuthMethodOidc(t *testing.T) {
resource.TestCheckResourceAttr("boundary_auth_method_oidc.foo", "name", "test"),
resource.TestCheckResourceAttr("boundary_auth_method_oidc.foo", authmethodOidcIssuerKey, "https://test-update.com"),
resource.TestCheckResourceAttr("boundary_auth_method_oidc.foo", authmethodOidcClientIdKey, "foo_id_update"),
resource.TestCheckResourceAttr("boundary_auth_method_oidc.foo", authmethodOidcMaxAgeKey, "1"),
resource.TestCheckResourceAttr("boundary_auth_method_oidc.foo", authmethodOidcMaxAgeKey, "-1"),
testAccCheckAuthMethodAttrAryValueSet(provider, "boundary_auth_method_oidc.foo", authmethodOidcIdpCaCertsKey, []string{fooAuthMethodOidcCaCerts}),
testAccCheckAuthMethodAttrAryValueSet(provider, "boundary_auth_method_oidc.foo", authmethodOidcAllowedAudiencesKey, []string{"foo_aud_update"}),
testAccCheckAuthMethodResourceExists(provider, "boundary_auth_method_oidc.foo"),
testAccIsPrimaryForScope(provider, "boundary_auth_method_oidc.foo", true),
testAccCheckAuthMethodResourceExists(provider, "boundary_auth_method_oidc.foo"),
),
},
importStep("boundary_auth_method_oidc.foo", "client_secret", "is_primary_for_scope"),
importStep("boundary_auth_method_oidc.foo", "client_secret", "is_primary_for_scope", authmethodOidcMaxAgeKey),
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 workaround unfortunately has to keep a -1 in the Terraform state, which isn't reflected in Boundary (as it gets removed). Went with ignoring checking for that difference, with checking that the -1 does exist as an attribute above in the tests

},
})
}
Expand Down