Skip to content

Error log verbosity enhanced #111

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+24 −24
Open

Error log verbosity enhanced #111

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f87dc0c
Added HTTP Host and URI to error logs
fabriziofiorucci May 29, 2025
986832c
Merge branch 'nginxinc:main' into ff20250529
fabriziofiorucci Aug 15, 2025
a561dd5
Fixed comments
fabriziofiorucci Aug 15, 2025
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
48 changes: 24 additions & 24 deletions openid_connect.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 + ": " +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

r.variables.arg_error_description);
} else {
r.error("OIDC expected authorization code but received: " + r.uri);
r.error("OIDC expected authorization code for " + r.headersIn['host'] + " but received: " + r.uri);
}
r.return(502);
return;
Expand Down Expand Up @@ -95,15 +95,15 @@ function getTokenClaims(r, token) {
r.subrequest('/_token_validation', 'token=' + token,
function(reply) {
if (reply.status !== 200) {
r.error("Failed to retrieve claims: HTTP " + reply.status);
r.error("Failed to retrieve claims for " + r.headersIn['host'] + r.uri + ": HTTP " + reply.status);
resolve(null);
return;
}
try {
const claims = JSON.parse(reply.responseText);
resolve(claims);
} catch (e) {
r.error("Failed to parse claims: " + e);
r.error("Failed to parse claims for " + r.headersIn['host'] + r.uri + ": " + e);
resolve(null);
}
}
Expand Down Expand Up @@ -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(' ')}`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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`);
return false;
}

// Audience must include the configured client
const aud = Array.isArray(claims.aud) ? claims.aud : claims.aud.split(',');
if (!aud.includes(r.variables.oidc_client)) {
r.error(`OIDC ID Token validation error: aud claim (${claims.aud}) ` +
r.error(`OIDC ID Token validation error for ` + r.headersIn['host'] + r.uri + `: aud claim (${claims.aud}) ` +
`does not include $oidc_client (${r.variables.oidc_client})`);
return false;
}
Expand All @@ -160,13 +160,13 @@ function validateIdTokenClaims(r, claims) {
: '';

if (claims.nonce !== clientNonceHash) {
r.error(`OIDC ID Token validation error: nonce from token (${claims.nonce}) ` +
r.error(`OIDC ID Token validation error for ` + r.headersIn['host'] + r.uri + `: nonce from token (${claims.nonce}) ` +
`does not match client (${clientNonceHash})`);
return false;
}
} else if (isNewSession(r)) {
r.error("OIDC ID Token validation error: " +
"missing nonce claim during initial authentication.");
r.error("OIDC ID Token validation error for " + r.headersIn['host'] + r.uri +
": missing nonce claim during initial authentication.");
return false;
}

Expand Down Expand Up @@ -227,7 +227,7 @@ async function exchangeCodeForTokens(r) {
});

if (reply.status === 504) {
r.error("OIDC timeout connecting to IdP during code exchange");
r.error("OIDC timeout connecting to IdP during code exchange for " + r.headersIn['host'] + r.uri);
r.return(504);
return null;
}
Expand All @@ -241,13 +241,13 @@ async function exchangeCodeForTokens(r) {
try {
const tokenset = JSON.parse(reply.responseText);
if (tokenset.error) {
r.error("OIDC " + tokenset.error + " " + tokenset.error_description);
r.error("OIDC for " + r.headersIn['host'] + r.uri + " " + tokenset.error + " " + tokenset.error_description);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double space after OIDC and no colon after the context

r.return(500);
return null;
}
return tokenset;
} catch (e) {
r.error("OIDC token response not JSON: " + reply.responseText);
r.error("OIDC token response not JSON for " + r.headersIn['host'] + r.uri + ": " + reply.responseText);
r.return(502);
return null;
}
Expand All @@ -267,9 +267,9 @@ async function refreshTokens(r) {
try {
const tokenset = JSON.parse(reply.responseText);
if (!tokenset.id_token) {
r.error("OIDC refresh response did not include id_token");
r.error("OIDC refresh response for " + r.headersIn['host'] + r.uri + " did not include id_token");
if (tokenset.error) {
r.error("OIDC " + tokenset.error + " " + tokenset.error_description);
r.error("OIDC error for " + r.headersIn['host'] + r.uri + " " + tokenset.error + " " + tokenset.error_description);
}
return null;
}
Expand Down Expand Up @@ -336,13 +336,13 @@ async function handleFrontChannelLogout(r) {

// Validate input parameters
if (!sid) {
r.error("Missing sid parameter in front-channel logout request");
r.error("Missing sid parameter in front-channel logout request for " + r.headersIn['host'] + r.uri);
r.return(400, "Missing sid");
return;
}

if (!requestIss) {
r.error("Missing iss parameter in front-channel logout request");
r.error("Missing iss parameter in front-channel logout request for " + r.headersIn['host'] + r.uri);
r.return(400, "Missing iss");
return;
}
Expand Down Expand Up @@ -373,7 +373,7 @@ async function handleFrontChannelLogout(r) {

const claims = await getTokenClaims(r, sessionJwt);
if (claims.iss !== requestIss) {
r.error("Issuer mismatch during logout. Received iss: " +
r.error("Issuer mismatch during logout for " + r.headersIn['host'] + r.uri + ": Received iss: " +
requestIss + ", expected: " + claims.iss);
r.return(400, "Issuer mismatch");
return;
Expand Down Expand Up @@ -401,7 +401,7 @@ function initiateNewAuth(r) {
);

if (missingConfig.length) {
r.error("OIDC missing configuration variables: $oidc_" + missingConfig.join(" $oidc_"));
r.error("OIDC missing configuration variables for " + r.headersIn['host'] + r.uri + ": $oidc_" + missingConfig.join(" $oidc_"));
r.return(500, r.variables.internal_error_message);
return;
}
Expand Down Expand Up @@ -467,7 +467,7 @@ function generateTokenRequestParams(r, grant_type) {
body += "&refresh_token=" + r.variables.refresh_token;
break;
default:
r.error("Unsupported grant type: " + grant_type);
r.error("Unsupported grant type for " + r.headersIn['host'] + r.uri + ": " + grant_type);
return;
}

Expand All @@ -493,21 +493,21 @@ function handleTokenError(r, reply) {
try {
const errorset = JSON.parse(reply.responseText);
if (errorset.error) {
r.error("OIDC error from IdP during token exchange: " +
r.error("OIDC error from IdP during token exchange for " + r.headersIn['host'] + r.uri + ": " +
errorset.error + ", " + errorset.error_description);
} else {
r.error("OIDC unexpected response from IdP (HTTP " +
r.error("OIDC unexpected response from IdP for " + r.headersIn['host'] + r.uri + " (HTTP " +
reply.status + "). " + reply.responseText);
}
} catch (e) {
r.error("OIDC unexpected response from IdP (HTTP " + reply.status + "). " +
r.error("OIDC unexpected response from IdP for " + r.headersIn['host'] + r.uri + " (HTTP " + reply.status + "). " +
reply.responseText);
}
}


function handleRefreshError(r, reply) {
let errorLog = "OIDC refresh failure";
let errorLog = "OIDC refresh failure for " + r.headersIn['host'] + r.uri;
if (reply.status === 504) {
errorLog += ", timeout waiting for IdP";
} else if (reply.status === 400) {
Expand Down