Skip to content

Commit

Permalink
refactor: bad sgid session returns 400 instead of 500 (#2249)
Browse files Browse the repository at this point in the history
Co-authored-by: KishenKumarrrrr <kishen@open.gov.sg>
  • Loading branch information
KishenKumarrrrr and KishenKumarrrrr committed Feb 23, 2024
1 parent 1c9bcee commit 12cc5c6
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 7 deletions.
10 changes: 8 additions & 2 deletions backend/src/core/middlewares/auth.middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -253,9 +253,15 @@ export const InitAuthMiddleware = (authService: AuthService) => {
const { code } = req.body
const logMeta = { code, action: 'verifySgidCode' }
try {
if (!req.session) {
/*
Since Feb 8 2024, this endpoint is called twice when users attempt to log in via SGID on GSIB machines.
This is most likely due to *.postman.gov.sg being whitelisted on SGProxy but the SGID url is not.
The additional API call is made without req.session.sgid set and thus we add a check here and return a HTTP 400
if this is the case.
*/
if (!req.session || !req.session.sgid) {
logger.error({ message: 'Session object not found!', ...logMeta })
return res.sendStatus(401)
return res.sendStatus(400)
}
const sgidUserInfo = await authService.verifySgidCode(req, code)
if (!sgidUserInfo.authenticated) {
Expand Down
8 changes: 3 additions & 5 deletions backend/src/core/services/auth.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,8 @@ export const InitAuthService = (redisService: RedisService): AuthService => {
}

/**
* Checks the user's sgID code and returns their singpass info if valid
* Checks the user's sgID code and returns their singpass info if valid.
* This function assumes that req.session.sgid has already been validated by the calling function.
* @param req
* @param code
*/
Expand All @@ -373,10 +374,7 @@ export const InitAuthService = (redisService: RedisService): AuthService => {
| { authenticated: true; data: UserInfoReturn }
| { authenticated: false; reason: string }
> => {
if (!req.session || !req.session.sgid) {
throw new Error('Unable to find user session')
}
const { codeVerifier, nonce } = req.session.sgid
const { codeVerifier, nonce } = req.session!.sgid

Check warning on line 377 in backend/src/core/services/auth.service.ts

View workflow job for this annotation

GitHub Actions / test-backend

Forbidden non-null assertion

Check warning on line 377 in backend/src/core/services/auth.service.ts

View workflow job for this annotation

GitHub Actions / Deploy backend to AWS Elastic Beanstalk / lint-test

Forbidden non-null assertion

if (typeof codeVerifier !== 'string' || typeof nonce !== 'string') {
throw new Error('Invalid parameter types')
Expand Down

0 comments on commit 12cc5c6

Please sign in to comment.