Updates to PAR redirect uri handling from review

hosts/main/IdentityServerExtensions.cs

@@ -32,6 +32,7 @@ internal static WebApplicationBuilder ConfigureIdentityServer(this WebApplicatio
                 options.UserInteraction.CreateAccountUrl = "/Account/Create";
 
                 options.Endpoints.EnablePushedAuthorizationEndpoint = true;
+                options.PushedAuthorization.AllowUnregisteredPushedRedirectUris = true;
             })
             //.AddServerSideSessions()
             .AddInMemoryClients(Clients.Get().ToList())
@@ -59,8 +60,7 @@ internal static WebApplicationBuilder ConfigureIdentityServer(this WebApplicatio
                     ResponseType = "id_token",
                     Scope = "openid profile"
                 }
-            })
-            .AddParRedirectUriValidator();
+            });
 
 
         builder.Services.AddDistributedMemoryCache();

src/IdentityServer/Configuration/DependencyInjection/BuilderExtensions/Additional.cs

@@ -61,17 +61,6 @@ public static IIdentityServerBuilder AddAppAuthRedirectUriValidator(this IIdenti
         return builder.AddRedirectUriValidator<StrictRedirectUriValidatorAppAuth>();
     }
 
-    /// <summary>
-    /// Adds a PAR compliant redirect URI validator (does strict validation but
-    /// also allows arbitrary pushed redirect uris).
-    /// </summary>
-    /// <param name="builder">The builder.</param>
-    /// <returns></returns>
-    public static IIdentityServerBuilder AddParRedirectUriValidator(this IIdentityServerBuilder builder)
-    {
-        return builder.AddRedirectUriValidator<ParRedirectUriValidator>();
-    }
-
     /// <summary>
     /// Adds the resource owner validator.
     /// </summary>

src/IdentityServer/Configuration/DependencyInjection/Options/IdentityServerOptions.cs

@@ -4,7 +4,6 @@
 
 #nullable enable
 
-using Duende.IdentityServer.Configuration.DependencyInjection.Options;
 using Duende.IdentityServer.Stores.Serialization;
 
 namespace Duende.IdentityServer.Configuration;

src/IdentityServer/Configuration/DependencyInjection/Options/PushedAuthorizationOptions.cs

@@ -2,13 +2,7 @@
 // See LICENSE in the project root for license information.
 
 
-using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using System.Threading.Tasks;
-
-namespace Duende.IdentityServer.Configuration.DependencyInjection.Options;
+namespace Duende.IdentityServer.Configuration;
 
 /// <summary>
 /// The Pushed Authorization Options.
@@ -45,5 +39,11 @@ public class PushedAuthorizationOptions
     /// precedence over this global configuration.
     /// </remarks>
     public int Lifetime { get; set; } = 60*10;
+
+    /// <summary>
+    /// Specifies whether clients may use redirect uris that were not previously
+    /// registered. 
+    /// </summary>
+    public bool AllowUnregisteredPushedRedirectUris { get; set; }
 }
 

src/IdentityServer/Validation/Default/StrictRedirectUriValidator.cs

@@ -2,6 +2,7 @@
 // See LICENSE in the project root for license information.
 
 
+using Duende.IdentityServer.Configuration;
 using Duende.IdentityServer.Extensions;
 using Duende.IdentityServer.Models;
 using System;
@@ -17,6 +18,17 @@ namespace Duende.IdentityServer.Validation;
 /// </summary>
 public class StrictRedirectUriValidator : IRedirectUriValidator
 {
+    private readonly IdentityServerOptions _options;
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="StrictRedirectUriValidator" />.
+    /// </summary>
+    /// <param name="options"></param>
+    public StrictRedirectUriValidator(IdentityServerOptions options)
+    {
+        _options = options;
+    }
+
     /// <summary>
     /// Checks if a given URI string is in a collection of strings (using ordinal ignore case comparison)
     /// </summary>
@@ -64,10 +76,19 @@ public virtual Task<bool> IsPostLogoutRedirectUriValidAsync(string requestedUri,
     ///   <c>true</c> is the URI is valid; <c>false</c> otherwise.
     /// </returns>
     public virtual Task<bool> IsRedirectUriValidAsync(RedirectUriValidationContext context)
-        // This method is identical to the default implementation on
-        // IRedirectUriValidator, but is virtual so that derived classes can
-        // override it. Leaving the default implementation in the interface
-        // avoids a breaking change for customizations, even though our default
-        // implementations no longer need it.
-        => IsRedirectUriValidAsync(context.RequestedUri, context.Client);
+    {
+        // Check if special case handling for PAR is enabled and that the client
+        // is a confidential client. If so, any pushed redirect uri is allowed
+        // on the PAR endpoint and at the authorize endpoint (if a redirect uri
+        // was pushed)
+        if (_options.PushedAuthorization.AllowUnregisteredPushedRedirectUris &&
+            context.Client.RequireClientSecret && 
+            (context.AuthorizeRequestType == AuthorizeRequestType.PushedAuthorization ||
+             context.AuthorizeRequestType == AuthorizeRequestType.AuthorizeWithPushedParameters))
+        {
+            return Task.FromResult(true);
+        }
+        // Otherwise, just use the default strict validation
+        return Task.FromResult(StringCollectionContainsString(context.Client.RedirectUris, context.RequestedUri));
+    }
 }

src/IdentityServer/Validation/Default/StrictRedirectUriValidatorAppAuth.cs

@@ -1,7 +1,8 @@
-// Copyright (c) Duende Software. All rights reserved.
+// Copyright (c) Duende Software. All rights reserved.
 // See LICENSE in the project root for license information.
 
 
+using Duende.IdentityServer.Configuration;
 using Duende.IdentityServer.Models;
 using Microsoft.Extensions.Logging;
 using System;
@@ -21,7 +22,9 @@ public class StrictRedirectUriValidatorAppAuth : StrictRedirectUriValidator
     /// Initializes a new instance of the <see cref="StrictRedirectUriValidatorAppAuth"/> class.
     /// </summary>
     /// <param name="logger">The logger.</param>
-    public StrictRedirectUriValidatorAppAuth(ILogger<StrictRedirectUriValidatorAppAuth> logger)
+    /// <param name="options">The options.</param>
+    public StrictRedirectUriValidatorAppAuth(ILogger<StrictRedirectUriValidatorAppAuth> logger, IdentityServerOptions options)
+        : base(options)
     {
         _logger = logger;
     }

src/IdentityServer/Validation/IRedirectUriValidator.cs

@@ -5,6 +5,7 @@
 #nullable enable
 
 using Duende.IdentityServer.Models;
+using System;
 using System.Collections.Generic;
 using System.Collections.Specialized;
 using System.Security.Claims;
@@ -23,12 +24,16 @@ public interface IRedirectUriValidator
     /// <param name="requestedUri">The requested URI.</param>
     /// <param name="client">The client.</param>
     /// <returns><c>true</c> is the URI is valid; <c>false</c> otherwise.</returns>
+    [Obsolete("This overload is deprecated and will be removed in a future version. Use the overload that takes a RedirectUriValidationContext parameter instead.")]
     Task<bool> IsRedirectUriValidAsync(string requestedUri, Client client);
 
     /// <summary>
     /// Determines whether a redirect URI is valid for a client.
     /// </summary>
-    Task<bool> IsRedirectUriValidAsync(RedirectUriValidationContext context) => IsRedirectUriValidAsync(context.RequestedUri, context.Client);
+    Task<bool> IsRedirectUriValidAsync(RedirectUriValidationContext context)
+#pragma warning disable CS0618 // Type or member is obsolete
+        => IsRedirectUriValidAsync(context.RequestedUri, context.Client);
+#pragma warning restore CS0618 // Type or member is obsolete
 
     /// <summary>
     /// Determines whether a post logout URI is valid for a client.

test/IdentityServer.UnitTests/Services/Default/ParRedirectUriValidatorTests.cs

@@ -7,6 +7,8 @@
 using FluentAssertions;
 using System.Threading.Tasks;
 using Duende.IdentityServer.Models;
+using Duende.IdentityServer.Configuration;
+using UnitTests.Common;
 
 namespace UnitTests.Services.Default;
 
@@ -15,7 +17,9 @@ public class ParRedirectUriValidatorTests
     [Fact]
     public async Task PushedRedirectUriCanBeUsedAsync()
     {
-        var subject = new ParRedirectUriValidator();
+        var options = TestIdentityServerOptions.Create();
+        options.PushedAuthorization.AllowUnregisteredPushedRedirectUris = true;
+        var subject = new StrictRedirectUriValidator(options);
         var redirectUri = "https://pushed.example.com";
         var pushedParameters = new NameValueCollection
         {
@@ -26,7 +30,11 @@ public async Task PushedRedirectUriCanBeUsedAsync()
         {
             AuthorizeRequestType = AuthorizeRequestType.AuthorizeWithPushedParameters,
             RequestParameters = pushedParameters,
-            RequestedUri = redirectUri
+            RequestedUri = redirectUri,
+            Client = new Client
+            {
+                RequireClientSecret = true,
+            }
         });
 
         result.Should().Be(true);
@@ -35,7 +43,9 @@ public async Task PushedRedirectUriCanBeUsedAsync()
     [Fact]
     public async Task AnythingIsPermittedAtParEndpoint()
     {
-        var subject = new ParRedirectUriValidator();
+        var options = TestIdentityServerOptions.Create();
+        options.PushedAuthorization.AllowUnregisteredPushedRedirectUris = true;
+        var subject = new StrictRedirectUriValidator(options);
         var redirectUri = "https://pushed.example.com";
         var pushedParameters = new NameValueCollection
         {
@@ -46,17 +56,22 @@ public async Task AnythingIsPermittedAtParEndpoint()
         {
             AuthorizeRequestType = AuthorizeRequestType.PushedAuthorization,
             RequestParameters = pushedParameters,
-            RequestedUri = redirectUri
+            RequestedUri = redirectUri,
+            Client = new Client
+            {
+                RequireClientSecret = true,
+            }
         });
 
         result.Should().Be(true);
     }
 
-
     [Fact]
-    public async Task NotUsingThePushedRedirectUriShouldFailAsync()
+    public async Task ConfigurationControlsPermissiveness()
     {
-        var subject = new ParRedirectUriValidator();
+        var options = TestIdentityServerOptions.Create();
+        options.PushedAuthorization.AllowUnregisteredPushedRedirectUris = false;
+        var subject = new StrictRedirectUriValidator(options);
         var pushedRedirectUri = "https://pushed.example.com";
         var pushedParameters = new NameValueCollection
         {
@@ -79,7 +94,9 @@ public async Task NotUsingThePushedRedirectUriShouldFailAsync()
     [Fact]
     public async Task UsingARegisteredPushedUriInsteadOfThePushedRedirectUriShouldSucceed()
     {
-        var subject = new ParRedirectUriValidator();
+        var options = TestIdentityServerOptions.Create();
+        options.PushedAuthorization.AllowUnregisteredPushedRedirectUris = true;
+        var subject = new StrictRedirectUriValidator(options);
         var pushedRedirectUri = "https://pushed.example.com";
         var pushedParameters = new NameValueCollection
         {
@@ -106,7 +123,9 @@ public async Task UsingARegisteredPushedUriInsteadOfThePushedRedirectUriShouldSu
     [Fact]
     public async Task AuthorizeEndpointWithoutPushedParametersIsStillStrict()
     {
-        var subject = new ParRedirectUriValidator();
+        var options = TestIdentityServerOptions.Create();
+        options.PushedAuthorization.AllowUnregisteredPushedRedirectUris = true;
+        var subject = new StrictRedirectUriValidator(options);
         var requestedRedirectUri = "https://requested.example.com";
         var authorizeParameters = new NameValueCollection
         {

test/IdentityServer.UnitTests/Validation/AuthorizeRequest Validation/Authorize_ProtocolValidation_Resources.cs

@@ -52,7 +52,7 @@ public Authorize_ProtocolValidation_Resources()
             new TestIssuerNameService("https://sts"),
             new InMemoryClientStore(_clients),
             new DefaultCustomAuthorizeRequestValidator(),
-            new StrictRedirectUriValidator(),
+            new StrictRedirectUriValidator(_options),
             _mockResourceValidator,
             _mockUserSession,
             Factory.CreateRequestObjectValidator(),

test/IdentityServer.UnitTests/Validation/Setup/Factory.cs

@@ -249,7 +249,7 @@ public static AuthorizeRequestValidator CreateAuthorizeRequestValidator(
 
         if (uriValidator == null)
         {
-            uriValidator = new StrictRedirectUriValidator();
+            uriValidator = new StrictRedirectUriValidator(options);
         }
 
         if (resourceValidator == null)

test/IdentityServer.UnitTests/Validation/StrictRedirectUriValidatorAppAuthValidation.cs

@@ -1,4 +1,4 @@
-// Copyright (c) Duende Software. All rights reserved.
+// Copyright (c) Duende Software. All rights reserved.
 // See LICENSE in the project root for license information.
 
 
@@ -44,7 +44,8 @@ public class StrictRedirectUriValidatorAppAuthValidation
     [InlineData("http://127.0.0.1:443/a/b?q=123#abc")]
     public async Task Loopback_Redirect_URIs_Should_Be_AllowedAsync(string requestedUri)
     {
-        var strictRedirectUriValidatorAppAuthValidator = new StrictRedirectUriValidatorAppAuth(TestLogger.Create<StrictRedirectUriValidatorAppAuth>());
+        var options = TestIdentityServerOptions.Create();
+        var strictRedirectUriValidatorAppAuthValidator = new StrictRedirectUriValidatorAppAuth(TestLogger.Create<StrictRedirectUriValidatorAppAuth>(), options);
 
         var result = await strictRedirectUriValidatorAppAuthValidator.IsRedirectUriValidAsync(requestedUri, clientWithValidLoopbackRedirectUri);
 
@@ -71,7 +72,8 @@ public async Task Loopback_Redirect_URIs_Should_Be_AllowedAsync(string requested
     [InlineData("http://127.0.0.1:#abc")]
     public async Task Loopback_Redirect_URIs_Should_Not_Be_AllowedAsync(string requestedUri)
     {
-        var strictRedirectUriValidatorAppAuthValidator = new StrictRedirectUriValidatorAppAuth(TestLogger.Create<StrictRedirectUriValidatorAppAuth>());
+        var options = TestIdentityServerOptions.Create();
+        var strictRedirectUriValidatorAppAuthValidator = new StrictRedirectUriValidatorAppAuth(TestLogger.Create<StrictRedirectUriValidatorAppAuth>(), options);
 
         var result = await strictRedirectUriValidatorAppAuthValidator.IsRedirectUriValidAsync(requestedUri, clientWithValidLoopbackRedirectUri);