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

Bug: OpenID Address Claim Does Not Conform to OpenID Standard #1184

Closed
Congyuwang opened this issue Oct 5, 2022 · 8 comments
Closed

Bug: OpenID Address Claim Does Not Conform to OpenID Standard #1184

Congyuwang opened this issue Oct 5, 2022 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@Congyuwang
Copy link

In OpenID token, Address should be a json object not an array of string. This causes more rigorous openID client libraries to reject responses from Casdoor.

Failed to parse payload JSON: Error(\"invalid length 0, expected struct AddressClaim with 6 elements\", line: 1, column: 1578) at line 8 column 1)

See OpenID Connect 1.0 Spec: https://openid.net/specs/openid-connect-core-1_0.html#AddressClaim

@Congyuwang Congyuwang changed the title Bug: OpenID Address Format Does Not Conform to OpenID Standard Bug: OpenID Address Claim Does Not Conform to OpenID Standard Oct 5, 2022
@casbin-bot
Copy link
Contributor

@casbin-bot casbin-bot added the bug Something isn't working label Oct 5, 2022
@Congyuwang
Copy link
Author

I use https://docs.rs/openidconnect/latest/openidconnect/ openidconnect library in rust.

@Congyuwang
Copy link
Author

Congyuwang commented Oct 5, 2022

Quote from https://openid.net/specs/openid-connect-core-1_0.html#AddressClaim

The Address Claim represents a physical mailing address. Implementations MAY return only a subset of the fields of an address, depending upon the information available and the End-User's privacy preferences. For example, the country and region might be returned without returning more fine-grained address information.

Implementations MAY return just the full address as a single string in the formatted sub-field, or they MAY return just the individual component fields using the other sub-fields, or they MAY return both. If both variants are returned, they SHOULD be describing the same address, with the formatted address indicating how the component fields are combined.

  • formatted: Full mailing address, formatted for display or use on a mailing label. This field MAY contain multiple lines, separated by newlines. Newlines can be represented either as a carriage return/line feed pair ("\r\n") or as a single line feed character ("\n").
  • street_address: Full street address component, which MAY include house number, street name, Post Office Box, and multi-line extended street address information. This field MAY contain multiple lines, separated by newlines. Newlines can be represented either as a carriage return/line feed pair ("\r\n") or as a single line feed character ("\n").
  • locality: City or locality component.
  • region: State, province, prefecture, or region component.
  • postal_code: Zip code or postal code component.
  • country: Country name component.

@Congyuwang
Copy link
Author

Note that opened-connect-core advice the possibility of returning "just the full address as a single string in the formatted sub-field". That is, it is also acceptable to include only the formatted field without including the other subfields such as street_address and so on.

@Congyuwang
Copy link
Author

Therefore, in short, I would consider the previous reverted {formatted: "some address string"} as a valid compliance solution to AddressClaim.

@hsluoyz
Copy link
Member

hsluoyz commented Oct 5, 2022

@Congyuwang the previous PR was reverted because it has broken all our SDK code. All applications will fail. Can you make a PR to make a perfect fix?

@hsluoyz
Copy link
Member

hsluoyz commented Oct 7, 2022

@gtn1024

@Congyuwang
Copy link
Author

I think a figured a solution. That is to implement access_token and openid_token separately. So you can have your SDKs still use the old access_token format, while improving your compliance to OpenID protocol.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants