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

fix: saved idx transaction should be available after terminal response #1185

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

- [#1182](https://github.com/okta/okta-auth-js/pull/1182) Fixes security question verification to accept `credentials.answer`
- [#1184](https://github.com/okta/okta-auth-js/pull/1184) Fixes type declarations: `ApiError`, `responseType`, `responseMode`
- [#1185](https://github.com/okta/okta-auth-js/pull/1185) Fixes "cancel" and "skip" action called after receiving a terminal or error response

## 6.4.2

Expand Down
33 changes: 24 additions & 9 deletions lib/TransactionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ import {
TransactionMetaOptions,
TransactionManagerOptions,
CookieStorage,
SavedIdxResponse
SavedIdxResponse,
IntrospectOptions
} from './types';
import { isRawIdxResponse } from './idx/types/idx-js';
import { warn } from './util';
Expand All @@ -36,7 +37,8 @@ import {
} from './util/sharedStorage';

export interface ClearTransactionMetaOptions extends TransactionMetaOptions {
clearSharedStorage?: boolean;
clearSharedStorage?: boolean; // true by default
clearIdxResponse?: boolean; // true by default
}
export default class TransactionManager {
options: TransactionManagerOptions;
Expand Down Expand Up @@ -68,17 +70,18 @@ export default class TransactionManager {
// Clear primary storage (by default, sessionStorage on browser)
transactionStorage.clearStorage();

// clear IDX response storage
this.clearIdxResponse();

// Usually we want to also clear shared storage unless another tab may need it to continue/complete a flow
if (this.enableSharedStorage && options.clearSharedStorage !== false) {
const state = options.state || meta?.state;
if (state) {
clearTransactionFromSharedStorage(this.storageManager, state);
}
}


if (options.clearIdxResponse !== false) {
this.clearIdxResponse();
}

if (!this.legacyWidgetSupport) {
return;
}
Expand Down Expand Up @@ -307,18 +310,19 @@ export default class TransactionManager {
// throw new AuthSdkError('Unable to parse the ' + REDIRECT_OAUTH_PARAMS_NAME + ' value from storage');
}

saveIdxResponse({ rawIdxResponse, requestDidSucceed }: SavedIdxResponse): void {
saveIdxResponse(data: SavedIdxResponse): void {
if (!this.saveLastResponse) {
return;
}
const storage = this.storageManager.getIdxResponseStorage();
if (!storage) {
return;
}
storage.setStorage({ rawIdxResponse, requestDidSucceed });
storage.setStorage(data);
}

loadIdxResponse(): SavedIdxResponse | null {
// eslint-disable-next-line complexity
loadIdxResponse(options?: IntrospectOptions): SavedIdxResponse | null {
if (!this.saveLastResponse) {
return null;
}
Expand All @@ -330,6 +334,17 @@ export default class TransactionManager {
if (!storedValue || !isRawIdxResponse(storedValue.rawIdxResponse)) {
return null;
}

if (options) {
const { stateHandle, interactionHandle } = options;
if (stateHandle && storedValue.stateHandle !== stateHandle) {
return null;
}
if (interactionHandle && storedValue.interactionHandle !== interactionHandle) {
return null;
}
}

return storedValue;
}

Expand Down
10 changes: 4 additions & 6 deletions lib/idx/idxState/v1/generateIdxAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,9 @@
*/

/* eslint-disable max-len, complexity */
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-nocheck
import { httpRequest } from '../../../http';
import { OktaAuthInterface } from '../../../types'; // auth-js/types
import { IdxActionParams } from '../../types/idx-js';
import { IdxActionFunction, IdxActionParams, IdxResponse, IdxToPersist } from '../../types/idx-js';
import { divideActionParamsByMutability } from './actionParser';
import { makeIdxState } from './makeIdxState';
import AuthApiError from '../../../errors/AuthApiError';
Expand All @@ -24,8 +22,8 @@ const generateDirectFetch = function generateDirectFetch(authClient: OktaAuthInt
actionDefinition,
defaultParamsForAction = {},
immutableParamsForAction = {},
toPersist = {}
}) {
toPersist = {} as IdxToPersist
}): IdxActionFunction {
const target = actionDefinition.href;
return async function(params: IdxActionParams = {}): Promise<IdxResponse> {
const headers = {
Expand Down Expand Up @@ -90,7 +88,7 @@ const generateDirectFetch = function generateDirectFetch(authClient: OktaAuthInt
// };
// };

const generateIdxAction = function generateIdxAction( authClient: OktaAuthInterface, actionDefinition, toPersist ) {
const generateIdxAction = function generateIdxAction( authClient: OktaAuthInterface, actionDefinition, toPersist ): IdxActionFunction {
// TODO: leaving this here to see where the polling is EXPECTED to drop into the code, but removing any accidental trigger of incomplete code
// const generator = actionDefinition.refresh ? generatePollingFetch : generateDirectFetch;
const generator = generateDirectFetch;
Expand Down
2 changes: 1 addition & 1 deletion lib/idx/introspect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export async function introspect (
let requestDidSucceed;

// try load from storage first
const savedIdxResponse = authClient.transactionManager.loadIdxResponse();
const savedIdxResponse = authClient.transactionManager.loadIdxResponse(options);
if (savedIdxResponse) {
rawIdxResponse = savedIdxResponse.rawIdxResponse;
requestDidSucceed = savedIdxResponse.requestDidSucceed;
Expand Down
14 changes: 7 additions & 7 deletions lib/idx/remediate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,6 @@ export async function remediate(
return { idxResponse };
}

// Reach to terminal state
const terminal = isTerminalResponse(idxResponse);
const messages = getMessagesFromResponse(idxResponse);
if (terminal) {
return { idxResponse, terminal, messages };
}

const remediator = getRemediator(neededToProceed, values, options);

// Try actions in idxResponse first
Expand Down Expand Up @@ -131,6 +124,13 @@ export async function remediate(
}
}

// Do not attempt to remediate if response is in terminal state
const terminal = isTerminalResponse(idxResponse);
const messages = getMessagesFromResponse(idxResponse);
if (terminal) {
return { idxResponse, terminal, messages };
}

if (!remediator) {
if (options.step) {
values = filterValuesForRemediation(idxResponse, options.step, values); // include only requested values
Expand Down
30 changes: 25 additions & 5 deletions lib/idx/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ declare interface RunData {
idxResponse?: IdxResponse;
canceled?: boolean;
interactionCode?: string;
shouldSaveResponse?: boolean;
shouldClearTransaction?: boolean;
clearSharedStorage?: boolean;
terminal?: boolean;
Expand Down Expand Up @@ -208,6 +209,7 @@ async function finalizeData(authClient, data: RunData): Promise<RunData> {
status,
} = data;
const { exchangeCodeForTokens } = options;
let shouldSaveResponse = false;
let shouldClearTransaction = false;
let clearSharedStorage = true;
let interactionCode;
Expand All @@ -218,6 +220,7 @@ async function finalizeData(authClient, data: RunData): Promise<RunData> {
let terminal;

if (idxResponse) {
shouldSaveResponse = !!(idxResponse.requestDidSucceed || idxResponse.stepUp);
enabledFeatures = getEnabledFeatures(idxResponse);
availableSteps = getAvailableSteps(idxResponse);
messages = getMessagesFromResponse(idxResponse);
Expand All @@ -226,7 +229,21 @@ async function finalizeData(authClient, data: RunData): Promise<RunData> {

if (terminal) {
status = IdxStatus.TERMINAL;
shouldClearTransaction = true;

// In most cases a terminal response should not clear transaction data. The user should cancel or skip to continue.
// A terminal "success" is a non-error response with no further actions available.
// In these narrow cases, saved transaction data should be cleared.
// One example of a terminal success is when the email verify flow is continued in another tab
const hasActions = Object.keys(idxResponse!.actions).length > 0;
const hasErrors = !!messages.find(msg => msg.class === 'ERROR');
const isTerminalSuccess = !hasActions && !hasErrors && idxResponse!.requestDidSucceed === true;
if (isTerminalSuccess) {
shouldClearTransaction = true;
} else {
// only save response if there are actions available (ignore messages)
shouldSaveResponse = shouldSaveResponse && hasActions;
}
// leave shared storage intact so the transaction can be continued in another tab
clearSharedStorage = false;
} else if (canceled) {
status = IdxStatus.CANCELED;
Expand All @@ -247,6 +264,7 @@ async function finalizeData(authClient, data: RunData): Promise<RunData> {
status,
interactionCode,
tokens,
shouldSaveResponse,
shouldClearTransaction,
clearSharedStorage,
enabledFeatures,
Expand Down Expand Up @@ -293,6 +311,7 @@ export async function run(
const {
idxResponse,
meta,
shouldSaveResponse,
shouldClearTransaction,
clearSharedStorage,
status,
Expand All @@ -312,15 +331,16 @@ export async function run(
// ensures state is saved to sessionStorage
saveTransactionMeta(authClient, { ...meta });

if (idxResponse) {
if (shouldSaveResponse) {
// Save intermediate idx response in storage to reduce introspect call
const { rawIdxState: rawIdxResponse, requestDidSucceed } = idxResponse;
const { rawIdxState: rawIdxResponse, requestDidSucceed } = idxResponse!;
authClient.transactionManager.saveIdxResponse({
rawIdxResponse,
requestDidSucceed
requestDidSucceed,
stateHandle: idxResponse!.context?.stateHandle,
interactionHandle: meta?.interactionHandle
});
}

}

// from idx-js, used by the widget
Expand Down
16 changes: 12 additions & 4 deletions lib/idx/types/idx-js.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,18 +210,26 @@ export interface IdxActions {
[key: string]: (params?: IdxActionParams) => Promise<IdxResponse>;
}

// Object returned from auth-js
export interface IdxToPersist {
interactionHandle?: string;
withCredentials?: boolean;
}

export interface IdxActionFunction {
(params: IdxActionParams): Promise<IdxResponse>;
neededParams?: Array<Array<IdxRemediationValue>>;
}

export interface IdxResponse {
proceed: (remediationName: string, params: unknown) => Promise<IdxResponse>;
neededToProceed: IdxRemediation[];
rawIdxState: RawIdxResponse;
interactionCode?: string;
actions: IdxActions;
toPersist: {
interactionHandle?: string;
};
toPersist: IdxToPersist;
context?: IdxContext;
requestDidSucceed?: boolean;
stepUp?: boolean;
}

export function isIdxResponse(obj: any): obj is IdxResponse {
Expand Down
8 changes: 7 additions & 1 deletion lib/types/Storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import { TransactionMeta } from './Transaction';
import { Cookies, CookieOptions } from './Cookies';
import { RawIdxResponse } from '../idx/types/idx-js';
import { IntrospectOptions } from '.';

// for V1 authn interface: tx.resume()
export interface TxStorage {
Expand Down Expand Up @@ -51,7 +52,12 @@ export interface TransactionStorage extends StorageProvider {
getStorage(): TransactionMeta;
}

export interface SavedIdxResponse {
export interface SavedIdxResponse extends
Pick<IntrospectOptions,
'stateHandle' |
'interactionHandle'
>
{
rawIdxResponse: RawIdxResponse;
requestDidSucceed?: boolean;
}
Expand Down
Loading