From d9bb915983cde864b1cc76efe3c320a6eae81f4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Tue, 7 Nov 2023 16:07:32 +0100 Subject: [PATCH] Update the ASP.NET Core/OWIN integrations to allow returning authentication tickets with a null or empty principal --- .../OpenIddictClientAspNetCoreHandler.cs | 16 ++++++++++------ .../OpenIddictClientOwinHandler.cs | 9 ++------- .../OpenIddictServerAspNetCoreHandler.cs | 12 ++++-------- .../OpenIddictServerOwinHandler.cs | 13 ++++--------- .../OpenIddictValidationAspNetCoreHandler.cs | 13 +++++-------- .../OpenIddictValidationOwinHandler.cs | 13 ++++--------- 6 files changed, 29 insertions(+), 47 deletions(-) diff --git a/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandler.cs b/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandler.cs index 2d546cde0..3aa889100 100644 --- a/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandler.cs +++ b/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandler.cs @@ -135,6 +135,14 @@ protected override async Task HandleAuthenticateAsync() else if (context.IsRejected) { + // Note: the missing_token error is special-cased to indicate to ASP.NET Core + // that no authentication result could be produced due to the lack of token. + // This also helps reducing the logging noise when no token is specified. + if (string.Equals(context.Error, Errors.MissingToken, StringComparison.Ordinal)) + { + return AuthenticateResult.NoResult(); + } + var properties = new AuthenticationProperties(new Dictionary { [Properties.Error] = context.Error, @@ -147,11 +155,6 @@ protected override async Task HandleAuthenticateAsync() else { - if (context.MergedPrincipal is not ClaimsPrincipal principal) - { - return AuthenticateResult.NoResult(); - } - // Restore or create a new authentication properties collection and populate it. var properties = CreateProperties(context.StateTokenPrincipal); properties.ExpiresUtc = context.StateTokenPrincipal?.GetExpirationDate(); @@ -314,7 +317,8 @@ protected override async Task HandleAuthenticateAsync() properties.SetParameter(Properties.UserinfoTokenPrincipal, context.UserinfoTokenPrincipal); } - return AuthenticateResult.Success(new AuthenticationTicket(principal, properties, + return AuthenticateResult.Success(new AuthenticationTicket( + context.MergedPrincipal ?? new ClaimsPrincipal(new ClaimsIdentity()), properties, OpenIddictClientAspNetCoreDefaults.AuthenticationScheme)); static AuthenticationProperties CreateProperties(ClaimsPrincipal? principal) diff --git a/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandler.cs b/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandler.cs index 25853042b..eaf0dd20e 100644 --- a/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandler.cs +++ b/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandler.cs @@ -168,11 +168,6 @@ public override async Task InvokeAsync() else { - if (context.MergedPrincipal is not ClaimsPrincipal principal) - { - return null; - } - // Restore or create a new authentication properties collection and populate it. var properties = CreateProperties(context.StateTokenPrincipal); properties.ExpiresUtc = context.StateTokenPrincipal?.GetExpirationDate(); @@ -240,7 +235,7 @@ public override async Task InvokeAsync() properties.Dictionary[Tokens.UserinfoToken] = context.UserinfoToken; } - return new AuthenticationTicket((ClaimsIdentity) principal.Identity, properties); + return new AuthenticationTicket(context.MergedPrincipal?.Identity as ClaimsIdentity, properties); static AuthenticationProperties CreateProperties(ClaimsPrincipal? principal) { @@ -270,7 +265,7 @@ static AuthenticationProperties CreateProperties(ClaimsPrincipal? principal) /// protected override async Task TeardownCoreAsync() { - // Note: OWIN authentication handlers cannot reliabily write to the response stream + // Note: OWIN authentication handlers cannot reliably write to the response stream // from ApplyResponseGrantAsync() or ApplyResponseChallengeAsync() because these methods // are susceptible to be invoked from AuthenticationHandler.OnSendingHeaderCallback(), // where calling Write() or WriteAsync() on the response stream may result in a deadlock diff --git a/src/OpenIddict.Server.AspNetCore/OpenIddictServerAspNetCoreHandler.cs b/src/OpenIddict.Server.AspNetCore/OpenIddictServerAspNetCoreHandler.cs index 847fa2be6..10bc310e6 100644 --- a/src/OpenIddict.Server.AspNetCore/OpenIddictServerAspNetCoreHandler.cs +++ b/src/OpenIddict.Server.AspNetCore/OpenIddictServerAspNetCoreHandler.cs @@ -186,15 +186,10 @@ OpenIddictServerEndpointType.Token when context.Request.IsRefreshTokenGrantType( _ => null }; - if (principal is null) - { - return AuthenticateResult.NoResult(); - } - // Restore or create a new authentication properties collection and populate it. var properties = CreateProperties(principal); - properties.ExpiresUtc = principal.GetExpirationDate(); - properties.IssuedUtc = principal.GetCreationDate(); + properties.ExpiresUtc = principal?.GetExpirationDate(); + properties.IssuedUtc = principal?.GetCreationDate(); List? tokens = null; @@ -311,7 +306,8 @@ OpenIddictServerEndpointType.Token when context.Request.IsRefreshTokenGrantType( properties.StoreTokens(tokens); } - return AuthenticateResult.Success(new AuthenticationTicket(principal, properties, + return AuthenticateResult.Success(new AuthenticationTicket( + principal ?? new ClaimsPrincipal(new ClaimsIdentity()), properties, OpenIddictServerAspNetCoreDefaults.AuthenticationScheme)); } diff --git a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandler.cs b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandler.cs index 9ca7f0a1a..7c17632c7 100644 --- a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandler.cs +++ b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandler.cs @@ -192,15 +192,10 @@ OpenIddictServerEndpointType.Token when context.Request.IsRefreshTokenGrantType( _ => null }; - if (principal is null) - { - return null; - } - // Restore or create a new authentication properties collection and populate it. var properties = CreateProperties(principal); - properties.ExpiresUtc = principal.GetExpirationDate(); - properties.IssuedUtc = principal.GetCreationDate(); + properties.ExpiresUtc = principal?.GetExpirationDate(); + properties.IssuedUtc = principal?.GetCreationDate(); // Attach the tokens to allow any OWIN component (e.g a controller) // to retrieve them (e.g to make an API request to another application). @@ -240,7 +235,7 @@ OpenIddictServerEndpointType.Token when context.Request.IsRefreshTokenGrantType( properties.Dictionary[Tokens.UserCode] = context.UserCode; } - return new AuthenticationTicket((ClaimsIdentity) principal.Identity, properties); + return new AuthenticationTicket(principal?.Identity as ClaimsIdentity, properties); } static AuthenticationProperties CreateProperties(ClaimsPrincipal? principal) @@ -270,7 +265,7 @@ static AuthenticationProperties CreateProperties(ClaimsPrincipal? principal) /// protected override async Task TeardownCoreAsync() { - // Note: OWIN authentication handlers cannot reliabily write to the response stream + // Note: OWIN authentication handlers cannot reliably write to the response stream // from ApplyResponseGrantAsync() or ApplyResponseChallengeAsync() because these methods // are susceptible to be invoked from AuthenticationHandler.OnSendingHeaderCallback(), // where calling Write() or WriteAsync() on the response stream may result in a deadlock diff --git a/src/OpenIddict.Validation.AspNetCore/OpenIddictValidationAspNetCoreHandler.cs b/src/OpenIddict.Validation.AspNetCore/OpenIddictValidationAspNetCoreHandler.cs index ee977f815..5f7949b96 100644 --- a/src/OpenIddict.Validation.AspNetCore/OpenIddictValidationAspNetCoreHandler.cs +++ b/src/OpenIddict.Validation.AspNetCore/OpenIddictValidationAspNetCoreHandler.cs @@ -5,6 +5,7 @@ */ using System.ComponentModel; +using System.Security.Claims; using System.Text.Encodings.Web; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -162,15 +163,10 @@ protected override async Task HandleAuthenticateAsync() _ => null }; - if (principal is null) - { - return AuthenticateResult.NoResult(); - } - var properties = new AuthenticationProperties { - ExpiresUtc = principal.GetExpirationDate(), - IssuedUtc = principal.GetCreationDate() + ExpiresUtc = principal?.GetExpirationDate(), + IssuedUtc = principal?.GetCreationDate() }; List? tokens = null; @@ -198,7 +194,8 @@ protected override async Task HandleAuthenticateAsync() properties.StoreTokens(tokens); } - return AuthenticateResult.Success(new AuthenticationTicket(principal, properties, + return AuthenticateResult.Success(new AuthenticationTicket( + principal ?? new ClaimsPrincipal(new ClaimsIdentity()), properties, OpenIddictValidationAspNetCoreDefaults.AuthenticationScheme)); } } diff --git a/src/OpenIddict.Validation.Owin/OpenIddictValidationOwinHandler.cs b/src/OpenIddict.Validation.Owin/OpenIddictValidationOwinHandler.cs index 93b5e6532..8d81df9b3 100644 --- a/src/OpenIddict.Validation.Owin/OpenIddictValidationOwinHandler.cs +++ b/src/OpenIddict.Validation.Owin/OpenIddictValidationOwinHandler.cs @@ -170,15 +170,10 @@ public override async Task InvokeAsync() _ => null }; - if (principal is null) - { - return null; - } - var properties = new AuthenticationProperties { - ExpiresUtc = principal.GetExpirationDate(), - IssuedUtc = principal.GetCreationDate() + ExpiresUtc = principal?.GetExpirationDate(), + IssuedUtc = principal?.GetCreationDate() }; // Attach the tokens to allow any OWIN/Katana component (e.g a controller) @@ -189,14 +184,14 @@ public override async Task InvokeAsync() properties.Dictionary[TokenTypeHints.AccessToken] = context.AccessToken; } - return new AuthenticationTicket((ClaimsIdentity) principal.Identity, properties); + return new AuthenticationTicket(principal?.Identity as ClaimsIdentity, properties); } } /// protected override async Task TeardownCoreAsync() { - // Note: OWIN authentication handlers cannot reliabily write to the response stream + // Note: OWIN authentication handlers cannot reliably write to the response stream // from ApplyResponseGrantAsync() or ApplyResponseChallengeAsync() because these methods // are susceptible to be invoked from AuthenticationHandler.OnSendingHeaderCallback(), // where calling Write() or WriteAsync() on the response stream may result in a deadlock