-
Notifications
You must be signed in to change notification settings - Fork 2k
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
acl: JWT as SSO auth method #15897
acl: JWT as SSO auth method #15897
Conversation
dc70538
to
519c746
Compare
5edd5d4
to
f4c7b09
Compare
e76b325
to
b526bac
Compare
Ember Asset Size actionAs of 696d4d4 Files that got Bigger 🚨:
Files that stayed the same size 🤷:
|
Ember Test Audit comparison
|
* Bones of JWT detection * JWT to token pipeline complete * Some live-demo fixes for template language * findSelf and loginJWT funcs made async * Acceptance tests and mirage mocks for JWT login * [ui] Allow for multiple JWT auth methods in the UI (#16665) * Split selectable jwt methods * repositions the dropdown to be next to the input field
6959151
to
f4e0056
Compare
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.
LGTM!
// Measure the OIDC endpoint performance. | ||
defer metrics.MeasureSince([]string{"nomad", "acl", "jwt", "oidc_jwt"}, time.Now()) | ||
|
||
// TODO why do we have DiscoverCaPem as an array but JWKSCaPem as a single string? |
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.
Just a quick check that this TODO is ok to leave before merging?
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.
Right, this is "a note for future us." Something we'll definitely revisit.
nomad/acl_endpoint.go
Outdated
defer metrics.MeasureSince([]string{"nomad", "acl", "login"}, time.Now()) | ||
|
||
// This endpoint can only be used once all servers in all federated regions | ||
// have been upgraded to 1.5.2 or greater, since JWT Auth method was |
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.
Should this read 1.5.3? Or should we just remove the specific version here and let that live in the minACLJWTAuthMethodVersion
constant so we don't need to update it if we have to ship a security update or whatever before this goes out?
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.
Good call, I'll adjust.
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.
See 696d4d4 for details.
This PR introduces a new ACL Auth Method: JWT, which allows users to exchange 3rd party JSON Web Tokens for Nomad ACL Tokens. We achieve this by:
ACL.Login
/v1/acl/login
-type=JWT
when creating and updating auth methods, and to thenomad login
commandAll the changes mentioned above were described in NMD-167, and all the commits in this pull request (except for changelog) were previously reviewed.