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.

Sign up for GitHub

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

Jump to bottom

[msal-browser] Remove default scopes from silent flow #1962

Merged
merged 11 commits into from
Jul 17, 2020
Merged

[msal-browser] Remove default scopes from silent flow #1962

merged 11 commits into from
Jul 17, 2020

Conversation

pkanher617
Copy link
Contributor

This PR removes the addition of openid and profile scopes when passing a requests to the SilentFlowClient in msal-common. This prevents cache misses when requesting from non-graph resources.

@github-actions github-actions bot added msal-browser Related to msal-browser package samples Related to the samples apps for the library. labels Jul 15, 2020
@coveralls
Copy link

coveralls commented Jul 15, 2020 •
edited
Loading

Coverage Status

Coverage remained the same at 81.758% when pulling 0f18c9e on fix-silent-default-scopes into e9a4e70 on dev.

@sameerag
Copy link
Member

Is this applicable to msal-node too?

Merge pull request #1964 from AzureAD/add-silent-request
3a6c4c5 
[msal-browser] Add SilentRequest.ts
@github-actions github-actions bot added the msal-common Related to msal-common package label Jul 16, 2020
@pkanher617
Copy link
Contributor Author

Is this applicable to msal-node too?

Yes I think you do not need to add default scopes to acquireTokenByRefreshToken and acquireTokenSilent in ClientApplication.ts

sameerag
sameerag approved these changes Jul 16, 2020
View reviewed changes
Copy link
Member

@sameerag sameerag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm. As discussed, please add an internal issue to track this behavior for msal-node

@pkanher617 pkanher617 mentioned this pull request Jul 16, 2020
Closed
8 tasks
@pkanher617 pkanher617 linked an issue Jul 16, 2020 that may be closed by this pull request
Access tokens in the cache are not used because all requests have openid and profile scopes #1936
Closed
13 tasks
@pkanher617 pkanher617 self-assigned this Jul 16, 2020
@pkanher617 pkanher617 linked an issue Jul 16, 2020 that may be closed by this pull request
Some questions about using msal@2.x with SPA and B2C - acquireTokenSilent cache and refresh_token #1932
Closed
4 tasks
@pkanher617 pkanher617 merged commit 564b211 into dev Jul 17, 2020
@pkanher617 pkanher617 deleted the fix-silent-default-scopes branch August 1, 2020 00:07
@dbpieter dbpieter mentioned this pull request Aug 10, 2020
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tnorling tnorling tnorling approved these changes

@sameerag sameerag sameerag approved these changes

@DarylThayil DarylThayil Awaiting requested review from DarylThayil

@jasonnutter jasonnutter Awaiting requested review from jasonnutter

@jmckennon jmckennon Awaiting requested review from jmckennon

@jo-arroyo jo-arroyo Awaiting requested review from jo-arroyo jo-arroyo is a code owner

@hectormmg hectormmg Awaiting requested review from hectormmg hectormmg is a code owner

Assignees

@pkanher617 pkanher617

Labels
msal-browser Related to msal-browser package msal-common Related to msal-common package samples Related to the samples apps for the library.
Projects
None yet
Milestone
@azure/msal-browser@2.0.0 - GA Release
4 participants
@pkanher617 @coveralls @sameerag @tnorling