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

Include original request params on /token request in acquireTokenInteractive #5403

Merged
merged 3 commits into from
Nov 21, 2022

Conversation

tnorling
Copy link
Collaborator

A bug in acquireTokenInteractive resulted in provided scopes and other request parameters being ignored on the /token request. This PR ensures that the /token request parameters match the parameters from the /authorize request

Fixes #5390

@codecov-commenter
Copy link

Codecov Report

Merging #5403 (812cc3a) into dev (f318796) will not change coverage.
The diff coverage is 50.00%.

Flag Coverage Δ *Carryforward flag
msal-angular 96.50% <ø> (ø) Carriedforward from f318796
msal-browser 86.47% <ø> (ø) Carriedforward from f318796
msal-common 85.37% <ø> (ø) Carriedforward from f318796
msal-core 82.84% <ø> (ø) Carriedforward from f318796
msal-node 83.37% <50.00%> (ø)
msal-node-extensions 76.03% <ø> (ø) Carriedforward from f318796
msal-react 94.61% <ø> (ø) Carriedforward from f318796
node-token-validation 88.88% <ø> (ø) Carriedforward from f318796

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
...ib/msal-node/src/client/PublicClientApplication.ts 76.31% <50.00%> (ø)

@@ -17,6 +17,7 @@ export const TEST_CONSTANTS = {
REDIRECT_URI: "http://localhost:8080",
CLIENT_SECRET: "MOCK_CLIENT_SECRET",
DEFAULT_GRAPH_SCOPE: ["user.read"],
DEFAULT_OIDC_SCOPES: ["openid", "profile", "offline_access"],
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be better to import this from msal-common (OIDC_DEFAULT_SCOPES)

Copy link
Contributor

@derisen derisen left a comment

Choose a reason for hiding this comment

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

LGTM! Curios how this didn't become apparent before -is it because the scopes passed to getAuthCodeUrl() would be consented by the user and later on acquireTokenSilent calls would succed using the RT.. 😕

@tnorling
Copy link
Collaborator Author

kenSilent calls would succed using the RT

It's a new feature, not sure how much usage it has gotten yet.

@tnorling tnorling enabled auto-merge (squash) November 21, 2022 17:16
@tnorling tnorling merged commit 27f9a85 into dev Nov 21, 2022
@tnorling tnorling deleted the fix-interactive-token-request branch November 21, 2022 17:21
@ghost
Copy link

ghost commented Nov 22, 2022

🎉@azure/msal-node@v1.14.4 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
msal-node Related to msal-node package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

acquireTokenInteractive Ignores Scopes when Requesting Access Token
4 participants