From ba1eb281d135400436c52c17edc71307bc038ec0 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Tue, 16 Jan 2018 11:40:05 -0800 Subject: [PATCH] Stop logging username/token Fixes https://github.com/aspnet/Security/issues/1259 --- .../JwtBearerHandler.cs | 2 +- .../LoggingExtensions.cs | 20 +++++--------- .../DefaultAuthorizationService.cs | 26 ++----------------- .../LoggingExtensions.cs | 24 +++++++---------- 4 files changed, 20 insertions(+), 52 deletions(-) diff --git a/src/Microsoft.AspNetCore.Authentication.JwtBearer/JwtBearerHandler.cs b/src/Microsoft.AspNetCore.Authentication.JwtBearer/JwtBearerHandler.cs index f894a97d0c..6d5c7f5f5e 100644 --- a/src/Microsoft.AspNetCore.Authentication.JwtBearer/JwtBearerHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication.JwtBearer/JwtBearerHandler.cs @@ -110,7 +110,7 @@ namespace Microsoft.AspNetCore.Authentication.JwtBearer } catch (Exception ex) { - Logger.TokenValidationFailed(token, ex); + Logger.TokenValidationFailed(ex); // Refresh the configuration for exceptions that may be caused by key rollovers. The user can also request a refresh in the event. if (Options.RefreshOnIssuerKeyNotFound && Options.ConfigurationManager != null diff --git a/src/Microsoft.AspNetCore.Authentication.JwtBearer/LoggingExtensions.cs b/src/Microsoft.AspNetCore.Authentication.JwtBearer/LoggingExtensions.cs index 38a75fecaf..5c6ca088a8 100644 --- a/src/Microsoft.AspNetCore.Authentication.JwtBearer/LoggingExtensions.cs +++ b/src/Microsoft.AspNetCore.Authentication.JwtBearer/LoggingExtensions.cs @@ -7,16 +7,16 @@ namespace Microsoft.Extensions.Logging { internal static class LoggingExtensions { - private static Action _tokenValidationFailed; + private static Action _tokenValidationFailed; private static Action _tokenValidationSucceeded; private static Action _errorProcessingMessage; static LoggingExtensions() { - _tokenValidationFailed = LoggerMessage.Define( + _tokenValidationFailed = LoggerMessage.Define( eventId: 1, logLevel: LogLevel.Information, - formatString: "Failed to validate the token {Token}"); + formatString: "Failed to validate the token."); _tokenValidationSucceeded = LoggerMessage.Define( eventId: 2, logLevel: LogLevel.Information, @@ -27,19 +27,13 @@ namespace Microsoft.Extensions.Logging formatString: "Exception occurred while processing message."); } - public static void TokenValidationFailed(this ILogger logger, string token, Exception ex) - { - _tokenValidationFailed(logger, token, ex); - } + public static void TokenValidationFailed(this ILogger logger, Exception ex) + => _tokenValidationFailed(logger, ex); public static void TokenValidationSucceeded(this ILogger logger) - { - _tokenValidationSucceeded(logger, null); - } + => _tokenValidationSucceeded(logger, null); public static void ErrorProcessingMessage(this ILogger logger, Exception ex) - { - _errorProcessingMessage(logger, ex); - } + => _errorProcessingMessage(logger, ex); } } diff --git a/src/Microsoft.AspNetCore.Authorization/DefaultAuthorizationService.cs b/src/Microsoft.AspNetCore.Authorization/DefaultAuthorizationService.cs index 9773ebbcc2..bc5d571c47 100644 --- a/src/Microsoft.AspNetCore.Authorization/DefaultAuthorizationService.cs +++ b/src/Microsoft.AspNetCore.Authorization/DefaultAuthorizationService.cs @@ -98,37 +98,15 @@ namespace Microsoft.AspNetCore.Authorization var result = _evaluator.Evaluate(authContext); if (result.Succeeded) { - _logger.UserAuthorizationSucceeded(GetUserNameForLogging(user)); + _logger.UserAuthorizationSucceeded(); } else { - _logger.UserAuthorizationFailed(GetUserNameForLogging(user)); + _logger.UserAuthorizationFailed(); } return result; } - private string GetUserNameForLogging(ClaimsPrincipal user) - { - var identity = user?.Identity; - if (identity != null) - { - var name = identity.Name; - if (name != null) - { - return name; - } - return GetClaimValue(identity, "sub") - ?? GetClaimValue(identity, ClaimTypes.Name) - ?? GetClaimValue(identity, ClaimTypes.NameIdentifier); - } - return null; - } - - private static string GetClaimValue(IIdentity identity, string claimsType) - { - return (identity as ClaimsIdentity)?.FindFirst(claimsType)?.Value; - } - /// /// Checks if a user meets a specific authorization policy. /// diff --git a/src/Microsoft.AspNetCore.Authorization/LoggingExtensions.cs b/src/Microsoft.AspNetCore.Authorization/LoggingExtensions.cs index 1d524dd74e..386df85e09 100644 --- a/src/Microsoft.AspNetCore.Authorization/LoggingExtensions.cs +++ b/src/Microsoft.AspNetCore.Authorization/LoggingExtensions.cs @@ -7,29 +7,25 @@ namespace Microsoft.Extensions.Logging { internal static class LoggingExtensions { - private static Action _userAuthorizationFailed; - private static Action _userAuthorizationSucceeded; + private static Action _userAuthorizationFailed; + private static Action _userAuthorizationSucceeded; static LoggingExtensions() { - _userAuthorizationSucceeded = LoggerMessage.Define( + _userAuthorizationSucceeded = LoggerMessage.Define( eventId: 1, logLevel: LogLevel.Information, - formatString: "Authorization was successful for user: {UserName}."); - _userAuthorizationFailed = LoggerMessage.Define( + formatString: "Authorization was successful."); + _userAuthorizationFailed = LoggerMessage.Define( eventId: 2, logLevel: LogLevel.Information, - formatString: "Authorization failed for user: {UserName}."); + formatString: "Authorization failed."); } - public static void UserAuthorizationSucceeded(this ILogger logger, string userName) - { - _userAuthorizationSucceeded(logger, userName, null); - } + public static void UserAuthorizationSucceeded(this ILogger logger) + => _userAuthorizationSucceeded(logger, null); - public static void UserAuthorizationFailed(this ILogger logger, string userName) - { - _userAuthorizationFailed(logger, userName, null); - } + public static void UserAuthorizationFailed(this ILogger logger) + => _userAuthorizationFailed(logger, null); } }