From fb26a378335be637d7fb448f4981ec3543d98727 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Mon, 29 Mar 2021 15:23:41 -0700 Subject: [PATCH 1/4] Update CertificateAuthenticationHandler.cs --- .../src/CertificateAuthenticationHandler.cs | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/Security/Authentication/Certificate/src/CertificateAuthenticationHandler.cs b/src/Security/Authentication/Certificate/src/CertificateAuthenticationHandler.cs index 8d5281847077..d9565d494b5e 100644 --- a/src/Security/Authentication/Certificate/src/CertificateAuthenticationHandler.cs +++ b/src/Security/Authentication/Certificate/src/CertificateAuthenticationHandler.cs @@ -79,6 +79,17 @@ protected override async Task HandleAuthenticateAsync() } var result = await ValidateCertificateAsync(clientCertificate); + + // Invoke the failed handler if validation failed, before updating the cache + if (result.Failure != null) + { + var authenticationFailedContext = await HandleFailureAsync(ex); + if (authenticationFailedContext.Result != null) + { + result = authenticationFailedContext.Result; + } + } + if (_cache != null) { _cache.Put(Context, clientCertificate, result); @@ -87,12 +98,7 @@ protected override async Task HandleAuthenticateAsync() } catch (Exception ex) { - var authenticationFailedContext = new CertificateAuthenticationFailedContext(Context, Scheme, Options) - { - Exception = ex - }; - - await Events.AuthenticationFailed(authenticationFailedContext); + var authenticationFailedContext = await HandleFailureAsync(ex); if (authenticationFailedContext.Result != null) { return authenticationFailedContext.Result; @@ -102,6 +108,17 @@ protected override async Task HandleAuthenticateAsync() } } + private async Task HandleFailureAsync(Exception error) + { + var authenticationFailedContext = new CertificateAuthenticationFailedContext(Context, Scheme, Options) + { + Exception = error + }; + + await Events.AuthenticationFailed(authenticationFailedContext); + return authenticationFailedContext; + } + private async Task ValidateCertificateAsync(X509Certificate2 clientCertificate) { // If we have a self signed cert, and they're not allowed, exit early and not bother with From a19f014ab81b67f54a8d2732dc589448d5e17da2 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Mon, 29 Mar 2021 15:28:40 -0700 Subject: [PATCH 2/4] Update JwtBearerEvents.cs --- src/Security/Authentication/JwtBearer/src/JwtBearerEvents.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Security/Authentication/JwtBearer/src/JwtBearerEvents.cs b/src/Security/Authentication/JwtBearer/src/JwtBearerEvents.cs index be8700528c55..6fc402ad1f60 100755 --- a/src/Security/Authentication/JwtBearer/src/JwtBearerEvents.cs +++ b/src/Security/Authentication/JwtBearer/src/JwtBearerEvents.cs @@ -12,7 +12,7 @@ namespace Microsoft.AspNetCore.Authentication.JwtBearer public class JwtBearerEvents { /// - /// Invoked if exceptions are thrown during request processing. The exceptions will be re-thrown after this event unless suppressed. + /// Invoked if authentication fails during request processing. The exceptions will be re-thrown after this event unless suppressed. /// public Func OnAuthenticationFailed { get; set; } = context => Task.CompletedTask; From 27b4cd4eca75d317b239a1238244812a3d8aedaa Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Mon, 29 Mar 2021 15:46:31 -0700 Subject: [PATCH 3/4] Update CertificateTests.cs --- .../Authentication/test/CertificateTests.cs | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/Security/Authentication/test/CertificateTests.cs b/src/Security/Authentication/test/CertificateTests.cs index fc1dd46e0b1c..6b805aaa623d 100644 --- a/src/Security/Authentication/test/CertificateTests.cs +++ b/src/Security/Authentication/test/CertificateTests.cs @@ -284,6 +284,30 @@ public async Task VerifyUntrustedClientCertEndsUpInForbidden() var response = await server.CreateClient().GetAsync("https://example.com/"); Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); } + + [Fact] + public async Task VerifyValidationFailureCanBeHandled() + { + var failCalled = false; + using var host = await CreateHost( + new CertificateAuthenticationOptions + { + Events = new CertificateAuthenticationEvents() + { + OnAuthenticationFailed = context => + { + context.Fail("Validation failed: " + context.Exception); + failCalled = true; + return Task.CompletedTask; + } + } + }, Certificates.SignedClient); + + using var server = host.GetTestServer(); + var response = await server.CreateClient().GetAsync("https://example.com/"); + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + Assert.True(failCalled); + } [Fact] public async Task VerifyClientCertWithUntrustedRootAndTrustedChainEndsUpInForbidden() @@ -752,7 +776,6 @@ private static async Task CreateHost( await host.StartAsync(); - var server = host.GetTestServer(); server.BaseAddress = baseAddress; return host; From 51debff3b718a310a21de3a0a059454dc2e5f0c8 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Mon, 29 Mar 2021 16:00:17 -0700 Subject: [PATCH 4/4] Fix --- .../Certificate/src/CertificateAuthenticationHandler.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Security/Authentication/Certificate/src/CertificateAuthenticationHandler.cs b/src/Security/Authentication/Certificate/src/CertificateAuthenticationHandler.cs index d9565d494b5e..0ffeeb74ecc6 100644 --- a/src/Security/Authentication/Certificate/src/CertificateAuthenticationHandler.cs +++ b/src/Security/Authentication/Certificate/src/CertificateAuthenticationHandler.cs @@ -83,7 +83,7 @@ protected override async Task HandleAuthenticateAsync() // Invoke the failed handler if validation failed, before updating the cache if (result.Failure != null) { - var authenticationFailedContext = await HandleFailureAsync(ex); + var authenticationFailedContext = await HandleFailureAsync(result.Failure); if (authenticationFailedContext.Result != null) { result = authenticationFailedContext.Result;