🚑️ OIDC: use Azure-AD unique identifier #2510
Conversation
|
@szuecs Do you see a chance this to be merged soon? |
We are facing a security vulnerability by the use of `DisplayName` as the unique identifier in Azure-AD. Since this field can be duplicated it can be exploited to gain privileges. https://morgansimonsen.com/2016/06/28/azure-ad-allows-duplicate-group-names/ Switching to the guaranteed unique identifier `onPremisesSamAccountName` available in the Microsoft Graph API Signed-off-by: Samuel Lang <[email protected]>
|
👍 |
1 similar comment
|
👍 |
| @@ -1075,7 +1075,9 @@ func (f *tokenOidcFilter) resolveDistributedClaimAzure(url *url.URL, oauth2Token | |||
| return nil, fmt.Errorf("unabled to decode response: %w", err) | |||
| } | |||
| for _, v := range target.Value { | |||
| values = append(values, v.DisplayName) | |||
| if v.OnPremisesSamAccountName != "" { | |||
There was a problem hiding this comment.
Switching to the guaranteed unique identifier onPremisesSamAccountName available in the Microsoft Graph API
Could it be empty?
There was a problem hiding this comment.
And should we have a test to ensure behaviour when it is empty?
There was a problem hiding this comment.
Yes it can be empty and I have extended a test to reflect it.
There was a problem hiding this comment.
onPremisesSamAccountName does not look like a standard attribute so we have to rely on you testing that this works correctly.
I guess id is also not stable/reliable to use for the purpose, is it?
There was a problem hiding this comment.
The onPremisesSamAccountName is the equivalent of the samAccountName issued with the standard group claim.
But it might be that someone rather would like the GUID / ID to be issued, it is configurable in Azure app registration.
If you want @AlexanderYastrebov the field selector could be extended to be configurable. If so, which way, using a flag or another filter parameter?
There was a problem hiding this comment.
We do not use azure-based auth and have no experience with it. Lets have it like you proposed and see if we get any feedback on the change from other users.
We are facing a security vulnerability by the use of
DisplayNameas the unique identifier in Azure-AD.Since this field can be duplicated it can be exploited to gain privileges.
https://morgansimonsen.com/2016/06/28/azure-ad-allows-duplicate-group-names/
Switching to the guaranteed unique identifier
onPremisesSamAccountNameavailable in the Microsoft Graph API