final pass

Author: jaredperreault-okta

lib/idx/remediators/Base/SelectAuthenticator.ts

@@ -60,18 +60,19 @@ export class SelectAuthenticator<T extends SelectAuthenticatorValues = SelectAut
     const matchedOption = this.findMatchedOption(authenticators, options!);
     if (matchedOption) {
 
-      // const isAuthenticatorCurrent = (auth) => {
-      //   return auth && !auth.value?.resend
-      //     && auth.value.id === matchedOption.relatesTo?.id;
-      // };
-
-      // // Don't select current authenticator (OKTA-612939)
-      // // Follow up: OKTA-646147 - original fix caused different issue
-      // const isCurrentAuthenticator = isAuthenticatorCurrent(context?.currentAuthenticator);                          // false
-      // const isCurrentAuthenticatorEnrollment = isAuthenticatorCurrent(context?.currentAuthenticatorEnrollment);      // true
-      // return !isCurrentAuthenticator && !isCurrentAuthenticatorEnrollment;                                           // false
+      // fix for OKTA-612939 (below) seems to have caused a bug when trying to re-select authenticators
+      // with multiple methodTypes. If `options.step` is passed, this remediation is explicitly being
+      // invoked, therefore do not guard against auto-remediating the selected authenticator (OKTA-646147)
+      if (this.options.step) {
+        return true;
+      }
 
-      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;
@@ -112,7 +113,6 @@ export class SelectAuthenticator<T extends SelectAuthenticatorValues = SelectAut
       .filter(authenticator => {
         return compareAuthenticators(authenticator, this.selectedAuthenticator) !== true;
       });
-    // return { ...this.values, authenticators, authenticator: undefined };
     return { ...this.values, authenticators };
   }
 

lib/oidc/introspect.ts

@@ -44,7 +44,6 @@ export async function oidcIntrospect (sdk, kind: TokenKind, token?: Token) {
   else {
     issuer = (token as any)?.claims?.iss;
   }
-  // issuer ??= sdk.options.issuer;
   issuer = issuer || sdk.options.issuer;
 
   if (!clientId) {

samples/test/features/mfa-password-and-email-magic-link.feature

@@ -16,7 +16,7 @@ Feature: Multi-Factor Authentication with Password and Email Magic Link
       And she has inserted her password
       And her password is correct
     When she submits the form
-    Then she is redirected to the "Verify Email" page
+    Then she is redirected to the "Verify Email" page       # fails here
     When she selects "Email" from the list of methods
       And she submits the form
       And she clicks the Email magic link

samples/test/features/mfa-password-and-email.feature

@@ -14,6 +14,7 @@ Feature: Multi-Factor Authentication with Password and Email
     When she fills in her username
       And she fills in her correct password
       And she submits the form
+    # fails here
     Then she is redirected to the "Verify Email" page
     When she selects "Email" from the list of methods
       And she submits the form
@@ -30,6 +31,7 @@ Feature: Multi-Factor Authentication with Password and Email
       And she has inserted her password
       And her password is correct
     When she submits the form
+    # fails here
     Then she is redirected to the "Verify Email" page
     When she selects "Email" from the list of methods
       And she submits the form

samples/test/steps/after.ts

@@ -12,10 +12,17 @@
 
 /* eslint-disable complexity */
 
+// import { After, AfterStep, Status } from '@cucumber/cucumber';
 import { After } from '@cucumber/cucumber';
 import ActionContext from '../support/context';
 import deleteSelfEnrolledUser from '../support/management-api/deleteSelfEnrolledUser';
 
+// NOTE: can be used to debug cucumber tests, just uncomment
+// AfterStep(async ({ result }) => {
+//   if (result.status === Status.FAILED) {
+//     await browser.debug();
+//   }
+// });
 
 // Comment out this after hook to persist test context
 // Extend the hook timeout to fight against org rate limit

samples/test/support/selectors/SelectAuthenticatorMethod.ts

@@ -17,7 +17,6 @@ class SelectAuthenticatorMethod extends PageWithTitle {
   title = 'Select authenticator method';
 
   get pageTitle() {return '#page-title-header';}
-  // get pageTitle() {return '#select-authenticator-page-title-header';}
 
   get options() { return '#authenticator-method-options';  }
   get submit() { return '#verify-form button[type=submit]';}

test/spec/idx/authenticate.ts

@@ -2524,94 +2524,4 @@ describe('idx/authenticate', () => {
     });
   });
 
-  describe('clears values.authenticator', () => {
-
-    beforeEach(() => {
-      const selectAuthenticatorResponse = IdxResponseFactory.build({
-        neededToProceed: [
-          SelectAuthenticatorAuthenticateRemediationFactory.build({
-            value: [
-              AuthenticatorValueFactory.build({
-                options: [
-                  EmailAuthenticatorOptionFactory.build(),
-                ]
-              })
-            ]
-          })
-        ],
-      });
-      const verifyAuthenticatorResponse = IdxResponseFactory.build({
-        neededToProceed: [
-          VerifyEmailRemediationFactory.build()
-        ],
-        context: IdxContextFactory.build({
-          authenticatorEnrollments: {
-            type: 'array',
-            value: [
-              EmailAuthenticatorFactory.build()
-            ]
-          }
-        })
-      });
-
-      Object.assign(testContext, {
-        selectAuthenticatorResponse,
-        verifyAuthenticatorResponse,
-      });
-    });
-
-    // OKTA-609234 - specifically confirm these remediation steps
-    fit('clears values.authenticator after being consumed to prevent auto-remediating', async () => {
-      const {
-        authClient,
-        selectAuthenticatorResponse,
-        verifyAuthenticatorResponse
-      } = testContext;
-
-      chainResponses([
-        selectAuthenticatorResponse,
-        verifyAuthenticatorResponse
-      ]);
-
-      jest.spyOn(selectAuthenticatorResponse, 'proceed');
-      jest.spyOn(mocked.introspect, 'introspect').mockResolvedValue(selectAuthenticatorResponse);
-
-      const res = await authenticate(authClient, {
-        username: 'foo',
-        authenticator: AuthenticatorKey.OKTA_EMAIL,
-        methodType: 'email'
-      });
-
-      expect(selectAuthenticatorResponse.proceed).toHaveBeenCalledWith('select-authenticator-authenticate', { 
-        authenticator: { id: 'id-email' } 
-      });
-      expect(res.status).toBe(IdxStatus.PENDING);
-      expect(res.nextStep).toEqual({
-        name: 'challenge-authenticator',
-        type: 'email',
-        authenticator: {
-          displayName: 'Email',
-          id: expect.any(String),
-          key: 'okta_email',
-          methods: [
-            { type: 'email' }
-          ],
-          type: 'email',
-        },
-        authenticatorEnrollments: [{
-          id: expect.any(String),
-          displayName: 'Email',
-          key: 'okta_email',
-          type: 'email',
-          methods: [
-            { type: 'email' }
-          ],
-        }],
-        inputs: [
-          { name: 'verificationCode', type: 'string', label: 'Enter code', required: true },
-        ]
-      });
-    });
-  });
-
 });

test/spec/idx/remediators/SelectAuthenticator.ts

@@ -19,7 +19,7 @@ describe('remediators/Base/SelectAuthenticator', () => {
           AuthenticatorValueFactory.build({
             options: [
               PhoneAuthenticatorOptionFactory.params({
-                // prevent resolving of authenticator by `relatesTo` in purpose
+                // prevent resolving of authenticator by `relatesTo` on purpose
                 // eslint-disable-next-line @typescript-eslint/no-explicit-any
                 _authenticator: 'cant_be_resolved' as any
               }).build(),
@@ -39,7 +39,7 @@ describe('remediators/Base/SelectAuthenticator', () => {
 
 describe('remediators/SelectAuthenticatorEnroll', () => {
   describe('canRemediate', () => {
-    xit('retuns false if matched authenticator is already the current one', () => {
+    it('retuns false if matched authenticator is already the current one', () => {
       const currentAuthenticator = EmailAuthenticatorFactory.build();
       const remediation = SelectAuthenticatorEnrollRemediationFactory.build({
         value: [
@@ -69,7 +69,7 @@ describe('remediators/SelectAuthenticatorEnroll', () => {
 
 describe('remediators/SelectAuthenticatorAuthenticate', () => {
   describe('canRemediate', () => {
-    xit('retuns false if matched authenticator is already the current one', () => {
+    it('retuns false if matched authenticator is already the current one', () => {
       const currentAuthenticatorEnrollment = PhoneAuthenticatorFactory.build();
       const remediation = SelectAuthenticatorAuthenticateRemediationFactory.build({
         value: [
@@ -95,36 +95,29 @@ describe('remediators/SelectAuthenticatorAuthenticate', () => {
       expect(r.canRemediate()).toBe(true);
     });
 
-    it('returns true if matched authenticator has a resend form', () => {
-      const phoneAuthenticator = PhoneAuthenticatorFactory.build();
+    // Fix for OKTA-646147
+    it('retuns true if `options.step` is explicitly passed', () => {
+      const currentAuthenticatorEnrollment = PhoneAuthenticatorFactory.build();
       const remediation = SelectAuthenticatorAuthenticateRemediationFactory.build({
         value: [
           AuthenticatorValueFactory.build({
             options: [
               PhoneAuthenticatorOptionFactory.params({
-                _authenticator: phoneAuthenticator,
+                _authenticator: currentAuthenticatorEnrollment,
               }).build(),
             ]
           }),
         ]
       });
       const context = IdxContextFactory.build({
         currentAuthenticatorEnrollment: {
-          value: {
-            ...phoneAuthenticator,
-            resend: ResendAuthenticatorFactory.build(),
-          }
-        },
-        authenticatorEnrollments: {
-          value: [phoneAuthenticator]
-        },
-        currentAuthenticator: {}
+          value: currentAuthenticatorEnrollment
+        }
       });
-
       const authenticators = [
-        phoneAuthenticator,
+        currentAuthenticatorEnrollment,
       ];
-      const r = new SelectAuthenticatorAuthenticate(remediation, { authenticators });
+      const r = new SelectAuthenticatorAuthenticate(remediation, { authenticators }, { step: 'select-authenticator-authenticate'});
       expect(r.canRemediate(context)).toBe(true);
       expect(r.canRemediate()).toBe(true);
     });