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

Add new JWT Auth Role Fields introduced with Vault 1.2 #566

Merged
merged 2 commits into from
Oct 17, 2019

Conversation

lawliet89
Copy link
Contributor

@lawliet89 lawliet89 commented Oct 16, 2019

Fixes #544

I have a couple of options for implementation:

  • Currently, I set the default values for the new fields as indicated by Vault API docs. Since these fields were newly introduced with Vault 1.2, I am not sure if they will cause issues with Vault < 1.2
  • If I don't set the default values, then to "unset" the fields after having set them, the user will have to change the fields to -1 or false respectively. Simply "removing" the fields will cause an infinite diff in plans and Vault does not actually disable them.

Which option should I go for? The former is "cleaner" but might have compatibility issues. The latter is "yucky". @tyrannosaurus-becks @kalafut

@kalafut kalafut self-assigned this Oct 16, 2019
@kalafut
Copy link
Contributor

kalafut commented Oct 16, 2019

@lawliet89 Thanks! The "clean" option will likely be fine but let me double check.

@kalafut
Copy link
Contributor

kalafut commented Oct 16, 2019

@lawliet89 Sending the values to pre-1.2 Vault is fine as they'll be ignored. For the default clock skews, wouldn't you want 0 instead of -1? -1 is explicitly disabling any clock skew.

@lawliet89
Copy link
Contributor Author

lawliet89 commented Oct 17, 2019

For the default clock skews, wouldn't you want 0 instead of -1? -1 is explicitly disabling any clock skew.

I was under the impression that the default is -1 because the docs weren't very clear.

I have changed the defaults to 0.

@kalafut
Copy link
Contributor

kalafut commented Oct 17, 2019

I was under the impression that the default is -1 because the docs weren't very clear.

Yeah that was one of the ickier settings we had to add (well past initial release). There was already a reasonable default skew built in that we needed to retain if the user did nothing or had upgraded, but then others wanted a custom skew or none at all. 😕

@kalafut kalafut merged commit ade20b3 into hashicorp:master Oct 17, 2019
@kalafut
Copy link
Contributor

kalafut commented Oct 17, 2019

Thanks!

@lawliet89 lawliet89 deleted the jwt-roles-keys branch October 18, 2019 01:57
dandandy pushed a commit to dandandy/terraform-provider-vault that referenced this pull request Jun 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add verbose_oidc_logging to vault_jwt_auth_backend_role
2 participants