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

Support COOP same-origin-allow-popups #7408

Open
wants to merge 4 commits into
base: popup-coop-sample
Choose a base branch
from
Open

Support COOP same-origin-allow-popups #7408

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9cd82be
Update PopupClient.ts
vickiez Nov 7, 2024
5aa33f1
Update RedirectClient.ts
vickiez Nov 7, 2024
6585340
Delete commented out code in PopupClient.ts
vickiez Nov 7, 2024
d269829
Remove timeout in PopupClient.ts
vickiez Nov 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 65 additions & 47 deletions lib/msal-browser/src/interaction_client/PopupClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export type PopupParams = {
export class PopupClient extends StandardInteractionClient {
private currentWindow: Window | undefined;
protected nativeStorage: BrowserCacheManager;
private authChannel: BroadcastChannel;
Copy link

Choose a reason for hiding this comment

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

Seems like BroadcastChannel is well supported (https://caniuse.com/broadcastchannel) although not sure if this PopupClient.ts code needs to run in super old contexts. If so, they might want to check if BroadcastChannel doesn't exist and fallback to their URI change polling code. But if this is demo fix for them then probably none of that is necessary anyway


constructor(
config: BrowserConfiguration,
Expand Down Expand Up @@ -85,6 +86,7 @@ export class PopupClient extends StandardInteractionClient {
// Properly sets this reference for the unload event.
this.unloadWindow = this.unloadWindow.bind(this);
this.nativeStorage = nativeStorageImpl;
this.authChannel = new BroadcastChannel('auth');
}

/**
Expand Down Expand Up @@ -546,58 +548,71 @@ export class PopupClient extends StandardInteractionClient {
popupWindowParent: Window
): Promise<string> {
return new Promise<string>((resolve, reject) => {
this.logger.verbose(
"PopupHandler.monitorPopupForHash - polling started"
);

const intervalId = setInterval(() => {
// Window is closed
if (popupWindow.closed) {
this.logger.error(
"PopupHandler.monitorPopupForHash - window closed"
);
clearInterval(intervalId);
setTimeout(() => {
vickiez marked this conversation as resolved.
Show resolved Hide resolved
this.logger.verbose(
"PopupHandler.monitorPopupForHash - polling started"
);
this.authChannel.onmessage = function (event) {
resolve(event.data);
}
this.authChannel.onmessageerror = function (event) {
console.log('BroadcastChannel error', event.data);
reject(
createBrowserAuthError(
BrowserAuthErrorCodes.userCancelled
BrowserAuthErrorCodes.popupWindowError
)
);
return;
}

let href = "";
try {
/*
* Will throw if cross origin,
* which should be caught and ignored
* since we need the interval to keep running while on STS UI.
*/
href = popupWindow.location.href;
} catch (e) {}

// Don't process blank pages or cross domain
if (!href || href === "about:blank") {
return;
}
clearInterval(intervalId);

let responseString = "";
const responseType =
this.config.auth.OIDCOptions.serverResponseType;
if (popupWindow) {
if (responseType === ServerResponseType.QUERY) {
responseString = popupWindow.location.search;
} else {
responseString = popupWindow.location.hash;
}
}

this.logger.verbose(
"PopupHandler.monitorPopupForHash - popup window is on same origin as caller"
);

resolve(responseString);
}
}, this.config.system.pollIntervalMilliseconds);

// const intervalId = setInterval(() => {
vickiez marked this conversation as resolved.
Show resolved Hide resolved
// // Window is closed
// if (popupWindow.closed) {
// this.logger.error(
// "PopupHandler.monitorPopupForHash - window closed"
// );
// clearInterval(intervalId);
// reject(
// createBrowserAuthError(
// BrowserAuthErrorCodes.userCancelled
// )
// );
// return;
// }

// let href = "";
// try {
// /*
// * Will throw if cross origin,
// * which should be caught and ignored
// * since we need the interval to keep running while on STS UI.
// */
// href = popupWindow.location.href;
// } catch (e) {}

// // Don't process blank pages or cross domain
// if (!href || href === "about:blank") {
// return;
// }
// clearInterval(intervalId);

// let responseString = "";
// const responseType =
// this.config.auth.OIDCOptions.serverResponseType;
// if (popupWindow) {
// if (responseType === ServerResponseType.QUERY) {
// responseString = popupWindow.location.search;
// } else {
// responseString = popupWindow.location.hash;
// }
// }

// this.logger.verbose(
// "PopupHandler.monitorPopupForHash - popup window is on same origin as caller"
// );

// resolve(responseString);
// }, this.config.system.pollIntervalMilliseconds);
}).finally(() => {
this.cleanPopup(popupWindow, popupWindowParent);
});
Expand Down Expand Up @@ -763,6 +778,9 @@ export class PopupClient extends StandardInteractionClient {
// Close window.
popupWindow.close();

// Close the broadcast channel
this.authChannel.close();

// Remove window unload function
popupWindowParent.removeEventListener(
"beforeunload",
Expand Down
12 changes: 11 additions & 1 deletion lib/msal-browser/src/interaction_client/RedirectClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ function getNavigationType(): NavigationTimingType | undefined {

export class RedirectClient extends StandardInteractionClient {
protected nativeStorage: BrowserCacheManager;
private authChannel = new BroadcastChannel('auth');

constructor(
config: BrowserConfiguration,
Expand Down Expand Up @@ -248,6 +249,9 @@ export class RedirectClient extends StandardInteractionClient {
"Back navigation event detected. Muting no_server_response error"
);
}
// Logout with popup case
this.authChannel.postMessage("");
window.close();
return null;
}

Expand Down Expand Up @@ -394,7 +398,8 @@ export class RedirectClient extends StandardInteractionClient {
ResponseHandler.validateInteractionType(
response,
this.browserCrypto,
InteractionType.Redirect
// TODO: Use correct type depending on the situation
InteractionType.Popup
);
} catch (e) {
if (e instanceof AuthError) {
Expand Down Expand Up @@ -422,6 +427,11 @@ export class RedirectClient extends StandardInteractionClient {

if (cachedHash) {
response = UrlUtils.getDeserializedResponse(cachedHash);

// Login with popup case
this.authChannel.postMessage(cachedHash);
window.close();

if (response) {
this.logger.verbose(
"Hash does not contain known properties, returning cached hash"
Expand Down
Loading