Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: MyInfo over sgID backend #6337

Merged
merged 23 commits into from
Jun 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
f04b12f
feat: MyInfo over sgID backend
kenjin-work May 16, 2023
efb3c54
refactor: reduce diff
kenjin-work May 16, 2023
bf34beb
feat: MyInfo over sgID backend
kenjin-work May 16, 2023
4d6c5eb
refactor: reduce diff
kenjin-work May 16, 2023
55a7636
Merge branch 'feat/sgid-myinfo-backend-beta' of https://github.com/op…
kenjin-work May 16, 2023
d0e1d3c
fix: return undefined in default cases
kenjin-work May 16, 2023
c8b73c1
fix: sgID validation of forms
kenjin-work May 16, 2023
e263733
fix: sgID submission tests
kenjin-work May 16, 2023
893c5c4
fix: email submission route tests
kenjin-work May 16, 2023
046159f
feat: split sgID auth and sgID MyInfo
kenjin-work May 17, 2023
35357d7
fix: rename functions
kenjin-work May 17, 2023
9a7e45b
fix: tests, add SGID_MyInfo form filling
kenjin-work May 17, 2023
f7ca1ce
feat: backend submissions and logout done
kenjin-work May 17, 2023
477078c
fix: sgID routes tests
kenjin-work May 17, 2023
4e547c0
fix: actually fix sgID routes tests
kenjin-work May 17, 2023
646f503
fix: verification controller tests
kenjin-work May 17, 2023
0f1cbe3
Address Shu Li's review
kenjin-work May 18, 2023
d999e4f
Address second round of reviews
kenjin-work May 19, 2023
56cfb0a
Split up login cookies for MyInfo and sgID-MyInfo
kenjin-work May 19, 2023
f7e90f7
Remove unsued param docs
kenjin-work May 22, 2023
4d6a58d
fix: turn off rememberMe for sgID over MyInfo
kenjin-work May 22, 2023
d88b4c0
fix: address reviews
kenjin-work May 25, 2023
39f311b
refactor: address review
kenjin-work Jun 9, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions shared/types/converter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
export interface MyInfoDataTransformer<T, U> {

/**
* NRIC/FIN.
*/
getUinFin(): string

/**
* Formats the sgID information to a string.
*/
_formatFieldValue(attr: T): string | undefined

/**
* Determine if frontend should lock the field to prevent it from being
* editable. The field is locked if it is government-verified and if it
* does not contain marriage-related information (decision by SNDGO & MSF due to
* overseas unregistered marriages).
*
* @param attr The field/attribute name directly obtained from the sgID
* information source.
* @param fieldValue FormSG field value.
*/
_isDataReadOnly(
attr: T,
fieldValue: string | undefined,
): boolean

/**
* Retrieves the fieldValue for the givern internal
* sgID-compatible attribute.
* @param attr Internal FormSG sgID attribute.
*/
getFieldValueForAttr(attr: U): {
fieldValue: string | undefined
isReadOnly: boolean
}
}
1 change: 1 addition & 0 deletions shared/types/form/form.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export enum FormAuthType {
CP = 'CP',
MyInfo = 'MyInfo',
SGID = 'SGID',
SGID_MyInfo = 'SGID_MyInfo',
}

export enum FormStatus {
Expand Down
1 change: 1 addition & 0 deletions shared/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ export * from './submission'
export * from './user'
export * from './utils'
export * from './payment'
export * from './converter'
8 changes: 6 additions & 2 deletions src/app/models/form.server.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,8 @@ const compileFormModel = (db: Mongoose): IFormModel => {
)
return (
myInfoFieldCount === 0 ||
(this.authType === FormAuthType.MyInfo &&
((this.authType === FormAuthType.MyInfo ||
this.authType == FormAuthType.SGID_MyInfo) &&
this.responseMode === FormResponseMode.Email &&
myInfoFieldCount <= 30)
)
Expand Down Expand Up @@ -559,7 +560,10 @@ const compileFormModel = (db: Mongoose): IFormModel => {

// Method to return myInfo attributes
FormSchema.methods.getUniqueMyInfoAttrs = function () {
if (this.authType !== FormAuthType.MyInfo) {
if (
this.authType !== FormAuthType.MyInfo &&
this.authType !== FormAuthType.SGID_MyInfo
) {
return []
}

Expand Down
72 changes: 71 additions & 1 deletion src/app/modules/form/public-form/public-form.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ import {
extractAuthCode,
validateMyInfoForm,
} from '../../myinfo/myinfo.util'
import { SGIDMyInfoData } from '../../sgid/sgid.adapter'
import {
SGID_COOKIE_NAME,
SGID_MYINFO_COOKIE_NAME,
SGID_MYINFO_LOGIN_COOKIE_NAME,
} from '../../sgid/sgid.constants'
import { SgidInvalidJwtError, SgidVerifyJwtError } from '../../sgid/sgid.errors'
import { SgidService } from '../../sgid/sgid.service'
import { validateSgidForm } from '../../sgid/sgid.util'
Expand Down Expand Up @@ -299,7 +305,9 @@ export const handleGetPublicForm: ControllerHandler<
})
}
case FormAuthType.SGID:
return SgidService.extractSgidJwtPayload(req.cookies.jwtSgid)
return SgidService.extractSgidSingpassJwtPayload(
req.cookies[SGID_COOKIE_NAME],
)
.map((spcpSession) => {
return res.json({
form: publicForm,
Expand All @@ -321,6 +329,56 @@ export const handleGetPublicForm: ControllerHandler<
}
return res.json({ form: publicForm, isIntranetUser })
})
case FormAuthType.SGID_MyInfo: {
const accessTokenCookie = req.cookies[SGID_MYINFO_COOKIE_NAME]
if (!accessTokenCookie) {
return res.json({
form: publicForm,
isIntranetUser,
})
}
res.clearCookie(SGID_MYINFO_COOKIE_NAME)
res.clearCookie(SGID_MYINFO_LOGIN_COOKIE_NAME)
return SgidService.extractSgidJwtMyInfoPayload(accessTokenCookie)
.asyncAndThen((auth) =>
SgidService.retrieveUserInfo({ accessToken: auth.accessToken }),
)
.andThen((userInfo) => {
const data = new SGIDMyInfoData(userInfo.data)
return MyInfoService.prefillAndSaveMyInfoFields(
tshuli marked this conversation as resolved.
Show resolved Hide resolved
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:
return new UnreachableCaseError(authType)
}
Expand Down Expand Up @@ -403,6 +461,16 @@ export const _handleFormAuthRedirect: ControllerHandler<
return SgidService.createRedirectUrl(
formId,
Boolean(isPersistentLogin),
[],
kenjin-work marked this conversation as resolved.
Show resolved Hide resolved
encodedQuery,
)
})
case FormAuthType.SGID_MyInfo:
return validateSgidForm(form).andThen(() => {
return SgidService.createRedirectUrl(
formId,
false,
form.getUniqueMyInfoAttrs(),
encodedQuery,
)
})
Expand Down Expand Up @@ -468,6 +536,7 @@ export const _handlePublicAuthLogout: ControllerHandler<
| FormAuthType.CP
| FormAuthType.MyInfo
| FormAuthType.SGID
| FormAuthType.SGID_MyInfo
},
PublicFormAuthLogoutDto
> = (req, res) => {
Expand All @@ -494,6 +563,7 @@ export const handlePublicAuthLogout = [
FormAuthType.CP,
FormAuthType.MyInfo,
FormAuthType.SGID,
FormAuthType.SGID_MyInfo,
)
.required(),
}),
Expand Down
10 changes: 8 additions & 2 deletions src/app/modules/form/public-form/public-form.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ import getFormModel from '../../../models/form.server.model'
import getFormFeedbackModel from '../../../models/form_feedback.server.model'
import { DatabaseError } from '../../core/core.errors'
import { MYINFO_LOGIN_COOKIE_NAME } from '../../myinfo/myinfo.constants'
import { SGID_COOKIE_NAME } from '../../sgid/sgid.constants'
import {
SGID_COOKIE_NAME,
SGID_MYINFO_LOGIN_COOKIE_NAME,
} from '../../sgid/sgid.constants'
import { JwtName } from '../../spcp/spcp.types'
import { FormNotFoundError } from '../form.errors'

Expand Down Expand Up @@ -73,9 +76,12 @@ export const getCookieNameByAuthType = (
| FormAuthType.SP
| FormAuthType.CP
| FormAuthType.MyInfo
| FormAuthType.SGID,
| FormAuthType.SGID
| FormAuthType.SGID_MyInfo,
): string => {
switch (authType) {
case FormAuthType.SGID_MyInfo:
tshuli marked this conversation as resolved.
Show resolved Hide resolved
return SGID_MYINFO_LOGIN_COOKIE_NAME
case FormAuthType.MyInfo:
return MYINFO_LOGIN_COOKIE_NAME
case FormAuthType.SGID:
Expand Down
9 changes: 7 additions & 2 deletions src/app/modules/myinfo/myinfo.adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ import {
MyInfoSource,
} from '@opengovsg/myinfo-gov-client'

import { MyInfoAttribute as InternalAttr } from '../../../../shared/types'
import {
MyInfoAttribute as InternalAttr,
MyInfoDataTransformer,
} from '../../../../shared/types'

import {
formatAddress,
Expand Down Expand Up @@ -155,7 +158,9 @@ export const internalAttrListToScopes = (
* extract the correct data by translating internal FormSG attributes
* to the correct keys in the data.
*/
export class MyInfoData {
export class MyInfoData
implements MyInfoDataTransformer<ExternalAttr, InternalAttr>
{
#personData: IPerson
#uinFin: string

Expand Down
3 changes: 2 additions & 1 deletion src/app/modules/myinfo/myinfo.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
AuthTypeMismatchError,
FormAuthNoEsrvcIdError,
} from '../form/form.errors'
import { SGIDMyInfoData } from '../sgid/sgid.adapter'
import { ProcessedFieldResponse } from '../submission/submission.types'

import { internalAttrListToScopes, MyInfoData } from './myinfo.adapter'
Expand Down Expand Up @@ -241,7 +242,7 @@ export class MyInfoServiceClass {
*/
prefillAndSaveMyInfoFields(
formId: string,
myInfoData: MyInfoData,
myInfoData: MyInfoData | SGIDMyInfoData,
currFormFields: LeanDocument<IFieldSchema[]>,
): ResultAsync<PossiblyPrefilledField[], MyInfoHashingError | DatabaseError> {
const prefilledFields = currFormFields.map((field) => {
Expand Down
9 changes: 8 additions & 1 deletion src/app/modules/myinfo/myinfo.util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
FormAuthNoEsrvcIdError,
FormNotFoundError,
} from '../form/form.errors'
import { SGID_MYINFO_LOGIN_COOKIE_NAME } from '../sgid/sgid.constants'
import { ProcessedFieldResponse } from '../submission/submission.types'

import { MYINFO_LOGIN_COOKIE_NAME } from './myinfo.constants'
Expand Down Expand Up @@ -299,8 +300,14 @@ export const isMyInfoAuthCodeCookie = (
*/
export const extractMyInfoLoginJwt = (
cookies: Record<string, unknown>,
authType: FormAuthType.MyInfo | FormAuthType.SGID_MyInfo,
): Result<string, MyInfoMissingLoginCookieError> => {
const jwt = cookies[MYINFO_LOGIN_COOKIE_NAME]
const jwt =
cookies[
authType === FormAuthType.MyInfo
? MYINFO_LOGIN_COOKIE_NAME
: SGID_MYINFO_LOGIN_COOKIE_NAME
]
if (typeof jwt === 'string' && !!jwt) {
return ok(jwt)
}
Expand Down
20 changes: 11 additions & 9 deletions src/app/modules/sgid/__tests__/sgid.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe('sgid.controller', () => {
okAsync(MOCK_TOKEN_RESULT),
)
SgidService.retrieveUserInfo.mockReturnValue(okAsync(MOCK_USER_INFO))
SgidService.createJwt.mockReturnValue(
SgidService.createSgidSingpassJwt.mockReturnValue(
ok({ jwt: MOCK_JWT, maxAge: MOCK_COOKIE_AGE }),
)
SgidService.getCookieSettings.mockReturnValue(MOCK_COOKIE_SETTINGS)
Expand All @@ -77,7 +77,7 @@ describe('sgid.controller', () => {
expect(FormService.retrieveFullFormById).not.toHaveBeenCalled()
expect(SgidService.retrieveAccessToken).not.toHaveBeenCalled()
expect(SgidService.retrieveUserInfo).not.toHaveBeenCalled()
expect(SgidService.createJwt).not.toHaveBeenCalled()
expect(SgidService.createSgidSingpassJwt).not.toHaveBeenCalled()
expect(SgidService.getCookieSettings).not.toHaveBeenCalled()
})

Expand All @@ -93,7 +93,7 @@ describe('sgid.controller', () => {
expect(MOCK_RESPONSE.redirect).not.toHaveBeenCalled()
expect(SgidService.retrieveAccessToken).not.toHaveBeenCalled()
expect(SgidService.retrieveUserInfo).not.toHaveBeenCalled()
expect(SgidService.createJwt).not.toHaveBeenCalled()
expect(SgidService.createSgidSingpassJwt).not.toHaveBeenCalled()
expect(SgidService.getCookieSettings).not.toHaveBeenCalled()
})

Expand All @@ -109,7 +109,7 @@ describe('sgid.controller', () => {
expect(MOCK_RESPONSE.redirect).toHaveBeenCalledWith(MOCK_DESTINATION)
expect(SgidService.retrieveAccessToken).not.toHaveBeenCalled()
expect(SgidService.retrieveUserInfo).not.toHaveBeenCalled()
expect(SgidService.createJwt).not.toHaveBeenCalled()
expect(SgidService.createSgidSingpassJwt).not.toHaveBeenCalled()
expect(SgidService.getCookieSettings).not.toHaveBeenCalled()
})

Expand All @@ -126,7 +126,7 @@ describe('sgid.controller', () => {
MOCK_AUTH_CODE,
)
expect(SgidService.retrieveUserInfo).not.toHaveBeenCalled()
expect(SgidService.createJwt).not.toHaveBeenCalled()
expect(SgidService.createSgidSingpassJwt).not.toHaveBeenCalled()
expect(SgidService.getCookieSettings).not.toHaveBeenCalled()
})

Expand All @@ -145,12 +145,14 @@ describe('sgid.controller', () => {
)
expect(MOCK_RESPONSE.cookie).toHaveBeenCalledWith('isLoginError', true)
expect(MOCK_RESPONSE.redirect).toHaveBeenCalledWith(MOCK_DESTINATION)
expect(SgidService.createJwt).not.toHaveBeenCalled()
expect(SgidService.createSgidSingpassJwt).not.toHaveBeenCalled()
expect(SgidService.getCookieSettings).not.toHaveBeenCalled()
})

it('should set isLoginError cookie and redirect when createJWT errors', async () => {
SgidService.createJwt.mockReturnValue(err(new ApplicationError()))
SgidService.createSgidSingpassJwt.mockReturnValue(
err(new ApplicationError()),
)
await SgidController.handleLogin(MOCK_LOGIN_REQ, MOCK_RESPONSE, jest.fn())
expect(SgidService.parseState).toHaveBeenCalledWith(MOCK_STATE)
expect(FormService.retrieveFullFormById).toHaveBeenCalledWith(MOCK_TARGET)
Expand All @@ -160,7 +162,7 @@ describe('sgid.controller', () => {
expect(SgidService.retrieveUserInfo).toHaveBeenCalledWith(
MOCK_TOKEN_RESULT,
)
expect(SgidService.createJwt).toHaveBeenCalledWith(
expect(SgidService.createSgidSingpassJwt).toHaveBeenCalledWith(
MOCK_USER_INFO.data,
MOCK_REMEMBER_ME,
)
Expand All @@ -178,7 +180,7 @@ describe('sgid.controller', () => {
expect(SgidService.retrieveUserInfo).toHaveBeenCalledWith(
MOCK_TOKEN_RESULT,
)
expect(SgidService.createJwt).toHaveBeenCalledWith(
expect(SgidService.createSgidSingpassJwt).toHaveBeenCalledWith(
MOCK_USER_INFO.data,
MOCK_REMEMBER_ME,
)
Expand Down
Loading