Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix AccessToken scope for vanity domains #4630

Merged
merged 3 commits into from
Dec 15, 2024
Merged

Conversation

jhholm
Copy link
Contributor

@jhholm jhholm commented Dec 13, 2024

Type

  • Bug Fix
  • New Feature
  • Sample

Related Issues?

Fixes #4594, probably fixes pnp/pnpframework#1091

What is in this Pull Request ?

The issue in #4594 is caused by calling GetAccessTokenAsync with "https://{resource}", but the logic behind the function is different between the overloaded implementations. Vanity domains will call string[] overload implementation and regular sharepoint domains will call string overload implementation of GetAcccessTokenAsync.

string overload appends './default' to scope
https://github.com/pnp/pnpframework/blob/569dd302a080d26ae21f098c0af2b70e398a8968/src/lib/PnP.Framework/AuthenticationManager.cs#L763

string[] overload doesn't append './default' to the scopes
https://github.com/pnp/pnpframework/blob/569dd302a080d26ae21f098c0af2b70e398a8968/src/lib/PnP.Framework/AuthenticationManager.cs#L806

@jhholm
Copy link
Contributor Author

jhholm commented Dec 13, 2024

Just noticed that I need to change the order. The reason it actually still worked, is because it reverts to the old logic that had no scopes at all.

@jhholm
Copy link
Contributor Author

jhholm commented Dec 13, 2024

Should be ok now

@gautamdsheth gautamdsheth merged commit c473439 into pnp:dev Dec 15, 2024
@gautamdsheth
Copy link
Collaborator

LGTM , thanks a lot @jhholm for this PR , merged it, much appreciated !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants