From 878569cd3febf3e3c99424e99105d5608e38fdc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Tue, 12 Nov 2024 18:27:57 +0100 Subject: [PATCH] Update the ASP.NET Core/OWIN hosts to support returning authentication properties for errored requests --- .../Startup.cs | 2 +- .../Worker.cs | 2 +- .../OpenIddictClientAspNetCoreHandler.cs | 23 +++++---- .../OpenIddictClientOwinHandler.cs | 21 +++++--- .../OpenIddictServerAspNetCoreHandler.cs | 23 +++++---- .../OpenIddictServerOwinHandler.cs | 21 +++++--- .../OpenIddictValidationAspNetCoreHandler.cs | 23 +++++---- .../OpenIddictValidationOwinHandler.cs | 21 +++++--- ...nIddictServerAspNetCoreIntegrationTests.cs | 49 ++++++++++++++++++ .../OpenIddictServerOwinIntegrationTests.cs | 51 ++++++++++++++++++- ...penIddictValidationOwinIntegrationTests.cs | 2 +- 11 files changed, 183 insertions(+), 55 deletions(-) diff --git a/sandbox/OpenIddict.Sandbox.AspNet.Server/Startup.cs b/sandbox/OpenIddict.Sandbox.AspNet.Server/Startup.cs index 287bfc7c..ad53f978 100644 --- a/sandbox/OpenIddict.Sandbox.AspNet.Server/Startup.cs +++ b/sandbox/OpenIddict.Sandbox.AspNet.Server/Startup.cs @@ -201,7 +201,7 @@ public class Startup ClientId = "mvc", ClientSecret = "901564A5-E7FE-42CB-B10D-61EF6A8F3654", ClientType = ClientTypes.Confidential, - ConsentType = ConsentTypes.Explicit, + ConsentType = ConsentTypes.Systematic, DisplayName = "MVC client application", RedirectUris = { diff --git a/sandbox/OpenIddict.Sandbox.AspNetCore.Server/Worker.cs b/sandbox/OpenIddict.Sandbox.AspNetCore.Server/Worker.cs index 6ad635fc..f4d44f2f 100644 --- a/sandbox/OpenIddict.Sandbox.AspNetCore.Server/Worker.cs +++ b/sandbox/OpenIddict.Sandbox.AspNetCore.Server/Worker.cs @@ -152,7 +152,7 @@ public class Worker : IHostedService ClientId = "mvc", ClientSecret = "901564A5-E7FE-42CB-B10D-61EF6A8F3654", ClientType = ClientTypes.Confidential, - ConsentType = ConsentTypes.Explicit, + ConsentType = ConsentTypes.Systematic, DisplayName = "MVC client application", DisplayNames = { diff --git a/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandler.cs b/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandler.cs index 1e0df7e7..67148aa4 100644 --- a/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandler.cs +++ b/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandler.cs @@ -156,17 +156,24 @@ public sealed class OpenIddictClientAspNetCoreHandler : AuthenticationHandler - { - [Properties.Error] = context.Error, - [Properties.ErrorDescription] = context.ErrorDescription, - [Properties.ErrorUri] = context.ErrorUri - }); + var properties = CreateAuthenticationProperties(); + properties.Items[Properties.Error] = context.Error; + properties.Items[Properties.ErrorDescription] = context.ErrorDescription; + properties.Items[Properties.ErrorUri] = context.ErrorUri; return AuthenticateResult.Fail(SR.GetResourceString(SR.ID0113), properties); } else + { + var properties = CreateAuthenticationProperties(); + + return AuthenticateResult.Success(new AuthenticationTicket( + context.MergedPrincipal ?? new ClaimsPrincipal(new ClaimsIdentity()), properties, + OpenIddictClientAspNetCoreDefaults.AuthenticationScheme)); + } + + AuthenticationProperties CreateAuthenticationProperties() { var properties = new AuthenticationProperties { @@ -336,9 +343,7 @@ public sealed class OpenIddictClientAspNetCoreHandler : AuthenticationHandler - { - [Properties.Error] = context.Error, - [Properties.ErrorDescription] = context.ErrorDescription, - [Properties.ErrorUri] = context.ErrorUri - }); + var properties = CreateAuthenticationProperties(); + properties.Dictionary[Properties.Error] = context.Error; + properties.Dictionary[Properties.ErrorDescription] = context.ErrorDescription; + properties.Dictionary[Properties.ErrorUri] = context.ErrorUri; - return new AuthenticationTicket(null, properties); + return new AuthenticationTicket(new ClaimsIdentity(), properties); } else + { + var properties = CreateAuthenticationProperties(); + + return new AuthenticationTicket(context.MergedPrincipal?.Identity as ClaimsIdentity ?? new ClaimsIdentity(), properties); + } + + AuthenticationProperties CreateAuthenticationProperties() { var properties = new AuthenticationProperties { @@ -240,7 +245,7 @@ public sealed class OpenIddictClientOwinHandler : AuthenticationHandler - { - [Properties.Error] = context.Error, - [Properties.ErrorDescription] = context.ErrorDescription, - [Properties.ErrorUri] = context.ErrorUri - }); + var properties = CreateAuthenticationProperties(); + properties.Items[Properties.Error] = context.Error; + properties.Items[Properties.ErrorDescription] = context.ErrorDescription; + properties.Items[Properties.ErrorUri] = context.ErrorUri; return AuthenticateResult.Fail(SR.GetResourceString(SR.ID0113), properties); } @@ -199,6 +197,15 @@ public sealed class OpenIddictServerAspNetCoreHandler : AuthenticationHandler null }; + var properties = CreateAuthenticationProperties(principal); + + return AuthenticateResult.Success(new AuthenticationTicket( + principal ?? new ClaimsPrincipal(new ClaimsIdentity()), properties, + OpenIddictServerAspNetCoreDefaults.AuthenticationScheme)); + } + + AuthenticationProperties CreateAuthenticationProperties(ClaimsPrincipal? principal = null) + { var properties = new AuthenticationProperties { ExpiresUtc = principal?.GetExpirationDate(), @@ -325,9 +332,7 @@ public sealed class OpenIddictServerAspNetCoreHandler : AuthenticationHandler - { - [Properties.Error] = context.Error, - [Properties.ErrorDescription] = context.ErrorDescription, - [Properties.ErrorUri] = context.ErrorUri - }); + var properties = CreateAuthenticationProperties(); + properties.Dictionary[Properties.Error] = context.Error; + properties.Dictionary[Properties.ErrorDescription] = context.ErrorDescription; + properties.Dictionary[Properties.ErrorUri] = context.ErrorUri; - return new AuthenticationTicket(null, properties); + return new AuthenticationTicket(new ClaimsIdentity(), properties); } else @@ -191,6 +189,13 @@ public sealed class OpenIddictServerOwinHandler : AuthenticationHandler null }; + var properties = CreateAuthenticationProperties(principal); + + return new AuthenticationTicket(principal?.Identity as ClaimsIdentity ?? new ClaimsIdentity(), properties); + } + + AuthenticationProperties CreateAuthenticationProperties(ClaimsPrincipal? principal = null) + { var properties = new AuthenticationProperties { ExpiresUtc = principal?.GetExpirationDate(), @@ -240,7 +245,7 @@ public sealed class OpenIddictServerOwinHandler : AuthenticationHandler - { - [Properties.Error] = context.Error, - [Properties.ErrorDescription] = context.ErrorDescription, - [Properties.ErrorUri] = context.ErrorUri - }); + var properties = CreateAuthenticationProperties(); + properties.Items[Properties.Error] = context.Error; + properties.Items[Properties.ErrorDescription] = context.ErrorDescription; + properties.Items[Properties.ErrorUri] = context.ErrorUri; return AuthenticateResult.Fail(SR.GetResourceString(SR.ID0113), properties); } @@ -177,6 +175,15 @@ public sealed class OpenIddictValidationAspNetCoreHandler : AuthenticationHandle _ => null }; + var properties = CreateAuthenticationProperties(principal); + + return AuthenticateResult.Success(new AuthenticationTicket( + principal ?? new ClaimsPrincipal(new ClaimsIdentity()), properties, + OpenIddictValidationAspNetCoreDefaults.AuthenticationScheme)); + } + + AuthenticationProperties CreateAuthenticationProperties(ClaimsPrincipal? principal = null) + { var properties = new AuthenticationProperties { ExpiresUtc = principal?.GetExpirationDate(), @@ -208,9 +215,7 @@ public sealed class OpenIddictValidationAspNetCoreHandler : AuthenticationHandle properties.StoreTokens(tokens); } - return AuthenticateResult.Success(new AuthenticationTicket( - principal ?? new ClaimsPrincipal(new ClaimsIdentity()), properties, - OpenIddictValidationAspNetCoreDefaults.AuthenticationScheme)); + return properties; } } diff --git a/src/OpenIddict.Validation.Owin/OpenIddictValidationOwinHandler.cs b/src/OpenIddict.Validation.Owin/OpenIddictValidationOwinHandler.cs index 8d81df9b..3375fe34 100644 --- a/src/OpenIddict.Validation.Owin/OpenIddictValidationOwinHandler.cs +++ b/src/OpenIddict.Validation.Owin/OpenIddictValidationOwinHandler.cs @@ -150,14 +150,12 @@ public sealed class OpenIddictValidationOwinHandler : AuthenticationHandler - { - [Properties.Error] = context.Error, - [Properties.ErrorDescription] = context.ErrorDescription, - [Properties.ErrorUri] = context.ErrorUri - }); + var properties = CreateAuthenticationProperties(); + properties.Dictionary[Properties.Error] = context.Error; + properties.Dictionary[Properties.ErrorDescription] = context.ErrorDescription; + properties.Dictionary[Properties.ErrorUri] = context.ErrorUri; - return new AuthenticationTicket(null, properties); + return new AuthenticationTicket(new ClaimsIdentity(), properties); } else @@ -170,6 +168,13 @@ public sealed class OpenIddictValidationOwinHandler : AuthenticationHandler null }; + var properties = CreateAuthenticationProperties(principal); + + return new AuthenticationTicket(principal?.Identity as ClaimsIdentity ?? new ClaimsIdentity(), properties); + } + + AuthenticationProperties CreateAuthenticationProperties(ClaimsPrincipal? principal = null) + { var properties = new AuthenticationProperties { ExpiresUtc = principal?.GetExpirationDate(), @@ -184,7 +189,7 @@ public sealed class OpenIddictValidationOwinHandler : AuthenticationHandler + { + options.EnableDegradedMode(); + options.SetAuthorizationEndpointUris("/authenticate/properties"); + + options.UseAspNetCore() + .EnableErrorPassthrough() + .EnableAuthorizationEndpointPassthrough(); + + options.AddEventHandler(builder => + { + builder.UseInlineHandler(context => + { + context.RejectIdentityToken = true; + + context.Properties["custom_property"] = "value"; + + return default; + }); + + builder.SetOrder(EvaluateValidatedTokens.Descriptor.Order + 1); + }); + }); + + await using var client = await server.CreateClientAsync(); + + // Act + var response = await client.PostAsync("/authenticate/properties", new OpenIddictRequest + { + ClientId = "Fabrikam", + IdTokenHint = "id_token_hint", + Nonce = "n-0S6_WzA2Mj", + RedirectUri = "http://www.fabrikam.com/path", + ResponseType = "id_token", + Scope = Scopes.OpenId + }); + + // Assert + var properties = new AuthenticationProperties(response.GetParameters() + .ToDictionary(parameter => parameter.Key, parameter => (string?) parameter.Value)); + + Assert.Equal("value", properties.Items["custom_property"]); + } + [Fact] public async Task ProcessChallenge_ImportsAuthenticationProperties() { diff --git a/test/OpenIddict.Server.Owin.IntegrationTests/OpenIddictServerOwinIntegrationTests.cs b/test/OpenIddict.Server.Owin.IntegrationTests/OpenIddictServerOwinIntegrationTests.cs index e170b01c..67fb892d 100644 --- a/test/OpenIddict.Server.Owin.IntegrationTests/OpenIddictServerOwinIntegrationTests.cs +++ b/test/OpenIddict.Server.Owin.IntegrationTests/OpenIddictServerOwinIntegrationTests.cs @@ -16,6 +16,7 @@ using Owin; using Xunit; using Xunit.Abstractions; using static OpenIddict.Server.OpenIddictServerEvents; +using static OpenIddict.Server.OpenIddictServerHandlers; using static OpenIddict.Server.OpenIddictServerHandlers.Protection; namespace OpenIddict.Server.Owin.IntegrationTests; @@ -128,6 +129,54 @@ public partial class OpenIddictServerOwinIntegrationTests : OpenIddictServerInte Assert.Equal(new DateTimeOffset(2120, 01, 01, 00, 00, 00, TimeSpan.Zero), properties.ExpiresUtc); } + [Fact] + public async Task ProcessAuthentication_CustomPropertiesAreAddedForErroredAuthenticationResults() + { + // Arrange + await using var server = await CreateServerAsync(options => + { + options.EnableDegradedMode(); + options.SetAuthorizationEndpointUris("/authenticate/properties"); + + options.UseOwin() + .EnableErrorPassthrough() + .EnableAuthorizationEndpointPassthrough(); + + options.AddEventHandler(builder => + { + builder.UseInlineHandler(context => + { + context.RejectIdentityToken = true; + + context.Properties["custom_property"] = "value"; + + return default; + }); + + builder.SetOrder(EvaluateValidatedTokens.Descriptor.Order + 1); + }); + }); + + await using var client = await server.CreateClientAsync(); + + // Act + var response = await client.PostAsync("/authenticate/properties", new OpenIddictRequest + { + ClientId = "Fabrikam", + IdTokenHint = "id_token_hint", + Nonce = "n-0S6_WzA2Mj", + RedirectUri = "http://www.fabrikam.com/path", + ResponseType = "id_token", + Scope = Scopes.OpenId + }); + + // Assert + var properties = new AuthenticationProperties(response.GetParameters() + .ToDictionary(parameter => parameter.Key, parameter => (string?) parameter.Value)); + + Assert.Equal("value", properties.Dictionary["custom_property"]); + } + [Fact] public async Task ProcessChallenge_ImportsAuthenticationProperties() { @@ -642,7 +691,7 @@ public partial class OpenIddictServerOwinIntegrationTests : OpenIddictServerInte else if (context.Request.Path == new PathString("/authenticate")) { var result = await context.Authentication.AuthenticateAsync(OpenIddictServerOwinDefaults.AuthenticationType); - if (result?.Identity is null) + if (result?.Identity is not { IsAuthenticated: true }) { return; } diff --git a/test/OpenIddict.Validation.Owin.IntegrationTests/OpenIddictValidationOwinIntegrationTests.cs b/test/OpenIddict.Validation.Owin.IntegrationTests/OpenIddictValidationOwinIntegrationTests.cs index 525a8b54..4297f7d2 100644 --- a/test/OpenIddict.Validation.Owin.IntegrationTests/OpenIddictValidationOwinIntegrationTests.cs +++ b/test/OpenIddict.Validation.Owin.IntegrationTests/OpenIddictValidationOwinIntegrationTests.cs @@ -162,7 +162,7 @@ public partial class OpenIddictValidationOwinIntegrationTests : OpenIddictValida if (context.Request.Path == new PathString("/authenticate")) { var result = await context.Authentication.AuthenticateAsync(OpenIddictValidationOwinDefaults.AuthenticationType); - if (result?.Identity is null) + if (result?.Identity is not { IsAuthenticated: true }) { context.Authentication.Challenge(OpenIddictValidationOwinDefaults.AuthenticationType); return;