IdentityModel / oidc-client-js

OpenID Connect (OIDC) and OAuth2 protocol support for browser-based JavaScript applications
Apache License 2.0
2.43k stars 840 forks source link

User profile params should return a map instead of an Array of maps #1323

Open fvoordeckers opened 3 years ago

fvoordeckers commented 3 years ago

Steps to Reproduce:

Expected behaviour:

Calling user.profile.params would return a map of the extra user params.

Actual result:

Calling user.profile.params returns the params as an array where each entry holds the exact same params as a map.

Result of the /me call:

image

Result of userManager.getUser():

image

brockallen commented 3 years ago

Can you show an example of the result from the userinfo endpoint for the token server you're using?

fvoordeckers commented 3 years ago

@brockallen the user info is in the first screenshot

fvoordeckers commented 3 years ago

@brockallen noticed the user being stored in the session storage, session storage seems to be fine.

Maybe related to this?

https://github.com/IdentityModel/oidc-client-js/blob/88c7c7ade5d7bdc127c78480018ed3d9b121c383/src/ResponseValidator.js#L141

https://github.com/IdentityModel/oidc-client-js/blob/88c7c7ade5d7bdc127c78480018ed3d9b121c383/src/ResponseValidator.js#L175

brockallen commented 3 years ago

It looks like the "params" is an object with the 2 props (role and the uri)? So I guess you're asking that it's just passed along as-is when merged into profile.

It's possible that the id_token has "params" as well, so how would those look merged?

fvoordeckers commented 3 years ago

Filtering out duplicates resulting in a single map instead of an array?

fvoordeckers commented 3 years ago

Just checked it and the params are also in the JWT token. So is this expected behaviour that the type of the params key changes from dict to array? The id token params are not profile params, to calling user.profile.params should not return params in the id_token. If you want the params in the jwt, you should decode it or provide a methode to get them, or get them combined with the profile params. The last case could return the combined array that is now stored in user.profile.params. And then user.profile.params could simply return the params on the profile as a map, as it is intended to?

brockallen commented 3 years ago

Filtering out duplicates

For arbitrary object hierarchies, I would think this would be difficult. In the past my attitude was to just create an array and each object from the different sources get added in.

brockallen commented 3 years ago

Just checked it and the params are also in the JWT token

Ah ok, so that explains why it's not just copied over directly.

fvoordeckers commented 3 years ago

Just not sure if the idea of merging those params is correct. By creating an array of param object you're deviating from what the response provides. Also, parsing params from the JWT could be done in a separate method by simply moving that piece of to a method. Atm there is no way to distinct these params from each other. It's just an array of objects.

brockallen commented 3 years ago

But what if the two response objects are: {address:{street:"1 Main St"}} and the other is: {address:{street:"2 Spruce Rd"}}

It would not make sense to merge these.

fvoordeckers commented 3 years ago

Hmm indeed the issue here is that there are 2 types of parameters being stored in a single property which from my point of view should not be the case. Parameters from the JWT are simply out of scope there and can be easily parsed in another way. The other parameter do actually have the intention to serve as profile parameters. At this moment all the profile parameters are merged into a single array, representing profile params, where a part of them are just params of your JWT. Think it's more a conceptual difference, maybe adding some extra documentation about this would clarify things for others