diff --git a/src/Umbraco.Core/BackOffice/ClaimsPrincipalExtensions.cs b/src/Umbraco.Core/BackOffice/ClaimsPrincipalExtensions.cs
index 62b827dc6d..6a792ce69b 100644
--- a/src/Umbraco.Core/BackOffice/ClaimsPrincipalExtensions.cs
+++ b/src/Umbraco.Core/BackOffice/ClaimsPrincipalExtensions.cs
@@ -27,18 +27,12 @@ namespace Umbraco.Extensions
if (backOfficeIdentity != null) return backOfficeIdentity;
}
- //Otherwise convert to a UmbracoBackOfficeIdentity if it's auth'd and has the back office session
- if (user.Identity is ClaimsIdentity claimsIdentity && claimsIdentity.IsAuthenticated && claimsIdentity.HasClaim(x => x.Type == Constants.Security.SessionIdClaimType))
+ //Otherwise convert to a UmbracoBackOfficeIdentity if it's auth'd
+ if (user.Identity is ClaimsIdentity claimsIdentity
+ && claimsIdentity.IsAuthenticated
+ && UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity, out var umbracoIdentity))
{
- try
- {
- return UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity);
- }
- catch (InvalidOperationException)
- {
- // We catch this error because it's what we throw when the required claims are not in the identity.
- // when that happens something strange is going on, we'll swallow this exception and return null.
- }
+ return umbracoIdentity;
}
return null;
diff --git a/src/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentity.cs b/src/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentity.cs
index 18684e072a..15db0e0dc7 100644
--- a/src/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentity.cs
+++ b/src/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentity.cs
@@ -12,9 +12,31 @@ namespace Umbraco.Core.BackOffice
[Serializable]
public class UmbracoBackOfficeIdentity : ClaimsIdentity
{
- public static UmbracoBackOfficeIdentity FromClaimsIdentity(ClaimsIdentity identity)
+ public static bool FromClaimsIdentity(ClaimsIdentity identity, out UmbracoBackOfficeIdentity backOfficeIdentity)
{
- return new UmbracoBackOfficeIdentity(identity);
+ //validate that all claims exist
+ foreach (var t in RequiredBackOfficeIdentityClaimTypes)
+ {
+ //if the identity doesn't have the claim, or the claim value is null
+ if (identity.HasClaim(x => x.Type == t) == false || identity.HasClaim(x => x.Type == t && x.Value.IsNullOrWhiteSpace()))
+ {
+ backOfficeIdentity = null;
+ return false;
+ }
+ }
+
+ backOfficeIdentity = new UmbracoBackOfficeIdentity(identity);
+ return true;
+ }
+
+ ///
+ /// Create a back office identity based on an existing claims identity
+ ///
+ ///
+ private UmbracoBackOfficeIdentity(ClaimsIdentity identity)
+ : base(identity.Claims, Constants.Security.BackOfficeAuthenticationType)
+ {
+ Actor = identity;
}
///
@@ -71,26 +93,6 @@ namespace Umbraco.Core.BackOffice
AddRequiredClaims(userId, username, realName, startContentNodes, startMediaNodes, culture, securityStamp, allowedApps, roles);
}
- ///
- /// Create a back office identity based on an existing claims identity
- ///
- ///
- private UmbracoBackOfficeIdentity(ClaimsIdentity identity)
- : base(identity.Claims, Constants.Security.BackOfficeAuthenticationType)
- {
- Actor = identity;
-
- //validate that all claims exist
- foreach (var t in RequiredBackOfficeIdentityClaimTypes)
- {
- //if the identity doesn't have the claim, or the claim value is null
- if (identity.HasClaim(x => x.Type == t) == false || identity.HasClaim(x => x.Type == t && x.Value.IsNullOrWhiteSpace()))
- {
- throw new InvalidOperationException("Cannot create a " + typeof(UmbracoBackOfficeIdentity) + " from " + typeof(ClaimsIdentity) + " since the required claim " + t + " is missing");
- }
- }
- }
-
public const string Issuer = Constants.Security.BackOfficeAuthenticationType;
///
diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentityTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentityTests.cs
index d3380c06e4..ed1d681a77 100644
--- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentityTests.cs
+++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentityTests.cs
@@ -16,7 +16,6 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice
[Test]
public void Create_From_Claims_Identity()
{
- var sessionId = Guid.NewGuid().ToString();
var securityStamp = Guid.NewGuid().ToString();
var claimsIdentity = new ClaimsIdentity(new[]
{
@@ -31,12 +30,12 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice
new Claim(Constants.Security.AllowedApplicationsClaimType, "content", ClaimValueTypes.String, TestIssuer, TestIssuer),
new Claim(Constants.Security.AllowedApplicationsClaimType, "media", ClaimValueTypes.String, TestIssuer, TestIssuer),
new Claim(ClaimTypes.Locality, "en-us", ClaimValueTypes.String, TestIssuer, TestIssuer),
- new Claim(Constants.Security.SessionIdClaimType, sessionId, Constants.Security.SessionIdClaimType, TestIssuer, TestIssuer),
new Claim(ClaimsIdentity.DefaultRoleClaimType, "admin", ClaimValueTypes.String, TestIssuer, TestIssuer),
new Claim(Constants.Security.SecurityStampClaimType, securityStamp, ClaimValueTypes.String, TestIssuer, TestIssuer),
});
- var backofficeIdentity = UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity);
+ if (!UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity, out var backofficeIdentity))
+ Assert.Fail();
Assert.AreEqual(1234, backofficeIdentity.Id);
//Assert.AreEqual(sessionId, backofficeIdentity.SessionId);
@@ -61,13 +60,15 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice
new Claim(ClaimTypes.Name, "testing", ClaimValueTypes.String, TestIssuer, TestIssuer),
});
- Assert.Throws(() => UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity));
+ if (UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity, out var backofficeIdentity))
+ Assert.Fail();
+
+ Assert.Pass();
}
[Test]
public void Create_From_Claims_Identity_Required_Claim_Null()
{
- var sessionId = Guid.NewGuid().ToString();
var claimsIdentity = new ClaimsIdentity(new[]
{
//null or empty
@@ -79,11 +80,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice
new Claim(Constants.Security.AllowedApplicationsClaimType, "content", ClaimValueTypes.String, TestIssuer, TestIssuer),
new Claim(Constants.Security.AllowedApplicationsClaimType, "media", ClaimValueTypes.String, TestIssuer, TestIssuer),
new Claim(ClaimTypes.Locality, "en-us", ClaimValueTypes.String, TestIssuer, TestIssuer),
- new Claim(Constants.Security.SessionIdClaimType, sessionId, Constants.Security.SessionIdClaimType, TestIssuer, TestIssuer),
new Claim(ClaimsIdentity.DefaultRoleClaimType, "admin", ClaimValueTypes.String, TestIssuer, TestIssuer),
});
- Assert.Throws(() => UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity));
+ if (UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity, out var backofficeIdentity))
+ Assert.Fail();
+
+ Assert.Pass();
}
diff --git a/src/Umbraco.Web.BackOffice/Security/BackOfficeSecureDataFormat.cs b/src/Umbraco.Web.BackOffice/Security/BackOfficeSecureDataFormat.cs
index 88017f509a..3a23ae6c88 100644
--- a/src/Umbraco.Web.BackOffice/Security/BackOfficeSecureDataFormat.cs
+++ b/src/Umbraco.Web.BackOffice/Security/BackOfficeSecureDataFormat.cs
@@ -60,18 +60,8 @@ namespace Umbraco.Web.BackOffice.Security
return null;
}
- UmbracoBackOfficeIdentity identity;
-
- try
- {
- identity = UmbracoBackOfficeIdentity.FromClaimsIdentity((ClaimsIdentity)decrypt.Principal.Identity);
- }
- catch (Exception)
- {
- //if it cannot be created return null, will be due to serialization errors in user data most likely due to corrupt cookies or cookies
- //for previous versions of Umbraco
+ if (!UmbracoBackOfficeIdentity.FromClaimsIdentity((ClaimsIdentity)decrypt.Principal.Identity, out var identity))
return null;
- }
//return the ticket with a UmbracoBackOfficeIdentity
var ticket = new AuthenticationTicket(new ClaimsPrincipal(identity), decrypt.Properties, decrypt.AuthenticationScheme);
diff --git a/src/Umbraco.Web/Security/PreviewAuthenticationMiddleware.cs b/src/Umbraco.Web/Security/PreviewAuthenticationMiddleware.cs
index 799edb5f60..feecd85364 100644
--- a/src/Umbraco.Web/Security/PreviewAuthenticationMiddleware.cs
+++ b/src/Umbraco.Web/Security/PreviewAuthenticationMiddleware.cs
@@ -1,4 +1,5 @@
-using System.Security.Claims;
+using System;
+using System.Security.Claims;
using System.Threading.Tasks;
using Microsoft.Owin;
using Umbraco.Core;
@@ -59,7 +60,10 @@ namespace Umbraco.Web.Security
//Ok, we've got a real ticket, now we can add this ticket's identity to the current
// Principal, this means we'll have 2 identities assigned to the principal which we can
// use to authorize the preview and allow for a back office User.
- claimsPrincipal.AddIdentity(UmbracoBackOfficeIdentity.FromClaimsIdentity(unprotected.Identity));
+ if (!UmbracoBackOfficeIdentity.FromClaimsIdentity(unprotected.Identity, out var umbracoIdentity))
+ throw new InvalidOperationException("Cannot convert identity");
+
+ claimsPrincipal.AddIdentity(umbracoIdentity);
}
}
}
diff --git a/src/Umbraco.Web/Security/UmbracoSecureDataFormat.cs b/src/Umbraco.Web/Security/UmbracoSecureDataFormat.cs
index 41a2ad3bba..73c1c3fd55 100644
--- a/src/Umbraco.Web/Security/UmbracoSecureDataFormat.cs
+++ b/src/Umbraco.Web/Security/UmbracoSecureDataFormat.cs
@@ -55,18 +55,8 @@ namespace Umbraco.Web.Security
return null;
}
- UmbracoBackOfficeIdentity identity;
-
- try
- {
- identity = UmbracoBackOfficeIdentity.FromClaimsIdentity(decrypt.Identity);
- }
- catch (Exception)
- {
- //if it cannot be created return null, will be due to serialization errors in user data most likely due to corrupt cookies or cookies
- //for previous versions of Umbraco
+ if (!UmbracoBackOfficeIdentity.FromClaimsIdentity(decrypt.Identity, out var identity))
return null;
- }
//return the ticket with a UmbracoBackOfficeIdentity
var ticket = new AuthenticationTicket(identity, decrypt.Properties);
diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj
index 959aab671c..51249d6348 100755
--- a/src/Umbraco.Web/Umbraco.Web.csproj
+++ b/src/Umbraco.Web/Umbraco.Web.csproj
@@ -152,7 +152,6 @@
-