diff --git a/src/Umbraco.Core/Security/ClaimsIdentityExtensions.cs b/src/Umbraco.Core/Security/ClaimsIdentityExtensions.cs index 9278f6773e..a7338cebb1 100644 --- a/src/Umbraco.Core/Security/ClaimsIdentityExtensions.cs +++ b/src/Umbraco.Core/Security/ClaimsIdentityExtensions.cs @@ -82,7 +82,8 @@ namespace Umbraco.Extensions } } - if (identity.HasClaim(x => x.Type == Constants.Security.StartMediaNodeIdClaimType) == false) + if (identity.HasClaim(x => x.Type == Constants.Security.StartMediaNodeIdClaimType) == false && + startMediaNodes != null) { foreach (var startMediaNode in startMediaNodes) { diff --git a/src/Umbraco.Core/Security/UmbracoBackOfficeIdentity.cs b/src/Umbraco.Core/Security/UmbracoBackOfficeIdentity.cs index 86ebd7f7d3..03d0699a12 100644 --- a/src/Umbraco.Core/Security/UmbracoBackOfficeIdentity.cs +++ b/src/Umbraco.Core/Security/UmbracoBackOfficeIdentity.cs @@ -12,187 +12,5 @@ namespace Umbraco.Core.Security [Serializable] public class UmbracoBackOfficeIdentity : ClaimsIdentity { - // TODO: Ideally we remove this class and only deal with ClaimsIdentity as a best practice. All things relevant to our own - // identity are part of claims. This class would essentially become extension methods on a ClaimsIdentity for resolving - // values from it. - // public static bool FromClaimsIdentity(ClaimsIdentity identity, out UmbracoBackOfficeIdentity backOfficeIdentity) - // { - // // 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) - { - } - - /// - /// Creates a new UmbracoBackOfficeIdentity - /// - /// - /// - /// - /// - /// - /// - /// - /// - /// - public UmbracoBackOfficeIdentity(string userId, string username, string realName, - IEnumerable startContentNodes, IEnumerable startMediaNodes, string culture, - string securityStamp, IEnumerable allowedApps, IEnumerable roles) - : base(Enumerable.Empty(), Constants.Security.BackOfficeAuthenticationType) //this ctor is used to ensure the IsAuthenticated property is true - { - if (allowedApps == null) - throw new ArgumentNullException(nameof(allowedApps)); - if (string.IsNullOrWhiteSpace(username)) - throw new ArgumentException("Value cannot be null or whitespace.", nameof(username)); - if (string.IsNullOrWhiteSpace(realName)) - throw new ArgumentException("Value cannot be null or whitespace.", nameof(realName)); - if (string.IsNullOrWhiteSpace(culture)) - throw new ArgumentException("Value cannot be null or whitespace.", nameof(culture)); - if (string.IsNullOrWhiteSpace(securityStamp)) - throw new ArgumentException("Value cannot be null or whitespace.", nameof(securityStamp)); - AddRequiredClaims(userId, username, realName, startContentNodes, startMediaNodes, culture, securityStamp, allowedApps, roles); - } - - /// - /// Creates a new UmbracoBackOfficeIdentity - /// - /// - /// The original identity created by the ClaimsIdentityFactory - /// - /// - /// - /// - /// - /// - /// - /// - /// - /// - public UmbracoBackOfficeIdentity(ClaimsIdentity childIdentity, - string userId, string username, string realName, - IEnumerable startContentNodes, IEnumerable startMediaNodes, string culture, - string securityStamp, IEnumerable allowedApps, IEnumerable roles) - : base(childIdentity.Claims, Constants.Security.BackOfficeAuthenticationType) - { - if (string.IsNullOrWhiteSpace(username)) - throw new ArgumentException("Value cannot be null or whitespace.", nameof(username)); - if (string.IsNullOrWhiteSpace(realName)) - throw new ArgumentException("Value cannot be null or whitespace.", nameof(realName)); - if (string.IsNullOrWhiteSpace(culture)) - throw new ArgumentException("Value cannot be null or whitespace.", nameof(culture)); - if (string.IsNullOrWhiteSpace(securityStamp)) - throw new ArgumentException("Value cannot be null or whitespace.", nameof(securityStamp)); - - AddRequiredClaims(userId, username, realName, startContentNodes, startMediaNodes, culture, securityStamp, allowedApps, roles); - } - - public const string Issuer = Constants.Security.BackOfficeAuthenticationType; - - /// - /// Returns the required claim types for a back office identity - /// - /// - /// This does not include the role claim type or allowed apps type since that is a collection and in theory could be empty - /// - public static IEnumerable RequiredBackOfficeIdentityClaimTypes => new[] - { - ClaimTypes.NameIdentifier, //id - ClaimTypes.Name, //username - ClaimTypes.GivenName, - Constants.Security.StartContentNodeIdClaimType, - Constants.Security.StartMediaNodeIdClaimType, - ClaimTypes.Locality, - Constants.Security.SecurityStampClaimType - }; - - /// - /// Adds claims based on the ctor data - /// - private void AddRequiredClaims(string userId, string username, string realName, - IEnumerable startContentNodes, IEnumerable startMediaNodes, string culture, - string securityStamp, IEnumerable allowedApps, IEnumerable roles) - { - // TODO: This has been moved, delete - //This is the id that 'identity' uses to check for the user id - if (HasClaim(x => x.Type == ClaimTypes.NameIdentifier) == false) - AddClaim(new Claim(ClaimTypes.NameIdentifier, userId, ClaimValueTypes.String, Issuer, Issuer, this)); - - if (HasClaim(x => x.Type == ClaimTypes.Name) == false) - AddClaim(new Claim(ClaimTypes.Name, username, ClaimValueTypes.String, Issuer, Issuer, this)); - - if (HasClaim(x => x.Type == ClaimTypes.GivenName) == false) - AddClaim(new Claim(ClaimTypes.GivenName, realName, ClaimValueTypes.String, Issuer, Issuer, this)); - - if (HasClaim(x => x.Type == Constants.Security.StartContentNodeIdClaimType) == false && startContentNodes != null) - { - foreach (var startContentNode in startContentNodes) - { - AddClaim(new Claim(Constants.Security.StartContentNodeIdClaimType, startContentNode.ToInvariantString(), ClaimValueTypes.Integer32, Issuer, Issuer, this)); - } - } - - if (HasClaim(x => x.Type == Constants.Security.StartMediaNodeIdClaimType) == false && startMediaNodes != null) - { - foreach (var startMediaNode in startMediaNodes) - { - AddClaim(new Claim(Constants.Security.StartMediaNodeIdClaimType, startMediaNode.ToInvariantString(), ClaimValueTypes.Integer32, Issuer, Issuer, this)); - } - } - - if (HasClaim(x => x.Type == ClaimTypes.Locality) == false) - AddClaim(new Claim(ClaimTypes.Locality, culture, ClaimValueTypes.String, Issuer, Issuer, this)); - - //The security stamp claim is also required - if (HasClaim(x => x.Type == Constants.Security.SecurityStampClaimType) == false) - AddClaim(new Claim(Constants.Security.SecurityStampClaimType, securityStamp, ClaimValueTypes.String, Issuer, Issuer, this)); - - //Add each app as a separate claim - if (HasClaim(x => x.Type == Constants.Security.AllowedApplicationsClaimType) == false && allowedApps != null) - { - foreach (var application in allowedApps) - { - AddClaim(new Claim(Constants.Security.AllowedApplicationsClaimType, application, ClaimValueTypes.String, Issuer, Issuer, this)); - } - } - - //Claims are added by the ClaimsIdentityFactory because our UserStore supports roles, however this identity might - // not be made with that factory if it was created with a different ticket so perform the check - if (HasClaim(x => x.Type == DefaultRoleClaimType) == false && roles != null) - { - //manually add them - foreach (var roleName in roles) - { - AddClaim(new Claim(RoleClaimType, roleName, ClaimValueTypes.String, Issuer, Issuer, this)); - } - } - - } - - /// - /// - /// Gets the type of authenticated identity. - /// - /// - /// The type of authenticated identity. This property always returns "UmbracoBackOffice". - /// - public override string AuthenticationType => Issuer; } } diff --git a/src/Umbraco.Infrastructure/Security/BackOfficeClaimsPrincipalFactory.cs b/src/Umbraco.Infrastructure/Security/BackOfficeClaimsPrincipalFactory.cs index 77f707d812..f3f284fa84 100644 --- a/src/Umbraco.Infrastructure/Security/BackOfficeClaimsPrincipalFactory.cs +++ b/src/Umbraco.Infrastructure/Security/BackOfficeClaimsPrincipalFactory.cs @@ -4,7 +4,7 @@ using System.Security.Claims; using System.Threading.Tasks; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Options; -using Umbraco.Core.Security; +using Umbraco.Extensions; namespace Umbraco.Core.Security { @@ -45,8 +45,7 @@ namespace Umbraco.Core.Security // TODO: We want to remove UmbracoBackOfficeIdentity and only rely on ClaimsIdentity, once // that is done then we'll create a ClaimsIdentity with all of the requirements here instead - var umbracoIdentity = new UmbracoBackOfficeIdentity( - baseIdentity, + baseIdentity.AddRequiredClaims( user.Id, user.UserName, user.Name, @@ -57,7 +56,7 @@ namespace Umbraco.Core.Security user.AllowedSections, user.Roles.Select(x => x.RoleId).ToArray()); - return new ClaimsPrincipal(umbracoIdentity); + return new ClaimsPrincipal(baseIdentity); } /// diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/BackOfficeClaimsPrincipalFactoryTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/BackOfficeClaimsPrincipalFactoryTests.cs index e681fc6841..8662b6cfcb 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/BackOfficeClaimsPrincipalFactoryTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/BackOfficeClaimsPrincipalFactoryTests.cs @@ -56,13 +56,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice } [Test] - public async Task CreateAsync_Should_Create_Principal_With_Umbraco_Identity() + public async Task CreateAsync_Should_Create_Principal_With_Claims_Identity() { BackOfficeClaimsPrincipalFactory sut = CreateSut(); ClaimsPrincipal claimsPrincipal = await sut.CreateAsync(_testUser); - var umbracoBackOfficeIdentity = claimsPrincipal.Identity as UmbracoBackOfficeIdentity; + var umbracoBackOfficeIdentity = claimsPrincipal.Identity as ClaimsIdentity; Assert.IsNotNull(umbracoBackOfficeIdentity); } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentityTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentityTests.cs index 825abd6658..018c375982 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentityTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentityTests.cs @@ -112,8 +112,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice new Claim("TestClaim1", "test", ClaimValueTypes.Integer32, TestIssuer, TestIssuer) }); - var identity = new UmbracoBackOfficeIdentity( - claimsIdentity, + claimsIdentity.AddRequiredClaims( "1234", "testing", "hello world", @@ -124,8 +123,8 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice new[] { "content", "media" }, new[] { "admin" }); - Assert.AreEqual(12, identity.Claims.Count()); - Assert.IsNull(identity.Actor); + Assert.AreEqual(12, claimsIdentity.Claims.Count()); + Assert.IsNull(claimsIdentity.Actor); } } } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Extensions/ClaimsPrincipalExtensionsTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Extensions/ClaimsPrincipalExtensionsTests.cs index 7b699e7b0c..d8930cc58e 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Extensions/ClaimsPrincipalExtensionsTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Extensions/ClaimsPrincipalExtensionsTests.cs @@ -8,6 +8,7 @@ using NUnit.Framework; using Umbraco.Core; using Umbraco.Core.Security; using Umbraco.Extensions; +using ClaimsIdentityExtensions = Umbraco.Extensions.ClaimsIdentityExtensions; namespace Umbraco.Tests.UnitTests.Umbraco.Core.Extensions { @@ -17,7 +18,8 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.Extensions [Test] public void Get_Remaining_Ticket_Seconds() { - var backOfficeIdentity = new UmbracoBackOfficeIdentity( + var backOfficeIdentity = new ClaimsIdentity(); + backOfficeIdentity.AddRequiredClaims( Constants.Security.SuperUserIdAsString, "test", "test", @@ -27,6 +29,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.Extensions Guid.NewGuid().ToString(), Enumerable.Empty(), Enumerable.Empty()); + var principal = new ClaimsPrincipal(backOfficeIdentity); var expireSeconds = 99; @@ -40,8 +43,8 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.Extensions Constants.Security.TicketExpiresClaimType, expires, ClaimValueTypes.DateTime, - UmbracoBackOfficeIdentity.Issuer, - UmbracoBackOfficeIdentity.Issuer, + ClaimsIdentityExtensions.Issuer, + ClaimsIdentityExtensions.Issuer, backOfficeIdentity)); var ticketRemainingSeconds = principal.GetRemainingAuthSeconds(then); diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Security/BackOfficeAntiforgeryTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Security/BackOfficeAntiforgeryTests.cs index 568318006e..494ab7fdc3 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Security/BackOfficeAntiforgeryTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Security/BackOfficeAntiforgeryTests.cs @@ -13,6 +13,7 @@ using Microsoft.Net.Http.Headers; using NUnit.Framework; using Umbraco.Core; using Umbraco.Core.Security; +using Umbraco.Extensions; using Umbraco.Web.BackOffice.Security; namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Security @@ -22,18 +23,21 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Security { private HttpContext GetHttpContext() { + var identity = new ClaimsIdentity(); + identity.AddRequiredClaims( + Constants.Security.SuperUserIdAsString, + "test", + "test", + Enumerable.Empty(), + Enumerable.Empty(), + "en-US", + Guid.NewGuid().ToString(), + Enumerable.Empty(), + Enumerable.Empty()); + var httpContext = new DefaultHttpContext() { - User = new ClaimsPrincipal(new UmbracoBackOfficeIdentity( - Constants.Security.SuperUserIdAsString, - "test", - "test", - Enumerable.Empty(), - Enumerable.Empty(), - "en-US", - Guid.NewGuid().ToString(), - Enumerable.Empty(), - Enumerable.Empty())) + User = new ClaimsPrincipal(identity) }; httpContext.Request.IsHttps = true; return httpContext; diff --git a/src/Umbraco.Tests/TestHelpers/ControllerTesting/AuthenticateEverythingMiddleware.cs b/src/Umbraco.Tests/TestHelpers/ControllerTesting/AuthenticateEverythingMiddleware.cs index e702753236..104486bf14 100644 --- a/src/Umbraco.Tests/TestHelpers/ControllerTesting/AuthenticateEverythingMiddleware.cs +++ b/src/Umbraco.Tests/TestHelpers/ControllerTesting/AuthenticateEverythingMiddleware.cs @@ -1,10 +1,12 @@ using System; +using System.Security.Claims; using System.Threading.Tasks; using Microsoft.Owin; using Microsoft.Owin.Security; using Microsoft.Owin.Security.Infrastructure; using Owin; using Umbraco.Core.Security; +using Umbraco.Extensions; namespace Umbraco.Tests.TestHelpers.ControllerTesting { @@ -28,8 +30,17 @@ namespace Umbraco.Tests.TestHelpers.ControllerTesting protected override Task AuthenticateCoreAsync() { var securityStamp = Guid.NewGuid().ToString(); - var identity = new UmbracoBackOfficeIdentity( - Umbraco.Core.Constants.Security.SuperUserIdAsString, "admin", "Admin", new[] { -1 }, new[] { -1 }, "en-US", securityStamp, new[] { "content", "media", "members" }, new[] { "admin" }); + + var identity = new ClaimsIdentity(); + identity.AddRequiredClaims(Core.Constants.Security.SuperUserIdAsString, + "admin", + "Admin", + new[] { -1 }, + new[] { -1 }, + "en-US", + securityStamp, + new[] { "content", "media", "members" }, + new[] { "admin" }); return Task.FromResult(new AuthenticationTicket( identity, diff --git a/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs index 8f0f0c4f8d..2223fbfb6b 100644 --- a/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs +++ b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs @@ -19,6 +19,7 @@ using Umbraco.Core.Services; using Umbraco.Extensions; using Umbraco.Net; using Umbraco.Web.Common.Security; +using ClaimsIdentityExtensions = Umbraco.Extensions.ClaimsIdentityExtensions; namespace Umbraco.Web.BackOffice.Security { @@ -157,8 +158,8 @@ namespace Umbraco.Web.BackOffice.Security Constants.Security.TicketExpiresClaimType, ctx.Properties.ExpiresUtc.Value.ToString("o"), ClaimValueTypes.DateTime, - UmbracoBackOfficeIdentity.Issuer, - UmbracoBackOfficeIdentity.Issuer, + ClaimsIdentityExtensions.Issuer, + ClaimsIdentityExtensions.Issuer, backOfficeIdentity)); }, @@ -175,10 +176,10 @@ namespace Umbraco.Web.BackOffice.Security : Guid.NewGuid(); // add our session claim - backOfficeIdentity.AddClaim(new Claim(Constants.Security.SessionIdClaimType, session.ToString(), ClaimValueTypes.String, UmbracoBackOfficeIdentity.Issuer, UmbracoBackOfficeIdentity.Issuer, backOfficeIdentity)); + backOfficeIdentity.AddClaim(new Claim(Constants.Security.SessionIdClaimType, session.ToString(), ClaimValueTypes.String, ClaimsIdentityExtensions.Issuer, ClaimsIdentityExtensions.Issuer, backOfficeIdentity)); // since it is a cookie-based authentication add that claim - backOfficeIdentity.AddClaim(new Claim(ClaimTypes.CookiePath, "/", ClaimValueTypes.String, UmbracoBackOfficeIdentity.Issuer, UmbracoBackOfficeIdentity.Issuer, backOfficeIdentity)); + backOfficeIdentity.AddClaim(new Claim(ClaimTypes.CookiePath, "/", ClaimValueTypes.String, ClaimsIdentityExtensions.Issuer, ClaimsIdentityExtensions.Issuer, backOfficeIdentity)); } return Task.CompletedTask;