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

Group ID instead of Group Name when using Azure AD OICD #12178

Closed
bgsz opened this issue Jun 8, 2020 · 19 comments
Closed

Group ID instead of Group Name when using Azure AD OICD #12178

bgsz opened this issue Jun 8, 2020 · 19 comments

Comments

@bgsz
Copy link

bgsz commented Jun 8, 2020

Can you explain why in the Groups tab I only see the ID instead of the name when I use Azure AD as OICD. Is this the fault of the Token configuration I get from Azure? I've tried all the configurations but none work.
Problem occurs for both 1.x and 2.0 versions
image

@reasonerjt
Copy link
Contributor

It is related to the group claim setting in your OIDC provider configuration.
You need to make sure the group name is in the ID token and set the group claim to point to it.

@reasonerjt reasonerjt self-assigned this Jun 9, 2020
@reasonerjt reasonerjt added the more-info-needed The issue author need to provide more details and context to the issue label Jun 9, 2020
@yaron
Copy link

yaron commented Jul 2, 2020

Azure doesn't support using the group name in the id token for AAD groups because the group name might not be unique. To use group names we'd need to use the azure api to fetch the groups with their ids. I'd be willing to put a couple of days of work in to create a PR to do this, but since this would be an Azure specific solution, would it actually be considered for merging?

Edit: This would be needed to fetch all groups: https://docs.microsoft.com/en-us/graph/api/group-list?view=graph-rest-1.0
Users would have to give the harbor application registration access to this api call for it to work.

@mrdima
Copy link

mrdima commented Jul 6, 2020

I'm all for Yaron's proposal. @reasonerjt can you consider this would be useful to include?

@reasonerjt
Copy link
Contributor

@yaron
Thanks for the explanation, I now understand the issue.

However, for simplicity and maintainability, we want to keep a unified workflow for all OIDC providers. Such that in the pipeline we'll only test dex .

Currently there's no plan to add specific logic for different OIDC vendor.

@reasonerjt reasonerjt added priority/low and removed more-info-needed The issue author need to provide more details and context to the issue labels Sep 23, 2020
@yaron
Copy link

yaron commented Sep 23, 2020

I missed this at first, but it should be possible through dex.
https://github.com/dexidp/dex/blob/master/Documentation/connectors/microsoft.md

So there's two possibilities here

  1. Dex is not implemented correctly in the case of AAD.
  2. The documentation should be updated to make users do the consent part and give harbor the permissions to let dex get the groupnames.

I don't know which is the case. I hope I'll have some time to test this next week, but if you could take a look at that page and check the implementation you'd probably know much faster.

@stale
Copy link

stale bot commented Dec 24, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale label Dec 24, 2020
@mlushpenko
Copy link

Would be happy to see this as well if possible

@stale stale bot removed the Stale label Mar 26, 2021
@mlushpenko
Copy link

@reasonerjt would it be possible to add a description field to groups in the UI? That way you don't have to change OIDC logic, but for end-users there will be an option to look up group name once in Azure and save it to harbor UI for future reference/management.

@stale
Copy link

stale bot commented Jun 26, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale label Jun 26, 2021
@ccittadino-ctic
Copy link

Are we able to get an update on @yaron 's suggested fix for this? Not having group names is causing headaches for managing our Harbor instance(s). Currently we have to copy/paste and ask around for what different GUIDs mean. For reference - the above enhancement seems to be used by other products. For example, we use Rancher with our same Azure AD and it requests graph api access, users and groups come through beautifully. How can we push to get this implemented with Harbor?

@stale stale bot removed the Stale label Jun 28, 2021
@stale
Copy link

stale bot commented Apr 17, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale label Apr 17, 2022
@jgreat
Copy link

jgreat commented May 17, 2022

Ping on this issue, this makes managing access with Azure AD groups difficult.

@github-actions github-actions bot removed the Stale label Jul 7, 2022
@github-actions
Copy link

github-actions bot commented Sep 6, 2022

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.

@github-actions github-actions bot added the Stale label Sep 6, 2022
@github-actions
Copy link

github-actions bot commented Oct 6, 2022

This issue was closed because it has been stalled for 30 days with no activity. If this issue is still relevant, please re-open a new issue.

@github-actions github-actions bot closed this as completed Oct 6, 2022
@johanot
Copy link

johanot commented May 25, 2023

A low effort workaround could be an ability to attach metadata to an OIDC group in Harbor, e.g. a tag or a comment. If I had the GUI spit out a tag/comment for me, next to the UUID, it would make life much easier for me.

@l-drews
Copy link

l-drews commented Dec 13, 2023

We found a feasible workaround for displaying the Group Name instead of the UUID. We configured our Azure AD application to send the names instead of the UUIDs. We had to manually edit the Manifest via App Registrations > your-harbor-app > Manifest and used the cloud_displayname in the additionalProperties list.

{
   // ...
   "optionalClaims":{
      // ...
      "idToken":[
         // ...
         {
            "name": "groups",
            "source": null,
            "essential": false,
            "additionalProperties": [
               "cloud_displayname"
            ]
         }
      ]
      // ...
   }
}

@DT0002
Copy link

DT0002 commented Dec 13, 2023

looks like this does not work for the admin group ? or did you also change "accesstoken" & "saml2Token" in the manifest

@DT0002
Copy link

DT0002 commented Dec 13, 2023

combination with changing the manifest and keeping an app role for the admin group seems to do the job

@l-drews
Copy link

l-drews commented Dec 13, 2023

combination with changing the manifest and keeping an app role for the admin group seems to do the job

I just tested it with a new AAD application which has no app roles and using the admin group works like expected. However, I had some troubles when I used my old application that previously used app roles.

EDIT: The users page still displays "Unknown" in the administrator column.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants