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

AZURE_AD_ALLOWED_PRINCIPALS #192

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

yaegashi
Copy link

This pull request adds support for AZURE_AD_ALLOWED_PRINCIPALS, which allows specifying a comma-separated list of Object IDs for Entra ID (Azure AD) user/group principals that are allowed to log in to the app. If the list is empty, all authenticated users are allowed to log in.

This feature utilizes the Microsoft Graph API endpoint /me/getMemberObjects, enabling support for transitive group members. It is highly beneficial as it allows managing access for individuals within a multi-nested organizational group hierarchy, which is commonly found in large enterprise tenants.

@yaegashi
Copy link
Author

@microsoft-github-policy-service agree

@yaegashi yaegashi force-pushed the azure-ad-allowed-principals branch from 59f78fc to e3515ed Compare September 28, 2023 09:49
New environment variable AZURE_AD_ALLOWED_PRINCIPALS is a comma separated list
of Object IDs for Azure AD user/group principals that are allowed to log in to
the app. If the list is empty, all authenticated users are allowed to log in.

This feature utilizes the Microsoft Graph API endpoint /me/getMemberObjects.
@yaegashi yaegashi force-pushed the azure-ad-allowed-principals branch from 81424e0 to e7e0726 Compare October 4, 2023 06:05
@lyajedi
Copy link

lyajedi commented Nov 3, 2023

This looks nice but one note may need to be added for the scope, User.Read requires having admin consent granted first.
For some orgs, this may be impossible and in that case it's better to just have authorization handled with the roles or groups claim in ID token instead.

@yaegashi
Copy link
Author

yaegashi commented Nov 4, 2023

Hi @lyajedi, thanks for the comment,

In my org, I belong to more than 200 groups, so my ID token cannot include a list of object ids in the groups claim. Instead, it has only a reference to an Azure AD Graph endpoint (graph.windows.net) as described in this document. My understanding is that there is no other way than using MS Graph call to correctly handle all the possible situations.

@lyajedi
Copy link

lyajedi commented Nov 4, 2023

Hi @lyajedi, thanks for the comment,

In my org, I belong to more than 200 groups, so my ID token cannot include a list of object ids in the groups claim. Instead, it has only a reference to an Azure AD Graph endpoint (graph.windows.net) as described in this document. My understanding is that there is no other way than using MS Graph call to correctly handle all the possible situations.

I also have more than 200 groups, and grant admin consent requires approval, so I have 3 alternative solutions for this case:

  1. Create app roles with a name like "chatuser" in app registration and assign the groups to the roles, use roles claim in ID token for RBAC (if "chatuser" in roles claim in ID token).
  2. Add groups claim in app registration -> Token configuration, check all groups for id token, then go to Enterprise Applications -> Single sign-on -> Edit Attributes & Claims, click the groups claim and add a filter, in my org the groups for access control are usually named as access--, so I can change the filter to match with prefix "access-" or something and this will make the groups in the ID token fewer than 200 groups. This approach doesn't require to update Azure AD app configuration like assigning roles every time you have new groups to add to ACL.
  3. Do the same thing in 1), and then check Groups assigned to the application while adding groups claim in app registration -> Token configuration, assign the required groups to the roles in Enterprise Application, then the groups claim in ID token will just have the assigned groups. However, this approach is redundant comparing to the first approach.
image

For an AAD app, if admin consent is granted, can just enable Assignment required? in Enterprise Applications -> Properties, and assign the users/groups to a role, this will deny the non-assigned users/groups during the OAuth flow and don't require any handling in code.

I eventually added the access control with approach 2), as I don't want to spend time on going through the procedure and getting the approval for admin consent (may take weeks), I just changed your code under try-catch part like below

          isAllowed = false
          decodedToken = jwt.decode(tokens.id_token);
          isAllowed = azureAdAllowedPrincipals.some(group => decodedToken.groups.includes(group));

@yaegashi
Copy link
Author

yaegashi commented Nov 6, 2023

I had never realized the actual use case of group filters (your 2nd solution) before. It serves as an excellent workaround for the limitation of a maximum of 200 entries in a group claim. I agree that my code should reference the content of ID tokens exclusively, rather than going through the trouble of calling the MS Graph API.

Other solutions do not support nested members in the group assigned to apps, so they were not considered as options from the beginning.

@checkso
Copy link

checkso commented Nov 14, 2023

Hi guys,
was just reading through your conversation.
Wouldn't it be sufficient to just check the "assignment needed" on the enterprise application.
Than only the users/groups you add to the enterprise application will have access to the application.

@yaegashi
Copy link
Author

@checkso Yes, it will work for groups or organizations with a small number of users. However, as I mentioned earlier, it may not be sufficient for large enterprises with complex, multi-level group hierarchies, as it does not support the inclusion of nested group members within an assigned group.

@checkso
Copy link

checkso commented Nov 15, 2023

@yaegashi yes you are right, might not work for large enterprises.
I went with the Assignment required flag and use dynamic groups with the memberof filter.

@lyajedi
Copy link

lyajedi commented Nov 16, 2023

Hi guys, was just reading through your conversation. Wouldn't it be sufficient to just check the "assignment needed" on the enterprise application. Than only the users/groups you add to the enterprise application will have access to the application.

I mentioned assignment required in my reply before, but this still requires admin consent to be granted first to scopes like user profile read.

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

Successfully merging this pull request may close these issues.

3 participants