-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Updating Vault docs for JWT support of numeric bound_claims #24921
Conversation
@@ -134,11 +134,11 @@ entities attempting to login. At least one of the bound values must be set. | |||
- `bound_subject` `(string: <optional>)` - If set, requires that the `sub` | |||
claim matches this value. | |||
- `bound_claims` `(map: <optional>)` - If set, a map of claims (keys) to match against respective claim values (values). | |||
The expected value may be a single string or a list of strings. The interpretation of the bound | |||
Each expected value may be a string, integer, boolean or a list of strings. The interpretation of the bound |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I left a list of strings
instead of any other list, e.g. a list of integers. The reason is that the logic in the plugin needs additional refactoring to make it work for a list of integers, and I don't know if there is a use case for it as of now (it actually works for a list of booleans, but that doesn't seem a reasonable use case to me). It wasn't difficult to add support for single-value numeric claims, so we kept the change minimal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, makes sense to me!
claim values is configured with `bound_claims_type`. Keys support [JSON pointer](/vault/docs/auth/jwt#claim-specifications-and-json-pointer) | ||
syntax for referencing claims. | ||
- `bound_claims_type` `(string: "string")` - Configures the interpretation of the bound_claims values. | ||
If `"string"` (the default), the values will treated as string literals and must match exactly. | ||
If `"string"` (the default), the values will be treated as literals and must match exactly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here bound_claims_type
uses 2 kinds: string
and glob
. However, in reality, they are glob
and non-glob
. If the value is an integer or a boolean, the default interpretation (string) will work just fine. That is why I converted string literals
to just literals
. I don't know if this needs expanded explanation in the doc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense to me -- thanks for updating this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+, thanks!
@austingebauer Hey Austin, I just created this PR for updating the docs related to PRs hashicorp/vault-plugin-auth-jwt#160 and hashicorp/vault-plugin-auth-jwt#265. Could you take a look when you have time? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this! This looks great. I'll get this merged.
claim values is configured with `bound_claims_type`. Keys support [JSON pointer](/vault/docs/auth/jwt#claim-specifications-and-json-pointer) | ||
syntax for referencing claims. | ||
- `bound_claims_type` `(string: "string")` - Configures the interpretation of the bound_claims values. | ||
If `"string"` (the default), the values will treated as string literals and must match exactly. | ||
If `"string"` (the default), the values will be treated as literals and must match exactly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense to me -- thanks for updating this!
@@ -339,7 +339,8 @@ This specifies that the value in the JWT claim "division" should be copied to th | |||
"department" claim value will also be copied into metadata but will retain the key name. If a claim is configured in `claim_mappings`, | |||
it must existing in the JWT or else the authentication will fail. | |||
|
|||
Note: the metadata key name "role" is reserved and may not be used for claim mappings. | |||
Note: the metadata key name "role" is reserved and may not be used for claim mappings. Since Vault 1.16 the role name is available | |||
by the key `role` in the alias metadata of the entity after a successful login. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for including this.
Thank you @VioletHynes and @austingebauer! In order to get this merged in, is there anything else I should do? I am not sure about the Vercel check - I couldn't log in with GitHub - it told me "Account not found": |
Nothing left for you to do! I'll handle getting it over the line and fixing any GHA failures. Thanks a bunch :) |
This change updates the documentation in the part related to
bound_claims
in the JWT auth method.The updates on the 2 pages are based on these PRs:
Now,
role
is available on the Entity Alias metadata after a successful login through OIDC/JWT.Also, JWT tokens with numeric claims (as can be seen in an example in the PR) now validate correctly.