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

Userinfo response is not as expected as describe in spec #1038

Closed
ldeveloperl1985 opened this issue Mar 14, 2019 · 9 comments
Closed

Userinfo response is not as expected as describe in spec #1038

ldeveloperl1985 opened this issue Mar 14, 2019 · 9 comments
Labels
enhancement libs update, re-factroring, etc.
Projects
Milestone

Comments

@ldeveloperl1985
Copy link

ldeveloperl1985 commented Mar 14, 2019

Describe the issue

I’m dealing with get_user_info endpoint in oxd. The response is very strange - every claim is an array.
According to specs it should be single value.

Userinfo response in spec https://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse

Steps To Reproduce

Request to userinfo to OP server or request using OXD API. I am using OXD API.

Expected behavior

but it should be like

{
        "claims":{
            "sub": "248289761001",
            "name": "Jane Doe",
            "given_name": "Jane",
            "family_name": "Doe",
            "preferred_username": "j.doe",
            "email": "janedoe@example.com",
            "picture": "http://example.com/janedoe/me.jpg"
        }
    }

Actual behavior

Response return from oxd user_info -> op userinfo endpoint is

{
        "claims":{
            "sub": ["248289761001"],
            "name": ["Jane Doe"],
            "given_name": ["Jane"],
            "family_name": ["Doe"],
            "preferred_username": ["j.doe"],
            "email": ["janedoe@example.com"],
            "picture": ["http://example.com/janedoe/me.jpg"]
        }
    }

Environment:

  • OS: (Ubuntu14.04LTS)
  • Gluu version(3.1.5)
  • OXD Version(4.0)

Discussion

Meghna Joshi: 
{
        "claims":{
            "sub": ["248289761001"],
            "name": ["Jane Doe"],
            "given_name": ["Jane"],
            "family_name": ["Doe"],
            "preferred_username": ["j.doe"],
            "email": ["janedoe@example.com"],
            "picture": ["http://example.com/janedoe/me.jpg"]
        }
    }
so is it ok or bug?

Alexander Altshuler:
it is not as defined in the standard

Yuriy Zabrovarnyy:
user info oxauth-client always returns list of arrays https://github.com/GluuFederation/oxAuth/blob/532db4ad0950579aa5ccb3555b6b6057ddd926b1/Client/src/main/java/org/xdi/oxauth/client/UserInfoResponse.java#L20

Alexander Altshuler:
https://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse
openid.net
Final: OpenID Connect Core 1.0 incorporating errata set 1
OpenID Connect Core 1.0 incorporating errata set 1

Yuriy Zabrovarnyy:
do we have in spec said that it has to be single valued? I guess Javier tried to unify handling.
in spec there are examples of nested json objects, like "auth_time": {"essential": true}
so it seems any valid json are valid
you anyway need to handle arrays

Alexander Altshuler:
The sub (subject) Claim MUST always be returned in the UserInfo Response.
NOTE: Due to the possibility of token substitution attacks (see Section 16.11), the UserInfo Response is not guaranteed to be about the End-User identified by the sub (subject) element of the ID Token. The sub Claim in the UserInfo Response MUST be verified to exactly match the sub Claim in the ID Token; if they do not match, the UserInfo Response values MUST NOT be used.
here is defined that sub must be the same string as in id_token
and for sure arrays it is not what most of the users expect here

Yuriy Zabrovarnyy: 
do you expect json object as value inside sub ?
I does not expect it but in spec I see example "sub": {"value": "248289761001"}
in general I agree of course )

Alexander Altshuler:
“sub”: {“value”: “248289761001”}
it is not returned claim example, it is claim request for particular value

Yuriy Zabrovarnyy:
right, anyway lets involve Javier to this discussion as author of this code
Meghna Joshi please open ticket on oxauth about userinfo
@yuriyz yuriyz added this to the 4.1 milestone Mar 14, 2019
@yuriyz yuriyz added the enhancement libs update, re-factroring, etc. label Mar 14, 2019
@yuriyz
Copy link
Contributor

yuriyz commented Mar 14, 2019

oxd returns what oxauth-client returns. Right now it seems to always return claim arrays for userinfo.

See

private Map<String, List<String>> claims;

@nynymike
Copy link
Contributor

I disagree that this is even a bug. The example in the spec says explicitly that it is NON-NORMATIVE. I did not see anywhere in the response where it specified the values MUST be strings. We have more important things to do. Returning as arrays makes sense because you always know what kind of object it is, versus having to test whether it is a string or array. This has been discussed ad naseum in the past.

@qbert2k
Copy link
Contributor

qbert2k commented Mar 15, 2019

oxAuth is returning the claims in the correct format as specified in the specs, for example:


REQUEST:

GET /restv1/userinfo HTTP/1.1
Host: ce.gluu.info
Authorization: Bearer c7124969-1119-4632-abc9-60bd6f10a190

RESPONSE:

HTTP/1.1 200
Cache-Control: no-store, private
Content-Length: 611
Content-Type: application/json;charset=utf-8
Pragma: no-cache
Server: Jetty(9.4.11.v20180605)

{"address":{"street_address":"621 East 6th Street","country":"US","locality":"Austin","region":"Texas"},"sub":"yD_9P9HYVAIw99w8UMheByjbmPhhmpb6tYtUYSvY5Vk","website":"http://gluu.org","zoneinfo":"America/Chicago","birthdate":"1/6/1983","gender":"male","profile":"https://www.facebook.com/profile","preferred_username":"admin","given_name":"Admin","middle_name":"admin","locale":"en-US","picture":"http://www.gluu.org/wp-content/uploads/2012/04/mike3.png","updated_at":1506584082831,"name":"Default Admin User","nickname":"admin","family_name":"User","email_verified":true,"email":"test_user@example.org"}

But, the code in oxauth-client is prepared to also handle custom multivalued claims.

@yurem
Copy link
Contributor

yurem commented Mar 15, 2019

in this case oxd convert response

@yuriyz
Copy link
Contributor

yuriyz commented Mar 15, 2019

@yurem oxauth-client returns map of list, see link above.

private Map<String, List<String>> claims;

I think it's not big dial. It's also easier to handle for client code when values are unified.

@yurem
Copy link
Contributor

yurem commented Mar 15, 2019

@yuriyz I don't mind

yuriyz added a commit to GluuFederation/oxd that referenced this issue Mar 15, 2019
@yuriyz
Copy link
Contributor

yuriyz commented Mar 15, 2019

Corrected oxd and transfered oxauth response as is, for this used oxauth-client's UserInfoResponse.getEntity(). Commit : GluuFederation/oxd@576053c

@ldeveloperl1985
Copy link
Author

@yuriyz thank you!
is latest OXD build with this changes available?

@yuriyz
Copy link
Contributor

yuriyz commented Mar 15, 2019

not yet

@yuriyz yuriyz modified the milestones: 4.1, 4.2 Dec 12, 2019
@yurem yurem added this to Done in CE 4.2.0 Jan 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement libs update, re-factroring, etc.
Projects
No open projects
CE 4.2.0
  
Done
Development

No branches or pull requests

5 participants