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

The retrieveUserInfoFromAccessToken doesn't return a UserResponse #54

Open
adriano-di-giovanni opened this issue May 12, 2021 · 7 comments

Comments

@adriano-di-giovanni
Copy link

The retrieveUserInfoFromAccessToken doesn't return a UserResponse

Instead, it returns a response that has the shape described in the docs

@adriano-di-giovanni
Copy link
Author

UserInfoResponse is now defined as a Record<string, any>. Could it be more specific?

@mooreds
Copy link
Contributor

mooreds commented May 13, 2021

@adriano-di-giovanni hmmm. The userinfo response is defined by the OIDC specification. Here's a couple of relevant excerpts:

Upon receipt of the UserInfo Request, the UserInfo Endpoint MUST return the JSON Serialization of the UserInfo Response as in Section 13.3 in the HTTP response body

https://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse

The parameters are serialized into a JSON object structure by adding each parameter at the highest structure level. Parameter names and string values are represented as JSON strings. Numerical values are represented as JSON numbers. Boolean values are represented as JSON booleans. Omitted parameters and parameters with no value SHOULD be omitted from the object and not represented by a JSON null value, unless otherwise specified. A parameter MAY have a JSON object or a JSON array as its value.

https://openid.net/specs/openid-connect-core-1_0.html#JSONSerialization

I'm no typescript expert, but this would seem to indicate that a record type is the correct type. Am I misunderstanding things? Is there an example of another typescript OAuth/OIDC library you can point me to that represents this object in a better way?

@adriano-di-giovanni
Copy link
Author

adriano-di-giovanni commented May 14, 2021

The UserinfoResponse type is correct but pretty vague. It would be nice to use a type describing the supported claims

interface UserinfoResponse {
  sub?: string;
  name?: string;
  given_name?: string;
  family_name?: string;
  // ...
}

as defined in the spec:

This specification defines a set of standard Claims. They can be requested to be returned either in the UserInfo Response, per Section 5.3.2, or in the ID Token, per Section 2

https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims

@mooreds
Copy link
Contributor

mooreds commented May 14, 2021

Thanks @adriano-di-giovanni

I opened up two issues in our main issue repo for tracking this. The original issue you reported is definitely a bug. If you'd like to submit a PR against the client library changing that, we'd welcome it, otherwise we'll try to get that fixed soon.

The suggestion here: #54 (comment) is a new enhancement and I filed that as well. Less sure about when that will be done (juggling quite a few priorities right now).

@adriano-di-giovanni
Copy link
Author

@mooreds I thought the client library was generated. If so, what should be changed?

@mooreds
Copy link
Contributor

mooreds commented May 14, 2021

It is generated, you are correct.

For the bug, I think the change is as simple as modifying https://github.com/FusionAuth/fusionauth-client-builder/blob/master/src/main/api/retrieveUserInfoFromAccessToken.json to have a successResponse of UserinfoResponse.

The feature is a bigger effort, not quite sure what needs to be done there, probably creating a new domain object. We generate our domain objects from our java domain objects, so that is work that would have to be done internally, as I'm not sure what else it would affect.

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

No branches or pull requests

2 participants