Clean up logs for Negotiate Authentication and Authorization (#21927)

* Clean up logs for Negotiate Authentication and Authorization

* Add missing arg in NegotiateLoggingExtensions.cs

* Adjust formatting

* Remaining text changes

* Update src/Security/Authentication/Negotiate/src/Internal/NegotiateLoggingExtensions.cs

Co-authored-by: Chris Ross <Tratcher@Outlook.com>

* Update src/Security/Authentication/Negotiate/src/Internal/NegotiateLoggingExtensions.cs

Co-authored-by: Chris Ross <Tratcher@Outlook.com>

* Update src/Security/Authentication/Negotiate/src/NegotiateHandler.cs

Co-authored-by: Chris Ross <Tratcher@Outlook.com>

* Update src/Security/Authentication/Negotiate/src/Internal/NegotiateLoggingExtensions.cs

Co-authored-by: Chris Ross <Tratcher@Outlook.com>

* Apply changes to feedback committed

* Update logger call to refactored name

Co-authored-by: Chris Ross <Tratcher@Outlook.com>
Co-authored-by: Giuseppe Campanelli <campanelli_g@yahoo.com>
This commit is contained in:
Giuseppe Campanelli 2020-05-19 11:59:29 -04:00 committed by GitHub
parent 02bf53de96
commit 78edd18524
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 18 additions and 10 deletions

View File

@ -17,6 +17,7 @@ namespace Microsoft.Extensions.Logging
private static Action<ILogger, Exception> _challengeNegotiate;
private static Action<ILogger, Exception> _reauthenticating;
private static Action<ILogger, Exception> _deferring;
private static Action<ILogger, string, Exception> _negotiateError;
static NegotiateLoggingExtensions()
{
@ -43,7 +44,7 @@ namespace Microsoft.Extensions.Logging
_challengeNegotiate = LoggerMessage.Define(
eventId: new EventId(6, "ChallengeNegotiate"),
logLevel: LogLevel.Debug,
formatString: "Challenged 401 Negotiate");
formatString: "Challenged 401 Negotiate.");
_reauthenticating = LoggerMessage.Define(
eventId: new EventId(7, "Reauthenticating"),
logLevel: LogLevel.Debug,
@ -60,6 +61,10 @@ namespace Microsoft.Extensions.Logging
eventId: new EventId(10, "ClientError"),
logLevel: LogLevel.Debug,
formatString: "The users authentication request was invalid.");
_negotiateError = LoggerMessage.Define<string>(
eventId: new EventId(11, "NegotiateError"),
logLevel: LogLevel.Debug,
formatString: "Negotiate error code: {error}.");
}
public static void IncompleteNegotiateChallenge(this ILogger logger)
@ -91,5 +96,8 @@ namespace Microsoft.Extensions.Logging
public static void ClientError(this ILogger logger, Exception ex)
=> _clientError(logger, ex);
public static void NegotiateError(this ILogger logger, string error)
=> _negotiateError(logger, error, null);
}
}

View File

@ -42,7 +42,7 @@ namespace Microsoft.AspNetCore.Authentication.Negotiate
{ }
/// <summary>
/// The handler calls methods on the events which give the application control at certain points where processing is occurring.
/// The handler calls methods on the events which give the application control at certain points where processing is occurring.
/// If it is not provided a default instance is supplied which does nothing when the methods are called.
/// </summary>
protected new NegotiateEvents Events
@ -129,9 +129,9 @@ namespace Microsoft.AspNetCore.Authentication.Negotiate
_negotiateState ??= Options.StateFactory.CreateInstance();
var outgoing = _negotiateState.GetOutgoingBlob(token, out var errorType, out var exception);
Logger.LogInformation(errorType.ToString());
if (errorType != BlobErrorType.None)
{
Logger.NegotiateError(errorType.ToString());
_negotiateState.Dispose();
_negotiateState = null;
if (persistence?.State != null)

View File

@ -32,7 +32,7 @@ namespace Microsoft.AspNetCore.Authorization.Infrastructure
public override string ToString()
{
return $"{nameof(DenyAnonymousAuthorizationRequirement)}:Requires an authenticated user.";
return $"{nameof(DenyAnonymousAuthorizationRequirement)}: Requires an authenticated user.";
}
}
}

View File

@ -20,7 +20,7 @@ namespace Microsoft.Extensions.Logging
_userAuthorizationFailed = LoggerMessage.Define<string>(
eventId: new EventId(2, "UserAuthorizationFailed"),
logLevel: LogLevel.Information,
formatString: "Authorization failed for {0}");
formatString: "Authorization failed. {0}");
}
public static void UserAuthorizationSucceeded(this ILogger logger)

View File

@ -1221,7 +1221,7 @@ namespace Microsoft.AspNetCore.Authorization.Test
Assert.Equal("UserAuthorizationFailed", eventId.Name);
var message = formatter(state, exception);
Assert.Equal("Authorization failed for These requirements were not met:" + Environment.NewLine + "LogRequirement" + Environment.NewLine + "LogRequirement", message);
Assert.Equal("Authorization failed. These requirements were not met:" + Environment.NewLine + "LogRequirement" + Environment.NewLine + "LogRequirement", message);
}
var authorizationService = BuildAuthorizationService(services =>
@ -1241,7 +1241,7 @@ namespace Microsoft.AspNetCore.Authorization.Test
// Assert
}
[Fact]
public async Task Authorize_ShouldLogExplicitFailedWhenFailedCall()
{
@ -1254,7 +1254,7 @@ namespace Microsoft.AspNetCore.Authorization.Test
Assert.Equal("UserAuthorizationFailed", eventId.Name);
var message = formatter(state, exception);
Assert.Equal("Authorization failed for Fail() was explicitly called.", message);
Assert.Equal("Authorization failed. Fail() was explicitly called.", message);
}
var authorizationService = BuildAuthorizationService(services =>

View File

@ -15,7 +15,7 @@ namespace Microsoft.AspNetCore.Authorization.Test
{
return new DenyAnonymousAuthorizationRequirement();
}
[Fact]
public void ToString_ShouldReturnFormatValue()
{
@ -26,7 +26,7 @@ namespace Microsoft.AspNetCore.Authorization.Test
var formattedValue = requirement.ToString();
// Assert
Assert.Equal("DenyAnonymousAuthorizationRequirement:Requires an authenticated user.", formattedValue);
Assert.Equal("DenyAnonymousAuthorizationRequirement: Requires an authenticated user.", formattedValue);
}
}
}