Skip to content

Conversation

@shayaantx
Copy link

@shayaantx shayaantx commented Oct 13, 2024

…es + don't show clientSecret/secret in debug logs

By submitting a PR to this repository, you agree to the terms within the Auth0 Code of Conduct. Please see the contributing guidelines for how to create and submit a high-quality PR for this repo.

Description

Describe the purpose of this PR along with any background information and the impacts of the proposed change. For the benefit of the community, please do not assume prior context.

Extra logging to troubleshoot backchannel logout failing. In my case we were using a different version of jose in our app than the one used in this library (which uses openid-client), resulting in multiple class definitions of KeyStore being used resulting in the error "key must be an instance of a key instantiated by JWK.asKey, a valid JWK.asKey input, or a JWKS.KeyStore instance" being suppressed

  • Once I used the same version of jose, then instanceof checks in openid-client did not fail anymore during the LogoutToken verification.

Provide details that support your chosen implementation, including: breaking changes, alternatives considered, changes to the API, etc.

No breaking changes, just logging changes

If the UI is being changed, please provide screenshots.

None

References

None

Testing

Describe how this can be tested by reviewers. Be specific about anything not tested and reasons why. If this library has unit and/or integration testing, tests should be added for new functionality and existing tests should complete without errors.

Existing test cases should cover this

Please include any manual steps for testing end-to-end or functionality not covered by unit/integration tests.

If you want to test this, run through backchannel logout

Also include details of the environment this PR was developed in (language/platform/browser version).

  • This change adds test coverage for new/changed/fixed functionality

Checklist

  • I have added documentation for new/changed functionality in this PR or in auth0.com/docs
  • All active GitHub checks for tests, formatting, and security are passing
  • The correct base branch is being used, if not the default branch

…es + don't show clientSecret/secret in debug logs
@shayaantx shayaantx requested a review from a team as a code owner October 13, 2024 17:43
@kushalshit27 kushalshit27 requested a review from Copilot July 18, 2025 06:24
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances troubleshooting capabilities for backchannel logout functionality by adding additional debug logging while also improving security by redacting sensitive information from logs. The changes help developers identify specific failure points in the backchannel logout process while preventing accidental exposure of secrets.

  • Added debug logging for missing logout tokens and JWT verification failures
  • Redacted sensitive information (clientSecret and secret) from configuration debug logs
  • Improved clarity of existing error logging messages

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
middleware/auth.js Redacts clientSecret and secret values from configuration debug logs
lib/context.js Adds debug logging for backchannel logout failures and improves existing error message clarity

const auth = function (params) {
const config = getConfig(params);
debug('configuration object processed, resulting configuration: %O', config);
debug('configuration object processed, resulting configuration: %O', {...config, clientSecret: "REDACTED", secret: "REDACTED"});
Copy link

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

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

The spread operator approach may not handle nested objects containing sensitive data. Consider using a dedicated function to recursively redact sensitive fields or ensure no nested sensitive data exists in the config object.

Suggested change
debug('configuration object processed, resulting configuration: %O', {...config, clientSecret: "REDACTED", secret: "REDACTED"});
const redactSensitiveFields = (obj, fieldsToRedact) => {
if (obj && typeof obj === 'object') {
return Object.keys(obj).reduce((acc, key) => {
acc[key] = fieldsToRedact.includes(key)
? "REDACTED"
: redactSensitiveFields(obj[key], fieldsToRedact);
return acc;
}, Array.isArray(obj) ? [] : {});
}
return obj;
};
const redactedConfig = redactSensitiveFields(config, ['clientSecret', 'secret']);
debug('configuration object processed, resulting configuration: %O', redactedConfig);

Copilot uses AI. Check for mistakes.
res.setHeader('cache-control', 'no-store');
const logoutToken = req.body.logout_token;
if (!logoutToken) {
debug('req.oidc.backchannelLogout() failed due to missing logout token', req.body);
Copy link

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

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

Logging the entire req.body could potentially expose sensitive information. Consider logging only non-sensitive fields or a sanitized version of the request body.

Suggested change
debug('req.oidc.backchannelLogout() failed due to missing logout token', req.body);
debug('req.oidc.backchannelLogout() failed due to missing logout token. logout_token present: %s', !!req.body.logout_token);

Copilot uses AI. Check for mistakes.
@gyaneshgouraw-okta
Copy link
Contributor

@claude

@github-actions
Copy link

github-actions bot commented Oct 1, 2025

Claude finished @gyaneshgouraw-okta's task —— View job


I'll review this PR that adds logging for backchannel logout troubleshooting and redacts sensitive information from debug logs.

Todo List

  • Read the diff to understand changes
  • Read AGENTS.md for project context
  • Read modified files for deeper analysis
  • Create pending review
  • Add inline feedback for any issues
  • Submit overall review assessment

Review Complete ✅

I've submitted a code review highlighting two security concerns that need to be addressed:

  1. Request body logging exposure in lib/context.js:457 - The debug statement logs the entire request body which may contain sensitive information
  2. Incomplete sensitive data redaction in middleware/auth.js:25 - The current redaction only handles top-level properties and won't catch nested sensitive fields

The PR improves troubleshooting capabilities but these security issues should be resolved before merging to prevent accidental exposure of sensitive information in debug logs.


Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Overall Assessment

❌ Security concerns need to be addressed

The PR improves troubleshooting capabilities but has two security issues:

  1. Request body logging exposure: The new debug statement logs the entire req.body which may contain sensitive information beyond just the logout token
  2. Incomplete sensitive data redaction: The current approach only redacts top-level sensitive fields but won't handle nested objects that might contain secrets

Both issues should be resolved to prevent accidental exposure of sensitive information in debug logs.

res.setHeader('cache-control', 'no-store');
const logoutToken = req.body.logout_token;
if (!logoutToken) {
debug('req.oidc.backchannelLogout() failed due to missing logout token', req.body);
Copy link

Choose a reason for hiding this comment

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

Logging the entire req.body exposes sensitive information that may be present in the request. The logout token itself could contain sensitive claims, and other fields in the body might contain secrets.

Suggested change
debug('req.oidc.backchannelLogout() failed due to missing logout token', req.body);
debug('req.oidc.backchannelLogout() failed due to missing logout token. logout_token present: %s', !!req.body.logout_token);

const auth = function (params) {
const config = getConfig(params);
debug('configuration object processed, resulting configuration: %O', config);
debug('configuration object processed, resulting configuration: %O', {...config, clientSecret: "REDACTED", secret: "REDACTED"});
Copy link

Choose a reason for hiding this comment

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

The spread operator approach only handles top-level properties. If the config object contains nested objects with sensitive fields, they won't be redacted. Consider using a recursive redaction function to handle nested sensitive data.

Suggested change
debug('configuration object processed, resulting configuration: %O', {...config, clientSecret: "REDACTED", secret: "REDACTED"});
const redactSensitiveFields = (obj) => {
if (!obj || typeof obj !== 'object') return obj;
const result = Array.isArray(obj) ? [...obj] : { ...obj };
Object.keys(result).forEach(key => {
if (['clientSecret', 'secret'].includes(key)) {
result[key] = 'REDACTED';
} else if (typeof result[key] === 'object') {
result[key] = redactSensitiveFields(result[key]);
}
});
return result;
};
debug('configuration object processed, resulting configuration: %O', redactSensitiveFields(config));

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants