Graph-Controls icon indicating copy to clipboard operation
Graph-Controls copied to clipboard

Change WindowsProvider permission scope separator to space

Open Richasy opened this issue 2 years ago • 3 comments

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)
  • [ ] 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

Richasy avatar Jun 17 '22 02:06 Richasy

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 🙌

ghost avatar Jun 17 '22 02:06 ghost

@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.

shweaver-MSFT avatar Jun 21 '22 17:06 shweaver-MSFT

@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

Richasy avatar Jun 22 '22 02:06 Richasy

image

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 avatar Dec 15 '22 21:12 michael-hawker

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.

michael-hawker avatar Dec 15 '22 22:12 michael-hawker