Skip to content

Commit

Permalink
Fix bug in Mobile Safari 18.x: start poll request when document is vi…
Browse files Browse the repository at this point in the history
…sible and awaken (#1552)

OKTA-823470 

Fix bug in Mobile Safari 18.x: start poll request when document is visible and awaken
  • Loading branch information
denysoblohin-okta authored Jan 10, 2025
1 parent d0134cf commit 4413b3e
Show file tree
Hide file tree
Showing 19 changed files with 429 additions and 69 deletions.
9 changes: 5 additions & 4 deletions lib/authn/util/poll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import AuthSdkError from '../../errors/AuthSdkError';
import AuthPollStopError from '../../errors/AuthPollStopError';
import { AuthnTransactionState } from '../types';
import { getStateToken } from './stateToken';
import { isIOS } from '../../features';
import { isMobileSafari18 } from '../../features';

interface PollOptions {
delay?: number;
Expand Down Expand Up @@ -79,13 +79,14 @@ export function getPollFn(sdk, res: AuthnTransactionState, ref) {
var href = pollLink.href + toQueryString(opts);
return post(sdk, href, getStateToken(res), {
saveAuthnState: false,
withCredentials: true
withCredentials: true,
pollingIntent: true,
});
}

const delayNextPoll = (ms) => {
// no need for extra logic in non-iOS environments, just continue polling
if (!isIOS()) {
if (!isMobileSafari18()) {
return delayFn(ms);
}

Expand Down Expand Up @@ -135,7 +136,7 @@ export function getPollFn(sdk, res: AuthnTransactionState, ref) {
}

// don't trigger polling request if page is hidden wait until window is visible again
if (isIOS() && document.hidden) {
if (isMobileSafari18() && document.hidden) {
let handler;
return new Promise<void>((resolve) => {
handler = () => {
Expand Down
1 change: 1 addition & 0 deletions lib/base/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export interface FeaturesAPI {
isIE11OrLess(): boolean;
isDPoPSupported(): boolean;
isIOS(): boolean;
isMobileSafari18(): boolean;
}


Expand Down
2 changes: 2 additions & 0 deletions lib/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

export const STATE_TOKEN_KEY_NAME = 'oktaStateToken';
export const DEFAULT_POLLING_DELAY = 500;
export const IOS_PAGE_AWAKEN_TIMEOUT = 500;
export const IOS_MAX_RETRY_COUNT = 3;
export const DEFAULT_MAX_CLOCK_SKEW = 300;
export const DEFAULT_CACHE_DURATION = 86400;
export const TOKEN_STORAGE_NAME = 'okta-token-storage';
Expand Down
10 changes: 10 additions & 0 deletions lib/features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
/* eslint-disable node/no-unsupported-features/node-builtins */
/* global document, window, TextEncoder, navigator */

import { UAParser } from 'ua-parser-js';
import { webcrypto } from './crypto';

const isWindowsPhone = /windows phone|iemobile|wpdesktop/i;
Expand Down Expand Up @@ -95,3 +96,12 @@ export function isIOS () {
// @ts-expect-error - MSStream is not in `window` type, unsurprisingly
(/iPad|iPhone|iPod/.test(navigator.userAgent) && !window.MSStream);
}

export function isMobileSafari18 () {
if (isBrowser()) {
const { browser, os } = new UAParser().getResult();
return os.name?.toLowerCase() === 'ios' && !!browser.name?.toLowerCase()?.includes('safari')
&& browser.major === '18';
}
return false;
}
97 changes: 93 additions & 4 deletions lib/http/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@

/* eslint-disable complexity */
import { isString, clone, isAbsoluteUrl, removeNils } from '../util';
import { STATE_TOKEN_KEY_NAME, DEFAULT_CACHE_DURATION } from '../constants';
import {
STATE_TOKEN_KEY_NAME,
DEFAULT_CACHE_DURATION,
IOS_MAX_RETRY_COUNT,
IOS_PAGE_AWAKEN_TIMEOUT
} from '../constants';
import {
OktaAuthHttpInterface,
RequestOptions,
Expand All @@ -23,7 +28,21 @@ import {
HttpResponse
} from './types';
import { AuthApiError, OAuthError, APIError, WWWAuthError } from '../errors';
import { isMobileSafari18 } from '../features';


// For iOS track last date when document became visible
let dateDocumentBecameVisible = 0;
let trackDateDocumentBecameVisible: () => void;
if (isMobileSafari18()) {
dateDocumentBecameVisible = Date.now();
trackDateDocumentBecameVisible = () => {
if (!document.hidden) {
dateDocumentBecameVisible = Date.now();
}
};
document.addEventListener('visibilitychange', trackDateDocumentBecameVisible);
}

const formatError = (sdk: OktaAuthHttpInterface, error: HttpResponse | Error): AuthApiError | OAuthError => {
if (error instanceof Error) {
Expand Down Expand Up @@ -96,6 +115,7 @@ const formatError = (sdk: OktaAuthHttpInterface, error: HttpResponse | Error): A
return err;
};

// eslint-disable-next-line max-statements
export function httpRequest(sdk: OktaAuthHttpInterface, options: RequestOptions): Promise<any> {
options = options || {};

Expand All @@ -113,7 +133,8 @@ export function httpRequest(sdk: OktaAuthHttpInterface, options: RequestOptions)
withCredentials = options.withCredentials === true, // default value is false
storageUtil = sdk.options.storageUtil,
storage = storageUtil!.storage,
httpCache = sdk.storageManager.getHttpCache(sdk.options.cookies);
httpCache = sdk.storageManager.getHttpCache(sdk.options.cookies),
pollingIntent = options.pollingIntent;

if (options.cacheResponse) {
var cacheContents = httpCache.getStorage();
Expand Down Expand Up @@ -142,8 +163,76 @@ export function httpRequest(sdk: OktaAuthHttpInterface, options: RequestOptions)
withCredentials
};

var err, res;
return sdk.options.httpRequestClient!(method!, url!, ajaxOptions)
var err, res, promise;

if (pollingIntent && isMobileSafari18()) {
let waitForVisibleAndAwakenDocument: () => Promise<void>;
let waitForAwakenDocument: () => Promise<void>;
let recursiveFetch: () => Promise<HttpResponse>;
let retryCount = 0;

// Safari on iOS has a bug:
// Performing `fetch` right after document became visible can fail with `Load failed` error.
// Running fetch after short timeout fixes this issue.
waitForAwakenDocument = () => {
const timeSinceDocumentIsVisible = Date.now() - dateDocumentBecameVisible;
if (timeSinceDocumentIsVisible < IOS_PAGE_AWAKEN_TIMEOUT) {
return new Promise<void>((resolve) => setTimeout(() => {
if (!document.hidden) {
resolve();
} else {
resolve(waitForVisibleAndAwakenDocument());
}
}, IOS_PAGE_AWAKEN_TIMEOUT - timeSinceDocumentIsVisible));
} else {
return Promise.resolve();
}
};

// Returns a promise that resolves when document is visible for 500 ms
waitForVisibleAndAwakenDocument = () => {
if (document.hidden) {
let pageVisibilityHandler: () => void;
return new Promise<void>((resolve) => {
pageVisibilityHandler = () => {
if (!document.hidden) {
document.removeEventListener('visibilitychange', pageVisibilityHandler);
resolve(waitForAwakenDocument());
}
};
document.addEventListener('visibilitychange', pageVisibilityHandler);
});
} else {
return waitForAwakenDocument();
}
};

// Restarts fetch on 'Load failed' error
// This error can occur when `fetch` does not respond
// (due to CORS error, non-existing host, or network error)
const retryableFetch = (): Promise<HttpResponse> => {
return sdk.options.httpRequestClient!(method!, url!, ajaxOptions).catch((err) => {
const isNetworkError = err?.message === 'Load failed';
if (isNetworkError && retryCount < IOS_MAX_RETRY_COUNT) {
retryCount++;
return recursiveFetch();
}
throw err;
});
};

// Final promise to fetch that wraps logic with waiting for visible document
// and retrying fetch request on network error
recursiveFetch = (): Promise<HttpResponse> => {
return waitForVisibleAndAwakenDocument().then(retryableFetch);
};

promise = recursiveFetch();
} else {
promise = sdk.options.httpRequestClient!(method!, url!, ajaxOptions);
}

return promise
.then(function(resp) {
res = resp.responseText;
if (res && isString(res)) {
Expand Down
1 change: 1 addition & 0 deletions lib/http/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export interface RequestOptions {
storageUtil?: StorageUtil;
cacheResponse?: boolean;
headers?: RequestHeaders;
pollingIntent?: boolean;
}

export interface FetchOptions {
Expand Down
11 changes: 8 additions & 3 deletions lib/idx/idxState/v1/generateIdxAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
*/

/* eslint-disable max-len, complexity */
import { httpRequest } from '../../../http';
import { httpRequest, RequestOptions } from '../../../http';
import { OktaAuthIdxInterface } from '../../types'; // auth-js/types
import { IdxActionFunction, IdxActionParams, IdxResponse, IdxToPersist } from '../../types/idx-js';
import { divideActionParamsByMutability } from './actionParser';
Expand All @@ -36,13 +36,18 @@ const generateDirectFetch = function generateDirectFetch(authClient: OktaAuthIdx
});

try {
const response = await httpRequest(authClient, {
const options: RequestOptions = {
url: target,
method: actionDefinition.method,
headers,
args: body,
withCredentials: toPersist?.withCredentials ?? true
});
};
const isPolling = actionDefinition.name === 'poll' || actionDefinition.name?.endsWith('-poll');
if (isPolling) {
options.pollingIntent = true;
}
const response = await httpRequest(authClient, options);

return authClient.idx.makeIdxResponse({ ...response }, toPersist, true);
}
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@
"node-cache": "^5.1.2",
"p-cancelable": "^2.0.0",
"tiny-emitter": "1.1.0",
"ua-parser-js": "^2.0.0",
"webcrypto-shim": "^0.1.5",
"xhr2": "0.1.3"
},
Expand Down Expand Up @@ -211,7 +212,7 @@
"rollup-plugin-cleanup": "^3.2.1",
"rollup-plugin-license": "^2.8.1",
"rollup-plugin-multi-input": "^1.3.1",
"rollup-plugin-typescript2": "^0.30.0",
"rollup-plugin-typescript2": "^0.36.0",
"rollup-plugin-visualizer": "~5.5.4",
"shelljs": "0.8.5",
"ts-jest": "^28.0.2",
Expand Down
3 changes: 2 additions & 1 deletion test/spec/TokenManager/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ const mocked = {
isBrowser: () => typeof window !== 'undefined',
isIE11OrLess: () => false,
isLocalhost: () => false,
isTokenVerifySupported: () => true
isTokenVerifySupported: () => true,
isMobileSafari18: () => false
}
};
jest.mock('../../../lib/features', () => {
Expand Down
3 changes: 2 additions & 1 deletion test/spec/TokenManager/expireEvents.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
jest.mock('../../../lib/features', () => {
return {
isLocalhost: () => true, // to allow configuring expireEarlySeconds
isIE11OrLess: () => false
isIE11OrLess: () => false,
isMobileSafari18: () => false
};
});

Expand Down
20 changes: 10 additions & 10 deletions test/spec/authn/mfa-challenge.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ jest.mock('lib/features', () => {
const actual = jest.requireActual('../../../lib/features');
return {
...actual,
isIOS: () => false
isMobileSafari18: () => false
};
});
import OktaAuth from '@okta/okta-auth-js';
Expand Down Expand Up @@ -1581,7 +1581,7 @@ describe('MFA_CHALLENGE', function () {
});

// mocks iOS environment
jest.spyOn(mocked.features, 'isIOS').mockReturnValue(true);
jest.spyOn(mocked.features, 'isMobileSafari18').mockReturnValue(true);

const { response: mfaPush } = await util.generateXHRPair({
uri: 'https://auth-js-test.okta.com'
Expand All @@ -1592,17 +1592,17 @@ describe('MFA_CHALLENGE', function () {
}, 'success', 'https://auth-js-test.okta.com');

// mocks flow of wait, wait, wait, success
context.httpSpy = jest.spyOn(mocked.http, 'post')
.mockResolvedValueOnce(mfaPush.response)
.mockResolvedValueOnce(mfaPush.response)
.mockResolvedValueOnce(mfaPush.response)
.mockResolvedValueOnce(success.response);

context.httpSpy = jest.fn()
.mockResolvedValueOnce({responseText: JSON.stringify(mfaPush.response)})
.mockResolvedValueOnce({responseText: JSON.stringify(mfaPush.response)})
.mockResolvedValueOnce({responseText: JSON.stringify(mfaPush.response)})
.mockResolvedValueOnce({responseText: JSON.stringify(success.response)});

const oktaAuth = new OktaAuth({
issuer: 'https://auth-js-test.okta.com'
issuer: 'https://auth-js-test.okta.com',
httpRequestClient: context.httpSpy
});

context.transaction = oktaAuth.tx.createTransaction(mfaPush.response);
});

Expand Down
41 changes: 30 additions & 11 deletions test/spec/features/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,29 +64,48 @@ describe('features (browser)', function() {
});
});

describe('isIOS', () => {
it('can succeed', () => {
const iOSAgents = [
'Mozilla/5.0 (iPhone; CPU iPhone OS 14_7 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) CriOS/92.0.4515.90 Mobile/15E148 Safari/604.1',
'Mozilla/5.0 (iPhone; CPU iPhone OS 14_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/14.0.1 Mobile/15E148 Safari/604.1',
'Mozilla/5.0 (iPhone; CPU iPhone OS 14_7 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) CriOS/92.0.4515.90 Mobile/15E148 Safari/604.1',
'Mozilla/5.0 (iPhone; CPU iPhone OS 14_7_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/14.1.2 Mobile/15E148 Safari/604.1'
];
describe('isIOS, isMobileSafari18', () => {
const iOSAgents = [
'Mozilla/5.0 (iPhone; CPU iPhone OS 14_7 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) CriOS/92.0.4515.90 Mobile/15E148 Safari/604.1',
'Mozilla/5.0 (iPhone; CPU iPhone OS 14_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/14.0.1 Mobile/15E148 Safari/604.1',
'Mozilla/5.0 (iPhone; CPU iPhone OS 14_7 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) CriOS/92.0.4515.90 Mobile/15E148 Safari/604.1',
'Mozilla/5.0 (iPhone; CPU iPhone OS 14_7_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/14.1.2 Mobile/15E148 Safari/604.1',
'Mozilla/5.0 (iPhone; CPU iPhone OS 18_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) EdgiOS/130.0.2849.80 Version/18.0 Mobile/15E148 Safari/604.1',
'Mozilla/5.0 (iPhone; CPU iPhone OS 18_2_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) EdgiOS/132.0.2957.32 Version/18.0 Mobile/15E148 Safari/604.1',
];
const mobileSafari18Agents = [
'Mozilla/5.0 (iPhone; CPU iPhone OS 18_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/18.0 Mobile/15E148 Safari/604.1',
'Mozilla/5.0 (iPhone; CPU iPhone OS 18_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Teak/5.9 Version/18 Mobile/15E148 Safari/604.1',
'Mozilla/5.0 (iPhone; CPU iPhone OS 18_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/18.2 Mobile/15E148 Safari/604.1',
'Mozilla/5.0 (iPad; CPU OS 18_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/18.2 Mobile/15E148 Safari/604.1',
];

for (let userAgent of iOSAgents) {
for (let userAgent of iOSAgents) {
it('can succeed for ' + userAgent, () => {
jest.spyOn(global.navigator, 'userAgent', 'get').mockReturnValue(userAgent);
expect(OktaAuth.features.isIOS()).toBe(true);
}
});
expect(OktaAuth.features.isMobileSafari18()).toBe(false);
});
}
for (let userAgent of mobileSafari18Agents) {
// eslint-disable-next-line jasmine/no-spec-dupes
it('can succeed for ' + userAgent, () => {
jest.spyOn(global.navigator, 'userAgent', 'get').mockReturnValue(userAgent);
expect(OktaAuth.features.isIOS()).toBe(true);
expect(OktaAuth.features.isMobileSafari18()).toBe(true);
});
}

it('returns false if navigator is unavailable', () => {
jest.spyOn(global, 'navigator', 'get').mockReturnValue(undefined as never);
expect(OktaAuth.features.isIOS()).toBe(false);
expect(OktaAuth.features.isMobileSafari18()).toBe(false);
});

it('returns false if userAgent is unavailable', () => {
jest.spyOn(global.navigator, 'userAgent', 'get').mockReturnValue(undefined as never);
expect(OktaAuth.features.isIOS()).toBe(false);
expect(OktaAuth.features.isMobileSafari18()).toBe(false);
});
});
});
Loading

0 comments on commit 4413b3e

Please sign in to comment.