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 optional OIDCDisableUserInfo setting for OIDC auth provider #19566

Merged
merged 2 commits into from
Jan 9, 2024
Merged

Add new optional OIDCDisableUserInfo setting for OIDC auth provider #19566

merged 2 commits into from
Jan 9, 2024

Conversation

eumikhailov
Copy link
Contributor

@eumikhailov eumikhailov commented Dec 29, 2023

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:

The AD FS UserInfo endpoint always returns the subject claim as specified in the OpenID standards. AD FS doesn't support additional claims requested via the UserInfo endpoint

Fixes #19318

@hashicorp-cla
Copy link

hashicorp-cla commented Dec 29, 2023

CLA assistant check
All committers have signed the CLA.

@eumikhailov
Copy link
Contributor Author

Hi @tgross!
Please take a look :)

@tgross
Copy link
Member

tgross commented Jan 2, 2024

@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.

@eumikhailov
Copy link
Contributor Author

@tgross happy new year! 😃
PS I've signed CLA

Copy link
Member

@tgross tgross left a 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.

website/content/api-docs/acl/auth-methods.mdx Outdated Show resolved Hide resolved
Copy link
Member

@tgross tgross left a 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?

@eumikhailov
Copy link
Contributor Author

eumikhailov commented Jan 3, 2024

@tgross tested with Microsoft AD FS 4.0 and it's work!

  1. Build up Nomad
home/wsl/nomad# ./nomad version
Nomad v1.7.3-dev
....
Revision 8b4301ddbb404b581a8ee536db9793360c45f8aa+CHANGES
  1. Setup oidc
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}
  1. Setup binding rule
home/wsl/nomad# ./nomad acl binding-rule info 743327e9-117a-3041-7e1c-718df4516900
ID           = 743327e9-117a-3041-7e1c-718df4516900
Description  = <none>
Auth Method  = microsoft-oidc
Selector     = "\"role-mgmt\" in list.roles"
Bind Type    = management
Bind Name    = <none>
Create Time  = ***
Modify Time  = ***
Create Index = 42
Modify Index = 42
  1. Logon in UI works fine
    image

@eumikhailov eumikhailov marked this pull request as ready for review January 3, 2024 16:49
Copy link
Member

@tgross tgross left a 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!

@vercel vercel bot requested a deployment to Preview – nomad-storybook-and-ui January 9, 2024 18:08 Abandoned
@eumikhailov
Copy link
Contributor Author

@tgross OK, fixed!

Copy link
Member

@tgross tgross left a 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.

netdata-be added a commit to netdata-be/terraform-provider-nomad that referenced this pull request Feb 7, 2024
In nomad 1.7.3 a new config has been [introduced](hashicorp/nomad#19566)
Adding support for this config parameter
netdata-be added a commit to netdata-be/terraform-provider-nomad that referenced this pull request Feb 7, 2024
In nomad 1.7.3 a new config has been [introduced](hashicorp/nomad#19566)
Adding support for this config parameter
lgfa29 pushed a commit to netdata-be/terraform-provider-nomad that referenced this pull request Feb 8, 2024
In nomad 1.7.3 a new config has been [introduced](hashicorp/nomad#19566)
Adding support for this config parameter
lgfa29 pushed a commit to hashicorp/terraform-provider-nomad that referenced this pull request Feb 8, 2024
In nomad 1.7.3 a new config has been [introduced](hashicorp/nomad#19566)
Adding support for this config parameter
nvanthao pushed a commit to nvanthao/nomad that referenced this pull request Mar 1, 2024
…ovider (hashicorp#19566)

Add new optional `OIDCDisableUserInfo` setting for OIDC auth provider which
disables a request to the identity provider to get OIDC UserInfo.

This option is helpful when your identity provider doesn't send any additional
claims from the UserInfo endpoint, such as Microsoft AD FS OIDC Provider:

> The AD FS UserInfo endpoint always returns the subject claim as specified in the
> OpenID standards. AD FS doesn't support additional claims requested via the
> UserInfo endpoint

Fixes hashicorp#19318
nvanthao pushed a commit to nvanthao/nomad that referenced this pull request Mar 1, 2024
…ovider (hashicorp#19566)

Add new optional `OIDCDisableUserInfo` setting for OIDC auth provider which
disables a request to the identity provider to get OIDC UserInfo.

This option is helpful when your identity provider doesn't send any additional
claims from the UserInfo endpoint, such as Microsoft AD FS OIDC Provider:

> The AD FS UserInfo endpoint always returns the subject claim as specified in the
> OpenID standards. AD FS doesn't support additional claims requested via the
> UserInfo endpoint

Fixes hashicorp#19318
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.5.x backport to 1.5.x release line backport/1.6.x backport to 1.6.x release line backport/1.7.x backport to 1.7.x release line theme/auth
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OIDC: add resource url-parameter for self-hosted ADFS
3 participants