Skip to content

fix(security): mask credentials in Entra ID debug logs - #3213

Open
marevol wants to merge 1 commit into
masterfrom
entraid-mask-debug-logs
Open

fix(security): mask credentials in Entra ID debug logs#3213
marevol wants to merge 1 commit into
masterfrom
entraid-mask-debug-logs

Conversation

@marevol

@marevol marevol commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Problem

#3077 truncated idToken and refreshToken in EntraIdAuthenticator's debug logs, but
three other call sites were left writing credentials verbatim:

Method Statement Leaks
getAccessToken(AuthorizationCode, String) logger.debug("authCode={}, ...", authCode, ...) the authorization code, in full
processAuthenticationData() logger.debug("process authentication: url: {}, params: {}", urlBuf, params) code / id_token from the POST body, plus the request URL including its query string
containsAuthenticationData() logger.debug("params={}", params) the same parameter map again

parseAuthenticationResponse() logged the full callback URL as well.

Fix

Three helpers, and every one of those sites routed through them:

  • maskSecret(String) - keeps an 8-character prefix plus ***, so two different codes are
    still distinguishable in a log without the value being usable. Null/empty pass through.
  • maskParams(Map<String, List<String>>) - masks only the values of code, id_token,
    access_token, refresh_token and client_secret (case-insensitive). state, error and
    error_description stay verbatim, and the key set is preserved, so a failed login is still
    diagnosable from the log.
  • maskQueryString(String) - drops the query string from a logged URL. Every parameter it
    holds is already logged separately through maskParams.

containsAuthenticationData() only ever reads the key set, so it now logs only
params.keySet().

The two values #3077 masked inline now reuse maskSecret(). That also removes an NPE:
validateNonce() called idToken.substring(...) outside its try block, so a response
without an id token failed with a NullPointerException instead of the intended
SsoLoginException.

Tests

EntraIdAuthenticator's unit test class, 3 new tests:

  • test_maskSecret - long / short / exactly-prefix-length / null / empty
  • test_maskParams_masksCredentialsAndKeepsDiagnostics - credentials masked, diagnostics kept,
    key set preserved, caller's map not mutated
  • test_maskParams_isCaseInsensitiveOnKeys

Verified falsifiable: with maskSecret() stubbed to return its input, all 3 fail.

Tests run: 26, Failures: 0, Errors: 0, Skipped: 0

PR #3077 truncated the id token and the refresh token, but three call sites
were left writing credentials verbatim when debug logging is on:

- getAccessToken() logged the authorization code in full
- processAuthenticationData() logged the whole parameter map, which carries
  code and id_token, plus the request URL including its query string
- containsAuthenticationData() logged the same parameter map again

Add maskSecret(), maskParams() and maskQueryString(), and route every one of
these through them. Credential values are truncated to an 8-character prefix
so they can still be correlated across log lines; state, error and
error_description stay verbatim, and the parameter key set is preserved, so a
failed login is still diagnosable. containsAuthenticationData() only reads the
key set, so it now logs only that.

The two values PR #3077 masked inline now reuse maskSecret(). That also
removes an NPE: validateNonce() called idToken.substring() outside its try
block, so a response without an id token failed with a NullPointerException
instead of the intended SsoLoginException.
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.

1 participant