Error log verbosity enhanced #111
Conversation
There was a problem hiding this comment.
You are accessing headers by index using the original casing, so the case matters here. See https://nginx.org/en/docs/njs/reference.html#r_headers_in
openid_connect.js
Outdated
| @@ -56,10 +56,10 @@ async function codeExchange(r) { | |||
| // Check authorization code presence | |||
| if (!r.variables.arg_code || r.variables.arg_code.length == 0) { | |||
| if (r.variables.arg_error) { | |||
| r.error("OIDC error receiving authorization code: " + | |||
| r.error("OIDC error receiving authorization code for " + r.headersIn['host'] + r.uri + ": " + | |||
There was a problem hiding this comment.
I pointed this out in the main comment:
You are accessing headers by index using the original casing, so the case matters here. See https://nginx.org/en/docs/njs/reference.html#r_headers_in
There was a problem hiding this comment.
For template literals, it’s better to use the headersIn.foo approach.
openid_connect.js
Outdated
| @@ -131,21 +131,21 @@ function validateIdTokenClaims(r, claims) { | |||
| const missingClaims = requiredClaims.filter((claim) => !claims[claim]); | |||
|
|
|||
| if (missingClaims.length > 0) { | |||
| r.error(`OIDC ID Token validation error: missing claim(s) ${missingClaims.join(' ')}`); | |||
| r.error(`OIDC ID Token validation error for ` + r.headersIn['host'] + r.uri + `: missing claim(s) ${missingClaims.join(' ')}`); | |||
There was a problem hiding this comment.
In this and similar cases, it’s better to follow a consistent stylistic approach
| return false; | ||
| } | ||
|
|
||
| // Check 'iat' validity | ||
| const iat = Math.floor(Number(claims.iat)); | ||
| if (String(iat) !== claims.iat || iat < 1) { | ||
| r.error("OIDC ID Token validation error: iat claim is not a valid number"); | ||
| r.error(`OIDC ID Token validation error for ` + r.headersIn['Host'] + r.uri + `: iat claim is not a valid number`); |
There was a problem hiding this comment.
When I talk about style, this spot is a good example. Here you switched to template literals, but in #L168 you are using string concatenation under the same circumstances. Was this done for a reason I didn’t notice?
Added HTTP Host and URI to all error logs