diff --git a/sandbox/OpenIddict.Sandbox.AspNet.Client/OpenIddict.Sandbox.AspNet.Client.csproj b/sandbox/OpenIddict.Sandbox.AspNet.Client/OpenIddict.Sandbox.AspNet.Client.csproj index 47d1c57f..11172833 100644 --- a/sandbox/OpenIddict.Sandbox.AspNet.Client/OpenIddict.Sandbox.AspNet.Client.csproj +++ b/sandbox/OpenIddict.Sandbox.AspNet.Client/OpenIddict.Sandbox.AspNet.Client.csproj @@ -27,7 +27,6 @@ - diff --git a/sandbox/OpenIddict.Sandbox.AspNet.Server/OpenIddict.Sandbox.AspNet.Server.csproj b/sandbox/OpenIddict.Sandbox.AspNet.Server/OpenIddict.Sandbox.AspNet.Server.csproj index 8999f06c..c8011677 100644 --- a/sandbox/OpenIddict.Sandbox.AspNet.Server/OpenIddict.Sandbox.AspNet.Server.csproj +++ b/sandbox/OpenIddict.Sandbox.AspNet.Server/OpenIddict.Sandbox.AspNet.Server.csproj @@ -31,7 +31,6 @@ - diff --git a/shared/OpenIddict.Extensions/Helpers/OpenIddictHelpers.cs b/shared/OpenIddict.Extensions/Helpers/OpenIddictHelpers.cs index 677a4d75..56122787 100644 --- a/shared/OpenIddict.Extensions/Helpers/OpenIddictHelpers.cs +++ b/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; /// /// Exposes common helpers used by the OpenIddict assemblies. @@ -69,4 +73,73 @@ internal static class OpenIddictHelpers } } } + + /// + /// Extracts the parameters from the specified query string. + /// + /// The query string, which may start with a '?'. + /// The parameters extracted from the specified query string. + /// is . + public static IReadOnlyDictionary 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())); + } + + /// + /// Creates a merged principal based on the specified principals. + /// + /// The collection of principals to merge. + /// The merged principal. + 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); + } } diff --git a/src/OpenIddict.Abstractions/OpenIddictResources.resx b/src/OpenIddict.Abstractions/OpenIddictResources.resx index 9794dd4d..e6dbccb6 100644 --- a/src/OpenIddict.Abstractions/OpenIddictResources.resx +++ b/src/OpenIddict.Abstractions/OpenIddictResources.resx @@ -1313,6 +1313,9 @@ Alternatively, you can disable the token storage feature by calling 'services.Ad A password must be specified when using the resource owner password credentials grant. + + The request forgery protection claim cannot be resolved from the state token. + The security token is missing. diff --git a/src/OpenIddict.Client.AspNetCore/OpenIddict.Client.AspNetCore.csproj b/src/OpenIddict.Client.AspNetCore/OpenIddict.Client.AspNetCore.csproj index ea75375d..8b3bed31 100644 --- a/src/OpenIddict.Client.AspNetCore/OpenIddict.Client.AspNetCore.csproj +++ b/src/OpenIddict.Client.AspNetCore/OpenIddict.Client.AspNetCore.csproj @@ -27,6 +27,10 @@ + + + + diff --git a/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandler.cs b/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandler.cs index bd7fc2ca..e4c72ccc 100644 --- a/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandler.cs +++ b/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 CreatePrincipal( + OpenIddictClientEndpointType.Redirection => OpenIddictHelpers.CreateMergedPrincipal( context.FrontchannelIdentityTokenPrincipal, context.BackchannelIdentityTokenPrincipal, context.UserinfoTokenPrincipal), @@ -304,45 +304,6 @@ public class OpenIddictClientAspNetCoreHandler : AuthenticationHandler 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. diff --git a/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandlers.cs b/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandlers.cs index a88d741d..b9ea1d04 100644 --- a/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandlers.cs +++ b/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. diff --git a/src/OpenIddict.Client.Owin/OpenIddict.Client.Owin.csproj b/src/OpenIddict.Client.Owin/OpenIddict.Client.Owin.csproj index 2ded9ea0..675e9366 100644 --- a/src/OpenIddict.Client.Owin/OpenIddict.Client.Owin.csproj +++ b/src/OpenIddict.Client.Owin/OpenIddict.Client.Owin.csproj @@ -19,6 +19,10 @@ + + + + diff --git a/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandler.cs b/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandler.cs index f372c5f4..2d01fd37 100644 --- a/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandler.cs +++ b/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 CreatePrincipal( + OpenIddictClientEndpointType.Redirection => OpenIddictHelpers.CreateMergedPrincipal( context.FrontchannelIdentityTokenPrincipal, context.BackchannelIdentityTokenPrincipal, context.UserinfoTokenPrincipal), @@ -232,45 +232,6 @@ public class OpenIddictClientOwinHandler : AuthenticationHandler 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. diff --git a/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs b/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs index 8c284fc3..1b912ef8 100644 --- a/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs +++ b/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. diff --git a/src/OpenIddict.Client/OpenIddict.Client.csproj b/src/OpenIddict.Client/OpenIddict.Client.csproj index 9053a55d..02ba4e48 100644 --- a/src/OpenIddict.Client/OpenIddict.Client.csproj +++ b/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 + + + + diff --git a/src/OpenIddict.Client/OpenIddictClientService.cs b/src/OpenIddict.Client/OpenIddictClientService.cs index 57180da9..fac61c57 100644 --- a/src/OpenIddict.Client/OpenIddictClientService.cs +++ b/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); - } } diff --git a/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs b/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs index 811aed59..344452b2 100644 --- a/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs +++ b/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 : 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; + } } } } diff --git a/src/OpenIddict.Server/OpenIddict.Server.csproj b/src/OpenIddict.Server/OpenIddict.Server.csproj index 91e739de..1ca371a6 100644 --- a/src/OpenIddict.Server/OpenIddict.Server.csproj +++ b/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 + + + + diff --git a/src/OpenIddict.Server/OpenIddictServerHandlers.Authentication.cs b/src/OpenIddict.Server/OpenIddictServerHandlers.Authentication.cs index ac794dc4..f68fe969 100644 --- a/src/OpenIddict.Server/OpenIddictServerHandlers.Authentication.cs +++ b/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;