From 853fa9c04c9862b386f867369c57d31c1ff3ac02 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Wed, 10 Apr 2024 21:31:24 -0500 Subject: [PATCH 01/21] Initial implementation of PAR in OIDC Handler Adds support for pushed authorization requests (RFC 9126) to the OpenIdConnect authentication handler. --- .../samples/OpenIdConnectSample/Startup.cs | 6 +- .../src/Events/OpenIdConnectEvents.cs | 12 ++ .../src/Events/PushedAuthorizationContext.cs | 59 +++++++++ .../OpenIdConnect/src/LoggingExtensions.cs | 6 + .../OpenIdConnect/src/OpenIdConnectHandler.cs | 115 ++++++++++++++++++ .../OpenIdConnect/src/OpenIdConnectOptions.cs | 7 ++ .../OpenIdConnect/src/PublicAPI.Unshipped.txt | 13 ++ 7 files changed, 216 insertions(+), 2 deletions(-) create mode 100644 src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs diff --git a/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Startup.cs b/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Startup.cs index 7c53f799c443..d1afa8bf1763 100644 --- a/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Startup.cs +++ b/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Startup.cs @@ -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")) { @@ -108,6 +108,8 @@ public void ConfigureServices(IServiceCollection services) o.ClientSecret = "secret"; // for code flow o.Authority = "https://demo.duendesoftware.com/"; + o.UsePushedAuthorization = true; + o.ResponseType = OpenIdConnectResponseType.Code; o.SaveTokens = true; o.GetClaimsFromUserInfoEndpoint = true; diff --git a/src/Security/Authentication/OpenIdConnect/src/Events/OpenIdConnectEvents.cs b/src/Security/Authentication/OpenIdConnect/src/Events/OpenIdConnectEvents.cs index c6deddf3b4ec..2a66006bd2dd 100644 --- a/src/Security/Authentication/OpenIdConnect/src/Events/OpenIdConnectEvents.cs +++ b/src/Security/Authentication/OpenIdConnect/src/Events/OpenIdConnectEvents.cs @@ -61,6 +61,11 @@ public class OpenIdConnectEvents : RemoteAuthenticationEvents /// public Func OnUserInformationReceived { get; set; } = context => Task.CompletedTask; + /// + /// Invoked before authorization parameters are pushed using PAR. + /// + public Func OnPushAuthorization { get; set; } = context => Task.CompletedTask; + /// /// Invoked if exceptions are thrown during request processing. The exceptions will be re-thrown after this event unless suppressed. /// @@ -113,4 +118,11 @@ public class OpenIdConnectEvents : RemoteAuthenticationEvents /// Invoked when user information is retrieved from the UserInfoEndpoint. /// public virtual Task UserInformationReceived(UserInformationReceivedContext context) => OnUserInformationReceived(context); + + /// + /// Invoked before authorization parameters are pushed during PAR. + /// + /// + /// + public virtual Task PushAuthorization(PushedAuthorizationContext context) => OnPushAuthorization(context); } diff --git a/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs b/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs new file mode 100644 index 000000000000..0de7d5842a39 --- /dev/null +++ b/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs @@ -0,0 +1,59 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Net.Http; +using Microsoft.AspNetCore.Http; +using Microsoft.IdentityModel.Protocols.OpenIdConnect; + +namespace Microsoft.AspNetCore.Authentication.OpenIdConnect; + +/// +/// A context for . +/// +public class PushedAuthorizationContext : PropertiesContext +{ + /// + /// Initializes a new instance of . + /// + /// + public PushedAuthorizationContext(HttpContext context, AuthenticationScheme scheme, OpenIdConnectOptions options, OpenIdConnectMessage message, HttpRequestMessage parRequest, AuthenticationProperties properties) + : base(context, scheme, options, properties) + => ProtocolMessage = message; + + /// + /// Gets or sets the . + /// + public OpenIdConnectMessage ProtocolMessage { get; } + + /// + /// Indicates if the OnPushAuthorization event chose to handle pushing the + /// authorization request. If true, the handler will not attempt to push the + /// authorization request. + /// + public bool HandledPush { get; private set; } + + /// + /// Tells the handler to skip the process of pushing authorization. The + /// OnPushAuthorization event may have pushed the request or decided that + /// pushing the request was not required. + /// + public void HandlePush() => HandledPush = true; + + /// + /// 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. + /// + public bool HandledClientAuthentication { get; private set; } + + /// + /// 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. + /// + public void HandleClientAuthentication() => HandledClientAuthentication = true; +} + diff --git a/src/Security/Authentication/OpenIdConnect/src/LoggingExtensions.cs b/src/Security/Authentication/OpenIdConnect/src/LoggingExtensions.cs index 3be7240ab4a7..fced206bc471 100644 --- a/src/Security/Authentication/OpenIdConnect/src/LoggingExtensions.cs +++ b/src/Security/Authentication/OpenIdConnect/src/LoggingExtensions.cs @@ -74,6 +74,12 @@ 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, "PushAuthorization.HandledClientAuthentication", EventName = "PushAuthorizationHandledClientAuthentication")] + public static partial void PushAuthorizationHandledClientAuthentication(this ILogger logger); + + [LoggerMessage(58, LogLevel.Debug, "PushAuthorization.HandledPush", EventName = "PushAuthorizationHandledPush")] + public static partial void PushAuthorizationHandledPush(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 f0b93c306727..0464193eb4d7 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs @@ -1,6 +1,7 @@ // 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 System.Globalization; using System.IdentityModel.Tokens.Jwt; using System.Linq; @@ -485,6 +486,13 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert "Cannot redirect to the authorization endpoint, the configuration may be missing or invalid."); } + var parRequired = ConfigFlagEnabled("require_pushed_authoriation_requests"); + + if (Options.UsePushedAuthorization || parRequired) + { + await PushAuthorizationRequest(message, properties); + } + if (Options.AuthenticationMethod == OpenIdConnectRedirectBehavior.RedirectGet) { var redirectUri = message.CreateAuthenticationRequestUrl(); @@ -516,6 +524,113 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert throw new NotImplementedException($"An unsupported authentication method has been configured: {Options.AuthenticationMethod}"); } + private bool GetConfigString(string name, [NotNullWhen(true)] out string? value) + { + if (_configuration?.AdditionalData.TryGetValue(name, out var configValue) ?? false) + { + if (configValue is string v) + { + value = v; + return true; + } + } + value = null; + return false; + } + + private bool ConfigFlagEnabled(string name) + { + if (_configuration?.AdditionalData.TryGetValue(name, out var configValue) ?? false) + { + if (configValue is bool v) + { + return v; + } + } + return false; + } + + private async Task PushAuthorizationRequest(OpenIdConnectMessage authorizeRequest, AuthenticationProperties properties) + { + if (GetConfigString("pushed_authorization_request_endpoint", out var parEndpoint)) + { + // Build context and run event + var parRequest = new HttpRequestMessage(HttpMethod.Post, parEndpoint) + { + Content = new FormUrlEncodedContent(authorizeRequest.Parameters), + }; + var context = new PushedAuthorizationContext(Context, Scheme, Options, authorizeRequest, parRequest, properties); + await Events.PushAuthorization(context); + + // If the event handled auth, we skip out default auth behavior + if (context.HandledClientAuthentication) + { + Logger.PushAuthorizationHandledClientAuthentication(); + } + else + { + SetClientAuthenticationHeader(parRequest); + } + + // If the event handled the push, there's nothing left to do and we bail out + if (context.HandledPush) + { + Logger.PushAuthorizationHandledPush(); + return; + } + + var parResponseMessage = await Backchannel.SendAsync(parRequest, Context.RequestAborted); + + var requestUri = await GetRequestUri(parResponseMessage); + authorizeRequest.Parameters.Clear(); + authorizeRequest.Parameters.Add("client_id", Options.ClientId); + authorizeRequest.Parameters.Add("request_uri", requestUri); + } + else + { + throw new InvalidOperationException("Attempt to push authorization with no pushed authorization endpoint configured."); + } + } + + private async Task GetRequestUri(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(); + var message = new OpenIdConnectMessage(parResponseString); + + var requestUri = message.GetParameter("request_uri"); + if(requestUri == null) + { + throw CreateOpenIdConnectProtocolException(message, parResponseMessage); + } + return requestUri; + } + + private void SetClientAuthenticationHeader(HttpRequestMessage parRequest) + { + if (string.IsNullOrEmpty(Options.ClientId)) + { + throw new InvalidOperationException("Missing client id"); + } + if (string.IsNullOrEmpty(Options.ClientSecret)) + { + throw new InvalidOperationException("Missing client secret"); + } + var escapedClientId = Uri.EscapeDataString(Options.ClientId); + var escapedClientSecret = Uri.EscapeDataString(Options.ClientSecret); + var credential = $"{escapedClientId}:{escapedClientSecret}"; + var encodedCredential = Convert.ToBase64String(Encoding.UTF8.GetBytes(credential)); + + parRequest.Headers.Authorization = new AuthenticationHeaderValue("basic", encodedCredential); + } + /// /// Invoked to process incoming OpenIdConnect messages. /// diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs index f25095862518..43da1a4897ec 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs @@ -408,4 +408,11 @@ public bool MapInboundClaims /// When using TokenHandler, will be a . /// public bool UseSecurityTokenValidator { get; set; } + + /// + /// Flag to set whether the handler should push authorization parameters on the backchannel before + /// redirecting to the identity provider. + /// The default is 'false'. + /// + public bool UsePushedAuthorization { get; set; } } diff --git a/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt b/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt index 49118745ed9c..eda3d9d0acd2 100644 --- a/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt +++ b/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt @@ -1,2 +1,15 @@ #nullable enable +Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectEvents.OnPushAuthorization.get -> System.Func! +Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectEvents.OnPushAuthorization.set -> void Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectOptions.AdditionalAuthorizationParameters.get -> System.Collections.Generic.IDictionary! +Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectOptions.UsePushedAuthorization.get -> bool +Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectOptions.UsePushedAuthorization.set -> void +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() -> void +Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext.ProtocolMessage.get -> Microsoft.IdentityModel.Protocols.OpenIdConnect.OpenIdConnectMessage! +Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext.ProtocolMessage.set -> void +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! message, System.Net.Http.HttpRequestMessage! parRequest, Microsoft.AspNetCore.Authentication.AuthenticationProperties! properties) -> void +virtual Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectEvents.PushAuthorization(Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext! context) -> System.Threading.Tasks.Task! From a75529e1eef6b034074b91500b54964a744b2b88 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Wed, 10 Apr 2024 22:22:36 -0500 Subject: [PATCH 02/21] Fix API declaration --- .../Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt b/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt index eda3d9d0acd2..aebfaf898ce0 100644 --- a/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt +++ b/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt @@ -10,6 +10,5 @@ Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext.Han Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext.HandledPush.get -> bool Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext.HandlePush() -> void Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext.ProtocolMessage.get -> Microsoft.IdentityModel.Protocols.OpenIdConnect.OpenIdConnectMessage! -Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext.ProtocolMessage.set -> void 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! message, System.Net.Http.HttpRequestMessage! parRequest, Microsoft.AspNetCore.Authentication.AuthenticationProperties! properties) -> void virtual Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectEvents.PushAuthorization(Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext! context) -> System.Threading.Tasks.Task! From 8837fa9edbc8b9acb783faa4ebdef869e3189e9e Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Thu, 6 Jun 2024 14:03:31 -0500 Subject: [PATCH 03/21] PAR updates from review --- .../src/Events/PushedAuthorizationContext.cs | 50 ++++++++++++--- .../OpenIdConnect/src/LoggingExtensions.cs | 7 ++- .../OpenIdConnect/src/OpenIdConnectHandler.cs | 62 +++++++++---------- .../OpenIdConnect/src/OpenIdConnectOptions.cs | 2 +- .../OpenIdConnect/src/PublicAPI.Unshipped.txt | 6 +- 5 files changed, 83 insertions(+), 44 deletions(-) diff --git a/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs b/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs index 0de7d5842a39..d852cef98762 100644 --- a/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs +++ b/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs @@ -1,6 +1,7 @@ // 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 System.Net.Http; using Microsoft.AspNetCore.Http; using Microsoft.IdentityModel.Protocols.OpenIdConnect; @@ -18,26 +19,61 @@ public class PushedAuthorizationContext : PropertiesContext public PushedAuthorizationContext(HttpContext context, AuthenticationScheme scheme, OpenIdConnectOptions options, OpenIdConnectMessage message, HttpRequestMessage parRequest, AuthenticationProperties properties) : base(context, scheme, options, properties) - => ProtocolMessage = message; + { + ProtocolMessage = message; + PushedAuthorizationRequest = parRequest; + } /// /// Gets or sets the . /// public OpenIdConnectMessage ProtocolMessage { get; } + public HttpRequestMessage PushedAuthorizationRequest { get; } /// /// Indicates if the OnPushAuthorization event chose to handle pushing the /// authorization request. If true, the handler will not attempt to push the - /// authorization request. + /// authorization request, and will instead use the RequestUri from this + /// event in the subsequent authorize request. + /// + public bool HandledPush { [MemberNotNull("RequestUri")]get; private set; } + + /// + /// 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. + /// + public bool SkippedPush { get; private set; } + + /// + /// 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. + /// + public string? RequestUri { get; private set; } + + /// + /// 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. /// - public bool HandledPush { get; private set; } + [MemberNotNull("RequestUri")] + public void HandlePush(string requestUri) + { + HandledPush = true; + RequestUri = requestUri; + } /// - /// Tells the handler to skip the process of pushing authorization. The - /// OnPushAuthorization event may have pushed the request or decided that - /// pushing the request was not required. + /// Tells the handler to skip pushing authorization entirely. If this is + /// called, the handler will not use pushed authorization on the subsequent + /// authorize call. /// - public void HandlePush() => HandledPush = true; + public void SkipPush() + { + SkippedPush = true; + } /// /// Indicates if the OnPushAuthorization event chose to handle client diff --git a/src/Security/Authentication/OpenIdConnect/src/LoggingExtensions.cs b/src/Security/Authentication/OpenIdConnect/src/LoggingExtensions.cs index fced206bc471..6965d08f9f84 100644 --- a/src/Security/Authentication/OpenIdConnect/src/LoggingExtensions.cs +++ b/src/Security/Authentication/OpenIdConnect/src/LoggingExtensions.cs @@ -74,12 +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, "PushAuthorization.HandledClientAuthentication", EventName = "PushAuthorizationHandledClientAuthentication")] + [LoggerMessage(57, LogLevel.Debug, "The PushAuthorization event handled client authentication", EventName = "PushAuthorizationHandledClientAuthentication")] public static partial void PushAuthorizationHandledClientAuthentication(this ILogger logger); - [LoggerMessage(58, LogLevel.Debug, "PushAuthorization.HandledPush", EventName = "PushAuthorizationHandledPush")] + [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); diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs index 0464193eb4d7..37c028090d2f 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs @@ -486,7 +486,7 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert "Cannot redirect to the authorization endpoint, the configuration may be missing or invalid."); } - var parRequired = ConfigFlagEnabled("require_pushed_authoriation_requests"); + var parRequired = ConfigFlagEnabled("require_pushed_authorization_requests"); if (Options.UsePushedAuthorization || parRequired) { @@ -552,36 +552,50 @@ private bool ConfigFlagEnabled(string name) private async Task PushAuthorizationRequest(OpenIdConnectMessage authorizeRequest, AuthenticationProperties properties) { + // TODO - Make this check happen later, so that the event can decide what to do if there is nothing configured or in disco? if (GetConfigString("pushed_authorization_request_endpoint", out var parEndpoint)) { // Build context and run event - var parRequest = new HttpRequestMessage(HttpMethod.Post, parEndpoint) - { - Content = new FormUrlEncodedContent(authorizeRequest.Parameters), - }; + var parRequest = new HttpRequestMessage(HttpMethod.Post, parEndpoint); var context = new PushedAuthorizationContext(Context, Scheme, Options, authorizeRequest, parRequest, properties); await Events.PushAuthorization(context); - // If the event handled auth, we skip out default auth behavior + // If the event handled client authentication, skip the default auth behavior if (context.HandledClientAuthentication) { Logger.PushAuthorizationHandledClientAuthentication(); } else { - SetClientAuthenticationHeader(parRequest); + // Otherwise, add the client secret to the parameters (if available) + if (!string.IsNullOrEmpty(Options.ClientSecret)) + { + authorizeRequest.Parameters.Add(OpenIdConnectParameterNames.ClientSecret, Options.ClientSecret); + } } - // If the event handled the push, there's nothing left to do and we bail out - if (context.HandledPush) + string requestUri; + + // If the event handled the push, it either means the event will + // supply the request uri, or that it decided to skip the push. + if (context.SkippedPush) { - Logger.PushAuthorizationHandledPush(); + Logger.PushAuthorizationSkippedPush(); return; } + else if (context.HandledPush) + { + Logger.PushAuthorizationHandledPush(); + requestUri = context.RequestUri; + } + else + { + // TODO - Log this too? + parRequest.Content = new FormUrlEncodedContent(authorizeRequest.Parameters); + var parResponseMessage = await Backchannel.SendAsync(parRequest, Context.RequestAborted); + requestUri = await GetPushedAuthorizationRequestUri(parResponseMessage); + } - var parResponseMessage = await Backchannel.SendAsync(parRequest, Context.RequestAborted); - - var requestUri = await GetRequestUri(parResponseMessage); authorizeRequest.Parameters.Clear(); authorizeRequest.Parameters.Add("client_id", Options.ClientId); authorizeRequest.Parameters.Add("request_uri", requestUri); @@ -592,7 +606,7 @@ private async Task PushAuthorizationRequest(OpenIdConnectMessage authorizeReques } } - private async Task GetRequestUri(HttpResponseMessage parResponseMessage) + private async Task GetPushedAuthorizationRequestUri(HttpResponseMessage parResponseMessage) { // Check content type var contentType = parResponseMessage.Content.Headers.ContentType; @@ -606,31 +620,13 @@ private async Task GetRequestUri(HttpResponseMessage parResponseMessage) var message = new OpenIdConnectMessage(parResponseString); var requestUri = message.GetParameter("request_uri"); - if(requestUri == null) + if (requestUri == null) { throw CreateOpenIdConnectProtocolException(message, parResponseMessage); } return requestUri; } - private void SetClientAuthenticationHeader(HttpRequestMessage parRequest) - { - if (string.IsNullOrEmpty(Options.ClientId)) - { - throw new InvalidOperationException("Missing client id"); - } - if (string.IsNullOrEmpty(Options.ClientSecret)) - { - throw new InvalidOperationException("Missing client secret"); - } - var escapedClientId = Uri.EscapeDataString(Options.ClientId); - var escapedClientSecret = Uri.EscapeDataString(Options.ClientSecret); - var credential = $"{escapedClientId}:{escapedClientSecret}"; - var encodedCredential = Convert.ToBase64String(Encoding.UTF8.GetBytes(credential)); - - parRequest.Headers.Authorization = new AuthenticationHeaderValue("basic", encodedCredential); - } - /// /// Invoked to process incoming OpenIdConnect messages. /// diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs index 43da1a4897ec..f1c492751626 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs @@ -411,7 +411,7 @@ public bool MapInboundClaims /// /// Flag to set whether the handler should push authorization parameters on the backchannel before - /// redirecting to the identity provider. + /// redirecting to the identity provider. See . /// The default is 'false'. /// public bool UsePushedAuthorization { get; set; } diff --git a/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt b/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt index aebfaf898ce0..3fc79d5cb893 100644 --- a/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt +++ b/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt @@ -8,7 +8,11 @@ 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() -> void +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! message, System.Net.Http.HttpRequestMessage! parRequest, Microsoft.AspNetCore.Authentication.AuthenticationProperties! properties) -> void +Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext.PushedAuthorizationRequest.get -> System.Net.Http.HttpRequestMessage! +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! From d343623fd5ce2c63b29f4fbaef9ecdd7c0fad5d2 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Fri, 7 Jun 2024 11:29:50 -0500 Subject: [PATCH 04/21] Simplify PAR event context --- .../src/Events/PushedAuthorizationContext.cs | 16 ++-- .../OpenIdConnect/src/OpenIdConnectHandler.cs | 89 ++++++++++--------- .../OpenIdConnect/src/PublicAPI.Unshipped.txt | 3 +- 3 files changed, 53 insertions(+), 55 deletions(-) diff --git a/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs b/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs index d852cef98762..80d6c30bd289 100644 --- a/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs +++ b/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics.CodeAnalysis; -using System.Net.Http; using Microsoft.AspNetCore.Http; using Microsoft.IdentityModel.Protocols.OpenIdConnect; @@ -17,18 +16,16 @@ public class PushedAuthorizationContext : PropertiesContext. /// /// - public PushedAuthorizationContext(HttpContext context, AuthenticationScheme scheme, OpenIdConnectOptions options, OpenIdConnectMessage message, HttpRequestMessage parRequest, AuthenticationProperties properties) + public PushedAuthorizationContext(HttpContext context, AuthenticationScheme scheme, OpenIdConnectOptions options, OpenIdConnectMessage parRequest, AuthenticationProperties properties) : base(context, scheme, options, properties) { - ProtocolMessage = message; - PushedAuthorizationRequest = parRequest; + ProtocolMessage = parRequest; } /// - /// Gets or sets the . + /// Gets or sets the that will be sent to the PAR endpoint. /// public OpenIdConnectMessage ProtocolMessage { get; } - public HttpRequestMessage PushedAuthorizationRequest { get; } /// /// Indicates if the OnPushAuthorization event chose to handle pushing the @@ -54,11 +51,10 @@ public PushedAuthorizationContext(HttpContext context, AuthenticationScheme sche public string? RequestUri { get; private set; } /// - /// 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. + /// 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. /// - [MemberNotNull("RequestUri")] public void HandlePush(string requestUri) { HandledPush = true; diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs index 37c028090d2f..a1d793bcbee1 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs @@ -552,60 +552,63 @@ private bool ConfigFlagEnabled(string name) private async Task PushAuthorizationRequest(OpenIdConnectMessage authorizeRequest, AuthenticationProperties properties) { - // TODO - Make this check happen later, so that the event can decide what to do if there is nothing configured or in disco? - if (GetConfigString("pushed_authorization_request_endpoint", out var parEndpoint)) - { - // Build context and run event - var parRequest = new HttpRequestMessage(HttpMethod.Post, parEndpoint); - var context = new PushedAuthorizationContext(Context, Scheme, Options, authorizeRequest, parRequest, properties); - await Events.PushAuthorization(context); + // 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(); - } - else + // If the event handled client authentication, skip the default auth behavior + if (context.HandledClientAuthentication) + { + Logger.PushAuthorizationHandledClientAuthentication(); + } + else + { + // Otherwise, add the client secret to the parameters (if available) + if (!string.IsNullOrEmpty(Options.ClientSecret)) { - // Otherwise, add the client secret to the parameters (if available) - if (!string.IsNullOrEmpty(Options.ClientSecret)) - { - authorizeRequest.Parameters.Add(OpenIdConnectParameterNames.ClientSecret, Options.ClientSecret); - } + parRequest.Parameters.Add(OpenIdConnectParameterNames.ClientSecret, Options.ClientSecret); } + } - string requestUri; - - // If the event handled the push, it either means the event will - // supply the request uri, or that it decided to skip the push. - if (context.SkippedPush) - { - Logger.PushAuthorizationSkippedPush(); - return; - } - else if (context.HandledPush) - { - Logger.PushAuthorizationHandledPush(); - requestUri = context.RequestUri; - } - else - { - // TODO - Log this too? - parRequest.Content = new FormUrlEncodedContent(authorizeRequest.Parameters); - var parResponseMessage = await Backchannel.SendAsync(parRequest, Context.RequestAborted); - requestUri = await GetPushedAuthorizationRequestUri(parResponseMessage); - } + string requestUri; - authorizeRequest.Parameters.Clear(); - authorizeRequest.Parameters.Add("client_id", Options.ClientId); - authorizeRequest.Parameters.Add("request_uri", requestUri); + // If the event handled the push, it either means the event will + // supply the request uri, or that it decided to skip the push. + if (context.SkippedPush) + { + Logger.PushAuthorizationSkippedPush(); + return; + } + else if (context.HandledPush) + { + Logger.PushAuthorizationHandledPush(); + requestUri = context.RequestUri; } else { - throw new InvalidOperationException("Attempt to push authorization with no pushed authorization endpoint configured."); + GetConfigString("pushed_authorization_request_endpoint", out var parEndpoint); + if (string.IsNullOrEmpty(parEndpoint)) + { + new InvalidOperationException("Attempt to push authorization with no pushed authorization endpoint configured."); + } + + // TODO - If we get support for PAR in wilson, we can replace GetConfigString with something like this: + // var requestMessage = new HttpRequestMessage(HttpMethod.Post, parRequest.ParEndpoint ?? _configuration?.ParEndpoint); + + 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); + requestUri = await GetPushedAuthorizationRequestUri(parResponseMessage); } + + authorizeRequest.Parameters.Clear(); + authorizeRequest.Parameters.Add("client_id", Options.ClientId); + authorizeRequest.Parameters.Add("request_uri", requestUri); } + // TODO Compare with similar use case in RedeemAuthorizationCodeAsync private async Task GetPushedAuthorizationRequestUri(HttpResponseMessage parResponseMessage) { // Check content type diff --git a/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt b/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt index 3fc79d5cb893..5e604c26617a 100644 --- a/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt +++ b/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt @@ -10,8 +10,7 @@ Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext.Han 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! message, System.Net.Http.HttpRequestMessage! parRequest, Microsoft.AspNetCore.Authentication.AuthenticationProperties! properties) -> void -Microsoft.AspNetCore.Authentication.OpenIdConnect.PushedAuthorizationContext.PushedAuthorizationRequest.get -> System.Net.Http.HttpRequestMessage! +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 From 61813a12cb8f2b8f018879a1dd106c9c86ef0cc7 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Fri, 7 Jun 2024 11:52:14 -0500 Subject: [PATCH 05/21] Clarify a few PAR related comments --- .../OpenIdConnect/src/OpenIdConnectHandler.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs index a1d793bcbee1..91796ab67119 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs @@ -562,9 +562,9 @@ private async Task PushAuthorizationRequest(OpenIdConnectMessage authorizeReques { Logger.PushAuthorizationHandledClientAuthentication(); } + // Otherwise, add the client secret to the parameters (if available) else { - // Otherwise, add the client secret to the parameters (if available) if (!string.IsNullOrEmpty(Options.ClientSecret)) { parRequest.Parameters.Add(OpenIdConnectParameterNames.ClientSecret, Options.ClientSecret); @@ -573,13 +573,13 @@ private async Task PushAuthorizationRequest(OpenIdConnectMessage authorizeReques string requestUri; - // If the event handled the push, it either means the event will - // supply the request uri, or that it decided to skip the push. + // 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(); From cdc0796985f6b25a6f956cce51b3cf4d441773f3 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Fri, 7 Jun 2024 15:13:19 -0500 Subject: [PATCH 06/21] Enable PAR by default, if available in disco --- .../OpenIdConnect/src/OpenIdConnectHandler.cs | 9 +++++++-- .../OpenIdConnect/src/OpenIdConnectOptions.cs | 4 ++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs index 91796ab67119..f77335aa9f59 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs @@ -486,9 +486,10 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert "Cannot redirect to the authorization endpoint, the configuration may be missing or invalid."); } - var parRequired = ConfigFlagEnabled("require_pushed_authorization_requests"); + var parRequired = ConfigFlagEnabled("require_pushed_authorization_requests"); + GetConfigString("pushed_authorization_request_endpoint", out var parEndpoint); - if (Options.UsePushedAuthorization || parRequired) + if ((Options.UsePushedAuthorization && !string.IsNullOrEmpty(parEndpoint)) || parRequired) { await PushAuthorizationRequest(message, properties); } @@ -524,6 +525,8 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert throw new NotImplementedException($"An unsupported authentication method has been configured: {Options.AuthenticationMethod}"); } + // TODO GetConfigString and ConfigFlagEnabled are only used in PAR, and won't be necessary after + // https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/pull/2499 merges private bool GetConfigString(string name, [NotNullWhen(true)] out string? value) { if (_configuration?.AdditionalData.TryGetValue(name, out var configValue) ?? false) @@ -538,6 +541,8 @@ private bool GetConfigString(string name, [NotNullWhen(true)] out string? value) return false; } + // TODO GetConfigString and ConfigFlagEnabled are only used in PAR, and won't be necessary after + // https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/pull/2499 merges private bool ConfigFlagEnabled(string name) { if (_configuration?.AdditionalData.TryGetValue(name, out var configValue) ?? false) diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs index f1c492751626..27e4fc0a81d1 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs @@ -412,7 +412,7 @@ public bool MapInboundClaims /// /// Flag to set whether the handler should push authorization parameters on the backchannel before /// redirecting to the identity provider. See . - /// The default is 'false'. + /// The default is 'true'. /// - public bool UsePushedAuthorization { get; set; } + public bool UsePushedAuthorization { get; set; } = true; } From 5d6eff345e42848c24f99ce27d535c6d445278d4 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Fri, 7 Jun 2024 15:14:17 -0500 Subject: [PATCH 07/21] Very minor changes for clarity --- .../src/Events/PushedAuthorizationContext.cs | 24 +++++++++---------- .../OpenIdConnect/src/OpenIdConnectOptions.cs | 7 +++--- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs b/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs index 80d6c30bd289..b886f1fe94cf 100644 --- a/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs +++ b/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs @@ -33,7 +33,18 @@ public PushedAuthorizationContext(HttpContext context, AuthenticationScheme sche /// authorization request, and will instead use the RequestUri from this /// event in the subsequent authorize request. /// - public bool HandledPush { [MemberNotNull("RequestUri")]get; private set; } + public bool HandledPush { [MemberNotNull("RequestUri")] get; private set; } + + /// + /// 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. + /// + public void HandlePush(string requestUri) + { + HandledPush = true; + RequestUri = requestUri; + } /// /// Indicates if the OnPushAuthorization event chose to skip pushing the @@ -50,17 +61,6 @@ public PushedAuthorizationContext(HttpContext context, AuthenticationScheme sche /// public string? RequestUri { get; private set; } - /// - /// 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. - /// - public void HandlePush(string requestUri) - { - HandledPush = true; - RequestUri = requestUri; - } - /// /// Tells the handler to skip pushing authorization entirely. If this is /// called, the handler will not use pushed authorization on the subsequent diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs index 27e4fc0a81d1..19349bd47db4 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs @@ -410,9 +410,10 @@ public bool MapInboundClaims public bool UseSecurityTokenValidator { get; set; } /// - /// Flag to set whether the handler should push authorization parameters on the backchannel before - /// redirecting to the identity provider. See . - /// The default is 'true'. + /// Flag to set whether the handler should push authorization parameters on + /// the backchannel before redirecting to the identity provider, if the + /// identity provider supports pushed authorization. See . The default is 'true'. /// public bool UsePushedAuthorization { get; set; } = true; } From ea017ffd70f883797074113a2952412d4cc83f35 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Wed, 17 Jul 2024 15:34:47 -0500 Subject: [PATCH 08/21] Add enum to control PAR behavior --- .../samples/OpenIdConnectSample/Startup.cs | 2 - .../OpenIdConnect/src/OpenIdConnectHandler.cs | 40 +++++++++++++++---- .../OpenIdConnect/src/OpenIdConnectOptions.cs | 11 ++--- .../OpenIdConnect/src/PublicAPI.Unshipped.txt | 8 +++- .../src/PushedAuthorizationBehavior.cs | 28 +++++++++++++ 5 files changed, 72 insertions(+), 17 deletions(-) create mode 100644 src/Security/Authentication/OpenIdConnect/src/PushedAuthorizationBehavior.cs diff --git a/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Startup.cs b/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Startup.cs index d1afa8bf1763..e64c83ad2191 100644 --- a/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Startup.cs +++ b/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Startup.cs @@ -108,8 +108,6 @@ public void ConfigureServices(IServiceCollection services) o.ClientSecret = "secret"; // for code flow o.Authority = "https://demo.duendesoftware.com/"; - o.UsePushedAuthorization = true; - o.ResponseType = OpenIdConnectResponseType.Code; o.SaveTokens = true; o.GetClaimsFromUserInfoEndpoint = true; diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs index f77335aa9f59..de9233f1a77d 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs @@ -486,12 +486,39 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert "Cannot redirect to the authorization endpoint, the configuration may be missing or invalid."); } - var parRequired = ConfigFlagEnabled("require_pushed_authorization_requests"); GetConfigString("pushed_authorization_request_endpoint", out var parEndpoint); - if ((Options.UsePushedAuthorization && !string.IsNullOrEmpty(parEndpoint)) || parRequired) + switch (Options.PushedAuthorizationBehavior) { - await PushAuthorizationRequest(message, properties); + 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 + var requiredInDiscovery = ConfigFlagEnabled("require_pushed_authorization_requests"); + if (requiredInDiscovery) + { + 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 = !parEndpoint.IsNullOrEmpty(); + if (!endpointIsConfigured) + { + 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) @@ -526,7 +553,7 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert } // TODO GetConfigString and ConfigFlagEnabled are only used in PAR, and won't be necessary after - // https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/pull/2499 merges + // Microsoft.IdentityModel.Protocols.OpenIdConnect is updated to v7.6.1 or later. private bool GetConfigString(string name, [NotNullWhen(true)] out string? value) { if (_configuration?.AdditionalData.TryGetValue(name, out var configValue) ?? false) @@ -542,7 +569,7 @@ private bool GetConfigString(string name, [NotNullWhen(true)] out string? value) } // TODO GetConfigString and ConfigFlagEnabled are only used in PAR, and won't be necessary after - // https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/pull/2499 merges + // Microsoft.IdentityModel.Protocols.OpenIdConnect is updated to v7.6.1 or later. private bool ConfigFlagEnabled(string name) { if (_configuration?.AdditionalData.TryGetValue(name, out var configValue) ?? false) @@ -598,9 +625,6 @@ private async Task PushAuthorizationRequest(OpenIdConnectMessage authorizeReques new InvalidOperationException("Attempt to push authorization with no pushed authorization endpoint configured."); } - // TODO - If we get support for PAR in wilson, we can replace GetConfigString with something like this: - // var requestMessage = new HttpRequestMessage(HttpMethod.Post, parRequest.ParEndpoint ?? _configuration?.ParEndpoint); - var requestMessage = new HttpRequestMessage(HttpMethod.Post, parEndpoint); requestMessage.Content = new FormUrlEncodedContent(parRequest.Parameters); requestMessage.Version = Backchannel.DefaultRequestVersion; diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs index 19349bd47db4..aa1b69caa61c 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs @@ -410,10 +410,11 @@ public bool MapInboundClaims public bool UseSecurityTokenValidator { get; set; } /// - /// Flag to set whether the handler should push authorization parameters on - /// the backchannel before redirecting to the identity provider, if the - /// identity provider supports pushed authorization. See . The default is 'true'. + /// Controls wether the handler should push authorization parameters on the + /// backchannel before redirecting to the identity provider. See . /// - public bool UsePushedAuthorization { get; set; } = true; + /// Defaults to . + public PushedAuthorizationBehavior PushedAuthorizationBehavior { get; set; } = PushedAuthorizationBehavior.UseIfAvailable; } diff --git a/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt b/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt index 5e604c26617a..a9cf371f30ad 100644 --- a/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt +++ b/src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt @@ -2,8 +2,12 @@ Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectEvents.OnPushAuthorization.get -> System.Func! Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectEvents.OnPushAuthorization.set -> void Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectOptions.AdditionalAuthorizationParameters.get -> System.Collections.Generic.IDictionary! -Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectOptions.UsePushedAuthorization.get -> bool -Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectOptions.UsePushedAuthorization.set -> void +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 diff --git a/src/Security/Authentication/OpenIdConnect/src/PushedAuthorizationBehavior.cs b/src/Security/Authentication/OpenIdConnect/src/PushedAuthorizationBehavior.cs new file mode 100644 index 000000000000..ddefa1f61042 --- /dev/null +++ b/src/Security/Authentication/OpenIdConnect/src/PushedAuthorizationBehavior.cs @@ -0,0 +1,28 @@ +// 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; + +/// +/// Enum containing the options for use of Pushed Authorization (PAR). +/// +public enum PushedAuthorizationBehavior + { + /// + /// Use Pushed Authorization (PAR) if the PAR endpoint is available in the identity provider's discovery document or the explicit . This is the default value. + /// + UseIfAvailable, + /// + /// Never use Pushed Authorization (PAR), even if the PAR endpoint is available in the identity provider's discovery document or the explicit . + /// + /// TODO - Document the required by disco but disabled here case + /// + /// + Disable, + /// + /// 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 . + /// + Require + } From 635b6662b2f6009013d5ffe938176613fe43ae30 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Wed, 17 Jul 2024 21:55:43 -0500 Subject: [PATCH 09/21] Added basic tests for pushed authorization --- .../OpenIdConnectChallengeTests.cs | 107 ++++++++++++++++++ .../test/OpenIdConnect/TestSettings.cs | 8 +- 2 files changed, 113 insertions(+), 2 deletions(-) diff --git a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs index f8bfc8a9fb4f..e7e68434eb89 100644 --- a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs +++ b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs @@ -2,6 +2,9 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Net; +using System.Net.Http; +using System.Text; +using System.Web; using Microsoft.AspNetCore.Authentication.OpenIdConnect; using Microsoft.AspNetCore.DataProtection; using Microsoft.Extensions.Logging.Abstractions; @@ -691,4 +694,108 @@ public async Task Challenge_WithAdditionalAuthorizationParameters() settings.ValidateChallengeRedirect(res.Headers.Location); Assert.Contains("prompt=login&audience=https%3A%2F%2Fapi.example.com", res.Headers.Location.Query); } + + [Fact] + public async Task Challenge_WithPushedAuthorization() + { + var mockBackchannel = new ParMockBackchannel(); + var settings = new TestSettings(opt => + { + opt.ClientId = "Test Id"; + + opt.PushedAuthorizationBehavior = PushedAuthorizationBehavior.UseIfAvailable; + // 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"; + opt.Configuration.AdditionalData["pushed_authorization_request_endpoint"] = "https://testauthority/par"; + }, mockBackchannel); + + var server = settings.CreateTestServer(); + var transaction = await server.SendAsync(TestServerBuilder.TestHost + TestServerBuilder.Challenge); + + var res = transaction.Response; + + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + + // We should be redirected with a request_uri and client_id, as per spec + Assert.Contains("request_uri=my_reference_value", res.Headers.Location.Query); + Assert.Contains("client_id=Test%20Id", res.Headers.Location.Query); + + // The request_uri takes the place of all the other usual parameters + Assert.DoesNotContain("redirect_uri", res.Headers.Location.Query); + Assert.DoesNotContain("scope", res.Headers.Location.Query); + + Assert.Contains("redirect_uri", mockBackchannel.PushedParameters); + Assert.Contains("scope", mockBackchannel.PushedParameters); + } + + [Fact] + public async Task Challenge_WithPushedAuthorizationAndAdditionalParameters() + { + var mockBackchannel = new ParMockBackchannel(); + var settings = new TestSettings(opt => + { + opt.ClientId = "Test Id"; + + opt.PushedAuthorizationBehavior = PushedAuthorizationBehavior.UseIfAvailable; + // 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"; + opt.Configuration.AdditionalData["pushed_authorization_request_endpoint"] = "https://testauthority/par"; + + opt.AdditionalAuthorizationParameters.Add("prompt", "login"); // This should get pushed too, so we don't expect to see it in the authorize redirect. + + }, mockBackchannel); + + var server = settings.CreateTestServer(); + var transaction = await server.SendAsync(TestServerBuilder.TestHost + TestServerBuilder.Challenge); + + var res = transaction.Response; + + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + + // We should be redirected with a request_uri and client_id, as per spec + Assert.Contains("request_uri=my_reference_value", res.Headers.Location.Query); + Assert.Contains("client_id=Test%20Id", res.Headers.Location.Query); + + // The request_uri takes the place of all the other usual parameters + Assert.DoesNotContain("prompt", res.Headers.Location.Query); + + Assert.Equal("login", mockBackchannel.PushedParameters["prompt"]); + } +} + +class ParMockBackchannel : HttpMessageHandler +{ + public IDictionary PushedParameters { get; set; } = new Dictionary(); + + protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + if (string.Equals("/tokens", request.RequestUri.AbsolutePath, StringComparison.Ordinal)) + { + return new HttpResponseMessage() + { + Content = + new StringContent("{ \"id_token\": \"my_id_token\", \"access_token\": \"my_access_token\" }", Encoding.ASCII, "application/json") + }; + } + if (string.Equals("/user", request.RequestUri.AbsolutePath, StringComparison.Ordinal)) + { + return new HttpResponseMessage() { Content = new StringContent("{ }", Encoding.ASCII, "application/json") }; + } + if (string.Equals("/par", request.RequestUri.AbsolutePath, StringComparison.Ordinal)) + { + var content = await request.Content.ReadAsStringAsync(); + var query = HttpUtility.ParseQueryString(content); + foreach(string key in query) + { + PushedParameters[key] = query[key]; + } + return new HttpResponseMessage() { Content = new StringContent("{ \"request_uri\": \"my_reference_value\", \"expires_in\": 60}", Encoding.ASCII, "application/json") }; + } + + throw new NotImplementedException(request.RequestUri.ToString()); + } } diff --git a/src/Security/Authentication/test/OpenIdConnect/TestSettings.cs b/src/Security/Authentication/test/OpenIdConnect/TestSettings.cs index 620706f647b0..219ae141f622 100644 --- a/src/Security/Authentication/test/OpenIdConnect/TestSettings.cs +++ b/src/Security/Authentication/test/OpenIdConnect/TestSettings.cs @@ -26,13 +26,17 @@ public TestSettings() : this(configure: null) { } - public TestSettings(Action configure) + public TestSettings(Action configure, HttpMessageHandler backchannel = null) { + if (backchannel == null) + { + backchannel = new MockBackchannel(); + } _configureOptions = o => { configure?.Invoke(o); _options = o; - _options.BackchannelHttpHandler = new MockBackchannel(); + _options.BackchannelHttpHandler = backchannel; }; } From fddc18fe4f7fc16de3a0074bdfaa9367d63bda05 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Wed, 17 Jul 2024 22:02:25 -0500 Subject: [PATCH 10/21] Seal PushedAuthorizationContext --- .../OpenIdConnect/src/Events/PushedAuthorizationContext.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs b/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs index b886f1fe94cf..a72ac7e6f31a 100644 --- a/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs +++ b/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Authentication.OpenIdConnect; /// /// A context for . /// -public class PushedAuthorizationContext : PropertiesContext +public sealed class PushedAuthorizationContext : PropertiesContext { /// /// Initializes a new instance of . From 6f7987c08c17cfb3e2a902176d61803cc92935ae Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Wed, 17 Jul 2024 22:54:47 -0500 Subject: [PATCH 11/21] Add more PAR unit tests --- .../OpenIdConnect/src/OpenIdConnectHandler.cs | 4 +- .../OpenIdConnectChallengeTests.cs | 161 +++++++++++++++++- 2 files changed, 161 insertions(+), 4 deletions(-) diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs index de9233f1a77d..7b3665653023 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs @@ -503,7 +503,7 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert var requiredInDiscovery = ConfigFlagEnabled("require_pushed_authorization_requests"); if (requiredInDiscovery) { - new InvalidOperationException("Pushed authorization is required by the OpenId Connect provider, but disabled by the OpenIdConnectOptions.PushedAuthorizationBehavior."); + throw new InvalidOperationException("Pushed authorization is required by the OpenId Connect provider, but disabled by the OpenIdConnectOptions.PushedAuthorizationBehavior."); } // Otherwise do nothing @@ -513,7 +513,7 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert var endpointIsConfigured = !parEndpoint.IsNullOrEmpty(); if (!endpointIsConfigured) { - new InvalidOperationException("Pushed authorization is required by the OpenIdConnectOptions.PushedAuthorizationBehavior, but no pushed authorization endpoint is available."); + throw new InvalidOperationException("Pushed authorization is required by the OpenIdConnectOptions.PushedAuthorizationBehavior, but no pushed authorization endpoint is available."); } // Otherwise push diff --git a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs index e7e68434eb89..4a72de8f630e 100644 --- a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs +++ b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs @@ -702,6 +702,7 @@ public async Task Challenge_WithPushedAuthorization() var settings = new TestSettings(opt => { opt.ClientId = "Test Id"; + opt.ClientSecret = "secret"; opt.PushedAuthorizationBehavior = PushedAuthorizationBehavior.UseIfAvailable; // Instead of using discovery, this test hard codes the configuration that disco would retrieve. @@ -712,7 +713,7 @@ public async Task Challenge_WithPushedAuthorization() }, mockBackchannel); var server = settings.CreateTestServer(); - var transaction = await server.SendAsync(TestServerBuilder.TestHost + TestServerBuilder.Challenge); + var transaction = await server.SendAsync(ChallengeEndpoint); var res = transaction.Response; @@ -725,9 +726,165 @@ public async Task Challenge_WithPushedAuthorization() // The request_uri takes the place of all the other usual parameters Assert.DoesNotContain("redirect_uri", res.Headers.Location.Query); Assert.DoesNotContain("scope", res.Headers.Location.Query); + Assert.DoesNotContain("client_secret", res.Headers.Location.Query); Assert.Contains("redirect_uri", mockBackchannel.PushedParameters); Assert.Contains("scope", mockBackchannel.PushedParameters); + Assert.Contains("client_secret", mockBackchannel.PushedParameters); + } + + [Fact] + public async Task Challenge_WithPushedAuthorization_RequiredByOptions_EndpointMissingInDiscovery() + { + 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.AdditionalData["pushed_authorization_request_endpoint"]; + }, mockBackchannel); + + var server = settings.CreateTestServer(); + var exception = await Assert.ThrowsAsync(() => server.SendAsync(ChallengeEndpoint)); + Assert.Equal("Pushed authorization is required by the OpenIdConnectOptions.PushedAuthorizationBehavior, but no pushed authorization endpoint is available.", exception.Message); + } + + [Fact] + public async Task Challenge_WithPushedAuthorization_DisabledByOptions_RequiredInDiscovery() + { + var mockBackchannel = new ParMockBackchannel(); + var settings = new TestSettings(opt => + { + opt.ClientId = "Test Id"; + opt.ClientSecret = "secret"; + + opt.PushedAuthorizationBehavior = PushedAuthorizationBehavior.Disable; + // 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"; + + opt.Configuration.AdditionalData["require_pushed_authorization_requests"] = true; + }, mockBackchannel); + + var server = settings.CreateTestServer(); + var exception = await Assert.ThrowsAsync(() => server.SendAsync(ChallengeEndpoint)); + Assert.Equal("Pushed authorization is required by the OpenId Connect provider, but disabled by the OpenIdConnectOptions.PushedAuthorizationBehavior.", exception.Message); + } + + [Fact] + public async Task Challenge_WithPushedAuthorization_Skipped() + { + var mockBackchannel = new ParMockBackchannel(); + var settings = new TestSettings(opt => + { + opt.ClientId = "Test Id"; + + opt.PushedAuthorizationBehavior = PushedAuthorizationBehavior.UseIfAvailable; + // 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"; + opt.Configuration.AdditionalData["pushed_authorization_request_endpoint"] = "https://testauthority/par"; + + opt.Events.OnPushAuthorization = ctx => + { + ctx.SkipPush(); + return Task.CompletedTask; + }; + }, mockBackchannel); + + var server = settings.CreateTestServer(); + var transaction = await server.SendAsync(ChallengeEndpoint); + + var res = transaction.Response; + + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + + // We didn't push, so we don't have a request_uri, and the mock didn't capture any values + Assert.DoesNotContain("request_uri=my_reference_value", res.Headers.Location.Query); + Assert.Empty(mockBackchannel.PushedParameters); + + // All the usual parameters are present + Assert.Contains("redirect_uri", res.Headers.Location.Query); + Assert.Contains("scope", res.Headers.Location.Query); + } + + [Fact] + public async Task Challenge_WithPushedAuthorization_Handled() + { + var mockBackchannel = new ParMockBackchannel(); + var eventProvidedRequestUri = "request_uri_from_event"; + var settings = new TestSettings(opt => + { + opt.ClientId = "Test Id"; + + opt.PushedAuthorizationBehavior = PushedAuthorizationBehavior.UseIfAvailable; + // 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"; + opt.Configuration.AdditionalData["pushed_authorization_request_endpoint"] = "https://testauthority/par"; + + opt.Events.OnPushAuthorization = ctx => + { + ctx.HandlePush(eventProvidedRequestUri); + return Task.CompletedTask; + }; + }, mockBackchannel); + + var server = settings.CreateTestServer(); + var transaction = await server.SendAsync(ChallengeEndpoint); + + var res = transaction.Response; + + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + + Assert.Contains("request_uri=request_uri_from_event", res.Headers.Location.Query); + Assert.Empty(mockBackchannel.PushedParameters); + } + + [Fact] + public async Task Challenge_WithPushedAuthorization_HandleClientAuthentication() + { + var mockBackchannel = new ParMockBackchannel(); + var settings = new TestSettings(opt => + { + opt.ClientId = "Test Id"; + opt.ClientSecret = "secret"; + + opt.PushedAuthorizationBehavior = PushedAuthorizationBehavior.UseIfAvailable; + // 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"; + opt.Configuration.AdditionalData["pushed_authorization_request_endpoint"] = "https://testauthority/par"; + + opt.Events.OnPushAuthorization = ctx => + { + ctx.HandleClientAuthentication(); + return Task.CompletedTask; + }; + }, mockBackchannel); + + var server = settings.CreateTestServer(); + var transaction = await server.SendAsync(ChallengeEndpoint); + + var res = transaction.Response; + + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + + // No secret is set, since the event handled it + Assert.DoesNotContain("scope", res.Headers.Location.Query); + Assert.DoesNotContain("client_secret", mockBackchannel.PushedParameters); } [Fact] @@ -750,7 +907,7 @@ public async Task Challenge_WithPushedAuthorizationAndAdditionalParameters() }, mockBackchannel); var server = settings.CreateTestServer(); - var transaction = await server.SendAsync(TestServerBuilder.TestHost + TestServerBuilder.Challenge); + var transaction = await server.SendAsync(ChallengeEndpoint); var res = transaction.Response; From b62ea102c0c4e8e2d97586b8a5c933785d2e0373 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Wed, 17 Jul 2024 23:04:04 -0500 Subject: [PATCH 12/21] Clean up compiler warnings --- .../OpenIdConnectSample.csproj | 2 +- .../OpenIdConnect/src/OpenIdConnectHandler.cs | 2 +- .../OpenIdConnectEventTests_Handler.cs | 31 +++++++++++++++++++ 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/OpenIdConnectSample.csproj b/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/OpenIdConnectSample.csproj index 9381309a7e0f..d86299175e8b 100644 --- a/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/OpenIdConnectSample.csproj +++ b/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/OpenIdConnectSample.csproj @@ -2,7 +2,7 @@ $(DefaultNetCoreTargetFramework) - aspnet5-OpenIdConnectSample-20151210110318 + OutOfProcess diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs index 7b3665653023..3fc5845276a4 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs @@ -510,7 +510,7 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert break; case PushedAuthorizationBehavior.Require: // Fail if required in options but unavailable in disco - var endpointIsConfigured = !parEndpoint.IsNullOrEmpty(); + 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."); diff --git a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectEventTests_Handler.cs b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectEventTests_Handler.cs index 7279b02a724a..f06243c23e5b 100644 --- a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectEventTests_Handler.cs +++ b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectEventTests_Handler.cs @@ -1134,6 +1134,22 @@ public async Task OnRedirectToSignedOutRedirectUri_Skipped_NoRedirect() events.ValidateExpectations(); } + [Fact] + public void OnPushAuthorization_SkipPush_DoesNotPush() + { + var events = new ExpectedOidcEvents + { + ExpectPushAuthorization = true + }; + events.OnPushAuthorization = context => + { + context.SkipPush(); + return Task.CompletedTask; + }; + var server = CreateServer(events, AppNotImpl); + + } + private class ExpectedOidcEvents : OpenIdConnectEvents { public bool ExpectMessageReceived { get; set; } @@ -1172,6 +1188,9 @@ private class ExpectedOidcEvents : OpenIdConnectEvents public bool ExpectRedirectToSignedOut { get; set; } public bool InvokedRedirectToSignedOut { get; set; } + public bool ExpectPushAuthorization { get; set; } + public bool InvokedPushAuthorization { get; set; } + public override Task MessageReceived(MessageReceivedContext context) { InvokedMessageReceived = true; @@ -1244,6 +1263,12 @@ public override Task SignedOutCallbackRedirect(RemoteSignOutContext context) return base.SignedOutCallbackRedirect(context); } + public override Task PushAuthorization(PushedAuthorizationContext context) + { + InvokedPushAuthorization = true; + return base.PushAuthorization(context); + } + public void ValidateExpectations() { Assert.Equal(ExpectMessageReceived, InvokedMessageReceived); @@ -1258,6 +1283,7 @@ public void ValidateExpectations() Assert.Equal(ExpectRedirectForSignOut, InvokedRedirectForSignOut); Assert.Equal(ExpectRemoteSignOut, InvokedRemoteSignOut); Assert.Equal(ExpectRedirectToSignedOut, InvokedRedirectToSignedOut); + Assert.Equal(ExpectPushAuthorization, InvokedPushAuthorization); } } @@ -1285,6 +1311,7 @@ private TestServer CreateServer(OpenIdConnectEvents events, RequestDelegate appC UserInfoEndpoint = "http://testhost/user", EndSessionEndpoint = "http://testhost/end" }; + // o.Configuration.AdditionalData["pushed_authorization_request_endpoint"] = "http://testhost/par"; o.StateDataFormat = new TestStateDataFormat(); o.UseSecurityTokenValidator = false; o.TokenHandler = new TestTokenHandler(); @@ -1397,6 +1424,10 @@ protected override Task SendAsync(HttpRequestMessage reques { return Task.FromResult(new HttpResponseMessage() { Content = new StringContent("{ }", Encoding.ASCII, "application/json") }); } + // if (string.Equals("/par", request.RequestUri.AbsolutePath, StringComparison.Ordinal)) + // { + // return Task.FromResult(new HttpResponseMessage() { Content = new StringContent("{ \"request_uri\": \"urn:ietf:params:oauth:request_uri:my_reference_value\", \"expires_in\": 60}", Encoding.ASCII, "application/json") }); + // } throw new NotImplementedException(request.RequestUri.ToString()); } From a202e8c411715e634593ae00a34c1c76f4a828ab Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Thu, 18 Jul 2024 08:22:06 -0500 Subject: [PATCH 13/21] Revert commented out config in sample --- .../samples/OpenIdConnectSample/OpenIdConnectSample.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/OpenIdConnectSample.csproj b/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/OpenIdConnectSample.csproj index d86299175e8b..bab8fec2acb2 100644 --- a/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/OpenIdConnectSample.csproj +++ b/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/OpenIdConnectSample.csproj @@ -2,7 +2,7 @@ $(DefaultNetCoreTargetFramework) - + aspnet5-OpenIdConnectSample-20151210110318 OutOfProcess From 9ed07b3f05e171739b0a1b959fc65c2ba4baa40f Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Thu, 18 Jul 2024 08:42:38 -0500 Subject: [PATCH 14/21] Revert work in progress test --- .../OpenIdConnectEventTests_Handler.cs | 30 ------------------- 1 file changed, 30 deletions(-) diff --git a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectEventTests_Handler.cs b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectEventTests_Handler.cs index f06243c23e5b..afd010a6faeb 100644 --- a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectEventTests_Handler.cs +++ b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectEventTests_Handler.cs @@ -1134,22 +1134,6 @@ public async Task OnRedirectToSignedOutRedirectUri_Skipped_NoRedirect() events.ValidateExpectations(); } - [Fact] - public void OnPushAuthorization_SkipPush_DoesNotPush() - { - var events = new ExpectedOidcEvents - { - ExpectPushAuthorization = true - }; - events.OnPushAuthorization = context => - { - context.SkipPush(); - return Task.CompletedTask; - }; - var server = CreateServer(events, AppNotImpl); - - } - private class ExpectedOidcEvents : OpenIdConnectEvents { public bool ExpectMessageReceived { get; set; } @@ -1188,9 +1172,6 @@ private class ExpectedOidcEvents : OpenIdConnectEvents public bool ExpectRedirectToSignedOut { get; set; } public bool InvokedRedirectToSignedOut { get; set; } - public bool ExpectPushAuthorization { get; set; } - public bool InvokedPushAuthorization { get; set; } - public override Task MessageReceived(MessageReceivedContext context) { InvokedMessageReceived = true; @@ -1263,12 +1244,6 @@ public override Task SignedOutCallbackRedirect(RemoteSignOutContext context) return base.SignedOutCallbackRedirect(context); } - public override Task PushAuthorization(PushedAuthorizationContext context) - { - InvokedPushAuthorization = true; - return base.PushAuthorization(context); - } - public void ValidateExpectations() { Assert.Equal(ExpectMessageReceived, InvokedMessageReceived); @@ -1283,7 +1258,6 @@ public void ValidateExpectations() Assert.Equal(ExpectRedirectForSignOut, InvokedRedirectForSignOut); Assert.Equal(ExpectRemoteSignOut, InvokedRemoteSignOut); Assert.Equal(ExpectRedirectToSignedOut, InvokedRedirectToSignedOut); - Assert.Equal(ExpectPushAuthorization, InvokedPushAuthorization); } } @@ -1424,10 +1398,6 @@ protected override Task SendAsync(HttpRequestMessage reques { return Task.FromResult(new HttpResponseMessage() { Content = new StringContent("{ }", Encoding.ASCII, "application/json") }); } - // if (string.Equals("/par", request.RequestUri.AbsolutePath, StringComparison.Ordinal)) - // { - // return Task.FromResult(new HttpResponseMessage() { Content = new StringContent("{ \"request_uri\": \"urn:ietf:params:oauth:request_uri:my_reference_value\", \"expires_in\": 60}", Encoding.ASCII, "application/json") }); - // } throw new NotImplementedException(request.RequestUri.ToString()); } From 3261d7cad7df638b7121325b50d27bc0e7c24a2b Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Thu, 18 Jul 2024 08:44:21 -0500 Subject: [PATCH 15/21] Clean up test --- .../test/OpenIdConnect/OpenIdConnectEventTests_Handler.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectEventTests_Handler.cs b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectEventTests_Handler.cs index afd010a6faeb..7279b02a724a 100644 --- a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectEventTests_Handler.cs +++ b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectEventTests_Handler.cs @@ -1285,7 +1285,6 @@ private TestServer CreateServer(OpenIdConnectEvents events, RequestDelegate appC UserInfoEndpoint = "http://testhost/user", EndSessionEndpoint = "http://testhost/end" }; - // o.Configuration.AdditionalData["pushed_authorization_request_endpoint"] = "http://testhost/par"; o.StateDataFormat = new TestStateDataFormat(); o.UseSecurityTokenValidator = false; o.TokenHandler = new TestTokenHandler(); From 388de5b4a54778cea049cb2a939dbd8b9274e075 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Thu, 18 Jul 2024 09:00:30 -0500 Subject: [PATCH 16/21] Disallow multiple PAR context actions You can indicate if the event handled authentication, handled the push entirely, or opted out of pushing. Doing more than 1 of those is at best redundant but often ambiguous, so it is always disallowed. --- .../src/Events/PushedAuthorizationContext.cs | 17 +++++++- .../OpenIdConnectChallengeTests.cs | 41 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs b/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs index a72ac7e6f31a..97d6883cb7a2 100644 --- a/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs +++ b/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs @@ -42,6 +42,10 @@ public PushedAuthorizationContext(HttpContext context, AuthenticationScheme sche /// public void HandlePush(string requestUri) { + if (SkippedPush || HandledClientAuthentication) + { + throw new InvalidOperationException("TODO MESSASGE"); + } HandledPush = true; RequestUri = requestUri; } @@ -68,6 +72,10 @@ public void HandlePush(string requestUri) /// public void SkipPush() { + if (HandledPush || HandledClientAuthentication) + { + throw new InvalidOperationException("TODO MESSASGE"); + } SkippedPush = true; } @@ -86,6 +94,13 @@ public void SkipPush() /// replace that with an alternative authentication mode, such as /// private_key_jwt. /// - public void HandleClientAuthentication() => HandledClientAuthentication = true; + public void HandleClientAuthentication() + { + if (SkippedPush || HandledPush) + { + throw new InvalidOperationException("TODO MESSASGE"); + } + HandledClientAuthentication = true; + } } diff --git a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs index 4a72de8f630e..6fd4ec43f572 100644 --- a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs +++ b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs @@ -922,6 +922,47 @@ public async Task Challenge_WithPushedAuthorizationAndAdditionalParameters() Assert.Equal("login", mockBackchannel.PushedParameters["prompt"]); } + + [Theory] + [InlineData(true, true, true)] + [InlineData(true, true, false)] + [InlineData(true, false, true)] + [InlineData(false, true, true)] + public async Task Challenge_WithPushedAuthorization_MultipleContextActionsAreNotAllowed(bool handlePush, bool skipPush, bool handleAuth) + { + var mockBackchannel = new ParMockBackchannel(); + var settings = new TestSettings(opt => + { + opt.ClientId = "Test Id"; + + // 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"; + opt.Configuration.AdditionalData["pushed_authorization_request_endpoint"] = "https://testauthority/par"; + + opt.Events.OnPushAuthorization = ctx => + { + if (handlePush) + { + ctx.HandlePush("some request uri"); + } + if (skipPush) + { + ctx.SkipPush(); + } + if (handleAuth) + { + ctx.HandleClientAuthentication(); + } + return Task.CompletedTask; + }; + + }, mockBackchannel); + + var server = settings.CreateTestServer(); + await Assert.ThrowsAsync(() => server.SendAsync(ChallengeEndpoint)); + } } class ParMockBackchannel : HttpMessageHandler From d599a816641c261a27270bde724de2ed2e4ac724 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Thu, 18 Jul 2024 15:45:57 -0500 Subject: [PATCH 17/21] Update src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs Co-authored-by: Stephen Halter --- .../Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs index 3fc5845276a4..2dd6cc4e2dfc 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs @@ -637,7 +637,6 @@ private async Task PushAuthorizationRequest(OpenIdConnectMessage authorizeReques authorizeRequest.Parameters.Add("request_uri", requestUri); } - // TODO Compare with similar use case in RedeemAuthorizationCodeAsync private async Task GetPushedAuthorizationRequestUri(HttpResponseMessage parResponseMessage) { // Check content type From 637116533fde2d619418dd55ba669674082d383d Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Thu, 18 Jul 2024 15:46:36 -0500 Subject: [PATCH 18/21] Update src/Security/Authentication/OpenIdConnect/src/PushedAuthorizationBehavior.cs Co-authored-by: Stephen Halter --- .../OpenIdConnect/src/PushedAuthorizationBehavior.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Security/Authentication/OpenIdConnect/src/PushedAuthorizationBehavior.cs b/src/Security/Authentication/OpenIdConnect/src/PushedAuthorizationBehavior.cs index ddefa1f61042..55b3bd9b3159 100644 --- a/src/Security/Authentication/OpenIdConnect/src/PushedAuthorizationBehavior.cs +++ b/src/Security/Authentication/OpenIdConnect/src/PushedAuthorizationBehavior.cs @@ -16,9 +16,7 @@ public enum PushedAuthorizationBehavior UseIfAvailable, /// /// Never use Pushed Authorization (PAR), even if the PAR endpoint is available in the identity provider's discovery document or the explicit . - /// - /// TODO - Document the required by disco but disabled here case - /// + /// If the identity provider's discovery document indicates that it requires Pushed Authorization (PAR), the handler will fail. /// Disable, /// From 9645f05e642751ca928d5d49dee32f0003440ab0 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Thu, 18 Jul 2024 15:47:11 -0500 Subject: [PATCH 19/21] Update src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs Co-authored-by: Stephen Halter --- .../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 2dd6cc4e2dfc..c5e1a512f93e 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs @@ -647,7 +647,7 @@ private async Task GetPushedAuthorizationRequestUri(HttpResponseMessage } // Parse response - var parResponseString = await parResponseMessage.Content.ReadAsStringAsync(); + var parResponseString = await parResponseMessage.Content.ReadAsStringAsync(Context.RequestAborted); var message = new OpenIdConnectMessage(parResponseString); var requestUri = message.GetParameter("request_uri"); From 5adc108e3314b3f2555a2a6231aa9fd45d8ef1ef Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Thu, 18 Jul 2024 15:54:29 -0500 Subject: [PATCH 20/21] Fix exception messages when multiple PAR event methods are called --- .../OpenIdConnect/src/Events/PushedAuthorizationContext.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs b/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs index 97d6883cb7a2..97236694174d 100644 --- a/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs +++ b/src/Security/Authentication/OpenIdConnect/src/Events/PushedAuthorizationContext.cs @@ -44,7 +44,7 @@ public void HandlePush(string requestUri) { if (SkippedPush || HandledClientAuthentication) { - throw new InvalidOperationException("TODO MESSASGE"); + throw new InvalidOperationException("Only one of HandlePush, SkipPush, and HandledClientAuthentication may be called in the OnPushAuthorization event."); } HandledPush = true; RequestUri = requestUri; @@ -74,7 +74,7 @@ public void SkipPush() { if (HandledPush || HandledClientAuthentication) { - throw new InvalidOperationException("TODO MESSASGE"); + throw new InvalidOperationException("Only one of HandlePush, SkipPush, and HandledClientAuthentication may be called in the OnPushAuthorization event."); } SkippedPush = true; } @@ -98,7 +98,7 @@ public void HandleClientAuthentication() { if (SkippedPush || HandledPush) { - throw new InvalidOperationException("TODO MESSASGE"); + throw new InvalidOperationException("Only one of HandlePush, SkipPush, and HandledClientAuthentication may be called in the OnPushAuthorization event."); } HandledClientAuthentication = true; } From 20bcf338288b7214f7b29ee3b6f81cd1aeafdc13 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Thu, 18 Jul 2024 20:08:57 -0500 Subject: [PATCH 21/21] Use PAR support from System.IdentityModel to simplify discovery --- .../OpenIdConnect/src/OpenIdConnectHandler.cs | 38 ++----------------- .../OpenIdConnectChallengeTests.cs | 16 ++++---- 2 files changed, 11 insertions(+), 43 deletions(-) diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs index c5e1a512f93e..fb20d8918cb4 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs @@ -1,7 +1,6 @@ // 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 System.Globalization; using System.IdentityModel.Tokens.Jwt; using System.Linq; @@ -486,7 +485,7 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert "Cannot redirect to the authorization endpoint, the configuration may be missing or invalid."); } - GetConfigString("pushed_authorization_request_endpoint", out var parEndpoint); + var parEndpoint = _configuration?.PushedAuthorizationRequestEndpoint; switch (Options.PushedAuthorizationBehavior) { @@ -500,8 +499,7 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert break; case PushedAuthorizationBehavior.Disable: // Fail if disabled in options but required by disco - var requiredInDiscovery = ConfigFlagEnabled("require_pushed_authorization_requests"); - if (requiredInDiscovery) + if (_configuration?.RequirePushedAuthorizationRequests == true) { throw new InvalidOperationException("Pushed authorization is required by the OpenId Connect provider, but disabled by the OpenIdConnectOptions.PushedAuthorizationBehavior."); } @@ -552,36 +550,6 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert throw new NotImplementedException($"An unsupported authentication method has been configured: {Options.AuthenticationMethod}"); } - // TODO GetConfigString and ConfigFlagEnabled are only used in PAR, and won't be necessary after - // Microsoft.IdentityModel.Protocols.OpenIdConnect is updated to v7.6.1 or later. - private bool GetConfigString(string name, [NotNullWhen(true)] out string? value) - { - if (_configuration?.AdditionalData.TryGetValue(name, out var configValue) ?? false) - { - if (configValue is string v) - { - value = v; - return true; - } - } - value = null; - return false; - } - - // TODO GetConfigString and ConfigFlagEnabled are only used in PAR, and won't be necessary after - // Microsoft.IdentityModel.Protocols.OpenIdConnect is updated to v7.6.1 or later. - private bool ConfigFlagEnabled(string name) - { - if (_configuration?.AdditionalData.TryGetValue(name, out var configValue) ?? false) - { - if (configValue is bool v) - { - return v; - } - } - return false; - } - private async Task PushAuthorizationRequest(OpenIdConnectMessage authorizeRequest, AuthenticationProperties properties) { // Build context and run event @@ -619,7 +587,7 @@ private async Task PushAuthorizationRequest(OpenIdConnectMessage authorizeReques } else { - GetConfigString("pushed_authorization_request_endpoint", out var parEndpoint); + var parEndpoint = _configuration?.PushedAuthorizationRequestEndpoint; if (string.IsNullOrEmpty(parEndpoint)) { new InvalidOperationException("Attempt to push authorization with no pushed authorization endpoint configured."); diff --git a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs index 6fd4ec43f572..abc76954193c 100644 --- a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs +++ b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs @@ -709,7 +709,7 @@ public async Task Challenge_WithPushedAuthorization() // This makes it easier to manipulate the discovery results opt.Configuration = new OpenIdConnectConfiguration(); opt.Configuration.AuthorizationEndpoint = "https://testauthority/authorize"; - opt.Configuration.AdditionalData["pushed_authorization_request_endpoint"] = "https://testauthority/par"; + opt.Configuration.PushedAuthorizationRequestEndpoint = "https://testauthority/par"; }, mockBackchannel); var server = settings.CreateTestServer(); @@ -749,7 +749,7 @@ public async Task Challenge_WithPushedAuthorization_RequiredByOptions_EndpointMi opt.Configuration.AuthorizationEndpoint = "https://testauthority/authorize"; // We are NOT setting the endpoint (as if the disco document didn't contain it) - //opt.Configuration.AdditionalData["pushed_authorization_request_endpoint"]; + //opt.Configuration.PushedAuthorizationRequestEndpoint; }, mockBackchannel); var server = settings.CreateTestServer(); @@ -772,7 +772,7 @@ public async Task Challenge_WithPushedAuthorization_DisabledByOptions_RequiredIn opt.Configuration = new OpenIdConnectConfiguration(); opt.Configuration.AuthorizationEndpoint = "https://testauthority/authorize"; - opt.Configuration.AdditionalData["require_pushed_authorization_requests"] = true; + opt.Configuration.RequirePushedAuthorizationRequests = true; }, mockBackchannel); var server = settings.CreateTestServer(); @@ -793,7 +793,7 @@ public async Task Challenge_WithPushedAuthorization_Skipped() // This makes it easier to manipulate the discovery results opt.Configuration = new OpenIdConnectConfiguration(); opt.Configuration.AuthorizationEndpoint = "https://testauthority/authorize"; - opt.Configuration.AdditionalData["pushed_authorization_request_endpoint"] = "https://testauthority/par"; + opt.Configuration.PushedAuthorizationRequestEndpoint = "https://testauthority/par"; opt.Events.OnPushAuthorization = ctx => { @@ -832,7 +832,7 @@ public async Task Challenge_WithPushedAuthorization_Handled() // This makes it easier to manipulate the discovery results opt.Configuration = new OpenIdConnectConfiguration(); opt.Configuration.AuthorizationEndpoint = "https://testauthority/authorize"; - opt.Configuration.AdditionalData["pushed_authorization_request_endpoint"] = "https://testauthority/par"; + opt.Configuration.PushedAuthorizationRequestEndpoint = "https://testauthority/par"; opt.Events.OnPushAuthorization = ctx => { @@ -866,7 +866,7 @@ public async Task Challenge_WithPushedAuthorization_HandleClientAuthentication() // This makes it easier to manipulate the discovery results opt.Configuration = new OpenIdConnectConfiguration(); opt.Configuration.AuthorizationEndpoint = "https://testauthority/authorize"; - opt.Configuration.AdditionalData["pushed_authorization_request_endpoint"] = "https://testauthority/par"; + opt.Configuration.PushedAuthorizationRequestEndpoint = "https://testauthority/par"; opt.Events.OnPushAuthorization = ctx => { @@ -900,7 +900,7 @@ public async Task Challenge_WithPushedAuthorizationAndAdditionalParameters() // This makes it easier to manipulate the discovery results opt.Configuration = new OpenIdConnectConfiguration(); opt.Configuration.AuthorizationEndpoint = "https://testauthority/authorize"; - opt.Configuration.AdditionalData["pushed_authorization_request_endpoint"] = "https://testauthority/par"; + opt.Configuration.PushedAuthorizationRequestEndpoint = "https://testauthority/par"; opt.AdditionalAuthorizationParameters.Add("prompt", "login"); // This should get pushed too, so we don't expect to see it in the authorize redirect. @@ -939,7 +939,7 @@ public async Task Challenge_WithPushedAuthorization_MultipleContextActionsAreNot // This makes it easier to manipulate the discovery results opt.Configuration = new OpenIdConnectConfiguration(); opt.Configuration.AuthorizationEndpoint = "https://testauthority/authorize"; - opt.Configuration.AdditionalData["pushed_authorization_request_endpoint"] = "https://testauthority/par"; + opt.Configuration.PushedAuthorizationRequestEndpoint = "https://testauthority/par"; opt.Events.OnPushAuthorization = ctx => {