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: upgrade sgid sdk #6584

Merged
merged 25 commits into from
Aug 18, 2023
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
f213dc9
feat: upgrade to sgID v2 SDK
kschiew Aug 1, 2023
9a00344
chore: update docker setup
kschiew Aug 1, 2023
9bf1f58
chore: update package.json
kschiew Aug 1, 2023
db68a7f
Merge branch 'develop' into feat/upgrade_sgid_sdk
kschiew Aug 1, 2023
34f1dc0
fix: update bson version for tests
kschiew Aug 7, 2023
7cb5e1b
chore: update package-lock.json
kschiew Aug 7, 2023
f55b46c
Merge branch 'develop' into feat/upgrade_sgid_sdk
kschiew Aug 8, 2023
fb0cf46
chore: commit package.json files with no-verify
kschiew Aug 8, 2023
d47240d
chore: remove console logs
kschiew Aug 8, 2023
d2d4237
chore: update package-lock
kschiew Aug 8, 2023
d4b8860
chore: update package-lock
kschiew Aug 8, 2023
6cd2efa
chore: change node version
kschiew Aug 8, 2023
904ab2e
chore: change npm version
kschiew Aug 8, 2023
e85045f
chore: pin bson version
kschiew Aug 8, 2023
6d1c606
chore: actually pin bson versoin
kschiew Aug 8, 2023
3aa4482
fix: fix helmet type import
kschiew Aug 10, 2023
97f4327
Merge branch 'develop' into feat/upgrade_sgid_sdk
kschiew Aug 10, 2023
d762726
Merge branch 'develop' into feat/upgrade_sgid_sdk
justynoh Aug 16, 2023
5d7a754
chore: update deps
justynoh Aug 16, 2023
ce04f02
chore: update mockpass dev dependency version
justynoh Aug 16, 2023
7ca931a
chore: revert out-of-scope changes
justynoh Aug 16, 2023
ad06fe7
chore: refactors
justynoh Aug 18, 2023
71be416
chore: remove unneeded packages
justynoh Aug 18, 2023
1077177
test: update sgid service tests with generatePkcePair mocks
justynoh Aug 18, 2023
c361601
test: fix tests again
justynoh Aug 18, 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
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
Loading