Make the "rfp" validation logic mandatory and move common helpers to OpenIddictHelpers

sandbox/OpenIddict.Sandbox.AspNet.Client/OpenIddict.Sandbox.AspNet.Client.csproj

@@ -27,7 +27,6 @@
     <PackageReference Include="Autofac.Owin" />
     <PackageReference Include="Microsoft.AspNet.Mvc" />
     <PackageReference Include="Microsoft.AspNet.Web.Optimization" />
-    <PackageReference Include="Microsoft.CodeDom.Providers.DotNetCompilerPlatform" />
     <PackageReference Include="Microsoft.Extensions.DependencyInjection" />
     <PackageReference Include="Microsoft.Owin.Host.SystemWeb" />
     <PackageReference Include="Microsoft.Owin.Security.Cookies" />

sandbox/OpenIddict.Sandbox.AspNet.Server/OpenIddict.Sandbox.AspNet.Server.csproj

@@ -31,7 +31,6 @@
     <PackageReference Include="Microsoft.AspNet.Mvc" />
     <PackageReference Include="Microsoft.AspNet.Web.Optimization" />
     <PackageReference Include="Microsoft.AspNet.WebApi.Owin" />
-    <PackageReference Include="Microsoft.CodeDom.Providers.DotNetCompilerPlatform" />
     <PackageReference Include="Microsoft.Extensions.DependencyInjection" />
     <PackageReference Include="Microsoft.Owin.Host.SystemWeb" />
     <PackageReference Include="Microsoft.Owin.Security.Cookies" />

shared/OpenIddict.Extensions/Helpers/OpenIddictHelpers.cs

@@ -1,4 +1,8 @@
-namespace OpenIddict.Extensions;
+using System.Security.Claims;
+using Microsoft.Extensions.Primitives;
+using Microsoft.IdentityModel.Tokens;
+
+namespace OpenIddict.Extensions;
 
 /// <summary>
 /// Exposes common helpers used by the OpenIddict assemblies.
@@ -69,4 +73,73 @@ internal static class OpenIddictHelpers
             }
         }
     }
+
+    /// <summary>
+    /// Extracts the parameters from the specified query string.
+    /// </summary>
+    /// <param name="query">The query string, which may start with a '?'.</param>
+    /// <returns>The parameters extracted from the specified query string.</returns>
+    /// <exception cref="ArgumentNullException"><paramref name="query"/> is <see langword="null"/>.</exception>
+    public static IReadOnlyDictionary<string, StringValues> ParseQuery(string query)
+    {
+        if (query is null)
+        {
+            throw new ArgumentNullException(nameof(query));
+        }
+
+        return query.TrimStart(Separators.QuestionMark[0])
+            .Split(new[] { Separators.Ampersand[0], Separators.Semicolon[0] }, StringSplitOptions.RemoveEmptyEntries)
+            .Select(parameter => parameter.Split(Separators.EqualsSign, StringSplitOptions.RemoveEmptyEntries))
+            .Select(parts => (
+                Key: parts[0] is string key ? Uri.UnescapeDataString(key) : null,
+                Value: parts.Length > 1 && parts[1] is string value ? Uri.UnescapeDataString(value) : null))
+            // Note: ignore empty values to match the logic used by OWIN for IOwinRequest.Query.
+            .Where(pair => !string.IsNullOrEmpty(pair.Key) && !string.IsNullOrEmpty(pair.Value))
+            .GroupBy(pair => pair.Key)
+            .ToDictionary(pair => pair.Key!, pair => new StringValues(pair.Select(parts => parts.Value).ToArray()));
+    }
+
+    /// <summary>
+    /// Creates a merged principal based on the specified principals.
+    /// </summary>
+    /// <param name="principals">The collection of principals to merge.</param>
+    /// <returns>The merged principal.</returns>
+    public static ClaimsPrincipal CreateMergedPrincipal(params ClaimsPrincipal?[] principals)
+    {
+        // Note: components like the client handler can be used as a pure OAuth 2.0 stack for
+        // delegation scenarios where the identity of the user is not needed. In this case,
+        // since no principal can be resolved from a token or a userinfo response to construct
+        // a user identity, a fake one containing an "unauthenticated" identity (i.e with its
+        // AuthenticationType property deliberately left to null) is used to allow the host
+        // to return a "successful" authentication result for these delegation-only scenarios.
+        if (!principals.Any(principal => principal?.Identity is ClaimsIdentity { IsAuthenticated: true }))
+        {
+            return new ClaimsPrincipal(new ClaimsIdentity());
+        }
+
+        // Create a new composite identity containing the claims of all the principals.
+        var identity = new ClaimsIdentity(TokenValidationParameters.DefaultAuthenticationType);
+
+        foreach (var principal in principals)
+        {
+            // Note: the principal may be null if no value was extracted from the corresponding token.
+            if (principal is null)
+            {
+                continue;
+            }
+
+            foreach (var claim in principal.Claims)
+            {
+                // If a claim with the same type and the same value already exist, skip it.
+                if (identity.HasClaim(claim.Type, claim.Value))
+                {
+                    continue;
+                }
+
+                identity.AddClaim(claim);
+            }
+        }
+
+        return new ClaimsPrincipal(identity);
+    }
 }

src/OpenIddict.Abstractions/OpenIddictResources.resx

@@ -1313,6 +1313,9 @@ Alternatively, you can disable the token storage feature by calling 'services.Ad
   <data name="ID0338" xml:space="preserve">
     <value>A password must be specified when using the resource owner password credentials grant.</value>
   </data>
+  <data name="ID0339" xml:space="preserve">
+    <value>The request forgery protection claim cannot be resolved from the state token.</value>
+  </data>
   <data name="ID2000" xml:space="preserve">
     <value>The security token is missing.</value>
   </data>

src/OpenIddict.Client.AspNetCore/OpenIddict.Client.AspNetCore.csproj

@@ -27,6 +27,10 @@
     <PackageReference Include="Microsoft.Extensions.Caching.Abstractions" />
   </ItemGroup>
 
+  <ItemGroup>
+    <Compile Include="..\..\shared\OpenIddict.Extensions\*\*.cs" />
+  </ItemGroup>
+
   <ItemGroup>
     <Using Include="Microsoft.AspNetCore.Authentication" />
     <Using Include="Microsoft.AspNetCore.Http" />

src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandler.cs

@@ -9,7 +9,7 @@ using System.Text.Encodings.Web;
 using System.Text.Json;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
-using Microsoft.IdentityModel.Tokens;
+using OpenIddict.Extensions;
 using static OpenIddict.Client.AspNetCore.OpenIddictClientAspNetCoreConstants;
 using Properties = OpenIddict.Client.AspNetCore.OpenIddictClientAspNetCoreConstants.Properties;
 
@@ -145,7 +145,7 @@ public class OpenIddictClientAspNetCoreHandler : AuthenticationHandler<OpenIddic
             {
                 // Create a composite principal containing claims resolved from the frontchannel
                 // and backchannel identity tokens and the userinfo token principal, if available.
-                OpenIddictClientEndpointType.Redirection => CreatePrincipal(
+                OpenIddictClientEndpointType.Redirection => OpenIddictHelpers.CreateMergedPrincipal(
                     context.FrontchannelIdentityTokenPrincipal,
                     context.BackchannelIdentityTokenPrincipal,
                     context.UserinfoTokenPrincipal),
@@ -304,45 +304,6 @@ public class OpenIddictClientAspNetCoreHandler : AuthenticationHandler<OpenIddic
             return AuthenticateResult.Success(new AuthenticationTicket(principal, properties,
                 OpenIddictClientAspNetCoreDefaults.AuthenticationScheme));
 
-            static ClaimsPrincipal CreatePrincipal(params ClaimsPrincipal?[] principals)
-            {
-                // Note: the OpenIddict client handler can be used as a pure OAuth 2.0-only stack for
-                // delegation scenarios where the identity of the user is not needed. In this case,
-                // since no principal can be resolved from a token or a userinfo response to construct
-                // a user identity, a fake one containing an "unauthenticated" identity (i.e with its
-                // AuthenticationType property deliberately left to null) is used to allow the host
-                // to return a "successful" authentication result for these delegation-only scenarios.
-                if (!principals.Any(principal => principal?.Identity is ClaimsIdentity { IsAuthenticated: true }))
-                {
-                    return new ClaimsPrincipal(new ClaimsIdentity());
-                }
-
-                // Create a new composite identity containing the claims of all the principals.
-                var identity = new ClaimsIdentity(TokenValidationParameters.DefaultAuthenticationType);
-
-                foreach (var principal in principals)
-                {
-                    // Note: the principal may be null if no value was extracted from the corresponding token.
-                    if (principal is null)
-                    {
-                        continue;
-                    }
-
-                    foreach (var claim in principal.Claims)
-                    {
-                        // If a claim with the same type and the same value already exist, skip it.
-                        if (identity.HasClaim(claim.Type, claim.Value))
-                        {
-                            continue;
-                        }
-
-                        identity.AddClaim(claim);
-                    }
-                }
-
-                return new ClaimsPrincipal(identity);
-            }
-
             static AuthenticationProperties CreateProperties(ClaimsPrincipal? principal)
             {
                 // Note: the principal may be null if no value was extracted from the corresponding token.

src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandlers.cs

@@ -254,12 +254,10 @@ public static partial class OpenIddictClientAspNetCoreHandlers
                 throw new InvalidOperationException(SR.GetResourceString(SR.ID0114));
 
             // Resolve the request forgery protection from the state token principal.
-            // If the claim cannot be found, this means the protection was disabled
-            // using a custom event handler. In this case, bypass the validation.
             var identifier = context.StateTokenPrincipal.GetClaim(Claims.RequestForgeryProtection);
             if (string.IsNullOrEmpty(identifier))
             {
-                return default;
+                throw new InvalidOperationException(SR.GetResourceString(SR.ID0339));
             }
 
             // Resolve the cookie builder from the OWIN integration options.

src/OpenIddict.Client.Owin/OpenIddict.Client.Owin.csproj

@@ -19,6 +19,10 @@
     <PackageReference Include="Microsoft.Owin.Security" />
   </ItemGroup>
 
+  <ItemGroup>
+    <Compile Include="..\..\shared\OpenIddict.Extensions\*\*.cs" />
+  </ItemGroup>
+
   <ItemGroup>
     <Using Include="Microsoft.Owin" />
     <Using Include="Microsoft.Owin.Infrastructure" />

src/OpenIddict.Client.Owin/OpenIddictClientOwinHandler.cs

@@ -6,8 +6,8 @@
 
 using System.Security.Claims;
 using System.Text.Json;
-using Microsoft.IdentityModel.Tokens;
 using Microsoft.Owin.Security.Infrastructure;
+using OpenIddict.Extensions;
 using static OpenIddict.Client.Owin.OpenIddictClientOwinConstants;
 using Properties = OpenIddict.Client.Owin.OpenIddictClientOwinConstants.Properties;
 
@@ -161,7 +161,7 @@ public class OpenIddictClientOwinHandler : AuthenticationHandler<OpenIddictClien
             {
                 // Create a composite principal containing claims resolved from the frontchannel
                 // and backchannel identity tokens and the userinfo token principal, if available.
-                OpenIddictClientEndpointType.Redirection => CreatePrincipal(
+                OpenIddictClientEndpointType.Redirection => OpenIddictHelpers.CreateMergedPrincipal(
                     context.FrontchannelIdentityTokenPrincipal,
                     context.BackchannelIdentityTokenPrincipal,
                     context.UserinfoTokenPrincipal),
@@ -232,45 +232,6 @@ public class OpenIddictClientOwinHandler : AuthenticationHandler<OpenIddictClien
 
             return new AuthenticationTicket((ClaimsIdentity) principal.Identity, properties);
 
-            static ClaimsPrincipal CreatePrincipal(params ClaimsPrincipal?[] principals)
-            {
-                // Note: the OpenIddict client handler can be used as a pure OAuth 2.0-only stack for
-                // delegation scenarios where the identity of the user is not needed. In this case,
-                // since no principal can be resolved from a token or a userinfo response to construct
-                // a user identity, a fake one containing an "unauthenticated" identity (i.e with its
-                // AuthenticationType property deliberately left to null) is used to allow the host
-                // to return a "successful" authentication result for these delegation-only scenarios.
-                if (!principals.Any(principal => principal?.Identity is ClaimsIdentity { IsAuthenticated: true }))
-                {
-                    return new ClaimsPrincipal(new ClaimsIdentity());
-                }
-
-                // Create a new composite identity containing the claims of all the principals.
-                var identity = new ClaimsIdentity(TokenValidationParameters.DefaultAuthenticationType);
-
-                foreach (var principal in principals)
-                {
-                    // Note: the principal may be null if no value was extracted from the corresponding token.
-                    if (principal is null)
-                    {
-                        continue;
-                    }
-
-                    foreach (var claim in principal.Claims)
-                    {
-                        // If a claim with the same type and the same value already exist, skip it.
-                        if (identity.HasClaim(claim.Type, claim.Value))
-                        {
-                            continue;
-                        }
-
-                        identity.AddClaim(claim);
-                    }
-                }
-
-                return new ClaimsPrincipal(identity);
-            }
-
             static AuthenticationProperties CreateProperties(ClaimsPrincipal? principal)
             {
                 // Note: the principal may be null if no value was extracted from the corresponding token.

src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs

@@ -253,12 +253,10 @@ public static partial class OpenIddictClientOwinHandlers
                 throw new InvalidOperationException(SR.GetResourceString(SR.ID0120));
 
             // Resolve the request forgery protection from the state token principal.
-            // If the claim cannot be found, this means the protection was disabled
-            // using a custom event handler. In this case, bypass the validation.
             var identifier = context.StateTokenPrincipal.GetClaim(Claims.RequestForgeryProtection);
             if (string.IsNullOrEmpty(identifier))
             {
-                return default;
+                throw new InvalidOperationException(SR.GetResourceString(SR.ID0339));
             }
 
             // Resolve the cookie manager and the cookie options from the OWIN integration options.

src/OpenIddict.Client/OpenIddict.Client.csproj

@@ -29,6 +29,10 @@ To use the client feature on ASP.NET Core or OWIN/Katana, reference the OpenIddi
     <PackageReference Include="Portable.BouncyCastle" />
   </ItemGroup>
 
+  <ItemGroup>
+    <Compile Include="..\..\shared\OpenIddict.Extensions\*\*.cs" />
+  </ItemGroup>
+
   <ItemGroup>
     <Using Include="OpenIddict.Abstractions" />
     <Using Include="OpenIddict.Abstractions.OpenIddictConstants" Static="true" />

src/OpenIddict.Client/OpenIddictClientService.cs

@@ -9,6 +9,7 @@ using System.Security.Claims;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using Microsoft.IdentityModel.Tokens;
+using OpenIddict.Extensions;
 
 namespace OpenIddict.Client;
 
@@ -399,7 +400,7 @@ public class OpenIddictClientService
 
             // Create a composite principal containing claims resolved from the
             // backchannel identity token and the userinfo token, if available.
-            return (context.TokenResponse, CreatePrincipal(
+            return (context.TokenResponse, OpenIddictHelpers.CreateMergedPrincipal(
                 context.BackchannelIdentityTokenPrincipal,
                 context.UserinfoTokenPrincipal));
         }
@@ -491,7 +492,7 @@ public class OpenIddictClientService
 
             // Create a composite principal containing claims resolved from the
             // backchannel identity token and the userinfo token, if available.
-            return (context.TokenResponse, CreatePrincipal(
+            return (context.TokenResponse, OpenIddictHelpers.CreateMergedPrincipal(
                 context.BackchannelIdentityTokenPrincipal,
                 context.UserinfoTokenPrincipal));
         }
@@ -576,7 +577,7 @@ public class OpenIddictClientService
 
             // Create a composite principal containing claims resolved from the
             // backchannel identity token and the userinfo token, if available.
-            return (context.TokenResponse, CreatePrincipal(
+            return (context.TokenResponse, OpenIddictHelpers.CreateMergedPrincipal(
                 context.BackchannelIdentityTokenPrincipal,
                 context.UserinfoTokenPrincipal));
         }
@@ -937,43 +938,4 @@ public class OpenIddictClientService
             }
         }
     }
-
-    private static ClaimsPrincipal CreatePrincipal(params ClaimsPrincipal?[] principals)
-    {
-        // Note: the OpenIddict client handler can be used as a pure OAuth 2.0-only stack for
-        // delegation scenarios where the identity of the user is not needed. In this case,
-        // since no principal can be resolved from a token or a userinfo response to construct
-        // a user identity, a fake one containing an "unauthenticated" identity (i.e with its
-        // AuthenticationType property deliberately left to null) is used to allow ASP.NET Core
-        // to return a "successful" authentication result for these delegation-only scenarios.
-        if (!principals.Any(principal => principal?.Identity is ClaimsIdentity { IsAuthenticated: true }))
-        {
-            return new ClaimsPrincipal(new ClaimsIdentity());
-        }
-
-        // Create a new composite identity containing the claims of all the principals.
-        var identity = new ClaimsIdentity(TokenValidationParameters.DefaultAuthenticationType);
-
-        foreach (var principal in principals)
-        {
-            // Note: the principal may be null if no value was extracted from the corresponding token.
-            if (principal is null)
-            {
-                continue;
-            }
-
-            foreach (var claim in principal.Claims)
-            {
-                // If a claim with the same type and the same value already exist, skip it.
-                if (identity.HasClaim(claim.Type, claim.Value))
-                {
-                    continue;
-                }
-
-                identity.AddClaim(claim);
-            }
-        }
-
-        return new ClaimsPrincipal(identity);
-    }
 }

src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs

@@ -15,6 +15,7 @@ using System.Text;
 using System.Text.Json;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
+using OpenIddict.Extensions;
 
 #if !SUPPORTS_KEY_DERIVATION_WITH_SPECIFIED_HASH_ALGORITHM
 using Org.BouncyCastle.Crypto;
@@ -1226,15 +1227,15 @@ public class OpenIddictApplicationManager<TApplication> : IOpenIddictApplication
 
             // To prevent issuer fixation attacks where a malicious client would specify an "iss" parameter
             // in the callback URL, ensure the query - if present - doesn't include an "iss" parameter.
-            if (!string.IsNullOrEmpty(uri.Query) && uri.Query.TrimStart(Separators.QuestionMark[0])
-                .Split(new[] { Separators.Ampersand[0], Separators.Semicolon[0] }, StringSplitOptions.RemoveEmptyEntries)
-                .Select(parameter => parameter.Split(Separators.EqualsSign, StringSplitOptions.RemoveEmptyEntries))
-                .Select(parts => parts[0] is string value ? Uri.UnescapeDataString(value) : null)
-                .Contains(Parameters.Iss, StringComparer.OrdinalIgnoreCase))
+            if (!string.IsNullOrEmpty(uri.Query))
             {
-                yield return new ValidationResult(SR.FormatID2134(Parameters.Iss));
+                var parameters = OpenIddictHelpers.ParseQuery(uri.Query);
+                if (parameters.ContainsKey(Parameters.Iss))
+                {
+                    yield return new ValidationResult(SR.FormatID2134(Parameters.Iss));
 
-                break;
+                    break;
+                }
             }
         }
     }

src/OpenIddict.Server/OpenIddict.Server.csproj

@@ -27,6 +27,10 @@ To use the server feature on ASP.NET Core or OWIN/Katana, reference the OpenIddi
     <PackageReference Include="Portable.BouncyCastle" />
   </ItemGroup>
 
+  <ItemGroup>
+    <Compile Include="..\..\shared\OpenIddict.Extensions\*\*.cs" />
+  </ItemGroup>
+
   <ItemGroup>
     <Using Include="OpenIddict.Abstractions" />
     <Using Include="OpenIddict.Abstractions.OpenIddictConstants" Static="true" />

src/OpenIddict.Server/OpenIddictServerHandlers.Authentication.cs

@@ -9,6 +9,7 @@ using System.Diagnostics;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
+using OpenIddict.Extensions;
 
 namespace OpenIddict.Server;
 
@@ -538,20 +539,20 @@ public static partial class OpenIddictServerHandlers
                 //
                 // Note: while OAuth 2.0 parameters are case-sentitive, the following check deliberately
                 // uses a case-insensitive comparison to ensure that all variations of "iss" are rejected.
-                if (!string.IsNullOrEmpty(uri.Query) && uri.Query.TrimStart(Separators.QuestionMark[0])
-                    .Split(new[] { Separators.Ampersand[0], Separators.Semicolon[0] }, StringSplitOptions.RemoveEmptyEntries)
-                    .Select(parameter => parameter.Split(Separators.EqualsSign, StringSplitOptions.RemoveEmptyEntries))
-                    .Select(parts => parts[0] is string value ? Uri.UnescapeDataString(value) : null)
-                    .Contains(Parameters.Iss, StringComparer.OrdinalIgnoreCase))
+                if (!string.IsNullOrEmpty(uri.Query))
                 {
-                    context.Logger.LogInformation(SR.GetResourceString(SR.ID6181), Parameters.RedirectUri, Parameters.Iss);
+                    var parameters = OpenIddictHelpers.ParseQuery(uri.Query);
+                    if (parameters.ContainsKey(Parameters.Iss))
+                    {
+                        context.Logger.LogInformation(SR.GetResourceString(SR.ID6181), Parameters.RedirectUri, Parameters.Iss);
 
-                    context.Reject(
-                        error: Errors.InvalidRequest,
-                        description: SR.FormatID2135(Parameters.RedirectUri, Parameters.Iss),
-                        uri: SR.FormatID8000(SR.ID2135));
+                        context.Reject(
+                            error: Errors.InvalidRequest,
+                            description: SR.FormatID2135(Parameters.RedirectUri, Parameters.Iss),
+                            uri: SR.FormatID8000(SR.ID2135));
 
-                    return default;
+                        return default;
+                    }
                 }
 
                 return default;