Skip to content

Commit

Permalink
feat/sgid-pocdex-login (#2228)
Browse files Browse the repository at this point in the history
* feat/sgid-pocdex-login

* fix: isWhitelistedEmail bug

---------

Co-authored-by: KishenKumarrrrr <kishen@open.gov.sg>
  • Loading branch information
KishenKumarrrrr and KishenKumarrrrr committed Sep 27, 2023
1 parent 5fa6ca2 commit 6fb0876
Show file tree
Hide file tree
Showing 22 changed files with 608 additions and 153 deletions.
6 changes: 3 additions & 3 deletions .secrets.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@
"filename": "backend/src/core/middlewares/auth.middleware.ts",
"hashed_secret": "159500287c06851df741128ec4b073ea394414b6",
"is_verified": false,
"line_number": 21
"line_number": 23
}
],
"backend/src/core/services/auth.service.ts": [
Expand All @@ -209,7 +209,7 @@
"filename": "backend/src/core/services/auth.service.ts",
"hashed_secret": "f114703480996b273d7e57cbd195b4ab1e70a21b",
"is_verified": false,
"line_number": 63
"line_number": 65
}
],
"backend/src/email/services/tests/email-template.service.test.ts": [
Expand Down Expand Up @@ -365,5 +365,5 @@
}
]
},
"generated_at": "2023-09-11T09:38:16Z"
"generated_at": "2023-09-25T09:48:43Z"
}
8 changes: 4 additions & 4 deletions backend/package-lock.json

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

2 changes: 1 addition & 1 deletion backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"@aws-sdk/client-sns": "3.370.0",
"@aws-sdk/rds-signer": "3.370.0",
"@aws-sdk/s3-request-presigner": "3.370.0",
"@opengovsg/sgid-client": "^2.0.0",
"@opengovsg/sgid-client": "^2.1.0",
"@sentry/node": "5.30.0",
"async-retry": "1.3.3",
"axios": "0.21.4",
Expand Down
45 changes: 42 additions & 3 deletions backend/src/core/middlewares/auth.middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { AuthService, experimentService } from '@core/services'
import { getRequestIp } from '@core/utils/request'
import { DEFAULT_TX_EMAIL_RATE_LIMIT } from '@core/models'
import { ApiAuthenticationError } from '@core/errors/rest-api.errors'
import { SgidPublicOfficerEmployment } from '@core/types'

export interface AuthMiddleware {
getOtp: Handler
Expand All @@ -14,6 +15,7 @@ export interface AuthMiddleware {
logout: Handler
getSgidUrl: Handler
verifySgidResponse: Handler
selectSgidProfile: Handler
}

export enum AuthType {
Expand Down Expand Up @@ -240,7 +242,7 @@ export const InitAuthMiddleware = (authService: AuthService) => {
}

/**
* Verifies that the sgID response is valid
* Verifies that the sgID response is valid and returns the user profiles to choose from
* @param req
* @param res
*/
Expand All @@ -257,10 +259,46 @@ export const InitAuthMiddleware = (authService: AuthService) => {
}
const sgidUserInfo = await authService.verifySgidCode(req, code)
if (!sgidUserInfo.authenticated) {
logger.error({ message: sgidUserInfo.reason, ...logMeta })
return res.status(401).json({ message: sgidUserInfo.reason })
}
const userEmail = authService.getSgidUserEmail(sgidUserInfo.data)
const user = await authService.findOrCreateUser(userEmail)
const userProfiles = await authService.getSgidUserProfiles(
sgidUserInfo.data
)
// Set user profiles in the session object so we can verify the profile selected by the user
req.session.sgid = {
...req.session.sgid,
profiles: [...userProfiles],
}
return res.status(200).json({ userProfiles })
} catch (e) {
const message = (e as Error).message
logger.error({ message, ...logMeta })
return res.status(500).json({ message })
}
}

const selectSgidProfile = async (
req: Request,
res: Response
): Promise<Response> => {
const { workEmail } = req.body
const logMeta = { action: 'selectSgidProfile' }
try {
if (!req.session) {
logger.error({ message: 'Session object not found!', ...logMeta })
return res.sendStatus(401)
}
if (
!req.session.sgid?.profiles ||
!req.session.sgid.profiles.some(
(p: SgidPublicOfficerEmployment) => p.workEmail === workEmail
)
) {
logger.error({ message: 'Selected profile is not valid', ...logMeta })
return res.sendStatus(401)
}
const user = await authService.findOrCreateUser(workEmail)
req.session.user = {
id: user.id,
createdAt: user.createdAt,
Expand All @@ -283,5 +321,6 @@ export const InitAuthMiddleware = (authService: AuthService) => {
logout,
getSgidUrl,
verifySgidResponse,
selectSgidProfile,
}
}
45 changes: 45 additions & 0 deletions backend/src/core/routes/auth.routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ export const InitAuthRoutes = (authMiddleware: AuthMiddleware): Router => {
}),
}

const selectSgidProfileValidator = {
[Segments.BODY]: Joi.object({
workEmail: Joi.string().required(),
}),
}

// actual routes here

/**
Expand Down Expand Up @@ -164,6 +170,45 @@ export const InitAuthRoutes = (authMiddleware: AuthMiddleware): Router => {
authMiddleware.verifySgidResponse
)

/**
* paths:
* /auth/login/sgid/profile:
* post:
* summary: Select sgid profile to login with
* tags:
* - Authentication
* requestBody:
* required: true
* content:
* application/json:
* schema:
* type: object
* properties:
* workEmail:
* type: string
* required:
* - workEmail
*
* responses:
* "200":
* content:
* application/json:
* schema:
* type: object
* "401":
* content:
* application/json:
* schema:
* type: object
* "500":
* description: Internal Server Error
*/
router.post(
'/login/sgid/profile',
celebrate(selectSgidProfileValidator),
authMiddleware.selectSgidProfile
)

/**
* paths:
* /auth/userinfo:
Expand Down
123 changes: 102 additions & 21 deletions backend/src/core/services/auth.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Request } from 'express'
import config from '@core/config'
import { loggerWithLabel } from '@core/logger'
import { User } from '@core/models'
import { isValidDomain, validateDomain } from '@core/utils/validate-domain'
import { validateDomain } from '@core/utils/validate-domain'
import { ApiKeyService, MailService, RedisService } from '@core/services'
import { HashedOtp, VerifyOtpInput } from '@core/interfaces'
import { Transaction } from 'sequelize/types'
Expand All @@ -13,6 +13,7 @@ import {
UserInfoReturn,
generatePkcePair,
} from '@opengovsg/sgid-client'
import { SgidPublicOfficerEmployment } from '@core/types'

export interface AuthService {
canSendOtp(email: string): Promise<void>
Expand All @@ -30,7 +31,9 @@ export interface AuthService {
| { authenticated: true; data: UserInfoReturn }
| { authenticated: false; reason: string }
>
getSgidUserEmail(userInfo: UserInfoReturn): string
getSgidUserProfiles(
userInfo: UserInfoReturn
): Promise<SgidPublicOfficerEmployment[]>
}

export const InitAuthService = (redisService: RedisService): AuthService => {
Expand All @@ -47,7 +50,6 @@ export const InitAuthService = (redisService: RedisService): AuthService => {
clientSecret: SGID_CLIENT_SECRET,
privateKey: SGID_PRIVATE_KEY,
redirectUri: SGID_REDIRECT_URI,
validDomains: SGID_VALID_DOMAINS,
} = config.get('sgid')

const sgidClient = new SgidClient({
Expand All @@ -57,9 +59,9 @@ export const InitAuthService = (redisService: RedisService): AuthService => {
redirectUri: SGID_REDIRECT_URI,
})

const SGID_OGP_WORK_EMAIL_SCOPE = 'ogpofficerinfo.work_email'
const sgidDomainsToWhitelist =
SGID_VALID_DOMAINS.split(';').filter(isValidDomain)
const SGID_PUBLIC_OFFICER_EMPLOYMENT_SCOPE =
'pocdex.public_officer_employments'
const SGID_FIELD_EMPTY = 'NA'
const otpCharset = '234567ABCDEFGHIJKLMNOPQRSTUVWXYZ'
/**
* Generate a six digit otp
Expand Down Expand Up @@ -343,7 +345,7 @@ export const InitAuthService = (redisService: RedisService): AuthService => {
const { codeChallenge, codeVerifier } = generatePkcePair()

const { url, nonce } = sgidClient.authorizationUrl({
scope: ['openid', SGID_OGP_WORK_EMAIL_SCOPE].join(' '),
scope: ['openid', SGID_PUBLIC_OFFICER_EMPLOYMENT_SCOPE].join(' '),
codeChallenge,
})

Expand Down Expand Up @@ -405,22 +407,101 @@ export const InitAuthService = (redisService: RedisService): AuthService => {
}

/**
* Helper method to retrieve the user's email from their singpass info.
* User's email must be from the list of whitelisted domains.
* Helper method to retrieve the user's valid profiles from their singpass info.
* @param userInfo
*/
const getSgidUserEmail = (userInfo: UserInfoReturn): string => {
const email = userInfo.data[SGID_OGP_WORK_EMAIL_SCOPE]
if (!email) {
throw new Error('No email found')
}
const isEmailDomainValid = sgidDomainsToWhitelist.some((domain: string) =>
email.endsWith(domain)
)
if (!isEmailDomainValid) {
throw new Error('Invalid email')
const getSgidUserProfiles = async (
userInfo: UserInfoReturn
): Promise<SgidPublicOfficerEmployment[]> => {
const profiles = JSON.parse(
userInfo.data[SGID_PUBLIC_OFFICER_EMPLOYMENT_SCOPE]
) as SgidPublicOfficerEmployment[]
const validProfiles = await validateSgidUserProfiles(profiles)
const cleanedProfiles = cleanSgidUserProfiles(validProfiles)
return cleanedProfiles
}

/**
* Helper method to validate the user's profiles returned by SGID.
* A profile is valid only if the user's work email exists and is whitelisted by Postman
* @param userProfiles
*/
const validateSgidUserProfiles = async (
userProfiles: SgidPublicOfficerEmployment[]
): Promise<SgidPublicOfficerEmployment[]> => {
const logMeta = { action: 'validateSgidUserProfiles' }
// Only the value of workEmail is important for access to Postman.
const validProfiles = []
for (const profile of userProfiles) {
// We want to log the absence of workEmail to measure the data completeness from SGID.
if (profile.workEmail === SGID_FIELD_EMPTY) {
logger.warn({
message: 'Work email is missing from SGID data',
...logMeta,
profile,
})
continue
}
if (!(await isWhitelistedEmail(profile.workEmail))) {
logger.warn({
message: 'Work email is not a whitelisted email',
...logMeta,
profile,
})
continue
}
validProfiles.push(profile)
}
return email.toLowerCase()
return validProfiles
}

/**
* Helper method to clean the user's profiles returned by SGID
* @param userProfiles
*/
const cleanSgidUserProfiles = (
userProfiles: SgidPublicOfficerEmployment[]
): SgidPublicOfficerEmployment[] => {
const logMeta = { action: 'cleanSgidUserProfiles' }
const cleanedProfiles = userProfiles.map((profile) => {
// DB only accepts lowercase emails
profile.workEmail = profile.workEmail.toLowerCase()
// If SGID does not have the field, we want to log the missing value and return an empty string
if (profile.agencyName === SGID_FIELD_EMPTY) {
profile.agencyName = ''
logger.warn({
message: 'Agency name is missing from SGID data',
...logMeta,
profile,
})
}
if (profile.departmentName === SGID_FIELD_EMPTY) {
profile.departmentName = ''
logger.warn({
message: 'Department name is missing from SGID data',
...logMeta,
profile,
})
}
if (profile.employmentTitle === SGID_FIELD_EMPTY) {
profile.employmentTitle = ''
logger.warn({
message: 'Employment title is missing from SGID data',
...logMeta,
profile,
})
}
if (profile.employmentType === SGID_FIELD_EMPTY) {
profile.employmentType = ''
logger.warn({
message: 'Employment type is missing from SGID data',
...logMeta,
profile,
})
}
return profile
})
return cleanedProfiles
}

return {
Expand All @@ -433,6 +514,6 @@ export const InitAuthService = (redisService: RedisService): AuthService => {
getUserForApiKey,
getSgidUrl,
verifySgidCode,
getSgidUserEmail,
getSgidUserProfiles,
}
}
7 changes: 7 additions & 0 deletions backend/src/core/types/auth.types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export type SgidPublicOfficerEmployment = {
agencyName: string
departmentName: string
employmentTitle: string
employmentType: string
workEmail: string
}
1 change: 1 addition & 0 deletions backend/src/core/types/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export * from './parse-csv.types'
export * from './auth.types'
3 changes: 1 addition & 2 deletions frontend/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ const App = () => {
return (
<Routes>
<Route path="/" element={<Landing />}></Route>
<Route path="/sgid-login/callback" element={<Callback />}></Route>
<Route path="/sgid-login" element={<Login />}></Route>
<Route path="/login/callback" element={<Callback />}></Route>
<Route path="/login" element={<Login />}></Route>
<Route path="/test/*" element={<TestUtils />}></Route>
<Route path="/p/:version/:id" element={<ProtectedPage />}></Route>
Expand Down
Loading

0 comments on commit 6fb0876

Please sign in to comment.