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

Authorization Code Flow for Single Page Applications: Cleanup Popup Flows and Window Nav #1236

Conversation

pkanher617
Copy link
Contributor

@pkanher617 pkanher617 commented Jan 23, 2020

This PR adds cleanup for different popup related errors, as well as window navigation actions such as redirect and refresh. This does not affect token acquisition, but does force users to be aware that handleRedirectCallback() should not be used with the popup APIs, as it will cause the window to handle the response in the popup, rather than the main window.

This should fix the issue raised in #1231.

Copy link
Contributor

@jmckennon jmckennon left a comment

Choose a reason for hiding this comment

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

Looks good, I still think we could do with less inline comments overall, but I appreciate the focus and smaller size on this PR. Much easier to review and understand!

samples/VanillaJSTestApp/index_authCode.html Show resolved Hide resolved
Copy link
Contributor

@jasonnutter jasonnutter left a comment

Choose a reason for hiding this comment

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

Need to use addEventListener and removeEventListener.

lib/msal-browser/src/interaction_handler/PopupHandler.ts Outdated Show resolved Hide resolved
lib/msal-browser/src/interaction_handler/PopupHandler.ts Outdated Show resolved Hide resolved
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.

Approving. Please close Jason's comments and merge it.

@pkanher617 pkanher617 merged commit db98b9f into authorization-code-flow-base Jan 29, 2020
@pkanher617 pkanher617 deleted the authorization-code-flow-cleanupPopup-flows branch March 12, 2020 22:59
azure-pipelines bot pushed a commit to pkanher617/microsoft-authentication-library-for-js that referenced this pull request Jan 4, 2022
…cleanupPopup-flows

Authorization Code Flow for Single Page Applications: Cleanup Popup Flows and Window Nav
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants