Skip to content

Commit

Permalink
fix: Allow users to set OIDC maxAge value to 0 to require immediate r…
Browse files Browse the repository at this point in the history
…eauth (#364)

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

Previously, setting 0 as a value for maxAge in an OIDC resource block would not update Boundary, as it was getting treated as a nil value in the provider.
This fix addresses that issue, ensuring that both setting the value to 0 will require immediate reauthorisation, as well as removing the maxAge paramater defaulting the reauth time length to the TTL of the chose OIDC provider

* Update doc strings, fix imports, and run go generate

* add changes to CHANGELOG
  • Loading branch information
mikemountain authored Mar 22, 2023
1 parent 9ad42da commit 01d322b
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 14 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

Canonical reference for changes, improvements, and bugfixes for the Boundary Terraform provider.

## Next

### Bug Fix
* Allow users to set OIDC maxAge value to 0 to require immediate reauth ([PR](https://github.com/hashicorp/terraform-provider-boundary/pull/364))

## 1.1.4 (February 15, 2023)

### New and Improved
Expand Down
4 changes: 2 additions & 2 deletions docs/resources/auth_method_oidc.md
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
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
// 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
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),
},
})
}
Expand Down

0 comments on commit 01d322b

Please sign in to comment.