JWT Auth method Role Creation - Set bound_claims values to be as per Vault format

ram-parameswaran commented 9 months ago

Description

Initially raised by(on behalf of) ent customer via Zendesk in an enterprise support engagement.

Update jwtAuthBackendRoleDataToWrite function to set the bound_claims values as a map of string:string instead of string:string[]

Checklist

[x] Added CHANGELOG entry

Output from acceptance testing:

resource "vault_jwt_auth_backend_role" "jwt-test-role" {
  role_name                     = "jwt-test-role"
  role_type                       = "jwt"
  token_explicit_max_ttl = "60"
  user_claim                     = "user_email"
  bound_claims_type      = "string"
  token_policies               = ["jwt-role-test-policy"]
  bound_claims                = {
       project_name = "jwt-role-test"
       branch = "jwt-role-test-branch"
  }
}

$ vault read auth/jwt/role/jwt-test-role
Key                        Value
---                        -----
allowed_redirect_uris      <nil>
bound_audiences            []
bound_claims               map[project_name:jwt-role-test branch:jwt-role-test-branch]
bound_claims_type          string
bound_subject              n/a
claim_mappings             <nil>
clock_skew_leeway          0
expiration_leeway          0
groups_claim               n/a
max_age                    0
not_before_leeway          0
oidc_scopes                <nil>
role_type                  jwt
token_bound_cidrs          []
token_explicit_max_ttl     1m
token_max_ttl              0s
token_no_default_policy    false
token_num_uses             0
token_period               0s
token_policies             [jwt-role-test-policy]
token_ttl                  0s
token_type                 default
user_claim                 user_email
user_claim_json_pointer    false
verbose_oidc_logging       false

resource "vault_jwt_auth_backend_role" "jwt-test-role" {
  role_name                      = "jwt-test-role"
  role_type                        = "jwt"
  token_explicit_max_ttl  = "60"
  bound_audiences          = ["https://myco.test"]
  user_claim                      = "user_email"
  token_policies                = ["jwt-role-test-policy"]
}

$ vault read auth/jwt/role/jwt-test-role
Key                        Value
---                        -----
allowed_redirect_uris      <nil>
bound_audiences            [https://myco.test]
bound_claims               <nil>
bound_claims_type          string
bound_subject              n/a
claim_mappings             <nil>
clock_skew_leeway          0
expiration_leeway          0
groups_claim               n/a
max_age                    0
not_before_leeway          0
oidc_scopes                <nil>
role_type                  jwt
token_bound_cidrs          []
token_explicit_max_ttl     1m
token_max_ttl              0s
token_no_default_policy    false
token_num_uses             0
token_period               0s
token_policies             [jwt-role-test-policy]
token_ttl                  0s
token_type                 default
user_claim                 user_email
user_claim_json_pointer    false
verbose_oidc_logging       false

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

ram-parameswaran commented 7 months ago

Thanks for the PR @ram-parameswaran !

Question: Will this change be backwards compatible with existing state files? i.e. What happens when an apply is run without this change and then another apply is run with this change? @fairclothjm tests show my changes are backward compatible. Let me know if any other changes needed.

fairclothjm commented 6 months ago

Thanks @ram-parameswaran, I have thought about this some more and taken a look at the history of this resource. This will be a breaking change. See https://github.com/hashicorp/terraform-provider-vault/issues/956 which this change will no longer support.

Instead of modifying the behavior of bound_claims, which seems to be a moving target, I propose we add a new field bound_claims_json that allows passing a string containing a JSON-encoded object. For example,

resource "vault_jwt_auth_backend_role" "jwt-gitlabcorp-ansible-test-plays-7922" {
  # ...
  bound_claims_json = jsonencode(
    {
      zip = "zap",
      foo = "bar"
    }
  )
}

This would be similar to vault_kv_secret_v2's data_json. And like that resource, this new field would allow any arbitrary json structure. What do you think @ram-parameswaran?

ram-parameswaran commented 6 months ago

Thanks @ram-parameswaran, I have thought about this some more and taken a look at the history of this resource. This will be a breaking change. See #956 which this change will no longer support.

Instead of modifying the behavior of bound_claims, which seems to be a moving target, I propose we add a new field bound_claims_json that allows passing a string containing a JSON-encoded object. For example,
resource "vault_jwt_auth_backend_role" "jwt-gitlabcorp-ansible-test-plays-7922" {
  # ...
  bound_claims_json = jsonencode(
    {
      zip = "zap",
      foo = "bar"
    }
  )
}
This would be similar to vault_kv_secret_v2's data_json. And like that resource, this new field would allow any arbitrary json structure. What do you think @ram-parameswaran?

@fairclothjm i agree! this is a much cleaner approach and makes it a more cleaner implementation.

hashicorp / terraform-provider-vault