Authorization code is not properly redacted when logging raw values
Closed this issue · 2 comments
Hello,
We are using Duende.IdentityServer 6.2.3 and .NET 6.0.14.
Describe the bug
During custom token request validation error flows (ICustomTokenRequestValidator.ValidateAsync), we are seeing authorization codes being printed in plaintext and not properly redacted in the raw value dumped in our logs (ApplicationInsights).
To Reproduce
The bug can be seen when logs are written for multiple validation error scenarios, e.g. attempting to validate a CustomTokenRequestValidationContext using an implementation of ICustomTokenRequestValidator when LaunchContext.Properties is null or when a user (retrieved using ValidatedRequest.Subject) is disabled or not present in the user store.
When validation errors occur, we expect the raw dump to have properly redacted all sensitive information before logging to ApplicationInsights.
Log output/exception with stacktrace
# Expected logs when "code" is properly redacted during an invalid user flow.
Custom token request validator{ error: "invalid_user" }, details: TokenRequestValidationLog
{ ClientId: "myClientId", ClientName: "myClientName", GrantType: "authorization_code",
Scopes: null, AuthorizationCode: "****6C-1", RefreshToken: "********", UserName: null,
AuthenticationContextReferenceClasses: null, Tenant: null, IdP: null,
Raw: [("grant_type": "authorization_code"), ("code": "***REDACTED***"),
("redirect_uri": "myRedirectUri"), ("code_verifier": "myCodeVerifier"),
("client_id": "myClientId"), ("client_secret": "***REDACTED***")] }
Additional context
In the example screenshots, we can see that the AuthorizationCode property is properly censored, but the code property in the raw is not. These logs seem to be produced by IdentityServer/TokenRequestValidator.cs line 274 and IdentityServer/TokenRequestValidationLog.cs.
Hi, thanks for raising this issue. The cause of this issue seems to be that the default value for the TokenRequestSensitiveValuesFilter option doesn't include the code. We're going to consider changing that default, but as a workaround, you can set that option to include code. The current default is to filter client_secret, password, client_assertion, refresh_token, and device_code, so I would set the option to include all of those values, and also code.
Okay, we're going to change the default in the 6.3 release for consistency between the raw and parsed value. But to put your mind at ease about logs you already have, I just want to point out that codes are one time use only, so by the time they are logged they should not be useful any more.