Skip to content

Implement UserInfo Endpoint #176

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

Closed
jgrandja opened this issue Dec 10, 2020 · 22 comments
Closed

Implement UserInfo Endpoint #176

jgrandja opened this issue Dec 10, 2020 · 22 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@jgrandja
Copy link
Collaborator

jgrandja commented Dec 10, 2020

This issue will deliver OpenID Connect 1.0 UserInfo Endpoint.

Related gh-53

@jgrandja jgrandja added type: enhancement A general enhancement status: on-hold We can't start working on this issue yet labels Dec 10, 2020
@idosal
Copy link
Contributor

idosal commented Dec 10, 2020

Hey, I'd love to take this when it opens up.

@jgrandja
Copy link
Collaborator Author

Thanks for your interest @idosal.

Please ping me towards end of January as a reminder.
We'll likely start the work on this at that time.
Thanks!

@amit-github-personal
Copy link

Hey @jgrandja , let me know when you're going to start working on OPENID Connect. I also want to work when it opens up

@idosal
Copy link
Contributor

idosal commented Jan 22, 2021

@jgrandja Still waiting on this. :)

@jgrandja
Copy link
Collaborator Author

Thanks for the offer @amit-github-personal . Work has already started on OIDC with #53 being merged last month.

As well, @idosal reached out last month and is interested in taking on this feature. Please stay tuned for upcoming features as they open up. It looks like features planned for 0.1.1 have all been assigned.

@idosal I'm planning on focusing on 0.1.1 feature development after I get 0.1.0 out on Feb 11. The UserInfo endpoint is scheduled for 0.1.1, so please go ahead and review the spec to gain an understanding of the requirements. Let me know if you have any questions.

@jgrandja jgrandja removed the status: on-hold We can't start working on this issue yet label Jan 26, 2021
@jgrandja jgrandja added this to the 0.1.1 milestone Jan 26, 2021
@idosal
Copy link
Contributor

idosal commented Jan 27, 2021

@jgrandja Thanks, I'll get on it.

@idosal
Copy link
Contributor

idosal commented Mar 3, 2021

Hey @jgrandja, after reviewing the spec I started playing around with the code. I have a few questions I'd like to clear up:

  1. Where should I get the claims? Is it implemented elsewhere, or is it a part of this task?
  2. I'm unclear about this passage: If the UserInfo Response is signed and/or encrypted, then the Claims are returned in a JWT and the content-type MUST be application/jwt. The response MAY be encrypted without also being signed. If both signing and encryption are requested, the response MUST be signed then encrypted, with the result being a Nested JWT, as defined in [JWT]. How should I handle this scenario?

@jgrandja
Copy link
Collaborator Author

jgrandja commented Mar 8, 2021

@idosal

Where should I get the claims? Is it implemented elsewhere, or is it a part of this task?

It is part of this task. The "source" for the claims will come from the attributes in the Resource Owner's Authentication, specifically Authentication.getPrincipal(). Given that Authentication.getPrincipal() could be different types of Principal objects, this will need to be configurable and the application would need to supply the "Mapper" instance.

For example, let's assume the configuration of a specific Authorization Server deployment authenticates users against an LDAP directory. During the authentication process, it will authenticate the user and upon successful authentication it will set the SecurityContext with the Principal model -> LdapAuthentication.LdapUser (hypothetically). The LdapUser instance would be obtain via currentAuthentication.getPrincipal() (the "source"), and then the attributes within would need to be mapped to the standard claims defined in OidcUserInfo. As mentioned above, we need this "Mapper" to be confgurable because it will be different depending on the Authentication scheme being used and supported by the Authorization Server deployment.

Does this make sense?

FYI, there are a few different ways the client can request claims. For this initial iteration, we will need to support "5.4. Requesting Claims using Scope Values" (defined in OidcScopes).

Regarding your 2nd point, we will not support JWS, JWE or Nested JWS within JWE for the UserInfo response. We'll keep it simple for the initial iteration, which is to return a application/json response.

@Scarange
Copy link

any update on this?

@idosal
Copy link
Contributor

idosal commented Apr 12, 2021

@Benzenes I'm on it. I was a little delayed due to personal circumstances, but I'll open a draft in the next few days.

@jgrandja jgrandja modified the milestones: 0.1.1, 0.1.2 May 7, 2021
@jgrandja
Copy link
Collaborator Author

@idosal How are things coming along? Were you able to make any progress?

@idosal
Copy link
Contributor

idosal commented May 22, 2021

@idosal How are things coming along? Were you able to make any progress?

Hey,
I opened draft #303 to see if I'm on the right track. I apologize it's been longer than anticipated, but once I have your guidance I'll get this done ASAP. Thanks!

@jgrandja jgrandja modified the milestones: 0.1.2, 0.2.0 Jun 25, 2021
@jgrandja jgrandja modified the milestones: 0.2.0, 0.2.1 Jul 21, 2021
@sjohnr sjohnr assigned sjohnr and unassigned idosal Sep 17, 2021
sjohnr pushed a commit to sjohnr/spring-authorization-server that referenced this issue Sep 17, 2021
sjohnr pushed a commit to sjohnr/spring-authorization-server that referenced this issue Sep 17, 2021
sjohnr pushed a commit to sjohnr/spring-authorization-server that referenced this issue Sep 21, 2021
@deejonz
Copy link

deejonz commented Oct 2, 2021

will this userinfo endpoint return the user authorities as well? i.e. roles
I tried the snapshot and I don't see them in the response

@sjohnr
Copy link
Member

sjohnr commented Oct 4, 2021

@deejonz, work on this endpoint has not been merged to main yet. In the PR, authorities would not be returned by default. However, the PR currently exposes a customization hook allowing you to provide your own mapper in which you could add anything you wish to the endpoint's response.

@deejonz
Copy link

deejonz commented Oct 5, 2021

@sjohnr Hi, do you mean the userInfoMapper? Otherwise can you point me to this hook you're talking about? Thanks.

@sjohnr
Copy link
Member

sjohnr commented Oct 5, 2021

@deejonz, yes, that's the one.

@deejonz
Copy link

deejonz commented Oct 5, 2021

thanks @sjohnr can you please give me an example how to call it? I really don't find a way.

@sjohnr
Copy link
Member

sjohnr commented Oct 5, 2021

Hi @deejonz,

This test demonstrates it. Perhaps you're asking how to use the configurer? You can check out numerous tests in this package to see some general examples of the OAuth2AuthorizationServerConfigurer in action. Does that help? If not, we'll be working on some documentation in the next few releases as well.

@deejonz
Copy link

deejonz commented Oct 6, 2021

Hi @sjohnr, it wasn't very straightforward but I managed to implement it, thanks a lot for your help!

@deejonz
Copy link

deejonz commented Oct 8, 2021

Hi @sjohnr sorry another question, why don't you leave OidcUserInfoClaimsMapper free to be extended for this purpose?

@sjohnr
Copy link
Member

sjohnr commented Oct 8, 2021

Thanks for your interest, @deejonz. There are several reasons, but the primary reason is that we always start with implementations that are closed, and slowly open up things that make sense to once APIs stabilize.

If you have a specific use case you're trying to achieve with the User Info Endpoint, let's wait until we get this merged, and you can open an issue with a description of that use case and why you feel this is needed. We can then discuss it and alternatives. Does that sound ok?

@deejonz
Copy link

deejonz commented Oct 10, 2021

yes, that sounds ok. Thanks @sjohnr

jgrandja pushed a commit that referenced this issue Oct 25, 2021
doba16 pushed a commit to doba16/spring-authorization-server that referenced this issue Apr 21, 2023
doba16 pushed a commit to doba16/spring-authorization-server that referenced this issue Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants