From 9d5c8007b46ce7a7159055ddf5b11202dd3b4a2f Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Tue, 10 Sep 2024 10:13:34 -0700 Subject: [PATCH 1/5] Fix Role claims mapping in OpenID access. --- .../OrchardCore.OpenId/Controllers/AccessController.cs | 6 ++++++ .../OrchardCore.OpenId/OpenIdExtensions.cs | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Controllers/AccessController.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Controllers/AccessController.cs index 84b21771f4e..01db9bd5954 100644 --- a/src/OrchardCore.Modules/OrchardCore.OpenId/Controllers/AccessController.cs +++ b/src/OrchardCore.Modules/OrchardCore.OpenId/Controllers/AccessController.cs @@ -606,11 +606,17 @@ private async Task ExchangeAuthorizationCodeOrRefreshTokenGrantTy { identity.AddClaim(new Claim(Claims.Subject, principal.GetUserIdentifier())); } + if (string.IsNullOrEmpty(principal.FindFirst(Claims.Name)?.Value)) { identity.AddClaim(new Claim(Claims.Name, principal.GetUserName())); } + if (string.IsNullOrEmpty(principal.FindFirst(Claims.Role)?.Value)) + { + identity.AddClaim(new Claim(Claims.Role, principal.GetRoles())); + } + identity.SetDestinations(GetDestinations); return SignIn(principal, OpenIddictServerAspNetCoreDefaults.AuthenticationScheme); diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs index c81d31f36b6..6e34d18983c 100644 --- a/src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs +++ b/src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs @@ -22,6 +22,11 @@ internal static string GetUserName(this ClaimsPrincipal principal) principal.FindFirst(ClaimTypes.Name)?.Value ?? throw new InvalidOperationException("No suitable user name can be found in the principal."); + internal static string GetRoles(this ClaimsPrincipal principal) + => principal.FindFirst(Claims.Role)?.Value ?? + principal.FindFirst(ClaimTypes.Role)?.Value ?? + throw new InvalidOperationException("No suitable role can be found in the principal."); + internal static async Task AnyAsync(this IAsyncEnumerable source) { ArgumentNullException.ThrowIfNull(source); From da0d3547175f12c05e3d24d163d8765b01436d2f Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Tue, 10 Sep 2024 10:20:59 -0700 Subject: [PATCH 2/5] support multiple roles --- .../OrchardCore.OpenId/Controllers/AccessController.cs | 5 ++++- .../OrchardCore.OpenId/OpenIdExtensions.cs | 9 +++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Controllers/AccessController.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Controllers/AccessController.cs index 01db9bd5954..de4e8414db2 100644 --- a/src/OrchardCore.Modules/OrchardCore.OpenId/Controllers/AccessController.cs +++ b/src/OrchardCore.Modules/OrchardCore.OpenId/Controllers/AccessController.cs @@ -614,7 +614,10 @@ private async Task ExchangeAuthorizationCodeOrRefreshTokenGrantTy if (string.IsNullOrEmpty(principal.FindFirst(Claims.Role)?.Value)) { - identity.AddClaim(new Claim(Claims.Role, principal.GetRoles())); + foreach (var role in principal.GetRoles()) + { + identity.AddClaim(new Claim(Claims.Role, role)); + } } identity.SetDestinations(GetDestinations); diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs index 6e34d18983c..3b0cf7ad0da 100644 --- a/src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs +++ b/src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs @@ -22,10 +22,11 @@ internal static string GetUserName(this ClaimsPrincipal principal) principal.FindFirst(ClaimTypes.Name)?.Value ?? throw new InvalidOperationException("No suitable user name can be found in the principal."); - internal static string GetRoles(this ClaimsPrincipal principal) - => principal.FindFirst(Claims.Role)?.Value ?? - principal.FindFirst(ClaimTypes.Role)?.Value ?? - throw new InvalidOperationException("No suitable role can be found in the principal."); + internal static string[] GetRoles(this ClaimsPrincipal principal) + => principal.FindAll(c => c.Type == Claims.Role || c.Type == ClaimTypes.Role) + .Select(x => x.ValueType) + .Distinct() + .ToArray(); internal static async Task AnyAsync(this IAsyncEnumerable source) { From 3137612126d6603e6fde13d485d0d85748e2f2b2 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Tue, 10 Sep 2024 11:06:37 -0700 Subject: [PATCH 3/5] fix an issue --- .../OrchardCore.OpenId/OpenIdExtensions.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs index 3b0cf7ad0da..9a4be04866c 100644 --- a/src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs +++ b/src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs @@ -23,8 +23,8 @@ internal static string GetUserName(this ClaimsPrincipal principal) throw new InvalidOperationException("No suitable user name can be found in the principal."); internal static string[] GetRoles(this ClaimsPrincipal principal) - => principal.FindAll(c => c.Type == Claims.Role || c.Type == ClaimTypes.Role) - .Select(x => x.ValueType) + => principal.FindAll(c => c.Type is Claims.Role || c.Type is ClaimTypes.Role) + .Select(x => x.Value) .Distinct() .ToArray(); From db0455031f438f64fdbf575cbc14cb59b5651aae Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Tue, 10 Sep 2024 11:17:23 -0700 Subject: [PATCH 4/5] cleanup --- .../Controllers/AccessController.cs | 81 +++++++------------ .../Controllers/UserInfoController.cs | 13 ++- 2 files changed, 33 insertions(+), 61 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Controllers/AccessController.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Controllers/AccessController.cs index de4e8414db2..ad0a9f9995e 100644 --- a/src/OrchardCore.Modules/OrchardCore.OpenId/Controllers/AccessController.cs +++ b/src/OrchardCore.Modules/OrchardCore.OpenId/Controllers/AccessController.cs @@ -103,17 +103,7 @@ public async Task Authorize() var identity = new ClaimsIdentity(result.Principal.Claims, OpenIddictServerAspNetCoreDefaults.AuthenticationScheme); 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 - // subject claim is correctly populated (and avoid an InvalidOperationException), it's manually added here. - if (string.IsNullOrEmpty(result.Principal.FindFirst(Claims.Subject)?.Value)) - { - identity.AddClaim(new Claim(Claims.Subject, result.Principal.GetUserIdentifier())); - } - if (string.IsNullOrEmpty(result.Principal.FindFirst(Claims.Name)?.Value)) - { - identity.AddClaim(new Claim(Claims.Name, result.Principal.GetUserName())); - } + PopulateIdentityClaims(result.Principal, identity); identity.SetScopes(request.GetScopes()); identity.SetResources(await GetResourcesAsync(request.GetScopes())); @@ -179,6 +169,30 @@ string GetRedirectUrl() } } + private static void PopulateIdentityClaims(ClaimsPrincipal principal, ClaimsIdentity identity) + { + // 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 + // subject claim is correctly populated (and avoid an InvalidOperationException), it's manually added here. + if (!principal.HasClaim(static claim => claim.Type is Claims.Subject)) + { + identity.AddClaim(new Claim(Claims.Subject, principal.GetUserIdentifier())); + } + + if (!principal.HasClaim(static claim => claim.Type is Claims.Name)) + { + identity.AddClaim(new Claim(Claims.Name, principal.GetUserName())); + } + + if (!principal.HasClaim(static claim => claim.Type is Claims.Role)) + { + foreach (var role in principal.GetRoles()) + { + identity.AddClaim(new Claim(Claims.Role, role)); + } + } + } + [ActionName(nameof(Authorize)), DisableCors] [FormValueRequired("submit.Accept"), HttpPost] public async Task AuthorizeAccept() @@ -231,17 +245,7 @@ public async Task AuthorizeAccept() var identity = new ClaimsIdentity(User.Claims, OpenIddictServerAspNetCoreDefaults.AuthenticationScheme); 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 - // subject claim is correctly populated (and avoid an InvalidOperationException), it's manually added here. - if (string.IsNullOrEmpty(User.FindFirst(Claims.Subject)?.Value)) - { - identity.AddClaim(new Claim(Claims.Subject, User.GetUserIdentifier())); - } - if (string.IsNullOrEmpty(User.FindFirst(Claims.Name)?.Value)) - { - identity.AddClaim(new Claim(Claims.Name, User.GetUserName())); - } + PopulateIdentityClaims(User, identity); identity.SetScopes(request.GetScopes()); identity.SetResources(await GetResourcesAsync(request.GetScopes())); @@ -527,17 +531,7 @@ private async Task ExchangePasswordGrantType(OpenIddictRequest re var identity = (ClaimsIdentity)principal.Identity; 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 - // subject claim is correctly populated (and avoid an InvalidOperationException), it's manually added here. - if (string.IsNullOrEmpty(principal.FindFirst(Claims.Subject)?.Value)) - { - identity.AddClaim(new Claim(Claims.Subject, principal.GetUserIdentifier())); - } - if (string.IsNullOrEmpty(principal.FindFirst(Claims.Name)?.Value)) - { - identity.AddClaim(new Claim(Claims.Name, principal.GetUserName())); - } + PopulateIdentityClaims(principal, identity); identity.SetScopes(request.GetScopes()); identity.SetResources(await GetResourcesAsync(request.GetScopes())); @@ -599,26 +593,7 @@ private async Task ExchangeAuthorizationCodeOrRefreshTokenGrantTy var identity = (ClaimsIdentity)principal.Identity; 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 - // subject claim is correctly populated (and avoid an InvalidOperationException), it's manually added here. - if (string.IsNullOrEmpty(principal.FindFirst(Claims.Subject)?.Value)) - { - identity.AddClaim(new Claim(Claims.Subject, principal.GetUserIdentifier())); - } - - if (string.IsNullOrEmpty(principal.FindFirst(Claims.Name)?.Value)) - { - identity.AddClaim(new Claim(Claims.Name, principal.GetUserName())); - } - - if (string.IsNullOrEmpty(principal.FindFirst(Claims.Role)?.Value)) - { - foreach (var role in principal.GetRoles()) - { - identity.AddClaim(new Claim(Claims.Role, role)); - } - } + PopulateIdentityClaims(principal, identity); identity.SetDestinations(GetDestinations); diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Controllers/UserInfoController.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Controllers/UserInfoController.cs index 375256166d9..5e451955644 100644 --- a/src/OrchardCore.Modules/OrchardCore.OpenId/Controllers/UserInfoController.cs +++ b/src/OrchardCore.Modules/OrchardCore.OpenId/Controllers/UserInfoController.cs @@ -122,10 +122,10 @@ public async Task Me() if (principal.HasScope(Scopes.Phone)) { - var phone = principal.FindFirst(Claims.PhoneNumber)?.Value ?? - principal.FindFirst(ClaimTypes.MobilePhone)?.Value ?? - principal.FindFirst(ClaimTypes.HomePhone)?.Value ?? - principal.FindFirst(ClaimTypes.OtherPhone)?.Value; + var phone = principal.FindFirst(Claims.PhoneNumber)?.Value + ?? principal.FindFirst(ClaimTypes.MobilePhone)?.Value + ?? principal.FindFirst(ClaimTypes.HomePhone)?.Value + ?? principal.FindFirst(ClaimTypes.OtherPhone)?.Value; if (!string.IsNullOrEmpty(phone)) { @@ -141,10 +141,7 @@ public async Task Me() if (principal.HasScope(Scopes.Roles)) { - var roles = principal.FindAll(Claims.Role) - .Concat(principal.FindAll(ClaimTypes.Role)) - .Select(claim => claim.Value) - .ToArray(); + var roles = principal.GetRoles(); if (roles.Length != 0) { From 97b63424bdec15463644e2ce27f4d9870d480a90 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Wed, 11 Sep 2024 15:51:50 -0700 Subject: [PATCH 5/5] Update src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Kévin Chalet --- src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs index 9a4be04866c..35243f8a8b4 100644 --- a/src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs +++ b/src/OrchardCore.Modules/OrchardCore.OpenId/OpenIdExtensions.cs @@ -23,7 +23,7 @@ internal static string GetUserName(this ClaimsPrincipal principal) throw new InvalidOperationException("No suitable user name can be found in the principal."); internal static string[] GetRoles(this ClaimsPrincipal principal) - => principal.FindAll(c => c.Type is Claims.Role || c.Type is ClaimTypes.Role) + => principal.FindAll(c => c.Type is Claims.Role or ClaimTypes.Role) .Select(x => x.Value) .Distinct() .ToArray();