From de390be2b8840631824f0b13bfeced75c1363b16 Mon Sep 17 00:00:00 2001 From: emmagarland Date: Sun, 14 Mar 2021 18:58:33 +0000 Subject: [PATCH 01/32] Added SignInManager for members and related unit tests (in progress) --- ...rManagerTests.cs => MemberManagerTests.cs} | 2 +- .../Security/MemberSignInManagerTests.cs | 110 +++++++++ src/Umbraco.Web.Common/Constants/Security.cs | 14 ++ .../Security/MemberManager.cs | 7 +- .../Security/MemberSignInManager.cs | 232 ++++++++++++++++++ 5 files changed, 358 insertions(+), 7 deletions(-) rename src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/{MemberIdentityUserManagerTests.cs => MemberManagerTests.cs} (99%) create mode 100644 src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs create mode 100644 src/Umbraco.Web.Common/Constants/Security.cs create mode 100644 src/Umbraco.Web.Common/Security/MemberSignInManager.cs diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberIdentityUserManagerTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberManagerTests.cs similarity index 99% rename from src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberIdentityUserManagerTests.cs rename to src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberManagerTests.cs index a319d33b34..4fe7ac5712 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberIdentityUserManagerTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberManagerTests.cs @@ -17,7 +17,7 @@ using Umbraco.Cms.Web.Common.Security; namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security { [TestFixture] - public class MemberIdentityUserManagerTests + public class MemberManagerTests { private Mock> _mockMemberStore; private Mock> _mockIdentityOptions; diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs new file mode 100644 index 0000000000..8dfc4a8a8a --- /dev/null +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs @@ -0,0 +1,110 @@ +using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Moq; +using NUnit.Framework; +using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Net; +using Umbraco.Cms.Core.Security; +using Umbraco.Cms.Web.Common.Security; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security +{ + [TestFixture] + public class MemberSignInManagerTests + { + private Mock> _mockMemberStore; + private Mock> _mockIdentityOptions; + private Mock> _mockPasswordHasher; + private Mock> _mockUserValidators; + private Mock>> _mockPasswordValidators; + private Mock _mockNormalizer; + private IdentityErrorDescriber _mockErrorDescriber; + private Mock _mockServiceProviders; + private Mock>> _mockLogger; + private Mock> _mockPasswordConfiguration; + + public MemberSignInManager CreateSut() + { + _mockMemberStore = new Mock>(); + _mockIdentityOptions = new Mock>(); + + var idOptions = new MemberIdentityOptions { Lockout = { AllowedForNewUsers = false } }; + _mockIdentityOptions.Setup(o => o.Value).Returns(idOptions); + _mockPasswordHasher = new Mock>(); + + var userValidators = new List>(); + _mockUserValidators = new Mock>(); + var validator = new Mock>(); + userValidators.Add(validator.Object); + + _mockPasswordValidators = new Mock>>(); + _mockNormalizer = new Mock(); + _mockErrorDescriber = new IdentityErrorDescriber(); + _mockServiceProviders = new Mock(); + _mockLogger = new Mock>>(); + _mockPasswordConfiguration = new Mock>(); + _mockPasswordConfiguration.Setup(x => x.Value).Returns(() => + new MemberPasswordConfigurationSettings() + { + + }); + + var pwdValidators = new List> + { + new PasswordValidator() + }; + + var userManager = new MemberManager( + new Mock().Object, + _mockMemberStore.Object, + _mockIdentityOptions.Object, + _mockPasswordHasher.Object, + userValidators, + pwdValidators, + new BackOfficeIdentityErrorDescriber(), + _mockServiceProviders.Object, + new Mock().Object, + new Mock>>().Object, + _mockPasswordConfiguration.Object); + + validator.Setup(v => v.ValidateAsync( + userManager, + It.IsAny())) + .Returns(Task.FromResult(IdentityResult.Success)).Verifiable(); + + return new MemberSignInManager( + userManager, + Mock.Of(), + Mock.Of>(), + Mock.Of>(), + Mock.Of>>(), + Mock.Of(), + Mock.Of>()); + } + + [Test] + public async Task WhenPasswordSignInAsyncIsCalled_AndEverythingIsSetup_ThenASignInResultShouldBeReturnedAsync() + { + //arrange + MemberSignInManager sut = CreateSut(); + var fakeUser = new MemberIdentityUser(777) + { + }; + string password = null; + bool lockoutOnfailure = false; + bool isPersistent = false; + + //act + SignInResult actual = await sut.PasswordSignInAsync(fakeUser, password, isPersistent, lockoutOnfailure); + + //assert + Assert.IsTrue(actual.Succeeded); + } + } +} diff --git a/src/Umbraco.Web.Common/Constants/Security.cs b/src/Umbraco.Web.Common/Constants/Security.cs new file mode 100644 index 0000000000..f2fcbc65f8 --- /dev/null +++ b/src/Umbraco.Web.Common/Constants/Security.cs @@ -0,0 +1,14 @@ +namespace Umbraco.Cms.Web.Common.Constants +{ + public static class Security + { + //TODO: implement 2Factor external + + public const string MemberAuthenticationType = "UmbracoMember"; + //public const string MemberExternalAuthenticationType = "UmbracoMemberExternalCookie"; + //public const string MemberExternalCookieName = "UMB_MEMBEREXTLOGIN"; + public const string MemberTokenAuthenticationType = "UmbracoMemberToken"; + //public const string MemberTwoFactorAuthenticationType = "UmbracoMemberTwoFactorCookie"; + //public const string MemberTwoFactorRememberMeAuthenticationType = "UmbracoMemberTwoFactorRememberMeCookie"; + } +} diff --git a/src/Umbraco.Web.Common/Security/MemberManager.cs b/src/Umbraco.Web.Common/Security/MemberManager.cs index ae91852f46..7762df9bb7 100644 --- a/src/Umbraco.Web.Common/Security/MemberManager.cs +++ b/src/Umbraco.Web.Common/Security/MemberManager.cs @@ -32,11 +32,7 @@ namespace Umbraco.Cms.Web.Common.Security IHttpContextAccessor httpContextAccessor, ILogger> logger, IOptions passwordConfiguration) - : base(ipResolver, store, optionsAccessor, passwordHasher, userValidators, passwordValidators, errors, services, logger, passwordConfiguration) - { - _httpContextAccessor = httpContextAccessor; - } - + : base(ipResolver, store, optionsAccessor, passwordHasher, userValidators, passwordValidators, errors, services, logger, passwordConfiguration) => _httpContextAccessor = httpContextAccessor; private string GetCurrentUserId(IPrincipal currentUser) { ClaimsIdentity umbIdentity = currentUser?.GetUmbracoIdentity(); @@ -52,7 +48,6 @@ namespace Umbraco.Cms.Web.Common.Security } //TODO: have removed all other member audit events - can revisit if we need member auditing on a user level in future - public void RaiseForgotPasswordRequestedEvent(IPrincipal currentUser, string userId) => throw new NotImplementedException(); public void RaiseForgotPasswordChangedSuccessEvent(IPrincipal currentUser, string userId) => throw new NotImplementedException(); diff --git a/src/Umbraco.Web.Common/Security/MemberSignInManager.cs b/src/Umbraco.Web.Common/Security/MemberSignInManager.cs new file mode 100644 index 0000000000..09c7379a28 --- /dev/null +++ b/src/Umbraco.Web.Common/Security/MemberSignInManager.cs @@ -0,0 +1,232 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Security.Claims; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Security; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Web.Common.Security +{ + //TODO: can any of this be combined/merged with BackOfficeSignInManager using T for the identity user? + //TODO: Need to implement events on member login/logout etc + public class MemberSignInManager : SignInManager + { + private const string ClaimType = "amr"; + private const string PasswordValue = "pwd"; + + public MemberSignInManager( + MemberManager memberManager, + IHttpContextAccessor contextAccessor, + IUserClaimsPrincipalFactory claimsFactory, + IOptions optionsAccessor, + ILogger> logger, + IAuthenticationSchemeProvider schemes, + IUserConfirmation confirmation) : + base(memberManager, contextAccessor, claimsFactory, optionsAccessor, logger, schemes, confirmation) + { + } + + /// + public override async Task PasswordSignInAsync(MemberIdentityUser user, string password, bool isPersistent, bool lockoutOnFailure) + { + // overridden to handle logging/events + SignInResult result = await base.PasswordSignInAsync(user, password, isPersistent, lockoutOnFailure); + return await HandleSignIn(user, user.UserName, result); + } + + /// + public override async Task PasswordSignInAsync(string userName, string password, bool isPersistent, bool lockoutOnFailure) + { + // overridden to handle logging/events + MemberIdentityUser user = await UserManager.FindByNameAsync(userName); + if (user == null) + { + return await HandleSignIn(null, userName, SignInResult.Failed); + } + + return await PasswordSignInAsync(user, password, isPersistent, lockoutOnFailure); + } + + /// + public override Task GetTwoFactorAuthenticationUserAsync() => throw new NotImplementedException("Two factor is not yet implemented for members"); + + /// + public override Task TwoFactorSignInAsync(string provider, string code, bool isPersistent, bool rememberClient) => throw new NotImplementedException("Two factor is not yet implemented for members"); + + /// + public override bool IsSignedIn(ClaimsPrincipal principal) + { + // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L126 + // replaced in order to use a custom auth type + // taken from BackOfficeSignInManager + + if (principal == null) + { + throw new ArgumentNullException(nameof(principal)); + } + return principal?.Identities != null && principal.Identities.Any(i => i.AuthenticationType == Constants.Security.MemberAuthenticationType); + } + + /// + public override async Task RefreshSignInAsync(MemberIdentityUser user) + { + // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L126 + // replaced in order to use a custom auth type + + AuthenticateResult auth = await Context.AuthenticateAsync(Constants.Security.MemberAuthenticationType); + IList claims = Array.Empty(); + + Claim authenticationMethod = auth?.Principal?.FindFirst(ClaimTypes.AuthenticationMethod); + Claim amr = auth?.Principal?.FindFirst(ClaimType); + + if (authenticationMethod != null || amr != null) + { + claims = new List(); + if (authenticationMethod != null) + { + claims.Add(authenticationMethod); + } + if (amr != null) + { + claims.Add(amr); + } + } + + await SignInWithClaimsAsync(user, auth?.Properties, claims); + } + + /// + public override async Task SignInWithClaimsAsync(MemberIdentityUser user, AuthenticationProperties authenticationProperties, IEnumerable additionalClaims) + { + // TODO: taken from BackOfficeSigninManager and more notes are there + // override to replace IdentityConstants.ApplicationScheme with Constants.Security.MemberAuthenticationType + // code taken from aspnetcore: https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs + // we also override to set the current HttpContext principal since this isn't done by default + + ClaimsPrincipal userPrincipal = await CreateUserPrincipalAsync(user); + foreach (Claim claim in additionalClaims) + { + userPrincipal.Identities.First().AddClaim(claim); + } + + // TODO: For future, this method gets called when performing 2FA logins + await Context.SignInAsync(Constants.Security.MemberAuthenticationType, + userPrincipal, + authenticationProperties ?? new AuthenticationProperties()); + } + + /// + public override async Task SignOutAsync() => + //TODO: does members need this custom signout type as per BackOfficeSignInManager? + // override to replace IdentityConstants.ApplicationScheme with Constants.Security.MemberAuthenticationType + // code taken from aspnetcore: https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs + await Context.SignOutAsync(Constants.Security.MemberAuthenticationType); + + /// + public override Task IsTwoFactorClientRememberedAsync(MemberIdentityUser user) => throw new NotImplementedException("Two factor is not yet implemented for members"); + + /// + public override Task RememberTwoFactorClientAsync(MemberIdentityUser user) => throw new NotImplementedException("Two factor is not yet implemented for members"); + + /// + public override Task ForgetTwoFactorClientAsync() => throw new NotImplementedException("Two factor is not yet implemented for members"); + + /// + public override Task TwoFactorRecoveryCodeSignInAsync(string recoveryCode) => throw new NotImplementedException("Two factor is not yet implemented for members"); + + /// + public override Task GetExternalLoginInfoAsync(string expectedXsrf = null) => throw new NotImplementedException("External login is not yet implemented for members"); + + /// + public override AuthenticationProperties ConfigureExternalAuthenticationProperties(string provider, string redirectUrl, string userId = null) => throw new NotImplementedException("External login is not yet implemented for members"); + + /// + public override Task> GetExternalAuthenticationSchemesAsync() => throw new NotImplementedException("External login is not yet implemented for members"); + + /// + /// TODO: Two factor is not yet implemented for members + protected override async Task SignInOrTwoFactorAsync(MemberIdentityUser user, bool isPersistent, string loginProvider = null, bool bypassTwoFactor = false) + { + // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs + // to replace custom auth types + + //TODO: There is currently no two factor so this needs changing once implemented + if (loginProvider != null) + { + await SignInAsync(user, isPersistent, loginProvider); + } + else + { + await SignInWithClaimsAsync(user, isPersistent, new Claim[] + { + new Claim(ClaimType, PasswordValue) + }); + } + return SignInResult.Success; + } + + /// + /// Called on any login attempt to update the AccessFailedCount and to raise events + /// + /// + /// + /// + /// + private async Task HandleSignIn(MemberIdentityUser user, string username, SignInResult result) + { + // TODO: More TODO notes in BackOfficeSignInManager + if (username.IsNullOrWhiteSpace()) + { + username = "UNKNOWN"; + } + + if (result.Succeeded) + { + //track the last login date + user.LastLoginDateUtc = DateTime.UtcNow; + if (user.AccessFailedCount > 0) + { + //we have successfully logged in, reset the AccessFailedCount + user.AccessFailedCount = 0; + } + await UserManager.UpdateAsync(user); + + Logger.LogInformation("User: {UserName} logged in from IP address {IpAddress}", username, Context.Connection.RemoteIpAddress); + if (user != null) + { + //TODO: what events do we want for members? + //_memberManager.RaiseLoginSuccessEvent(Context.User, user.Id); + } + } + else if (result.IsLockedOut) + { + //TODO: what events do we want for members? + //_memberManager.RaiseAccountLockedEvent(Context.User, user.Id); + Logger.LogInformation("Login attempt failed for username {UserName} from IP address {IpAddress}, the user is locked", username, Context.Connection.RemoteIpAddress); + } + else if (result.RequiresTwoFactor) + { + //TODO: what events do we want for members? + //_memberManager.RaiseLoginRequiresVerificationEvent(Context.User, user.Id); + Logger.LogInformation("Login attempt requires verification for username {UserName} from IP address {IpAddress}", username, Context.Connection.RemoteIpAddress); + } + else if (!result.Succeeded || result.IsNotAllowed) + { + Logger.LogInformation("Login attempt failed for username {UserName} from IP address {IpAddress}", username, Context.Connection.RemoteIpAddress); + } + else + { + throw new ArgumentOutOfRangeException(); + } + + return result; + } + } +} From a260bc37a8fc571247bfd81b2761437c761299ce Mon Sep 17 00:00:00 2001 From: emmagarland Date: Sun, 14 Mar 2021 21:59:30 +0000 Subject: [PATCH 02/32] Fixed unit tests to use correct store, added IPResolver --- .../Security/MemberManagerTests.cs | 50 +++++++++------ .../Security/MemberSignInManagerTests.cs | 64 ++++++++++++++----- .../Security/MemberSignInManager.cs | 18 +++--- 3 files changed, 90 insertions(+), 42 deletions(-) diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberManagerTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberManagerTests.cs index 4fe7ac5712..800b7923a4 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberManagerTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberManagerTests.cs @@ -10,8 +10,14 @@ using Microsoft.Extensions.Options; using Moq; using NUnit.Framework; using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Mapping; +using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Net; +using Umbraco.Cms.Core.Scoping; using Umbraco.Cms.Core.Security; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Tests.Common.Builders; +using Umbraco.Cms.Tests.Common.Builders.Extensions; using Umbraco.Cms.Web.Common.Security; namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security @@ -19,9 +25,10 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security [TestFixture] public class MemberManagerTests { - private Mock> _mockMemberStore; + private MemberUserStore _fakeMemberStore; private Mock> _mockIdentityOptions; private Mock> _mockPasswordHasher; + private Mock _mockMemberService; private Mock> _mockUserValidators; private Mock>> _mockPasswordValidators; private Mock _mockNormalizer; @@ -32,9 +39,14 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security public MemberManager CreateSut() { - _mockMemberStore = new Mock>(); - _mockIdentityOptions = new Mock>(); + _mockMemberService = new Mock(); + _fakeMemberStore = new MemberUserStore( + _mockMemberService.Object, + new UmbracoMapper(new MapDefinitionCollection(new List())), + new Mock().Object, + new IdentityErrorDescriber()); + _mockIdentityOptions = new Mock>(); var idOptions = new MemberIdentityOptions { Lockout = { AllowedForNewUsers = false } }; _mockIdentityOptions.Setup(o => o.Value).Returns(idOptions); _mockPasswordHasher = new Mock>(); @@ -63,7 +75,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security var userManager = new MemberManager( new Mock().Object, - _mockMemberStore.Object, + _fakeMemberStore, _mockIdentityOptions.Object, _mockPasswordHasher.Object, userValidators, @@ -101,10 +113,6 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security } }; - _mockMemberStore.Setup(x => - x.CreateAsync(fakeUser, fakeCancellationToken)) - .ReturnsAsync(IdentityResult.Failed(identityErrors)); - //act IdentityResult identityResult = await sut.CreateAsync(fakeUser); @@ -130,10 +138,6 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security } }; - _mockMemberStore.Setup(x => - x.CreateAsync(null, fakeCancellationToken)) - .ReturnsAsync(IdentityResult.Failed(identityErrors)); - //act var identityResult = new Func>(() => sut.CreateAsync(null)); @@ -142,20 +146,30 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security Assert.That(identityResult, Throws.ArgumentNullException); } - [Test] public async Task GivenICreateANewUser_AndTheUserIsPopulatedCorrectly_ThenIShouldGetASuccessResultAsync() { //arrange MemberManager sut = CreateSut(); - MemberIdentityUser fakeUser = new MemberIdentityUser() + var fakeUser = new MemberIdentityUser(777) { + UserName = "testUser", + Email = "test@test.com", + Name = "Test", + MemberTypeAlias = "Anything", PasswordConfig = "testConfig" }; - CancellationToken fakeCancellationToken = new CancellationToken() { }; - _mockMemberStore.Setup(x => - x.CreateAsync(fakeUser, fakeCancellationToken)) - .ReturnsAsync(IdentityResult.Success); + + var builder = new MemberTypeBuilder(); + MemberType memberType = builder.BuildSimpleMemberType(); + + IMember fakeMember = new Member(memberType) + { + Id = 777 + }; + + _mockMemberService.Setup(x => x.CreateMember(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())).Returns(fakeMember); + _mockMemberService.Setup(x => x.Save(fakeMember, false)); //act IdentityResult identityResult = await sut.CreateAsync(fakeUser); diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs index 8dfc4a8a8a..26742eb1d8 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs @@ -9,8 +9,11 @@ using Microsoft.Extensions.Options; using Moq; using NUnit.Framework; using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Mapping; using Umbraco.Cms.Core.Net; +using Umbraco.Cms.Core.Scoping; using Umbraco.Cms.Core.Security; +using Umbraco.Cms.Core.Services; using Umbraco.Cms.Web.Common.Security; namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security @@ -18,7 +21,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security [TestFixture] public class MemberSignInManagerTests { - private Mock> _mockMemberStore; + private MemberUserStore _fakeMemberStore; private Mock> _mockIdentityOptions; private Mock> _mockPasswordHasher; private Mock> _mockUserValidators; @@ -26,12 +29,20 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security private Mock _mockNormalizer; private IdentityErrorDescriber _mockErrorDescriber; private Mock _mockServiceProviders; - private Mock>> _mockLogger; + private Mock> _mockLogger; private Mock> _mockPasswordConfiguration; + private MemberManager _userManager; + private Mock _mockIpResolver = new Mock(); public MemberSignInManager CreateSut() { - _mockMemberStore = new Mock>(); + var _mockMemberService = new Mock(); + _fakeMemberStore = new MemberUserStore( + _mockMemberService.Object, + new UmbracoMapper(new MapDefinitionCollection(new List())), + new Mock().Object, + new IdentityErrorDescriber()); + _mockIdentityOptions = new Mock>(); var idOptions = new MemberIdentityOptions { Lockout = { AllowedForNewUsers = false } }; @@ -47,22 +58,19 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security _mockNormalizer = new Mock(); _mockErrorDescriber = new IdentityErrorDescriber(); _mockServiceProviders = new Mock(); - _mockLogger = new Mock>>(); + _mockLogger = new Mock>(); _mockPasswordConfiguration = new Mock>(); _mockPasswordConfiguration.Setup(x => x.Value).Returns(() => - new MemberPasswordConfigurationSettings() - { - - }); + new MemberPasswordConfigurationSettings() { }); var pwdValidators = new List> { new PasswordValidator() }; - var userManager = new MemberManager( - new Mock().Object, - _mockMemberStore.Object, + _userManager = new MemberManager( + _mockIpResolver.Object, + _fakeMemberStore, _mockIdentityOptions.Object, _mockPasswordHasher.Object, userValidators, @@ -74,12 +82,13 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security _mockPasswordConfiguration.Object); validator.Setup(v => v.ValidateAsync( - userManager, + _userManager, It.IsAny())) .Returns(Task.FromResult(IdentityResult.Success)).Verifiable(); return new MemberSignInManager( - userManager, + _userManager, + Mock.Of(), Mock.Of(), Mock.Of>(), Mock.Of>(), @@ -89,16 +98,17 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security } [Test] - public async Task WhenPasswordSignInAsyncIsCalled_AndEverythingIsSetup_ThenASignInResultShouldBeReturnedAsync() + public async Task WhenPasswordSignInAsyncIsCalled_AndEverythingIsSetup_ThenASignInResultSucceededShouldBeReturnedAsync() { //arrange MemberSignInManager sut = CreateSut(); var fakeUser = new MemberIdentityUser(777) { + UserName = "TestUser", }; - string password = null; + string password = "testPassword"; bool lockoutOnfailure = false; - bool isPersistent = false; + bool isPersistent = true; //act SignInResult actual = await sut.PasswordSignInAsync(fakeUser, password, isPersistent, lockoutOnfailure); @@ -106,5 +116,27 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security //assert Assert.IsTrue(actual.Succeeded); } + + [Test] + public async Task WhenPasswordSignInAsyncIsCalled_AndTheResultFails_ThenASignInFailedResultShouldBeReturnedAsync() + { + //arrange + MemberSignInManager sut = CreateSut(); + var fakeUser = new MemberIdentityUser(777) + { + UserName = "TestUser", + }; + string password = "testPassword"; + bool lockoutOnfailure = false; + bool isPersistent = true; + _mockIpResolver.Setup(x => x.GetCurrentRequestIpAddress()).Returns("127.0.0.1"); + + //act + SignInResult actual = await sut.PasswordSignInAsync(fakeUser, password, isPersistent, lockoutOnfailure); + + //assert + Assert.IsFalse(actual.Succeeded); + //_mockLogger.Verify(x => x.LogInformation("Login attempt failed for username TestUser from IP address 127.0.0.1", null)); + } } } diff --git a/src/Umbraco.Web.Common/Security/MemberSignInManager.cs b/src/Umbraco.Web.Common/Security/MemberSignInManager.cs index 09c7379a28..38b23d8e28 100644 --- a/src/Umbraco.Web.Common/Security/MemberSignInManager.cs +++ b/src/Umbraco.Web.Common/Security/MemberSignInManager.cs @@ -8,7 +8,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; -using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Net; using Umbraco.Cms.Core.Security; using Umbraco.Extensions; @@ -20,18 +20,19 @@ namespace Umbraco.Cms.Web.Common.Security { private const string ClaimType = "amr"; private const string PasswordValue = "pwd"; + private readonly IIpResolver _ipResolver; public MemberSignInManager( MemberManager memberManager, + IIpResolver ipResolver, IHttpContextAccessor contextAccessor, IUserClaimsPrincipalFactory claimsFactory, IOptions optionsAccessor, ILogger> logger, IAuthenticationSchemeProvider schemes, IUserConfirmation confirmation) : - base(memberManager, contextAccessor, claimsFactory, optionsAccessor, logger, schemes, confirmation) - { - } + base(memberManager, contextAccessor, claimsFactory, optionsAccessor, logger, schemes, confirmation) => + _ipResolver = ipResolver ?? throw new ArgumentNullException(nameof(ipResolver)); /// public override async Task PasswordSignInAsync(MemberIdentityUser user, string password, bool isPersistent, bool lockoutOnFailure) @@ -184,6 +185,7 @@ namespace Umbraco.Cms.Web.Common.Security // TODO: More TODO notes in BackOfficeSignInManager if (username.IsNullOrWhiteSpace()) { + //TODO: this might have unwanted effects if the member is called this username = "UNKNOWN"; } @@ -198,7 +200,7 @@ namespace Umbraco.Cms.Web.Common.Security } await UserManager.UpdateAsync(user); - Logger.LogInformation("User: {UserName} logged in from IP address {IpAddress}", username, Context.Connection.RemoteIpAddress); + Logger.LogInformation("User: {UserName} logged in from IP address {IpAddress}", username, _ipResolver.GetCurrentRequestIpAddress()); if (user != null) { //TODO: what events do we want for members? @@ -209,17 +211,17 @@ namespace Umbraco.Cms.Web.Common.Security { //TODO: what events do we want for members? //_memberManager.RaiseAccountLockedEvent(Context.User, user.Id); - Logger.LogInformation("Login attempt failed for username {UserName} from IP address {IpAddress}, the user is locked", username, Context.Connection.RemoteIpAddress); + Logger.LogInformation("Login attempt failed for username {UserName} from IP address {IpAddress}, the user is locked", username, _ipResolver.GetCurrentRequestIpAddress()); } else if (result.RequiresTwoFactor) { //TODO: what events do we want for members? //_memberManager.RaiseLoginRequiresVerificationEvent(Context.User, user.Id); - Logger.LogInformation("Login attempt requires verification for username {UserName} from IP address {IpAddress}", username, Context.Connection.RemoteIpAddress); + Logger.LogInformation("Login attempt requires verification for username {UserName} from IP address {IpAddress}", username, _ipResolver.GetCurrentRequestIpAddress()); } else if (!result.Succeeded || result.IsNotAllowed) { - Logger.LogInformation("Login attempt failed for username {UserName} from IP address {IpAddress}", username, Context.Connection.RemoteIpAddress); + Logger.LogInformation("Login attempt failed for username {UserName} from IP address {IpAddress}", username, _ipResolver.GetCurrentRequestIpAddress()); } else { From 76e0f8474884573abfb709276fef4c62b9d96e0a Mon Sep 17 00:00:00 2001 From: emmagarland Date: Sun, 14 Mar 2021 22:19:27 +0000 Subject: [PATCH 03/32] Amendments to tests for mocking --- .../Security/MemberSignInManagerTests.cs | 40 ++++++++++--------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs index 26742eb1d8..595dfb6ccd 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs @@ -31,8 +31,8 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security private Mock _mockServiceProviders; private Mock> _mockLogger; private Mock> _mockPasswordConfiguration; - private MemberManager _userManager; - private Mock _mockIpResolver = new Mock(); + private Mock _memberManager; + private readonly Mock _mockIpResolver = new Mock(); public MemberSignInManager CreateSut() { @@ -68,26 +68,27 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security new PasswordValidator() }; - _userManager = new MemberManager( - _mockIpResolver.Object, - _fakeMemberStore, - _mockIdentityOptions.Object, - _mockPasswordHasher.Object, - userValidators, - pwdValidators, - new BackOfficeIdentityErrorDescriber(), - _mockServiceProviders.Object, - new Mock().Object, - new Mock>>().Object, - _mockPasswordConfiguration.Object); + //_memberManager = new MemberManager( + // _mockIpResolver.Object, + // _fakeMemberStore, + // _mockIdentityOptions.Object, + // _mockPasswordHasher.Object, + // userValidators, + // pwdValidators, + // new BackOfficeIdentityErrorDescriber(), + // _mockServiceProviders.Object, + // new Mock().Object, + // new Mock>>().Object, + // _mockPasswordConfiguration.Object); - validator.Setup(v => v.ValidateAsync( - _userManager, - It.IsAny())) - .Returns(Task.FromResult(IdentityResult.Success)).Verifiable(); + //validator.Setup(v => v.ValidateAsync( + // _memberManager, + // It.IsAny())) + // .Returns(Task.FromResult(IdentityResult.Success)).Verifiable(); + _memberManager = new Mock(); return new MemberSignInManager( - _userManager, + _memberManager.As().Object, Mock.Of(), Mock.Of(), Mock.Of>(), @@ -109,6 +110,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security string password = "testPassword"; bool lockoutOnfailure = false; bool isPersistent = true; + _memberManager.Setup(x => x.FindByNameAsync(It.IsAny())).ReturnsAsync(fakeUser); //act SignInResult actual = await sut.PasswordSignInAsync(fakeUser, password, isPersistent, lockoutOnfailure); From ab16a408a8b852287952f8d655243b665988f1b8 Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska Date: Wed, 17 Mar 2021 14:04:48 +0100 Subject: [PATCH 04/32] Adds a config for configuring the access rules on the content dashboard - by default it granted for all user groups --- src/Umbraco.Web.UI/Umbraco.Web.UI.csproj | 1 + .../config/content.dashboard.access.config.js | 22 ++++++++++++++++ .../Dashboards/ContentDashboard.cs | 26 ++++++++++++++----- 3 files changed, 43 insertions(+), 6 deletions(-) create mode 100644 src/Umbraco.Web.UI/config/content.dashboard.access.config.js diff --git a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj index eb6649a7c4..7fb27deb7e 100644 --- a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj +++ b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj @@ -148,6 +148,7 @@ True Settings.settings + diff --git a/src/Umbraco.Web.UI/config/content.dashboard.access.config.js b/src/Umbraco.Web.UI/config/content.dashboard.access.config.js new file mode 100644 index 0000000000..93c727d85a --- /dev/null +++ b/src/Umbraco.Web.UI/config/content.dashboard.access.config.js @@ -0,0 +1,22 @@ +[ + { + "Type": "grant", + "Value": "admin" + }, + { + "Type": "grant", + "Value": "editor" + }, + { + "Type": "grant", + "Value": "sensitiveData" + }, + { + "Type": "grant", + "Value": "translator" + }, + { + "Type": "grant", + "Value": "writer" + } +] diff --git a/src/Umbraco.Web/Dashboards/ContentDashboard.cs b/src/Umbraco.Web/Dashboards/ContentDashboard.cs index 0cd96f738c..3dbc0cb693 100644 --- a/src/Umbraco.Web/Dashboards/ContentDashboard.cs +++ b/src/Umbraco.Web/Dashboards/ContentDashboard.cs @@ -1,6 +1,9 @@ -using Umbraco.Core; +using System.IO; +using Newtonsoft.Json; +using Umbraco.Core; using Umbraco.Core.Composing; using Umbraco.Core.Dashboards; +using Umbraco.Core.IO; namespace Umbraco.Web.Dashboards { @@ -9,7 +12,7 @@ namespace Umbraco.Web.Dashboards { public string Alias => "contentIntro"; - public string[] Sections => new [] { "content" }; + public string[] Sections => new[] { "content" }; public string View => "views/dashboard/default/startupdashboardintro.html"; @@ -17,11 +20,22 @@ namespace Umbraco.Web.Dashboards { get { - var rules = new IAccessRule[] + IAccessRule[] rules; + var dashboardConfig = Path.Combine(IOHelper.MapPath(SystemDirectories.Config), "content.dashboard.access.config.js"); + + if (File.Exists(dashboardConfig)) { - new AccessRule {Type = AccessRuleType.Deny, Value = Constants.Security.TranslatorGroupAlias}, - new AccessRule {Type = AccessRuleType.Grant, Value = Constants.Security.AdminGroupAlias} - }; + var rawJson = File.ReadAllText(dashboardConfig); + rules = JsonConvert.DeserializeObject(rawJson); + } + else + { + rules = new IAccessRule[] + { + new AccessRule {Type = AccessRuleType.Deny, Value = Constants.Security.TranslatorGroupAlias}, + new AccessRule {Type = AccessRuleType.Grant, Value = Constants.Security.AdminGroupAlias} + }; + } return rules; } } From 3393ac8d0369c097e306d419201fb299c939a560 Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska Date: Wed, 17 Mar 2021 14:05:47 +0100 Subject: [PATCH 05/32] Adds additional params indicating whether user is admin --- src/Umbraco.Web/Editors/DashboardController.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Web/Editors/DashboardController.cs b/src/Umbraco.Web/Editors/DashboardController.cs index eef0b5df93..c7ce4e1220 100644 --- a/src/Umbraco.Web/Editors/DashboardController.cs +++ b/src/Umbraco.Web/Editors/DashboardController.cs @@ -17,6 +17,7 @@ using Umbraco.Core.Logging; using Umbraco.Core.Persistence; using Umbraco.Core.Services; using Umbraco.Core.Dashboards; +using Umbraco.Core.Models; using Umbraco.Web.Services; namespace Umbraco.Web.Editors @@ -52,8 +53,9 @@ namespace Umbraco.Web.Editors var allowedSections = string.Join(",", user.AllowedSections); var language = user.Language; var version = UmbracoVersion.SemanticVersion.ToSemanticString(); + var isAdmin = user.IsAdmin(); - var url = string.Format(baseUrl + "{0}?section={0}&allowed={1}&lang={2}&version={3}", section, allowedSections, language, version); + var url = string.Format(baseUrl + "{0}?section={0}&allowed={1}&lang={2}&version={3}&admin={4}", section, allowedSections, language, version, isAdmin); var key = "umbraco-dynamic-dashboard-" + language + allowedSections.Replace(",", "-") + section; var content = AppCaches.RuntimeCache.GetCacheItem(key); From e7a1db5604d4dd750a7093cfbdfa0b223ad026f9 Mon Sep 17 00:00:00 2001 From: Nathan Woulfe Date: Wed, 17 Mar 2021 21:54:38 +1000 Subject: [PATCH 06/32] Add images in grid - fixes 9982 (#9987) Co-authored-by: Sebastiaan Janssen (cherry picked from commit e2019777fbfc1f9221d040cb9f0b82c57f8552b9) --- .../grid/editors/media.controller.js | 40 ++++++++----------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/editors/media.controller.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/editors/media.controller.js index 716ca405c1..94ea4b8604 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/editors/media.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/editors/media.controller.js @@ -1,9 +1,9 @@ angular.module("umbraco") .controller("Umbraco.PropertyEditors.Grid.MediaController", - function ($scope, userService, editorService, localizationService) { - - $scope.thumbnailUrl = getThumbnailUrl(); - + function ($scope, userService, editorService, localizationService) { + + $scope.thumbnailUrl = getThumbnailUrl(); + if (!$scope.model.config.startNodeId) { if ($scope.model.config.ignoreUserStartNodes === true) { $scope.model.config.startNodeId = -1; @@ -29,16 +29,16 @@ angular.module("umbraco") onlyImages: true, dataTypeKey: $scope.model.dataTypeKey, submit: model => { - updateControlValue(model.selection[0]); + updateControlValue(model.selection[0]); editorService.close(); }, - close: () => editorService.close() + close: () => editorService.close() }; editorService.mediaPicker(mediaPicker); }; - $scope.editImage = function() { + $scope.editImage = function() { const mediaCropDetailsConfig = { size: 'small', @@ -47,17 +47,17 @@ angular.module("umbraco") updateControlValue(model.target); editorService.close(); }, - close: () => editorService.close() + close: () => editorService.close() }; localizationService.localize('defaultdialogs_editSelectedMedia').then(value => { mediaCropDetailsConfig.title = value; editorService.mediaCropDetails(mediaCropDetailsConfig); - }); + }); } - + /** - * + * */ function getThumbnailUrl() { @@ -94,19 +94,15 @@ angular.module("umbraco") return url; } - + return null; } /** - * - * @param {object} selectedImage + * + * @param {object} selectedImage */ function updateControlValue(selectedImage) { - - const doGetThumbnail = $scope.control.value.focalPoint !== selectedImage.focalPoint - || $scope.control.value.image !== selectedImage.image; - // we could apply selectedImage directly to $scope.control.value, // but this allows excluding fields in future if needed $scope.control.value = { @@ -118,10 +114,6 @@ angular.module("umbraco") caption: selectedImage.caption, altText: selectedImage.altText }; - - - if (doGetThumbnail) { - $scope.thumbnailUrl = getThumbnailUrl(); - } - } + $scope.thumbnailUrl = getThumbnailUrl(); + } }); From 548435dcc3f1f23df7f83dcc77fdfc87963ea355 Mon Sep 17 00:00:00 2001 From: Sebastiaan Janssen Date: Wed, 17 Mar 2021 17:40:47 +0100 Subject: [PATCH 07/32] Bump version to 8.12.2 --- src/SolutionInfo.cs | 4 ++-- src/Umbraco.Web.UI/Umbraco.Web.UI.csproj | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/SolutionInfo.cs b/src/SolutionInfo.cs index e92eefdf52..4162f47da6 100644 --- a/src/SolutionInfo.cs +++ b/src/SolutionInfo.cs @@ -18,5 +18,5 @@ using System.Resources; [assembly: AssemblyVersion("8.0.0")] // these are FYI and changed automatically -[assembly: AssemblyFileVersion("8.12.1")] -[assembly: AssemblyInformationalVersion("8.12.1")] +[assembly: AssemblyFileVersion("8.12.2")] +[assembly: AssemblyInformationalVersion("8.12.2")] diff --git a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj index eb6649a7c4..2ad23e1b73 100644 --- a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj +++ b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj @@ -347,9 +347,9 @@ False True - 8121 + 8122 / - http://localhost:8121 + http://localhost:8122 False False From e7e5e18b19844efacc8c456623e2d97051383a25 Mon Sep 17 00:00:00 2001 From: Emma Garland Date: Wed, 17 Mar 2021 17:44:49 +0000 Subject: [PATCH 08/32] Fixed merge issues --- src/Umbraco.Web.Common/Security/MemberManager.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Web.Common/Security/MemberManager.cs b/src/Umbraco.Web.Common/Security/MemberManager.cs index d491a010e4..195312a41e 100644 --- a/src/Umbraco.Web.Common/Security/MemberManager.cs +++ b/src/Umbraco.Web.Common/Security/MemberManager.cs @@ -12,7 +12,6 @@ namespace Umbraco.Cms.Web.Common.Security { public class MemberManager : UmbracoUserManager, IMemberManager { - public MemberManager( IIpResolver ipResolver, IUserStore store, @@ -24,7 +23,9 @@ namespace Umbraco.Cms.Web.Common.Security IServiceProvider services, ILogger> logger, IOptions passwordConfiguration) - : base(ipResolver, store, optionsAccessor, passwordHasher, userValidators, passwordValidators, errors, services, logger, passwordConfiguration) => _httpContextAccessor = httpContextAccessor; + : base(ipResolver, store, optionsAccessor, passwordHasher, userValidators, passwordValidators, errors, + services, logger, passwordConfiguration) + { } } } From 72f83e50d8fb99fbeee30bb43562c97b23e20bfe Mon Sep 17 00:00:00 2001 From: Emma Garland Date: Wed, 17 Mar 2021 18:04:54 +0000 Subject: [PATCH 09/32] Changed param for UserManager --- .../Security/MemberSignInManagerTests.cs | 93 ++++--------------- .../Security/MemberSignInManager.cs | 2 +- 2 files changed, 19 insertions(+), 76 deletions(-) diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs index 595dfb6ccd..a756c60380 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs @@ -1,5 +1,3 @@ -using System; -using System.Collections.Generic; using System.Threading.Tasks; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http; @@ -8,12 +6,8 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Moq; using NUnit.Framework; -using Umbraco.Cms.Core.Configuration.Models; -using Umbraco.Cms.Core.Mapping; using Umbraco.Cms.Core.Net; -using Umbraco.Cms.Core.Scoping; using Umbraco.Cms.Core.Security; -using Umbraco.Cms.Core.Services; using Umbraco.Cms.Web.Common.Security; namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security @@ -21,74 +15,15 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security [TestFixture] public class MemberSignInManagerTests { - private MemberUserStore _fakeMemberStore; - private Mock> _mockIdentityOptions; - private Mock> _mockPasswordHasher; - private Mock> _mockUserValidators; - private Mock>> _mockPasswordValidators; - private Mock _mockNormalizer; - private IdentityErrorDescriber _mockErrorDescriber; - private Mock _mockServiceProviders; private Mock> _mockLogger; - private Mock> _mockPasswordConfiguration; - private Mock _memberManager; + private readonly Mock> _memberManager = MockUserManager(); private readonly Mock _mockIpResolver = new Mock(); public MemberSignInManager CreateSut() { - var _mockMemberService = new Mock(); - _fakeMemberStore = new MemberUserStore( - _mockMemberService.Object, - new UmbracoMapper(new MapDefinitionCollection(new List())), - new Mock().Object, - new IdentityErrorDescriber()); - - _mockIdentityOptions = new Mock>(); - - var idOptions = new MemberIdentityOptions { Lockout = { AllowedForNewUsers = false } }; - _mockIdentityOptions.Setup(o => o.Value).Returns(idOptions); - _mockPasswordHasher = new Mock>(); - - var userValidators = new List>(); - _mockUserValidators = new Mock>(); - var validator = new Mock>(); - userValidators.Add(validator.Object); - - _mockPasswordValidators = new Mock>>(); - _mockNormalizer = new Mock(); - _mockErrorDescriber = new IdentityErrorDescriber(); - _mockServiceProviders = new Mock(); _mockLogger = new Mock>(); - _mockPasswordConfiguration = new Mock>(); - _mockPasswordConfiguration.Setup(x => x.Value).Returns(() => - new MemberPasswordConfigurationSettings() { }); - - var pwdValidators = new List> - { - new PasswordValidator() - }; - - //_memberManager = new MemberManager( - // _mockIpResolver.Object, - // _fakeMemberStore, - // _mockIdentityOptions.Object, - // _mockPasswordHasher.Object, - // userValidators, - // pwdValidators, - // new BackOfficeIdentityErrorDescriber(), - // _mockServiceProviders.Object, - // new Mock().Object, - // new Mock>>().Object, - // _mockPasswordConfiguration.Object); - - //validator.Setup(v => v.ValidateAsync( - // _memberManager, - // It.IsAny())) - // .Returns(Task.FromResult(IdentityResult.Success)).Verifiable(); - - _memberManager = new Mock(); return new MemberSignInManager( - _memberManager.As().Object, + _memberManager.Object, Mock.Of(), Mock.Of(), Mock.Of>(), @@ -97,23 +32,31 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security Mock.Of(), Mock.Of>()); } + private static Mock> MockUserManager() where TUser : class + { + var store = new Mock>(); + var mgr = new Mock>(store.Object, null, null, null, null, null, null, null, null); + return mgr; + } [Test] public async Task WhenPasswordSignInAsyncIsCalled_AndEverythingIsSetup_ThenASignInResultSucceededShouldBeReturnedAsync() { //arrange + var userId = "bo8w3d32q9b98"; + _memberManager.Setup(x => x.GetUserIdAsync(It.IsAny())).ReturnsAsync(userId); MemberSignInManager sut = CreateSut(); var fakeUser = new MemberIdentityUser(777) { UserName = "TestUser", }; - string password = "testPassword"; - bool lockoutOnfailure = false; - bool isPersistent = true; + var password = "testPassword"; + var lockoutOnFailure = false; + var isPersistent = true; _memberManager.Setup(x => x.FindByNameAsync(It.IsAny())).ReturnsAsync(fakeUser); //act - SignInResult actual = await sut.PasswordSignInAsync(fakeUser, password, isPersistent, lockoutOnfailure); + SignInResult actual = await sut.PasswordSignInAsync(fakeUser, password, isPersistent, lockoutOnFailure); //assert Assert.IsTrue(actual.Succeeded); @@ -128,13 +71,13 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security { UserName = "TestUser", }; - string password = "testPassword"; - bool lockoutOnfailure = false; - bool isPersistent = true; + var password = "testPassword"; + var lockoutOnFailure = false; + var isPersistent = true; _mockIpResolver.Setup(x => x.GetCurrentRequestIpAddress()).Returns("127.0.0.1"); //act - SignInResult actual = await sut.PasswordSignInAsync(fakeUser, password, isPersistent, lockoutOnfailure); + SignInResult actual = await sut.PasswordSignInAsync(fakeUser, password, isPersistent, lockoutOnFailure); //assert Assert.IsFalse(actual.Succeeded); diff --git a/src/Umbraco.Web.Common/Security/MemberSignInManager.cs b/src/Umbraco.Web.Common/Security/MemberSignInManager.cs index 38b23d8e28..d3adcc4833 100644 --- a/src/Umbraco.Web.Common/Security/MemberSignInManager.cs +++ b/src/Umbraco.Web.Common/Security/MemberSignInManager.cs @@ -23,7 +23,7 @@ namespace Umbraco.Cms.Web.Common.Security private readonly IIpResolver _ipResolver; public MemberSignInManager( - MemberManager memberManager, + UserManager memberManager, IIpResolver ipResolver, IHttpContextAccessor contextAccessor, IUserClaimsPrincipalFactory claimsFactory, From 88820d082b7657109a9aa33b2fc0d4db03597394 Mon Sep 17 00:00:00 2001 From: Emma Garland Date: Wed, 17 Mar 2021 18:28:56 +0000 Subject: [PATCH 10/32] Setup for mocking --- .../Umbraco.Web.Common/Security/MemberSignInManagerTests.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs index a756c60380..49dc5d9d55 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs @@ -54,6 +54,9 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security var lockoutOnFailure = false; var isPersistent = true; _memberManager.Setup(x => x.FindByNameAsync(It.IsAny())).ReturnsAsync(fakeUser); + _memberManager.Setup(x => x.CheckPasswordAsync(fakeUser, password)).ReturnsAsync(true); + _memberManager.Setup(x => x.IsEmailConfirmedAsync(fakeUser)).ReturnsAsync(true); + _memberManager.Setup(x => x.IsLockedOutAsync(fakeUser)).ReturnsAsync(false); //act SignInResult actual = await sut.PasswordSignInAsync(fakeUser, password, isPersistent, lockoutOnFailure); From e17bf386a48df8fffa9e57f5360675e661f8eaac Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 18 Mar 2021 09:28:58 +0100 Subject: [PATCH 11/32] Merge pull request #9994 from umbraco/v8/bugfix/9993 Fixes #9993 - Cannot save empty image in Grid (cherry picked from commit 0ecc933921f2dea9a2a16d6f395b44a039663ec6) --- src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs index 862837381a..f9eacd9e73 100644 --- a/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs @@ -202,8 +202,8 @@ namespace Umbraco.Web.PropertyEditors _richTextPropertyValueEditor.GetReferences(x.Value))) yield return umbracoEntityReference; - foreach (var umbracoEntityReference in mediaValues.SelectMany(x => - _mediaPickerPropertyValueEditor.GetReferences(x.Value["udi"]))) + foreach (var umbracoEntityReference in mediaValues.Where(x => x.Value.HasValues) + .SelectMany(x => _mediaPickerPropertyValueEditor.GetReferences(x.Value["udi"]))) yield return umbracoEntityReference; } } From 73439d4cbab197fa9292761c04c50b08db8663ae Mon Sep 17 00:00:00 2001 From: Sebastiaan Janssen Date: Thu, 18 Mar 2021 13:01:46 +0100 Subject: [PATCH 12/32] Fixes #9983 - Getting kicked, if document type has a Umbraco.UserPicker property (#10002) * Fixes #9983 Temporary fix for this issue. using the entityservice like before. * Needed to remove the call to usersResource here as well for displaying the picked items * Don't need usersResource for now (cherry picked from commit 45de0a101eaa2b8f16e21a765f32928c7cb968be) --- .../userpicker/userpicker.controller.js | 16 +++------- .../userpicker/userpicker.controller.js | 29 +++++++++++-------- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/userpicker/userpicker.controller.js b/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/userpicker/userpicker.controller.js index a7021b2867..33d526c3cf 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/userpicker/userpicker.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/userpicker/userpicker.controller.js @@ -1,8 +1,8 @@ (function () { "use strict"; - function UserPickerController($scope, usersResource, localizationService, eventsService) { - + function UserPickerController($scope, entityResource, localizationService, eventsService) { + var vm = this; vm.users = []; @@ -102,17 +102,9 @@ vm.loading = true; // Get users - usersResource.getPagedResults(vm.usersOptions).then(function (users) { - - vm.users = users.items; - - vm.usersOptions.pageNumber = users.pageNumber; - vm.usersOptions.pageSize = users.pageSize; - vm.usersOptions.totalItems = users.totalItems; - vm.usersOptions.totalPages = users.totalPages; - + entityResource.getAll("User").then(function (data) { + vm.users = data; preSelect($scope.model.selection, vm.users); - vm.loading = false; }); } diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/userpicker/userpicker.controller.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/userpicker/userpicker.controller.js index f2055fea3a..217a9c8421 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/userpicker/userpicker.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/userpicker/userpicker.controller.js @@ -1,4 +1,4 @@ -function userPickerController($scope, usersResource , iconHelper, editorService, overlayService){ +function userPickerController($scope, iconHelper, editorService, overlayService, entityResource) { function trim(str, chr) { var rgxtrim = (!chr) ? new RegExp('^\\s+|\\s+$', 'g') : new RegExp('^' + chr + '+|' + chr + '+$', 'g'); @@ -92,17 +92,22 @@ function userPickerController($scope, usersResource , iconHelper, editorService, unsubscribe(); }); - //load user data - var modelIds = $scope.model.value ? $scope.model.value.split(',') : []; - - // entityResource.getByIds doesn't support "User" and we would like to show avatars in umb-user-preview as well. - usersResource.getUsers(modelIds).then(function (data) { - _.each(data, function (item, i) { - // set default icon if it's missing - item.icon = item.icon ? iconHelper.convertFromLegacyIcon(item.icon) : "icon-user"; - $scope.renderModel.push({ name: item.name, id: item.id, udi: item.udi, icon: item.icon, avatars: item.avatars }); - }); - }); + //load user data - split to an array of ints (map) + const modelIds = $scope.model.value ? $scope.model.value.split(',').map(x => +x) : []; + if(modelIds.length !== 0) { + entityResource.getAll("User").then(function (users) { + const filteredUsers = users.filter(user => modelIds.indexOf(user.id) !== -1); + filteredUsers.forEach(item => { + $scope.renderModel.push({ + name: item.name, + id: item.id, + udi: item.udi, + icon: item.icon = item.icon ? iconHelper.convertFromLegacyIcon(item.icon) : "icon-user", + avatars: item.avatars + }); + }); + }); + } } From eabfa7f414fcf89efbe34cf49b7e8563c0d2de3c Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska Date: Wed, 24 Mar 2021 15:28:06 +0100 Subject: [PATCH 13/32] Getting rid of the config file and implementing an appSetting instead --- .../Configuration/GlobalSettings.cs | 22 +++++++++++++++++++ .../Configuration/IGlobalSettings.cs | 9 ++++++++ src/Umbraco.Core/Constants-AppSettings.cs | 5 +++++ src/Umbraco.Web.UI/Umbraco.Web.UI.csproj | 1 - .../config/content.dashboard.access.config.js | 22 ------------------- src/Umbraco.Web.UI/web.Template.config | 1 + 6 files changed, 37 insertions(+), 23 deletions(-) delete mode 100644 src/Umbraco.Web.UI/config/content.dashboard.access.config.js diff --git a/src/Umbraco.Core/Configuration/GlobalSettings.cs b/src/Umbraco.Core/Configuration/GlobalSettings.cs index ba5baf9e87..0765a35a29 100644 --- a/src/Umbraco.Core/Configuration/GlobalSettings.cs +++ b/src/Umbraco.Core/Configuration/GlobalSettings.cs @@ -395,6 +395,28 @@ namespace Umbraco.Core.Configuration } } + /// + /// Gets a value indicating whether the content dashboard should be available to all users. + /// + /// + /// true if the dashboard is visible for all user groups; otherwise, false + /// and the default access rules for that dashboard will be in use. + /// + public bool AllowContentDashboardAccessToAllUsers + { + get + { + try + { + return bool.Parse(ConfigurationManager.AppSettings[Constants.AppSettings.AllowContentDashboardAccessToAllUsers]); + } + catch + { + return false; + } + } + } + /// /// An int value representing the time in milliseconds to lock the database for a write operation diff --git a/src/Umbraco.Core/Configuration/IGlobalSettings.cs b/src/Umbraco.Core/Configuration/IGlobalSettings.cs index 483829f85f..6016de2917 100644 --- a/src/Umbraco.Core/Configuration/IGlobalSettings.cs +++ b/src/Umbraco.Core/Configuration/IGlobalSettings.cs @@ -57,6 +57,15 @@ /// bool UseHttps { get; } + /// + /// Gets a value indicating whether the content dashboard should be available to all users. + /// + /// + /// true if the dashboard is visible for all user groups; otherwise, false + /// and the default access rules for that dashboard will be in use. + /// + bool AllowContentDashboardAccessToAllUsers { get; } + /// /// Returns a string value to determine if umbraco should skip version-checking. /// diff --git a/src/Umbraco.Core/Constants-AppSettings.cs b/src/Umbraco.Core/Constants-AppSettings.cs index f04f0e1f5f..1f096ab9f9 100644 --- a/src/Umbraco.Core/Constants-AppSettings.cs +++ b/src/Umbraco.Core/Constants-AppSettings.cs @@ -110,6 +110,11 @@ namespace Umbraco.Core /// public const string UseHttps = "Umbraco.Core.UseHttps"; + /// + /// A true/false value indicating whether the content dashboard should be visible for all user groups. + /// + public const string AllowContentDashboardAccessToAllUsers = "Umbraco.Core.AllowContentDashboardAccessToAllUsers"; + /// /// TODO: FILL ME IN /// diff --git a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj index 7fb27deb7e..eb6649a7c4 100644 --- a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj +++ b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj @@ -148,7 +148,6 @@ True Settings.settings - diff --git a/src/Umbraco.Web.UI/config/content.dashboard.access.config.js b/src/Umbraco.Web.UI/config/content.dashboard.access.config.js deleted file mode 100644 index 93c727d85a..0000000000 --- a/src/Umbraco.Web.UI/config/content.dashboard.access.config.js +++ /dev/null @@ -1,22 +0,0 @@ -[ - { - "Type": "grant", - "Value": "admin" - }, - { - "Type": "grant", - "Value": "editor" - }, - { - "Type": "grant", - "Value": "sensitiveData" - }, - { - "Type": "grant", - "Value": "translator" - }, - { - "Type": "grant", - "Value": "writer" - } -] diff --git a/src/Umbraco.Web.UI/web.Template.config b/src/Umbraco.Web.UI/web.Template.config index 03f462fb9e..ae141e5408 100644 --- a/src/Umbraco.Web.UI/web.Template.config +++ b/src/Umbraco.Web.UI/web.Template.config @@ -37,6 +37,7 @@ + From 6e54c6fefde685a0d10fe9a2fdf5c82177b8c0b0 Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska Date: Wed, 24 Mar 2021 15:34:05 +0100 Subject: [PATCH 14/32] Implementation for IContentDashboardSettings --- src/Umbraco.Core/ConfigsExtensions.cs | 6 +++ .../Dashboards/ContentDashboardSettings.cs | 43 +++++++++++++++++++ .../Dashboards/IContentDashboardSettings.cs | 13 ++++++ src/Umbraco.Core/Umbraco.Core.csproj | 2 + .../Dashboards/ContentDashboard.cs | 17 ++++---- 5 files changed, 73 insertions(+), 8 deletions(-) create mode 100644 src/Umbraco.Core/Dashboards/ContentDashboardSettings.cs create mode 100644 src/Umbraco.Core/Dashboards/IContentDashboardSettings.cs diff --git a/src/Umbraco.Core/ConfigsExtensions.cs b/src/Umbraco.Core/ConfigsExtensions.cs index d1672c6c7f..17dc63943a 100644 --- a/src/Umbraco.Core/ConfigsExtensions.cs +++ b/src/Umbraco.Core/ConfigsExtensions.cs @@ -5,9 +5,11 @@ using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.Grid; using Umbraco.Core.Configuration.HealthChecks; using Umbraco.Core.Configuration.UmbracoSettings; +using Umbraco.Core.Dashboards; using Umbraco.Core.IO; using Umbraco.Core.Logging; using Umbraco.Core.Manifest; +using Umbraco.Core.Services; namespace Umbraco.Core { @@ -48,6 +50,10 @@ namespace Umbraco.Core configDir, factory.GetInstance(), factory.GetInstance().Debug)); + + configs.Add(factory => + new ContentDashboardSettings(factory.GetInstance(), + factory.GetInstance())); } } } diff --git a/src/Umbraco.Core/Dashboards/ContentDashboardSettings.cs b/src/Umbraco.Core/Dashboards/ContentDashboardSettings.cs new file mode 100644 index 0000000000..3db808fe02 --- /dev/null +++ b/src/Umbraco.Core/Dashboards/ContentDashboardSettings.cs @@ -0,0 +1,43 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Umbraco.Core.Configuration; +using Umbraco.Core.Services; + +namespace Umbraco.Core.Dashboards +{ + public class ContentDashboardSettings: IContentDashboardSettings + { + private readonly IGlobalSettings _globalSettings; + private readonly IUserService _userService; + + public ContentDashboardSettings(IGlobalSettings globalSettings, IUserService userService) + { + _globalSettings = globalSettings; + _userService = userService; + } + + public IAccessRule[] GetAccessRulesFromConfig() + { + var rules = new List(); + + if (_globalSettings.AllowContentDashboardAccessToAllUsers) + { + var allUserGroups = _userService.GetAllUserGroups(); + + foreach (var userGroup in allUserGroups) + { + rules.Add(new AccessRule + { + Type = AccessRuleType.Grant, + Value = userGroup.Alias + }); + } + } + + return rules.ToArray(); + } + } +} diff --git a/src/Umbraco.Core/Dashboards/IContentDashboardSettings.cs b/src/Umbraco.Core/Dashboards/IContentDashboardSettings.cs new file mode 100644 index 0000000000..9b5ea7d7dd --- /dev/null +++ b/src/Umbraco.Core/Dashboards/IContentDashboardSettings.cs @@ -0,0 +1,13 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Umbraco.Core.Dashboards +{ + public interface IContentDashboardSettings + { + IAccessRule[] GetAccessRulesFromConfig(); + } +} diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 832b8a5801..63c7ac178d 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -131,6 +131,8 @@ + + diff --git a/src/Umbraco.Web/Dashboards/ContentDashboard.cs b/src/Umbraco.Web/Dashboards/ContentDashboard.cs index 3dbc0cb693..4b17bcf7ff 100644 --- a/src/Umbraco.Web/Dashboards/ContentDashboard.cs +++ b/src/Umbraco.Web/Dashboards/ContentDashboard.cs @@ -10,6 +10,7 @@ namespace Umbraco.Web.Dashboards [Weight(10)] public class ContentDashboard : IDashboard { + private readonly IContentDashboardSettings _dashboardSettings; public string Alias => "contentIntro"; public string[] Sections => new[] { "content" }; @@ -20,15 +21,9 @@ namespace Umbraco.Web.Dashboards { get { - IAccessRule[] rules; - var dashboardConfig = Path.Combine(IOHelper.MapPath(SystemDirectories.Config), "content.dashboard.access.config.js"); + var rules = _dashboardSettings.GetAccessRulesFromConfig(); - if (File.Exists(dashboardConfig)) - { - var rawJson = File.ReadAllText(dashboardConfig); - rules = JsonConvert.DeserializeObject(rawJson); - } - else + if (rules.Length == 0) { rules = new IAccessRule[] { @@ -36,8 +31,14 @@ namespace Umbraco.Web.Dashboards new AccessRule {Type = AccessRuleType.Grant, Value = Constants.Security.AdminGroupAlias} }; } + return rules; } } + + public ContentDashboard(IContentDashboardSettings dashboardSettings) + { + _dashboardSettings = dashboardSettings; + } } } From 61f486ebeaac426a6fc304d0bebbab9cc35d15e6 Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska Date: Wed, 24 Mar 2021 15:36:29 +0100 Subject: [PATCH 15/32] Cleanup --- src/Umbraco.Core/ConfigsExtensions.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Umbraco.Core/ConfigsExtensions.cs b/src/Umbraco.Core/ConfigsExtensions.cs index 17dc63943a..92d03adadb 100644 --- a/src/Umbraco.Core/ConfigsExtensions.cs +++ b/src/Umbraco.Core/ConfigsExtensions.cs @@ -1,6 +1,5 @@ using System.IO; using Umbraco.Core.Cache; -using Umbraco.Core.Composing; using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.Grid; using Umbraco.Core.Configuration.HealthChecks; From 56d5704167e940c540ae67864cdb5263b9b535f3 Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska Date: Wed, 24 Mar 2021 16:15:21 +0100 Subject: [PATCH 16/32] bool.Try --- src/Umbraco.Core/Configuration/GlobalSettings.cs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/Umbraco.Core/Configuration/GlobalSettings.cs b/src/Umbraco.Core/Configuration/GlobalSettings.cs index 0765a35a29..a451018169 100644 --- a/src/Umbraco.Core/Configuration/GlobalSettings.cs +++ b/src/Umbraco.Core/Configuration/GlobalSettings.cs @@ -406,14 +406,8 @@ namespace Umbraco.Core.Configuration { get { - try - { - return bool.Parse(ConfigurationManager.AppSettings[Constants.AppSettings.AllowContentDashboardAccessToAllUsers]); - } - catch - { - return false; - } + bool.TryParse(ConfigurationManager.AppSettings[Constants.AppSettings.AllowContentDashboardAccessToAllUsers], out var value); + return value; } } From f68d4d6968358c0c7aefe75aa2324cd591511005 Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska Date: Thu, 25 Mar 2021 11:11:41 +0100 Subject: [PATCH 17/32] Taking AllowContentDashboardAccessToAllUsers prop from GlobalSettings to ContentDashboardSettings and saving AccessRulesFromConfig into a backing field --- src/Umbraco.Core/ConfigsExtensions.cs | 5 +-- .../Configuration/GlobalSettings.cs | 17 -------- .../Configuration/IGlobalSettings.cs | 9 ---- .../Dashboards/ContentDashboardSettings.cs | 43 ++++++------------- .../Dashboards/IContentDashboardSettings.cs | 17 ++++---- .../Dashboards/ContentDashboard.cs | 42 +++++++++++++++--- 6 files changed, 59 insertions(+), 74 deletions(-) diff --git a/src/Umbraco.Core/ConfigsExtensions.cs b/src/Umbraco.Core/ConfigsExtensions.cs index 92d03adadb..10594fc970 100644 --- a/src/Umbraco.Core/ConfigsExtensions.cs +++ b/src/Umbraco.Core/ConfigsExtensions.cs @@ -8,7 +8,6 @@ using Umbraco.Core.Dashboards; using Umbraco.Core.IO; using Umbraco.Core.Logging; using Umbraco.Core.Manifest; -using Umbraco.Core.Services; namespace Umbraco.Core { @@ -50,9 +49,7 @@ namespace Umbraco.Core factory.GetInstance(), factory.GetInstance().Debug)); - configs.Add(factory => - new ContentDashboardSettings(factory.GetInstance(), - factory.GetInstance())); + configs.Add(() => new ContentDashboardSettings()); } } } diff --git a/src/Umbraco.Core/Configuration/GlobalSettings.cs b/src/Umbraco.Core/Configuration/GlobalSettings.cs index a451018169..c844abe75e 100644 --- a/src/Umbraco.Core/Configuration/GlobalSettings.cs +++ b/src/Umbraco.Core/Configuration/GlobalSettings.cs @@ -395,23 +395,6 @@ namespace Umbraco.Core.Configuration } } - /// - /// Gets a value indicating whether the content dashboard should be available to all users. - /// - /// - /// true if the dashboard is visible for all user groups; otherwise, false - /// and the default access rules for that dashboard will be in use. - /// - public bool AllowContentDashboardAccessToAllUsers - { - get - { - bool.TryParse(ConfigurationManager.AppSettings[Constants.AppSettings.AllowContentDashboardAccessToAllUsers], out var value); - return value; - } - } - - /// /// An int value representing the time in milliseconds to lock the database for a write operation /// diff --git a/src/Umbraco.Core/Configuration/IGlobalSettings.cs b/src/Umbraco.Core/Configuration/IGlobalSettings.cs index 6016de2917..483829f85f 100644 --- a/src/Umbraco.Core/Configuration/IGlobalSettings.cs +++ b/src/Umbraco.Core/Configuration/IGlobalSettings.cs @@ -57,15 +57,6 @@ /// bool UseHttps { get; } - /// - /// Gets a value indicating whether the content dashboard should be available to all users. - /// - /// - /// true if the dashboard is visible for all user groups; otherwise, false - /// and the default access rules for that dashboard will be in use. - /// - bool AllowContentDashboardAccessToAllUsers { get; } - /// /// Returns a string value to determine if umbraco should skip version-checking. /// diff --git a/src/Umbraco.Core/Dashboards/ContentDashboardSettings.cs b/src/Umbraco.Core/Dashboards/ContentDashboardSettings.cs index 3db808fe02..f8fb5c7b06 100644 --- a/src/Umbraco.Core/Dashboards/ContentDashboardSettings.cs +++ b/src/Umbraco.Core/Dashboards/ContentDashboardSettings.cs @@ -1,43 +1,24 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; -using Umbraco.Core.Configuration; -using Umbraco.Core.Services; +using System.Configuration; namespace Umbraco.Core.Dashboards { public class ContentDashboardSettings: IContentDashboardSettings { - private readonly IGlobalSettings _globalSettings; - private readonly IUserService _userService; - public ContentDashboardSettings(IGlobalSettings globalSettings, IUserService userService) + /// + /// Gets a value indicating whether the content dashboard should be available to all users. + /// + /// + /// true if the dashboard is visible for all user groups; otherwise, false + /// and the default access rules for that dashboard will be in use. + /// + public bool AllowContentDashboardAccessToAllUsers { - _globalSettings = globalSettings; - _userService = userService; - } - - public IAccessRule[] GetAccessRulesFromConfig() - { - var rules = new List(); - - if (_globalSettings.AllowContentDashboardAccessToAllUsers) + get { - var allUserGroups = _userService.GetAllUserGroups(); - - foreach (var userGroup in allUserGroups) - { - rules.Add(new AccessRule - { - Type = AccessRuleType.Grant, - Value = userGroup.Alias - }); - } + bool.TryParse(ConfigurationManager.AppSettings[Constants.AppSettings.AllowContentDashboardAccessToAllUsers], out var value); + return value; } - - return rules.ToArray(); } } } diff --git a/src/Umbraco.Core/Dashboards/IContentDashboardSettings.cs b/src/Umbraco.Core/Dashboards/IContentDashboardSettings.cs index 9b5ea7d7dd..862a28b90e 100644 --- a/src/Umbraco.Core/Dashboards/IContentDashboardSettings.cs +++ b/src/Umbraco.Core/Dashboards/IContentDashboardSettings.cs @@ -1,13 +1,14 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; - -namespace Umbraco.Core.Dashboards +namespace Umbraco.Core.Dashboards { public interface IContentDashboardSettings { - IAccessRule[] GetAccessRulesFromConfig(); + /// + /// Gets a value indicating whether the content dashboard should be available to all users. + /// + /// + /// true if the dashboard is visible for all user groups; otherwise, false + /// and the default access rules for that dashboard will be in use. + /// + bool AllowContentDashboardAccessToAllUsers { get; } } } diff --git a/src/Umbraco.Web/Dashboards/ContentDashboard.cs b/src/Umbraco.Web/Dashboards/ContentDashboard.cs index 4b17bcf7ff..260eb8baf9 100644 --- a/src/Umbraco.Web/Dashboards/ContentDashboard.cs +++ b/src/Umbraco.Web/Dashboards/ContentDashboard.cs @@ -1,9 +1,8 @@ -using System.IO; -using Newtonsoft.Json; +using System.Collections.Generic; using Umbraco.Core; using Umbraco.Core.Composing; using Umbraco.Core.Dashboards; -using Umbraco.Core.IO; +using Umbraco.Core.Services; namespace Umbraco.Web.Dashboards { @@ -11,6 +10,9 @@ namespace Umbraco.Web.Dashboards public class ContentDashboard : IDashboard { private readonly IContentDashboardSettings _dashboardSettings; + private readonly IUserService _userService; + private IAccessRule[] _accessRulesFromConfig; + public string Alias => "contentIntro"; public string[] Sections => new[] { "content" }; @@ -21,7 +23,7 @@ namespace Umbraco.Web.Dashboards { get { - var rules = _dashboardSettings.GetAccessRulesFromConfig(); + var rules = AccessRulesFromConfig; if (rules.Length == 0) { @@ -36,9 +38,39 @@ namespace Umbraco.Web.Dashboards } } - public ContentDashboard(IContentDashboardSettings dashboardSettings) + private IAccessRule[] AccessRulesFromConfig + { + get + { + if (_accessRulesFromConfig is null) + { + var rules = new List(); + + if (_dashboardSettings.AllowContentDashboardAccessToAllUsers) + { + var allUserGroups = _userService.GetAllUserGroups(); + + foreach (var userGroup in allUserGroups) + { + rules.Add(new AccessRule + { + Type = AccessRuleType.Grant, + Value = userGroup.Alias + }); + } + } + + _accessRulesFromConfig = rules.ToArray(); + } + + return _accessRulesFromConfig; + } + } + + public ContentDashboard(IContentDashboardSettings dashboardSettings, IUserService userService) { _dashboardSettings = dashboardSettings; + _userService = userService; } } } From eb8a180f1e0838eff2bab1b88b34904fd222d88d Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 29 Mar 2021 16:24:55 +1100 Subject: [PATCH 18/32] Moves base signin manager logic to a base class --- .../Security/MemberSignInManagerTests.cs | 1 - .../Security/BackOfficeSignInManager.cs | 414 +----------------- src/Umbraco.Web.Common/Constants/Security.cs | 14 - .../Security/MemberSignInManager.cs | 207 ++------- 4 files changed, 50 insertions(+), 586 deletions(-) delete mode 100644 src/Umbraco.Web.Common/Constants/Security.cs diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs index 49dc5d9d55..86f96c7d71 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs @@ -24,7 +24,6 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security _mockLogger = new Mock>(); return new MemberSignInManager( _memberManager.Object, - Mock.Of(), Mock.Of(), Mock.Of>(), Mock.Of>(), diff --git a/src/Umbraco.Web.BackOffice/Security/BackOfficeSignInManager.cs b/src/Umbraco.Web.BackOffice/Security/BackOfficeSignInManager.cs index 03ebb1aa45..3e921ba0f9 100644 --- a/src/Umbraco.Web.BackOffice/Security/BackOfficeSignInManager.cs +++ b/src/Umbraco.Web.BackOffice/Security/BackOfficeSignInManager.cs @@ -17,17 +17,22 @@ namespace Umbraco.Cms.Web.BackOffice.Security { using Constants = Core.Constants; - public class BackOfficeSignInManager : SignInManager, IBackOfficeSignInManager + /// + /// The sign in manager for back office users + /// + public class BackOfficeSignInManager : UmbracoSignInManager, IBackOfficeSignInManager { - // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs - private const string UmbracoSignInMgrLoginProviderKey = "LoginProvider"; - // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs - private const string UmbracoSignInMgrXsrfKey = "XsrfId"; - private readonly BackOfficeUserManager _userManager; private readonly IBackOfficeExternalLoginProviders _externalLogins; private readonly GlobalSettings _globalSettings; + protected override string AuthenticationType => Constants.Security.BackOfficeAuthenticationType; + + protected override string ExternalAuthenticationType => Constants.Security.BackOfficeExternalAuthenticationType; + + protected override string TwoFactorAuthenticationType => Constants.Security.BackOfficeTwoFactorAuthenticationType; + + protected override string TwoFactorRememberMeAuthenticationType => Constants.Security.BackOfficeTwoFactorRememberMeAuthenticationType; public BackOfficeSignInManager( BackOfficeUserManager userManager, @@ -46,261 +51,6 @@ namespace Umbraco.Cms.Web.BackOffice.Security _globalSettings = globalSettings.Value; } - // TODO: Have a look into RefreshSignInAsync since we might be able to use this new functionality for auto-cookie renewal in our middleware, though - // i suspect it's taken care of already. - - - /// - public override async Task PasswordSignInAsync(BackOfficeIdentityUser user, string password, bool isPersistent, bool lockoutOnFailure) - { - // override to handle logging/events - var result = await base.PasswordSignInAsync(user, password, isPersistent, lockoutOnFailure); - return await HandleSignIn(user, user.UserName, result); - } - - /// - public override async Task PasswordSignInAsync(string userName, string password, bool isPersistent, bool lockoutOnFailure) - { - // override to handle logging/events - var user = await UserManager.FindByNameAsync(userName); - if (user == null) - return await HandleSignIn(null, userName, SignInResult.Failed); - return await PasswordSignInAsync(user, password, isPersistent, lockoutOnFailure); - } - - /// - public override async Task GetTwoFactorAuthenticationUserAsync() - { - // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs - // replaced in order to use a custom auth type - - var info = await RetrieveTwoFactorInfoAsync(); - if (info == null) - { - return null; - } - return await UserManager.FindByIdAsync(info.UserId); - } - - /// - public override async Task TwoFactorSignInAsync(string provider, string code, bool isPersistent, bool rememberClient) - { - // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L552 - // replaced in order to use a custom auth type and to implement logging/events - - var twoFactorInfo = await RetrieveTwoFactorInfoAsync(); - if (twoFactorInfo == null || twoFactorInfo.UserId == null) - { - return SignInResult.Failed; - } - var user = await UserManager.FindByIdAsync(twoFactorInfo.UserId); - if (user == null) - { - return SignInResult.Failed; - } - - var error = await PreSignInCheck(user); - if (error != null) - { - return error; - } - if (await UserManager.VerifyTwoFactorTokenAsync(user, provider, code)) - { - await DoTwoFactorSignInAsync(user, twoFactorInfo, isPersistent, rememberClient); - return await HandleSignIn(user, user?.UserName, SignInResult.Success); - } - // If the token is incorrect, record the failure which also may cause the user to be locked out - await UserManager.AccessFailedAsync(user); - return await HandleSignIn(user, user?.UserName, SignInResult.Failed); - } - - - /// - public override bool IsSignedIn(ClaimsPrincipal principal) - { - // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L126 - // replaced in order to use a custom auth type - - if (principal == null) - { - throw new ArgumentNullException(nameof(principal)); - } - return principal?.Identities != null && - principal.Identities.Any(i => i.AuthenticationType == Constants.Security.BackOfficeAuthenticationType); - } - - /// - public override async Task RefreshSignInAsync(BackOfficeIdentityUser user) - { - // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L126 - // replaced in order to use a custom auth type - - var auth = await Context.AuthenticateAsync(Constants.Security.BackOfficeAuthenticationType); - IList claims = Array.Empty(); - - var authenticationMethod = auth?.Principal?.FindFirst(ClaimTypes.AuthenticationMethod); - var amr = auth?.Principal?.FindFirst("amr"); - - if (authenticationMethod != null || amr != null) - { - claims = new List(); - if (authenticationMethod != null) - { - claims.Add(authenticationMethod); - } - if (amr != null) - { - claims.Add(amr); - } - } - - await SignInWithClaimsAsync(user, auth?.Properties, claims); - } - - /// - public override async Task SignInWithClaimsAsync(BackOfficeIdentityUser user, AuthenticationProperties authenticationProperties, IEnumerable additionalClaims) - { - // override to replace IdentityConstants.ApplicationScheme with Constants.Security.BackOfficeAuthenticationType - // code taken from aspnetcore: https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs - // we also override to set the current HttpContext principal since this isn't done by default - - var userPrincipal = await CreateUserPrincipalAsync(user); - foreach (var claim in additionalClaims) - userPrincipal.Identities.First().AddClaim(claim); - - // FYI (just for informational purposes): - // This calls an ext method will eventually reaches `IAuthenticationService.SignInAsync` - // which then resolves the `IAuthenticationSignInHandler` for the current scheme - // by calling `IAuthenticationHandlerProvider.GetHandlerAsync(context, scheme);` - // which then calls `IAuthenticationSignInHandler.SignInAsync` = CookieAuthenticationHandler.HandleSignInAsync - - // Also note, that when the CookieAuthenticationHandler sign in is successful we handle that event within our - // own ConfigureUmbracoBackOfficeCookieOptions which assigns the current HttpContext.User to the IPrincipal created - - // Also note, this method gets called when performing 2FA logins - - await Context.SignInAsync(Constants.Security.BackOfficeAuthenticationType, - userPrincipal, - authenticationProperties ?? new AuthenticationProperties()); - } - - /// - public override async Task SignOutAsync() - { - // override to replace IdentityConstants.ApplicationScheme with Constants.Security.BackOfficeAuthenticationType - // code taken from aspnetcore: https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs - - await Context.SignOutAsync(Constants.Security.BackOfficeAuthenticationType); - await Context.SignOutAsync(Constants.Security.BackOfficeExternalAuthenticationType); - await Context.SignOutAsync(Constants.Security.BackOfficeTwoFactorAuthenticationType); - } - - - /// - public override async Task IsTwoFactorClientRememberedAsync(BackOfficeIdentityUser user) - { - // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L422 - // to replace the auth scheme - - var userId = await UserManager.GetUserIdAsync(user); - var result = await Context.AuthenticateAsync(Constants.Security.BackOfficeTwoFactorRememberMeAuthenticationType); - return (result?.Principal != null && result.Principal.FindFirstValue(ClaimTypes.Name) == userId); - } - - - /// - public override async Task RememberTwoFactorClientAsync(BackOfficeIdentityUser user) - { - // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L422 - // to replace the auth scheme - - var principal = await StoreRememberClient(user); - await Context.SignInAsync(Constants.Security.BackOfficeTwoFactorRememberMeAuthenticationType, - principal, - new AuthenticationProperties { IsPersistent = true }); - } - - - /// - public override Task ForgetTwoFactorClientAsync() - { - // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L422 - // to replace the auth scheme - - return Context.SignOutAsync(Constants.Security.BackOfficeTwoFactorRememberMeAuthenticationType); - } - - - /// - public override async Task TwoFactorRecoveryCodeSignInAsync(string recoveryCode) - { - // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L422 - // to replace the auth scheme - - var twoFactorInfo = await RetrieveTwoFactorInfoAsync(); - if (twoFactorInfo == null || twoFactorInfo.UserId == null) - { - return SignInResult.Failed; - } - var user = await UserManager.FindByIdAsync(twoFactorInfo.UserId); - if (user == null) - { - return SignInResult.Failed; - } - - var result = await UserManager.RedeemTwoFactorRecoveryCodeAsync(user, recoveryCode); - if (result.Succeeded) - { - await DoTwoFactorSignInAsync(user, twoFactorInfo, isPersistent: false, rememberClient: false); - return SignInResult.Success; - } - - // We don't protect against brute force attacks since codes are expected to be random. - return SignInResult.Failed; - } - - - /// - public override async Task GetExternalLoginInfoAsync(string expectedXsrf = null) - { - // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L422 - // to replace the auth scheme - - var auth = await Context.AuthenticateAsync(Constants.Security.BackOfficeExternalAuthenticationType); - var items = auth?.Properties?.Items; - if (auth?.Principal == null || items == null || !items.ContainsKey(UmbracoSignInMgrLoginProviderKey)) - { - return null; - } - - if (expectedXsrf != null) - { - if (!items.ContainsKey(UmbracoSignInMgrXsrfKey)) - { - return null; - } - var userId = items[UmbracoSignInMgrXsrfKey]; - if (userId != expectedXsrf) - { - return null; - } - } - - var providerKey = auth.Principal.FindFirstValue(ClaimTypes.NameIdentifier); - var provider = items[UmbracoSignInMgrLoginProviderKey] as string; - if (providerKey == null || provider == null) - { - return null; - } - - var providerDisplayName = (await GetExternalAuthenticationSchemesAsync()).FirstOrDefault(p => p.Name == provider)?.DisplayName ?? provider; - return new ExternalLoginInfo(auth.Principal, provider, providerKey, providerDisplayName) - { - AuthenticationTokens = auth.Properties.GetTokens(), - AuthenticationProperties = auth.Properties - }; - } - /// /// Custom ExternalLoginSignInAsync overload for handling external sign in with auto-linking /// @@ -367,66 +117,19 @@ namespace Umbraco.Cms.Web.BackOffice.Security return base.GetExternalAuthenticationSchemesAsync(); } - /// - protected override async Task SignInOrTwoFactorAsync(BackOfficeIdentityUser user, bool isPersistent, string loginProvider = null, bool bypassTwoFactor = false) - { - // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs - // to replace custom auth types - - if (!bypassTwoFactor && await IsTfaEnabled(user)) - { - if (!await IsTwoFactorClientRememberedAsync(user)) - { - // Store the userId for use after two factor check - var userId = await UserManager.GetUserIdAsync(user); - await Context.SignInAsync(IdentityConstants.TwoFactorUserIdScheme, StoreTwoFactorInfo(userId, loginProvider)); - return SignInResult.TwoFactorRequired; - } - } - // Cleanup external cookie - if (loginProvider != null) - { - await Context.SignOutAsync(Constants.Security.BackOfficeExternalAuthenticationType); - } - if (loginProvider == null) - { - await SignInWithClaimsAsync(user, isPersistent, new Claim[] { new Claim("amr", "pwd") }); - } - else - { - await SignInAsync(user, isPersistent, loginProvider); - } - return SignInResult.Success; - } - /// - /// Called on any login attempt to update the AccessFailedCount and to raise events + /// Overridden to deal with events/notificiations /// /// /// /// /// - private async Task HandleSignIn(BackOfficeIdentityUser user, string username, SignInResult result) + protected override async Task HandleSignIn(BackOfficeIdentityUser user, string username, SignInResult result) { - // TODO: Here I believe we can do all (or most) of the usermanager event raising so that it is not in the AuthenticationController - - if (username.IsNullOrWhiteSpace()) - { - username = "UNKNOWN"; // could happen in 2fa or something else weird - } + result = await base.HandleSignIn(user, username, result); if (result.Succeeded) { - //track the last login date - user.LastLoginDateUtc = DateTime.UtcNow; - if (user.AccessFailedCount > 0) - { - //we have successfully logged in, reset the AccessFailedCount - user.AccessFailedCount = 0; - } - await UserManager.UpdateAsync(user); - - Logger.LogInformation("User: {UserName} logged in from IP address {IpAddress}", username, Context.Connection.RemoteIpAddress); if (user != null) { _userManager.NotifyLoginSuccess(Context.User, user.Id); @@ -435,16 +138,13 @@ namespace Umbraco.Cms.Web.BackOffice.Security else if (result.IsLockedOut) { _userManager.NotifyAccountLocked(Context.User, user.Id); - Logger.LogInformation("Login attempt failed for username {UserName} from IP address {IpAddress}, the user is locked", username, Context.Connection.RemoteIpAddress); } else if (result.RequiresTwoFactor) { _userManager.NotifyLoginRequiresVerification(Context.User, user.Id); - Logger.LogInformation("Login attempt requires verification for username {UserName} from IP address {IpAddress}", username, Context.Connection.RemoteIpAddress); } else if (!result.Succeeded || result.IsNotAllowed) { - Logger.LogInformation("Login attempt failed for username {UserName} from IP address {IpAddress}", username, Context.Connection.RemoteIpAddress); } else { @@ -454,92 +154,6 @@ namespace Umbraco.Cms.Web.BackOffice.Security return result; } - // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L782 - // since it's not public - private async Task IsTfaEnabled(BackOfficeIdentityUser user) - => UserManager.SupportsUserTwoFactor && - await UserManager.GetTwoFactorEnabledAsync(user) && - (await UserManager.GetValidTwoFactorProvidersAsync(user)).Count > 0; - - // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L743 - // to replace custom auth types - private ClaimsPrincipal StoreTwoFactorInfo(string userId, string loginProvider) - { - var identity = new ClaimsIdentity(Constants.Security.BackOfficeTwoFactorAuthenticationType); - identity.AddClaim(new Claim(ClaimTypes.Name, userId)); - if (loginProvider != null) - { - identity.AddClaim(new Claim(ClaimTypes.AuthenticationMethod, loginProvider)); - } - return new ClaimsPrincipal(identity); - } - - // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs - // copy is required in order to use custom auth types - private async Task StoreRememberClient(BackOfficeIdentityUser user) - { - var userId = await UserManager.GetUserIdAsync(user); - var rememberBrowserIdentity = new ClaimsIdentity(Constants.Security.BackOfficeTwoFactorRememberMeAuthenticationType); - rememberBrowserIdentity.AddClaim(new Claim(ClaimTypes.Name, userId)); - if (UserManager.SupportsUserSecurityStamp) - { - var stamp = await UserManager.GetSecurityStampAsync(user); - rememberBrowserIdentity.AddClaim(new Claim(Options.ClaimsIdentity.SecurityStampClaimType, stamp)); - } - return new ClaimsPrincipal(rememberBrowserIdentity); - } - - // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs - // copy is required in order to use custom auth types - private async Task DoTwoFactorSignInAsync(BackOfficeIdentityUser user, TwoFactorAuthenticationInfo twoFactorInfo, bool isPersistent, bool rememberClient) - { - // When token is verified correctly, clear the access failed count used for lockout - await ResetLockout(user); - - var claims = new List - { - new Claim("amr", "mfa") - }; - - // Cleanup external cookie - if (twoFactorInfo.LoginProvider != null) - { - claims.Add(new Claim(ClaimTypes.AuthenticationMethod, twoFactorInfo.LoginProvider)); - await Context.SignOutAsync(Constants.Security.BackOfficeExternalAuthenticationType); - } - // Cleanup two factor user id cookie - await Context.SignOutAsync(Constants.Security.BackOfficeTwoFactorAuthenticationType); - if (rememberClient) - { - await RememberTwoFactorClientAsync(user); - } - await SignInWithClaimsAsync(user, isPersistent, claims); - } - - // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs - // copy is required in order to use a custom auth type - private async Task RetrieveTwoFactorInfoAsync() - { - var result = await Context.AuthenticateAsync(Constants.Security.BackOfficeTwoFactorAuthenticationType); - if (result?.Principal != null) - { - return new TwoFactorAuthenticationInfo - { - UserId = result.Principal.FindFirstValue(ClaimTypes.Name), - LoginProvider = result.Principal.FindFirstValue(ClaimTypes.AuthenticationMethod) - }; - } - return null; - } - - // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L891 - private class TwoFactorAuthenticationInfo - { - public string UserId { get; set; } - public string LoginProvider { get; set; } - } - - /// /// Used for auto linking/creating user accounts for external logins /// diff --git a/src/Umbraco.Web.Common/Constants/Security.cs b/src/Umbraco.Web.Common/Constants/Security.cs deleted file mode 100644 index f2fcbc65f8..0000000000 --- a/src/Umbraco.Web.Common/Constants/Security.cs +++ /dev/null @@ -1,14 +0,0 @@ -namespace Umbraco.Cms.Web.Common.Constants -{ - public static class Security - { - //TODO: implement 2Factor external - - public const string MemberAuthenticationType = "UmbracoMember"; - //public const string MemberExternalAuthenticationType = "UmbracoMemberExternalCookie"; - //public const string MemberExternalCookieName = "UMB_MEMBEREXTLOGIN"; - public const string MemberTokenAuthenticationType = "UmbracoMemberToken"; - //public const string MemberTwoFactorAuthenticationType = "UmbracoMemberTwoFactorCookie"; - //public const string MemberTwoFactorRememberMeAuthenticationType = "UmbracoMemberTwoFactorRememberMeCookie"; - } -} diff --git a/src/Umbraco.Web.Common/Security/MemberSignInManager.cs b/src/Umbraco.Web.Common/Security/MemberSignInManager.cs index d3adcc4833..392d20ebbe 100644 --- a/src/Umbraco.Web.Common/Security/MemberSignInManager.cs +++ b/src/Umbraco.Web.Common/Security/MemberSignInManager.cs @@ -1,38 +1,42 @@ using System; using System.Collections.Generic; -using System.Linq; -using System.Security.Claims; using System.Threading.Tasks; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; -using Umbraco.Cms.Core.Net; using Umbraco.Cms.Core.Security; -using Umbraco.Extensions; namespace Umbraco.Cms.Web.Common.Security { - //TODO: can any of this be combined/merged with BackOfficeSignInManager using T for the identity user? - //TODO: Need to implement events on member login/logout etc - public class MemberSignInManager : SignInManager + /// + /// The sign in manager for members + /// + public class MemberSignInManager : UmbracoSignInManager { - private const string ClaimType = "amr"; - private const string PasswordValue = "pwd"; - private readonly IIpResolver _ipResolver; - public MemberSignInManager( UserManager memberManager, - IIpResolver ipResolver, IHttpContextAccessor contextAccessor, IUserClaimsPrincipalFactory claimsFactory, IOptions optionsAccessor, ILogger> logger, IAuthenticationSchemeProvider schemes, IUserConfirmation confirmation) : - base(memberManager, contextAccessor, claimsFactory, optionsAccessor, logger, schemes, confirmation) => - _ipResolver = ipResolver ?? throw new ArgumentNullException(nameof(ipResolver)); + base(memberManager, contextAccessor, claimsFactory, optionsAccessor, logger, schemes, confirmation) + { } + + // use default scheme for members + protected override string AuthenticationType => IdentityConstants.ApplicationScheme; + + // use default scheme for members + protected override string ExternalAuthenticationType => IdentityConstants.ExternalScheme; + + // use default scheme for members + protected override string TwoFactorAuthenticationType => IdentityConstants.TwoFactorUserIdScheme; + + // use default scheme for members + protected override string TwoFactorRememberMeAuthenticationType => IdentityConstants.TwoFactorRememberMeScheme; /// public override async Task PasswordSignInAsync(MemberIdentityUser user, string password, bool isPersistent, bool lockoutOnFailure) @@ -56,179 +60,40 @@ namespace Umbraco.Cms.Web.Common.Security } /// - public override Task GetTwoFactorAuthenticationUserAsync() => throw new NotImplementedException("Two factor is not yet implemented for members"); + public override Task GetTwoFactorAuthenticationUserAsync() + => throw new NotImplementedException("Two factor is not yet implemented for members"); /// - public override Task TwoFactorSignInAsync(string provider, string code, bool isPersistent, bool rememberClient) => throw new NotImplementedException("Two factor is not yet implemented for members"); + public override Task TwoFactorSignInAsync(string provider, string code, bool isPersistent, bool rememberClient) + => throw new NotImplementedException("Two factor is not yet implemented for members"); /// - public override bool IsSignedIn(ClaimsPrincipal principal) - { - // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L126 - // replaced in order to use a custom auth type - // taken from BackOfficeSignInManager - - if (principal == null) - { - throw new ArgumentNullException(nameof(principal)); - } - return principal?.Identities != null && principal.Identities.Any(i => i.AuthenticationType == Constants.Security.MemberAuthenticationType); - } + public override Task IsTwoFactorClientRememberedAsync(MemberIdentityUser user) + => throw new NotImplementedException("Two factor is not yet implemented for members"); /// - public override async Task RefreshSignInAsync(MemberIdentityUser user) - { - // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L126 - // replaced in order to use a custom auth type - - AuthenticateResult auth = await Context.AuthenticateAsync(Constants.Security.MemberAuthenticationType); - IList claims = Array.Empty(); - - Claim authenticationMethod = auth?.Principal?.FindFirst(ClaimTypes.AuthenticationMethod); - Claim amr = auth?.Principal?.FindFirst(ClaimType); - - if (authenticationMethod != null || amr != null) - { - claims = new List(); - if (authenticationMethod != null) - { - claims.Add(authenticationMethod); - } - if (amr != null) - { - claims.Add(amr); - } - } - - await SignInWithClaimsAsync(user, auth?.Properties, claims); - } + public override Task RememberTwoFactorClientAsync(MemberIdentityUser user) + => throw new NotImplementedException("Two factor is not yet implemented for members"); /// - public override async Task SignInWithClaimsAsync(MemberIdentityUser user, AuthenticationProperties authenticationProperties, IEnumerable additionalClaims) - { - // TODO: taken from BackOfficeSigninManager and more notes are there - // override to replace IdentityConstants.ApplicationScheme with Constants.Security.MemberAuthenticationType - // code taken from aspnetcore: https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs - // we also override to set the current HttpContext principal since this isn't done by default - - ClaimsPrincipal userPrincipal = await CreateUserPrincipalAsync(user); - foreach (Claim claim in additionalClaims) - { - userPrincipal.Identities.First().AddClaim(claim); - } - - // TODO: For future, this method gets called when performing 2FA logins - await Context.SignInAsync(Constants.Security.MemberAuthenticationType, - userPrincipal, - authenticationProperties ?? new AuthenticationProperties()); - } + public override Task ForgetTwoFactorClientAsync() + => throw new NotImplementedException("Two factor is not yet implemented for members"); /// - public override async Task SignOutAsync() => - //TODO: does members need this custom signout type as per BackOfficeSignInManager? - // override to replace IdentityConstants.ApplicationScheme with Constants.Security.MemberAuthenticationType - // code taken from aspnetcore: https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs - await Context.SignOutAsync(Constants.Security.MemberAuthenticationType); + public override Task TwoFactorRecoveryCodeSignInAsync(string recoveryCode) + => throw new NotImplementedException("Two factor is not yet implemented for members"); /// - public override Task IsTwoFactorClientRememberedAsync(MemberIdentityUser user) => throw new NotImplementedException("Two factor is not yet implemented for members"); + public override Task GetExternalLoginInfoAsync(string expectedXsrf = null) + => throw new NotImplementedException("External login is not yet implemented for members"); /// - public override Task RememberTwoFactorClientAsync(MemberIdentityUser user) => throw new NotImplementedException("Two factor is not yet implemented for members"); + public override AuthenticationProperties ConfigureExternalAuthenticationProperties(string provider, string redirectUrl, string userId = null) + => throw new NotImplementedException("External login is not yet implemented for members"); /// - public override Task ForgetTwoFactorClientAsync() => throw new NotImplementedException("Two factor is not yet implemented for members"); + public override Task> GetExternalAuthenticationSchemesAsync() + => throw new NotImplementedException("External login is not yet implemented for members"); - /// - public override Task TwoFactorRecoveryCodeSignInAsync(string recoveryCode) => throw new NotImplementedException("Two factor is not yet implemented for members"); - - /// - public override Task GetExternalLoginInfoAsync(string expectedXsrf = null) => throw new NotImplementedException("External login is not yet implemented for members"); - - /// - public override AuthenticationProperties ConfigureExternalAuthenticationProperties(string provider, string redirectUrl, string userId = null) => throw new NotImplementedException("External login is not yet implemented for members"); - - /// - public override Task> GetExternalAuthenticationSchemesAsync() => throw new NotImplementedException("External login is not yet implemented for members"); - - /// - /// TODO: Two factor is not yet implemented for members - protected override async Task SignInOrTwoFactorAsync(MemberIdentityUser user, bool isPersistent, string loginProvider = null, bool bypassTwoFactor = false) - { - // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs - // to replace custom auth types - - //TODO: There is currently no two factor so this needs changing once implemented - if (loginProvider != null) - { - await SignInAsync(user, isPersistent, loginProvider); - } - else - { - await SignInWithClaimsAsync(user, isPersistent, new Claim[] - { - new Claim(ClaimType, PasswordValue) - }); - } - return SignInResult.Success; - } - - /// - /// Called on any login attempt to update the AccessFailedCount and to raise events - /// - /// - /// - /// - /// - private async Task HandleSignIn(MemberIdentityUser user, string username, SignInResult result) - { - // TODO: More TODO notes in BackOfficeSignInManager - if (username.IsNullOrWhiteSpace()) - { - //TODO: this might have unwanted effects if the member is called this - username = "UNKNOWN"; - } - - if (result.Succeeded) - { - //track the last login date - user.LastLoginDateUtc = DateTime.UtcNow; - if (user.AccessFailedCount > 0) - { - //we have successfully logged in, reset the AccessFailedCount - user.AccessFailedCount = 0; - } - await UserManager.UpdateAsync(user); - - Logger.LogInformation("User: {UserName} logged in from IP address {IpAddress}", username, _ipResolver.GetCurrentRequestIpAddress()); - if (user != null) - { - //TODO: what events do we want for members? - //_memberManager.RaiseLoginSuccessEvent(Context.User, user.Id); - } - } - else if (result.IsLockedOut) - { - //TODO: what events do we want for members? - //_memberManager.RaiseAccountLockedEvent(Context.User, user.Id); - Logger.LogInformation("Login attempt failed for username {UserName} from IP address {IpAddress}, the user is locked", username, _ipResolver.GetCurrentRequestIpAddress()); - } - else if (result.RequiresTwoFactor) - { - //TODO: what events do we want for members? - //_memberManager.RaiseLoginRequiresVerificationEvent(Context.User, user.Id); - Logger.LogInformation("Login attempt requires verification for username {UserName} from IP address {IpAddress}", username, _ipResolver.GetCurrentRequestIpAddress()); - } - else if (!result.Succeeded || result.IsNotAllowed) - { - Logger.LogInformation("Login attempt failed for username {UserName} from IP address {IpAddress}", username, _ipResolver.GetCurrentRequestIpAddress()); - } - else - { - throw new ArgumentOutOfRangeException(); - } - - return result; - } } } From 5f4818263f72a77c8279d34d17ad7dd6676f2388 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 29 Mar 2021 17:37:58 +1100 Subject: [PATCH 19/32] Fixes BackOfficeClaimsPrincipalFactory to use the correct auth type. Uses the correct UmbracoIdentityRole class, fixes up MemberSignInManagerTests, new MemberClaimsPrincipalFactory --- .../BackOfficeClaimsPrincipalFactory.cs | 45 +- .../Security/ClaimsIdentityExtensions.cs | 9 + .../Security/MemberUserStore.cs | 8 +- .../Security/UmbracoIdentityRole.cs | 8 + .../Security/MemberSignInManagerTests.cs | 51 +- .../ServiceCollectionExtensions.cs | 8 +- .../Security/MemberClaimsPrincipalFactory.cs | 15 + .../Security/MemberManager.cs | 1 + .../Security/MemberSignInManager.cs | 21 - .../Security/UmbracoSignInManager.cs | 453 ++++++++++++++++++ 10 files changed, 552 insertions(+), 67 deletions(-) create mode 100644 src/Umbraco.Web.Common/Security/MemberClaimsPrincipalFactory.cs create mode 100644 src/Umbraco.Web.Common/Security/UmbracoSignInManager.cs diff --git a/src/Umbraco.Infrastructure/Security/BackOfficeClaimsPrincipalFactory.cs b/src/Umbraco.Infrastructure/Security/BackOfficeClaimsPrincipalFactory.cs index 2bb9b1ab8d..9c6aeca80e 100644 --- a/src/Umbraco.Infrastructure/Security/BackOfficeClaimsPrincipalFactory.cs +++ b/src/Umbraco.Infrastructure/Security/BackOfficeClaimsPrincipalFactory.cs @@ -23,19 +23,28 @@ namespace Umbraco.Cms.Core.Security { } - /// - /// - /// Returns a ClaimsIdentity that has the required claims, and allows flowing of claims from external identity - /// - public override async Task CreateAsync(BackOfficeIdentityUser user) - { - if (user == null) - { - throw new ArgumentNullException(nameof(user)); - } + protected virtual string AuthenticationType { get; } = Constants.Security.BackOfficeAuthenticationType; + /// + protected override async Task GenerateClaimsAsync(BackOfficeIdentityUser user) + { + // NOTE: Have a look at the base implementation https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Extensions.Core/src/UserClaimsPrincipalFactory.cs#L79 + // since it's setting an authentication type which is not what we want. + // so we override this method to change it. + + // get the base ClaimsIdentity baseIdentity = await base.GenerateClaimsAsync(user); + // now create a new one with the correct authentication type + var id = new ClaimsIdentity( + AuthenticationType, + Options.ClaimsIdentity.UserNameClaimType, + Options.ClaimsIdentity.RoleClaimType); + + // and merge all others from the base implementation + id.MergeAllClaims(baseIdentity); + + // ensure our required claims are there baseIdentity.AddRequiredClaims( user.Id, user.UserName, @@ -51,21 +60,7 @@ namespace Umbraco.Cms.Core.Security // assigned which could be done in the OnExternalLogin callback baseIdentity.MergeClaimsFromBackOfficeIdentity(user); - return new ClaimsPrincipal(baseIdentity); - } - - /// - protected override async Task GenerateClaimsAsync(BackOfficeIdentityUser user) - { - // TODO: Have a look at the base implementation https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Extensions.Core/src/UserClaimsPrincipalFactory.cs#L79 - // since it's setting an authentication type that is probably not what we want. - // also, this is the method that we should be returning our UmbracoBackOfficeIdentity from , not the method above, - // the method above just returns a principal that wraps the identity and we dont use a custom principal, - // see https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Extensions.Core/src/UserClaimsPrincipalFactory.cs#L66 - - ClaimsIdentity identity = await base.GenerateClaimsAsync(user); - - return identity; + return id; } } } diff --git a/src/Umbraco.Infrastructure/Security/ClaimsIdentityExtensions.cs b/src/Umbraco.Infrastructure/Security/ClaimsIdentityExtensions.cs index 1a37376070..d4b61a934d 100644 --- a/src/Umbraco.Infrastructure/Security/ClaimsIdentityExtensions.cs +++ b/src/Umbraco.Infrastructure/Security/ClaimsIdentityExtensions.cs @@ -13,6 +13,15 @@ namespace Umbraco.Extensions // is re-issued and we don't want to merge old values of these. private static readonly string[] s_ignoredClaims = new[] { ClaimTypes.CookiePath, Constants.Security.SessionIdClaimType }; + public static void MergeAllClaims(this ClaimsIdentity destination, ClaimsIdentity source) + { + foreach (Claim claim in source.Claims + .Where(claim => !destination.HasClaim(claim.Type, claim.Value))) + { + destination.AddClaim(new Claim(claim.Type, claim.Value)); + } + } + public static void MergeClaimsFromBackOfficeIdentity(this ClaimsIdentity destination, ClaimsIdentity source) { foreach (Claim claim in source.Claims diff --git a/src/Umbraco.Infrastructure/Security/MemberUserStore.cs b/src/Umbraco.Infrastructure/Security/MemberUserStore.cs index c0b9a19ef1..7e36081e73 100644 --- a/src/Umbraco.Infrastructure/Security/MemberUserStore.cs +++ b/src/Umbraco.Infrastructure/Security/MemberUserStore.cs @@ -19,7 +19,7 @@ namespace Umbraco.Cms.Core.Security /// /// A custom user store that uses Umbraco member data /// - public class MemberUserStore : UserStoreBase, IdentityUserRole, IdentityUserLogin, IdentityUserToken, IdentityRoleClaim> + public class MemberUserStore : UserStoreBase, IdentityUserRole, IdentityUserLogin, IdentityUserToken, IdentityRoleClaim> { private const string genericIdentityErrorCode = "IdentityErrorUserStore"; private readonly IMemberService _memberService; @@ -562,7 +562,7 @@ namespace Umbraco.Cms.Core.Security } /// - protected override Task FindRoleAsync(string roleName, CancellationToken cancellationToken) + protected override Task FindRoleAsync(string roleName, CancellationToken cancellationToken) { if (string.IsNullOrWhiteSpace(roleName)) { @@ -572,10 +572,10 @@ namespace Umbraco.Cms.Core.Security IMemberGroup group = _memberService.GetAllRoles().SingleOrDefault(x => x.Name == roleName); if (group == null) { - return Task.FromResult((IdentityRole)null); + return Task.FromResult((UmbracoIdentityRole)null); } - return Task.FromResult(new IdentityRole(group.Name) + return Task.FromResult(new UmbracoIdentityRole(group.Name) { //TODO: what should the alias be? Id = group.Id.ToString() diff --git a/src/Umbraco.Infrastructure/Security/UmbracoIdentityRole.cs b/src/Umbraco.Infrastructure/Security/UmbracoIdentityRole.cs index 9d06dcd037..00c4038287 100644 --- a/src/Umbraco.Infrastructure/Security/UmbracoIdentityRole.cs +++ b/src/Umbraco.Infrastructure/Security/UmbracoIdentityRole.cs @@ -10,6 +10,14 @@ namespace Umbraco.Cms.Core.Models.Identity private string _id; private string _name; + public UmbracoIdentityRole(string roleName) : base(roleName) + { + } + + public UmbracoIdentityRole() + { + } + public event PropertyChangedEventHandler PropertyChanged { add diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs index 86f96c7d71..bdcc5ec75b 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs @@ -1,7 +1,11 @@ +using System; +using System.Net; using System.Threading.Tasks; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Moq; @@ -15,26 +19,45 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security [TestFixture] public class MemberSignInManagerTests { - private Mock> _mockLogger; - private readonly Mock> _memberManager = MockUserManager(); - private readonly Mock _mockIpResolver = new Mock(); + private Mock>> _mockLogger; + private readonly Mock> _memberManager = MockUserManager(); + + public MemberClaimsPrincipalFactory CreateClaimsFactory(UserManager userMgr) + => new MemberClaimsPrincipalFactory(userMgr, Options.Create(new MemberIdentityOptions())); public MemberSignInManager CreateSut() { - _mockLogger = new Mock>(); + // This all needs to be setup because internally aspnet resolves a bunch + // of services from the HttpContext.RequestServices. + var serviceProviderFactory = new DefaultServiceProviderFactory(); + var serviceCollection = new ServiceCollection(); + serviceCollection + .AddLogging() + .AddAuthentication() + .AddCookie(IdentityConstants.ApplicationScheme); + IServiceProvider serviceProvider = serviceProviderFactory.CreateServiceProvider(serviceCollection); + var httpContextFactory = new DefaultHttpContextFactory(serviceProvider); + IFeatureCollection features = new DefaultHttpContext().Features; + features.Set(new HttpConnectionFeature + { + LocalIpAddress = IPAddress.Parse("127.0.0.1") + }); + HttpContext httpContext = httpContextFactory.Create(features); + + _mockLogger = new Mock>>(); return new MemberSignInManager( _memberManager.Object, - Mock.Of(), - Mock.Of>(), + Mock.Of(x => x.HttpContext == httpContext), + CreateClaimsFactory(_memberManager.Object), Mock.Of>(), - Mock.Of>>(), + _mockLogger.Object, Mock.Of(), Mock.Of>()); } - private static Mock> MockUserManager() where TUser : class + private static Mock> MockUserManager() { - var store = new Mock>(); - var mgr = new Mock>(store.Object, null, null, null, null, null, null, null, null); + var store = new Mock>(); + var mgr = new Mock>(store.Object, null, null, null, null, null, null, null, null); return mgr; } @@ -42,8 +65,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security public async Task WhenPasswordSignInAsyncIsCalled_AndEverythingIsSetup_ThenASignInResultSucceededShouldBeReturnedAsync() { //arrange - var userId = "bo8w3d32q9b98"; - _memberManager.Setup(x => x.GetUserIdAsync(It.IsAny())).ReturnsAsync(userId); + var userId = "bo8w3d32q9b98"; MemberSignInManager sut = CreateSut(); var fakeUser = new MemberIdentityUser(777) { @@ -52,6 +74,9 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security var password = "testPassword"; var lockoutOnFailure = false; var isPersistent = true; + + _memberManager.Setup(x => x.GetUserIdAsync(It.IsAny())).ReturnsAsync(userId); + _memberManager.Setup(x => x.GetUserNameAsync(It.IsAny())).ReturnsAsync(fakeUser.UserName); _memberManager.Setup(x => x.FindByNameAsync(It.IsAny())).ReturnsAsync(fakeUser); _memberManager.Setup(x => x.CheckPasswordAsync(fakeUser, password)).ReturnsAsync(true); _memberManager.Setup(x => x.IsEmailConfirmedAsync(fakeUser)).ReturnsAsync(true); @@ -76,14 +101,12 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security var password = "testPassword"; var lockoutOnFailure = false; var isPersistent = true; - _mockIpResolver.Setup(x => x.GetCurrentRequestIpAddress()).Returns("127.0.0.1"); //act SignInResult actual = await sut.PasswordSignInAsync(fakeUser, password, isPersistent, lockoutOnFailure); //assert Assert.IsFalse(actual.Succeeded); - //_mockLogger.Verify(x => x.LogInformation("Login attempt failed for username TestUser from IP address 127.0.0.1", null)); } } } diff --git a/src/Umbraco.Web.Common/DependencyInjection/ServiceCollectionExtensions.cs b/src/Umbraco.Web.Common/DependencyInjection/ServiceCollectionExtensions.cs index 5182db4e20..b970f3e551 100644 --- a/src/Umbraco.Web.Common/DependencyInjection/ServiceCollectionExtensions.cs +++ b/src/Umbraco.Web.Common/DependencyInjection/ServiceCollectionExtensions.cs @@ -12,6 +12,7 @@ using SixLabors.ImageSharp.Web.DependencyInjection; using SixLabors.ImageSharp.Web.Processors; using SixLabors.ImageSharp.Web.Providers; using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Models.Identity; using Umbraco.Cms.Core.Security; using Umbraco.Cms.Web.Common.Security; @@ -67,10 +68,11 @@ namespace Umbraco.Extensions services.BuildMembersIdentity() .AddDefaultTokenProviders() .AddMemberManager() + .AddClaimsPrincipalFactory() .AddUserStore() .AddRoleStore() - .AddRoleValidator>() - .AddRoleManager>(); + .AddRoleValidator>() + .AddRoleManager>(); private static MemberIdentityBuilder BuildMembersIdentity(this IServiceCollection services) { @@ -78,7 +80,7 @@ namespace Umbraco.Extensions services.TryAddScoped, UserValidator>(); services.TryAddScoped, PasswordValidator>(); services.TryAddScoped, PasswordHasher>(); - return new MemberIdentityBuilder(typeof(IdentityRole), services); + return new MemberIdentityBuilder(typeof(UmbracoIdentityRole), services); } private static void RemoveIntParamenterIfValueGreatherThen(IDictionary commands, string parameter, int maxValue) diff --git a/src/Umbraco.Web.Common/Security/MemberClaimsPrincipalFactory.cs b/src/Umbraco.Web.Common/Security/MemberClaimsPrincipalFactory.cs new file mode 100644 index 0000000000..fe9a0eadd4 --- /dev/null +++ b/src/Umbraco.Web.Common/Security/MemberClaimsPrincipalFactory.cs @@ -0,0 +1,15 @@ +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Options; +using Umbraco.Cms.Core.Security; + + +namespace Umbraco.Cms.Web.Common.Security +{ + public class MemberClaimsPrincipalFactory : UserClaimsPrincipalFactory + { + public MemberClaimsPrincipalFactory(UserManager userManager, IOptions optionsAccessor) + : base(userManager, optionsAccessor) + { + } + } +} diff --git a/src/Umbraco.Web.Common/Security/MemberManager.cs b/src/Umbraco.Web.Common/Security/MemberManager.cs index 195312a41e..f3b80ba4bc 100644 --- a/src/Umbraco.Web.Common/Security/MemberManager.cs +++ b/src/Umbraco.Web.Common/Security/MemberManager.cs @@ -10,6 +10,7 @@ using Umbraco.Cms.Core.Security; namespace Umbraco.Cms.Web.Common.Security { + public class MemberManager : UmbracoUserManager, IMemberManager { public MemberManager( diff --git a/src/Umbraco.Web.Common/Security/MemberSignInManager.cs b/src/Umbraco.Web.Common/Security/MemberSignInManager.cs index 392d20ebbe..eeec3c2899 100644 --- a/src/Umbraco.Web.Common/Security/MemberSignInManager.cs +++ b/src/Umbraco.Web.Common/Security/MemberSignInManager.cs @@ -38,27 +38,6 @@ namespace Umbraco.Cms.Web.Common.Security // use default scheme for members protected override string TwoFactorRememberMeAuthenticationType => IdentityConstants.TwoFactorRememberMeScheme; - /// - public override async Task PasswordSignInAsync(MemberIdentityUser user, string password, bool isPersistent, bool lockoutOnFailure) - { - // overridden to handle logging/events - SignInResult result = await base.PasswordSignInAsync(user, password, isPersistent, lockoutOnFailure); - return await HandleSignIn(user, user.UserName, result); - } - - /// - public override async Task PasswordSignInAsync(string userName, string password, bool isPersistent, bool lockoutOnFailure) - { - // overridden to handle logging/events - MemberIdentityUser user = await UserManager.FindByNameAsync(userName); - if (user == null) - { - return await HandleSignIn(null, userName, SignInResult.Failed); - } - - return await PasswordSignInAsync(user, password, isPersistent, lockoutOnFailure); - } - /// public override Task GetTwoFactorAuthenticationUserAsync() => throw new NotImplementedException("Two factor is not yet implemented for members"); diff --git a/src/Umbraco.Web.Common/Security/UmbracoSignInManager.cs b/src/Umbraco.Web.Common/Security/UmbracoSignInManager.cs new file mode 100644 index 0000000000..ea29098bef --- /dev/null +++ b/src/Umbraco.Web.Common/Security/UmbracoSignInManager.cs @@ -0,0 +1,453 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Security.Claims; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Umbraco.Cms.Core.Models.Identity; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Web.Common.Security +{ + /// + /// Abstract sign in manager implementation allowing modifying all defeault authentication schemes + /// + /// + public abstract class UmbracoSignInManager : SignInManager + where TUser : UmbracoIdentityUser + { + // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs + protected const string UmbracoSignInMgrLoginProviderKey = "LoginProvider"; + // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs + protected const string UmbracoSignInMgrXsrfKey = "XsrfId"; + + public UmbracoSignInManager(UserManager userManager, IHttpContextAccessor contextAccessor, IUserClaimsPrincipalFactory claimsFactory, IOptions optionsAccessor, ILogger> logger, IAuthenticationSchemeProvider schemes, IUserConfirmation confirmation) : base(userManager, contextAccessor, claimsFactory, optionsAccessor, logger, schemes, confirmation) + { + } + + protected abstract string AuthenticationType { get; } + protected abstract string ExternalAuthenticationType { get; } + protected abstract string TwoFactorAuthenticationType { get; } + protected abstract string TwoFactorRememberMeAuthenticationType { get; } + + /// + public override async Task PasswordSignInAsync(TUser user, string password, bool isPersistent, bool lockoutOnFailure) + { + // override to handle logging/events + var result = await base.PasswordSignInAsync(user, password, isPersistent, lockoutOnFailure); + return await HandleSignIn(user, user.UserName, result); + } + + /// + public override async Task GetExternalLoginInfoAsync(string expectedXsrf = null) + { + // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L422 + // to replace the auth scheme + + var auth = await Context.AuthenticateAsync(ExternalAuthenticationType); + var items = auth?.Properties?.Items; + if (auth?.Principal == null || items == null || !items.ContainsKey(UmbracoSignInMgrLoginProviderKey)) + { + return null; + } + + if (expectedXsrf != null) + { + if (!items.ContainsKey(UmbracoSignInMgrXsrfKey)) + { + return null; + } + var userId = items[UmbracoSignInMgrXsrfKey]; + if (userId != expectedXsrf) + { + return null; + } + } + + var providerKey = auth.Principal.FindFirstValue(ClaimTypes.NameIdentifier); + if (providerKey == null || items[UmbracoSignInMgrLoginProviderKey] is not string provider) + { + return null; + } + + var providerDisplayName = (await GetExternalAuthenticationSchemesAsync()).FirstOrDefault(p => p.Name == provider)?.DisplayName ?? provider; + return new ExternalLoginInfo(auth.Principal, provider, providerKey, providerDisplayName) + { + AuthenticationTokens = auth.Properties.GetTokens(), + AuthenticationProperties = auth.Properties + }; + } + + /// + public override async Task GetTwoFactorAuthenticationUserAsync() + { + // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs + // replaced in order to use a custom auth type + + var info = await RetrieveTwoFactorInfoAsync(); + if (info == null) + { + return null; + } + return await UserManager.FindByIdAsync(info.UserId); + } + + /// + public override async Task PasswordSignInAsync(string userName, string password, bool isPersistent, bool lockoutOnFailure) + { + // override to handle logging/events + var user = await UserManager.FindByNameAsync(userName); + if (user == null) + { + return await HandleSignIn(null, userName, SignInResult.Failed); + } + + return await PasswordSignInAsync(user, password, isPersistent, lockoutOnFailure); + } + + /// + public override bool IsSignedIn(ClaimsPrincipal principal) + { + // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L126 + // replaced in order to use a custom auth type + + if (principal == null) + { + throw new ArgumentNullException(nameof(principal)); + } + return principal?.Identities != null && + principal.Identities.Any(i => i.AuthenticationType == AuthenticationType); + } + + /// + public override async Task TwoFactorSignInAsync(string provider, string code, bool isPersistent, bool rememberClient) + { + // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L552 + // replaced in order to use a custom auth type and to implement logging/events + + var twoFactorInfo = await RetrieveTwoFactorInfoAsync(); + if (twoFactorInfo == null || twoFactorInfo.UserId == null) + { + return SignInResult.Failed; + } + var user = await UserManager.FindByIdAsync(twoFactorInfo.UserId); + if (user == null) + { + return SignInResult.Failed; + } + + var error = await PreSignInCheck(user); + if (error != null) + { + return error; + } + if (await UserManager.VerifyTwoFactorTokenAsync(user, provider, code)) + { + await DoTwoFactorSignInAsync(user, twoFactorInfo, isPersistent, rememberClient); + return await HandleSignIn(user, user?.UserName, SignInResult.Success); + } + // If the token is incorrect, record the failure which also may cause the user to be locked out + await UserManager.AccessFailedAsync(user); + return await HandleSignIn(user, user?.UserName, SignInResult.Failed); + } + + /// + public override async Task RefreshSignInAsync(TUser user) + { + // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L126 + // replaced in order to use a custom auth type + + var auth = await Context.AuthenticateAsync(AuthenticationType); + IList claims = Array.Empty(); + + var authenticationMethod = auth?.Principal?.FindFirst(ClaimTypes.AuthenticationMethod); + var amr = auth?.Principal?.FindFirst("amr"); + + if (authenticationMethod != null || amr != null) + { + claims = new List(); + if (authenticationMethod != null) + { + claims.Add(authenticationMethod); + } + if (amr != null) + { + claims.Add(amr); + } + } + + await SignInWithClaimsAsync(user, auth?.Properties, claims); + } + + /// + public override async Task SignInWithClaimsAsync(TUser user, AuthenticationProperties authenticationProperties, IEnumerable additionalClaims) + { + // override to replace IdentityConstants.ApplicationScheme with custom AuthenticationType + // code taken from aspnetcore: https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs + // we also override to set the current HttpContext principal since this isn't done by default + + var userPrincipal = await CreateUserPrincipalAsync(user); + foreach (var claim in additionalClaims) + { + userPrincipal.Identities.First().AddClaim(claim); + } + + // FYI (just for informational purposes): + // This calls an ext method will eventually reaches `IAuthenticationService.SignInAsync` + // which then resolves the `IAuthenticationSignInHandler` for the current scheme + // by calling `IAuthenticationHandlerProvider.GetHandlerAsync(context, scheme);` + // which then calls `IAuthenticationSignInHandler.SignInAsync` = CookieAuthenticationHandler.HandleSignInAsync + + // Also note, that when the CookieAuthenticationHandler sign in is successful we handle that event within our + // own ConfigureUmbracoBackOfficeCookieOptions which assigns the current HttpContext.User to the IPrincipal created + + // Also note, this method gets called when performing 2FA logins + + await Context.SignInAsync( + AuthenticationType, + userPrincipal, + authenticationProperties ?? new AuthenticationProperties()); + } + + /// + public override async Task SignOutAsync() + { + // override to replace IdentityConstants.ApplicationScheme with custom auth types + // code taken from aspnetcore: https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs + + await Context.SignOutAsync(AuthenticationType); + await Context.SignOutAsync(ExternalAuthenticationType); + await Context.SignOutAsync(TwoFactorAuthenticationType); + } + + /// + public override async Task IsTwoFactorClientRememberedAsync(TUser user) + { + // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L422 + // to replace the auth scheme + + var userId = await UserManager.GetUserIdAsync(user); + var result = await Context.AuthenticateAsync(TwoFactorRememberMeAuthenticationType); + return (result?.Principal != null && result.Principal.FindFirstValue(ClaimTypes.Name) == userId); + } + + /// + public override async Task RememberTwoFactorClientAsync(TUser user) + { + // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L422 + // to replace the auth scheme + + var principal = await StoreRememberClient(user); + await Context.SignInAsync(TwoFactorRememberMeAuthenticationType, + principal, + new AuthenticationProperties { IsPersistent = true }); + } + + /// + public override async Task TwoFactorRecoveryCodeSignInAsync(string recoveryCode) + { + // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L422 + // to replace the auth scheme + + var twoFactorInfo = await RetrieveTwoFactorInfoAsync(); + if (twoFactorInfo == null || twoFactorInfo.UserId == null) + { + return SignInResult.Failed; + } + var user = await UserManager.FindByIdAsync(twoFactorInfo.UserId); + if (user == null) + { + return SignInResult.Failed; + } + + var result = await UserManager.RedeemTwoFactorRecoveryCodeAsync(user, recoveryCode); + if (result.Succeeded) + { + await DoTwoFactorSignInAsync(user, twoFactorInfo, isPersistent: false, rememberClient: false); + return SignInResult.Success; + } + + // We don't protect against brute force attacks since codes are expected to be random. + return SignInResult.Failed; + } + + /// + public override Task ForgetTwoFactorClientAsync() + { + // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L422 + // to replace the auth scheme + + return Context.SignOutAsync(TwoFactorRememberMeAuthenticationType); + } + + /// + /// Called on any login attempt to update the AccessFailedCount and to raise events + /// + /// + /// + /// + /// + protected virtual async Task HandleSignIn(TUser user, string username, SignInResult result) + { + // TODO: Here I believe we can do all (or most) of the usermanager event raising so that it is not in the AuthenticationController + + if (username.IsNullOrWhiteSpace()) + { + username = "UNKNOWN"; // could happen in 2fa or something else weird + } + + if (result.Succeeded) + { + //track the last login date + user.LastLoginDateUtc = DateTime.UtcNow; + if (user.AccessFailedCount > 0) + { + //we have successfully logged in, reset the AccessFailedCount + user.AccessFailedCount = 0; + } + await UserManager.UpdateAsync(user); + + Logger.LogInformation("User: {UserName} logged in from IP address {IpAddress}", username, Context.Connection.RemoteIpAddress); + } + else if (result.IsLockedOut) + { + Logger.LogInformation("Login attempt failed for username {UserName} from IP address {IpAddress}, the user is locked", username, Context.Connection.RemoteIpAddress); + } + else if (result.RequiresTwoFactor) + { + Logger.LogInformation("Login attempt requires verification for username {UserName} from IP address {IpAddress}", username, Context.Connection.RemoteIpAddress); + } + else if (!result.Succeeded || result.IsNotAllowed) + { + Logger.LogInformation("Login attempt failed for username {UserName} from IP address {IpAddress}", username, Context.Connection.RemoteIpAddress); + } + else + { + throw new ArgumentOutOfRangeException(); + } + + return result; + } + + /// + protected override async Task SignInOrTwoFactorAsync(TUser user, bool isPersistent, string loginProvider = null, bool bypassTwoFactor = false) + { + // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs + // to replace custom auth types + + if (!bypassTwoFactor && await IsTfaEnabled(user)) + { + if (!await IsTwoFactorClientRememberedAsync(user)) + { + // Store the userId for use after two factor check + var userId = await UserManager.GetUserIdAsync(user); + await Context.SignInAsync(IdentityConstants.TwoFactorUserIdScheme, StoreTwoFactorInfo(userId, loginProvider)); + return SignInResult.TwoFactorRequired; + } + } + // Cleanup external cookie + if (loginProvider != null) + { + await Context.SignOutAsync(ExternalAuthenticationType); + } + if (loginProvider == null) + { + await SignInWithClaimsAsync(user, isPersistent, new Claim[] { new Claim("amr", "pwd") }); + } + else + { + await SignInAsync(user, isPersistent, loginProvider); + } + return SignInResult.Success; + } + + // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L782 + // since it's not public + private async Task IsTfaEnabled(TUser user) + => UserManager.SupportsUserTwoFactor && + await UserManager.GetTwoFactorEnabledAsync(user) && + (await UserManager.GetValidTwoFactorProvidersAsync(user)).Count > 0; + + // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L743 + // to replace custom auth types + private ClaimsPrincipal StoreTwoFactorInfo(string userId, string loginProvider) + { + var identity = new ClaimsIdentity(TwoFactorAuthenticationType); + identity.AddClaim(new Claim(ClaimTypes.Name, userId)); + if (loginProvider != null) + { + identity.AddClaim(new Claim(ClaimTypes.AuthenticationMethod, loginProvider)); + } + return new ClaimsPrincipal(identity); + } + + // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs + // copy is required in order to use custom auth types + private async Task StoreRememberClient(TUser user) + { + var userId = await UserManager.GetUserIdAsync(user); + var rememberBrowserIdentity = new ClaimsIdentity(TwoFactorRememberMeAuthenticationType); + rememberBrowserIdentity.AddClaim(new Claim(ClaimTypes.Name, userId)); + if (UserManager.SupportsUserSecurityStamp) + { + var stamp = await UserManager.GetSecurityStampAsync(user); + rememberBrowserIdentity.AddClaim(new Claim(Options.ClaimsIdentity.SecurityStampClaimType, stamp)); + } + return new ClaimsPrincipal(rememberBrowserIdentity); + } + + // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs + // copy is required in order to use a custom auth type + private async Task RetrieveTwoFactorInfoAsync() + { + var result = await Context.AuthenticateAsync(TwoFactorAuthenticationType); + if (result?.Principal != null) + { + return new TwoFactorAuthenticationInfo + { + UserId = result.Principal.FindFirstValue(ClaimTypes.Name), + LoginProvider = result.Principal.FindFirstValue(ClaimTypes.AuthenticationMethod) + }; + } + return null; + } + + // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs + // copy is required in order to use custom auth types + private async Task DoTwoFactorSignInAsync(TUser user, TwoFactorAuthenticationInfo twoFactorInfo, bool isPersistent, bool rememberClient) + { + // When token is verified correctly, clear the access failed count used for lockout + await ResetLockout(user); + + var claims = new List + { + new Claim("amr", "mfa") + }; + + // Cleanup external cookie + if (twoFactorInfo.LoginProvider != null) + { + claims.Add(new Claim(ClaimTypes.AuthenticationMethod, twoFactorInfo.LoginProvider)); + await Context.SignOutAsync(ExternalAuthenticationType); + } + // Cleanup two factor user id cookie + await Context.SignOutAsync(TwoFactorAuthenticationType); + if (rememberClient) + { + await RememberTwoFactorClientAsync(user); + } + await SignInWithClaimsAsync(user, isPersistent, claims); + } + + // borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs#L891 + private class TwoFactorAuthenticationInfo + { + public string UserId { get; set; } + public string LoginProvider { get; set; } + } + } +} From 9fac0a147018a0e546616eb3caa15082a30c0c13 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 29 Mar 2021 18:05:13 +1100 Subject: [PATCH 20/32] Fixes tests --- .../Security/BackOfficeClaimsPrincipalFactory.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Infrastructure/Security/BackOfficeClaimsPrincipalFactory.cs b/src/Umbraco.Infrastructure/Security/BackOfficeClaimsPrincipalFactory.cs index 9c6aeca80e..c86bda44e0 100644 --- a/src/Umbraco.Infrastructure/Security/BackOfficeClaimsPrincipalFactory.cs +++ b/src/Umbraco.Infrastructure/Security/BackOfficeClaimsPrincipalFactory.cs @@ -45,7 +45,7 @@ namespace Umbraco.Cms.Core.Security id.MergeAllClaims(baseIdentity); // ensure our required claims are there - baseIdentity.AddRequiredClaims( + id.AddRequiredClaims( user.Id, user.UserName, user.Name, @@ -58,7 +58,7 @@ namespace Umbraco.Cms.Core.Security // now we can flow any custom claims that the actual user has currently // assigned which could be done in the OnExternalLogin callback - baseIdentity.MergeClaimsFromBackOfficeIdentity(user); + id.MergeClaimsFromBackOfficeIdentity(user); return id; } From 48a4620e1bec0a40f802c4b7fb926fa115da1fa2 Mon Sep 17 00:00:00 2001 From: Mole Date: Mon, 29 Mar 2021 14:39:02 +0200 Subject: [PATCH 21/32] Add macro notifications --- .../Notifications/MacroDeletedNotification.cs | 12 ++++++++++++ .../Notifications/MacroDeletingNotification.cs | 17 +++++++++++++++++ .../Notifications/MacroSavedNotification.cs | 17 +++++++++++++++++ .../Notifications/MacroSavingNotification.cs | 17 +++++++++++++++++ 4 files changed, 63 insertions(+) create mode 100644 src/Umbraco.Infrastructure/Services/Notifications/MacroDeletedNotification.cs create mode 100644 src/Umbraco.Infrastructure/Services/Notifications/MacroDeletingNotification.cs create mode 100644 src/Umbraco.Infrastructure/Services/Notifications/MacroSavedNotification.cs create mode 100644 src/Umbraco.Infrastructure/Services/Notifications/MacroSavingNotification.cs diff --git a/src/Umbraco.Infrastructure/Services/Notifications/MacroDeletedNotification.cs b/src/Umbraco.Infrastructure/Services/Notifications/MacroDeletedNotification.cs new file mode 100644 index 0000000000..56e0ea759b --- /dev/null +++ b/src/Umbraco.Infrastructure/Services/Notifications/MacroDeletedNotification.cs @@ -0,0 +1,12 @@ +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models; + +namespace Umbraco.Cms.Infrastructure.Services.Notifications +{ + public class MacroDeletedNotification : DeletedNotification + { + public MacroDeletedNotification(IMacro target, EventMessages messages) : base(target, messages) + { + } + } +} diff --git a/src/Umbraco.Infrastructure/Services/Notifications/MacroDeletingNotification.cs b/src/Umbraco.Infrastructure/Services/Notifications/MacroDeletingNotification.cs new file mode 100644 index 0000000000..fbf560cb77 --- /dev/null +++ b/src/Umbraco.Infrastructure/Services/Notifications/MacroDeletingNotification.cs @@ -0,0 +1,17 @@ +using System.Collections.Generic; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models; + +namespace Umbraco.Cms.Infrastructure.Services.Notifications +{ + public class MacroDeletingNotification : DeletingNotification + { + public MacroDeletingNotification(IMacro target, EventMessages messages) : base(target, messages) + { + } + + public MacroDeletingNotification(IEnumerable target, EventMessages messages) : base(target, messages) + { + } + } +} diff --git a/src/Umbraco.Infrastructure/Services/Notifications/MacroSavedNotification.cs b/src/Umbraco.Infrastructure/Services/Notifications/MacroSavedNotification.cs new file mode 100644 index 0000000000..e93b8e3cec --- /dev/null +++ b/src/Umbraco.Infrastructure/Services/Notifications/MacroSavedNotification.cs @@ -0,0 +1,17 @@ +using System.Collections.Generic; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models; + +namespace Umbraco.Cms.Infrastructure.Services.Notifications +{ + public class MacroSavedNotification : SavedNotification + { + public MacroSavedNotification(IMacro target, EventMessages messages) : base(target, messages) + { + } + + public MacroSavedNotification(IEnumerable target, EventMessages messages) : base(target, messages) + { + } + } +} diff --git a/src/Umbraco.Infrastructure/Services/Notifications/MacroSavingNotification.cs b/src/Umbraco.Infrastructure/Services/Notifications/MacroSavingNotification.cs new file mode 100644 index 0000000000..689272620c --- /dev/null +++ b/src/Umbraco.Infrastructure/Services/Notifications/MacroSavingNotification.cs @@ -0,0 +1,17 @@ +using System.Collections.Generic; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models; + +namespace Umbraco.Cms.Infrastructure.Services.Notifications +{ + public class MacroSavingNotification : SavingNotification + { + public MacroSavingNotification(IMacro target, EventMessages messages) : base(target, messages) + { + } + + public MacroSavingNotification(IEnumerable target, EventMessages messages) : base(target, messages) + { + } + } +} From f34558914987e9139acea31c1c729f5a1b667908 Mon Sep 17 00:00:00 2001 From: Mole Date: Mon, 29 Mar 2021 15:09:26 +0200 Subject: [PATCH 22/32] Switch MacroService to use Notifications and make it internal. --- .../Cache/DistributedCacheBinder_Handlers.cs | 22 ++++----- .../Compose/NotificationsComposer.cs | 4 +- .../Services/Implement/MacroService.cs | 48 ++++++------------- .../Cache/DistributedCacheBinderTests.cs | 3 -- 4 files changed, 29 insertions(+), 48 deletions(-) diff --git a/src/Umbraco.Infrastructure/Cache/DistributedCacheBinder_Handlers.cs b/src/Umbraco.Infrastructure/Cache/DistributedCacheBinder_Handlers.cs index 0782579497..c35942b311 100644 --- a/src/Umbraco.Infrastructure/Cache/DistributedCacheBinder_Handlers.cs +++ b/src/Umbraco.Infrastructure/Cache/DistributedCacheBinder_Handlers.cs @@ -33,7 +33,9 @@ namespace Umbraco.Cms.Core.Cache INotificationHandler, INotificationHandler, INotificationHandler, - INotificationHandler + INotificationHandler, + INotificationHandler, + INotificationHandler { private List _unbinders; @@ -96,12 +98,6 @@ namespace Umbraco.Cms.Core.Cache Bind(() => FileService.DeletedTemplate += FileService_DeletedTemplate, () => FileService.DeletedTemplate -= FileService_DeletedTemplate); - // bind to macro events - Bind(() => MacroService.Saved += MacroService_Saved, - () => MacroService.Saved -= MacroService_Saved); - Bind(() => MacroService.Deleted += MacroService_Deleted, - () => MacroService.Deleted -= MacroService_Deleted); - // bind to media events - handles all media changes Bind(() => MediaService.TreeChanged += MediaService_TreeChanged, () => MediaService.TreeChanged -= MediaService_TreeChanged); @@ -344,16 +340,20 @@ namespace Umbraco.Cms.Core.Cache #region MacroService - private void MacroService_Deleted(IMacroService sender, DeleteEventArgs e) + public void Handle(MacroDeletedNotification notification) { - foreach (var entity in e.DeletedEntities) + foreach (IMacro entity in notification.DeletedEntities) + { _distributedCache.RemoveMacroCache(entity); + } } - private void MacroService_Saved(IMacroService sender, SaveEventArgs e) + public void Handle(MacroSavedNotification notification) { - foreach (var entity in e.SavedEntities) + foreach (IMacro entity in notification.SavedEntities) + { _distributedCache.RefreshMacroCache(entity); + } } #endregion diff --git a/src/Umbraco.Infrastructure/Compose/NotificationsComposer.cs b/src/Umbraco.Infrastructure/Compose/NotificationsComposer.cs index be6539ac55..e03e10aa81 100644 --- a/src/Umbraco.Infrastructure/Compose/NotificationsComposer.cs +++ b/src/Umbraco.Infrastructure/Compose/NotificationsComposer.cs @@ -78,7 +78,9 @@ namespace Umbraco.Cms.Core.Compose .AddNotificationHandler() .AddNotificationHandler() .AddNotificationHandler() - .AddNotificationHandler(); + .AddNotificationHandler() + .AddNotificationHandler() + .AddNotificationHandler(); // add notification handlers for auditing builder diff --git a/src/Umbraco.Infrastructure/Services/Implement/MacroService.cs b/src/Umbraco.Infrastructure/Services/Implement/MacroService.cs index c42d29b3c0..68192218eb 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/MacroService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/MacroService.cs @@ -6,13 +6,14 @@ using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Persistence.Repositories; using Umbraco.Cms.Core.Scoping; +using Umbraco.Cms.Infrastructure.Services.Notifications; namespace Umbraco.Cms.Core.Services.Implement { /// /// Represents the Macro Service, which is an easy access to operations involving /// - public class MacroService : RepositoryService, IMacroService + internal class MacroService : RepositoryService, IMacroService { private readonly IMacroRepository _macroRepository; private readonly IAuditRepository _auditRepository; @@ -83,18 +84,19 @@ namespace Umbraco.Cms.Core.Services.Implement /// Optional id of the user deleting the macro public void Delete(IMacro macro, int userId = Cms.Core.Constants.Security.SuperUserId) { - using (var scope = ScopeProvider.CreateScope()) + using (IScope scope = ScopeProvider.CreateScope()) { - var deleteEventArgs = new DeleteEventArgs(macro); - if (scope.Events.DispatchCancelable(Deleting, this, deleteEventArgs)) + EventMessages eventMessages = EventMessagesFactory.Get(); + var deletingNotification = new MacroDeletingNotification(macro, eventMessages); + if (scope.Notifications.PublishCancelable(deletingNotification)) { scope.Complete(); return; } _macroRepository.Delete(macro); - deleteEventArgs.CanCancel = false; - scope.Events.Dispatch(Deleted, this, deleteEventArgs); + + scope.Notifications.Publish(new MacroDeletedNotification(macro, eventMessages).WithStateFrom(deletingNotification)); Audit(AuditType.Delete, userId, -1); scope.Complete(); @@ -108,10 +110,12 @@ namespace Umbraco.Cms.Core.Services.Implement /// Optional Id of the user deleting the macro public void Save(IMacro macro, int userId = Cms.Core.Constants.Security.SuperUserId) { - using (var scope = ScopeProvider.CreateScope()) + using (IScope scope = ScopeProvider.CreateScope()) { - var saveEventArgs = new SaveEventArgs(macro); - if (scope.Events.DispatchCancelable(Saving, this, saveEventArgs)) + EventMessages eventMessages = EventMessagesFactory.Get(); + var savingNotification = new MacroSavingNotification(macro, eventMessages); + + if (scope.Notifications.PublishCancelable(savingNotification)) { scope.Complete(); return; @@ -123,8 +127,8 @@ namespace Umbraco.Cms.Core.Services.Implement } _macroRepository.Save(macro); - saveEventArgs.CanCancel = false; - scope.Events.Dispatch(Saved, this, saveEventArgs); + + scope.Notifications.Publish(new MacroSavedNotification(macro, eventMessages).WithStateFrom(savingNotification)); Audit(AuditType.Save, userId, -1); scope.Complete(); @@ -154,27 +158,5 @@ namespace Umbraco.Cms.Core.Services.Implement { _auditRepository.Save(new AuditItem(objectId, type, userId, "Macro")); } - - #region Event Handlers - /// - /// Occurs before Delete - /// - public static event TypedEventHandler> Deleting; - - /// - /// Occurs after Delete - /// - public static event TypedEventHandler> Deleted; - - /// - /// Occurs before Save - /// - public static event TypedEventHandler> Saving; - - /// - /// Occurs after Save - /// - public static event TypedEventHandler> Saved; - #endregion } } diff --git a/src/Umbraco.Tests.Integration/Cache/DistributedCacheBinderTests.cs b/src/Umbraco.Tests.Integration/Cache/DistributedCacheBinderTests.cs index 2d9bcf2c9d..6b6b55ee6a 100644 --- a/src/Umbraco.Tests.Integration/Cache/DistributedCacheBinderTests.cs +++ b/src/Umbraco.Tests.Integration/Cache/DistributedCacheBinderTests.cs @@ -62,9 +62,6 @@ namespace Umbraco.Cms.Tests.Integration.Cache new EventDefinition>(null, FileService, new SaveEventArgs(Enumerable.Empty())), new EventDefinition>(null, FileService, new DeleteEventArgs(Enumerable.Empty())), - new EventDefinition>(null, MacroService, new SaveEventArgs(Enumerable.Empty())), - new EventDefinition>(null, MacroService, new DeleteEventArgs(Enumerable.Empty())), - // not managed //new EventDefinition>(null, ContentService, new SaveEventArgs(Enumerable.Empty()), "SavedBlueprint"), //new EventDefinition>(null, ContentService, new DeleteEventArgs(Enumerable.Empty()), "DeletedBlueprint"), From c881fa9e7d08c11954e18489827f70cdafceb947 Mon Sep 17 00:00:00 2001 From: Mads Rasmussen Date: Tue, 30 Mar 2021 17:03:56 +0200 Subject: [PATCH 23/32] Fixes tabbing-mode remains active after closing modal #9790 (#10074) --- .../components/forms/umbfocuslock.directive.js | 4 ---- .../src/less/application/umb-outline.less | 1 + src/Umbraco.Web.UI.Client/src/less/forms.less | 9 ++++++++- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/forms/umbfocuslock.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/forms/umbfocuslock.directive.js index 569f49b88a..f7cd32217e 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/forms/umbfocuslock.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/forms/umbfocuslock.directive.js @@ -29,10 +29,6 @@ var defaultFocusedElement = getAutoFocusElement(focusableElements); var firstFocusableElement = focusableElements[0]; var lastFocusableElement = focusableElements[focusableElements.length -1]; - - // We need to add the tabbing-active class in order to highlight the focused button since the default style is - // outline: none; set in the stylesheet specifically - bodyElement.classList.add('tabbing-active'); // If there is no default focused element put focus on the first focusable element in the nodelist if(defaultFocusedElement === null ){ diff --git a/src/Umbraco.Web.UI.Client/src/less/application/umb-outline.less b/src/Umbraco.Web.UI.Client/src/less/application/umb-outline.less index 939366d5ac..1f1c2c0e72 100644 --- a/src/Umbraco.Web.UI.Client/src/less/application/umb-outline.less +++ b/src/Umbraco.Web.UI.Client/src/less/application/umb-outline.less @@ -15,6 +15,7 @@ right: 0; border-radius: 3px; box-shadow: 0 0 2px 0px @ui-outline, inset 0 0 2px 2px @ui-outline; + pointer-events: none; } } diff --git a/src/Umbraco.Web.UI.Client/src/less/forms.less b/src/Umbraco.Web.UI.Client/src/less/forms.less index 90b2dbe37e..17c62037cc 100644 --- a/src/Umbraco.Web.UI.Client/src/less/forms.less +++ b/src/Umbraco.Web.UI.Client/src/less/forms.less @@ -308,7 +308,14 @@ select[size] { input[type="file"], input[type="radio"], input[type="checkbox"] { - .umb-outline(); + &:focus { + border-color: @inputBorderFocus; + outline: 0; + + .tabbing-active & { + outline: 2px solid @ui-outline; + } + } } From 15923aafc7723f1823a045b7e635cfb4cffbd786 Mon Sep 17 00:00:00 2001 From: Mads Rasmussen Date: Tue, 30 Mar 2021 17:03:56 +0200 Subject: [PATCH 24/32] Fixes tabbing-mode remains active after closing modal #9790 (#10074) (cherry picked from commit c881fa9e7d08c11954e18489827f70cdafceb947) --- .../components/forms/umbfocuslock.directive.js | 4 ---- .../src/less/application/umb-outline.less | 1 + src/Umbraco.Web.UI.Client/src/less/forms.less | 9 ++++++++- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/forms/umbfocuslock.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/forms/umbfocuslock.directive.js index 569f49b88a..f7cd32217e 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/forms/umbfocuslock.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/forms/umbfocuslock.directive.js @@ -29,10 +29,6 @@ var defaultFocusedElement = getAutoFocusElement(focusableElements); var firstFocusableElement = focusableElements[0]; var lastFocusableElement = focusableElements[focusableElements.length -1]; - - // We need to add the tabbing-active class in order to highlight the focused button since the default style is - // outline: none; set in the stylesheet specifically - bodyElement.classList.add('tabbing-active'); // If there is no default focused element put focus on the first focusable element in the nodelist if(defaultFocusedElement === null ){ diff --git a/src/Umbraco.Web.UI.Client/src/less/application/umb-outline.less b/src/Umbraco.Web.UI.Client/src/less/application/umb-outline.less index 939366d5ac..1f1c2c0e72 100644 --- a/src/Umbraco.Web.UI.Client/src/less/application/umb-outline.less +++ b/src/Umbraco.Web.UI.Client/src/less/application/umb-outline.less @@ -15,6 +15,7 @@ right: 0; border-radius: 3px; box-shadow: 0 0 2px 0px @ui-outline, inset 0 0 2px 2px @ui-outline; + pointer-events: none; } } diff --git a/src/Umbraco.Web.UI.Client/src/less/forms.less b/src/Umbraco.Web.UI.Client/src/less/forms.less index 90b2dbe37e..17c62037cc 100644 --- a/src/Umbraco.Web.UI.Client/src/less/forms.less +++ b/src/Umbraco.Web.UI.Client/src/less/forms.less @@ -308,7 +308,14 @@ select[size] { input[type="file"], input[type="radio"], input[type="checkbox"] { - .umb-outline(); + &:focus { + border-color: @inputBorderFocus; + outline: 0; + + .tabbing-active & { + outline: 2px solid @ui-outline; + } + } } From addf3326dc47c28f36302c9be8b561998258a459 Mon Sep 17 00:00:00 2001 From: Mole Date: Wed, 31 Mar 2021 13:19:00 +0200 Subject: [PATCH 25/32] Add notification classes --- .../DomainDeletedNotification.cs | 15 ++++++++++++++ .../DomainDeletingNotification.cs | 20 +++++++++++++++++++ .../Notifications/DomainSavedNotification.cs | 20 +++++++++++++++++++ .../Notifications/DomainSavingNotification.cs | 20 +++++++++++++++++++ 4 files changed, 75 insertions(+) create mode 100644 src/Umbraco.Infrastructure/Services/Notifications/DomainDeletedNotification.cs create mode 100644 src/Umbraco.Infrastructure/Services/Notifications/DomainDeletingNotification.cs create mode 100644 src/Umbraco.Infrastructure/Services/Notifications/DomainSavedNotification.cs create mode 100644 src/Umbraco.Infrastructure/Services/Notifications/DomainSavingNotification.cs diff --git a/src/Umbraco.Infrastructure/Services/Notifications/DomainDeletedNotification.cs b/src/Umbraco.Infrastructure/Services/Notifications/DomainDeletedNotification.cs new file mode 100644 index 0000000000..3210b5cc24 --- /dev/null +++ b/src/Umbraco.Infrastructure/Services/Notifications/DomainDeletedNotification.cs @@ -0,0 +1,15 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models; + +namespace Umbraco.Cms.Infrastructure.Services.Notifications +{ + public class DomainDeletedNotification : DeletedNotification + { + public DomainDeletedNotification(IDomain target, EventMessages messages) : base(target, messages) + { + } + } +} diff --git a/src/Umbraco.Infrastructure/Services/Notifications/DomainDeletingNotification.cs b/src/Umbraco.Infrastructure/Services/Notifications/DomainDeletingNotification.cs new file mode 100644 index 0000000000..b16a35ee14 --- /dev/null +++ b/src/Umbraco.Infrastructure/Services/Notifications/DomainDeletingNotification.cs @@ -0,0 +1,20 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using System.Collections.Generic; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models; + +namespace Umbraco.Cms.Infrastructure.Services.Notifications +{ + public class DomainDeletingNotification : DeletingNotification + { + public DomainDeletingNotification(IDomain target, EventMessages messages) : base(target, messages) + { + } + + public DomainDeletingNotification(IEnumerable target, EventMessages messages) : base(target, messages) + { + } + } +} diff --git a/src/Umbraco.Infrastructure/Services/Notifications/DomainSavedNotification.cs b/src/Umbraco.Infrastructure/Services/Notifications/DomainSavedNotification.cs new file mode 100644 index 0000000000..6cfaa2304c --- /dev/null +++ b/src/Umbraco.Infrastructure/Services/Notifications/DomainSavedNotification.cs @@ -0,0 +1,20 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using System.Collections.Generic; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models; + +namespace Umbraco.Cms.Infrastructure.Services.Notifications +{ + public class DomainSavedNotification : SavedNotification + { + public DomainSavedNotification(IDomain target, EventMessages messages) : base(target, messages) + { + } + + public DomainSavedNotification(IEnumerable target, EventMessages messages) : base(target, messages) + { + } + } +} diff --git a/src/Umbraco.Infrastructure/Services/Notifications/DomainSavingNotification.cs b/src/Umbraco.Infrastructure/Services/Notifications/DomainSavingNotification.cs new file mode 100644 index 0000000000..2689082314 --- /dev/null +++ b/src/Umbraco.Infrastructure/Services/Notifications/DomainSavingNotification.cs @@ -0,0 +1,20 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using System.Collections.Generic; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models; + +namespace Umbraco.Cms.Infrastructure.Services.Notifications +{ + public class DomainSavingNotification : SavingNotification + { + public DomainSavingNotification(IDomain target, EventMessages messages) : base(target, messages) + { + } + + public DomainSavingNotification(IEnumerable target, EventMessages messages) : base(target, messages) + { + } + } +} From e651742887f78d4f084bf6be71e4c7cfba648f21 Mon Sep 17 00:00:00 2001 From: Mole Date: Wed, 31 Mar 2021 13:26:31 +0200 Subject: [PATCH 26/32] Migrate events --- .../Cache/DistributedCacheBinder_Handlers.cs | 22 ++++---- .../Compose/NotificationsComposer.cs | 4 +- .../Services/Implement/DomainService.cs | 55 +++++-------------- .../Cache/DistributedCacheBinderTests.cs | 3 - 4 files changed, 29 insertions(+), 55 deletions(-) diff --git a/src/Umbraco.Infrastructure/Cache/DistributedCacheBinder_Handlers.cs b/src/Umbraco.Infrastructure/Cache/DistributedCacheBinder_Handlers.cs index 41195be9fe..a779e5adc4 100644 --- a/src/Umbraco.Infrastructure/Cache/DistributedCacheBinder_Handlers.cs +++ b/src/Umbraco.Infrastructure/Cache/DistributedCacheBinder_Handlers.cs @@ -35,7 +35,9 @@ namespace Umbraco.Cms.Core.Cache INotificationHandler, INotificationHandler, INotificationHandler, - INotificationHandler + INotificationHandler, + INotificationHandler, + INotificationHandler { private List _unbinders; @@ -72,12 +74,6 @@ namespace Umbraco.Cms.Core.Cache Bind(() => FileService.DeletedStylesheet += FileService_DeletedStylesheet, () => FileService.DeletedStylesheet -= FileService_DeletedStylesheet); - // bind to domain events - Bind(() => DomainService.Saved += DomainService_Saved, - () => DomainService.Saved -= DomainService_Saved); - Bind(() => DomainService.Deleted += DomainService_Deleted, - () => DomainService.Deleted -= DomainService_Deleted); - // bind to content type events Bind(() => ContentTypeService.Changed += ContentTypeService_Changed, () => ContentTypeService.Changed -= ContentTypeService_Changed); @@ -207,16 +203,20 @@ namespace Umbraco.Cms.Core.Cache #region DomainService - private void DomainService_Saved(IDomainService sender, SaveEventArgs e) + public void Handle(DomainSavedNotification notification) { - foreach (var entity in e.SavedEntities) + foreach (IDomain entity in notification.SavedEntities) + { _distributedCache.RefreshDomainCache(entity); + } } - private void DomainService_Deleted(IDomainService sender, DeleteEventArgs e) + public void Handle(DomainDeletedNotification notification) { - foreach (var entity in e.DeletedEntities) + foreach (IDomain entity in notification.DeletedEntities) + { _distributedCache.RemoveDomainCache(entity); + } } #endregion diff --git a/src/Umbraco.Infrastructure/Compose/NotificationsComposer.cs b/src/Umbraco.Infrastructure/Compose/NotificationsComposer.cs index e9e7bf30af..f5c9a9dbfc 100644 --- a/src/Umbraco.Infrastructure/Compose/NotificationsComposer.cs +++ b/src/Umbraco.Infrastructure/Compose/NotificationsComposer.cs @@ -82,7 +82,9 @@ namespace Umbraco.Cms.Core.Compose .AddNotificationHandler() .AddNotificationHandler() .AddNotificationHandler() - .AddNotificationHandler(); + .AddNotificationHandler() + .AddNotificationHandler() + .AddNotificationHandler(); // add notification handlers for auditing builder diff --git a/src/Umbraco.Infrastructure/Services/Implement/DomainService.cs b/src/Umbraco.Infrastructure/Services/Implement/DomainService.cs index 2b7d964a13..0540c04d64 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/DomainService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/DomainService.cs @@ -4,6 +4,7 @@ using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Persistence.Repositories; using Umbraco.Cms.Core.Scoping; +using Umbraco.Cms.Infrastructure.Services.Notifications; namespace Umbraco.Cms.Core.Services.Implement { @@ -28,25 +29,24 @@ namespace Umbraco.Cms.Core.Services.Implement public Attempt Delete(IDomain domain) { - var evtMsgs = EventMessagesFactory.Get(); + EventMessages eventMessages = EventMessagesFactory.Get(); - using (var scope = ScopeProvider.CreateScope()) + using (IScope scope = ScopeProvider.CreateScope()) { - var deleteEventArgs = new DeleteEventArgs(domain, evtMsgs); - if (scope.Events.DispatchCancelable(Deleting, this, deleteEventArgs)) + var deletingNotification = new DomainDeletingNotification(domain, eventMessages); + if (scope.Notifications.PublishCancelable(deletingNotification)) { scope.Complete(); - return OperationResult.Attempt.Cancel(evtMsgs); + return OperationResult.Attempt.Cancel(eventMessages); } _domainRepository.Delete(domain); scope.Complete(); - deleteEventArgs.CanCancel = false; - scope.Events.Dispatch(Deleted, this, deleteEventArgs); + scope.Notifications.Publish(new DomainDeletedNotification(domain, eventMessages).WithStateFrom(deletingNotification)); } - return OperationResult.Attempt.Succeed(evtMsgs); + return OperationResult.Attempt.Succeed(eventMessages); } public IDomain GetByName(string name) @@ -83,48 +83,23 @@ namespace Umbraco.Cms.Core.Services.Implement public Attempt Save(IDomain domainEntity) { - var evtMsgs = EventMessagesFactory.Get(); + EventMessages eventMessages = EventMessagesFactory.Get(); - using (var scope = ScopeProvider.CreateScope()) + using (IScope scope = ScopeProvider.CreateScope()) { - var saveEventArgs = new SaveEventArgs(domainEntity, evtMsgs); - if (scope.Events.DispatchCancelable(Saving, this, saveEventArgs)) + var savingNotification = new DomainSavingNotification(domainEntity, eventMessages); + if (scope.Notifications.PublishCancelable(savingNotification)) { scope.Complete(); - return OperationResult.Attempt.Cancel(evtMsgs); + return OperationResult.Attempt.Cancel(eventMessages); } _domainRepository.Save(domainEntity); scope.Complete(); - saveEventArgs.CanCancel = false; - scope.Events.Dispatch(Saved, this, saveEventArgs); + scope.Notifications.Publish(new DomainSavedNotification(domainEntity, eventMessages).WithStateFrom(savingNotification)); } - return OperationResult.Attempt.Succeed(evtMsgs); + return OperationResult.Attempt.Succeed(eventMessages); } - - #region Event Handlers - /// - /// Occurs before Delete - /// - public static event TypedEventHandler> Deleting; - - /// - /// Occurs after Delete - /// - public static event TypedEventHandler> Deleted; - - /// - /// Occurs before Save - /// - public static event TypedEventHandler> Saving; - - /// - /// Occurs after Save - /// - public static event TypedEventHandler> Saved; - - - #endregion } } diff --git a/src/Umbraco.Tests.Integration/Cache/DistributedCacheBinderTests.cs b/src/Umbraco.Tests.Integration/Cache/DistributedCacheBinderTests.cs index 80488e91b1..d61a256638 100644 --- a/src/Umbraco.Tests.Integration/Cache/DistributedCacheBinderTests.cs +++ b/src/Umbraco.Tests.Integration/Cache/DistributedCacheBinderTests.cs @@ -45,9 +45,6 @@ namespace Umbraco.Cms.Tests.Integration.Cache new EventDefinition>(null, FileService, new SaveEventArgs(Enumerable.Empty())), new EventDefinition>(null, FileService, new DeleteEventArgs(Enumerable.Empty())), - new EventDefinition>(null, DomainService, new SaveEventArgs(Enumerable.Empty())), - new EventDefinition>(null, DomainService, new DeleteEventArgs(Enumerable.Empty())), - new EventDefinition>(null, ContentTypeService, new SaveEventArgs(Enumerable.Empty())), new EventDefinition>(null, ContentTypeService, new DeleteEventArgs(Enumerable.Empty())), new EventDefinition>(null, MediaTypeService, new SaveEventArgs(Enumerable.Empty())), From 72e468428ad0c76ea4076d22693cc38f8a331eae Mon Sep 17 00:00:00 2001 From: Bjarne Fyrstenborg Date: Fri, 19 Mar 2021 16:57:40 +0100 Subject: [PATCH 27/32] Null check on scope and options to ensure backward compatibility (cherry picked from commit fe8cd239d2f4c528c1a8a3cf4c50e90bb43cacfc) --- .../src/common/services/listviewhelper.service.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/services/listviewhelper.service.js b/src/Umbraco.Web.UI.Client/src/common/services/listviewhelper.service.js index 14643dc9cd..f9ebba00ea 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/listviewhelper.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/listviewhelper.service.js @@ -572,13 +572,15 @@ * Method for opening an item in a list view for editing. * * @param {Object} item The item to edit + * @param {Object} scope The scope with options */ function editItem(item, scope) { + if (!item.editPath) { return; } - if (scope.options.useInfiniteEditor) + if (scope && scope.options && scope.options.useInfiniteEditor) { var editorModel = { id: item.id, From 9381c2e915472cc21bd5d68f06348cdd67744e76 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Wed, 31 Mar 2021 19:45:48 +0200 Subject: [PATCH 28/32] Added internals visible to configuration for Umbraco Forms. --- src/Umbraco.Core/Umbraco.Core.csproj | 22 ++++++++++++++++++- .../Umbraco.Infrastructure.csproj | 20 +++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index ce524a09a1..36b9c4d171 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -1,4 +1,4 @@ - + netstandard2.0 @@ -46,6 +46,26 @@ <_Parameter1>DynamicProxyGenAssembly2 + + + + <_Parameter1>Umbraco.Forms.Core + + + <_Parameter1>Umbraco.Forms.Core.Providers + + + <_Parameter1>Umbraco.Forms.Web + + + <_Parameter1>Umbraco.Forms.Core.V9 + + + <_Parameter1>Umbraco.Forms.Core.Providers.V9 + + + <_Parameter1>Umbraco.Forms.Web.V9 + diff --git a/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj b/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj index e5ee719a05..e870b02049 100644 --- a/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj +++ b/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj @@ -101,6 +101,26 @@ <_Parameter1>DynamicProxyGenAssembly2 + + + + <_Parameter1>Umbraco.Forms.Core + + + <_Parameter1>Umbraco.Forms.Core.Providers + + + <_Parameter1>Umbraco.Forms.Web + + + <_Parameter1>Umbraco.Forms.Core.V9 + + + <_Parameter1>Umbraco.Forms.Core.Providers.V9 + + + <_Parameter1>Umbraco.Forms.Web.V9 + From 1d9dbff72e05f8603ffc0ae0564515488728a77b Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Wed, 31 Mar 2021 19:49:54 +0200 Subject: [PATCH 29/32] Removed unnecessary V9 suffixes. --- src/Umbraco.Core/Umbraco.Core.csproj | 29 +++++++------------ .../Umbraco.Infrastructure.csproj | 11 +------ 2 files changed, 11 insertions(+), 29 deletions(-) diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 36b9c4d171..cade1041ac 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -47,25 +47,16 @@ <_Parameter1>DynamicProxyGenAssembly2 - - - <_Parameter1>Umbraco.Forms.Core - - - <_Parameter1>Umbraco.Forms.Core.Providers - - - <_Parameter1>Umbraco.Forms.Web - - - <_Parameter1>Umbraco.Forms.Core.V9 - - - <_Parameter1>Umbraco.Forms.Core.Providers.V9 - - - <_Parameter1>Umbraco.Forms.Web.V9 - + + + <_Parameter1>Umbraco.Forms.Core + + + <_Parameter1>Umbraco.Forms.Core.Providers + + + <_Parameter1>Umbraco.Forms.Web + diff --git a/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj b/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj index e870b02049..baa69dddb9 100644 --- a/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj +++ b/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj @@ -102,7 +102,7 @@ <_Parameter1>DynamicProxyGenAssembly2 - + <_Parameter1>Umbraco.Forms.Core @@ -112,15 +112,6 @@ <_Parameter1>Umbraco.Forms.Web - - <_Parameter1>Umbraco.Forms.Core.V9 - - - <_Parameter1>Umbraco.Forms.Core.Providers.V9 - - - <_Parameter1>Umbraco.Forms.Web.V9 - From aae7fc955f7241c89ddfadbb8adfb092f784d79e Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Fri, 2 Apr 2021 11:48:21 +0200 Subject: [PATCH 30/32] Made web layer internals visible to Umbraco Forms assemblies. --- src/Umbraco.Web.BackOffice/Umbraco.Web.BackOffice.csproj | 7 ++++++- src/Umbraco.Web.Common/Umbraco.Web.Common.csproj | 5 +++++ src/Umbraco.Web.Website/Umbraco.Web.Website.csproj | 7 ++++++- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Web.BackOffice/Umbraco.Web.BackOffice.csproj b/src/Umbraco.Web.BackOffice/Umbraco.Web.BackOffice.csproj index d49eb6e4f5..967e2043f4 100644 --- a/src/Umbraco.Web.BackOffice/Umbraco.Web.BackOffice.csproj +++ b/src/Umbraco.Web.BackOffice/Umbraco.Web.BackOffice.csproj @@ -1,4 +1,4 @@ - + net5.0 @@ -31,6 +31,11 @@ <_Parameter1>Umbraco.Tests.Integration + + + + <_Parameter1>Umbraco.Forms.Web + diff --git a/src/Umbraco.Web.Common/Umbraco.Web.Common.csproj b/src/Umbraco.Web.Common/Umbraco.Web.Common.csproj index bb2ecbc346..24f20f2f4a 100644 --- a/src/Umbraco.Web.Common/Umbraco.Web.Common.csproj +++ b/src/Umbraco.Web.Common/Umbraco.Web.Common.csproj @@ -40,6 +40,11 @@ <_Parameter1>Umbraco.Tests.UnitTests + + + + <_Parameter1>Umbraco.Forms.Web + diff --git a/src/Umbraco.Web.Website/Umbraco.Web.Website.csproj b/src/Umbraco.Web.Website/Umbraco.Web.Website.csproj index a7c5e7a277..31a2ef25b2 100644 --- a/src/Umbraco.Web.Website/Umbraco.Web.Website.csproj +++ b/src/Umbraco.Web.Website/Umbraco.Web.Website.csproj @@ -1,4 +1,4 @@ - + net5.0 @@ -34,5 +34,10 @@ <_Parameter1>Umbraco.Tests.Integration + + + + <_Parameter1>Umbraco.Forms.Web + From 32e1b8361f5ace781ab8146067d024dbebbf16c1 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Tue, 6 Apr 2021 08:23:07 +0200 Subject: [PATCH 31/32] Fix for members build issue --- .../DependencyInjection/ServiceCollectionExtensions.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Web.Common/DependencyInjection/ServiceCollectionExtensions.cs b/src/Umbraco.Web.Common/DependencyInjection/ServiceCollectionExtensions.cs index 5182db4e20..49ed441932 100644 --- a/src/Umbraco.Web.Common/DependencyInjection/ServiceCollectionExtensions.cs +++ b/src/Umbraco.Web.Common/DependencyInjection/ServiceCollectionExtensions.cs @@ -4,7 +4,6 @@ using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; -using Microsoft.Extensions.Options; using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.Web.Caching; using SixLabors.ImageSharp.Web.Commands; @@ -12,6 +11,7 @@ using SixLabors.ImageSharp.Web.DependencyInjection; using SixLabors.ImageSharp.Web.Processors; using SixLabors.ImageSharp.Web.Providers; using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Models.Identity; using Umbraco.Cms.Core.Security; using Umbraco.Cms.Web.Common.Security; @@ -69,8 +69,8 @@ namespace Umbraco.Extensions .AddMemberManager() .AddUserStore() .AddRoleStore() - .AddRoleValidator>() - .AddRoleManager>(); + .AddRoleValidator>() + .AddRoleManager>(); private static MemberIdentityBuilder BuildMembersIdentity(this IServiceCollection services) { @@ -78,7 +78,7 @@ namespace Umbraco.Extensions services.TryAddScoped, UserValidator>(); services.TryAddScoped, PasswordValidator>(); services.TryAddScoped, PasswordHasher>(); - return new MemberIdentityBuilder(typeof(IdentityRole), services); + return new MemberIdentityBuilder(typeof(UmbracoIdentityRole), services); } private static void RemoveIntParamenterIfValueGreatherThen(IDictionary commands, string parameter, int maxValue) From 18be3473331c064d311ee7808b733a898a9bfe68 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Tue, 6 Apr 2021 18:46:38 +0200 Subject: [PATCH 32/32] Fix todo by checking explicitly for Umbraco Core assembly names instead of "Umbraco.", to avoid special case forms, deploy etc. --- .../Composing/DefaultUmbracoAssemblyProvider.cs | 12 +----------- src/Umbraco.Core/Composing/ReferenceResolver.cs | 5 ++--- src/Umbraco.Core/Constants-Composing.cs | 13 ++++++++++++- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/Umbraco.Core/Composing/DefaultUmbracoAssemblyProvider.cs b/src/Umbraco.Core/Composing/DefaultUmbracoAssemblyProvider.cs index 516f26774a..60aa666034 100644 --- a/src/Umbraco.Core/Composing/DefaultUmbracoAssemblyProvider.cs +++ b/src/Umbraco.Core/Composing/DefaultUmbracoAssemblyProvider.cs @@ -16,16 +16,6 @@ namespace Umbraco.Cms.Core.Composing { private readonly Assembly _entryPointAssembly; private readonly ILoggerFactory _loggerFactory; - private static readonly string[] UmbracoCoreAssemblyNames = new[] - { - "Umbraco.Core", - "Umbraco.Infrastructure", - "Umbraco.PublishedCache.NuCache", - "Umbraco.Examine.Lucene", - "Umbraco.Web.Common", - "Umbraco.Web.BackOffice", - "Umbraco.Web.Website", - }; public DefaultUmbracoAssemblyProvider(Assembly entryPointAssembly, ILoggerFactory loggerFactory) { @@ -43,7 +33,7 @@ namespace Umbraco.Cms.Core.Composing { get { - var finder = new FindAssembliesWithReferencesTo(new[] { _entryPointAssembly }, UmbracoCoreAssemblyNames, true, _loggerFactory); + var finder = new FindAssembliesWithReferencesTo(new[] { _entryPointAssembly }, Constants.Composing.UmbracoCoreAssemblyNames, true, _loggerFactory); return finder.Find(); } } diff --git a/src/Umbraco.Core/Composing/ReferenceResolver.cs b/src/Umbraco.Core/Composing/ReferenceResolver.cs index 6ecd425ac1..fdc72a183b 100644 --- a/src/Umbraco.Core/Composing/ReferenceResolver.cs +++ b/src/Umbraco.Core/Composing/ReferenceResolver.cs @@ -82,9 +82,8 @@ namespace Umbraco.Cms.Core.Composing assemblyName.FullName.StartsWith(f, StringComparison.InvariantCultureIgnoreCase))) continue; - // don't include this item if it's Umbraco - // TODO: We should maybe pass an explicit list of these names in? - if (assemblyName.FullName.StartsWith("Umbraco.") || assemblyName.Name.EndsWith(".Views")) + // don't include this item if it's Umbraco Core + if (Constants.Composing.UmbracoCoreAssemblyNames.Any(x=>assemblyName.FullName.StartsWith(x) || assemblyName.Name.EndsWith(".Views"))) continue; var assembly = Assembly.Load(assemblyName); diff --git a/src/Umbraco.Core/Constants-Composing.cs b/src/Umbraco.Core/Constants-Composing.cs index a92f71ee04..747a74b8d8 100644 --- a/src/Umbraco.Core/Constants-Composing.cs +++ b/src/Umbraco.Core/Constants-Composing.cs @@ -9,6 +9,17 @@ /// Defines constants for composition. /// public static class Composing - { } + { + public static readonly string[] UmbracoCoreAssemblyNames = new[] + { + "Umbraco.Core", + "Umbraco.Infrastructure", + "Umbraco.PublishedCache.NuCache", + "Umbraco.Examine.Lucene", + "Umbraco.Web.Common", + "Umbraco.Web.BackOffice", + "Umbraco.Web.Website", + }; + } } }