Skip to content

Commit

Permalink
final pass
Browse files Browse the repository at this point in the history
  • Loading branch information
jaredperreault-okta committed Nov 9, 2023
1 parent 082a9b6 commit e32e657
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 123 deletions.
24 changes: 12 additions & 12 deletions lib/idx/remediators/Base/SelectAuthenticator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 };
}

Expand Down
1 change: 0 additions & 1 deletion lib/oidc/introspect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions samples/test/features/mfa-password-and-email.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
7 changes: 7 additions & 0 deletions samples/test/steps/after.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]';}
Expand Down
90 changes: 0 additions & 90 deletions test/spec/idx/authenticate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
]
});
});
});

});
29 changes: 11 additions & 18 deletions test/spec/idx/remediators/SelectAuthenticator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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: [
Expand Down Expand Up @@ -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: [
Expand All @@ -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);
});
Expand Down

0 comments on commit e32e657

Please sign in to comment.