Skip to content

Commit

Permalink
feat: upgrade sgid sdk (#6584)
Browse files Browse the repository at this point in the history
* feat: upgrade to sgID v2 SDK

* chore: update docker setup

* chore: update package.json

* fix: update bson version for tests

* chore: update package-lock.json

* chore: commit package.json files with no-verify

* chore: remove console logs

* chore: update package-lock

* chore: update package-lock

* chore: change node version

* chore: change npm version

* chore: pin bson version

* chore: actually pin bson versoin

* fix: fix helmet type import

* chore: update deps

* chore: update mockpass dev dependency version

* chore: revert out-of-scope changes

* chore: refactors

* chore: remove unneeded packages

* test: update sgid service tests with generatePkcePair mocks

* test: fix tests again

---------

Co-authored-by: Justyn Oh <justynoh@gmail.com>
  • Loading branch information
kschiew and justynoh committed Aug 18, 2023
1 parent b4afaca commit 5c79ac8
Show file tree
Hide file tree
Showing 16 changed files with 440 additions and 181 deletions.
2 changes: 1 addition & 1 deletion __tests__/setup/.test-env
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ MYINFO_CLIENT_SECRET=mockClientSecret
MYINFO_JWT_SECRET=mockJwtSecret
SINGPASS_ESRVC_ID=Test-eServiceId-Sp # Needed for MyInfo

SGID_HOSTNAME=http://localhost:5156/sgid
SGID_HOSTNAME=http://localhost:5156
SGID_CLIENT_ID=sgidclientid
SGID_CLIENT_SECRET=sgidclientsecret
SGID_FORM_LOGIN_REDIRECT_URI=http://localhost:5000/sgid/login
Expand Down
4 changes: 4 additions & 0 deletions __tests__/unit/backend/helpers/jest-express.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@ const mockRequest = <P extends Record<string, string>, B, Q = any>({
session,
query,
secure,
cookies,
others = {},
}: {
params?: P
body?: B
session?: Record<string, unknown>
query?: Q
secure?: boolean
cookies?: Record<string, string>
others?: Partial<Omit<Record<keyof Request, unknown>, 'query'>>
} = {}): Request<P, unknown, B, Q & Request['query']> => {
return {
Expand All @@ -21,6 +23,7 @@ const mockRequest = <P extends Record<string, string>, B, Q = any>({
session: session ?? {},
query: query ?? {},
secure: secure ?? true,
cookies: cookies ?? {},
get(name: string) {
if (name === 'cf-connecting-ip') return 'MOCK_IP'
return undefined
Expand All @@ -47,6 +50,7 @@ const mockResponse = (
redirect: jest.fn(),
cookie: jest.fn(),
set: jest.fn(),
clearCookie: jest.fn(),
...extraArgs,
}
return mockRes as Response
Expand Down
4 changes: 2 additions & 2 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ services:
- MYINFO_CLIENT_ID=mockClientId
- MYINFO_CLIENT_SECRET=mockClientSecret
- MYINFO_JWT_SECRET=mockJwtSecret
- SGID_HOSTNAME=http://localhost:5156/sgid
- SGID_HOSTNAME=http://localhost:5156
- SGID_CLIENT_ID=sgidclientid
- SGID_CLIENT_SECRET=sgidclientsecret
- SGID_ADMIN_LOGIN_REDIRECT_URI=http://localhost:5001/api/v3/auth/sgid/login
Expand Down Expand Up @@ -121,7 +121,7 @@ services:
- API_KEY_VERSION=v1

mockpass:
build: https://github.com/opengovsg/mockpass.git#v3.1.3
build: https://github.com/opengovsg/mockpass.git#v4.0.4
depends_on:
- backend
environment:
Expand Down
90 changes: 35 additions & 55 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
"@joi/date": "^2.1.0",
"@opengovsg/formsg-sdk": "^0.10.0",
"@opengovsg/myinfo-gov-client": "^4.1.1",
"@opengovsg/sgid-client": "^1.0.4",
"@opengovsg/sgid-client": "^2.0.0",
"@sentry/browser": "^7.51.2",
"@sentry/integrations": "^6.19.7",
"@stablelib/base64": "^1.0.1",
Expand Down Expand Up @@ -161,7 +161,7 @@
"@babel/core": "^7.20.7",
"@babel/plugin-transform-runtime": "^7.21.4",
"@babel/preset-env": "^7.22.5",
"@opengovsg/mockpass": "^3.1.0",
"@opengovsg/mockpass": "^4.0.4",
"@playwright/test": "^1.34.3",
"@types/bcrypt": "^5.0.0",
"@types/bluebird": "^3.5.38",
Expand Down
34 changes: 26 additions & 8 deletions src/app/modules/auth/sgid/auth-sgid.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { ErrorDto, GetSgidAuthUrlResponseDto } from 'shared/types'
import { createLoggerWithLabel } from '../../../config/logger'
import { createReqMeta } from '../../../utils/request'
import { ControllerHandler } from '../../core/core.types'
import { SGID_CODE_VERIFIER_COOKIE_NAME } from '../../sgid/sgid.constants'
import * as UserService from '../../user/user.service'
import * as AuthService from '../auth.service'
import { SessionUser } from '../auth.types'
Expand All @@ -23,17 +24,25 @@ export const generateAuthUrl: ControllerHandler<
}

return AuthSgidService.createRedirectUrl()
.map((redirectUrl) => res.status(StatusCodes.OK).send({ redirectUrl }))
.map(({ redirectUrl, codeVerifier }) =>
res
.status(StatusCodes.OK)
.cookie(SGID_CODE_VERIFIER_COOKIE_NAME, codeVerifier)
.send({ redirectUrl }),
)
.mapErr((error) => {
logger.error({
message: 'Failed to generate SGID auth url',
meta: logMeta,
error,
})
return res.sendStatus(StatusCodes.INTERNAL_SERVER_ERROR).json({
message:
'Generating SGID authentication url failed. Please try again later.',
})
return res
.sendStatus(StatusCodes.INTERNAL_SERVER_ERROR)
.json({
message:
'Generating SGID authentication url failed. Please try again later.',
})
.clearCookie(SGID_CODE_VERIFIER_COOKIE_NAME)
})
}

Expand All @@ -44,6 +53,8 @@ export const handleLogin: ControllerHandler<
{ code: string; state: string }
> = async (req, res) => {
const { code, state } = req.query
const codeVerifier = req.cookies[SGID_CODE_VERIFIER_COOKIE_NAME]
res.clearCookie(SGID_CODE_VERIFIER_COOKIE_NAME)

const logMeta = {
action: 'handleLogin',
Expand All @@ -63,11 +74,18 @@ export const handleLogin: ControllerHandler<
meta: logMeta,
})

status = StatusCodes.BAD_REQUEST
} else if (!codeVerifier) {
logger.error({
message: 'Error logging in via sgID: code verifier cookie is empty',
meta: logMeta,
})

status = StatusCodes.BAD_REQUEST
} else {
await AuthSgidService.retrieveAccessToken(code)
.andThen(({ accessToken }) =>
AuthSgidService.retrieveUserInfo(accessToken),
await AuthSgidService.retrieveAccessToken(code, codeVerifier)
.andThen(({ accessToken, sub }) =>
AuthSgidService.retrieveUserInfo(accessToken, sub),
)
.andThen((email) =>
AuthService.validateEmailDomain(email).andThen((agency) =>
Expand Down
Loading

0 comments on commit 5c79ac8

Please sign in to comment.