From c0dcc60c75fdcd8ec7e7ce603e1182a45580d08e Mon Sep 17 00:00:00 2001 From: stijnmoreels <9039753+stijnmoreels@users.noreply.github.com> Date: Mon, 4 Jul 2022 11:08:27 +0200 Subject: [PATCH 1/6] feat: change certificate auth target type --- .../features/security/auth/certificate.md | 9 +- .../CertificateAuthenticationConfigBuilder.cs | 7 + .../Extensions/FilterCollectionExtensions.cs | 5 +- .../Extensions/MvcOptionsExtensions.cs | 48 ++ .../CertificateAuthenticationFilterTests.cs | 447 +++++++++++++++++- .../CertificateAuthenticationFilterTests.cs | 16 + 6 files changed, 516 insertions(+), 16 deletions(-) create mode 100644 src/Arcus.WebApi.Security/Authentication/Certificates/Extensions/MvcOptionsExtensions.cs create mode 100644 src/Arcus.WebApi.Tests.Unit/Security/Authentication/CertificateAuthenticationFilterTests.cs diff --git a/docs/preview/features/security/auth/certificate.md b/docs/preview/features/security/auth/certificate.md index da6adce3..4fc7f984 100644 --- a/docs/preview/features/security/auth/certificate.md +++ b/docs/preview/features/security/auth/certificate.md @@ -74,10 +74,10 @@ builder.Services.AddSingleton(certificateValidator); builder.Services.AddControllers(mvcOptions => { // Adds certificate authentication to the request pipeline. - mvcOptions.Filters.AddCertificateAuthentication(); + mvcOptions.AddCertificateAuthenticationFilter(); // Additional consumer-configurable options to change the behavior of the authentication filter. - mvcOptions.Filters.AddCertificateAuthentication(configureOptions: options => + mvcOptions.AddCertificateAuthenticationFilter(configureOptions: options => { // Adds certificate authentication to the request pipeline with emitting security events during the authorization of the request. // (default: `false`) @@ -173,7 +173,4 @@ public class SystemController : ControllerBase return Ok(); } } -``` - - -[← back](/) +``` \ No newline at end of file diff --git a/src/Arcus.WebApi.Security/Authentication/Certificates/CertificateAuthenticationConfigBuilder.cs b/src/Arcus.WebApi.Security/Authentication/Certificates/CertificateAuthenticationConfigBuilder.cs index 00b3ca10..f713dfbd 100644 --- a/src/Arcus.WebApi.Security/Authentication/Certificates/CertificateAuthenticationConfigBuilder.cs +++ b/src/Arcus.WebApi.Security/Authentication/Certificates/CertificateAuthenticationConfigBuilder.cs @@ -137,6 +137,13 @@ private static IX509ValidationLocation GetValidationLocationImplementation(X509V /// public CertificateAuthenticationConfig Build() { + if (_locationAndKeyByRequirement.Count <= 0) + { + throw new InvalidOperationException( + "Cannot build up the certificate authentication validation because there's nothing configured to be validated on the client certificate, " + + $"please configure the certificate validation requirements with methods like {nameof(WithThumbprint)}, {nameof(WithIssuer)}"); + } + return new CertificateAuthenticationConfig(_locationAndKeyByRequirement); } } diff --git a/src/Arcus.WebApi.Security/Authentication/Certificates/Extensions/FilterCollectionExtensions.cs b/src/Arcus.WebApi.Security/Authentication/Certificates/Extensions/FilterCollectionExtensions.cs index 0eff3e52..fbc552ab 100644 --- a/src/Arcus.WebApi.Security/Authentication/Certificates/Extensions/FilterCollectionExtensions.cs +++ b/src/Arcus.WebApi.Security/Authentication/Certificates/Extensions/FilterCollectionExtensions.cs @@ -1,6 +1,7 @@ using System; using Arcus.WebApi.Security.Authentication.Certificates; using GuardNet; +using Microsoft.Extensions.DependencyInjection; // ReSharper disable once CheckNamespace namespace Microsoft.AspNetCore.Mvc.Filters @@ -16,13 +17,14 @@ public static partial class FilterCollectionExtensions /// The current MVC filters of the application. /// /// Thrown when the is null. + [Obsolete("Use the " + nameof(MvcOptionsExtensions.AddCertificateAuthenticationFilter) + " instead via the services.AddControllers(options => options." + nameof(MvcOptionsExtensions.AddCertificateAuthenticationFilter) + "())")] public static FilterCollection AddCertificateAuthentication(this FilterCollection filters) { Guard.NotNull(filters, nameof(filters), "Requires a set of MVC filters to add the certificate authentication MVC filter"); return AddCertificateAuthentication(filters, configureOptions: null); } - + /// /// Adds an certificate authentication MVC filter to the given that authenticates the incoming HTTP request. /// @@ -32,6 +34,7 @@ public static FilterCollection AddCertificateAuthentication(this FilterCollectio /// /// /// Thrown when the is null. + [Obsolete("Use the " + nameof(MvcOptionsExtensions.AddCertificateAuthenticationFilter) + " instead via the services.AddControllers(options => options." + nameof(MvcOptionsExtensions.AddCertificateAuthenticationFilter) + "(...))")] public static FilterCollection AddCertificateAuthentication( this FilterCollection filters, Action configureOptions) diff --git a/src/Arcus.WebApi.Security/Authentication/Certificates/Extensions/MvcOptionsExtensions.cs b/src/Arcus.WebApi.Security/Authentication/Certificates/Extensions/MvcOptionsExtensions.cs new file mode 100644 index 00000000..ea36743d --- /dev/null +++ b/src/Arcus.WebApi.Security/Authentication/Certificates/Extensions/MvcOptionsExtensions.cs @@ -0,0 +1,48 @@ +using System; +using Arcus.WebApi.Security.Authentication.Certificates; +using GuardNet; +using Microsoft.AspNetCore.Mvc; + +namespace Microsoft.Extensions.DependencyInjection +{ + /// + /// Extensions on the related to authentication. + /// + public static class MvcOptionsExtensions + { + /// + /// Adds an certificate authentication MVC filter to the given that authenticates the incoming HTTP request. + /// + /// The current MVC options of the application. + /// + /// Thrown when the is null. + public static MvcOptions AddCertificateAuthenticationFilter(this MvcOptions options) + { + Guard.NotNull(options, nameof(options), "Requires a set of MVC filters to add the certificate authentication MVC filter"); + + return AddCertificateAuthenticationFilter(options, configureOptions: null); + } + + /// + /// Adds an certificate authentication MVC filter to the given that authenticates the incoming HTTP request. + /// + /// The current MVC options of the application. + /// + /// The optional function to configure the set of additional consumer-configurable options to change the behavior of the certificate authentication. + /// + /// + /// Thrown when the is null. + public static MvcOptions AddCertificateAuthenticationFilter( + this MvcOptions options, + Action configureOptions) + { + Guard.NotNull(options, nameof(options), "Requires a set of MVC filters to add the certificate authentication MVC filter"); + + var authOptions = new CertificateAuthenticationOptions(); + configureOptions?.Invoke(authOptions); + + options.Filters.Add(new CertificateAuthenticationFilter(authOptions)); + return options; + } + } +} diff --git a/src/Arcus.WebApi.Tests.Integration/Security/Authentication/CertificateAuthenticationFilterTests.cs b/src/Arcus.WebApi.Tests.Integration/Security/Authentication/CertificateAuthenticationFilterTests.cs index 1791cdb2..a1173e30 100644 --- a/src/Arcus.WebApi.Tests.Integration/Security/Authentication/CertificateAuthenticationFilterTests.cs +++ b/src/Arcus.WebApi.Tests.Integration/Security/Authentication/CertificateAuthenticationFilterTests.cs @@ -23,7 +23,8 @@ using Xunit.Abstractions; using ILogger = Microsoft.Extensions.Logging.ILogger; -// ReSharper disable AccessToDisposedClosure +// Ignore obsolete warnings. +#pragma warning disable CS0618 namespace Arcus.WebApi.Tests.Integration.Security.Authentication { @@ -41,7 +42,7 @@ public CertificateAuthenticationFilterTests(ITestOutputHelper outputWriter) } [Fact] - public async Task AuthorizedRoute_WithCertificateAuthentication_ShouldFailWithUnauthorized_WhenClientCertificateSubjectNameDoesntMatch() + public async Task AuthorizedRoute_WithCertificateAuthenticationOnFilters_ShouldFailWithUnauthorized_WhenClientCertificateSubjectNameDoesntMatch() { // Arrange string subjectKey = "subject", subjectValue = $"subject-{Guid.NewGuid()}"; @@ -76,9 +77,88 @@ public async Task AuthorizedRoute_WithCertificateAuthentication_ShouldFailWithUn } } + [Fact] + public async Task AuthorizedRoute_WithCertificateAuthentication_ShouldFailWithUnauthorized_WhenClientCertificateSubjectNameDoesntMatch() + { + // Arrange + string subjectKey = "subject", subjectValue = $"subject-{Guid.NewGuid()}"; + using (X509Certificate2 clientCertificate = SelfSignedCertificate.CreateWithSubject("unrecognized-subject-name")) + { + var options = new TestApiServerOptions() + .ConfigureServices(services => + { + var certificateValidator = + new CertificateAuthenticationValidator( + new CertificateAuthenticationConfigBuilder() + .WithSubject(X509ValidationLocation.SecretProvider, subjectKey) + .Build()); + + services.AddSecretStore(stores => stores.AddInMemory(subjectKey, subjectValue)) + .AddSingleton(certificateValidator) + .AddClientCertificate(clientCertificate) + .AddMvc(opt => opt.AddCertificateAuthenticationFilter()); + }); + + await using (var server = await TestApiServer.StartNewAsync(options, _logger)) + { + var request = HttpRequestBuilder.Get(NoneAuthenticationController.GetRoute); + + // Act + using (HttpResponseMessage response = await server.SendAsync(request)) + { + // Assert + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + } + } + } + [Theory] [InlineData("", false)] [InlineData("thumbprint-noise", true)] + public async Task AuthorizedRoute_WithCertificateAuthenticationOnFilters_ShouldFailWithUnauthorized_WhenClientCertificateThumbprintDoesntMatch( + string thumbprintNoise, + bool expected) + { + // Arrange + using (X509Certificate2 clientCertificate = SelfSignedCertificate.Create()) + { + const string thumbprintKey = "thumbprint"; + + var options = new TestApiServerOptions() + .ConfigureServices(services => + { + var certificateValidator = + new CertificateAuthenticationValidator( + new CertificateAuthenticationConfigBuilder() + .WithThumbprint(X509ValidationLocation.SecretProvider, thumbprintKey) + .Build()); + + services.AddSecretStore(stores => stores.AddInMemory(thumbprintKey, clientCertificate.Thumbprint + thumbprintNoise)) + .AddSingleton(certificateValidator) + .AddClientCertificate(clientCertificate) + .AddMvc(opt => opt.Filters.AddCertificateAuthentication()); + }); + + await using (var server = await TestApiServer.StartNewAsync(options, _logger)) + { + var request = HttpRequestBuilder.Get(NoneAuthenticationController.GetRoute); + + // Act + using (HttpResponseMessage response = await server.SendAsync(request)) + { + // Assert + Assert.True( + (HttpStatusCode.Unauthorized == response.StatusCode) == expected, + $"Response HTTP status code {(expected ? "should" : "shouldn't")} be 'Unauthorized' but was '{response.StatusCode}'"); + } + } + } + } + + [Theory] + [InlineData("", false)] + [InlineData("thumbprint-noise", true)] public async Task AuthorizedRoute_WithCertificateAuthentication_ShouldFailWithUnauthorized_WhenClientCertificateThumbprintDoesntMatch( string thumbprintNoise, bool expected) @@ -100,6 +180,56 @@ public async Task AuthorizedRoute_WithCertificateAuthentication_ShouldFailWithUn services.AddSecretStore(stores => stores.AddInMemory(thumbprintKey, clientCertificate.Thumbprint + thumbprintNoise)) .AddSingleton(certificateValidator) .AddClientCertificate(clientCertificate) + .AddMvc(opt => opt.AddCertificateAuthenticationFilter()); + }); + + await using (var server = await TestApiServer.StartNewAsync(options, _logger)) + { + var request = HttpRequestBuilder.Get(NoneAuthenticationController.GetRoute); + + // Act + using (HttpResponseMessage response = await server.SendAsync(request)) + { + // Assert + Assert.True( + (HttpStatusCode.Unauthorized == response.StatusCode) == expected, + $"Response HTTP status code {(expected ? "should" : "shouldn't")} be 'Unauthorized' but was '{response.StatusCode}'"); + } + } + } + } + + [Theory] + [InlineData("known-subject", "known-issuername", false)] + [InlineData("unrecognizedSubjectName", "known-issuername", true)] + [InlineData("known-subject", "unrecognizedIssuerName", true)] + [InlineData("unrecognizedSubjectName", "unrecognizedIssuerName", true)] + public async Task AuthorizedRoute_WithCertificateAuthenticationViaSecretProviderOnFilters_ShouldFailWithUnauthorized_WhenAnyClientCertificateValidationDoesntSucceeds( + string subjectValue, + string issuerValue, + bool expected) + { + // Arrange + const string subjectKey = "subject", issuerKey = "issuer"; + using (X509Certificate2 clientCertificate = SelfSignedCertificate.CreateWithIssuerAndSubjectName(issuerValue, subjectValue)) + { + var options = new TestApiServerOptions() + .ConfigureServices(services => + { + var certificateValidator = + new CertificateAuthenticationValidator( + new CertificateAuthenticationConfigBuilder() + .WithSubject(X509ValidationLocation.SecretProvider, subjectKey) + .WithIssuer(X509ValidationLocation.SecretProvider, issuerKey) + .Build()); + + services.AddClientCertificate(clientCertificate) + .AddSingleton(certificateValidator) + .AddSecretStore(stores => stores.AddInMemory(new Dictionary + { + [subjectKey] = "CN=known-subject", + [issuerKey] = "CN=known-issuername" + })) .AddMvc(opt => opt.Filters.AddCertificateAuthentication()); }); @@ -150,7 +280,7 @@ public async Task AuthorizedRoute_WithCertificateAuthenticationViaSecretProvider [subjectKey] = "CN=known-subject", [issuerKey] = "CN=known-issuername" })) - .AddMvc(opt => opt.Filters.AddCertificateAuthentication()); + .AddMvc(opt => opt.AddCertificateAuthenticationFilter()); }); await using (var server = await TestApiServer.StartNewAsync(options, _logger)) @@ -174,6 +304,56 @@ public async Task AuthorizedRoute_WithCertificateAuthenticationViaSecretProvider [InlineData("unrecognizedSubjectName", "known-issuername", true)] [InlineData("known-subject", "unrecognizedIssuerName", true)] [InlineData("unrecognizedSubjectName", "unrecognizedIssuerName", true)] + public async Task AuthorizedRoute_WithCertificateAuthenticationViaConfigurationOnFilters_ShouldFailWithUnauthorized_WhenAnyClientCertificateValidationDoesntSucceeds( + string subjectValue, + string issuerValue, + bool expected) + { + // Arrange + const string subjectKey = "subject", issuerKey = "issuer"; + using (X509Certificate2 clientCertificate = SelfSignedCertificate.CreateWithIssuerAndSubjectName(issuerValue, subjectValue)) + { + var options = new TestApiServerOptions() + .ConfigureAppConfiguration(config => config.AddInMemoryCollection(new [] + { + new KeyValuePair(subjectKey, "CN=known-subject"), + new KeyValuePair(issuerKey, "CN=known-issuername") + })) + .ConfigureServices(services => + { + var certificateValidator = + new CertificateAuthenticationValidator( + new CertificateAuthenticationConfigBuilder() + .WithSubject(X509ValidationLocation.Configuration, subjectKey) + .WithIssuer(X509ValidationLocation.Configuration, issuerKey) + .Build()); + + services.AddSingleton(certificateValidator) + .AddClientCertificate(clientCertificate) + .AddMvc(opt => opt.Filters.AddCertificateAuthentication()); + }); + + await using (var server = await TestApiServer.StartNewAsync(options, _logger)) + { + var request = HttpRequestBuilder.Get(NoneAuthenticationController.GetRoute); + + // Act + using (HttpResponseMessage response = await server.SendAsync(request)) + { + // Assert + Assert.True( + (HttpStatusCode.Unauthorized == response.StatusCode) == expected, + $"Response HTTP status code {(expected ? "should" : "shouldn't")} be 'Unauthorized' but was '{response.StatusCode}'"); + } + } + } + } + + [Theory] + [InlineData("known-subject", "known-issuername", false)] + [InlineData("unrecognizedSubjectName", "known-issuername", true)] + [InlineData("known-subject", "unrecognizedIssuerName", true)] + [InlineData("unrecognizedSubjectName", "unrecognizedIssuerName", true)] public async Task AuthorizedRoute_WithCertificateAuthenticationViaConfiguration_ShouldFailWithUnauthorized_WhenAnyClientCertificateValidationDoesntSucceeds( string subjectValue, string issuerValue, @@ -200,6 +380,56 @@ public async Task AuthorizedRoute_WithCertificateAuthenticationViaConfiguration_ services.AddSingleton(certificateValidator) .AddClientCertificate(clientCertificate) + .AddMvc(opt => opt.AddCertificateAuthenticationFilter()); + }); + + await using (var server = await TestApiServer.StartNewAsync(options, _logger)) + { + var request = HttpRequestBuilder.Get(NoneAuthenticationController.GetRoute); + + // Act + using (HttpResponseMessage response = await server.SendAsync(request)) + { + // Assert + Assert.True( + (HttpStatusCode.Unauthorized == response.StatusCode) == expected, + $"Response HTTP status code {(expected ? "should" : "shouldn't")} be 'Unauthorized' but was '{response.StatusCode}'"); + } + } + } + } + + [Theory] + [InlineData("known-subject", "known-issuername", false)] + [InlineData("unrecognizedSubjectName", "known-issuername", true)] + [InlineData("known-subject", "unrecognizedIssuerName", true)] + [InlineData("unrecognizedSubjectName", "unrecognizedIssuerName", true)] + public async Task AuthorizedRoute_WithCertificateAuthenticationViaConfigurationAndSecretProviderOnFilters_ShouldFailWithUnauthorized_WhenAnyClientCertificateValidationDoesntSucceeds( + string subjectValue, + string issuerValue, + bool expected) + { + // Arrange + const string subjectKey = "subject", issuerKey = "issuer"; + using (X509Certificate2 clientCertificate = SelfSignedCertificate.CreateWithIssuerAndSubjectName(issuerValue, subjectValue)) + { + var options = new TestApiServerOptions() + .ConfigureAppConfiguration(config => config.AddInMemoryCollection(new [] + { + new KeyValuePair(subjectKey, "CN=known-subject") + })) + .ConfigureServices(services => + { + var certificateValidator = + new CertificateAuthenticationValidator( + new CertificateAuthenticationConfigBuilder() + .WithSubject(X509ValidationLocation.Configuration, subjectKey) + .WithIssuer(X509ValidationLocation.SecretProvider, issuerKey) + .Build()); + + services.AddSecretStore(stores => stores.AddInMemory(issuerKey, "CN=known-issuername")) + .AddClientCertificate(clientCertificate) + .AddSingleton(certificateValidator) .AddMvc(opt => opt.Filters.AddCertificateAuthentication()); }); @@ -250,7 +480,7 @@ public async Task AuthorizedRoute_WithCertificateAuthenticationViaConfigurationA services.AddSecretStore(stores => stores.AddInMemory(issuerKey, "CN=known-issuername")) .AddClientCertificate(clientCertificate) .AddSingleton(certificateValidator) - .AddMvc(opt => opt.Filters.AddCertificateAuthentication()); + .AddMvc(opt => opt.AddCertificateAuthenticationFilter()); }); await using (var server = await TestApiServer.StartNewAsync(options, _logger)) @@ -270,7 +500,7 @@ public async Task AuthorizedRoute_WithCertificateAuthenticationViaConfigurationA } [Fact] - public async Task AuthorizedRoute_WithCertificateAuthentication_ShouldFailOnInvalidBase64Format() + public async Task AuthorizedRoute_WithCertificateAuthenticationOnFilters_ShouldFailOnInvalidBase64Format() { // Arrange var options = new TestApiServerOptions() @@ -300,8 +530,39 @@ public async Task AuthorizedRoute_WithCertificateAuthentication_ShouldFailOnInva } } + [Fact] + public async Task AuthorizedRoute_WithCertificateAuthentication_ShouldFailOnInvalidBase64Format() + { + // Arrange + var options = new TestApiServerOptions() + .ConfigureServices(services => + { + var certificateValidator = + new CertificateAuthenticationValidator( + new CertificateAuthenticationConfigBuilder() + .Build()); + + services.AddSingleton(certificateValidator) + .AddMvc(opt => opt.AddCertificateAuthenticationFilter()); + }); + + await using (var server = await TestApiServer.StartNewAsync(options, _logger)) + { + var request = HttpRequestBuilder + .Get(NoneAuthenticationController.GetRoute) + .WithHeader("X-ARR-ClientCert", "something not even close to an client certificate export"); + + // Act + using (HttpResponseMessage response = await server.SendAsync(request)) + { + // Assert + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + } + } + [Fact] - public async Task AuthorizedRoute_WithCertificateAuthenticationInHeader_ShouldSucceed() + public async Task AuthorizedRoute_WithCertificateAuthenticationInHeaderOnFilters_ShouldSucceed() { // Arrange const string subjectKey = "subject", issuerKey = "issuer"; @@ -342,12 +603,55 @@ public async Task AuthorizedRoute_WithCertificateAuthenticationInHeader_ShouldSu } } } + + [Fact] + public async Task AuthorizedRoute_WithCertificateAuthenticationInHeader_ShouldSucceed() + { + // Arrange + const string subjectKey = "subject", issuerKey = "issuer"; + using (X509Certificate2 clientCertificate = SelfSignedCertificate.CreateWithIssuerAndSubjectName("known-issuername", "known-subject")) + { + var options = new TestApiServerOptions() + .ConfigureAppConfiguration(config => config.AddInMemoryCollection(new [] + { + new KeyValuePair(subjectKey, "CN=known-subject") + })) + .ConfigureServices(services => + { + var certificateValidator = + new CertificateAuthenticationValidator( + new CertificateAuthenticationConfigBuilder() + .WithSubject(X509ValidationLocation.Configuration, subjectKey) + .WithIssuer(X509ValidationLocation.SecretProvider, issuerKey) + .Build()); + + services.AddSecretStore(stores => stores.AddInMemory(issuerKey, "CN=known-issuername")) + .AddSingleton(certificateValidator) + .AddMvc(opt => opt.AddCertificateAuthenticationFilter()); + }); + + await using (var server = await TestApiServer.StartNewAsync(options, _logger)) + { + string base64String = Convert.ToBase64String(clientCertificate.Export(X509ContentType.Pkcs12), Base64FormattingOptions.None); + var request = HttpRequestBuilder + .Get(NoneAuthenticationController.GetRoute) + .WithHeader("X-ARR-ClientCert", base64String); + + // Act + using (HttpResponseMessage response = await server.SendAsync(request)) + { + // Assert + Assert.NotEqual(HttpStatusCode.Unauthorized, response.StatusCode); + } + } + } + } [Theory] [InlineData(BypassOnMethodController.CertificateRoute)] [InlineData(BypassCertificateController.BypassOverAuthenticationRoute)] [InlineData(AllowAnonymousCertificateController.Route)] - public async Task CertificateAuthorizedRoute_WithBypassAttribute_SkipsAuthentication(string route) + public async Task CertificateAuthorizedRoute_WithBypassAttributeOnFilters_SkipsAuthentication(string route) { // Arrange const string issuerKey = "issuer"; @@ -381,9 +685,48 @@ public async Task CertificateAuthorizedRoute_WithBypassAttribute_SkipsAuthentica } } } + + [Theory] + [InlineData(BypassOnMethodController.CertificateRoute)] + [InlineData(BypassCertificateController.BypassOverAuthenticationRoute)] + [InlineData(AllowAnonymousCertificateController.Route)] + public async Task CertificateAuthorizedRoute_WithBypassAttribute_SkipsAuthentication(string route) + { + // Arrange + const string issuerKey = "issuer"; + using (X509Certificate2 clientCertificate = SelfSignedCertificate.CreateWithIssuerAndSubjectName("issuer", "subject")) + { + var options = new TestApiServerOptions() + .ConfigureServices(services => + { + var certificateValidator = + new CertificateAuthenticationValidator( + new CertificateAuthenticationConfigBuilder() + .WithIssuer(X509ValidationLocation.SecretProvider, issuerKey) + .Build()); + + services.AddSecretStore(stores => stores.AddInMemory(issuerKey, "CN=issuer")) + .AddClientCertificate(clientCertificate) + .AddSingleton(certificateValidator) + .AddMvc(opt => opt.AddCertificateAuthenticationFilter()); + }); + + await using (var server = await TestApiServer.StartNewAsync(options, _logger)) + { + var request = HttpRequestBuilder.Get(route); + + // Act + using (HttpResponseMessage response = await server.SendAsync(request)) + { + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + } + } + } [Fact] - public async Task SharedAccessKeyAuthorizedRoute_DoesntEmitSecurityEventsByDefault_RunsAuthentication() + public async Task SharedAccessKeyAuthorizedRoute_DoesntEmitSecurityEventsByDefaultOnFilters_RunsAuthentication() { // Arrange const string issuerKey = "issuer"; @@ -422,10 +765,50 @@ public async Task SharedAccessKeyAuthorizedRoute_DoesntEmitSecurityEventsByDefau } } + [Fact] + public async Task SharedAccessKeyAuthorizedRoute_DoesntEmitSecurityEventsByDefault_RunsAuthentication() + { + // Arrange + const string issuerKey = "issuer"; + var spySink = new InMemorySink(); + var options = new TestApiServerOptions() + .ConfigureServices(services => + { + var certificateValidator = + new CertificateAuthenticationValidator( + new CertificateAuthenticationConfigBuilder() + .WithIssuer(X509ValidationLocation.SecretProvider, issuerKey) + .Build()); + + services.AddSecretStore(stores => stores.AddInMemory(issuerKey, "CN=issuer")) + .AddSingleton(certificateValidator) + .AddMvc(opt => opt.AddCertificateAuthenticationFilter()); + }) + .ConfigureHost(host => host.UseSerilog((context, config) => config.WriteTo.Sink(spySink))); + + await using (var server = await TestApiServer.StartNewAsync(options, _logger)) + { + var request = HttpRequestBuilder.Get(NoneAuthenticationController.GetRoute); + + // Act + using (HttpResponseMessage response = await server.SendAsync(request)) + { + // Assert + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + IEnumerable logEvents = spySink.DequeueLogEvents(); + Assert.DoesNotContain(logEvents, logEvent => + { + string message = logEvent.RenderMessage(); + return message.Contains("EventType") && message.Contains("Security"); + }); + } + } + } + [Theory] [InlineData(false)] [InlineData(true)] - public async Task SharedAccessKeyAuthorizedRoute_EmitsSecurityEventsWhenRequested_RunsAuthentication(bool emitsSecurityEvents) + public async Task SharedAccessKeyAuthorizedRoute_EmitsSecurityEventsWhenRequestedOnFilters_RunsAuthentication(bool emitsSecurityEvents) { // Arrange const string issuerKey = "issuer"; @@ -466,5 +849,51 @@ public async Task SharedAccessKeyAuthorizedRoute_EmitsSecurityEventsWhenRequeste } } } + + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task SharedAccessKeyAuthorizedRoute_EmitsSecurityEventsWhenRequested_RunsAuthentication(bool emitsSecurityEvents) + { + // Arrange + const string issuerKey = "issuer"; + var spySink = new InMemorySink(); + var options = new TestApiServerOptions() + .ConfigureServices(services => + { + var certificateValidator = + new CertificateAuthenticationValidator( + new CertificateAuthenticationConfigBuilder() + .WithIssuer(X509ValidationLocation.SecretProvider, issuerKey) + .Build()); + + services.AddSecretStore(stores => stores.AddInMemory(issuerKey, "CN=issuer")) + .AddSingleton(certificateValidator) + .AddMvc(opt => opt.AddCertificateAuthenticationFilter(authOptions => + { + authOptions.EmitSecurityEvents = emitsSecurityEvents; + })); + }) + .ConfigureHost(host => host.UseSerilog((context, config) => config.WriteTo.Sink(spySink))); + + await using (var server = await TestApiServer.StartNewAsync(options, _logger)) + { + var request = HttpRequestBuilder.Get(NoneAuthenticationController.GetRoute); + + // Act + using (HttpResponseMessage response = await server.SendAsync(request)) + { + // Assert + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + IEnumerable logEvents = spySink.DequeueLogEvents(); + Assert.True(emitsSecurityEvents == logEvents.Any(logEvent => + { + string message = logEvent.RenderMessage(); + return message.Contains("EventType") && message.Contains("Security"); + })); + } + } + } } } diff --git a/src/Arcus.WebApi.Tests.Unit/Security/Authentication/CertificateAuthenticationFilterTests.cs b/src/Arcus.WebApi.Tests.Unit/Security/Authentication/CertificateAuthenticationFilterTests.cs new file mode 100644 index 00000000..cee0a4c7 --- /dev/null +++ b/src/Arcus.WebApi.Tests.Unit/Security/Authentication/CertificateAuthenticationFilterTests.cs @@ -0,0 +1,16 @@ +using System; +using Arcus.WebApi.Security.Authentication.Certificates; +using Xunit; + +namespace Arcus.WebApi.Tests.Unit.Security.Authentication +{ + public class CertificateAuthenticationFilterTests + { + [Fact] + public void Create_WithoutCertificateAuthenticationOptions_Fails() + { + Assert.ThrowsAny( + () => new CertificateAuthenticationFilter(options: null)); + } + } +} From f1027b0070ae0ed0759610fb1a4ac8324c124223 Mon Sep 17 00:00:00 2001 From: stijnmoreels <9039753+stijnmoreels@users.noreply.github.com> Date: Mon, 4 Jul 2022 11:10:14 +0200 Subject: [PATCH 2/6] pr-style: remove blank line --- .../Authentication/CertificateAuthenticationFilterTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Arcus.WebApi.Tests.Integration/Security/Authentication/CertificateAuthenticationFilterTests.cs b/src/Arcus.WebApi.Tests.Integration/Security/Authentication/CertificateAuthenticationFilterTests.cs index a1173e30..c76a2cb1 100644 --- a/src/Arcus.WebApi.Tests.Integration/Security/Authentication/CertificateAuthenticationFilterTests.cs +++ b/src/Arcus.WebApi.Tests.Integration/Security/Authentication/CertificateAuthenticationFilterTests.cs @@ -850,7 +850,6 @@ public async Task SharedAccessKeyAuthorizedRoute_EmitsSecurityEventsWhenRequeste } } - [Theory] [InlineData(false)] [InlineData(true)] From 878ed7db6a6ec9ae8ccf129b35b3dea4af7b8b51 Mon Sep 17 00:00:00 2001 From: stijnmoreels <9039753+stijnmoreels@users.noreply.github.com> Date: Mon, 4 Jul 2022 11:21:29 +0200 Subject: [PATCH 3/6] pr-add: test for guarded cert auth config builder --- ...ificateAuthenticationConfigBuilderTests.cs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 src/Arcus.WebApi.Tests.Unit/Security/Authentication/CertificateAuthenticationConfigBuilderTests.cs diff --git a/src/Arcus.WebApi.Tests.Unit/Security/Authentication/CertificateAuthenticationConfigBuilderTests.cs b/src/Arcus.WebApi.Tests.Unit/Security/Authentication/CertificateAuthenticationConfigBuilderTests.cs new file mode 100644 index 00000000..ad7fe81f --- /dev/null +++ b/src/Arcus.WebApi.Tests.Unit/Security/Authentication/CertificateAuthenticationConfigBuilderTests.cs @@ -0,0 +1,19 @@ +using System; +using Arcus.WebApi.Security.Authentication.Certificates; +using Xunit; + +namespace Arcus.WebApi.Tests.Unit.Security.Authentication +{ + public class CertificateAuthenticationConfigBuilderTests + { + [Fact] + public void Build_WithoutAnything_Fails() + { + // Arrange + var builder = new CertificateAuthenticationConfigBuilder(); + + // Act / Assert + Assert.Throws(() => builder.Build()); + } + } +} From 80e64817f6946cb20143710845971a54c3616952 Mon Sep 17 00:00:00 2001 From: stijnmoreels <9039753+stijnmoreels@users.noreply.github.com> Date: Mon, 4 Jul 2022 11:35:30 +0200 Subject: [PATCH 4/6] pr-fix: add ignored cert auth config builder --- .../Authentication/CertificateAuthenticationFilterTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Arcus.WebApi.Tests.Integration/Security/Authentication/CertificateAuthenticationFilterTests.cs b/src/Arcus.WebApi.Tests.Integration/Security/Authentication/CertificateAuthenticationFilterTests.cs index c76a2cb1..89f937bb 100644 --- a/src/Arcus.WebApi.Tests.Integration/Security/Authentication/CertificateAuthenticationFilterTests.cs +++ b/src/Arcus.WebApi.Tests.Integration/Security/Authentication/CertificateAuthenticationFilterTests.cs @@ -509,6 +509,7 @@ public async Task AuthorizedRoute_WithCertificateAuthenticationOnFilters_ShouldF var certificateValidator = new CertificateAuthenticationValidator( new CertificateAuthenticationConfigBuilder() + .WithSubject(X509ValidationLocation.Configuration, "ignored-subject") .Build()); services.AddSingleton(certificateValidator) @@ -540,6 +541,7 @@ public async Task AuthorizedRoute_WithCertificateAuthentication_ShouldFailOnInva var certificateValidator = new CertificateAuthenticationValidator( new CertificateAuthenticationConfigBuilder() + .WithSubject(X509ValidationLocation.Configuration, "ignored-subject") .Build()); services.AddSingleton(certificateValidator) From 2fae5d6d279d00c89dfe9f513b35200b6f12f956 Mon Sep 17 00:00:00 2001 From: stijnmoreels <9039753+stijnmoreels@users.noreply.github.com> Date: Tue, 5 Jul 2022 09:39:48 +0200 Subject: [PATCH 5/6] pr-fix: make mvc options extensions partial --- .../Certificates/Extensions/MvcOptionsExtensions.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Arcus.WebApi.Security/Authentication/Certificates/Extensions/MvcOptionsExtensions.cs b/src/Arcus.WebApi.Security/Authentication/Certificates/Extensions/MvcOptionsExtensions.cs index ea36743d..e75c61cf 100644 --- a/src/Arcus.WebApi.Security/Authentication/Certificates/Extensions/MvcOptionsExtensions.cs +++ b/src/Arcus.WebApi.Security/Authentication/Certificates/Extensions/MvcOptionsExtensions.cs @@ -3,12 +3,13 @@ using GuardNet; using Microsoft.AspNetCore.Mvc; +// ReSharper disable once CheckNamespace namespace Microsoft.Extensions.DependencyInjection { /// /// Extensions on the related to authentication. /// - public static class MvcOptionsExtensions + public static partial class MvcOptionsExtensions { /// /// Adds an certificate authentication MVC filter to the given that authenticates the incoming HTTP request. From 47f7953147621a838aa6cb78049a412647191b4b Mon Sep 17 00:00:00 2001 From: stijnmoreels <9039753+stijnmoreels@users.noreply.github.com> Date: Tue, 5 Jul 2022 10:02:20 +0200 Subject: [PATCH 6/6] pr-fix: update with 'AddControllers' usage --- .../CertificateAuthenticationFilterTests.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Arcus.WebApi.Tests.Integration/Security/Authentication/CertificateAuthenticationFilterTests.cs b/src/Arcus.WebApi.Tests.Integration/Security/Authentication/CertificateAuthenticationFilterTests.cs index 89f937bb..782b821d 100644 --- a/src/Arcus.WebApi.Tests.Integration/Security/Authentication/CertificateAuthenticationFilterTests.cs +++ b/src/Arcus.WebApi.Tests.Integration/Security/Authentication/CertificateAuthenticationFilterTests.cs @@ -96,7 +96,7 @@ public async Task AuthorizedRoute_WithCertificateAuthentication_ShouldFailWithUn services.AddSecretStore(stores => stores.AddInMemory(subjectKey, subjectValue)) .AddSingleton(certificateValidator) .AddClientCertificate(clientCertificate) - .AddMvc(opt => opt.AddCertificateAuthenticationFilter()); + .AddControllers(opt => opt.AddCertificateAuthenticationFilter()); }); await using (var server = await TestApiServer.StartNewAsync(options, _logger)) @@ -180,7 +180,7 @@ public async Task AuthorizedRoute_WithCertificateAuthentication_ShouldFailWithUn services.AddSecretStore(stores => stores.AddInMemory(thumbprintKey, clientCertificate.Thumbprint + thumbprintNoise)) .AddSingleton(certificateValidator) .AddClientCertificate(clientCertificate) - .AddMvc(opt => opt.AddCertificateAuthenticationFilter()); + .AddControllers(opt => opt.AddCertificateAuthenticationFilter()); }); await using (var server = await TestApiServer.StartNewAsync(options, _logger)) @@ -280,7 +280,7 @@ public async Task AuthorizedRoute_WithCertificateAuthenticationViaSecretProvider [subjectKey] = "CN=known-subject", [issuerKey] = "CN=known-issuername" })) - .AddMvc(opt => opt.AddCertificateAuthenticationFilter()); + .AddControllers(opt => opt.AddCertificateAuthenticationFilter()); }); await using (var server = await TestApiServer.StartNewAsync(options, _logger)) @@ -380,7 +380,7 @@ public async Task AuthorizedRoute_WithCertificateAuthenticationViaConfiguration_ services.AddSingleton(certificateValidator) .AddClientCertificate(clientCertificate) - .AddMvc(opt => opt.AddCertificateAuthenticationFilter()); + .AddControllers(opt => opt.AddCertificateAuthenticationFilter()); }); await using (var server = await TestApiServer.StartNewAsync(options, _logger)) @@ -480,7 +480,7 @@ public async Task AuthorizedRoute_WithCertificateAuthenticationViaConfigurationA services.AddSecretStore(stores => stores.AddInMemory(issuerKey, "CN=known-issuername")) .AddClientCertificate(clientCertificate) .AddSingleton(certificateValidator) - .AddMvc(opt => opt.AddCertificateAuthenticationFilter()); + .AddControllers(opt => opt.AddCertificateAuthenticationFilter()); }); await using (var server = await TestApiServer.StartNewAsync(options, _logger)) @@ -545,7 +545,7 @@ public async Task AuthorizedRoute_WithCertificateAuthentication_ShouldFailOnInva .Build()); services.AddSingleton(certificateValidator) - .AddMvc(opt => opt.AddCertificateAuthenticationFilter()); + .AddControllers(opt => opt.AddCertificateAuthenticationFilter()); }); await using (var server = await TestApiServer.StartNewAsync(options, _logger)) @@ -629,7 +629,7 @@ public async Task AuthorizedRoute_WithCertificateAuthenticationInHeader_ShouldSu services.AddSecretStore(stores => stores.AddInMemory(issuerKey, "CN=known-issuername")) .AddSingleton(certificateValidator) - .AddMvc(opt => opt.AddCertificateAuthenticationFilter()); + .AddControllers(opt => opt.AddCertificateAuthenticationFilter()); }); await using (var server = await TestApiServer.StartNewAsync(options, _logger)) @@ -710,7 +710,7 @@ public async Task CertificateAuthorizedRoute_WithBypassAttribute_SkipsAuthentica services.AddSecretStore(stores => stores.AddInMemory(issuerKey, "CN=issuer")) .AddClientCertificate(clientCertificate) .AddSingleton(certificateValidator) - .AddMvc(opt => opt.AddCertificateAuthenticationFilter()); + .AddControllers(opt => opt.AddCertificateAuthenticationFilter()); }); await using (var server = await TestApiServer.StartNewAsync(options, _logger)) @@ -784,7 +784,7 @@ public async Task SharedAccessKeyAuthorizedRoute_DoesntEmitSecurityEventsByDefau services.AddSecretStore(stores => stores.AddInMemory(issuerKey, "CN=issuer")) .AddSingleton(certificateValidator) - .AddMvc(opt => opt.AddCertificateAuthenticationFilter()); + .AddControllers(opt => opt.AddCertificateAuthenticationFilter()); }) .ConfigureHost(host => host.UseSerilog((context, config) => config.WriteTo.Sink(spySink))); @@ -871,7 +871,7 @@ public async Task SharedAccessKeyAuthorizedRoute_EmitsSecurityEventsWhenRequeste services.AddSecretStore(stores => stores.AddInMemory(issuerKey, "CN=issuer")) .AddSingleton(certificateValidator) - .AddMvc(opt => opt.AddCertificateAuthenticationFilter(authOptions => + .AddControllers(opt => opt.AddCertificateAuthenticationFilter(authOptions => { authOptions.EmitSecurityEvents = emitsSecurityEvents; }));