From e3dc017d7ec39861bd65d7cb6580e243c01e59af Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 23 Feb 2021 10:41:00 +1100 Subject: [PATCH] Keep custom claims that are flowed from autolinking When auto linking with callbacks such as OnExternalLogin, custom claims can be added to the user which are flowed to the auth ticket/identity. However, when the security stamp validator executes, the identity is re-created manually without any knowledge of those custom claims so they are lost. This ensures that those custom claims flow through to the re-generated identity during the security stamp validation phase. --- .../Security/AppBuilderExtensions.cs | 23 ++++++++++--- .../BackOfficeClaimsIdentityFactory.cs | 12 +++---- .../Security/ClaimsIdentityExtensions.cs | 34 +++++++++++++++++++ src/Umbraco.Web/Umbraco.Web.csproj | 1 + 4 files changed, 58 insertions(+), 12 deletions(-) create mode 100644 src/Umbraco.Web/Security/ClaimsIdentityExtensions.cs diff --git a/src/Umbraco.Web/Security/AppBuilderExtensions.cs b/src/Umbraco.Web/Security/AppBuilderExtensions.cs index 8f33f10eea..ca92de49eb 100644 --- a/src/Umbraco.Web/Security/AppBuilderExtensions.cs +++ b/src/Umbraco.Web/Security/AppBuilderExtensions.cs @@ -169,11 +169,24 @@ namespace Umbraco.Web.Security // Enables the application to validate the security stamp when the user // logs in. This is a security feature which is used when you // change a password or add an external login to your account. - OnValidateIdentity = SecurityStampValidator - .OnValidateIdentity( - TimeSpan.FromMinutes(30), - (manager, user) => manager.GenerateUserIdentityAsync(user), - identity => identity.GetUserId()), + OnValidateIdentity = context => + { + var identity = context.Identity; + + return SecurityStampValidator + .OnValidateIdentity( + TimeSpan.FromMinutes(3), + async (manager, user) => + { + var regenerated = await manager.GenerateUserIdentityAsync(user); + + // Keep any custom claims from the original identity + regenerated.MergeClaimsFromBackOfficeIdentity(identity); + + return regenerated; + }, + identity => identity.GetUserId())(context); + } }; diff --git a/src/Umbraco.Web/Security/BackOfficeClaimsIdentityFactory.cs b/src/Umbraco.Web/Security/BackOfficeClaimsIdentityFactory.cs index d61d2ea711..2883ae8faf 100644 --- a/src/Umbraco.Web/Security/BackOfficeClaimsIdentityFactory.cs +++ b/src/Umbraco.Web/Security/BackOfficeClaimsIdentityFactory.cs @@ -27,12 +27,6 @@ namespace Umbraco.Web.Security { var baseIdentity = await base.CreateAsync(manager, user, authenticationType); - // now we can flow any custom claims that the actual user has currently assigned which could be done in the OnExternalLogin callback - foreach (var claim in user.Claims) - { - baseIdentity.AddClaim(new Claim(claim.ClaimType, claim.ClaimValue)); - } - var umbracoIdentity = new UmbracoBackOfficeIdentity(baseIdentity, user.Id, user.UserName, @@ -40,12 +34,16 @@ namespace Umbraco.Web.Security user.CalculatedContentStartNodeIds, user.CalculatedMediaStartNodeIds, user.Culture, - //NOTE - there is no session id assigned here, this is just creating the identity, a session id will be generated when the cookie is written + // NOTE - there is no session id assigned here, this is just creating the identity, a session id will be generated when the cookie is written Guid.NewGuid().ToString(), user.SecurityStamp, user.AllowedSections, user.Roles.Select(x => x.RoleId).ToArray()); + // now we can flow any custom claims that the actual user has currently + // assigned which could be done in the OnExternalLogin callback + umbracoIdentity.MergeClaimsFromBackOfficeIdentity(user); + return umbracoIdentity; } } diff --git a/src/Umbraco.Web/Security/ClaimsIdentityExtensions.cs b/src/Umbraco.Web/Security/ClaimsIdentityExtensions.cs new file mode 100644 index 0000000000..aa8549462d --- /dev/null +++ b/src/Umbraco.Web/Security/ClaimsIdentityExtensions.cs @@ -0,0 +1,34 @@ +using System.Linq; +using System.Security.Claims; +using Umbraco.Core.Models.Identity; +using Constants = Umbraco.Core.Constants; + +namespace Umbraco.Web.Security +{ + internal static class ClaimsIdentityExtensions + { + // Ignore these Claims when merging, these claims are dynamically added whenever the ticket + // is re-issued and we don't want to merge old values of these. + private static readonly string[] IgnoredClaims = new[] { ClaimTypes.CookiePath, Constants.Security.SessionIdClaimType }; + + internal static void MergeClaimsFromBackOfficeIdentity(this ClaimsIdentity destination, ClaimsIdentity source) + { + foreach (var claim in source.Claims + .Where(claim => !IgnoredClaims.Contains(claim.Type)) + .Where(claim => !destination.HasClaim(claim.Type, claim.Value))) + { + destination.AddClaim(new Claim(claim.Type, claim.Value)); + } + } + + internal static void MergeClaimsFromBackOfficeIdentity(this ClaimsIdentity destination, BackOfficeIdentityUser source) + { + foreach (var claim in source.Claims + .Where(claim => !IgnoredClaims.Contains(claim.ClaimType)) + .Where(claim => !destination.HasClaim(claim.ClaimType, claim.ClaimValue))) + { + destination.AddClaim(new Claim(claim.ClaimType, claim.ClaimValue)); + } + } + } +} diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 16068b66f0..8717ca6a13 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -288,6 +288,7 @@ +