-
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
Add new optional OIDCDisableUserInfo setting for OIDC auth provider #19566
Conversation
Hi @tgross! |
@eumikhailov I've been away for the holidays and haven't had a chance to review this yet. But I'll need you to sign the CLA before I can do so. |
@tgross happy new year! 😃 |
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.
Hi @eumikhailov I've left a comment in #19318 (comment) which we should probably discuss further before spending more time on this PR.
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.
@eumikhailov I don't have an environment I can test this in. Have you verified that this actually works with ADFS?
@tgross tested with Microsoft AD FS 4.0 and it's work!
home/wsl/nomad# ./nomad version
Nomad v1.7.3-dev
....
Revision 8b4301ddbb404b581a8ee536db9793360c45f8aa+CHANGES
home/wsl/nomad# ./nomad acl auth-method info microsoft-oidc
Name = microsoft-oidc
Type = OIDC
Locality = global
Max Token TTL = 1h0m0s
Token Name Format = ${auth_method_type}-${auth_method_name}
Default = false
Create Index = 34
Modify Index = 36
Auth Method Config
JWT Validation Public Keys = <none>
JWKS URL = <none>
OIDC Discovery URL = ***/adfs
OIDC Client ID = de2658c8-cfbc-4d65-9b5e-bc3e85d1552d
OIDC Client Secret = <none>
OIDC Disable UserInfo = true
OIDC Scopes = https://nomad.***/openid,openid
Bound audiences = de2658c8-cfbc-4d65-9b5e-bc3e85d1552d,https://nomad.***
Bound issuer = <none>
Allowed redirects URIs = http://localhost:4646/ui/settings/tokens
Discovery CA pem = <none>
JWKS CA cert = <none>
Signing algorithms = <none>
Expiration Leeway = 0s
NotBefore Leeway = 0s
ClockSkew Leeway = 0s
Claim mappings = {upn: upn}
List claim mappings = {roles: roles}
|
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.
Ok @eumikhailov I've tested this out as described in #19318 (comment). I only need two more things from you:
- Can you rebase this on
main
to resolve the merge conflict? - Can you run
make cl
to add a changelog entry?
Once that's done, I'll merge this and get it backported. Thanks!
@tgross OK, fixed! |
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! I'll merge once CI is green, and make sure the backports happen.
In nomad 1.7.3 a new config has been [introduced](hashicorp/nomad#19566) Adding support for this config parameter
In nomad 1.7.3 a new config has been [introduced](hashicorp/nomad#19566) Adding support for this config parameter
In nomad 1.7.3 a new config has been [introduced](hashicorp/nomad#19566) Adding support for this config parameter
In nomad 1.7.3 a new config has been [introduced](hashicorp/nomad#19566) Adding support for this config parameter
Add new optional OIDCDisableUserInfo setting for OIDC auth provider which disables a request to the identity provider to get OIDC UserInfo.
This options is helpful when your identity provider doesn't send any additional claims from the UserInfo endpoint, such as
Microsoft AD FS OIDC Provider:
Fixes #19318