diff --git a/src/Umbraco.Tests/Security/BackOfficeClaimsPrincipalFactoryTests.cs b/src/Umbraco.Tests/Security/BackOfficeClaimsPrincipalFactoryTests.cs new file mode 100644 index 0000000000..6f55381a62 --- /dev/null +++ b/src/Umbraco.Tests/Security/BackOfficeClaimsPrincipalFactoryTests.cs @@ -0,0 +1,85 @@ +using System.Collections.Generic; +using System.Linq; +using System.Security.Claims; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Options; +using Moq; +using NUnit.Framework; +using Umbraco.Core.Configuration; +using Umbraco.Core.Models.Membership; +using Umbraco.Core.Security; +using Umbraco.Web.Models.Identity; +using Umbraco.Web.Security; + +namespace Umbraco.Tests.Security +{ + [TestFixture] + public class BackOfficeClaimsPrincipalFactoryTests + { + private BackOfficeIdentityUser _testUser; + private Mock> _mockUserManager; + + [Test] + public async Task CreateAsync_Should_Create_Principal_With_Umbraco_Identity() + { + var sut = CreateSut(); + + var claimsPrincipal = await sut.CreateAsync(_testUser); + + var umbracoBackOfficeIdentity = claimsPrincipal.Identity as UmbracoBackOfficeIdentity; + Assert.IsNotNull(umbracoBackOfficeIdentity); + } + + [Test] + public async Task CreateAsync_Should_Create_NameId() + { + var sut = CreateSut(); + + var claimsPrincipal = await sut.CreateAsync(_testUser); + + Assert.True(claimsPrincipal.HasClaim(ClaimTypes.NameIdentifier, _testUser.Id.ToString())); + } + + [Test] + public async Task CreateAsync_Should_Create_IdentityProvider() + { + 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")); + } + + [SetUp] + public void Setup() + { + var mockGlobalSettings = new Mock(); + mockGlobalSettings.Setup(x => x.DefaultUILanguage).Returns("test"); + + _testUser = new BackOfficeIdentityUser(mockGlobalSettings.Object, 2, new List()) + { + UserName = "bob", + Name = "Bob", + Email = "bob@umbraco.test", + SecurityStamp = "B6937738-9C17-4C7D-A25A-628A875F5177" + }; + + _mockUserManager = new Mock>(new Mock>().Object, + null, null, null, null, null, null, null, null); + _mockUserManager.Setup(x => x.GetUserIdAsync(_testUser)).ReturnsAsync(_testUser.Id.ToString); + _mockUserManager.Setup(x => x.GetUserNameAsync(_testUser)).ReturnsAsync(_testUser.UserName); + _mockUserManager.Setup(x => x.SupportsUserSecurityStamp).Returns(false); + _mockUserManager.Setup(x => x.SupportsUserClaim).Returns(false); + _mockUserManager.Setup(x => x.SupportsUserRole).Returns(false); + } + + private BackOfficeClaimsPrincipalFactory CreateSut() + { + return new BackOfficeClaimsPrincipalFactory(_mockUserManager.Object, + new OptionsWrapper(new IdentityOptions())); + } + } +} diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 9815c94728..1c2d0ba0da 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -148,6 +148,7 @@ + diff --git a/src/Umbraco.Web/Security/BackOfficeClaimsPrincipalFactory.cs b/src/Umbraco.Web/Security/BackOfficeClaimsPrincipalFactory.cs index 2a90650d97..faefbce1cf 100644 --- a/src/Umbraco.Web/Security/BackOfficeClaimsPrincipalFactory.cs +++ b/src/Umbraco.Web/Security/BackOfficeClaimsPrincipalFactory.cs @@ -1,9 +1,11 @@ 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; @@ -19,10 +21,11 @@ namespace Umbraco.Web.Security public override async Task CreateAsync(TUser user) { - var claimsPrincipal = await base.CreateAsync(user); - + var baseIdentity = await base.GenerateClaimsAsync(user); + baseIdentity.AddClaim(new Claim("http://schemas.microsoft.com/accesscontrolservice/2010/07/claims/identityprovider", "ASP.NET Identity")); + var umbracoIdentity = new UmbracoBackOfficeIdentity( - claimsPrincipal.Identity as ClaimsIdentity, + baseIdentity, user.Id, user.UserName, user.Name, diff --git a/src/Umbraco.Web/Security/BackOfficeUserManager2.cs b/src/Umbraco.Web/Security/BackOfficeUserManager2.cs index af83982647..4ec21e59a5 100644 --- a/src/Umbraco.Web/Security/BackOfficeUserManager2.cs +++ b/src/Umbraco.Web/Security/BackOfficeUserManager2.cs @@ -6,6 +6,7 @@ using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Microsoft.Owin.Security.DataProtection; +using Umbraco.Core; using Umbraco.Core.Configuration; using Umbraco.Core.Mapping; using Umbraco.Core.Security; @@ -293,7 +294,10 @@ namespace Umbraco.Web.Security /// public async Task ChangePasswordWithResetAsync(int userId, string token, string newPassword) { - var user = await base.FindByIdAsync(userId.ToString()); + var userIdAsString = userId.TryConvertTo(); + if (!userIdAsString.Success) throw new InvalidOperationException("Unable to convert userId to int"); + + var user = await base.FindByIdAsync(userIdAsString.Result); var result = await base.ResetPasswordAsync(user, token, newPassword); if (result.Succeeded) RaisePasswordChangedEvent(userId); return result; diff --git a/src/Umbraco.Web/Security/BackOfficeUserStore2.cs b/src/Umbraco.Web/Security/BackOfficeUserStore2.cs index d5ed266f4f..7cad8d6726 100644 --- a/src/Umbraco.Web/Security/BackOfficeUserStore2.cs +++ b/src/Umbraco.Web/Security/BackOfficeUserStore2.cs @@ -66,12 +66,11 @@ namespace Umbraco.Web.Security public Task GetUserIdAsync(BackOfficeIdentityUser user, CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); if (user == null) throw new ArgumentNullException(nameof(user)); - - return Task.FromResult(user.Id.ToString()); + + return Task.FromResult(UserIdToString(user.Id)); } public Task GetUserNameAsync(BackOfficeIdentityUser user, CancellationToken cancellationToken) @@ -156,14 +155,8 @@ namespace Umbraco.Web.Security cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); if (user == null) throw new ArgumentNullException(nameof(user)); - - var asInt = user.Id.TryConvertTo(); - if (asInt == false) - { - throw new InvalidOperationException("The user id must be an integer to work with the Umbraco"); - } - - var found = _userService.GetUserById(asInt.Result); + + var found = _userService.GetUserById(user.Id); if (found != null) { // we have to remember whether Logins property is dirty, since the UpdateMemberProperties will reset it. @@ -194,19 +187,13 @@ namespace Umbraco.Web.Security cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); if (user == null) throw new ArgumentNullException(nameof(user)); - - var asInt = user.Id.TryConvertTo(); - if (asInt == false) - { - throw new InvalidOperationException("The user id must be an integer to work with the Umbraco"); - } - - var found = _userService.GetUserById(asInt.Result); + + var found = _userService.GetUserById(user.Id); if (found != null) { _userService.Delete(found); } - _externalLoginService.DeleteUserLogins(asInt.Result); + _externalLoginService.DeleteUserLogins(user.Id); return Task.FromResult(IdentityResult.Success); } @@ -222,10 +209,7 @@ namespace Umbraco.Web.Security cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); - var asInt = userId.TryConvertTo(); - if (asInt == false) throw new InvalidOperationException("The user id must be an integer to work with the Umbraco"); - - var user = _userService.GetUserById(asInt.Result); + var user = _userService.GetUserById(UserIdToInt(userId)); if (user == null) return null; return await Task.FromResult(AssignLoginsCallback(_mapper.Map(user))); @@ -312,7 +296,7 @@ namespace Umbraco.Web.Security cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); if (user == null) throw new ArgumentNullException(nameof(user)); - if (email.IsNullOrWhiteSpace()) throw new ArgumentNullException("email"); + // if (email.IsNullOrWhiteSpace()) throw new ArgumentNullException("email"); user.Email = email; @@ -898,8 +882,7 @@ namespace Umbraco.Web.Security private void ThrowIfDisposed() { - if (_disposed) - throw new ObjectDisposedException(GetType().Name); + if (_disposed) throw new ObjectDisposedException(GetType().Name); } public Task ValidateSessionIdAsync(string userId, string sessionId) @@ -907,10 +890,26 @@ namespace Umbraco.Web.Security Guid guidSessionId; if (Guid.TryParse(sessionId, out guidSessionId)) { - // TODO: SB: Normalize string to int conversion - return Task.FromResult(_userService.ValidateLoginSession(int.Parse(sessionId), guidSessionId)); + return Task.FromResult(_userService.ValidateLoginSession(UserIdToInt(userId), guidSessionId)); } + return Task.FromResult(false); } + + private string UserIdToString(int userId) + { + var attempt = userId.TryConvertTo(); + if (attempt.Success) return attempt.Result; + + throw new InvalidOperationException("Unable to convert user ID to string", attempt.Exception); + } + + private int UserIdToInt(string userId) + { + var attempt = userId.TryConvertTo(); + if (attempt.Success) return attempt.Result; + + throw new InvalidOperationException("Unable to convert user ID to int", attempt.Exception); + } } }