From 286bbeb7f8f48ac628b558385239224f82abc673 Mon Sep 17 00:00:00 2001 From: Harrison Ulrich Date: Wed, 14 May 2025 14:23:58 -0600 Subject: [PATCH 1/3] Add event to allow caller to handle OIDC PAR failure. --- .../src/Events/OpenIdConnectEvents.cs | 12 ++++ .../PushedAuthorizationFailedContext.cs | 35 ++++++++++ .../OpenIdConnect/src/LoggingExtensions.cs | 5 +- .../OpenIdConnect/src/OpenIdConnectHandler.cs | 68 ++++++++++++------- .../OpenIdConnect/src/PublicAPI.Unshipped.txt | 8 +++ 5 files changed, 102 insertions(+), 26 deletions(-) create mode 100644 src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationFailedContext.cs diff --git a/src/Security/Authentication/OpenIdConnect/src/Events/OpenIdConnectEvents.cs b/src/Security/Authentication/OpenIdConnect/src/Events/OpenIdConnectEvents.cs index 2a66006bd2dd..9f607298b1d2 100644 --- a/src/Security/Authentication/OpenIdConnect/src/Events/OpenIdConnectEvents.cs +++ b/src/Security/Authentication/OpenIdConnect/src/Events/OpenIdConnectEvents.cs @@ -66,6 +66,11 @@ public class OpenIdConnectEvents : RemoteAuthenticationEvents /// public Func OnPushAuthorization { get; set; } = context => Task.CompletedTask; + /// + /// Invoked when authorization parameters pushed using PAR result in a failure. + /// + public Func OnPushAuthorizationFailed { get; set; } = context => Task.CompletedTask; + /// /// Invoked if exceptions are thrown during request processing. The exceptions will be re-thrown after this event unless suppressed. /// @@ -125,4 +130,11 @@ public class OpenIdConnectEvents : RemoteAuthenticationEvents /// /// public virtual Task PushAuthorization(PushedAuthorizationContext context) => OnPushAuthorization(context); + + /// + /// Invoked when authorization parameters pushed using PAR result in a failure. + /// + /// + /// + public virtual Task PushAuthorizationFailed(PushedAuthorizationFailedContext context) => OnPushAuthorizationFailed(context); } diff --git a/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationFailedContext.cs b/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationFailedContext.cs new file mode 100644 index 000000000000..f3abef634e82 --- /dev/null +++ b/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationFailedContext.cs @@ -0,0 +1,35 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Http; + +namespace Microsoft.AspNetCore.Authentication.OpenIdConnect; + +/// +/// A context for . +/// +public sealed class PushedAuthorizationFailedContext : PropertiesContext +{ + /// + /// Initializes a new instance of . + /// + /// + public PushedAuthorizationFailedContext(HttpContext context, AuthenticationScheme scheme, OpenIdConnectOptions options, + AuthenticationProperties properties, Exception exception) + : base(context, scheme, options, properties) + { + Exception = exception; + } + + /// + /// Gets or sets the exception associated with the failure. + /// + public Exception Exception { get; } + + /// + /// Tells the handler that the OnPushAuthorizationFailed event has handled the process of the + /// error and the handler does not need to throw an exception. + /// + public bool Handled { get; set; } +} + diff --git a/src/Security/Authentication/OpenIdConnect/src/LoggingExtensions.cs b/src/Security/Authentication/OpenIdConnect/src/LoggingExtensions.cs index 6965d08f9f84..ccc4ef72b4a8 100644 --- a/src/Security/Authentication/OpenIdConnect/src/LoggingExtensions.cs +++ b/src/Security/Authentication/OpenIdConnect/src/LoggingExtensions.cs @@ -74,7 +74,7 @@ internal static partial class LoggingExtensions [LoggerMessage(37, LogLevel.Debug, "The UserInformationReceived event returned Skipped.", EventName = "UserInformationReceivedSkipped")] public static partial void UserInformationReceivedSkipped(this ILogger logger); - [LoggerMessage(57, LogLevel.Debug, "The PushAuthorization event handled client authentication", EventName = "PushAuthorizationHandledClientAuthentication")] + [LoggerMessage(60, LogLevel.Debug, "The PushAuthorization event handled client authentication", EventName = "PushAuthorizationHandledClientAuthentication")] public static partial void PushAuthorizationHandledClientAuthentication(this ILogger logger); [LoggerMessage(58, LogLevel.Debug, "The PushAuthorization event handled pushing authorization", EventName = "PushAuthorizationHandledPush")] @@ -83,6 +83,9 @@ internal static partial class LoggingExtensions [LoggerMessage(59, LogLevel.Debug, "The PushAuthorization event skipped pushing authorization", EventName = "PushAuthorizationSkippedPush")] public static partial void PushAuthorizationSkippedPush(this ILogger logger); + [LoggerMessage(61, LogLevel.Debug, "PushAuthorizationFailed.HandledResponse", EventName = "PushAuthorizationFailedHandledResponse")] + public static partial void PushAuthorizationFailedHandledResponse(this ILogger logger); + [LoggerMessage(3, LogLevel.Warning, "The query string for Logout is not a well-formed URI. Redirect URI: '{RedirectUrl}'.", EventName = "InvalidLogoutQueryStringRedirectUrl")] public static partial void InvalidLogoutQueryStringRedirectUrl(this ILogger logger, string redirectUrl); diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs index 716126163e04..c6af4dae1a5a 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs @@ -487,35 +487,53 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert var parEndpoint = _configuration?.PushedAuthorizationRequestEndpoint; - switch (Options.PushedAuthorizationBehavior) + try { - case PushedAuthorizationBehavior.UseIfAvailable: - // Push if endpoint is in disco - if (!string.IsNullOrEmpty(parEndpoint)) - { - await PushAuthorizationRequest(message, properties, parEndpoint); - } + switch (Options.PushedAuthorizationBehavior) + { + case PushedAuthorizationBehavior.UseIfAvailable: + // Push if endpoint is in disco + if (!string.IsNullOrEmpty(parEndpoint)) + { + await PushAuthorizationRequest(message, properties, parEndpoint); + } - break; - case PushedAuthorizationBehavior.Disable: - // Fail if disabled in options but required by disco - if (_configuration?.RequirePushedAuthorizationRequests == true) - { - throw new InvalidOperationException("Pushed authorization is required by the OpenId Connect provider, but disabled by the OpenIdConnectOptions.PushedAuthorizationBehavior."); - } + break; + case PushedAuthorizationBehavior.Disable: + // Fail if disabled in options but required by disco + if (_configuration?.RequirePushedAuthorizationRequests == true) + { + throw new InvalidOperationException("Pushed authorization is required by the OpenId Connect provider, but disabled by the OpenIdConnectOptions.PushedAuthorizationBehavior."); + } - // Otherwise do nothing - break; - case PushedAuthorizationBehavior.Require: - // Fail if required in options but unavailable in disco - if (string.IsNullOrEmpty(parEndpoint)) - { - throw new InvalidOperationException("Pushed authorization is required by the OpenIdConnectOptions.PushedAuthorizationBehavior, but no pushed authorization endpoint is available."); - } + // Otherwise do nothing + break; + case PushedAuthorizationBehavior.Require: + // Fail if required in options but unavailable in disco + if (string.IsNullOrEmpty(parEndpoint)) + { + throw new InvalidOperationException("Pushed authorization is required by the OpenIdConnectOptions.PushedAuthorizationBehavior, but no pushed authorization endpoint is available."); + } + + // Otherwise push + await PushAuthorizationRequest(message, properties, parEndpoint); + break; + } + } + catch(Exception exception) + { + var failContext = new PushedAuthorizationFailedContext(Context, Scheme, Options, properties, exception) - // Otherwise push - await PushAuthorizationRequest(message, properties, parEndpoint); - break; + await Events.PushAuthorizationFailed(failContext); + if (failContext.Handled) + { + Logger.PushAuthorizationFailedHandledResponse(); + return; + } + else + { + throw; + } } if (Options.AuthenticationMethod == OpenIdConnectRedirectBehavior.RedirectGet) diff --git a/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt b/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt index 7dc5c58110bf..cd842d468f92 100644 --- a/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt +++ b/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt @@ -1 +1,9 @@ #nullable enable +Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationFailedContext +Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationFailedContext.Exception.get -> System.Exception! +Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationFailedContext.Handled.get -> bool +Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationFailedContext.Handled.set -> void +Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationFailedContext.PushedAuthorizationFailedContext(Microsoft.AspNetCore.Http.HttpContext! context, Microsoft.AspNetCore.Authentication.AuthenticationScheme! scheme, Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectOptions! options, Microsoft.AspNetCore.Authentication.AuthenticationProperties! properties, System.Exception! exception) -> void +virtual Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectEvents.PushAuthorizationFailed(Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationFailedContext! context) -> System.Threading.Tasks.Task! +Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectEvents.OnPushAuthorizationFailed.set -> void +Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectEvents.OnPushAuthorizationFailed.get -> System.Func! From 15e29f9211accbe89c13c6f82d5bc30c6dc6286e Mon Sep 17 00:00:00 2001 From: Harrison Ulrich Date: Wed, 14 May 2025 16:19:57 -0600 Subject: [PATCH 2/3] Include test for OnPuthAuthorizationFailed event --- .../OpenIdConnectChallengeTests.cs | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs index 39504a60c711..d0f031b1d968 100644 --- a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs +++ b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs @@ -963,6 +963,45 @@ public async Task Challenge_WithPushedAuthorization_MultipleContextActionsAreNot var server = settings.CreateTestServer(); await Assert.ThrowsAsync(() => server.SendAsync(ChallengeEndpoint)); } + + [Fact] + public async Task Challenge_WithPushedAuthorization_FailureHandled() + { + var onPushAuthorizationFailedCalled = false; + var mockBackchannel = new ParMockBackchannel(); + var settings = new TestSettings(opt => + { + opt.ClientId = "Test Id"; + opt.ClientSecret = "secret"; + + opt.PushedAuthorizationBehavior = PushedAuthorizationBehavior.Require; + // Instead of using discovery, this test hard codes the configuration that disco would retrieve. + // This makes it easier to manipulate the discovery results + opt.Configuration = new OpenIdConnectConfiguration(); + opt.Configuration.AuthorizationEndpoint = "https://testauthority/authorize"; + + // We are NOT setting the endpoint (as if the disco document didn't contain it) + //opt.Configuration.PushedAuthorizationRequestEndpoint; + + opt.Events.OnPushAuthorizationFailed = ctx => + { + onPushAuthorizationFailedCalled = true; + ctx.Response.Redirect("CustomErrorRoute"); + ctx.Handled = true; + return Task.CompletedTask; + }; + }, mockBackchannel); + + var server = settings.CreateTestServer(); + var transaction = await server.SendAsync(ChallengeEndpoint); + + var res = transaction.Response; + + Assert.True(onPushAuthorizationFailedCalled); + + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + Assert.Equal("CustomErrorRoute", res.Headers.Location.OriginalString); + } } class ParMockBackchannel : HttpMessageHandler From 1bd80455839fe326a4f33246b1979d2e0f617acb Mon Sep 17 00:00:00 2001 From: Harrison Ulrich Date: Wed, 14 May 2025 17:03:33 -0600 Subject: [PATCH 3/3] Fix broken build due to copy/paste error --- .../Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs index c6af4dae1a5a..165f42572d0a 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs @@ -522,7 +522,7 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert } catch(Exception exception) { - var failContext = new PushedAuthorizationFailedContext(Context, Scheme, Options, properties, exception) + var failContext = new PushedAuthorizationFailedContext(Context, Scheme, Options, properties, exception); await Events.PushAuthorizationFailed(failContext); if (failContext.Handled)