oauth2_proxy icon indicating copy to clipboard operation
oauth2_proxy copied to clipboard

Added OIDC support for UserInfo Endpoint Email Verification

Open dcwangmit01 opened this issue 7 years ago • 8 comments

  • Current OIDC implementation asserts that user email check must come from JWT token claims. OIDC specification also allows for source of user email to be fetched from userinfo profile endpoint. http://openid.net/specs/openid-connect-core-1_0.html#UserInfo

  • Modified current code to search for email in JWT token claims first, and then fall back to requesting email from userinfo endpoint.

dcwangmit01 avatar Oct 19 '17 23:10 dcwangmit01

Hi @jehiah @ericchiang,

I noticed that the two of you were responsible for adding the most recent OIDC provider. I was hoping for feedback on this patch which was required to make the OIDC provider work for me. I'm new to OIDC/oauth, so feedback is appreciated.

Thanks! -dave

dcwangmit01 avatar Oct 19 '17 23:10 dcwangmit01

Adding this comment hidden by code updates...

I think the OIDC spec allows for email to be returned in either JWT claims OR the UserInfo endpoint. The code changes involving the UserInfo endpoint follow the same pattern as the LinkedIn, Gitlab, and Azure providers. It first searches JWT token for the email. If it cannot find it in the JWT token, it falls back to searching the UserInfo endpoint.

dcwangmit01 avatar Oct 20 '17 17:10 dcwangmit01

Hi All,

I'd still like to get this pull request in. I've made the changes that @ericchiang had requested. Sorry it took a few months.

This PR has been rebased on the latest code.

-dave

dcwangmit01 avatar Jan 06 '18 07:01 dcwangmit01

Hi @jehiah @ericchiang,

Would you mind having a look to review?

Thanks! -dave

dcwangmit01 avatar Jan 17 '18 23:01 dcwangmit01

@ericchiang I made the changes you've requested. Would you mind reviewing?

dcwangmit01 avatar May 16 '18 04:05 dcwangmit01

Hello @jehiah @ericchiang, Can this please be merged and released?

This does resolve my issue: https://github.com/bitly/oauth2_proxy/issues/615

Thanks

Kaszaq avatar Jun 11 '18 12:06 Kaszaq

Hi @jehiah @mbland @ploxiln,

Would any of you mind reviewing this pull request and potentially merging it? A few people have used this patch successfully. My teams have been using it in multiple deployments successfully for several months now. We'd love to get rid of our fork.

Thanks!

dcwangmit01 avatar Jun 20 '18 21:06 dcwangmit01

@mbland and I do not have permissions to merge in this repo, sorry

ploxiln avatar Jun 20 '18 21:06 ploxiln