From ad06fe7cb1f66e920b2f6be5ac28b8ccd0140f42 Mon Sep 17 00:00:00 2001 From: Justyn Oh Date: Fri, 18 Aug 2023 12:34:11 +0800 Subject: [PATCH] chore: refactors --- .../modules/auth/sgid/auth-sgid.controller.ts | 7 +- .../modules/auth/sgid/auth-sgid.service.ts | 14 +- .../public-form/public-form.controller.ts | 179 ++++++++++-------- src/app/modules/sgid/sgid.controller.ts | 1 - src/app/modules/sgid/sgid.errors.ts | 6 + src/app/modules/sgid/sgid.service.ts | 12 +- 6 files changed, 130 insertions(+), 89 deletions(-) diff --git a/src/app/modules/auth/sgid/auth-sgid.controller.ts b/src/app/modules/auth/sgid/auth-sgid.controller.ts index 06cace2713..99033b4703 100644 --- a/src/app/modules/auth/sgid/auth-sgid.controller.ts +++ b/src/app/modules/auth/sgid/auth-sgid.controller.ts @@ -1,4 +1,3 @@ -import { generatePkcePair } from '@opengovsg/sgid-client' import { StatusCodes } from 'http-status-codes' import { ErrorDto, GetSgidAuthUrlResponseDto } from 'shared/types' @@ -24,10 +23,8 @@ export const generateAuthUrl: ControllerHandler< ...createReqMeta(req), } - const { codeChallenge, codeVerifier } = generatePkcePair() - - return AuthSgidService.createRedirectUrl(codeChallenge) - .map((redirectUrl) => + return AuthSgidService.createRedirectUrl() + .map(({ redirectUrl, codeVerifier }) => res .status(StatusCodes.OK) .cookie(SGID_CODE_VERIFIER_COOKIE_NAME, codeVerifier) diff --git a/src/app/modules/auth/sgid/auth-sgid.service.ts b/src/app/modules/auth/sgid/auth-sgid.service.ts index 2213244f25..3322b95e75 100644 --- a/src/app/modules/auth/sgid/auth-sgid.service.ts +++ b/src/app/modules/auth/sgid/auth-sgid.service.ts @@ -1,4 +1,4 @@ -import { SgidClient } from '@opengovsg/sgid-client' +import { generatePkcePair, SgidClient } from '@opengovsg/sgid-client' import fs from 'fs' import { err, ok, Result, ResultAsync } from 'neverthrow' @@ -41,14 +41,18 @@ export class AuthSgidServiceClass { /** * Create a URL to sgID which is used to redirect the user for authentication + * @returns The redirectUrl and the associated code verifier */ - createRedirectUrl( - codeChallenge: string, - ): Result { + createRedirectUrl(): Result< + { redirectUrl: string; codeVerifier: string }, + SgidCreateRedirectUrlError + > { const logMeta = { action: 'createRedirectUrl', } + const { codeChallenge, codeVerifier } = generatePkcePair() + try { const result = this.client.authorizationUrl({ state: SGID_LOGIN_OAUTH_STATE, @@ -56,7 +60,7 @@ export class AuthSgidServiceClass { nonce: null, codeChallenge, }) - return ok(result.url) + return ok({ redirectUrl: result.url, codeVerifier }) } catch (error) { logger.error({ message: 'Error while creating redirect URL', diff --git a/src/app/modules/form/public-form/public-form.controller.ts b/src/app/modules/form/public-form/public-form.controller.ts index bafe1beb0a..29d27ecc67 100644 --- a/src/app/modules/form/public-form/public-form.controller.ts +++ b/src/app/modules/form/public-form/public-form.controller.ts @@ -1,7 +1,6 @@ -import { generatePkcePair } from '@opengovsg/sgid-client' import { celebrate, Joi, Segments } from 'celebrate' import { StatusCodes } from 'http-status-codes' -import { err } from 'neverthrow' +import { err, ok, Result } from 'neverthrow' import { UnreachableCaseError } from 'ts-essentials' import { @@ -39,7 +38,11 @@ import { SGID_MYINFO_COOKIE_NAME, SGID_MYINFO_LOGIN_COOKIE_NAME, } from '../../sgid/sgid.constants' -import { SgidInvalidJwtError, SgidVerifyJwtError } from '../../sgid/sgid.errors' +import { + SgidInvalidJwtError, + SgidMalformedMyInfoCookieError, + SgidVerifyJwtError, +} from '../../sgid/sgid.errors' import { SgidService } from '../../sgid/sgid.service' import { validateSgidForm } from '../../sgid/sgid.util' import { InvalidJwtError, VerifyJwtError } from '../../spcp/spcp.errors' @@ -276,55 +279,79 @@ export const handleGetPublicForm: ControllerHandler< return res.json({ form: publicForm, isIntranetUser }) }) case FormAuthType.SGID_MyInfo: { - const { jwt: accessToken = '', sub = '' } = JSON.parse( - req.cookies[SGID_MYINFO_COOKIE_NAME] ?? '{}', - ) - if (!accessToken) { - return res.json({ - form: publicForm, - isIntranetUser, - }) - } - res.clearCookie(SGID_MYINFO_COOKIE_NAME) - res.clearCookie(SGID_MYINFO_LOGIN_COOKIE_NAME) - return SgidService.extractSgidJwtMyInfoPayload(accessToken) - .asyncAndThen((auth) => - SgidService.retrieveUserInfo({ accessToken: auth.accessToken, sub }), - ) - .andThen((userInfo) => { - const data = new SGIDMyInfoData(userInfo.data) - return MyInfoService.prefillAndSaveMyInfoFields( - form._id, - data, - form.toJSON().form_fields, - ).map((prefilledFields) => { - return res - .cookie( - SGID_MYINFO_LOGIN_COOKIE_NAME, - createMyInfoLoginCookie(data.getUinFin()), - MYINFO_LOGIN_COOKIE_OPTIONS, - ) - .json({ - form: { - ...publicForm, - form_fields: prefilledFields as FormFieldDto[], - }, - spcpSession: { userName: data.getUinFin() }, - isIntranetUser, - }) - }) - }) - .mapErr((error) => { + const parseSgidMyInfoCookie = Result.fromThrowable( + () => + JSON.parse(req.cookies[SGID_MYINFO_COOKIE_NAME] ?? '{}') as { + jwt?: string + sub?: string + }, + (error) => { logger.error({ - message: 'sgID: MyInfo login error', + message: 'Error while calling JSON.parse on SGID MyInfo cookie', meta: logMeta, error, }) - return res.json({ + return new SgidMalformedMyInfoCookieError() + }, + ) + return parseSgidMyInfoCookie() + .mapErr(() => + res.json({ form: publicForm, - myInfoError: true, isIntranetUser, - }) + }), + ) + .map(({ jwt: accessToken = '', sub = '' }) => { + if (!accessToken) { + return res.json({ + form: publicForm, + isIntranetUser, + }) + } + res.clearCookie(SGID_MYINFO_COOKIE_NAME) + res.clearCookie(SGID_MYINFO_LOGIN_COOKIE_NAME) + return SgidService.extractSgidJwtMyInfoPayload(accessToken) + .asyncAndThen((auth) => + SgidService.retrieveUserInfo({ + accessToken: auth.accessToken, + sub, + }), + ) + .andThen((userInfo) => { + const data = new SGIDMyInfoData(userInfo.data) + return MyInfoService.prefillAndSaveMyInfoFields( + form._id, + data, + form.toJSON().form_fields, + ).map((prefilledFields) => { + return res + .cookie( + SGID_MYINFO_LOGIN_COOKIE_NAME, + createMyInfoLoginCookie(data.getUinFin()), + MYINFO_LOGIN_COOKIE_OPTIONS, + ) + .json({ + form: { + ...publicForm, + form_fields: prefilledFields as FormFieldDto[], + }, + spcpSession: { userName: data.getUinFin() }, + isIntranetUser, + }) + }) + }) + .mapErr((error) => { + logger.error({ + message: 'sgID: MyInfo login error', + meta: logMeta, + error, + }) + return res.json({ + form: publicForm, + myInfoError: true, + isIntranetUser, + }) + }) }) } default: @@ -459,37 +486,41 @@ export const _handleFormAuthRedirect: ControllerHandler< }) } case FormAuthType.SGID: - return validateSgidForm(form).andThen(() => { - const { codeChallenge, codeVerifier } = generatePkcePair() - res.cookie( - SGID_CODE_VERIFIER_COOKIE_NAME, - codeVerifier, - SgidService.getCookieSettings(), + return validateSgidForm(form) + .andThen(() => + SgidService.createRedirectUrl( + formId, + Boolean(isPersistentLogin), + [], + encodedQuery, + ), ) - return SgidService.createRedirectUrl( - formId, - Boolean(isPersistentLogin), - [], - codeChallenge, - encodedQuery, - ) - }) + .andThen(({ redirectUrl, codeVerifier }) => { + res.cookie( + SGID_CODE_VERIFIER_COOKIE_NAME, + codeVerifier, + SgidService.getCookieSettings(), + ) + return ok(redirectUrl) + }) case FormAuthType.SGID_MyInfo: - return validateSgidForm(form).andThen(() => { - const { codeChallenge, codeVerifier } = generatePkcePair() - res.cookie( - SGID_CODE_VERIFIER_COOKIE_NAME, - codeVerifier, - SgidService.getCookieSettings(), + return validateSgidForm(form) + .andThen(() => + SgidService.createRedirectUrl( + formId, + false, + form.getUniqueMyInfoAttrs(), + encodedQuery, + ), ) - return SgidService.createRedirectUrl( - formId, - false, - form.getUniqueMyInfoAttrs(), - codeChallenge, - encodedQuery, - ) - }) + .andThen(({ redirectUrl, codeVerifier }) => { + res.cookie( + SGID_CODE_VERIFIER_COOKIE_NAME, + codeVerifier, + SgidService.getCookieSettings(), + ) + return ok(redirectUrl) + }) default: return err( new AuthTypeMismatchError(form.authType), diff --git a/src/app/modules/sgid/sgid.controller.ts b/src/app/modules/sgid/sgid.controller.ts index f2bb54a5ca..8223649dae 100644 --- a/src/app/modules/sgid/sgid.controller.ts +++ b/src/app/modules/sgid/sgid.controller.ts @@ -68,7 +68,6 @@ export const handleLogin: ControllerHandler< return res.redirect(target) } - // const codeVerifier = req.cookies.get(SGID_CODE_VERIFIER_COOKIE_NAME) const codeVerifier = req.cookies[SGID_CODE_VERIFIER_COOKIE_NAME] if (!codeVerifier) { logger.error({ diff --git a/src/app/modules/sgid/sgid.errors.ts b/src/app/modules/sgid/sgid.errors.ts index 0d0467ddbe..7860f1c9ab 100644 --- a/src/app/modules/sgid/sgid.errors.ts +++ b/src/app/modules/sgid/sgid.errors.ts @@ -24,6 +24,12 @@ export class SgidFetchUserInfoError extends ApplicationError { } } +export class SgidMalformedMyInfoCookieError extends ApplicationError { + constructor(message = 'SGID MyInfo cookie is malformed') { + super(message) + } +} + /** * JWT could not be decoded. */ diff --git a/src/app/modules/sgid/sgid.service.ts b/src/app/modules/sgid/sgid.service.ts index 36eab3ca4e..5533791300 100644 --- a/src/app/modules/sgid/sgid.service.ts +++ b/src/app/modules/sgid/sgid.service.ts @@ -1,4 +1,4 @@ -import { SgidClient } from '@opengovsg/sgid-client' +import { generatePkcePair, SgidClient } from '@opengovsg/sgid-client' import fs from 'fs' import Jwt from 'jsonwebtoken' import { err, ok, Result, ResultAsync } from 'neverthrow' @@ -74,14 +74,17 @@ export class SgidServiceClass { * @param requestedAttributes - sgID attributes requested by this form * @param encodedQuery base64 encoded queryId for frontend to retrieve stored query params (usually contains prefilled form information) * for an extended period of time + * @returns The redirectUrl and the associated code verifier */ createRedirectUrl( formId: string, rememberMe: boolean, requestedAttributes: InternalAttr[], - codeChallenge: string, encodedQuery?: string, - ): Result { + ): Result< + { redirectUrl: string; codeVerifier: string }, + SgidCreateRedirectUrlError + > { const state = encodedQuery ? `${formId},${rememberMe},${encodedQuery}` : `${formId},${rememberMe}` @@ -91,6 +94,7 @@ export class SgidServiceClass { state, } const scopes = internalAttrListToScopes(requestedAttributes) + const { codeChallenge, codeVerifier } = generatePkcePair() const result = this.client.authorizationUrl({ state, scope: scopes, @@ -98,7 +102,7 @@ export class SgidServiceClass { codeChallenge, }) if (typeof result.url === 'string') { - return ok(result.url) + return ok({ redirectUrl: result.url, codeVerifier }) } else { logger.error({ message: 'Error while creating redirect URL',