connector/microsoft: Remove Directory.Read.All requirement #4479
+198
−58
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove Directory.Read.All Requirement from Microsoft Connector
Summary
This PR reworks the Microsoft connector to avoid API calls that require
Directory.Read.Allpermissions, significantly reducing the permission scope needed for the connector to function.Problem
The previous implementation required
Directory.Read.Allpermission to fetch group memberships, which is a highly privileged permission that many organizations are reluctant to grant. This created a barrier to adoption for security-conscious environments.Solution
Instead of using:
/me/getMemberGroups(requiresDirectory.Read.All)/directoryObjects/getByIds(requiresDirectory.Read.All)We now use:
/me/memberOf/microsoft.graph.group- which only requires permissions configured on the app registrationScope Changes
user.readopeniddirectory.read.allhttps://graph.microsoft.com/.defaultThe
.defaultscope tells Microsoft to use the permissions already configured on the app registration, giving administrators full control over what permissions are granted.Changes
Code Changes
scopeUserandscopeGroupsconstants withscopeOpenIDandscopeDefaultget()HTTP method for fetching group memberships via GET requestsgetGroupNames()toqueryGroups()for clarityIdfield to thegroupstruct to support both name and ID formatsTest Changes
TestUserGroupsWithGroupIDFormat- verifies group ID format supportTestLoginURLWithCustomScopes- verifies custom scope configurationTestLoginURLWithOfflineAccess- verifies offline_access scope handlingTestUserGroupsWithWhitelist- verifies whitelist filtering behaviorTestUserGroupsNotInRequiredGroups- verifies required groups validationTestUserGroupsInRequiredGroups- verifies required groups success caseMigration Notes
Users upgrading to this version should:
User.ReadorGroup.Read.All)Directory.Read.Allpermission if it was only used for dexgroupNameFormatconfiguration option continues to work as beforeTesting
All tests pass:
Related Issues