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

ADFS 4.0 groups claim could not be converted to list #62

Closed
ljupchokotev opened this issue Jul 26, 2019 · 2 comments · Fixed by #63
Closed

ADFS 4.0 groups claim could not be converted to list #62

ljupchokotev opened this issue Jul 26, 2019 · 2 comments · Fixed by #63
Assignees

Comments

@ljupchokotev
Copy link

We have OIDC configured to work with ADFS 4.0 and we are hitting an issue when we want to use groups_claim. What happens is that ADFS issues the groups claim as string if there is only one group that the user belongs to. If there are multiple groups, then it sends the groups claim as an array which Vault accepts.

We are hitting the following error: https://github.com/hashicorp/vault-plugin-auth-jwt/blob/master/path_login.go#L345 because Vault expects the groups claim to always be an array.

I tried to force ADFS to send the groups claim as an array even if there is only one group, but I couldn't figure out how to do it.

Is this something that can be fixed on Vault's side?

@kalafut kalafut self-assigned this Jul 26, 2019
@kalafut
Copy link
Contributor

kalafut commented Jul 26, 2019

Certainly not the first instance of this, and I'm a little surprised how often this claim is coming back from IdPs as a different type depending on the length. Adding some flexibility to the parsing is something we'll being investigating soon, and have already added when parsing bound_claims for similar reasons.

In the mean time, can you possibly add membership to a dummy group on the ADFS side, thus forcing it to output a list? Pretty hacky, I know...

@ljupchokotev
Copy link
Author

We already did something similar as a workaround. Instead of creating a dummy group we just have a custom claim rule which issues a dummy value on the same claim which forces it to have at least two values and then ADFS formats that as an array.

It would be nice to handle this type of thing on Vault's side though.

kalafut pushed a commit that referenced this issue Jul 26, 2019
This also refactors a crazy long test function signature into
a config structure.

Fixed #62
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants