Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
853fa9c
Initial implementation of PAR in OIDC Handler
josephdecock Apr 11, 2024
a75529e
Fix API declaration
josephdecock Apr 11, 2024
8837fa9
PAR updates from review
josephdecock Jun 6, 2024
d343623
Simplify PAR event context
josephdecock Jun 7, 2024
61813a1
Clarify a few PAR related comments
josephdecock Jun 7, 2024
cdc0796
Enable PAR by default, if available in disco
josephdecock Jun 7, 2024
5d6eff3
Very minor changes for clarity
josephdecock Jun 7, 2024
ea017ff
Add enum to control PAR behavior
josephdecock Jul 17, 2024
635b666
Added basic tests for pushed authorization
josephdecock Jul 18, 2024
fddc18f
Seal PushedAuthorizationContext
josephdecock Jul 18, 2024
6f7987c
Add more PAR unit tests
josephdecock Jul 18, 2024
b62ea10
Clean up compiler warnings
josephdecock Jul 18, 2024
a202e8c
Revert commented out config in sample
josephdecock Jul 18, 2024
9ed07b3
Revert work in progress test
josephdecock Jul 18, 2024
3261d7c
Clean up test
josephdecock Jul 18, 2024
388de5b
Disallow multiple PAR context actions
josephdecock Jul 18, 2024
d599a81
Update src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHan…
josephdecock Jul 18, 2024
6371165
Update src/Security/Authentication/OpenIdConnect/src/PushedAuthorizat…
josephdecock Jul 18, 2024
9645f05
Update src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHan…
josephdecock Jul 18, 2024
5adc108
Fix exception messages when multiple PAR event methods are called
josephdecock Jul 18, 2024
080e8f3
Merge branch 'josephdecock/par' of https://github.com/josephdecock/as…
josephdecock Jul 18, 2024
120b45c
Merge branch 'main' into josephdecock/par
Jul 18, 2024
20bcf33
Use PAR support from System.IdentityModel to simplify discovery
josephdecock Jul 19, 2024
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 @@ -2,7 +2,7 @@

<PropertyGroup>
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
<UserSecretsId>aspnet5-OpenIdConnectSample-20151210110318</UserSecretsId>
<UserSecretsId>aspnet5-OpenIdConnectSample-20151210110318</UserSecretsId>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<UserSecretsId>aspnet5-OpenIdConnectSample-20151210110318</UserSecretsId>
<UserSecretsId>aspnet5-OpenIdConnectSample-20151210110318</UserSecretsId>

<AspNetCoreHostingModel>OutOfProcess</AspNetCoreHostingModel>
</PropertyGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ public static bool DisallowsSameSiteNone(string userAgent)
return true;
}

// Cover Chrome 50-69, because some versions are broken by SameSite=None,
// Cover Chrome 50-69, because some versions are broken by SameSite=None,
// and none in this range require it.
// Note: this covers some pre-Chromium Edge versions,
// Note: this covers some pre-Chromium Edge versions,
// but pre-Chromium Edge does not require SameSite=None.
if (userAgent.Contains("Chrome/5") || userAgent.Contains("Chrome/6"))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ public class OpenIdConnectEvents : RemoteAuthenticationEvents
/// </summary>
public Func<UserInformationReceivedContext, Task> OnUserInformationReceived { get; set; } = context => Task.CompletedTask;

/// <summary>
/// Invoked before authorization parameters are pushed using PAR.
/// </summary>
public Func<PushedAuthorizationContext, Task> OnPushAuthorization { 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 @@ -113,4 +118,11 @@ public class OpenIdConnectEvents : RemoteAuthenticationEvents
/// Invoked when user information is retrieved from the UserInfoEndpoint.
/// </summary>
public virtual Task UserInformationReceived(UserInformationReceivedContext context) => OnUserInformationReceived(context);

/// <summary>
/// Invoked before authorization parameters are pushed during PAR.
/// </summary>
/// <param name="context"></param>
/// <returns></returns>
public virtual Task PushAuthorization(PushedAuthorizationContext context) => OnPushAuthorization(context);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics.CodeAnalysis;
using Microsoft.AspNetCore.Http;
using Microsoft.IdentityModel.Protocols.OpenIdConnect;

namespace Microsoft.AspNetCore.Authentication.OpenIdConnect;

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

/// <summary>
/// Gets or sets the <see cref="OpenIdConnectMessage"/> that will be sent to the PAR endpoint.
/// </summary>
public OpenIdConnectMessage ProtocolMessage { get; }

/// <summary>
/// Indicates if the OnPushAuthorization event chose to handle pushing the
/// authorization request. If true, the handler will not attempt to push the
/// authorization request, and will instead use the RequestUri from this
/// event in the subsequent authorize request.
/// </summary>
public bool HandledPush { [MemberNotNull("RequestUri")] get; private set; }

/// <summary>
/// Tells the handler that the OnPushAuthorization event has handled the process of pushing
/// authorization, and that the handler should use the provided request_uri
/// on the subsequent authorize call.
/// </summary>
public void HandlePush(string requestUri)
{
if (SkippedPush || HandledClientAuthentication)
{
throw new InvalidOperationException("Only one of HandlePush, SkipPush, and HandledClientAuthentication may be called in the OnPushAuthorization event.");
}
HandledPush = true;
RequestUri = requestUri;
}

/// <summary>
/// Indicates if the OnPushAuthorization event chose to skip pushing the
/// authorization request. If true, the handler will not attempt to push the
/// authorization request, and will not use pushed authorization in the
/// subsequent authorize request.
/// </summary>
public bool SkippedPush { get; private set; }

/// <summary>
/// The request_uri parameter to use in the subsequent authorize call, if
/// the OnPushAuthorization event chose to handle pushing the authorization
/// request, and null otherwise.
/// </summary>
public string? RequestUri { get; private set; }

/// <summary>
/// Tells the handler to skip pushing authorization entirely. If this is
/// called, the handler will not use pushed authorization on the subsequent
/// authorize call.
/// </summary>
public void SkipPush()
{
if (HandledPush || HandledClientAuthentication)
{
throw new InvalidOperationException("Only one of HandlePush, SkipPush, and HandledClientAuthentication may be called in the OnPushAuthorization event.");
}
SkippedPush = true;
}

/// <summary>
/// Indicates if the OnPushAuthorization event chose to handle client
/// authentication for the pushed authorization request. If true, the
/// handler will not attempt to set authentication parameters for the pushed
/// authorization request.
/// </summary>
public bool HandledClientAuthentication { get; private set; }

/// <summary>
/// Tells the handler to skip setting client authentication properties for
/// pushed authorization. The handler uses the client_secret_basic
/// authentication mode by default, but the OnPushAuthorization event may
/// replace that with an alternative authentication mode, such as
/// private_key_jwt.
/// </summary>
public void HandleClientAuthentication()
{
if (SkippedPush || HandledPush)
{
throw new InvalidOperationException("Only one of HandlePush, SkipPush, and HandledClientAuthentication may be called in the OnPushAuthorization event.");
}
HandledClientAuthentication = true;
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,15 @@ 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")]
public static partial void PushAuthorizationHandledClientAuthentication(this ILogger logger);

[LoggerMessage(58, LogLevel.Debug, "The PushAuthorization event handled pushing authorization", EventName = "PushAuthorizationHandledPush")]
public static partial void PushAuthorizationHandledPush(this ILogger logger);

[LoggerMessage(59, LogLevel.Debug, "The PushAuthorization event skipped pushing authorization", EventName = "PushAuthorizationSkippedPush")]
public static partial void PushAuthorizationSkippedPush(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 @@ -485,6 +485,40 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert
"Cannot redirect to the authorization endpoint, the configuration may be missing or invalid.");
}

var parEndpoint = _configuration?.PushedAuthorizationRequestEndpoint;

switch (Options.PushedAuthorizationBehavior)
{
case PushedAuthorizationBehavior.UseIfAvailable:
// Push if endpoint is in disco
if (!string.IsNullOrEmpty(parEndpoint))
{
await PushAuthorizationRequest(message, properties);
}

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
var endpointIsConfigured = !string.IsNullOrEmpty(parEndpoint);
if (!endpointIsConfigured)
{
throw new InvalidOperationException("Pushed authorization is required by the OpenIdConnectOptions.PushedAuthorizationBehavior, but no pushed authorization endpoint is available.");
}

// Otherwise push
await PushAuthorizationRequest(message, properties);
break;
}

if (Options.AuthenticationMethod == OpenIdConnectRedirectBehavior.RedirectGet)
{
var redirectUri = message.CreateAuthenticationRequestUrl();
Expand Down Expand Up @@ -516,6 +550,82 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert
throw new NotImplementedException($"An unsupported authentication method has been configured: {Options.AuthenticationMethod}");
}

private async Task PushAuthorizationRequest(OpenIdConnectMessage authorizeRequest, AuthenticationProperties properties)
{
// Build context and run event
var parRequest = authorizeRequest.Clone();
var context = new PushedAuthorizationContext(Context, Scheme, Options, parRequest, properties);
await Events.PushAuthorization(context);

// If the event handled client authentication, skip the default auth behavior
if (context.HandledClientAuthentication)
{
Logger.PushAuthorizationHandledClientAuthentication();
}
// Otherwise, add the client secret to the parameters (if available)
else
{
if (!string.IsNullOrEmpty(Options.ClientSecret))
{
parRequest.Parameters.Add(OpenIdConnectParameterNames.ClientSecret, Options.ClientSecret);
}
}

string requestUri;

// The event can either entirely skip pushing to the par endpoint...
if (context.SkippedPush)
{
Logger.PushAuthorizationSkippedPush();
return;
}
// ... or handle pushing to the par endpoint itself, in which case it will supply the request uri
else if (context.HandledPush)
{
Logger.PushAuthorizationHandledPush();
requestUri = context.RequestUri;
}
else
{
var parEndpoint = _configuration?.PushedAuthorizationRequestEndpoint;
if (string.IsNullOrEmpty(parEndpoint))
{
new InvalidOperationException("Attempt to push authorization with no pushed authorization endpoint configured.");
}

var requestMessage = new HttpRequestMessage(HttpMethod.Post, parEndpoint);
requestMessage.Content = new FormUrlEncodedContent(parRequest.Parameters);
requestMessage.Version = Backchannel.DefaultRequestVersion;
var parResponseMessage = await Backchannel.SendAsync(requestMessage, Context.RequestAborted);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing that I've encountered is that some providers actually support PAR, but not for all clients. We ran into this when we added similar logic to IdentityModel.OidcClient (DuendeArchive/IdentityModel.OidcClient#428).

I just noticed this from the previous thread about making UseIfAvailable the default. I was also concerned that there may be providers that authenticate the PAR request with something other than the client_secret parameter. Would it make sense to log a warning and fallback to a normal non-pushed authorization attempt if we cannot get a PAR request URI in the UseIfAvailable case?

I'm still fine with merging this in a preview to see how it goes, but not many people use previews compared to major releases. It might be a good idea for us to update this behavior in RC1 even if no one complains to be extra safe, but I'm curious what everyone else thinks about this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the one hand, in the issue I linked to in IdentityModel.OidcClient, the issue was for public clients specifically and most users of the OidcHandler will be confidential clients.

On the other hand, if you have customized the behavior of the handler to call the token endpoint using a different form of authentication, presumably you need to do the same for the PAR requests that UseIfAvailable will start making for you.

But, users who are customizing the handler's authentication already are advanced users and personally I'm okay with expecting them to sort this out.

My feeling is to keep the UseIfAvailable default. For RC1, we could take a look at exactly what the error message is if authentication to the PAR endpoint fails and log a hint about the new APIs.

requestUri = await GetPushedAuthorizationRequestUri(parResponseMessage);
}

authorizeRequest.Parameters.Clear();
authorizeRequest.Parameters.Add("client_id", Options.ClientId);
authorizeRequest.Parameters.Add("request_uri", requestUri);
}

private async Task<string> GetPushedAuthorizationRequestUri(HttpResponseMessage parResponseMessage)
{
// Check content type
var contentType = parResponseMessage.Content.Headers.ContentType;
if (!(contentType?.MediaType?.Equals("application/json", StringComparison.OrdinalIgnoreCase) ?? false))
{
throw new InvalidOperationException("Invalid response from pushed authorization: content type is not application/json.");
}

// Parse response
var parResponseString = await parResponseMessage.Content.ReadAsStringAsync(Context.RequestAborted);
var message = new OpenIdConnectMessage(parResponseString);

var requestUri = message.GetParameter("request_uri");
if (requestUri == null)
{
throw CreateOpenIdConnectProtocolException(message, parResponseMessage);
}
return requestUri;
}

/// <summary>
/// Invoked to process incoming OpenIdConnect messages.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -408,4 +408,13 @@ public bool MapInboundClaims
/// When using TokenHandler, <see cref="TokenValidatedContext.SecurityToken"/> will be a <see cref="JsonWebToken"/>.
/// </remarks>
public bool UseSecurityTokenValidator { get; set; }

/// <summary>
/// Controls wether the handler should push authorization parameters on the
/// backchannel before redirecting to the identity provider. See <see
/// href="https://tools.ietf.org/html/9126"/>.
/// </summary>
/// <value>Defaults to <see
/// cref="PushedAuthorizationBehavior.UseIfAvailable" />.</value>
public PushedAuthorizationBehavior PushedAuthorizationBehavior { get; set; } = PushedAuthorizationBehavior.UseIfAvailable;
}
Original file line number Diff line number Diff line change
@@ -1,2 +1,21 @@
#nullable enable
Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectEvents.OnPushAuthorization.get -> System.Func<Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext!, System.Threading.Tasks.Task!>!
Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectEvents.OnPushAuthorization.set -> void
Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectOptions.AdditionalAuthorizationParameters.get -> System.Collections.Generic.IDictionary<string!, string!>!
Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectOptions.PushedAuthorizationBehavior.get -> Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationBehavior
Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectOptions.PushedAuthorizationBehavior.set -> void
Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationBehavior
Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationBehavior.Disable = 1 -> Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationBehavior
Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationBehavior.Require = 2 -> Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationBehavior
Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationBehavior.UseIfAvailable = 0 -> Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationBehavior
Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext
Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext.HandleClientAuthentication() -> void
Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext.HandledClientAuthentication.get -> bool
Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext.HandledPush.get -> bool
Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext.HandlePush(string! requestUri) -> void
Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext.ProtocolMessage.get -> Microsoft.IdentityModel.Protocols.OpenIdConnect.OpenIdConnectMessage!
Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext.PushedAuthorizationContext(Microsoft.AspNetCore.Http.HttpContext! context, Microsoft.AspNetCore.Authentication.AuthenticationScheme! scheme, Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectOptions! options, Microsoft.IdentityModel.Protocols.OpenIdConnect.OpenIdConnectMessage! parRequest, Microsoft.AspNetCore.Authentication.AuthenticationProperties! properties) -> void
Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext.RequestUri.get -> string?
Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext.SkippedPush.get -> bool
Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext.SkipPush() -> void
virtual Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectEvents.PushAuthorization(Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext! context) -> System.Threading.Tasks.Task!
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.IdentityModel.Protocols.OpenIdConnect;

namespace Microsoft.AspNetCore.Authentication.OpenIdConnect;

/// <summary>
/// Enum containing the options for use of Pushed Authorization (PAR).
/// </summary>
public enum PushedAuthorizationBehavior
{
/// <summary>
/// Use Pushed Authorization (PAR) if the PAR endpoint is available in the identity provider's discovery document or the explicit <see cref="OpenIdConnectConfiguration"/>. This is the default value.
/// </summary>
UseIfAvailable,
/// <summary>
/// Never use Pushed Authorization (PAR), even if the PAR endpoint is available in the identity provider's discovery document or the explicit <see cref="OpenIdConnectConfiguration"/>.
/// If the identity provider's discovery document indicates that it requires Pushed Authorization (PAR), the handler will fail.
/// </summary>
Disable,
/// <summary>
/// Always use Pushed Authorization (PAR), and emit errors if the PAR endpoint is not available in the identity provider's discovery document or the explicit <see cref="OpenIdConnectConfiguration"/>.
/// </summary>
Require
}
Loading