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

gather custom claims from IdP's IDToken (was Header X-Vouch-IdP-Claims-Cognito-Groups not set) #377

Open
bonnydeal opened this issue Mar 30, 2021 · 13 comments

Comments

@bonnydeal
Copy link

I have set up a claim for "cognito:groups" in the config.
The vouch log reports:
{"level":"info","ts":1617014646.7303452,"msg":"Vouch.header.claims cognito:groups will be forwarded downstream in the Header X-Vouch-IdP-Claims-Cognito-Groups"}
However the header is not being set.
The value for "cognito:groups" is present and correct in the JWT.
All the other claims are being set correctly.

The comment here indicates that this is not expected to work until #183 and #184 are resolved.

I see #183 and #184 are closed, but not seeing the header. Is there some other problem?

@bnfinet
Copy link
Member

bnfinet commented Apr 2, 2021

I believe it should work, but I am not a cognito user and I have not tested that fix.

@bonnydeal could you please post logs and config in the manner described in the README? Without those it's difficult to advise.

@bonnydeal
Copy link
Author

bonnydeal commented Apr 3, 2021

I have put the logs and the nginx conf into the gist here.

https://gist.github.com/bonnydeal/2b9a57f37d930194ea730e94f9752838

@bnfinet
Copy link
Member

bnfinet commented Apr 4, 2021

I think you want to get rid of the add_header directives.
https://gist.github.com/bonnydeal/2b9a57f37d930194ea730e94f9752838#file-nginx-conf-L47-L51

Is there something else which needs to be configured on the Cognito end to offer that information?

@bonnydeal can you please show logs that include the full roundtrip including /auth. The Usernifo call back to AWS Cognito is most critical.

@bonnydeal
Copy link
Author

bonnydeal commented Apr 4, 2021

Hi
Thanks for the response.

I tried to create a new vouch log with testing on (sorry, I forgot that in the earlier gist).
However, I get a 400 bad request when i click login on the testing page.
With testing turned off, it works again.

If i get rid of the add_header directives, I don't get any of the claims forwarded.
With the add_header in place I get them all apart from cognito groups.

I have checked the jwt token, and it does contain the cognito groups. (decoded jwt included in the gist)
https://gist.github.com/bonnydeal/7e7b5dce99363ce81bbfe46b1cd01fe2

@bnfinet
Copy link
Member

bnfinet commented Apr 4, 2021

@bonnydeal if testing isn't working that may point at the problem with your setup

Please upload the log and config associated with testing: true. If you're getting an error, lets troubleshoot that first.

Could you please re-read these and follow the instructions. You may need to logout first.
https://github.com/vouch/vouch-proxy#okay-i-looked-at-the-issues-and-have-tried-some-things-with-my-configs-but-its-still-not-working

Specifically, I need the roundtrip and the userinfo call.

@bonnydeal
Copy link
Author

Hi

I have created a new gist with testing:true and attempted to log in.
https://gist.github.com/bonnydeal/091fd87199550a7839ba25147654ad66

I am using the docker method described here: https://github.com/vouch/vouch-proxy#okay-i-looked-at-the-issues-and-have-tried-some-things-with-my-configs-but-its-still-not-working:

docker run --name vouch_proxy -v $PWD/config:/config -v $PWD/certs:/certs -it --rm --entrypoint /do.sh voucher/vouch-proxy:alpine bug_report yourdomain.com anotherdomain.com someothersecret

@bnfinet
Copy link
Member

bnfinet commented Apr 5, 2021

@bonnydeal when you are redirected to the testing mode /login screen after accessing your protected website, do you click on the link to AWS Cognito (the 302 redirect)? What happens then?

@bonnydeal
Copy link
Author

OK,
my bad i didn't realise you had to click all the links in the testing page!

Now I have managed to login and get to my protected page and all works properly in testing mode as well.
I have created a new gist with the vouch log.
https://gist.github.com/bonnydeal/df33895b0ed02bfb49f11be2154b8855

@bnfinet
Copy link
Member

bnfinet commented Apr 6, 2021

It does not appear that Cognito is passing the group array in the Userinfo call

https://gist.github.com/bonnydeal/df33895b0ed02bfb49f11be2154b8855#file-vouch-testing-log-L109-L110

This seems similar to what @rogerscuall was seeing in #221

@bonnydeal where did the JWT you decoded come from? How are you seeing it from Cognito?

@bonnydeal
Copy link
Author

bonnydeal commented Apr 6, 2021 via email

@bnfinet
Copy link
Member

bnfinet commented Apr 7, 2021

upon further inspection #221 went fallow and was closed without resolution by the OP. So I was mistaken when I said that it should work.

The issue is that the IdToken returned from the IdP is not inspected for claims.

OIDC looks at userinfo

if err = common.MapClaims(data, customClaims); err != nil {

It could be improved to do something similar to what ADFS does by grabbing the IdToken out of userinfo call

adfsUser := structs.ADFSUser{}
json.Unmarshal([]byte(idToken), &adfsUser)
log.Infof("adfs adfsUser: %+v", adfsUser)
// data contains an access token, refresh token, and id token
// Please note that in order for custom claims to work you MUST set allatclaims in ADFS to be passed
// https://oktotechnologies.ca/2018/08/26/adfs-openidconnect-configuration/
if err = common.MapClaims([]byte(idToken), customClaims); err != nil {
return err
}

It'd be pretty nice to add that logic to pkg/providers/common.go.

@bonnydeal do you have any interest in working on that?

@bnfinet bnfinet changed the title Header X-Vouch-IdP-Claims-Cognito-Groups not set. gather custom claims from IdP's IDToken (was Header X-Vouch-IdP-Claims-Cognito-Groups not set) Apr 7, 2021
@bonnydeal
Copy link
Author

I would be interested on working on that, but I have no experience in "Golang". I will try to have a look tonight.

I have also contacted AWS and they confirm it is not in the userInfo response. I have made a feature request for it to be included, but there is no guarantee that it will be or in what time-frame.
Another (AWS specific) idea would be to create a new endpoint for userInfo that calls a lambda which calls the cognito userInfo and decorates the returned data with the required fields.

@bonnydeal
Copy link
Author

Hi i got put on something else.
In the meantime I found an AWS specific solution.
This is to create a pre token generation trigger in the cognito user pool.
This trigger then calls a lamda which adds the group(s) to the userinfo

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

No branches or pull requests

2 participants