goharbor / harbor

An open source trusted cloud native registry project that stores, signs, and scans content.
https://goharbor.io
Apache License 2.0
23.95k stars 4.74k forks source link

OIDC | Custom group claim value can be string or array #15416

Open mayerraphael opened 3 years ago

mayerraphael commented 3 years ago

Expected behavior and actual behavior: If only one group is available in the specified group claim, this claim is of type StringOrUri. Only if two or more groups are available, the claim value becomes an array. This behaviour is explained for the "aud" claim at RFC751.

C#s System.IdentityModel.Tokens.Jwt for example uses this logic by default for all claims. Single value = StringOrUri. Multiple Values = Array.

IdentityProviders like IdentityServer4 also handle claims like this.

The current behavior seems to always expect an array, even if only one group is provided. If i log in via OIDC and only have one group as a StringOrUri value, the custom group claim is ignored. If two or more groups are specified, and the claim value is of type array, the group mapping works.

Steps to reproduce the problem: Add group claim mapping. Only provide one group in the claim. The claim is issued as value "string" from the IdentityServer. Group permissions do not work. Only if two or more groups are present and the claim is of type Array[StringOrUri], even if the additional groups are of no relevance, access to projects that have group permissions now works.

Versions:

reasonerjt commented 3 years ago

@mayerraphael May I know what OIDC provider you are using?

It's been a while but I recall I tested against several OIDC providers and the values of groups are all lists in the payload

mayerraphael commented 3 years ago

@reasonerjt IdentityServer 4.

I can recommend the JWT Builder by Jamie Kurtz where you can replicate the behavior: http://jwtbuilder.jamiekurtz.com/

Single value = string, multiple values with same key = list.

Single value: image

Multiple values: image

reasonerjt commented 3 years ago

Thanks @mayerraphael

I understand this requirement but it may not be a priority. When the support for OIDC group was designed, it was determined that the value of the group claim in the token has to be an array of strings, and this worked for the OIDC providers that we tested. Is it possible to modify the setting of your OIDC to make it a list of strings?

mayerraphael commented 3 years ago

@reasonerjt No, it is not. Its default behavior for everything that uses the official Microsoft library "System.IdentityModel.Tokens.Jwt".

github-actions[bot] commented 2 years ago

This issue is being marked stale due to a period of inactivity. If this issue is still relevant, please comment or remove the stale label. Otherwise, this issue will close in 30 days.