Update DeviceCodeRequest/UsernamePasswordRequest to utilize OAuth2Value.ReservedScopes rather than individual constants #5426
Conversation
…ith TokenClient/ScopeHelper * Brings DeviceCodeRequest and UsernamePasswordRequest scopes production into somewhat better alignment with TokenClient.GetDefaultScopes(). * Allows OAuth2Value.ReservedScopes to be respected across all grant types.
@microsoft-github-policy-service agree |
| deviceCodeScopes.Add(OAuth2Value.ScopeProfile); | ||
| deviceCodeScopes.Add(OAuth2Value.ScopeOpenId); | ||
| var deviceCodeScopes = new HashSet<string>(AuthenticationRequestParameters.Scope); | ||
| deviceCodeScopes.UnionWith(OAuth2Value.ReservedScopes); |
There was a problem hiding this comment.
ReservedScopes are just offlineaccess + profile + openId
What is the actual issue you are seeing? Is it perf related?
There was a problem hiding this comment.
Yes, there is no performance or functional difference sought here. The somewhat justifiable intent was to bring these token clients into alignment with TokenClient.GetDefaultScopes(), i.e., consistent use of UnionWith the parameterized scopes.
But full disclosure, I have a (mostly) OIDC-compatible provider that doesn't tolerate the offline_access scope in some cases and another that doesn't take profile on a token request, and this change makes makes patch maintenance more straightforward (and also allows for reflection-based patching of the reserved scopes across all grant types per-provider). Obviously I would have preferred that 6412672 had been merged.
Problem this proposes to address
The
OAuth2Value.ReservedScopescollection is not used byDeviceCodeRequestandUsernamePasswordRequest, even though those requests utilize the same reserved scopes.Changes proposed in this request
DeviceCodeRequestandUsernamePasswordRequestinto somewhat better alignment withTokenClient.GetDefaultScopes().OAuth2Value.ReservedScopesto be respected across all grant types rather than just those that use aTokenClient.Testing
N/A. No impacts to resulting scopes.
Performance impact
N/A. No performance impact.
Documentation
N/A. No documentation changes required.