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

Make popup poll interval configurable #5276

Merged
merged 9 commits into from
Oct 10, 2022
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Make popup poll interval configurable #5276",
"packageName": "@azure/msal-browser",
"email": "hemoral@microsoft.com",
"dependentChangeType": "patch"
}
1 change: 1 addition & 0 deletions lib/msal-browser/docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ See [Caching in MSAL](./caching.md) for more.
| `asyncPopups` | Sets whether popups are opened asynchronously. When set to false, blank popups are opened before anything else happens. When set to true, popups are opened when making the network request. Can be set to true for scenarios where `about:blank` is not supported, e.g. desktop apps or progressive web apps | boolean | `false` |
| `allowRedirectInIframe` | By default, MSAL will not allow redirect operations to be initiated when the application is inside an iframe. Set this flag to `true` to remove this check. | boolean | `false` |
| `cryptoOptions` | Config object for crypto operations in the browser. | See [below](#crypto-config-options.) | See [below](#crypto-config-options.) |
| `pollIntervalMilliseconds` | Interval of time in milliseconds between polls of popup URL hash during authenticaiton. | integer (milliseconds) | `30` |

#### Logger Config Options

Expand Down
7 changes: 6 additions & 1 deletion lib/msal-browser/src/config/Configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { SystemOptions, LoggerOptions, INetworkModule, DEFAULT_SYSTEM_OPTIONS, Constants, ProtocolMode, LogLevel, StubbedNetworkModule, AzureCloudInstance, AzureCloudOptions, ApplicationTelemetry } from "@azure/msal-common";
import { BrowserUtils } from "../utils/BrowserUtils";
import { BrowserCacheLocation } from "../utils/BrowserConstants";
import { BrowserCacheLocation, BrowserConstants } from "../utils/BrowserConstants";
import { INavigationClient } from "../navigation/INavigationClient";
import { NavigationClient } from "../navigation/NavigationClient";

Expand Down Expand Up @@ -140,6 +140,10 @@ export type BrowserSystemOptions = SystemOptions & {
* Options related to browser crypto APIs
*/
cryptoOptions?: CryptoOptions;
/**
* Sets the interval length in milliseconds for polling the location attribute in popup windows (default is 30ms)
*/
pollIntervalMilliseconds?: number;
};

export type CryptoOptions = {
Expand Down Expand Up @@ -259,6 +263,7 @@ export function buildConfiguration({ auth: userInputAuth, cache: userInputCache,
allowRedirectInIframe: false,
allowNativeBroker: false,
nativeBrokerHandshakeTimeout: userInputSystem?.nativeBrokerHandshakeTimeout || DEFAULT_NATIVE_BROKER_HANDSHAKE_TIMEOUT_MS,
pollIntervalMilliseconds: BrowserConstants.DEFAULT_POLL_INTERVAL_MS,
cryptoOptions: {
useMsrCrypto: false,
entropy: undefined
Expand Down
6 changes: 3 additions & 3 deletions lib/msal-browser/src/interaction_client/PopupClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ export class PopupClient extends StandardInteractionClient {
* Polling for popups needs to be tick-based,
* since a non-trivial amount of time can be spent on interaction (which should not count against the timeout).
*/
const maxTicks = this.config.system.windowHashTimeout / BrowserConstants.POLL_INTERVAL_MS;
const maxTicks = this.config.system.windowHashTimeout / this.config.system.pollIntervalMilliseconds;
let ticks = 0;

this.logger.verbose("PopupHandler.monitorPopupForHash - polling started");
Expand Down Expand Up @@ -348,7 +348,7 @@ export class PopupClient extends StandardInteractionClient {
clearInterval(intervalId);
reject(BrowserAuthError.createMonitorPopupTimeoutError());
}
}, BrowserConstants.POLL_INTERVAL_MS);
}, this.config.system.pollIntervalMilliseconds);
});
}

Expand Down Expand Up @@ -390,7 +390,7 @@ export class PopupClient extends StandardInteractionClient {
clearInterval(intervalId);
this.cleanPopup(popupWindow);
resolve();
}, BrowserConstants.POLL_INTERVAL_MS);
}, this.config.system.pollIntervalMilliseconds);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export class SilentAuthCodeClient extends StandardInteractionClient {
this.logger.verbose("Auth code client created");

// Create silent handler
const silentHandler = new SilentHandler(authClient, this.browserStorage, authCodeRequest, this.logger, this.config.system.navigateFrameWait);
const silentHandler = new SilentHandler(authClient, this.browserStorage, authCodeRequest, this.logger, this.config.system);

// Handle auth code parameters from request
return silentHandler.handleCodeResponseFromServer(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export class SilentIframeClient extends StandardInteractionClient {
nativeBroker: NativeMessageHandler.isNativeAvailable(this.config, this.logger, this.nativeMessageHandler, silentRequest.authenticationScheme)
});
// Create silent handler
const silentHandler = new SilentHandler(authClient, this.browserStorage, authCodeRequest, this.logger, this.config.system.navigateFrameWait);
const silentHandler = new SilentHandler(authClient, this.browserStorage, authCodeRequest, this.logger, this.config.system);
// Get the frame handle for the silent request
const msalFrame = await silentHandler.initiateAuthRequest(navigateUrl);
// Monitor the window for the hash. Return the string value and close the popup when the hash is received. Default timeout is 60 seconds.
Expand Down
12 changes: 7 additions & 5 deletions lib/msal-browser/src/interaction_handler/SilentHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,19 @@

import { UrlString, StringUtils, CommonAuthorizationCodeRequest, AuthorizationCodeClient, Constants, Logger } from "@azure/msal-common";
import { InteractionHandler } from "./InteractionHandler";
import { BrowserConstants } from "../utils/BrowserConstants";
import { BrowserAuthError } from "../error/BrowserAuthError";
import { BrowserCacheManager } from "../cache/BrowserCacheManager";
import { DEFAULT_IFRAME_TIMEOUT_MS } from "../config/Configuration";
import { BrowserSystemOptions, DEFAULT_IFRAME_TIMEOUT_MS } from "../config/Configuration";

export class SilentHandler extends InteractionHandler {

private navigateFrameWait: number;
constructor(authCodeModule: AuthorizationCodeClient, storageImpl: BrowserCacheManager, authCodeRequest: CommonAuthorizationCodeRequest, logger: Logger, navigateFrameWait: number) {
private pollIntervalMilliseconds: number;

constructor(authCodeModule: AuthorizationCodeClient, storageImpl: BrowserCacheManager, authCodeRequest: CommonAuthorizationCodeRequest, logger: Logger, systemOptions: Required<Pick<BrowserSystemOptions, "navigateFrameWait" | "pollIntervalMilliseconds">>) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Make sure this change also gets addressed in 1P

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! Will do!

super(authCodeModule, storageImpl, authCodeRequest, logger);
this.navigateFrameWait = navigateFrameWait;
this.navigateFrameWait = systemOptions.navigateFrameWait;
this.pollIntervalMilliseconds = systemOptions.pollIntervalMilliseconds;
}

/**
Expand Down Expand Up @@ -82,7 +84,7 @@ export class SilentHandler extends InteractionHandler {
resolve(contentHash);
return;
}
}, BrowserConstants.POLL_INTERVAL_MS);
}, this.pollIntervalMilliseconds);
});
}

Expand Down
2 changes: 1 addition & 1 deletion lib/msal-browser/src/utils/BrowserConstants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export const BrowserConstants = {
/**
* Default popup monitor poll interval in milliseconds
*/
POLL_INTERVAL_MS: 50,
DEFAULT_POLL_INTERVAL_MS: 30,
/**
* Msal-browser SKU
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { TestStorageManager } from "../cache/TestStorageManager";
import { BrowserCacheManager } from "../../src/cache/BrowserCacheManager";

const DEFAULT_IFRAME_TIMEOUT_MS = 6000;
const DEFAULT_POLL_INTERVAL_MS = 30;

const testPkceCodes = {
challenge: "TestChallenge",
Expand Down Expand Up @@ -128,7 +129,7 @@ describe("SilentHandler.ts Unit Tests", () => {
describe("Constructor", () => {

it("creates a subclass of InteractionHandler called SilentHandler", () => {
const silentHandler = new SilentHandler(authCodeModule, browserStorage, defaultTokenRequest, browserRequestLogger, DEFAULT_IFRAME_TIMEOUT_MS);
const silentHandler = new SilentHandler(authCodeModule, browserStorage, defaultTokenRequest, browserRequestLogger, { navigateFrameWait: DEFAULT_IFRAME_TIMEOUT_MS, pollIntervalMilliseconds: DEFAULT_POLL_INTERVAL_MS });
expect(silentHandler instanceof SilentHandler).toBe(true);
expect(silentHandler instanceof InteractionHandler).toBe(true);
});
Expand All @@ -137,22 +138,22 @@ describe("SilentHandler.ts Unit Tests", () => {
describe("initiateAuthRequest()", () => {

it("throws error if requestUrl is empty", async () => {
const silentHandler = new SilentHandler(authCodeModule, browserStorage, defaultTokenRequest, browserRequestLogger, DEFAULT_IFRAME_TIMEOUT_MS);
const silentHandler = new SilentHandler(authCodeModule, browserStorage, defaultTokenRequest, browserRequestLogger, { navigateFrameWait: DEFAULT_IFRAME_TIMEOUT_MS, pollIntervalMilliseconds: DEFAULT_POLL_INTERVAL_MS });
await expect(silentHandler.initiateAuthRequest("")).rejects.toMatchObject(BrowserAuthError.createEmptyNavigationUriError());
//@ts-ignore
await expect(silentHandler.initiateAuthRequest(null)).rejects.toMatchObject(BrowserAuthError.createEmptyNavigationUriError());
});

it("Creates a frame asynchronously when created with default timeout", async () => {
const silentHandler = new SilentHandler(authCodeModule, browserStorage, defaultTokenRequest, browserRequestLogger, DEFAULT_IFRAME_TIMEOUT_MS);
const silentHandler = new SilentHandler(authCodeModule, browserStorage, defaultTokenRequest, browserRequestLogger, { navigateFrameWait: DEFAULT_IFRAME_TIMEOUT_MS, pollIntervalMilliseconds: DEFAULT_POLL_INTERVAL_MS });
const loadFrameSpy = sinon.spy(silentHandler, <any>"loadFrame");
const authFrame = await silentHandler.initiateAuthRequest(testNavUrl);
expect(loadFrameSpy.called).toBe(true);
expect(authFrame instanceof HTMLIFrameElement).toBe(true);
}, DEFAULT_IFRAME_TIMEOUT_MS + 1000);

it("Creates a frame synchronously when created with a timeout of 0", async () => {
const silentHandler = new SilentHandler(authCodeModule, browserStorage, defaultTokenRequest, browserRequestLogger, 0);
const silentHandler = new SilentHandler(authCodeModule, browserStorage, defaultTokenRequest, browserRequestLogger, { navigateFrameWait: 0, pollIntervalMilliseconds: DEFAULT_POLL_INTERVAL_MS } );
const loadFrameSyncSpy = sinon.spy(silentHandler, <any>"loadFrameSync");
const loadFrameSpy = sinon.spy(silentHandler, <any>"loadFrame");
const authFrame = await silentHandler.initiateAuthRequest(testNavUrl);
Expand All @@ -171,7 +172,7 @@ describe("SilentHandler.ts Unit Tests", () => {
}
};

const silentHandler = new SilentHandler(authCodeModule, browserStorage, defaultTokenRequest, browserRequestLogger, DEFAULT_IFRAME_TIMEOUT_MS);
const silentHandler = new SilentHandler(authCodeModule, browserStorage, defaultTokenRequest, browserRequestLogger, { navigateFrameWait: DEFAULT_IFRAME_TIMEOUT_MS, pollIntervalMilliseconds: DEFAULT_POLL_INTERVAL_MS });
// @ts-ignore
silentHandler.monitorIframeForHash(iframe, 500)
.catch(() => {
Expand All @@ -191,7 +192,7 @@ describe("SilentHandler.ts Unit Tests", () => {
}
};

const silentHandler = new SilentHandler(authCodeModule, browserStorage, defaultTokenRequest, browserRequestLogger, DEFAULT_IFRAME_TIMEOUT_MS);
const silentHandler = new SilentHandler(authCodeModule, browserStorage, defaultTokenRequest, browserRequestLogger, { navigateFrameWait: DEFAULT_IFRAME_TIMEOUT_MS, pollIntervalMilliseconds: DEFAULT_POLL_INTERVAL_MS });
// @ts-ignore
silentHandler.monitorIframeForHash(iframe, 2000)
.catch(() => {
Expand Down Expand Up @@ -227,7 +228,7 @@ describe("SilentHandler.ts Unit Tests", () => {
}
};

const silentHandler = new SilentHandler(authCodeModule, browserStorage, defaultTokenRequest, browserRequestLogger, DEFAULT_IFRAME_TIMEOUT_MS);
const silentHandler = new SilentHandler(authCodeModule, browserStorage, defaultTokenRequest, browserRequestLogger, { navigateFrameWait: DEFAULT_IFRAME_TIMEOUT_MS, pollIntervalMilliseconds: DEFAULT_POLL_INTERVAL_MS });
// @ts-ignore
silentHandler.monitorIframeForHash(iframe, 1000)
.then((hash: string) => {
Expand Down