Ask for fewer scopes and sign in once - #9
Merged
Merged
Conversation
The widest plausible scopes were chosen deliberately: signing in through Graph PowerShell means borrowing whatever that shared application has already been consented, so the scope most likely to be granted mattered more than the narrowest one that would do. Directory.Read.All and RoleManagement.ReadWrite.Directory won on that basis. That reasoning does not survive registering an application of your own, where the consented set is whatever you declare, so the narrowest scope that works is also the one that gets consented. Group.Read.All covers the group name lookup, which is the only thing Directory.Read.All was ever here for, and reading the assignment schedules and writing their requests is what RoleAssignmentSchedule.ReadWrite.Directory is for, rather than write access to every other directory RBAC setting. Nothing the commands do changes. Group names still resolve, and roleDefinition still expands on the schedule resources under the narrower scope, so role names continue to be shown without RoleManagement.Read.Directory. The cost falls on tenants still reaching PIM through Graph PowerShell, where the narrower scopes are less likely to have been consented already. Such a tenant can still name the set it has with --scope, which the README now points at from this direction instead of the other.
The cache file was named for the tenant and a digest of the scopes and claims, which left out the one thing that changes when someone passes --client-id. With the tenant and scopes unchanged the name came out the same, so switching application handed back a token issued to the previous one, silently and precisely in the case where the caller is trying to stop using it. A token belongs to the application it was issued to. Two applications are consented separately and the audience differs, so their tokens are no more interchangeable than two scope sets are, and they are now filed apart for the same reason. The parts of the key are joined by newlines, which none of them can contain. Claims were previously appended after a space, leaving them contiguous with the scope list they followed, so keys that differed could in principle have rendered alike.
The refresh token was filed with the access token it arrived with, under a key that included the scopes. Asking for the other area found nothing to renew and opened the browser again, so using both roles and groups meant signing in twice for no reason a user could see. The identity platform issues one refresh token per application, not per scope set, and will redeem it for any scopes that application has been consented. The scopes it happened to be obtained alongside say nothing about what it can fetch, so keying on them was the whole of the problem. It is now filed per application and tenant, and access tokens keep their own entries, since those really are only good for the scopes they were issued with. Signing in for one area now covers the other, which redeems the same refresh token silently. An area the tenant has not consented to is refused at the token endpoint and falls through to a sign-in, so a tenant that consented to one area alone is no worse off than before: the refusal surfaces where it can be answered rather than being reported as a failure. A claims challenge still goes straight to the browser. A refresh cannot produce the acrs claim being asked for, whichever key it was found under. Resolving the cache directory is separated from naming a file within it so that the two keys a call needs are derived from one directory lookup, leaving a single place where that lookup can fail.
winebarrel
enabled auto-merge
September 9, 2026 05:04
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9 +/- ##
==========================================
+ Coverage 96.98% 97.04% +0.06%
==========================================
Files 7 7
Lines 531 542 +11
==========================================
+ Hits 515 526 +11
Misses 16 16 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Three changes that come out of registering an application of your own instead of
signing in through Graph PowerShell.
Ask for only the scopes each area uses
The widest plausible scopes were chosen on purpose: borrowing a shared
first-party application means the scope most likely to have been consented
already matters more than the narrowest one that would do. That reasoning does
not survive registering your own application, where the consented set is
whatever you declare.
Directory.Read.All→Group.Read.All, which covers the group name lookupthat was the only thing it was ever here for
RoleManagement.ReadWrite.Directory→RoleAssignmentSchedule.ReadWrite.Directory,rather than write access to every other directory RBAC setting
Nothing the commands do changes. Group names still resolve, and
roleDefinitionstill expands on the schedule resources under the narrower scope, so role names
are shown without
RoleManagement.Read.Directory. Verified against a realtenant: same output, same rows, for both areas.
The cost falls on tenants still reaching PIM through Graph PowerShell, where the
narrower scopes are less likely to have been consented.
--scopestill namesthe set a tenant has, and the README now points at it from this direction.
Key the token cache by application
The cache file was named for the tenant and a digest of the scopes and claims,
leaving out the one thing that changes when someone passes
--client-id. Withthe tenant and scopes unchanged the name came out the same, so switching
application handed back a token issued to the previous one -- precisely in the
case where the caller is trying to stop using it.
Sign in once for both areas
The refresh token was filed with the access token it arrived with, under a key
that included the scopes, so asking for the other area found nothing to renew
and opened the browser again. Using both roles and groups meant signing in twice
for no reason a user could see.
The identity platform issues one refresh token per application, not per scope
set, and redeems it for any scopes that application has been consented. It is
now filed per application and tenant; access tokens keep their own entries,
since those really are only good for the scopes they were issued with.
Per-area independence is unchanged. An area the tenant has not consented to is
refused at the token endpoint and falls through to a sign-in, so the refusal
surfaces where it can be answered rather than being reported as a failure. A
claims challenge still goes straight to the browser, since a refresh cannot
produce the acrs claim being asked for.