Skip to content
This repository has been archived by the owner on Jul 31, 2024. It is now read-only.

Commit

Permalink
Introduce LoggingOptions.AuthorizeRequestSensitiveValuesFilter
Browse files Browse the repository at this point in the history
We take some sensitive parameters with the "AuthorizeRequest" endpoint with some ICustomAuthorizeRequestValidator implementation to validate them.

We don't want those sensitive parameters be logged when the validation fails.

This commit introduces `AuthorizeRequestSensitiveValuesFilter` similar to `TokenRequestSensitiveValuesFilter`. This will allow us customize which parameter gets scrubbed.
  • Loading branch information
mustakimali committed Aug 13, 2020
1 parent bf84eeb commit 6a93eb7
Show file tree
Hide file tree
Showing 9 changed files with 40 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,14 @@ public class LoggingOptions
OidcConstants.TokenRequest.RefreshToken,
OidcConstants.TokenRequest.DeviceCode
};

/// <summary>
///
/// </summary>
public ICollection<string> AuthorizeRequestSensitiveValuesFilter { get; set; } =
new HashSet<string>
{
OidcConstants.AuthorizeRequest.IdTokenHint
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Collections.Specialized;
using System.Net;
using System.Threading.Tasks;
using IdentityServer4.Configuration;
using IdentityServer4.Endpoints.Results;
using IdentityServer4.Extensions;
using IdentityServer4.Hosting;
Expand All @@ -25,13 +26,14 @@ internal class AuthorizeCallbackEndpoint : AuthorizeEndpointBase
public AuthorizeCallbackEndpoint(
IEventService events,
ILogger<AuthorizeCallbackEndpoint> logger,
IdentityServerOptions options,
IAuthorizeRequestValidator validator,
IAuthorizeInteractionResponseGenerator interactionGenerator,
IAuthorizeResponseGenerator authorizeResponseGenerator,
IUserSession userSession,
IConsentMessageStore consentResponseStore,
IAuthorizationParametersMessageStore authorizationParametersMessageStore = null)
: base(events, logger, validator, interactionGenerator, authorizeResponseGenerator, userSession)
: base(events, logger, options, validator, interactionGenerator, authorizeResponseGenerator, userSession)
{
_consentResponseStore = consentResponseStore;
_authorizationParametersMessageStore = authorizationParametersMessageStore;
Expand Down
4 changes: 3 additions & 1 deletion src/IdentityServer4/src/Endpoints/AuthorizeEndpoint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Collections.Specialized;
using System.Net;
using System.Threading.Tasks;
using IdentityServer4.Configuration;
using IdentityServer4.Endpoints.Results;
using IdentityServer4.Extensions;
using IdentityServer4.Hosting;
Expand All @@ -20,11 +21,12 @@ internal class AuthorizeEndpoint : AuthorizeEndpointBase
public AuthorizeEndpoint(
IEventService events,
ILogger<AuthorizeEndpoint> logger,
IdentityServerOptions options,
IAuthorizeRequestValidator validator,
IAuthorizeInteractionResponseGenerator interactionGenerator,
IAuthorizeResponseGenerator authorizeResponseGenerator,
IUserSession userSession)
: base(events, logger, validator, interactionGenerator, authorizeResponseGenerator, userSession)
: base(events, logger, options, validator, interactionGenerator, authorizeResponseGenerator, userSession)
{
}

Expand Down
8 changes: 6 additions & 2 deletions src/IdentityServer4/src/Endpoints/AuthorizeEndpointBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Security.Claims;
using System.Threading.Tasks;
using IdentityModel;
using IdentityServer4.Configuration;
using IdentityServer4.Endpoints.Results;
using IdentityServer4.Events;
using IdentityServer4.Extensions;
Expand All @@ -25,6 +26,7 @@ internal abstract class AuthorizeEndpointBase : IEndpointHandler
private readonly IAuthorizeResponseGenerator _authorizeResponseGenerator;

private readonly IEventService _events;
private readonly IdentityServerOptions _options;

private readonly IAuthorizeInteractionResponseGenerator _interactionGenerator;

Expand All @@ -33,12 +35,14 @@ internal abstract class AuthorizeEndpointBase : IEndpointHandler
protected AuthorizeEndpointBase(
IEventService events,
ILogger<AuthorizeEndpointBase> logger,
IdentityServerOptions options,
IAuthorizeRequestValidator validator,
IAuthorizeInteractionResponseGenerator interactionGenerator,
IAuthorizeResponseGenerator authorizeResponseGenerator,
IUserSession userSession)
{
_events = events;
_options = options;
Logger = logger;
_validator = validator;
_interactionGenerator = interactionGenerator;
Expand Down Expand Up @@ -119,7 +123,7 @@ protected async Task<IEndpointResult> CreateErrorResultAsync(

if (request != null)
{
var details = new AuthorizeRequestValidationLog(request);
var details = new AuthorizeRequestValidationLog(request, _options.Logging.AuthorizeRequestSensitiveValuesFilter);
Logger.LogInformation("{@validationDetails}", details);
}

Expand All @@ -137,7 +141,7 @@ protected async Task<IEndpointResult> CreateErrorResultAsync(

private void LogRequest(ValidatedAuthorizeRequest request)
{
var details = new AuthorizeRequestValidationLog(request);
var details = new AuthorizeRequestValidationLog(request, _options.Logging.AuthorizeRequestSensitiveValuesFilter);
Logger.LogDebug(nameof(ValidatedAuthorizeRequest) + Environment.NewLine + "{@validationDetails}", details);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ internal class AuthorizeRequestValidationLog

public Dictionary<string, string> Raw { get; set; }

public AuthorizeRequestValidationLog(ValidatedAuthorizeRequest request)
public AuthorizeRequestValidationLog(ValidatedAuthorizeRequest request, IEnumerable<string> sensitiveValuesFilter)
{
Raw = request.Raw.ToScrubbedDictionary(OidcConstants.AuthorizeRequest.IdTokenHint);
Raw = request.Raw.ToScrubbedDictionary(sensitiveValuesFilter.ToArray());

if (request.Client != null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -835,14 +835,14 @@ private AuthorizeRequestValidationResult Valid(ValidatedAuthorizeRequest request

private void LogError(string message, ValidatedAuthorizeRequest request)
{
var requestDetails = new AuthorizeRequestValidationLog(request);
var requestDetails = new AuthorizeRequestValidationLog(request, _options.Logging.AuthorizeRequestSensitiveValuesFilter);
_logger.LogError(message + "\n{@requestDetails}", requestDetails);
}

private void LogError(string message, string detail, ValidatedAuthorizeRequest request)
{
var requestDetails = new AuthorizeRequestValidationLog(request);
var requestDetails = new AuthorizeRequestValidationLog(request, _options.Logging.AuthorizeRequestSensitiveValuesFilter);
_logger.LogError(message + ": {detail}\n{@requestDetails}", detail, requestDetails);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using FluentAssertions;
using IdentityServer.UnitTests.Common;
using IdentityServer4;
using IdentityServer4.Configuration;
using IdentityServer4.Endpoints;
using IdentityServer4.Endpoints.Results;
using IdentityServer4.Extensions;
Expand All @@ -28,6 +29,8 @@ public class AuthorizeCallbackEndpointTests

private ILogger<AuthorizeCallbackEndpoint> _fakeLogger = TestLogger.Create<AuthorizeCallbackEndpoint>();

private IdentityServerOptions _options = new IdentityServerOptions();

private MockConsentMessageStore _mockUserConsentResponseMessageStore = new MockConsentMessageStore();

private MockUserSession _mockUserSession = new MockUserSession();
Expand Down Expand Up @@ -225,6 +228,7 @@ internal void Init()
_subject = new AuthorizeCallbackEndpoint(
_fakeEventService,
_fakeLogger,
_options,
_stubAuthorizeRequestValidator,
_stubInteractionGenerator,
_stubAuthorizeResponseGenerator,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using FluentAssertions;
using IdentityServer.UnitTests.Common;
using IdentityServer4;
using IdentityServer4.Configuration;
using IdentityServer4.Endpoints;
using IdentityServer4.Endpoints.Results;
using IdentityServer4.Hosting;
Expand All @@ -30,6 +31,8 @@ public class AuthorizeEndpointBaseTests

private ILogger<TestAuthorizeEndpoint> _fakeLogger = TestLogger.Create<TestAuthorizeEndpoint>();

private IdentityServerOptions _options = new IdentityServerOptions();

private MockUserSession _mockUserSession = new MockUserSession();

private NameValueCollection _params = new NameValueCollection();
Expand Down Expand Up @@ -180,6 +183,7 @@ internal void Init()
_subject = new TestAuthorizeEndpoint(
_fakeEventService,
_fakeLogger,
_options,
_stubAuthorizeRequestValidator,
_stubInteractionGenerator,
_stubAuthorizeResponseGenerator,
Expand All @@ -191,11 +195,12 @@ internal class TestAuthorizeEndpoint : AuthorizeEndpointBase
public TestAuthorizeEndpoint(
IEventService events,
ILogger<TestAuthorizeEndpoint> logger,
IdentityServerOptions options,
IAuthorizeRequestValidator validator,
IAuthorizeInteractionResponseGenerator interactionGenerator,
IAuthorizeResponseGenerator authorizeResponseGenerator,
IUserSession userSession)
: base(events, logger, validator, interactionGenerator, authorizeResponseGenerator, userSession)
: base(events, logger, options, validator, interactionGenerator, authorizeResponseGenerator, userSession)
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using FluentAssertions;
using IdentityServer.UnitTests.Common;
using IdentityServer4;
using IdentityServer4.Configuration;
using IdentityServer4.Endpoints;
using IdentityServer4.Endpoints.Results;
using IdentityServer4.Models;
Expand All @@ -28,6 +29,8 @@ public class AuthorizeEndpointTests

private ILogger<AuthorizeEndpoint> _fakeLogger = TestLogger.Create<AuthorizeEndpoint>();

private IdentityServerOptions _options = new IdentityServerOptions();

private MockUserSession _mockUserSession = new MockUserSession();

private NameValueCollection _params = new NameValueCollection();
Expand Down Expand Up @@ -100,6 +103,7 @@ internal void Init()
_subject = new AuthorizeEndpoint(
_fakeEventService,
_fakeLogger,
_options,
_stubAuthorizeRequestValidator,
_stubInteractionGenerator,
_stubAuthorizeResponseGenerator,
Expand Down

0 comments on commit 6a93eb7

Please sign in to comment.