diff --git a/src/Umbraco.Tests/Security/BackOfficeClaimsPrincipalFactoryTests.cs b/src/Umbraco.Tests/Security/BackOfficeClaimsPrincipalFactoryTests.cs index 6f55381a62..b7b516318c 100644 --- a/src/Umbraco.Tests/Security/BackOfficeClaimsPrincipalFactoryTests.cs +++ b/src/Umbraco.Tests/Security/BackOfficeClaimsPrincipalFactoryTests.cs @@ -1,11 +1,12 @@ -using System.Collections.Generic; -using System.Linq; +using System; +using System.Collections.Generic; using System.Security.Claims; using System.Threading.Tasks; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Options; using Moq; using NUnit.Framework; +using Umbraco.Core; using Umbraco.Core.Configuration; using Umbraco.Core.Models.Membership; using Umbraco.Core.Security; @@ -20,6 +21,40 @@ namespace Umbraco.Tests.Security private BackOfficeIdentityUser _testUser; private Mock> _mockUserManager; + [Test] + public void Ctor_When_UserManager_Is_Null_Expect_ArgumentNullException() + { + Assert.Throws(() => new BackOfficeClaimsPrincipalFactory( + null, + new OptionsWrapper(new IdentityOptions()))); + } + + [Test] + public void Ctor_When_Options_Are_Null_Expect_ArgumentNullException() + { + Assert.Throws(() => new BackOfficeClaimsPrincipalFactory( + new Mock>(new Mock>().Object, + null, null, null, null, null, null, null, null).Object, + null)); + } + + [Test] + public void Ctor_When_Options_Value_Is_Null_Expect_ArgumentNullException() + { + Assert.Throws(() => new BackOfficeClaimsPrincipalFactory( + new Mock>(new Mock>().Object, + null, null, null, null, null, null, null, null).Object, + new OptionsWrapper(null))); + } + + [Test] + public void CreateAsync_When_User_Is_Null_Expect_ArgumentNullException() + { + var sut = CreateSut(); + + Assert.ThrowsAsync(async () => await sut.CreateAsync(null)); + } + [Test] public async Task CreateAsync_Should_Create_Principal_With_Umbraco_Identity() { @@ -34,23 +69,95 @@ namespace Umbraco.Tests.Security [Test] public async Task CreateAsync_Should_Create_NameId() { + const string expectedClaimType = ClaimTypes.NameIdentifier; + var expectedClaimValue = _testUser.Id.ToString(); + var sut = CreateSut(); var claimsPrincipal = await sut.CreateAsync(_testUser); - Assert.True(claimsPrincipal.HasClaim(ClaimTypes.NameIdentifier, _testUser.Id.ToString())); + Assert.True(claimsPrincipal.HasClaim(expectedClaimType, expectedClaimValue)); + Assert.True(claimsPrincipal.GetUmbracoIdentity().Actor.HasClaim(expectedClaimType, expectedClaimValue)); + } + + [Test] + public async Task CreateAsync_Should_Create_Name() + { + const string expectedClaimType = ClaimTypes.Name; + var expectedClaimValue = _testUser.UserName; + + var sut = CreateSut(); + + var claimsPrincipal = await sut.CreateAsync(_testUser); + + Assert.True(claimsPrincipal.HasClaim(expectedClaimType, expectedClaimValue)); + Assert.True(claimsPrincipal.GetUmbracoIdentity().Actor.HasClaim(expectedClaimType, expectedClaimValue)); } [Test] public async Task CreateAsync_Should_Create_IdentityProvider() { + const string expectedClaimType = "http://schemas.microsoft.com/accesscontrolservice/2010/07/claims/identityprovider"; + const string expectedClaimValue = "ASP.NET Identity"; + var sut = CreateSut(); var claimsPrincipal = await sut.CreateAsync(_testUser); - Assert.True(claimsPrincipal.HasClaim( - "http://schemas.microsoft.com/accesscontrolservice/2010/07/claims/identityprovider", - "ASP.NET Identity")); + Assert.True(claimsPrincipal.HasClaim(expectedClaimType, expectedClaimValue)); + Assert.True(claimsPrincipal.GetUmbracoIdentity().Actor.HasClaim(expectedClaimType, expectedClaimValue)); + } + + [Test] + public async Task CreateAsync_When_SecurityStamp_Supported_Expect_SecurityStamp_Claim() + { + const string expectedClaimType = Constants.Web.SecurityStampClaimType; + var expectedClaimValue = _testUser.SecurityStamp; + + _mockUserManager.Setup(x => x.SupportsUserSecurityStamp).Returns(true); + _mockUserManager.Setup(x => x.GetSecurityStampAsync(_testUser)).ReturnsAsync(_testUser.SecurityStamp); + + var sut = CreateSut(); + + var claimsPrincipal = await sut.CreateAsync(_testUser); + + Assert.True(claimsPrincipal.HasClaim(expectedClaimType, expectedClaimValue)); + Assert.True(claimsPrincipal.GetUmbracoIdentity().Actor.HasClaim(expectedClaimType, expectedClaimValue)); + } + + [Test] + public async Task CreateAsync_When_Roles_Supported_Expect_Role_Claims_In_UmbracoIdentity() + { + const string expectedClaimType = ClaimTypes.Role; + const string expectedClaimValue = "b87309fb-4caf-48dc-b45a-2b752d051508"; + + _testUser.Roles.Add(new Core.Models.Identity.IdentityUserRole{RoleId = expectedClaimValue}); + _mockUserManager.Setup(x => x.SupportsUserRole).Returns(true); + _mockUserManager.Setup(x => x.GetRolesAsync(_testUser)).ReturnsAsync(new[] {expectedClaimValue}); + + var sut = CreateSut(); + + var claimsPrincipal = await sut.CreateAsync(_testUser); + + Assert.True(claimsPrincipal.HasClaim(expectedClaimType, expectedClaimValue)); + } + + [Test] + public async Task CreateAsync_When_UserClaims_Supported_Expect_UserClaims_In_Actor() + { + const string expectedClaimType = "custom"; + const string expectedClaimValue = "val"; + + _testUser.Claims.Add(new Core.Models.Identity.IdentityUserClaim {ClaimType = expectedClaimType, ClaimValue = expectedClaimValue}); + _mockUserManager.Setup(x => x.SupportsUserClaim).Returns(true); + _mockUserManager.Setup(x => x.GetClaimsAsync(_testUser)).ReturnsAsync( + new List {new Claim(expectedClaimType, expectedClaimValue)}); + + var sut = CreateSut(); + + var claimsPrincipal = await sut.CreateAsync(_testUser); + + Assert.True(claimsPrincipal.GetUmbracoIdentity().Actor.HasClaim(expectedClaimType, expectedClaimValue)); } [SetUp] diff --git a/src/Umbraco.Web/Security/BackOfficeClaimsPrincipalFactory.cs b/src/Umbraco.Web/Security/BackOfficeClaimsPrincipalFactory.cs index faefbce1cf..fe22981831 100644 --- a/src/Umbraco.Web/Security/BackOfficeClaimsPrincipalFactory.cs +++ b/src/Umbraco.Web/Security/BackOfficeClaimsPrincipalFactory.cs @@ -1,11 +1,9 @@ using System; -using System.Collections.Generic; using System.Linq; using System.Security.Claims; using System.Threading.Tasks; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Options; -using Umbraco.Core; using Umbraco.Core.Security; using Umbraco.Web.Models.Identity; @@ -14,6 +12,9 @@ namespace Umbraco.Web.Security public class BackOfficeClaimsPrincipalFactory : UserClaimsPrincipalFactory where TUser : BackOfficeIdentityUser { + private const string _identityProviderClaimType = "http://schemas.microsoft.com/accesscontrolservice/2010/07/claims/identityprovider"; + private const string _identityProviderClaimValue = "ASP.NET Identity"; + public BackOfficeClaimsPrincipalFactory(UserManager userManager, IOptions optionsAccessor) : base(userManager, optionsAccessor) { @@ -21,8 +22,12 @@ namespace Umbraco.Web.Security public override async Task CreateAsync(TUser user) { + if (user == null) throw new ArgumentNullException(nameof(user)); + var baseIdentity = await base.GenerateClaimsAsync(user); - baseIdentity.AddClaim(new Claim("http://schemas.microsoft.com/accesscontrolservice/2010/07/claims/identityprovider", "ASP.NET Identity")); + + // Required by ASP.NET 4.x anti-forgery implementation + baseIdentity.AddClaim(new Claim(_identityProviderClaimType, _identityProviderClaimValue)); var umbracoIdentity = new UmbracoBackOfficeIdentity( baseIdentity, diff --git a/src/Umbraco.Web/Security/BackOfficeUserManager.cs b/src/Umbraco.Web/Security/BackOfficeUserManager.cs index 286d7ba1a7..b9d72ce0c1 100644 --- a/src/Umbraco.Web/Security/BackOfficeUserManager.cs +++ b/src/Umbraco.Web/Security/BackOfficeUserManager.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Security.Claims; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Identity; @@ -89,6 +90,12 @@ namespace Umbraco.Web.Security options.Password.RequireLowercase = passwordConfiguration.RequireLowercase; options.Password.RequireUppercase = passwordConfiguration.RequireUppercase; + // Ensure Umbraco security stamp claim type is used + options.ClaimsIdentity.UserIdClaimType = ClaimTypes.NameIdentifier; + options.ClaimsIdentity.UserNameClaimType = ClaimTypes.Name; + options.ClaimsIdentity.RoleClaimType = ClaimTypes.Role; + options.ClaimsIdentity.SecurityStampClaimType = Constants.Web.SecurityStampClaimType; + options.Lockout.AllowedForNewUsers = true; options.Lockout.MaxFailedAccessAttempts = passwordConfiguration.MaxFailedAccessAttemptsBeforeLockout; //NOTE: This just needs to be in the future, we currently don't support a lockout timespan, it's either they are locked diff --git a/src/Umbraco.Web/Security/IUserSessionStore.cs b/src/Umbraco.Web/Security/IUserSessionStore.cs index b7575475d3..06b7c2f165 100644 --- a/src/Umbraco.Web/Security/IUserSessionStore.cs +++ b/src/Umbraco.Web/Security/IUserSessionStore.cs @@ -10,6 +10,6 @@ namespace Umbraco.Core.Security public interface IUserSessionStore : IUserStore where TUser : class { - Task ValidateSessionIdAsync(string userId, string sessionId); // TODO: SB: Should take a user??? + Task ValidateSessionIdAsync(string userId, string sessionId); } } diff --git a/src/Umbraco.Web/Security/OwinDataProtectorTokenProvider.cs b/src/Umbraco.Web/Security/OwinDataProtectorTokenProvider.cs index 4e90980478..51a42aae60 100644 --- a/src/Umbraco.Web/Security/OwinDataProtectorTokenProvider.cs +++ b/src/Umbraco.Web/Security/OwinDataProtectorTokenProvider.cs @@ -99,7 +99,8 @@ namespace Umbraco.Web.Security public Task CanGenerateTwoFactorTokenAsync(UserManager manager, TUser user) { - return Task.FromResult(true); + // This token provider is designed for flows such as password reset and account confirmation + return Task.FromResult(false); } }