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.

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

Conversation

mikemountain
Copy link
Contributor

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 chosen OIDC provider.

…eauth

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
@hashicorp-cla
Copy link

hashicorp-cla commented Mar 21, 2023

CLA assistant check
All committers have signed the CLA.

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

Copy link
Contributor

@louisruch louisruch left a comment

Choose a reason for hiding this comment

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

LGTM

@louisruch
Copy link
Contributor

@mikemountain Before you merge this lets have a chat

@mikemountain mikemountain added this to the 0.13.x milestone Mar 22, 2023
@mikemountain mikemountain merged commit 01d322b into main Mar 22, 2023
@mikemountain mikemountain deleted the mikemountain-bugfix-maxage-oidc-reauthenticate branch March 22, 2023 18:52
mikemountain added a commit that referenced this pull request Apr 21, 2023
…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
mikemountain added a commit that referenced this pull request Apr 21, 2023
* fix: Allow users to set OIDC maxAge value to 0 to require immediate reauth (#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

* spelling: host_set_plugin exmaple to example

* Update target.md (#349)

Update docs for Target resource removing `scope_id` from `boundary_host` resources to prevent error

```
╷
│ Error: Unsupported argument
│
│   on main.tf line 79, in resource "boundary_host" "foo":
│   79:   scope_id        = boundary_scope.project.id
│
│ An argument named "scope_id" is not expected here.
```

* feature: add worker_filter option to Boundary Credential Store Vault (#375)

* feature: add worker_filter option to Boundary Credential Store Vault

* Update changelog

* chore: update deps

* small fixes for cherry-picks

* downgrade hclog

---------

Co-authored-by: mocofound <aharness@hashicorp.com>
Co-authored-by: Steven Zamborsky <97125550+stevenzamborsky@users.noreply.github.com>
Co-authored-by: Louis Ruch <louisruch@gmail.com>
grantorchard pushed a commit to grantorchard/terraform-provider-boundary that referenced this pull request Aug 28, 2023
…eauth (hashicorp#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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants