Change WindowsProvider permission scope separator to space

Richasy commented 2 years ago

Fixes #192

PR Type

What kind of change does this PR introduce?

Bugfix

What is the current behavior?

Failed when trying to grant AAD multiple permissions

What is the new behavior?

Grant AAD account multiple permissions normally

PR Checklist

Please check if your PR fulfills the following requirements:

[x] Tested code with current supported SDKs
[ ] Sample in sample app has been added / updated (for bug fixes / features)
- [ ] Icon has been created (if new sample) following the Thumbnail Style Guide and templates
[ ] Tests for the changes have been added (for bug fixes / features) (if applicable)
[x] Header has been added to all new source files (run build/UpdateHeaders.bat)
[x] Contains NO breaking changes

Other information

ghost commented 2 years ago

Thanks Richasy 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 🙌

shweaver-MSFT commented 2 years ago

@Richasy can you share some more details about the failure/error you are seeing? It seems like you are fixing a bug, but it's hard to tell if this could be breaking functionality. I think understanding the error will help us figure out which it is.

Richasy commented 2 years ago

@shweaver-MSFT Here are the error message:

Login failed: Failed Login failed: 3399614466: AADSTS65002: Consent between first party application '{my-app-client-id}' and first party resource '00000003-0000-0000-c000-000000000000' must be configured via preauthorization - applications owned and operated by Microsoft must get approval from the API owner before requesting tokens for that API. Trace ID: 72841998-32b2-46ec-ad49-e804e9d22000 Correlation ID: 9bb2e503-2bbe-4cb2-acb2-3b854f60cbe0 Timestamp: 2022-06-22 02:49:19Z

Code here:

// string[] RequestScopes = { "User.Read", "Tasks.ReadWrite" };
var webAccountProviderConfig = new WebAccountProviderConfig(WebAccountProviderType.Any, Constants.GraphClientId);
var authProvider = new WindowsProvider(RequestScopes, webAccountProviderConfig);

I hide the ClientId. It seems that AAD provider and MSA provider treat scope a little differently. We expected two permissions, A and B, but after connecting with a comma, AAD provider recognized it as one permission, namely A,B, obviously we do not have this permission, so the error message returned is not pre-authorized

michael-hawker commented 1 year ago

Posting the image of that function here as well for full context, nice find @Arlodotexe, still would be good to know where that's called out in the docs. Following up with the Graph doc team.

michael-hawker commented 1 year ago

Ah, got pointed to the doc finally here: https://learn.microsoft.com/en-us/azure/active-directory/develop/v2-protocols-oidc#send-the-sign-in-request

A space-separated list of scopes.

CommunityToolkit / Graph-Controls