fix: Don't auto remediate SelectAuthenticator with current authentica…

…tor (#1426)

OKTA-612939 fix: Don't auto remediate SelectAuthenticator with current authenticator

.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

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

lib/idx/remediate.ts

@@ -75,7 +75,7 @@ export async function remediate(
     return { idxResponse };
   }
 
-  const remediator = getRemediator(neededToProceed, values, options);
+  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)!;
+    const gr = getRemediator(idxResponse, values, options)!;
     const nextStep = getNextStep(authClient, gr, idxResponse);
     return {
       idxResponse,

lib/idx/remediators/Base/Remediator.ts

@@ -94,7 +94,7 @@ export class Remediator<T extends RemediationValues = RemediationValues> {
 
   // 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) {

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<T extends SelectAuthenticatorValues = SelectAut
 
   // Find matched authenticator in provided order
   findMatchedOption(authenticators, options) {
-    let option;
+    let option: IdxOption | undefined;
     for (let authenticator of authenticators) {
       option = options
         .find(({ relatesTo }) => relatesTo.key === authenticator.key);
@@ -41,7 +41,8 @@ export class SelectAuthenticator<T extends SelectAuthenticatorValues = SelectAut
     return option;
   }
 
-  canRemediate() {
+  /* eslint complexity:[0,9] */
+  canRemediate(context?: IdxContext) {
     const { authenticators, authenticator } = this.values;
     const authenticatorFromRemediation = getAuthenticatorFromRemediation(this.remediation);
     const { options } = authenticatorFromRemediation;
@@ -56,9 +57,14 @@ export class SelectAuthenticator<T extends SelectAuthenticatorValues = SelectAut
     }
 
     // Proceed with provided authenticators
-    const matchedOption = this.findMatchedOption(authenticators, options);
+    const matchedOption = this.findMatchedOption(authenticators, options!);
     if (matchedOption) {
-      return true;
+      // Don't select current authenticator (OKTA-612939)
+      const isCurrentAuthenticator = context?.currentAuthenticator
+        && context?.currentAuthenticator.value.id === matchedOption.relatesTo?.id;
+      const isCurrentAuthenticatorEnrollment = context?.currentAuthenticatorEnrollment
+        && context?.currentAuthenticatorEnrollment.value.id === matchedOption.relatesTo?.id;
+      return !isCurrentAuthenticator && !isCurrentAuthenticatorEnrollment;
     }
 
     return false;

lib/idx/util.ts

@@ -210,13 +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,
 ): 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 
@@ -247,7 +248,7 @@ export function getRemediator(
       // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
       const T = getRemediatorClass(remediation, options)!;
       remediator = new T(remediation, values, options);
-      if (remediator.canRemediate()) {
+      if (remediator.canRemediate(context)) {
         // found the remediator
         return remediator;
       }
@@ -284,7 +285,7 @@ export function handleFailedResponse(
   if (terminal) {
     return { idxResponse, terminal, messages };
   } else {
-    const remediator = getRemediator(idxResponse.neededToProceed, {}, options);
+    const remediator = getRemediator(idxResponse, {}, options);
     const nextStep = remediator && getNextStep(authClient, remediator, idxResponse);
     return {
       idxResponse,

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",

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 });
 });
 

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

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

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) => {

samples/test/support/action/selectAuthenticatorMethod.ts

@@ -0,0 +1,19 @@
+/*!
+ * 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 SelectAuthenticatorMethod from '../selectors/SelectAuthenticatorMethod';
+import selectOption from './selectOption';
+
+export default async (methodType: string) => {
+  await selectOption('value', methodType, SelectAuthenticatorMethod.options);
+};

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(); 

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,

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, {});
 
       });