Skip to content

Bigred8982/issue 59462 OIDC par failure #61947

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,11 @@ public class OpenIdConnectEvents : RemoteAuthenticationEvents
/// </summary>
public Func<PushedAuthorizationContext, Task> OnPushAuthorization { get; set; } = context => Task.CompletedTask;

/// <summary>
/// Invoked when authorization parameters pushed using PAR result in a failure.
/// </summary>
public Func<PushedAuthorizationFailedContext, Task> OnPushAuthorizationFailed { get; set; } = context => Task.CompletedTask;

/// <summary>
/// Invoked if exceptions are thrown during request processing. The exceptions will be re-thrown after this event unless suppressed.
/// </summary>
Expand Down Expand Up @@ -125,4 +130,11 @@ public class OpenIdConnectEvents : RemoteAuthenticationEvents
/// <param name="context"></param>
/// <returns></returns>
public virtual Task PushAuthorization(PushedAuthorizationContext context) => OnPushAuthorization(context);

/// <summary>
/// Invoked when authorization parameters pushed using PAR result in a failure.
/// </summary>
/// <param name="context"></param>
/// <returns></returns>
public virtual Task PushAuthorizationFailed(PushedAuthorizationFailedContext context) => OnPushAuthorizationFailed(context);
}
Original file line number Diff line number Diff line change
@@ -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;

/// <summary>
/// A context for <see cref="OpenIdConnectEvents.PushAuthorization(PushedAuthorizationContext)"/>.
/// </summary>
public sealed class PushedAuthorizationFailedContext : PropertiesContext<OpenIdConnectOptions>
{
/// <summary>
/// Initializes a new instance of <see cref="PushedAuthorizationFailedContext"/>.
/// </summary>
/// <inheritdoc />
public PushedAuthorizationFailedContext(HttpContext context, AuthenticationScheme scheme, OpenIdConnectOptions options,
AuthenticationProperties properties, Exception exception)
: base(context, scheme, options, properties)
{
Exception = exception;
}

/// <summary>
/// Gets or sets the exception associated with the failure.
/// </summary>
public Exception Exception { get; }

/// <summary>
/// Tells the handler that the OnPushAuthorizationFailed event has handled the process of the
/// error and the handler does not need to throw an exception.
/// </summary>
public bool Handled { get; set; }
}

Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand All @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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<Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationFailedContext!, System.Threading.Tasks.Task!>!
Original file line number Diff line number Diff line change
Expand Up @@ -963,6 +963,45 @@ public async Task Challenge_WithPushedAuthorization_MultipleContextActionsAreNot
var server = settings.CreateTestServer();
await Assert.ThrowsAsync<InvalidOperationException>(() => 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
Expand Down
Loading