Skip to content

Commit

Permalink
Merge pull request opengovsg#562 from justin-tay/master
Browse files Browse the repository at this point in the history 
feat: use spec compliant error responses for ndi oidc
  • Loading branch information
@LoneRifle
LoneRifle authored Jul 12, 2023
2 parents eedaf7b + f07f89e commit 555316b
Showing 1 changed file with 76 additions and 35 deletions.
111 changes: 76 additions & 35 deletions lib/express/oidc/v2-ndi.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,22 +49,40 @@ function config(app, { showLoginPage }) {
} = req.query

if (scope !== 'openid') {
return res.status(400).send(`Unknown scope ${scope}`)
return res.status(400).send({
error: 'invalid_scope',
error_description: `Unknown scope ${scope}`,
})
}
if (response_type !== 'code') {
return res.status(400).send(`Unknown response_type ${response_type}`)
return res.status(400).send({
error: 'unsupported_response_type',
error_description: `Unknown response_type ${response_type}`,
})
}
if (!client_id) {
return res.status(400).send('Missing client_id')
return res.status(400).send({
error: 'invalid_request',
error_description: 'Missing client_id',
})
}
if (!redirectURI) {
return res.status(400).send('Missing redirect_uri')
return res.status(400).send({
error: 'invalid_request',
error_description: 'Missing redirect_uri',
})
}
if (!nonce) {
return res.status(400).send('Missing nonce')
return res.status(400).send({
error: 'invalid_request',
error_description: 'Missing nonce',
})
}
if (!state) {
return res.status(400).send('Missing state')
return res.status(400).send({
error: 'invalid_request',
error_description: 'Missing state',
})
}

// Identical to OIDC v1
Expand Down Expand Up @@ -128,27 +146,43 @@ function config(app, { showLoginPage }) {

// Only SP requires client_id
if (idp === 'singPass' && !client_id) {
return res.status(400).send('Missing client_id')
return res.status(400).send({
error: 'invalid_request',
error_description: 'Missing client_id',
})
}
if (!redirectURI) {
return res.status(400).send('Missing redirect_uri')
return res.status(400).send({
error: 'invalid_request',
error_description: 'Missing redirect_uri',
})
}
if (grant_type !== 'authorization_code') {
return res.status(400).send(`Unknown grant_type ${grant_type}`)
return res.status(400).send({
error: 'unsupported_grant_type',
error_description: `Unknown grant_type ${grant_type}`,
})
}
if (!authCode) {
return res.status(400).send('Missing code')
return res.status(400).send({
error: 'invalid_request',
error_description: 'Missing code',
})
}
if (
client_assertion_type !==
'urn:ietf:params:oauth:client-assertion-type:jwt-bearer'
) {
return res
.status(400)
.send(`Unknown client_assertion_type ${client_assertion_type}`)
return res.status(400).send({
error: 'invalid_request',
error_description: `Unknown client_assertion_type ${client_assertion_type}`,
})
}
if (!clientAssertion) {
return res.status(400).send('Missing client_assertion')
return res.status(400).send({
error: 'invalid_request',
error_description: 'Missing client_assertion',
})
}

// Step 0: Get the RP keyset
Expand All @@ -165,11 +199,10 @@ function config(app, { showLoginPage }) {
method: 'GET',
}).then((response) => response.text())
} catch (e) {
return res
.status(400)
.send(
`Failed to fetch RP JWKS from specified endpoint: ${e.message}`,
)
return res.status(400).send({
error: 'invalid_client',
error_description: `Failed to fetch RP JWKS from specified endpoint: ${e.message}`,
})
}
} else {
// If the endpoint is not defined, default to the sample keyset we provided.
Expand All @@ -180,7 +213,10 @@ function config(app, { showLoginPage }) {
try {
rpKeysetJson = JSON.parse(rpKeysetString)
} catch (e) {
return res.status(400).send(`Unable to parse RP keyset: ${e.message}`)
return res.status(400).send({
error: 'invalid_client',
error_description: `Unable to parse RP keyset: ${e.message}`,
})
}

const rpKeyset = await jose.JWK.asKeyStore(rpKeysetJson)
Expand All @@ -192,33 +228,37 @@ function config(app, { showLoginPage }) {
rpKeyset,
).verify(clientAssertion)
} catch (e) {
return res
.status(400)
.send(`Unable to verify client_assertion: ${e.message}`)
return res.status(400).send({
error: 'invalid_client',
error_description: `Unable to verify client_assertion: ${e.message}`,
})
}

let clientAssertionClaims
try {
clientAssertionClaims = JSON.parse(clientAssertionVerified.payload)
} catch (e) {
return res
.status(400)
.send(`Unable to parse client_assertion: ${e.message}`)
return res.status(400).send({
error: 'invalid_client',
error_description: `Unable to parse client_assertion: ${e.message}`,
})
}

if (idp === 'singPass') {
if (clientAssertionClaims['sub'] !== client_id) {
return res
.status(400)
.send('Incorrect sub in client_assertion claims')
return res.status(400).send({
error: 'invalid_client',
error_description: 'Incorrect sub in client_assertion claims',
})
}
} else {
// Since client_id is not given for corpPass, sub claim is required in
// order to get aud for id_token.
if (!clientAssertionClaims['sub']) {
return res
.status(400)
.send('Missing sub in client_assertion claims')
return res.status(400).send({
error: 'invalid_client',
error_description: 'Missing sub in client_assertion claims',
})
}
}

Expand All @@ -228,9 +268,10 @@ function config(app, { showLoginPage }) {
)}/${idp.toLowerCase()}/v2`

if (clientAssertionClaims['aud'] !== iss) {
return res
.status(400)
.send('Incorrect aud in client_assertion claims')
return res.status(400).send({
error: 'invalid_client',
error_description: 'Incorrect aud in client_assertion claims',
})
}

// Step 1: Obtain profile for which the auth code requested data for
Expand Down

0 comments on commit 555316b

Please sign in to comment.