From ca5304c6f938bb9cd6d1c26042a175d473b5d9ab Mon Sep 17 00:00:00 2001 From: "denys.oblohin" Date: Thu, 29 Jun 2023 20:58:07 +0300 Subject: [PATCH 01/10] fix: Don't auto remediate SelectAuthenticator with current authenticator --- lib/idx/remediate.ts | 6 +-- lib/idx/remediators/Base/Remediator.ts | 2 +- .../remediators/Base/SelectAuthenticator.ts | 13 ++++--- lib/idx/util.ts | 7 ++-- .../idx/remediators/SelectAuthenticator.ts | 38 +++++++++++++++++++ 5 files changed, 54 insertions(+), 12 deletions(-) create mode 100644 test/spec/idx/remediators/SelectAuthenticator.ts diff --git a/lib/idx/remediate.ts b/lib/idx/remediate.ts index b48f628e7..e2da0efc5 100644 --- a/lib/idx/remediate.ts +++ b/lib/idx/remediate.ts @@ -67,7 +67,7 @@ export async function remediate( values: RemediationValues, options: RemediateOptions ): Promise { - let { neededToProceed, interactionCode } = idxResponse; + let { neededToProceed, interactionCode, context } = idxResponse; const { flow } = options; // If the response contains an interaction code, there is no need to remediate @@ -75,7 +75,7 @@ export async function remediate( return { idxResponse }; } - const remediator = getRemediator(neededToProceed, values, options); + const remediator = getRemediator(neededToProceed, values, options, context); // Try actions in idxResponse first const actionFromValues = getActionFromValues(values, idxResponse); @@ -175,7 +175,7 @@ export async function remediate( // return nextStep directly if (options.useGenericRemediator && !idxResponse.interactionCode && !isTerminalResponse(idxResponse)) { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const gr = getRemediator(idxResponse.neededToProceed, values, options)!; + const gr = getRemediator(idxResponse.neededToProceed, values, options, idxResponse.context)!; const nextStep = getNextStep(authClient, gr, idxResponse); return { idxResponse, diff --git a/lib/idx/remediators/Base/Remediator.ts b/lib/idx/remediators/Base/Remediator.ts index ca872fc94..1c495d9b8 100644 --- a/lib/idx/remediators/Base/Remediator.ts +++ b/lib/idx/remediators/Base/Remediator.ts @@ -94,7 +94,7 @@ export class Remediator { // Override this method to provide custom check /* eslint-disable-next-line no-unused-vars, @typescript-eslint/no-unused-vars */ - canRemediate(): boolean { + canRemediate(context?: IdxContext): boolean { const required = getRequiredValues(this.remediation); const needed = required!.find((key) => !this.hasData(key)); if (needed) { diff --git a/lib/idx/remediators/Base/SelectAuthenticator.ts b/lib/idx/remediators/Base/SelectAuthenticator.ts index 88a582a45..96c979731 100644 --- a/lib/idx/remediators/Base/SelectAuthenticator.ts +++ b/lib/idx/remediators/Base/SelectAuthenticator.ts @@ -14,7 +14,7 @@ import { Remediator, RemediationValues } from './Remediator'; import { getAuthenticatorFromRemediation } from '../util'; -import { IdxRemediationValue } from '../../types/idx-js'; +import { IdxRemediationValue, IdxContext, IdxOption } from '../../types/idx-js'; import { Authenticator, isAuthenticator } from '../../types/api'; import { compareAuthenticators, findMatchedOption} from '../../authenticator/util'; @@ -30,7 +30,7 @@ export class SelectAuthenticator relatesTo.key === authenticator.key); @@ -41,7 +41,7 @@ export class SelectAuthenticator { + describe('canRemediate', () => { + it('retuns false if matched authenticator is already the current one', () => { + const currentAuthenticator = PhoneAuthenticatorFactory.build(); + const remediation = SelectAuthenticatorEnrollRemediationFactory.build({ + value: [ + AuthenticatorValueFactory.build({ + options: [ + PhoneAuthenticatorOptionFactory.params({ + _authenticator: currentAuthenticator, + }).build(), + ] + }), + ] + }); + const context = IdxContextFactory.build({ + currentAuthenticator: { + value: currentAuthenticator + } + }); + const authenticators = [ + currentAuthenticator, + ]; + const r = new SelectAuthenticatorAuthenticate(remediation, { authenticators }); + expect(r.canRemediate(context)).toBe(false); + expect(r.canRemediate()).toBe(true); + }); + }); +}); From 4684aa2209282b814ab61d3f0c3d60ed9e1536df Mon Sep 17 00:00:00 2001 From: "denys.oblohin" Date: Thu, 29 Jun 2023 21:02:18 +0300 Subject: [PATCH 02/10] chlog --- CHANGELOG.md | 6 ++++++ package.json | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cea5cd0ad..b7290b303 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## 7.3.1 + +### Fixes + +- [#1426](https://github.com/okta/okta-auth-js/pull/1426) fix: Don't auto remediate SelectAuthenticator with current authenticator + ## 7.3.0 ### Features diff --git a/package.json b/package.json index aee128c00..a12e62dba 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "private": true, "name": "@okta/okta-auth-js", "description": "The Okta Auth SDK", - "version": "7.3.0", + "version": "7.3.1", "homepage": "https://github.com/okta/okta-auth-js", "license": "Apache-2.0", "main": "build/cjs/exports/default.js", From c81d115c7f9377d7110a84b33f721d4e960856b8 Mon Sep 17 00:00:00 2001 From: "denys.oblohin" Date: Fri, 30 Jun 2023 11:09:47 +0300 Subject: [PATCH 03/10] 1st param of getRemediator - idxResponse --- lib/idx/remediate.ts | 4 +- lib/idx/util.ts | 6 +-- test/spec/idx/remediate.ts | 26 +++++------ test/spec/idx/util.ts | 89 ++++++++++++++++++++++---------------- 4 files changed, 69 insertions(+), 56 deletions(-) diff --git a/lib/idx/remediate.ts b/lib/idx/remediate.ts index e2da0efc5..30c29ae2d 100644 --- a/lib/idx/remediate.ts +++ b/lib/idx/remediate.ts @@ -75,7 +75,7 @@ export async function remediate( return { idxResponse }; } - const remediator = getRemediator(neededToProceed, values, options, context); + const remediator = getRemediator(idxResponse, values, options); // Try actions in idxResponse first const actionFromValues = getActionFromValues(values, idxResponse); @@ -175,7 +175,7 @@ export async function remediate( // return nextStep directly if (options.useGenericRemediator && !idxResponse.interactionCode && !isTerminalResponse(idxResponse)) { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const gr = getRemediator(idxResponse.neededToProceed, values, options, idxResponse.context)!; + const gr = getRemediator(idxResponse, values, options)!; const nextStep = getNextStep(authClient, gr, idxResponse); return { idxResponse, diff --git a/lib/idx/util.ts b/lib/idx/util.ts index 2a2e4f5f9..c725c97f1 100644 --- a/lib/idx/util.ts +++ b/lib/idx/util.ts @@ -210,14 +210,14 @@ function getRemediatorClass(remediation: IdxRemediation, options: RemediateOptio // Return first match idxRemediation in allowed remediators // eslint-disable-next-line complexity export function getRemediator( - idxRemediations: IdxRemediation[], + idxResponse: IdxResponse, values: RemediationValues, options: RemediateOptions, - context?: IdxContext, ): Remediator | undefined { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const remediators = options.remediators!; const useGenericRemediator = options.useGenericRemediator; + const {neededToProceed: idxRemediations, context} = idxResponse; let remediator: Remediator; // remediation name specified by caller - fast-track remediator lookup @@ -285,7 +285,7 @@ export function handleFailedResponse( if (terminal) { return { idxResponse, terminal, messages }; } else { - const remediator = getRemediator(idxResponse.neededToProceed, {}, options, idxResponse.context); + const remediator = getRemediator(idxResponse, {}, options); const nextStep = remediator && getNextStep(authClient, remediator, idxResponse); return { idxResponse, diff --git a/test/spec/idx/remediate.ts b/test/spec/idx/remediate.ts index 8cc035480..e18eee367 100644 --- a/test/spec/idx/remediate.ts +++ b/test/spec/idx/remediate.ts @@ -88,8 +88,8 @@ describe('idx/remediate', () => { nextStep: {} }); expect(util.getRemediator).toHaveBeenCalledTimes(2); - expect(util.getRemediator).toHaveBeenNthCalledWith(1, idxResponse.neededToProceed, { resend: true }, {}); - expect(util.getRemediator).toHaveBeenNthCalledWith(2, responseFromAction.neededToProceed, {}, { actions: [] }); + expect(util.getRemediator).toHaveBeenNthCalledWith(1, idxResponse, { resend: true }, {}); + expect(util.getRemediator).toHaveBeenNthCalledWith(2, responseFromAction, {}, { actions: [] }); }); it('will handle exceptions', async () => { @@ -145,8 +145,8 @@ describe('idx/remediate', () => { nextStep: {} }); expect(util.getRemediator).toHaveBeenCalledTimes(2); - expect(util.getRemediator).toHaveBeenNthCalledWith(1, idxResponse.neededToProceed, {}, { actions: ['some-action'] }); - expect(util.getRemediator).toHaveBeenNthCalledWith(2, responseFromAction.neededToProceed, {}, { actions: [] }); + expect(util.getRemediator).toHaveBeenNthCalledWith(1, idxResponse, {}, { actions: ['some-action'] }); + expect(util.getRemediator).toHaveBeenNthCalledWith(2, responseFromAction, {}, { actions: [] }); }); it('will handle exceptions', async () => { const { authClient } = testContext; @@ -195,8 +195,8 @@ describe('idx/remediate', () => { nextStep: {} }); expect(util.getRemediator).toHaveBeenCalledTimes(2); - expect(util.getRemediator).toHaveBeenNthCalledWith(1, idxResponse.neededToProceed, {}, { actions: ['some-remediation'] }); - expect(util.getRemediator).toHaveBeenNthCalledWith(2, responseFromRemediation.neededToProceed, {}, { actions: [] }); + expect(util.getRemediator).toHaveBeenNthCalledWith(1, idxResponse, {}, { actions: ['some-remediation'] }); + expect(util.getRemediator).toHaveBeenNthCalledWith(2, responseFromRemediation, {}, { actions: [] }); }); it('will handle exceptions', async () => { let { authClient, idxResponse } = testContext; @@ -280,8 +280,8 @@ describe('idx/remediate', () => { nextStep: {} }); expect(util.getRemediator).toHaveBeenCalledTimes(2); - expect(util.getRemediator).toHaveBeenNthCalledWith(1, idxResponse.neededToProceed, {}, { actions: [action] }); - expect(util.getRemediator).toHaveBeenNthCalledWith(2, responseFromAction.neededToProceed, {}, { actions: [] }); + expect(util.getRemediator).toHaveBeenNthCalledWith(1, idxResponse, {}, { actions: [action] }); + expect(util.getRemediator).toHaveBeenNthCalledWith(2, responseFromAction, {}, { actions: [] }); }); it('will handle exceptions', async () => { let { authClient, idxResponse } = testContext; @@ -339,8 +339,8 @@ describe('idx/remediate', () => { nextStep: {} }); expect(util.getRemediator).toHaveBeenCalledTimes(2); - expect(util.getRemediator).toHaveBeenNthCalledWith(1, idxResponse.neededToProceed, {}, { actions: [action] }); - expect(util.getRemediator).toHaveBeenNthCalledWith(2, responseFromRemediation.neededToProceed, {}, { actions: [] }); + expect(util.getRemediator).toHaveBeenNthCalledWith(1, idxResponse, {}, { actions: [action] }); + expect(util.getRemediator).toHaveBeenNthCalledWith(2, responseFromRemediation, {}, { actions: [] }); }); it('will handle exceptions', async () => { let { authClient, idxResponse } = testContext; @@ -432,7 +432,7 @@ describe('idx/remediate', () => { }, }); expect(util.getRemediator).toHaveBeenCalledTimes(1); - expect(util.getRemediator).toHaveBeenNthCalledWith(1, idxResponse.neededToProceed, {}, { step: 'some-remediation' }); + expect(util.getRemediator).toHaveBeenNthCalledWith(1, idxResponse, {}, { step: 'some-remediation' }); expect(idxResponse.proceed).toHaveBeenCalledWith('some-remediation', {}); }); it('will handle exceptions', async () => { @@ -530,8 +530,8 @@ describe('idx/remediate', () => { }); expect(idxResponse.proceed).toHaveBeenCalledWith(name, data); expect(util.getRemediator).toHaveBeenCalledTimes(2); - expect(util.getRemediator).toHaveBeenNthCalledWith(1, idxResponse.neededToProceed, {}, {}); - expect(util.getRemediator).toHaveBeenNthCalledWith(2, responseFromProceed.neededToProceed, valuesAfterProceed, {}); + expect(util.getRemediator).toHaveBeenNthCalledWith(1, idxResponse, {}, {}); + expect(util.getRemediator).toHaveBeenNthCalledWith(2, responseFromProceed, valuesAfterProceed, {}); }); diff --git a/test/spec/idx/util.ts b/test/spec/idx/util.ts index 2af716ec2..2b4fd0305 100644 --- a/test/spec/idx/util.ts +++ b/test/spec/idx/util.ts @@ -218,75 +218,83 @@ describe('idx/util', () => { describe('A Remediator exists that matches one of the idx remediations', () => { beforeEach(() => { - const idxRemediations = [{ + const neededToProceed = [{ name: 'foo' }, { name: 'bar' }]; + const idxResponse = { + neededToProceed + } as IdxResponse; testContext = { ...testContext, - idxRemediations + neededToProceed, + idxResponse }; }); it('if first Remediator can remediate, returns the first Remediator instance', () => { - const { idxRemediations, values, options, FooRemediator } = testContext; + const { idxResponse, neededToProceed, values, options, FooRemediator } = testContext; FooRemediator.prototype.canRemediate = jest.fn().mockReturnValue(true); - expect(getRemediator(idxRemediations, values, options)).toBeInstanceOf(FooRemediator); - expect(FooRemediator).toHaveBeenCalledWith(idxRemediations[0], values, options); + expect(getRemediator(idxResponse, values, options)).toBeInstanceOf(FooRemediator); + expect(FooRemediator).toHaveBeenCalledWith(neededToProceed[0], values, options); }); it('if first matched Remediator cannot remediate, but 2nd Remediator can, returns the 2nd Remediator', () => { - const { idxRemediations, values, options, FooRemediator, BarRemediator } = testContext; + const { idxResponse, neededToProceed, values, options, FooRemediator, BarRemediator } = testContext; FooRemediator.prototype.canRemediate = jest.fn().mockReturnValue(false); BarRemediator.prototype.canRemediate = jest.fn().mockReturnValue(true); - expect(getRemediator(idxRemediations, values, options)).toBeInstanceOf(BarRemediator); - expect(BarRemediator).toHaveBeenCalledWith(idxRemediations[1], values, options); + expect(getRemediator(idxResponse, values, options)).toBeInstanceOf(BarRemediator); + expect(BarRemediator).toHaveBeenCalledWith(neededToProceed[1], values, options); }); it('if no Remediator can remediate, returns the first matching Remediator instance', () => { - const { idxRemediations, values, options, FooRemediator, BarRemediator } = testContext; + const { idxResponse, neededToProceed, values, options, FooRemediator, BarRemediator } = testContext; FooRemediator.prototype.canRemediate = jest.fn().mockReturnValue(false); BarRemediator.prototype.canRemediate = jest.fn().mockReturnValue(false); - expect(getRemediator(idxRemediations, values, options)).toBeInstanceOf(FooRemediator); - expect(FooRemediator).toHaveBeenCalledWith(idxRemediations[0], values, options); + expect(getRemediator(idxResponse, values, options)).toBeInstanceOf(FooRemediator); + expect(FooRemediator).toHaveBeenCalledWith(neededToProceed[0], values, options); }); describe('with options.step', () => { it('returns a remediator instance if a Remediator exists that matches the idx remediation with the given step name', () => { - const { idxRemediations, values, options, BarRemediator } = testContext; + const { idxResponse, neededToProceed, values, options, BarRemediator } = testContext; options.step = 'bar'; - expect(getRemediator(idxRemediations, values, options)).toBeInstanceOf(BarRemediator); - expect(BarRemediator).toHaveBeenCalledWith(idxRemediations[1], values, options); + expect(getRemediator(idxResponse, values, options)).toBeInstanceOf(BarRemediator); + expect(BarRemediator).toHaveBeenCalledWith(neededToProceed[1], values, options); }); it('returns undefined if no Remediator could be found matching the idx remediation with the given step name', () => { - const { idxRemediations, values, options } = testContext; + const { idxResponse, values, options } = testContext; options.step = 'bar'; options.remediators = { 'other': jest.fn() }; - expect(getRemediator(idxRemediations, values, options)).toBe(undefined); + expect(getRemediator(idxResponse, values, options)).toBe(undefined); }); it('returns undefined if no idx remediation is found matching the given step name', () => { - const { values, options } = testContext; + const { idxResponse, values, options } = testContext; options.step = 'bar'; - const idxRemediations = [{ + const neededToProceed = [{ name: 'other' }]; - expect(getRemediator(idxRemediations, values, options)).toBe(undefined); + idxResponse.neededToProceed = neededToProceed; + expect(getRemediator(idxResponse, values, options)).toBe(undefined); }); }); }); it('returns undefined if no Remediator exists that matches the idx remediations', () => { const { values, options } = testContext; - const idxRemediations = [{ + const neededToProceed = [{ name: 'unknown' }]; + const idxResponse = { + neededToProceed + } as IdxResponse; options.remediators = { other: jest.fn() }; - expect(getRemediator(idxRemediations, values, options)).toBe(undefined); + expect(getRemediator(idxResponse, values, options)).toBe(undefined); }); describe('with options.useGenericRemediator', () => { @@ -295,56 +303,61 @@ describe('idx/util', () => { ...testContext.options, useGenericRemediator: true }; - const idxRemediations = [{ + const neededToProceed = [{ name: 'foo' }, { name: 'bar' }]; + const idxResponse = { + neededToProceed + } as IdxResponse; testContext = { ...testContext, - idxRemediations, + neededToProceed, + idxResponse, options }; }); describe('with options.step', () => { it('returns GenericRemediator instance if one idx remediation can match the given step name', () => { - const { idxRemediations, values, options } = testContext; + const { idxResponse, neededToProceed, values, options } = testContext; options.step = 'bar'; - expect(getRemediator(idxRemediations, values, options)).toBeInstanceOf(GenericRemediator); - expect(GenericRemediator).toHaveBeenCalledWith(idxRemediations[1], values, options); + expect(getRemediator(idxResponse, values, options)).toBeInstanceOf(GenericRemediator); + expect(GenericRemediator).toHaveBeenCalledWith(neededToProceed[1], values, options); }); it('returns undefined if no idx remediation is found matching the given step name', () => { - const { values, options } = testContext; + const { idxResponse, values, options } = testContext; options.step = 'bar'; - const idxRemediations = [{ + const neededToProceed = [{ name: 'other' }]; - expect(getRemediator(idxRemediations, values, options)).toBe(undefined); + idxResponse.neededToProceed = neededToProceed; + expect(getRemediator(idxResponse, values, options)).toBe(undefined); }); }); describe('without options.step', () => { it('if first Remediator can remediate, returns the first Remediator instance', () => { - const { idxRemediations, values, options, FooRemediator } = testContext; + const { idxResponse, neededToProceed, values, options, FooRemediator } = testContext; FooRemediator.prototype.canRemediate = jest.fn().mockReturnValue(true); - expect(getRemediator(idxRemediations, values, options)).toBeInstanceOf(GenericRemediator); - expect(GenericRemediator).toHaveBeenCalledWith(idxRemediations[0], values, options); + expect(getRemediator(idxResponse, values, options)).toBeInstanceOf(GenericRemediator); + expect(GenericRemediator).toHaveBeenCalledWith(neededToProceed[0], values, options); }); it('if first matched Remediator cannot remediate, but 2nd Remediator can, returns the first Remediator instance', () => { - const { idxRemediations, values, options, FooRemediator, BarRemediator } = testContext; + const { idxResponse, neededToProceed, values, options, FooRemediator, BarRemediator } = testContext; FooRemediator.prototype.canRemediate = jest.fn().mockReturnValue(false); BarRemediator.prototype.canRemediate = jest.fn().mockReturnValue(true); - expect(getRemediator(idxRemediations, values, options)).toBeInstanceOf(GenericRemediator); - expect(GenericRemediator).toHaveBeenCalledWith(idxRemediations[0], values, options); + expect(getRemediator(idxResponse, values, options)).toBeInstanceOf(GenericRemediator); + expect(GenericRemediator).toHaveBeenCalledWith(neededToProceed[0], values, options); }); it('if no Remediator can remediate, returns the first Remediator instance', () => { - const { idxRemediations, values, options, FooRemediator, BarRemediator } = testContext; + const { idxResponse, neededToProceed, values, options, FooRemediator, BarRemediator } = testContext; FooRemediator.prototype.canRemediate = jest.fn().mockReturnValue(false); BarRemediator.prototype.canRemediate = jest.fn().mockReturnValue(false); - expect(getRemediator(idxRemediations, values, options)).toBeInstanceOf(GenericRemediator); - expect(GenericRemediator).toHaveBeenCalledWith(idxRemediations[0], values, options); + expect(getRemediator(idxResponse, values, options)).toBeInstanceOf(GenericRemediator); + expect(GenericRemediator).toHaveBeenCalledWith(neededToProceed[0], values, options); }); }); From e93999fcdcc9dbcdbfd4df0b052c4f22313a70fb Mon Sep 17 00:00:00 2001 From: "denys.oblohin" Date: Fri, 30 Jun 2023 11:47:33 +0300 Subject: [PATCH 04/10] lint fix --- lib/idx/remediate.ts | 2 +- lib/idx/util.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/idx/remediate.ts b/lib/idx/remediate.ts index 30c29ae2d..6cf698621 100644 --- a/lib/idx/remediate.ts +++ b/lib/idx/remediate.ts @@ -67,7 +67,7 @@ export async function remediate( values: RemediationValues, options: RemediateOptions ): Promise { - let { neededToProceed, interactionCode, context } = idxResponse; + let { neededToProceed, interactionCode } = idxResponse; const { flow } = options; // If the response contains an interaction code, there is no need to remediate diff --git a/lib/idx/util.ts b/lib/idx/util.ts index c725c97f1..7d3f1a34a 100644 --- a/lib/idx/util.ts +++ b/lib/idx/util.ts @@ -3,7 +3,7 @@ import * as remediators from './remediators'; import { RemediationValues, Remediator, RemediatorConstructor } from './remediators'; import { GenericRemediator } from './remediators/GenericRemediator'; import { OktaAuthIdxInterface, IdxFeature, NextStep, RemediateOptions, RemediationResponse, RunOptions } from './types'; -import { IdxMessage, IdxRemediation, IdxRemediationValue, IdxResponse, IdxContext } from './types/idx-js'; +import { IdxMessage, IdxRemediation, IdxRemediationValue, IdxResponse } from './types/idx-js'; export function isTerminalResponse(idxResponse: IdxResponse) { const { neededToProceed, interactionCode } = idxResponse; From 8096b5e4f69f8bc5b5e742df171360b4252be0e1 Mon Sep 17 00:00:00 2001 From: "denys.oblohin" Date: Fri, 30 Jun 2023 12:41:58 +0300 Subject: [PATCH 05/10] sample-express-embedded-auth-with-sdk timeout --- .bacon.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.bacon.yml b/.bacon.yml index 7665107bf..747829bee 100644 --- a/.bacon.yml +++ b/.bacon.yml @@ -56,7 +56,7 @@ test_suites: - name: sample-express-embedded-auth-with-sdk script_path: ../okta-auth-js/scripts/samples sort_order: '6' - timeout: '20' + timeout: '30' script_name: e2e-express-embedded-auth-with-sdk criteria: MERGE queue_name: small From 997c9b7033851d588c0fd7368c8ebe48969ec9a9 Mon Sep 17 00:00:00 2001 From: "denys.oblohin" Date: Thu, 6 Jul 2023 11:50:18 +0300 Subject: [PATCH 06/10] change SSR feature spec . --- .../web-server/routes/authenticator.js | 6 ++-- ...vice-registration-custom-attribute.feature | 2 +- .../self-service-registration.feature | 16 +++++----- samples/test/steps/when.ts | 8 +++++ .../support/action/selectAuthenticator.ts | 10 +++++++ .../action/selectAuthenticatorMethod.ts | 29 +++++++++++++++++++ .../selectors/SelectAuthenticatorMethod.ts | 26 +++++++++++++++++ samples/test/support/selectors/index.ts | 2 ++ 8 files changed, 88 insertions(+), 11 deletions(-) create mode 100644 samples/test/support/action/selectAuthenticatorMethod.ts create mode 100644 samples/test/support/selectors/SelectAuthenticatorMethod.ts diff --git a/samples/generated/express-embedded-auth-with-sdk/web-server/routes/authenticator.js b/samples/generated/express-embedded-auth-with-sdk/web-server/routes/authenticator.js index 62bf597db..0bc3a6ae2 100644 --- a/samples/generated/express-embedded-auth-with-sdk/web-server/routes/authenticator.js +++ b/samples/generated/express-embedded-auth-with-sdk/web-server/routes/authenticator.js @@ -109,10 +109,11 @@ router.get('/enroll-authenticator/okta_email/enrollment-data', (req, res) => { const { options } = inputs[0]; renderPage({ req, res, - render: () => renderTemplate(req, res, 'select-authenticator', { + render: () => renderTemplate(req, res, 'select-authenticator-method', { options, action: '/enroll-authenticator/okta_email/enrollment-data', canSkip: false, + title: 'Select authenticator method' }) }); }); @@ -134,7 +135,8 @@ router.get('/enroll-authenticator/okta_email', (req, res) => { router.post('/enroll-authenticator/okta_email/enrollment-data', async (req, res, next) => { const authClient = getAuthClient(req); - const transaction = await authClient.idx.proceed({ authenticator: 'okta_email' }); + const { methodType } = req.body; + const transaction = await authClient.idx.proceed({ methodType }); handleTransaction({ req, res, next, authClient, transaction }); }); diff --git a/samples/test/features/self-service-registration-custom-attribute.feature b/samples/test/features/self-service-registration-custom-attribute.feature index 7c4dd96a8..1648132f8 100644 --- a/samples/test/features/self-service-registration-custom-attribute.feature +++ b/samples/test/features/self-service-registration-custom-attribute.feature @@ -16,4 +16,4 @@ Feature: Add another Required Attribute to the Profile Enrollment Policy And she fills out another property And she submits the form # Then her user is created in the "Staged" state - Then she is redirected to the "Select Authenticator" page + Then she is redirected to the "Select Authenticator Method" page diff --git a/samples/test/features/self-service-registration.feature b/samples/test/features/self-service-registration.feature index 2ef8f9416..f628241be 100644 --- a/samples/test/features/self-service-registration.feature +++ b/samples/test/features/self-service-registration.feature @@ -20,8 +20,8 @@ Scenario: Mary signs up for an account with Password, setups up required Email f And she fills out her Last Name And she fills out her Email And she submits the form - Then she is redirected to the "Select Authenticator" page - When she selects the "Email" factor + Then she is redirected to the "Select Authenticator Method" page + When she selects the "email" method And she submits the form Then she sees a page to input a code for email authenticator enrollment When she inputs the correct code from her "Email" @@ -46,8 +46,8 @@ Scenario: Mary signs up for an account with Password, setups up required Email f And she fills out her Last Name And she fills out her Email And she submits the form - Then she is redirected to the "Select Authenticator" page - When she selects the "Email" factor + Then she is redirected to the "Select Authenticator Method" page + When she selects the "email" method And she submits the form Then she sees a page to input a code for email authenticator enrollment When she inputs the correct code from her "Email" @@ -89,8 +89,8 @@ Scenario: Mary signs up for an account with Password, sets up required Email fac And she fills out her Last Name And she fills out her Email And she submits the form - Then she is redirected to the "Select Authenticator" page - When she selects the "Email" factor + Then she is redirected to the "Select Authenticator Method" page + When she selects the "email" method And she submits the form Then she sees a page to input a code for email authenticator enrollment When she inputs the correct code from her "Email" @@ -117,8 +117,8 @@ Scenario: Mary signs up for an account with Password, setups up required Email f And she fills out her Last Name And she fills out her Email And she submits the form - Then she is redirected to the "Select Authenticator" page - When she selects the "Email" factor + Then she is redirected to the "Select Authenticator Method" page + When she selects the "email" method And she submits the form Then she sees a page to input a code for email authenticator enrollment When she clicks the Email magic link for email verification diff --git a/samples/test/steps/when.ts b/samples/test/steps/when.ts index d04257f80..28f502334 100644 --- a/samples/test/steps/when.ts +++ b/samples/test/steps/when.ts @@ -20,6 +20,7 @@ import enterLiveUserEmail from '../support/action/context-enabled/live-user/ente import enterRecoveryEmail from '../support/action/context-enabled/live-user/enterRecoveyEmail'; import submitForm from '../support/action/submitForm'; import selectAuthenticator from '../support/action/selectAuthenticator'; +import selectAuthenticatorMethod from '../support/action/selectAuthenticatorMethod'; import selectEnrollmentChannel from '../support/action/selectEnrollmentChannel'; import inputInvalidEmail from '../support/action/inputInvalidEmail'; import enterRegistrationField from '../support/action/context-enabled/live-user/enterRegistrationField'; @@ -166,6 +167,13 @@ When( } ); +When( + 'she selects the {string} method', + async (methodType: string) => { + await selectAuthenticatorMethod(methodType); + } +); + When( 'she selects the {string} enrollment method', async (enrollmentMethod: string) => { diff --git a/samples/test/support/action/selectAuthenticator.ts b/samples/test/support/action/selectAuthenticator.ts index 20f260084..3e64de7be 100644 --- a/samples/test/support/action/selectAuthenticator.ts +++ b/samples/test/support/action/selectAuthenticator.ts @@ -11,9 +11,19 @@ */ +import { ElementArray } from 'webdriverio'; import SelectAuthenticator from '../selectors/SelectAuthenticator'; import selectOption from './selectOption'; export default async (authenticatorKey: string) => { + const $select = await $(SelectAuthenticator.options); + const options = await $select.$$('option') as ElementArray; + const optionsStr = (await Promise.all(options.map(async el => { + const value = await el.getAttribute('value'); + const text = await el.getText(); + return `${value}: ${text}`; + }))).join(', '); + console.log(`[debug] Should select authenticator ${authenticatorKey}. Available options: ${optionsStr}`); + await selectOption('value', authenticatorKey, SelectAuthenticator.options); }; diff --git a/samples/test/support/action/selectAuthenticatorMethod.ts b/samples/test/support/action/selectAuthenticatorMethod.ts new file mode 100644 index 000000000..05e57eb8f --- /dev/null +++ b/samples/test/support/action/selectAuthenticatorMethod.ts @@ -0,0 +1,29 @@ +/*! + * Copyright (c) 2015-present, Okta, Inc. and/or its affiliates. All rights reserved. + * The Okta software accompanied by this notice is provided pursuant to the Apache License, Version 2.0 (the "License.") + * + * You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0. + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * + * See the License for the specific language governing permissions and limitations under the License. + */ + + +import { ElementArray } from 'webdriverio'; +import SelectAuthenticatorMethod from '../selectors/SelectAuthenticatorMethod'; +import selectOption from './selectOption'; + +export default async (methodType: string) => { + const $select = await $(SelectAuthenticatorMethod.options); + const options = await $select.$$('option') as ElementArray; + const optionsStr = (await Promise.all(options.map(async el => { + const value = await el.getAttribute('value'); + const text = await el.getText(); + return `${value}: ${text}`; + }))).join(', '); + console.log(`[debug] Should select method ${methodType}. Available options: ${optionsStr}`); + + await selectOption('value', methodType, SelectAuthenticatorMethod.options); +}; diff --git a/samples/test/support/selectors/SelectAuthenticatorMethod.ts b/samples/test/support/selectors/SelectAuthenticatorMethod.ts new file mode 100644 index 000000000..890a4272b --- /dev/null +++ b/samples/test/support/selectors/SelectAuthenticatorMethod.ts @@ -0,0 +1,26 @@ +/*! + * Copyright (c) 2015-present, Okta, Inc. and/or its affiliates. All rights reserved. + * The Okta software accompanied by this notice is provided pursuant to the Apache License, Version 2.0 (the "License.") + * + * You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0. + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * + * See the License for the specific language governing permissions and limitations under the License. + */ + +import { PageWithTitle } from './Page'; + + +class SelectAuthenticatorMethod extends PageWithTitle { + title = 'Select authenticator method'; + + get pageTitle() {return '#page-title-header';} + + get options() { return '#authenticator-method-options'; } + get submit() { return '#verify-form button[type=submit]';} + get skip() { return '#skip-button'; } +} + +export default new SelectAuthenticatorMethod(); diff --git a/samples/test/support/selectors/index.ts b/samples/test/support/selectors/index.ts index df6f2bc1f..752e11f7b 100644 --- a/samples/test/support/selectors/index.ts +++ b/samples/test/support/selectors/index.ts @@ -21,6 +21,7 @@ import Unauth from './Unauth'; import UserHome from './UserHome'; import PasswordRecover from './PasswordRecover'; import SelectAuthenticator from './SelectAuthenticator'; +import SelectAuthenticatorMethod from './SelectAuthenticatorMethod'; import PasswordReset from './PasswordReset'; import Home from './Home'; import { Page } from './Page'; @@ -59,6 +60,7 @@ const pages: { [key: string]: Page } = { 'Self Service Password Reset View': PasswordRecover, 'Self Service Password Reset': PasswordRecover, 'Select Authenticator': SelectAuthenticator, + 'Select Authenticator Method': SelectAuthenticatorMethod, 'Enter Code': ChallengeEmailAuthenticator, 'Challenge email authenticator': ChallengeEmailAuthenticator, 'Challenge Password Authenticator': ChallengePasswordAuthenticator, From 0b2f16f6bc82d8928e821b10ac91d90274a917ac Mon Sep 17 00:00:00 2001 From: "denys.oblohin" Date: Fri, 7 Jul 2023 20:39:14 +0300 Subject: [PATCH 07/10] Use currentAuthenticatorEnrollment (OKTA-609234) --- lib/idx/remediators/Base/SelectAuthenticator.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/idx/remediators/Base/SelectAuthenticator.ts b/lib/idx/remediators/Base/SelectAuthenticator.ts index 96c979731..367a2e7fd 100644 --- a/lib/idx/remediators/Base/SelectAuthenticator.ts +++ b/lib/idx/remediators/Base/SelectAuthenticator.ts @@ -61,7 +61,9 @@ export class SelectAuthenticator Date: Mon, 10 Jul 2023 16:36:51 +0300 Subject: [PATCH 08/10] remove debug --- samples/test/support/action/selectAuthenticator.ts | 10 ---------- .../test/support/action/selectAuthenticatorMethod.ts | 10 ---------- 2 files changed, 20 deletions(-) diff --git a/samples/test/support/action/selectAuthenticator.ts b/samples/test/support/action/selectAuthenticator.ts index 3e64de7be..20f260084 100644 --- a/samples/test/support/action/selectAuthenticator.ts +++ b/samples/test/support/action/selectAuthenticator.ts @@ -11,19 +11,9 @@ */ -import { ElementArray } from 'webdriverio'; import SelectAuthenticator from '../selectors/SelectAuthenticator'; import selectOption from './selectOption'; export default async (authenticatorKey: string) => { - const $select = await $(SelectAuthenticator.options); - const options = await $select.$$('option') as ElementArray; - const optionsStr = (await Promise.all(options.map(async el => { - const value = await el.getAttribute('value'); - const text = await el.getText(); - return `${value}: ${text}`; - }))).join(', '); - console.log(`[debug] Should select authenticator ${authenticatorKey}. Available options: ${optionsStr}`); - await selectOption('value', authenticatorKey, SelectAuthenticator.options); }; diff --git a/samples/test/support/action/selectAuthenticatorMethod.ts b/samples/test/support/action/selectAuthenticatorMethod.ts index 05e57eb8f..e8f4d6b93 100644 --- a/samples/test/support/action/selectAuthenticatorMethod.ts +++ b/samples/test/support/action/selectAuthenticatorMethod.ts @@ -11,19 +11,9 @@ */ -import { ElementArray } from 'webdriverio'; import SelectAuthenticatorMethod from '../selectors/SelectAuthenticatorMethod'; import selectOption from './selectOption'; export default async (methodType: string) => { - const $select = await $(SelectAuthenticatorMethod.options); - const options = await $select.$$('option') as ElementArray; - const optionsStr = (await Promise.all(options.map(async el => { - const value = await el.getAttribute('value'); - const text = await el.getText(); - return `${value}: ${text}`; - }))).join(', '); - console.log(`[debug] Should select method ${methodType}. Available options: ${optionsStr}`); - await selectOption('value', methodType, SelectAuthenticatorMethod.options); }; From bf6aa0476a674231824c8d841ee82300c3cc8da8 Mon Sep 17 00:00:00 2001 From: "denys.oblohin" Date: Mon, 10 Jul 2023 16:36:58 +0300 Subject: [PATCH 09/10] add test --- .../idx/remediators/SelectAuthenticator.ts | 41 +++++++++++++++++-- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/test/spec/idx/remediators/SelectAuthenticator.ts b/test/spec/idx/remediators/SelectAuthenticator.ts index 3ea13be51..431f15ce7 100644 --- a/test/spec/idx/remediators/SelectAuthenticator.ts +++ b/test/spec/idx/remediators/SelectAuthenticator.ts @@ -1,21 +1,24 @@ -import { SelectAuthenticatorAuthenticate } from '../../../../lib/idx/remediators'; +import { SelectAuthenticatorAuthenticate, SelectAuthenticatorEnroll } from '../../../../lib/idx/remediators'; import { SelectAuthenticatorEnrollRemediationFactory, + SelectAuthenticatorAuthenticateRemediationFactory, AuthenticatorValueFactory, PhoneAuthenticatorOptionFactory, + EmailAuthenticatorOptionFactory, IdxContextFactory, PhoneAuthenticatorFactory, + EmailAuthenticatorFactory, } from '@okta/test.support/idx'; -describe('remediators/Base/SelectAuthenticator', () => { +describe('remediators/SelectAuthenticatorEnroll', () => { describe('canRemediate', () => { it('retuns false if matched authenticator is already the current one', () => { - const currentAuthenticator = PhoneAuthenticatorFactory.build(); + const currentAuthenticator = EmailAuthenticatorFactory.build(); const remediation = SelectAuthenticatorEnrollRemediationFactory.build({ value: [ AuthenticatorValueFactory.build({ options: [ - PhoneAuthenticatorOptionFactory.params({ + EmailAuthenticatorOptionFactory.params({ _authenticator: currentAuthenticator, }).build(), ] @@ -30,6 +33,36 @@ describe('remediators/Base/SelectAuthenticator', () => { const authenticators = [ currentAuthenticator, ]; + const r = new SelectAuthenticatorEnroll(remediation, { authenticators }); + expect(r.canRemediate(context)).toBe(false); + expect(r.canRemediate()).toBe(true); + }); + }); +}); + +describe('remediators/SelectAuthenticatorAuthenticate', () => { + describe('canRemediate', () => { + it('retuns false if matched authenticator is already the current one', () => { + const currentAuthenticatorEnrollment = PhoneAuthenticatorFactory.build(); + const remediation = SelectAuthenticatorAuthenticateRemediationFactory.build({ + value: [ + AuthenticatorValueFactory.build({ + options: [ + PhoneAuthenticatorOptionFactory.params({ + _authenticator: currentAuthenticatorEnrollment, + }).build(), + ] + }), + ] + }); + const context = IdxContextFactory.build({ + currentAuthenticatorEnrollment: { + value: currentAuthenticatorEnrollment + } + }); + const authenticators = [ + currentAuthenticatorEnrollment, + ]; const r = new SelectAuthenticatorAuthenticate(remediation, { authenticators }); expect(r.canRemediate(context)).toBe(false); expect(r.canRemediate()).toBe(true); From 5f3a51f14d5554edeb7775efc735da82aad61d4a Mon Sep 17 00:00:00 2001 From: "denys.oblohin" Date: Mon, 10 Jul 2023 18:02:27 +0300 Subject: [PATCH 10/10] lint fix . --- lib/idx/remediators/Base/SelectAuthenticator.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/idx/remediators/Base/SelectAuthenticator.ts b/lib/idx/remediators/Base/SelectAuthenticator.ts index 367a2e7fd..fb8947d90 100644 --- a/lib/idx/remediators/Base/SelectAuthenticator.ts +++ b/lib/idx/remediators/Base/SelectAuthenticator.ts @@ -41,6 +41,7 @@ export class SelectAuthenticator