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

Indicate that max_token_ttl is required for OIDC auth methods #360

Merged
merged 8 commits into from
Oct 8, 2023

Conversation

am-ak
Copy link
Contributor

@am-ak am-ak commented Oct 4, 2023

  • Include an example for OIDC auth method
  • correct the description of max_token_ttl that it is a required argument for OIDC auth method

- Include an example for OIDC auth method
- correct the description of max_token_ttl that it is a required argument for OIDC auth method
Comment on lines 49 to 73
Deine an `OIDC` auth method:
```hcl
resource "consul_acl_auth_method" "oidc" {
name = "auth0"
type = "oidc"
max_token_ttl = "5m"
config_json = jsonencode({
"OIDCDiscoveryURL": "https://<AUTH0_DOMAIN>/",
"OIDCClientID": "<AUTH0_CLIENT_ID>",
"OIDCClientSecret": "<AUTH0_CLIENT_SECRET>",
"BoundAudiences": ["<AUTH0_CLIENT_ID>"],
"AllowedRedirectURIs": [
"http://localhost:8550/oidc/callback",
"http://localhost:8500/ui/oidc/callback"
],
"ClaimMappings": {
"http://consul.internal/first_name": "first_name",
"http://consul.internal/last_name": "last_name"
},
"ListClaimMappings": {
"http://consul.internal/groups": "groups"
}
})
}
```

Choose a reason for hiding this comment

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

Suggested change
Deine an `OIDC` auth method:
```hcl
resource "consul_acl_auth_method" "oidc" {
name = "auth0"
type = "oidc"
max_token_ttl = "5m"
config_json = jsonencode({
"OIDCDiscoveryURL": "https://<AUTH0_DOMAIN>/",
"OIDCClientID": "<AUTH0_CLIENT_ID>",
"OIDCClientSecret": "<AUTH0_CLIENT_SECRET>",
"BoundAudiences": ["<AUTH0_CLIENT_ID>"],
"AllowedRedirectURIs": [
"http://localhost:8550/oidc/callback",
"http://localhost:8500/ui/oidc/callback"
],
"ClaimMappings": {
"http://consul.internal/first_name": "first_name",
"http://consul.internal/last_name": "last_name"
},
"ListClaimMappings": {
"http://consul.internal/groups": "groups"
}
})
}
```
Define an `OIDC` auth method:
```hcl
resource "consul_acl_auth_method" "oidc" {
name = "auth0"
type = "oidc"
max_token_ttl = "5m"
config_json = jsonencode({
"OIDCDiscoveryURL": "https://<AUTH0_DOMAIN>/",
"OIDCClientID": "<AUTH0_CLIENT_ID>",
"OIDCClientSecret": "<AUTH0_CLIENT_SECRET>",
"BoundAudiences": ["<AUTH0_CLIENT_ID>"],
"AllowedRedirectURIs": [
"http://localhost:8550/oidc/callback",
"http://localhost:8500/ui/oidc/callback"
],
"ClaimMappings": {
"http://consul.internal/first_name": "first_name",
"http://consul.internal/last_name": "last_name"
},
"ListClaimMappings": {
"http://consul.internal/groups": "groups"
}
})
}
```

docs/resources/acl_auth_method.md Outdated Show resolved Hide resolved
@remilapeyre
Copy link
Collaborator

Hi @am-ak, thank you very much for reporting this issue. I had missed this when working on consul_acl_auth_mode, it is pretty rare that arguments can be optional or required based on another one.

I kept most of your implementation but migrated the consul_acl_auth_method documentation to the tfplugindocs framework as this is an ongoing migration that I do as we update the documentation of each resources. This is what most of my commits are about, sorry for the noise.

I also updated the example in the documentation to use the one in the Consul documentation.

Thanks for your contribution!

@remilapeyre remilapeyre changed the title Update acl_auth_method.md Indicate that max_token_ttl is required for OIDC auth methods Oct 8, 2023
@remilapeyre remilapeyre merged commit e13f206 into hashicorp:master Oct 8, 2023
4 checks passed
remilapeyre added a commit to absolutelightning/terraform-provider-consul that referenced this pull request Oct 9, 2023
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