Filter subject tokens (from token exchange) from logs

[krosn]

Which version of Duende IdentityServer are you using?
6.3.6

Which version of .NET are you using?
.Net 8.0.100

Describe the bug

Not sure if this is actually a bug, but it does seem strange that the list of sensitive filter values for TokenRequests does not include the Subject Token.

To Reproduce

Perform a token exchange and observe that the original token (ex. an access token) is logged in the Subject Token field.

Current log output:

{
  "ClientId": "whatever",
  "ClientName": "whatever",
  "GrantType": "urn:ietf:params:oauth:grant-type:token-exchange",
  "Scopes": "whatever",
  "AuthorizationCode": "********",
  "RefreshToken": "********",
  "Raw": {
    "token_exchange_source_hint": "whatever",
    "grant_type": "urn:ietf:params:oauth:grant-type:token-exchange",
    "subject_token": "!!!!!!!!TOKEN!!!!!!!!!!",
    "subject_token_type": "urn:ietf:params:oauth:token-type:access_token",
    "scope": "whatever",
    "client_id": "whatever",
    "client_secret": "***REDACTED***"
  }
}

Expected behavior

I think it might be safer to include the OidcConstants.TokenRequest.SubjectToken constant in the default list of TokenRequestSensitiveValuesFilter in the LoggerOptions.

Suggested log output:

{
  "ClientId": "whatever",
  "ClientName": "whatever",
  "GrantType": "urn:ietf:params:oauth:grant-type:token-exchange",
  "Scopes": "whatever",
  "AuthorizationCode": "********",
  "RefreshToken": "********",
  "Raw": {
    "token_exchange_source_hint": "whatever",
    "grant_type": "urn:ietf:params:oauth:grant-type:token-exchange",
    "subject_token": "***REDACTED**",
    "subject_token_type": "urn:ietf:params:oauth:token-type:access_token",
    "scope": "whatever",
    "client_id": "whatever",
    "client_secret": "***REDACTED***"
  }
}

Additional context

If this suggestion sounds good, I can go ahead and open a PR.

[brockallen]

Is subject token always an access token? Could it be an id_token?

[krosn]

In my use case, it will always be an access token, but it's possible that someone else's use case may involve a workflow where the subject token is an Id token. Either way, as long as it's possible for a subject token to be an access token, it seems like it would be safer to redact it by default. It's just a suggestion though. The existing logger options allowed me to add the subject token field to the list of sensitive token request values, so it's certainly easy enough to redact it if this current behavior is considered more desirable.

[josephdecock]

I think this seems like it would be a good change to the defaults. Id tokens could be sensitive if the claims contain PII. And since we don't know if the subject token would be an access or id token, it seems best to be cautious to me.

[krosn]

@josephdecock, I created a PR with the change described above. Let me know if you'd like anything changed. Thanks!

#1521