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] Fix response handling when navigateToLoginRequestUrl=false #1973

Merged
merged 4 commits into from
Jul 17, 2020
Merged

[msal-browser] Fix response handling when navigateToLoginRequestUrl=false #1973

merged 4 commits into from
Jul 17, 2020

Conversation

pkanher617
Copy link
Contributor

This PR fixes an issue in @azure/msal-browser where the response was not being handled correctly if navigateToLoginRequestUrl=false. Also adds a sample to easily test for this, will add end-to-end tests soon.

@github-actions github-actions bot added msal-browser Related to msal-browser package msal-common Related to msal-common package samples Related to the samples apps for the library. labels Jul 15, 2020
@pkanher617 pkanher617 changed the base branch from dev to add-silent-request July 15, 2020 22:43
tnorling
tnorling requested changes Jul 15, 2020
View reviewed changes
Copy link
Collaborator

@tnorling tnorling left a comment

Choose a reason for hiding this comment

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

Can you add a unit test? Also I would rather find a way to manipulate config options within the E2E test than to create a new sample for each config change we want to test.

@jasonnutter jasonnutter linked an issue Jul 16, 2020 that may be closed by this pull request
handleRedirectPromise not working as expected when navigateToLoginRequestUrl is set to false #1959
Closed
13 tasks
@pkanher617 pkanher617 changed the base branch from add-silent-request to fix-silent-default-scopes July 16, 2020 18:27
@github-actions github-actions bot added the msal@1.x Related to msal@1.x (implicit flow) label Jul 16, 2020
@pkanher617 pkanher617 self-assigned this Jul 16, 2020
tnorling
tnorling approved these changes Jul 16, 2020
View reviewed changes
Copy link
Collaborator

@tnorling tnorling left a comment

Choose a reason for hiding this comment

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

Nitpick: Can we call the sample something like navigateToLoginPageFalse to be more accurate? Dont have a better suggestion atm

@pkanher617 pkanher617 changed the base branch from fix-silent-default-scopes to dev July 17, 2020 03:29
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.005%) to 81.753% when pulling 3a420d1 on fix-navigate-logic-2.0 into 564b211 on dev.

@jasonnutter jasonnutter merged commit 0ef8b29 into dev Jul 17, 2020
@pkanher617 pkanher617 deleted the fix-navigate-logic-2.0 branch August 1, 2020 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasonnutter jasonnutter jasonnutter approved these changes

@tnorling tnorling tnorling approved these changes

@DarylThayil DarylThayil Awaiting requested review from DarylThayil

@jmckennon jmckennon Awaiting requested review from jmckennon

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

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

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

Assignees

@pkanher617 pkanher617

Labels
msal@1.x Related to msal@1.x (implicit flow) 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
Development

Successfully merging this pull request may close these issues.

handleRedirectPromise not working as expected when navigateToLoginRequestUrl is set to false
4 participants
@pkanher617 @coveralls @jasonnutter @tnorling