Handle scenario when Entra Application have Group as member #34594
Conversation
jingcheng16
added
the
product/all-users-all-environments
"When you assign a group to an application, only users directly in the group will have access. The assignment does not cascade to nested groups."
corehq/apps/sso/utils/entra.py
Outdated
| else: | ||
| # Service Principal represents an application | ||
| # Which make the query too complicated | ||
| raise NotImplementedError("The application have Service Principal member") |
There was a problem hiding this comment.
how will the user see this error message if this is raised? Is this an error message the user will see directly? Also, should one ServicePrincipal member trigger a failure for the whole task?
Suggested rewrite of error message:
| raise NotImplementedError("The application have Service Principal member") | |
| raise NotImplementedError("ServicePrincipal members are not supported. Please include only Users or Groups as members of this SSO application.") |
There was a problem hiding this comment.
Hi Biyeun, user won't see this error message, this will only be raised as MSGraphIssue.OTHER_ERROR, should we send this to user?
should one ServicePrincipal member trigger a failure for the whole task?
Are you saying we still processed other members in this idp but skip the Service Principal only?
There was a problem hiding this comment.
Nested applications (ServicePrincipal members) are not supported. Please include only Users or Groups as members of this SSO application
We should send this to the user and we should not complete the task in this case, as the list of users would be incomplete
There was a problem hiding this comment.
Changed in 451b524
Co-authored-by: Biyeun <bbuczyk@dimagi.com>
corehq/apps/sso/exceptions.py
Outdated
|
|
||
| class EntraUnsupportedType(Exception): | ||
| def __init__(self, message): | ||
| super().__init__(f"{message}") |
There was a problem hiding this comment.
Yes! You're right. Fixed in f65a830
Technical Summary
Ticket: https://dimagi.atlassian.net/browse/SAAS-15528
When get all roles from an entra application, Microsoft Graph can return
User,Group,ServicePrincipal. We don't handle theServicePrincipalfor now, because it means another entra application, which is too complicated for now.This PR add the support for
Group. It will query all members of the group, and add all users in that group to theuser_idsset. We won't consider members in the nested group, because Microsoft explicitly said: "When you assign a group to an application, only users directly in the group will have access. The assignment does not cascade to nested groups."Review by commit 馃悺
Safety Assurance
Safety story
Tested on staging.
I've tested 4 scenarios:
Automated test coverage
QA Plan
No QA ... ?
Rollback instructions
Labels & Review