From 48ecaea6f66407c56abf42a0bb55361e557174cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Fri, 23 Dec 2022 21:04:15 +0100 Subject: [PATCH] Migrate the OpenID module to OpenIddict 4.0 --- src/OrchardCore.Build/Dependencies.props | 4 +- .../OpenIdServerConfiguration.cs | 24 ++- .../OpenIdValidationConfiguration.cs | 8 +- .../Controllers/AccessController.cs | 163 +++++++----------- .../OrchardCore.OpenId/OpenIdExtensions.cs | 7 + .../YesSql/Stores/OpenIdApplicationStore.cs | 24 +-- 6 files changed, 110 insertions(+), 120 deletions(-) diff --git a/src/OrchardCore.Build/Dependencies.props b/src/OrchardCore.Build/Dependencies.props index 993b8d82ea9..5e7bc008e51 100644 --- a/src/OrchardCore.Build/Dependencies.props +++ b/src/OrchardCore.Build/Dependencies.props @@ -45,8 +45,8 @@ - - + + diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Configuration/OpenIdServerConfiguration.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Configuration/OpenIdServerConfiguration.cs index 98af0479b2a..3d853833c32 100644 --- a/src/OrchardCore.Modules/OrchardCore.OpenId/Configuration/OpenIdServerConfiguration.cs +++ b/src/OrchardCore.Modules/OrchardCore.OpenId/Configuration/OpenIdServerConfiguration.cs @@ -75,34 +75,46 @@ public void Configure(OpenIddictServerOptions options) options.SigningCredentials.Add(new SigningCredentials(key, SecurityAlgorithms.RsaSha256)); } + // Note: while endpoint paths in OrchardCore are stored as PathString instances, + // OpenIddict uses System.Uri. To ensure the System.Uri instances created from + // a PathString don't represent root-relative URIs (which would break path-based + // multi-tenancy support), the leading '/' that is always present in PathString + // instances is manually removed from the endpoint path before URIs are created. + if (settings.AuthorizationEndpointPath.HasValue) { - options.AuthorizationEndpointUris.Add(new Uri(settings.AuthorizationEndpointPath.Value, UriKind.Relative)); + options.AuthorizationEndpointUris.Add(new Uri( + settings.AuthorizationEndpointPath.ToUriComponent()[1..], UriKind.Relative)); } if (settings.LogoutEndpointPath.HasValue) { - options.LogoutEndpointUris.Add(new Uri(settings.LogoutEndpointPath.Value, UriKind.Relative)); + options.LogoutEndpointUris.Add(new Uri( + settings.LogoutEndpointPath.ToUriComponent()[1..], UriKind.Relative)); } if (settings.TokenEndpointPath.HasValue) { - options.TokenEndpointUris.Add(new Uri(settings.TokenEndpointPath.Value, UriKind.Relative)); + options.TokenEndpointUris.Add(new Uri( + settings.TokenEndpointPath.ToUriComponent()[1..], UriKind.Relative)); } if (settings.UserinfoEndpointPath.HasValue) { - options.UserinfoEndpointUris.Add(new Uri(settings.UserinfoEndpointPath.Value, UriKind.Relative)); + options.UserinfoEndpointUris.Add(new Uri( + settings.UserinfoEndpointPath.ToUriComponent()[1..], UriKind.Relative)); } if (settings.IntrospectionEndpointPath.HasValue) { - options.IntrospectionEndpointUris.Add(new Uri(settings.IntrospectionEndpointPath.Value, UriKind.Relative)); + options.IntrospectionEndpointUris.Add(new Uri( + settings.IntrospectionEndpointPath.ToUriComponent()[1..], UriKind.Relative)); } if (settings.RevocationEndpointPath.HasValue) { - options.RevocationEndpointUris.Add(new Uri(settings.RevocationEndpointPath.Value, UriKind.Relative)); + options.RevocationEndpointUris.Add(new Uri( + settings.RevocationEndpointPath.ToUriComponent()[1..], UriKind.Relative)); } // For now, response types and response modes are not directly diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Configuration/OpenIdValidationConfiguration.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Configuration/OpenIdValidationConfiguration.cs index 81882d44af0..c257904388f 100644 --- a/src/OrchardCore.Modules/OrchardCore.OpenId/Configuration/OpenIdValidationConfiguration.cs +++ b/src/OrchardCore.Modules/OrchardCore.OpenId/Configuration/OpenIdValidationConfiguration.cs @@ -9,8 +9,8 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; -using Microsoft.IdentityModel.Protocols.OpenIdConnect; using Microsoft.IdentityModel.Tokens; +using OpenIddict.Abstractions; using OpenIddict.Validation; using OpenIddict.Validation.AspNetCore; using OpenIddict.Validation.DataProtection; @@ -105,7 +105,7 @@ public void Configure(OpenIddictValidationOptions options) if (settings.Authority != null) { options.Issuer = settings.Authority; - options.MetadataAddress = settings.MetadataAddress; + options.ConfigurationEndpoint = settings.MetadataAddress; options.Audiences.Add(settings.Audience); // Note: OpenIddict 3.0 only accepts tokens issued with a non-empty token type (e.g "at+jwt") @@ -140,9 +140,9 @@ public void Configure(OpenIddictValidationOptions options) return; } - options.Configuration = new OpenIdConnectConfiguration + options.Configuration = new OpenIddictConfiguration { - Issuer = configuration.Authority?.AbsoluteUri + Issuer = configuration.Authority }; // Import the signing keys from the OpenID server configuration. diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Controllers/AccessController.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Controllers/AccessController.cs index 3fdd8653c17..ce89210f340 100644 --- a/src/OrchardCore.Modules/OrchardCore.OpenId/Controllers/AccessController.cs +++ b/src/OrchardCore.Modules/OrchardCore.OpenId/Controllers/AccessController.cs @@ -104,10 +104,7 @@ public async Task Authorize() case ConsentTypes.External when authorizations.Any(): case ConsentTypes.Explicit when authorizations.Any() && !request.HasPrompt(Prompts.Consent): var identity = new ClaimsIdentity(result.Principal.Claims, OpenIddictServerAspNetCoreDefaults.AuthenticationScheme); - var principal = new ClaimsPrincipal(identity); - - identity.AddClaim(OpenIdConstants.Claims.EntityType, OpenIdConstants.EntityTypes.User, - Destinations.AccessToken, Destinations.IdentityToken); + identity.AddClaim(new Claim(OpenIdConstants.Claims.EntityType, OpenIdConstants.EntityTypes.User)); // Note: while ASP.NET Core Identity uses the legacy WS-Federation claims (exposed by the ClaimTypes class), // OpenIddict uses the newer JWT claims defined by the OpenID Connect specification. To ensure the mandatory @@ -121,30 +118,23 @@ public async Task Authorize() identity.AddClaim(new Claim(Claims.Name, result.Principal.GetUserName())); } - principal.SetScopes(request.GetScopes()); - principal.SetResources(await GetResourcesAsync(request.GetScopes())); + identity.SetScopes(request.GetScopes()); + identity.SetResources(await GetResourcesAsync(request.GetScopes())); // Automatically create a permanent authorization to avoid requiring explicit consent // for future authorization or token requests containing the same scopes. var authorization = authorizations.LastOrDefault(); - if (authorization == null) - { - authorization = await _authorizationManager.CreateAsync( - principal: principal, - subject: principal.GetUserIdentifier(), - client: await _applicationManager.GetIdAsync(application), - type: AuthorizationTypes.Permanent, - scopes: principal.GetScopes()); - } - - principal.SetAuthorizationId(await _authorizationManager.GetIdAsync(authorization)); + authorization ??= await _authorizationManager.CreateAsync( + identity: identity, + subject: identity.GetUserIdentifier(), + client: await _applicationManager.GetIdAsync(application), + type: AuthorizationTypes.Permanent, + scopes: identity.GetScopes()); - foreach (var claim in principal.Claims) - { - claim.SetDestinations(GetDestinations(claim, principal)); - } + identity.SetAuthorizationId(await _authorizationManager.GetIdAsync(authorization)); + identity.SetDestinations(GetDestinations); - return SignIn(principal, OpenIddictServerAspNetCoreDefaults.AuthenticationScheme); + return SignIn(new ClaimsPrincipal(identity), OpenIddictServerAspNetCoreDefaults.AuthenticationScheme); case ConsentTypes.Explicit when request.HasPrompt(Prompts.None): return Forbid(new AuthenticationProperties(new Dictionary @@ -242,10 +232,7 @@ public async Task AuthorizeAccept() default: var identity = new ClaimsIdentity(User.Claims, OpenIddictServerAspNetCoreDefaults.AuthenticationScheme); - var principal = new ClaimsPrincipal(identity); - - identity.AddClaim(OpenIdConstants.Claims.EntityType, OpenIdConstants.EntityTypes.User, - Destinations.AccessToken, Destinations.IdentityToken); + identity.AddClaim(new Claim(OpenIdConstants.Claims.EntityType, OpenIdConstants.EntityTypes.User)); // Note: while ASP.NET Core Identity uses the legacy WS-Federation claims (exposed by the ClaimTypes class), // OpenIddict uses the newer JWT claims defined by the OpenID Connect specification. To ensure the mandatory @@ -259,30 +246,23 @@ public async Task AuthorizeAccept() identity.AddClaim(new Claim(Claims.Name, User.GetUserName())); } - principal.SetScopes(request.GetScopes()); - principal.SetResources(await GetResourcesAsync(request.GetScopes())); + identity.SetScopes(request.GetScopes()); + identity.SetResources(await GetResourcesAsync(request.GetScopes())); // Automatically create a permanent authorization to avoid requiring explicit consent // for future authorization or token requests containing the same scopes. var authorization = authorizations.LastOrDefault(); - if (authorization == null) - { - authorization = await _authorizationManager.CreateAsync( - principal: principal, - subject: principal.GetUserIdentifier(), - client: await _applicationManager.GetIdAsync(application), - type: AuthorizationTypes.Permanent, - scopes: principal.GetScopes()); - } - - principal.SetAuthorizationId(await _authorizationManager.GetIdAsync(authorization)); + authorization ??= await _authorizationManager.CreateAsync( + identity: identity, + subject: identity.GetUserIdentifier(), + client: await _applicationManager.GetIdAsync(application), + type: AuthorizationTypes.Permanent, + scopes: identity.GetScopes()); - foreach (var claim in principal.Claims) - { - claim.SetDestinations(GetDestinations(claim, principal)); - } + identity.SetAuthorizationId(await _authorizationManager.GetIdAsync(authorization)); + identity.SetDestinations(GetDestinations); - return SignIn(principal, OpenIddictServerAspNetCoreDefaults.AuthenticationScheme); + return SignIn(new ClaimsPrincipal(identity), OpenIddictServerAspNetCoreDefaults.AuthenticationScheme); } } @@ -460,15 +440,13 @@ private async Task ExchangeClientCredentialsGrantType(OpenIddictR OpenIddictServerAspNetCoreDefaults.AuthenticationScheme, Claims.Name, Claims.Role); - identity.AddClaim(OpenIdConstants.Claims.EntityType, OpenIdConstants.EntityTypes.Application, - Destinations.AccessToken, Destinations.IdentityToken); + identity.AddClaim(new Claim(OpenIdConstants.Claims.EntityType, OpenIdConstants.EntityTypes.Application)); + identity.AddClaim(new Claim(Claims.Subject, request.ClientId)); - identity.AddClaim(Claims.Subject, request.ClientId, - Destinations.AccessToken, Destinations.IdentityToken); - - identity.AddClaim(Claims.Name, - await _applicationManager.GetDisplayNameAsync(application), - Destinations.AccessToken, Destinations.IdentityToken); + // Always add a "name" claim for grant_type=client_credentials in both + // access and identity tokens even if the "name" scope wasn't requested. + identity.AddClaim(new Claim(Claims.Name, await _applicationManager.GetDisplayNameAsync(application)) + .SetDestinations(Destinations.AccessToken, Destinations.IdentityToken)); // If the role service is available, add all the role claims // associated with the application roles in the database. @@ -476,8 +454,11 @@ await _applicationManager.GetDisplayNameAsync(application), foreach (var role in await _applicationManager.GetRolesAsync(application)) { - identity.AddClaim(identity.RoleClaimType, role, - Destinations.AccessToken, Destinations.IdentityToken); + // Since the claims added in this block have a dynamic name, directly set the destinations + // here instead of relying on the GetDestination() helper that only works with static claims. + + identity.AddClaim(new Claim(identity.RoleClaimType, role) + .SetDestinations(Destinations.AccessToken, Destinations.IdentityToken)); if (roleService != null) { @@ -488,11 +469,11 @@ await _applicationManager.GetDisplayNameAsync(application), } } - var principal = new ClaimsPrincipal(identity); - principal.SetScopes(request.GetScopes()); - principal.SetResources(await GetResourcesAsync(request.GetScopes())); + identity.SetScopes(request.GetScopes()); + identity.SetResources(await GetResourcesAsync(request.GetScopes())); + identity.SetDestinations(GetDestinations); - return SignIn(principal, OpenIddictServerAspNetCoreDefaults.AuthenticationScheme); + return SignIn(new ClaimsPrincipal(identity), OpenIddictServerAspNetCoreDefaults.AuthenticationScheme); } private async Task ExchangePasswordGrantType(OpenIddictRequest request) @@ -547,9 +528,7 @@ private async Task ExchangePasswordGrantType(OpenIddictRequest re } var identity = (ClaimsIdentity)principal.Identity; - - identity.AddClaim(OpenIdConstants.Claims.EntityType, OpenIdConstants.EntityTypes.User, - Destinations.AccessToken, Destinations.IdentityToken); + identity.AddClaim(new Claim(OpenIdConstants.Claims.EntityType, OpenIdConstants.EntityTypes.User)); // Note: while ASP.NET Core Identity uses the legacy WS-Federation claims (exposed by the ClaimTypes class), // OpenIddict uses the newer JWT claims defined by the OpenID Connect specification. To ensure the mandatory @@ -563,28 +542,21 @@ private async Task ExchangePasswordGrantType(OpenIddictRequest re identity.AddClaim(new Claim(Claims.Name, principal.GetUserName())); } - principal.SetScopes(request.GetScopes()); - principal.SetResources(await GetResourcesAsync(request.GetScopes())); + identity.SetScopes(request.GetScopes()); + identity.SetResources(await GetResourcesAsync(request.GetScopes())); // Automatically create a permanent authorization to avoid requiring explicit consent // for future authorization or token requests containing the same scopes. var authorization = authorizations.FirstOrDefault(); - if (authorization == null) - { - authorization = await _authorizationManager.CreateAsync( - principal: principal, - subject: principal.GetUserIdentifier(), - client: await _applicationManager.GetIdAsync(application), - type: AuthorizationTypes.Permanent, - scopes: principal.GetScopes()); - } - - principal.SetAuthorizationId(await _authorizationManager.GetIdAsync(authorization)); + authorization ??= await _authorizationManager.CreateAsync( + identity: identity, + subject: identity.GetUserIdentifier(), + client: await _applicationManager.GetIdAsync(application), + type: AuthorizationTypes.Permanent, + scopes: identity.GetScopes()); - foreach (var claim in principal.Claims) - { - claim.SetDestinations(GetDestinations(claim, principal)); - } + identity.SetAuthorizationId(await _authorizationManager.GetIdAsync(authorization)); + identity.SetDestinations(GetDestinations); return SignIn(principal, OpenIddictServerAspNetCoreDefaults.AuthenticationScheme); } @@ -625,9 +597,7 @@ private async Task ExchangeAuthorizationCodeOrRefreshTokenGrantTy } var identity = (ClaimsIdentity)principal.Identity; - - identity.AddClaim(OpenIdConstants.Claims.EntityType, OpenIdConstants.EntityTypes.User, - Destinations.AccessToken, Destinations.IdentityToken); + identity.AddClaim(new Claim(OpenIdConstants.Claims.EntityType, OpenIdConstants.EntityTypes.User)); // Note: while ASP.NET Core Identity uses the legacy WS-Federation claims (exposed by the ClaimTypes class), // OpenIddict uses the newer JWT claims defined by the OpenID Connect specification. To ensure the mandatory @@ -641,15 +611,12 @@ private async Task ExchangeAuthorizationCodeOrRefreshTokenGrantTy identity.AddClaim(new Claim(Claims.Name, principal.GetUserName())); } - foreach (var claim in principal.Claims) - { - claim.SetDestinations(GetDestinations(claim, principal)); - } + identity.SetDestinations(GetDestinations); return SignIn(principal, OpenIddictServerAspNetCoreDefaults.AuthenticationScheme); } - private IEnumerable GetDestinations(Claim claim, ClaimsPrincipal principal) + private static IEnumerable GetDestinations(Claim claim) { // Note: by default, claims are NOT automatically included in the access and identity tokens. // To allow OpenIddict to serialize them, you must attach them a destination, that specifies @@ -657,23 +624,27 @@ private IEnumerable GetDestinations(Claim claim, ClaimsPrincipal princip switch (claim.Type) { + // If the claim already includes destinations (set before this helper is called), flow them as-is. + case string when claim.GetDestinations() is { IsDefaultOrEmpty: false } destinations: + return destinations; + // Never include the security stamp in the access and identity tokens, as it's a secret value. case "AspNet.Identity.SecurityStamp": - break; + return Enumerable.Empty(); // Only add the claim to the id_token if the corresponding scope was granted. // The other claims will only be added to the access_token. case OpenIdConstants.Claims.EntityType: - case Claims.Name when principal.HasScope(Scopes.Profile): - case Claims.Email when principal.HasScope(Scopes.Email): - case Claims.Role when principal.HasScope(Scopes.Roles): - yield return Destinations.AccessToken; - yield return Destinations.IdentityToken; - break; + case Claims.Name when claim.Subject.HasScope(Scopes.Profile): + case Claims.Email when claim.Subject.HasScope(Scopes.Email): + case Claims.Role when claim.Subject.HasScope(Scopes.Roles): + return new[] + { + Destinations.AccessToken, + Destinations.IdentityToken + }; - default: - yield return Destinations.AccessToken; - break; + default: return new[] { Destinations.AccessToken }; } } diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs index 06e8ce78c83..f567d30da83 100644 --- a/src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs +++ b/src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs @@ -8,11 +8,18 @@ namespace OrchardCore.OpenId { internal static class OpenIdExtensions { + internal static string GetUserIdentifier(this ClaimsIdentity identity) + => identity.FindFirst(Claims.Subject)?.Value ?? + identity.FindFirst(ClaimTypes.NameIdentifier)?.Value ?? + identity.FindFirst(ClaimTypes.Upn)?.Value ?? + throw new InvalidOperationException("No suitable user identifier can be found in the identity."); + internal static string GetUserIdentifier(this ClaimsPrincipal principal) => principal.FindFirst(Claims.Subject)?.Value ?? principal.FindFirst(ClaimTypes.NameIdentifier)?.Value ?? principal.FindFirst(ClaimTypes.Upn)?.Value ?? throw new InvalidOperationException("No suitable user identifier can be found in the principal."); + internal static string GetUserName(this ClaimsPrincipal principal) => principal.FindFirst(Claims.Name)?.Value ?? principal.FindFirst(ClaimTypes.Name)?.Value ?? diff --git a/src/OrchardCore/OrchardCore.OpenId.Core/YesSql/Stores/OpenIdApplicationStore.cs b/src/OrchardCore/OrchardCore.OpenId.Core/YesSql/Stores/OpenIdApplicationStore.cs index 1960813abad..68e5c2f0d3b 100644 --- a/src/OrchardCore/OrchardCore.OpenId.Core/YesSql/Stores/OpenIdApplicationStore.cs +++ b/src/OrchardCore/OrchardCore.OpenId.Core/YesSql/Stores/OpenIdApplicationStore.cs @@ -108,32 +108,32 @@ public virtual async ValueTask FindByPhysicalIdAsync(string identi } /// - public virtual IAsyncEnumerable FindByPostLogoutRedirectUriAsync(string address, CancellationToken cancellationToken) + public virtual IAsyncEnumerable FindByPostLogoutRedirectUriAsync(string uri, CancellationToken cancellationToken) { - if (string.IsNullOrEmpty(address)) + if (string.IsNullOrEmpty(uri)) { - throw new ArgumentException("The address cannot be null or empty.", nameof(address)); + throw new ArgumentException("The URI cannot be null or empty.", nameof(uri)); } cancellationToken.ThrowIfCancellationRequested(); return _session.Query( - index => index.LogoutRedirectUri == address, + index => index.LogoutRedirectUri == uri, collection: OpenIdCollection).ToAsyncEnumerable(); } /// - public virtual IAsyncEnumerable FindByRedirectUriAsync(string address, CancellationToken cancellationToken) + public virtual IAsyncEnumerable FindByRedirectUriAsync(string uri, CancellationToken cancellationToken) { - if (string.IsNullOrEmpty(address)) + if (string.IsNullOrEmpty(uri)) { - throw new ArgumentException("The address cannot be null or empty.", nameof(address)); + throw new ArgumentException("The URI cannot be null or empty.", nameof(uri)); } cancellationToken.ThrowIfCancellationRequested(); return _session.Query( - index => index.RedirectUri == address, + index => index.RedirectUri == uri, collection: OpenIdCollection).ToAsyncEnumerable(); } @@ -420,14 +420,14 @@ public virtual ValueTask SetPermissionsAsync(TApplication application, Immutable /// public virtual ValueTask SetPostLogoutRedirectUrisAsync(TApplication application, - ImmutableArray addresses, CancellationToken cancellationToken) + ImmutableArray uris, CancellationToken cancellationToken) { if (application == null) { throw new ArgumentNullException(nameof(application)); } - application.PostLogoutRedirectUris = addresses; + application.PostLogoutRedirectUris = uris; return default; } @@ -458,14 +458,14 @@ public virtual ValueTask SetPropertiesAsync(TApplication application, ImmutableD /// public virtual ValueTask SetRedirectUrisAsync(TApplication application, - ImmutableArray addresses, CancellationToken cancellationToken) + ImmutableArray uris, CancellationToken cancellationToken) { if (application == null) { throw new ArgumentNullException(nameof(application)); } - application.RedirectUris = addresses; + application.RedirectUris = uris; return default; }