From 960f2950aeefe9b74b0e293357db7c91404c5446 Mon Sep 17 00:00:00 2001 From: Mahesh Makani Date: Tue, 24 Sep 2024 11:09:56 +0100 Subject: [PATCH 1/3] feat(passwordless): reset password with passcodes handle case for non-existent users --- .../ete/reset_password_passcode.7.cy.ts | 25 ++++++++++++ .../pages/PasscodeEmailSent.stories.tsx | 14 +++++++ src/client/pages/PasscodeEmailSent.tsx | 3 ++ .../pages/ResetPasswordEmailSentPage.tsx | 1 + .../controllers/sendChangePasswordEmail.ts | 27 ++++++++++++- .../lib/okta/idx/shared/errorHandling.ts | 2 +- src/server/routes/resetPassword.ts | 39 ++++++++++++++++++- 7 files changed, 108 insertions(+), 3 deletions(-) diff --git a/cypress/integration/ete/reset_password_passcode.7.cy.ts b/cypress/integration/ete/reset_password_passcode.7.cy.ts index d88708edd..1770b3a9b 100644 --- a/cypress/integration/ete/reset_password_passcode.7.cy.ts +++ b/cypress/integration/ete/reset_password_passcode.7.cy.ts @@ -756,4 +756,29 @@ describe('Password reset recovery flows - with Passcodes', () => { }); }); }); + + context('NON_EXISTENT user', () => { + it('shows the passcode page with no account info, and using passcode returns error', () => { + const emailAddress = randomMailosaurEmail(); + cy.visit(`/reset-password?usePasscodesResetPassword=true`); + + cy.contains('Reset password'); + cy.get('input[name=email]').type(emailAddress); + cy.get('[data-cy="main-form-submit-button"]').click(); + + // passcode page + cy.url().should('include', '/reset-password/email-sent'); + cy.contains('Enter your one-time code'); + cy.contains('Don’t have an account?'); + + cy.get('input[name=code]').clear().type('123456'); + cy.contains('Submit one-time code').click(); + + cy.url().should('include', '/reset-password/code'); + cy.contains('Enter your one-time code'); + cy.contains('Don’t have an account?'); + + cy.contains('Incorrect code'); + }); + }); }); diff --git a/src/client/pages/PasscodeEmailSent.stories.tsx b/src/client/pages/PasscodeEmailSent.stories.tsx index ed30ee8eb..58e6d9ae4 100644 --- a/src/client/pages/PasscodeEmailSent.stories.tsx +++ b/src/client/pages/PasscodeEmailSent.stories.tsx @@ -47,6 +47,20 @@ WithPasscode.story = { name: 'with passcode', }; +export const WithNoAccountInfo = () => ( + +); +WithNoAccountInfo.story = { + name: 'with no account info', +}; + export const WithPasscodeError = () => ( { const [recaptchaErrorMessage, setRecaptchaErrorMessage] = useState(''); const [recaptchaErrorContext, setRecaptchaErrorContext] = @@ -147,6 +149,7 @@ export const PasscodeEmailSent = ({ formError={formError} queryString={queryString} shortRequestId={shortRequestId} + noAccountInfo={noAccountInfo} /> ); diff --git a/src/client/pages/ResetPasswordEmailSentPage.tsx b/src/client/pages/ResetPasswordEmailSentPage.tsx index 2b2733c99..f1c5f9c90 100644 --- a/src/client/pages/ResetPasswordEmailSentPage.tsx +++ b/src/client/pages/ResetPasswordEmailSentPage.tsx @@ -42,6 +42,7 @@ export const ResetPasswordEmailSentPage = () => { fieldErrors={fieldErrors} passcode={token} expiredPage={buildUrl('/reset-password/expired')} + noAccountInfo /> ); } diff --git a/src/server/controllers/sendChangePasswordEmail.ts b/src/server/controllers/sendChangePasswordEmail.ts index 19da1f882..68ce9791f 100644 --- a/src/server/controllers/sendChangePasswordEmail.ts +++ b/src/server/controllers/sendChangePasswordEmail.ts @@ -100,7 +100,7 @@ const setEncryptedCookieOkta = ( * - [x] With only password authenticator * - [x] With only email authenticator * - [x] Non-ACTIVE user states - * - [ ] Non-Existent users + * - [x] Non-Existent users - In `sendEmailInOkta` method * * @param {Request} req - Express request object * @param {ResponseWithRequestState} res - Express response object @@ -812,6 +812,31 @@ export const sendEmailInOkta = async ( error.status === 404 && error.code === 'E0000007' ) { + // if we're using passcodes, then show the email sent page with OTP input + // even if the user doesn't exist + if (passcodesEnabled && usePasscodesResetPassword) { + // set the encrypted state cookie to persist the email and stateHandle + // which we need to persist during the passcode reset flow + setEncryptedStateCookie(res, { + email, + stateHandle: `02.id.${crypto.randomBytes(30).toString('base64')}`, // generate a 40 character random string to use in the 46 character stateHandle + // 30 minutes in the future + stateHandleExpiresAt: new Date( + Date.now() + 30 * 60 * 1000, + ).toISOString(), + userState: 'NON_EXISTENT', // set the user state to non-existent, so we can handle this case if the user attempts to submit the passcode + }); + + // show the email sent page, with passcode instructions + return res.redirect( + 303, + addQueryParamsToPath( + '/reset-password/email-sent', + res.locals.queryParams, + ), + ); + } + setEncryptedCookieOkta(res, email); return res.redirect( diff --git a/src/server/lib/okta/idx/shared/errorHandling.ts b/src/server/lib/okta/idx/shared/errorHandling.ts index 7a6776933..2f7ffd9a8 100644 --- a/src/server/lib/okta/idx/shared/errorHandling.ts +++ b/src/server/lib/okta/idx/shared/errorHandling.ts @@ -48,7 +48,7 @@ export const handlePasscodeError = ({ const html = renderer(emailSentPage, { requestState: mergeRequestState(state, { queryParams: { - returnUrl: state.queryParams.returnUrl, + ...state.queryParams, emailSentSuccess: false, }, pageData: { diff --git a/src/server/routes/resetPassword.ts b/src/server/routes/resetPassword.ts index df6660f36..948305463 100644 --- a/src/server/routes/resetPassword.ts +++ b/src/server/routes/resetPassword.ts @@ -25,6 +25,8 @@ import { import { OAuthError } from '@/server/models/okta/Error'; import { forgotPassword } from '@/server/lib/okta/api/users'; import { buildUrlWithQueryParams } from '@/shared/lib/routeUtils'; +import { GenericErrors, RegistrationErrors } from '@/shared/model/Errors'; +import { logger } from '@/server/lib/serverSideLogger'; // reset password email form router.get('/reset-password', (req: Request, res: ResponseWithRequestState) => { @@ -145,9 +147,18 @@ router.post( // make sure we have the encrypted state cookie and the code otherwise redirect to the reset page if (encryptedState?.stateHandle && code) { - const { stateHandle } = encryptedState; + const { stateHandle, userState } = encryptedState; try { + // check for non-existent user state + // in this case throw an error to show the user the passcode is invalid + if (userState === 'NON_EXISTENT') { + throw new OAuthError({ + error: 'api.authn.error.PASSCODE_INVALID', + error_description: RegistrationErrors.PASSCODE_INVALID, + }); + } + // submit the passcode to Okta const challengeAnswerResponse = await submitPasscode({ passcode: code, @@ -250,6 +261,32 @@ router.post( if (res.headersSent) { return; } + + // log the error + logger.error(`${req.method} ${req.originalUrl} Error`, error); + + // handle any other error, show generic error message + const html = renderer('/reset-password/code', { + requestState: mergeRequestState(res.locals, { + queryParams: { + ...res.locals.queryParams, + emailSentSuccess: false, + }, + pageData: { + email: encryptedState?.email, + timeUntilTokenExpiry: convertExpiresAtToExpiryTimeInMs( + encryptedState?.stateHandleExpiresAt, + ), + formError: { + message: GenericErrors.DEFAULT, + severity: 'UNEXPECTED', + }, + token: code, + }, + }), + pageTitle: 'Check Your Inbox', + }); + return res.type('html').send(html); } } From de7e71fe6954848e61ba936a3b9dcff42ca29c8d Mon Sep 17 00:00:00 2001 From: Mahesh Makani Date: Tue, 24 Sep 2024 13:51:49 +0100 Subject: [PATCH 2/3] refactor(EncryptedState): validate and modify `stateHandle`, remove `undefined` values The `EncryptedState` cookie gets quite large due to the nature of encryption and addition verification tags and signing. In order to reduce the size of the cookie we set we have done two things. 1. For the Okta IDX API, it turns out we only need the first part of the `stateHandle` to persist. The API continues to work with just this bit. - The `stateHandle` string is 817 characters long, however only the first part before the `~` is needed, which is just 46 characters in length. - This is actually the same bit that is required for the `stateToken` parameter in the `/login/token/redirect` endpoint used after authentication - see: https://github.com/guardian/gateway/blob/34311cdf8524f247a11841fbf7bdf9568f5829ea/src/server/lib/okta/idx/shared/idxFetch.ts#L128 2. We remove `undefined` keys and values, this means we dont have to persist the key and undefined value in the encrypted JSON, thus saving space. --- src/server/lib/encryptedStateCookie.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/server/lib/encryptedStateCookie.ts b/src/server/lib/encryptedStateCookie.ts index 2a5c926d9..31a15549b 100644 --- a/src/server/lib/encryptedStateCookie.ts +++ b/src/server/lib/encryptedStateCookie.ts @@ -21,11 +21,27 @@ export const setEncryptedStateCookie = ( res: Response, state: EncryptedState, ) => { + // validate and modify any fields before encrypting + const validated: EncryptedState = { + ...state, + // We only need the first part of the state handle before the delimiter + // which is also much shorter to reduce the size of the cookie, but everything + // continues to work as expected + stateHandle: state.stateHandle?.split('~')[0], + }; + + // remove any undefined values + const cleaned: EncryptedState = Object.fromEntries( + Object.entries(validated).filter(([, value]) => value !== undefined), + ); + + // encrypt the state const encrypted = encrypt( - JSON.stringify(state), + JSON.stringify(cleaned), getConfiguration().encryptionSecretKey, // prevent the key from lingering in memory by only calling when needed ); + // set the cookie return res.cookie( encryptedStateCookieName, encrypted, From 2edf804926a4eec3cd3b20acd83326c4050660be Mon Sep 17 00:00:00 2001 From: Mahesh Makani Date: Wed, 25 Sep 2024 08:50:32 +0100 Subject: [PATCH 3/3] feat(idx): add additional metrics for reset password --- src/server/controllers/sendChangePasswordEmail.ts | 11 +++++++++++ src/server/models/Metrics.ts | 1 + 2 files changed, 12 insertions(+) diff --git a/src/server/controllers/sendChangePasswordEmail.ts b/src/server/controllers/sendChangePasswordEmail.ts index 68ce9791f..73ebaa7a0 100644 --- a/src/server/controllers/sendChangePasswordEmail.ts +++ b/src/server/controllers/sendChangePasswordEmail.ts @@ -472,6 +472,11 @@ const changePasswordEmailIdx = async ( flagStatus: false, }); + // track the success metrics + trackMetric( + `OktaIDXResetPasswordSend::${user.status as Status}::Success`, + ); + // now that the placeholder password has been set, the user will be in // 1. ACTIVE users - has email + password authenticator (okta idx email verified) // or 2. ACTIVE users - has only password authenticator (okta idx email not verified) @@ -495,6 +500,9 @@ const changePasswordEmailIdx = async ( logger.error('Okta changePasswordEmailIdx failed', error); + // track the failure metrics + trackMetric(`OktaIDXResetPasswordSend::${user.status as Status}::Failure`); + // don't throw the error, so we can fall back to okta classic flow } }; @@ -827,6 +835,9 @@ export const sendEmailInOkta = async ( userState: 'NON_EXISTENT', // set the user state to non-existent, so we can handle this case if the user attempts to submit the passcode }); + // track the success metrics + trackMetric(`OktaIDXResetPasswordSend::NON_EXISTENT::Success`); + // show the email sent page, with passcode instructions return res.redirect( 303, diff --git a/src/server/models/Metrics.ts b/src/server/models/Metrics.ts index 0fdcc24a5..a0e484253 100644 --- a/src/server/models/Metrics.ts +++ b/src/server/models/Metrics.ts @@ -38,6 +38,7 @@ type ConditionalMetrics = | 'OktaIDXInteract' | 'OktaIDXRegister' | 'OktaIDXResetPasswordSend' + | `OktaIDXResetPasswordSend::NON_EXISTENT` | `OktaIDXResetPasswordSend::${Status}` | `OktaIDX::${IDXPath}` | 'OktaRegistration'