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

Support mapping groups' IDs to names for Django groups #215

Open
tim-schilling opened this issue Jan 24, 2022 · 8 comments
Open

Support mapping groups' IDs to names for Django groups #215

tim-schilling opened this issue Jan 24, 2022 · 8 comments

Comments

@tim-schilling
Copy link
Member

tim-schilling commented Jan 24, 2022

Moving this part of the conversation from #173 to here.

Looks like sAMAccountName is not available for groups entirely managed within Azure AD: https://docs.microsoft.com/en-us/azure/active-directory/hybrid/how-to-connect-fed-group-claims

sAMAccountName and On Premises Group SID attributes are only available on Group objects synced from Active Directory. They aren't available on groups created in Azure Active Directory or Office365. Applications configured in Azure Active Directory to get synced on-premises group attributes get them for synced groups only.

For configurations with only Azure Active Directory, the above means that the group claims will only include the id. This results in the group names being UUIDs which aren't reasonably maintainable. To mitigate that, I propose a setting is added that will handle the mapping of id to group name.

Setting: GROUPS_CLAIM_MAPPING
A dictionary of Azure AD Group ID to Django Group Name mappings. When a groups claim contains one of these IDs, the corresponding Django group will be used (and created if needed) using the name from the mapping.

Fund with Polar
@tim-schilling
Copy link
Member Author

I've implemented a workaround in my application by subclassing the backend and decided that I'd be better off creating the groups identified in the mapping on AppConfig.ready rather than key off of MIRROR_GROUPS. The justification here is that the application clearly expects these types of groups so it's fine to create them ahead of time. I assume the purpose of MIRROR_GROUPS is that a user's groups claim could be as high as 100 or 200 with some number of them the being unnecessary to the application so they shouldn't be created ahead of time.

@JonasKs
Copy link
Member

JonasKs commented Jan 29, 2022

Hi. This seems reasonable, but normally I’d recommend implementing graph or roles for this purpose then.

In other words:

  • user authenticates
  • token is validated
  • fetch azure to get groups if groups hasn’t been updated for x time

or:

  • creating application roles and act on those.

@tim-schilling
Copy link
Member Author

I'm a bit hesitant to add another round trip request to my authentication flow. I'll take a look at application roles to see if that'll work better for us.

@JonasKs
Copy link
Member

JonasKs commented Feb 1, 2022

That's how we manage things now atleast, using app roles instead of groups.

@tim-schilling
Copy link
Member Author

@JonasKs I couldn't get app roles to work. It continued to only use the ids. I'm happy to close this issue if the preferred approach is to manually manage it.

@jameskirsop
Copy link

@JonasKs, do you have details on how you've got roles handed back to your Django auth process? I can't seem to see where to add them to the response from AzureAD.

Support for groups in the way that @tim-schilling described would be my preferred option (being able to map names to groups instead of/alongside the UUIDs), but App Roles would be a reasonable substitute should there be documentation on how to make it work...

@JonasKs
Copy link
Member

JonasKs commented Feb 18, 2022

I haven't used company groups for Azure AD, since we default on ADFS. Groups in Azure AD is not nested, which means if a user is member of x, and x is member of y, y is not in the token. For us, this don't work, and we default to application roles for Azure AD applications.

@brammittendorff
Copy link

Hi. This seems reasonable, but normally I’d recommend implementing graph or roles for this purpose then.

In other words:

* user authenticates

* token is validated

* fetch azure to get groups if groups hasn’t been updated for x time

or:

* creating application roles and act on those.

So the second option we would also like to do, are there any examples? Because we do not seem to find anything in the documentation about this?

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

No branches or pull requests

4 participants