Add optional Scopes parameter to BaseProvider

CommunityToolkit / Graph-Controls

Set of Helpers and Controls for Windows development using the Microsoft Graph.

https://docs.microsoft.com/en-us/windows/communitytoolkit/graph/overview

Other

154 stars 39 forks source link

Add optional Scopes parameter to BaseProvider #183

Closed zateutsch closed 2 years ago

zateutsch commented 2 years ago

Fixes #

PR Type

What kind of change does this PR introduce?

Feature

What is the current behavior?

GetTokenAsync always requests token with scopes set in class construction.

What is the new behavior?

Optional parameter for GetTokenAsync to supply scopes specific to token request.

Please check if your PR fulfills the following requirements:

[x] Tested code with current supported SDKs
[x] Contains NO breaking changes

Other information

Change was already included in documentation, must have been lost along the way.

ghost commented 2 years ago

Thanks zateutsch for opening a Pull Request! The reviewers will test the PR and highlight if there is any merge conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

michael-hawker commented 2 years ago

Hmm.. I thought there was a reason we didn't do this as there was an underlying issue trying to change the requested scope later...

@shweaver-MSFT mind filling the knowledge gap here?

michael-hawker commented 2 years ago

@zateutsch found the previous reversion of this in PR #122 from @shweaver-MSFT. FYI @nmetulev.

shweaver-MSFT commented 2 years ago

@michael-hawker, good job digging up that older PR. You are so right, I removed that functionality because WAM will fail if you pass in new scopes. To quote myself:

WAM doesn't support incremental consent, so this function is deceiving. Only pre-authorized scopes will return a token successfully, in which case they should be included with the rest of the scopes provided during provider construction.