-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
UserInfo endpoint #376
Comments
It's recommended, not mandatory:
https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata We currently add additional claims to the ID Token. For example: {
"aud": "example-app",
"email": "elroy77@example.com",
"exp": 1458191394,
"iat": 1458148194,
"iss": "http://127.0.0.1:5556",
"name": "Elroy Jonez",
"sub": "elroy-id"
} But yes, we should absolutely have a userinfo endpoint. |
I was just thinking today how the |
This would actually be really easy, since we currently use the ID Token as the access token[0]. We'd just decode that for the user. [0] https://github.com/coreos/dex/blob/2d5fb0b47a3150d4fb2696f0736c12ee57d72ba7/server/http.go#L495-L500 |
Is this still planned? I would like to see this implemented before 2.0 gets final. |
This isn't since all information we'd return with a userinfo endpoint is already returned in the id_token. Also, right now nothing uses the access_token returned by dex v2 (it's actually just a random string), so implementing this requires some design around what an access_token is internally to dex. I understand that their are clients that require this optional endpoint, but it's not a high priority for us. |
Proposal: Let's backpack off the proto types add for refresh tokens in #757 to implement an access token. The token will look like: message AccessToken {
string subject = 1;
string name = 2;
string email = 3;
bool email_verified = 4;
// Time at which the access token expires, represented as a unix time.
//
// This is the same value as the id_token.
int64 expiry_unix = 5;
} The access token fields will be filled out by the The access token struct will be serialized, then signed as a JWS using the signing keys maintained by dex, and returned as the "access_token" payload. When a user hits the On a refresh, the new We also may consider using ECDSA keys if the signature is significantly shorter. Because [0] https://godoc.org/github.com/coreos/dex/storage#AuthCode |
Seems like a great approach. This is almost exactly what the OIDC spec recommends if your attempting to build a stateless OIDC provider too. |
We also might consider encrypting the token instead of signing it. Just to make sure no one tries to rely on our access_token format. |
A userinfo endpoint is required to pass the basic OpenID Connect Conformance Profile to achieve certification. Certification issue: #42 |
I am trying to use dex with spinnaker and it needs the userInfoUri endpoint. Without this it's not possible to use dex with it. See https://www.spinnaker.io/setup/security/authentication/oauth/#bring-your-own-provider |
Same w/ anyone attempting to integrate with Centrify. Centrify doesn't include any of the expected data in that id_token JWT. The expectation is that you will call the userinfo endpoint to gain the detail you are looking for. details here: https://docs.centrify.com/en/centrify/appref/index.html?version=1507675723#page/cloudhelp%2Fgen%2FSAML-gen-OpenID.2.html |
The same issue happens when integrating with the GenericOAuthenticator module used by JupyterHub for OpenID Connect authentication. |
I am trying to use dex as an authentication provider with GitLab, and the omniauth-openid-connect gem which GitLab uses requires the userinfo endpoint as well. I understand the philosophical standpoint that all information should be taken from the token, but at the same time, it seems like this stanza prevents using dex with a wide variety of software, which is a real shame. Please consider the pragmatic solution of implementing the userinfo endpoint. Thank you! |
Will work on this in the upcoming weeks and submit a proposal for review |
That’s great news! Thank you. |
@rithujohn191 I forked Dex and successfully implemented the User Info endpoint. I will share it as soon as I finish polishing it, but I will give some details on how it works. Connectors of type oidc have a new configuration option userInfo where a user info endpoint for the OP can be provided. E.g.:
How it works technically:
Note: I did not consider token refresh. This has only be tested with the generic OAuthenticator plugin of Jupyterhub which uses a basic OAuth2 flow. |
@fjbsantiago Does your implementation also work with other connectors like LDAP? |
@markuslindenberg Sorry, but I only implemented this for Open ID Connect connectors, unfortunately. |
Just a heads up. Any dex user info implementation we add to dex will work with any backing connector (LDAP, SAML, GitHub, etc), not just with OpenID Connect. |
I have another quick question @ericchiang. Some OPs return different kinds of claims from the userinfo endpoint like, for example, birth date and gender. |
@NobodysNightmare I wrote an implementation for the UserInfo endpoint. It is currently pending as a pull request. It would be nice if you could try it to see if it solves your issue with omniauth-openid-connect, unless you already found a workaround. |
@fjbsantiago I have made a simple test of your code as openshift(kubernetes) auth provider As i reviewed openshift openid plugin code I think the subject in AccessToken should be the same as in id_token |
@hardbone12 rolled my own, borrowed mostly from @fjbsantiago , fixed the subject issue |
Hey Folks, any news on this? |
Since I am not familiar with all flows and OIDC specifications, I have some dummy questions:
|
I've created a PR which adds userinfo_endpoint based on the current code and the previous PR from @jackielii #1454. Also available as a Docker image from https://hub.docker.com/r/mdbraber/dex |
I am currently trying to get dex working in a ruby environment. On the RP side I am currently working with omniauth-openid-connect, which has some issues of its own.
However: Working around some of those initial issues, I am now stuck with omniauth-openid-connect trying to get additional claims from the userinfo endpoint, which is not implemented by dex.
In reading the OIDC specification, I failed to recognize whether this is a mandatory endpoint or an optional endpoint, but since it appears right away in section 1.3 of the spec, I would assume it is mandatory.
Are there any plans on implementing this endpoint?
If not, I might start working on it, but that depends on a number of other factors and it would be my first larger golang contribution anywhere, not to mention that I have barely read enough of the specs...
The text was updated successfully, but these errors were encountered: