From fe5dcd83bb9e60a9b4537fbfa26b4b60bb52a2c5 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 1 Dec 2020 18:14:37 +1100 Subject: [PATCH] removes the 2FA store implementation since that will need to be manually enabled --- .../BackOffice/BackOfficeUserStore.cs | 86 +++++++------------ .../BackOffice/IBackOfficeUserManager.cs | 13 --- .../Controllers/AuthenticationController.cs | 4 +- .../Security/BackOfficeUserManager.cs | 25 +++--- .../Security/BackOfficeUserManagerAuditer.cs | 52 +++++------ 5 files changed, 73 insertions(+), 107 deletions(-) diff --git a/src/Umbraco.Infrastructure/BackOffice/BackOfficeUserStore.cs b/src/Umbraco.Infrastructure/BackOffice/BackOfficeUserStore.cs index cdd81a3fe3..b271f5aa41 100644 --- a/src/Umbraco.Infrastructure/BackOffice/BackOfficeUserStore.cs +++ b/src/Umbraco.Infrastructure/BackOffice/BackOfficeUserStore.cs @@ -12,6 +12,7 @@ using Umbraco.Core.Mapping; using Umbraco.Core.Models; using Umbraco.Core.Models.Identity; using Umbraco.Core.Models.Membership; +using Umbraco.Core.Scoping; using Umbraco.Core.Services; namespace Umbraco.Core.BackOffice @@ -22,15 +23,17 @@ namespace Umbraco.Core.BackOffice IUserLoginStore, IUserRoleStore, IUserSecurityStampStore, - IUserLockoutStore, - IUserTwoFactorStore, + IUserLockoutStore, IUserSessionStore - // TODO: This would require additional columns/tables for now people will need to implement this on their own - //IUserPhoneNumberStore, - // TODO: To do this we need to implement IQueryable - we'll have an IQuerable implementation soon with the UmbracoLinqPadDriver implementation - //IQueryableUserStore + // TODO: This would require additional columns/tables and then a lot of extra coding support to make this happen natively within umbraco + //IUserTwoFactorStore, + // TODO: This would require additional columns/tables for now people will need to implement this on their own + //IUserPhoneNumberStore, + // TODO: To do this we need to implement IQueryable - we'll have an IQuerable implementation soon with the UmbracoLinqPadDriver implementation + //IQueryableUserStore { + private readonly IScopeProvider _scopeProvider; private readonly IUserService _userService; private readonly IEntityService _entityService; private readonly IExternalLoginService _externalLoginService; @@ -38,8 +41,9 @@ namespace Umbraco.Core.BackOffice private readonly UmbracoMapper _mapper; private bool _disposed = false; - public BackOfficeUserStore(IUserService userService, IEntityService entityService, IExternalLoginService externalLoginService, IOptions globalSettings, UmbracoMapper mapper) + public BackOfficeUserStore(IScopeProvider scopeProvider, IUserService userService, IEntityService entityService, IExternalLoginService externalLoginService, IOptions globalSettings, UmbracoMapper mapper) { + _scopeProvider = scopeProvider; _userService = userService; _entityService = entityService; _externalLoginService = externalLoginService; @@ -168,28 +172,31 @@ namespace Umbraco.Core.BackOffice throw new InvalidOperationException("The user id must be an integer to work with the Umbraco"); } - // TODO: Wrap this in a scope! - - var found = _userService.GetUserById(asInt.Result); - if (found != null) + using (var scope = _scopeProvider.CreateScope()) { - // we have to remember whether Logins property is dirty, since the UpdateMemberProperties will reset it. - var isLoginsPropertyDirty = user.IsPropertyDirty(nameof(BackOfficeIdentityUser.Logins)); - - if (UpdateMemberProperties(found, user)) + var found = _userService.GetUserById(asInt.Result); + if (found != null) { - _userService.Save(found); + // we have to remember whether Logins property is dirty, since the UpdateMemberProperties will reset it. + var isLoginsPropertyDirty = user.IsPropertyDirty(nameof(BackOfficeIdentityUser.Logins)); + + if (UpdateMemberProperties(found, user)) + { + _userService.Save(found); + } + + if (isLoginsPropertyDirty) + { + _externalLoginService.Save( + found.Id, + user.Logins.Select(x => new ExternalLogin( + x.LoginProvider, + x.ProviderKey, + x.UserData))); + } } - if (isLoginsPropertyDirty) - { - _externalLoginService.Save( - found.Id, - user.Logins.Select(x => new ExternalLogin( - x.LoginProvider, - x.ProviderKey, - x.UserData))); - } + scope.Complete(); } return Task.FromResult(IdentityResult.Success); @@ -627,35 +634,6 @@ namespace Umbraco.Core.BackOffice return user; } - /// - /// Sets whether two factor authentication is enabled for the user - /// - /// - /// - /// - /// - public virtual Task SetTwoFactorEnabledAsync(BackOfficeIdentityUser user, bool enabled, CancellationToken cancellationToken = default(CancellationToken)) - { - cancellationToken.ThrowIfCancellationRequested(); - ThrowIfDisposed(); - - user.TwoFactorEnabled = false; - return Task.CompletedTask; - } - - /// - /// Returns whether two factor authentication is enabled for the user - /// - /// - /// - public virtual Task GetTwoFactorEnabledAsync(BackOfficeIdentityUser user, CancellationToken cancellationToken = default(CancellationToken)) - { - cancellationToken.ThrowIfCancellationRequested(); - ThrowIfDisposed(); - - return Task.FromResult(false); - } - #region IUserLockoutStore /// diff --git a/src/Umbraco.Infrastructure/BackOffice/IBackOfficeUserManager.cs b/src/Umbraco.Infrastructure/BackOffice/IBackOfficeUserManager.cs index 47de2e3956..c026c256f5 100644 --- a/src/Umbraco.Infrastructure/BackOffice/IBackOfficeUserManager.cs +++ b/src/Umbraco.Infrastructure/BackOffice/IBackOfficeUserManager.cs @@ -318,20 +318,7 @@ namespace Umbraco.Core.BackOffice void RaiseForgotPasswordChangedSuccessEvent(IPrincipal currentUser, int userId); SignOutAuditEventArgs RaiseLogoutSuccessEvent(IPrincipal currentUser, int userId); UserInviteEventArgs RaiseSendingUserInvite(IPrincipal currentUser, UserInvite invite, IUser createdUser); - bool HasSendingUserInviteEventHandler { get; } - - event EventHandler AccountLocked; - event EventHandler AccountUnlocked; - event EventHandler ForgotPasswordRequested; - event EventHandler ForgotPasswordChangedSuccess; - event EventHandler LoginFailed; - event EventHandler LoginRequiresVerification; - event EventHandler LoginSuccess; - event EventHandler LogoutSuccess; - event EventHandler PasswordChanged; - event EventHandler PasswordReset; - event EventHandler ResetAccessFailedCount; } } diff --git a/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs b/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs index 5c95be2afc..8275d427da 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs @@ -340,9 +340,7 @@ namespace Umbraco.Web.BackOffice.Controllers StatusCode = StatusCodes.Status402PaymentRequired }; - - - //return verifyResponse; + return verifyResponse; } // return BadRequest (400), we don't want to return a 401 because that get's intercepted diff --git a/src/Umbraco.Web.BackOffice/Security/BackOfficeUserManager.cs b/src/Umbraco.Web.BackOffice/Security/BackOfficeUserManager.cs index 30deb3d436..e774c39eb5 100644 --- a/src/Umbraco.Web.BackOffice/Security/BackOfficeUserManager.cs +++ b/src/Umbraco.Web.BackOffice/Security/BackOfficeUserManager.cs @@ -436,17 +436,20 @@ namespace Umbraco.Web.Common.Security public bool HasSendingUserInviteEventHandler => SendingUserInvite != null; - public event EventHandler AccountLocked; - public event EventHandler AccountUnlocked; - public event EventHandler ForgotPasswordRequested; - public event EventHandler ForgotPasswordChangedSuccess; - public event EventHandler LoginFailed; - public event EventHandler LoginRequiresVerification; - public event EventHandler LoginSuccess; - public event EventHandler LogoutSuccess; - public event EventHandler PasswordChanged; - public event EventHandler PasswordReset; - public event EventHandler ResetAccessFailedCount; + // TODO: These static events are problematic. Moving forward we don't want static events at all but we cannot + // have non-static events here because the user manager is a Scoped instance not a singleton + // so we'll have to deal with this a diff way i.e. refactoring how events are done entirely + public static event EventHandler AccountLocked; + public static event EventHandler AccountUnlocked; + public static event EventHandler ForgotPasswordRequested; + public static event EventHandler ForgotPasswordChangedSuccess; + public static event EventHandler LoginFailed; + public static event EventHandler LoginRequiresVerification; + public static event EventHandler LoginSuccess; + public static event EventHandler LogoutSuccess; + public static event EventHandler PasswordChanged; + public static event EventHandler PasswordReset; + public static event EventHandler ResetAccessFailedCount; /// /// Raised when a user is invited diff --git a/src/Umbraco.Web.BackOffice/Security/BackOfficeUserManagerAuditer.cs b/src/Umbraco.Web.BackOffice/Security/BackOfficeUserManagerAuditer.cs index afa50ee8cd..019eed7e39 100644 --- a/src/Umbraco.Web.BackOffice/Security/BackOfficeUserManagerAuditer.cs +++ b/src/Umbraco.Web.BackOffice/Security/BackOfficeUserManagerAuditer.cs @@ -1,4 +1,5 @@ -using System; +using Microsoft.Extensions.Options; +using System; using System.Threading.Tasks; using Umbraco.Core; using Umbraco.Core.BackOffice; @@ -14,18 +15,16 @@ namespace Umbraco.Web.Common.Security /// internal class BackOfficeUserManagerAuditer : IDisposable { - private readonly IBackOfficeUserManager _backOfficeUserManager; private readonly IAuditService _auditService; private readonly IUserService _userService; private readonly GlobalSettings _globalSettings; private bool _disposedValue; - public BackOfficeUserManagerAuditer(IBackOfficeUserManager backOfficeUserManager, IAuditService auditService, IUserService userService, GlobalSettings globalSettings) + public BackOfficeUserManagerAuditer(IAuditService auditService, IUserService userService, IOptions globalSettings) { - _backOfficeUserManager = backOfficeUserManager; _auditService = auditService; _userService = userService; - _globalSettings = globalSettings; + _globalSettings = globalSettings.Value; } /// @@ -34,17 +33,18 @@ namespace Umbraco.Web.Common.Security public void Start() { // NOTE: This was migrated as-is from v8 including these missing entries - //_backOfficeUserManager.AccountLocked += ; - //_backOfficeUserManager.AccountUnlocked += ; - _backOfficeUserManager.ForgotPasswordRequested += OnForgotPasswordRequest; - _backOfficeUserManager.ForgotPasswordChangedSuccess += OnForgotPasswordChange; - _backOfficeUserManager.LoginFailed += OnLoginFailed; - //_backOfficeUserManager.LoginRequiresVerification += ; - _backOfficeUserManager.LoginSuccess += OnLoginSuccess; - _backOfficeUserManager.LogoutSuccess += OnLogoutSuccess; - _backOfficeUserManager.PasswordChanged += OnPasswordChanged; - _backOfficeUserManager.PasswordReset += OnPasswordReset; - //_backOfficeUserManager.ResetAccessFailedCount += ; + // TODO: See note about static events in BackOfficeUserManager + //BackOfficeUserManager.AccountLocked += ; + //BackOfficeUserManager.AccountUnlocked += ; + BackOfficeUserManager.ForgotPasswordRequested += OnForgotPasswordRequest; + BackOfficeUserManager.ForgotPasswordChangedSuccess += OnForgotPasswordChange; + BackOfficeUserManager.LoginFailed += OnLoginFailed; + //BackOfficeUserManager.LoginRequiresVerification += ; + BackOfficeUserManager.LoginSuccess += OnLoginSuccess; + BackOfficeUserManager.LogoutSuccess += OnLogoutSuccess; + BackOfficeUserManager.PasswordChanged += OnPasswordChanged; + BackOfficeUserManager.PasswordReset += OnPasswordReset; + //BackOfficeUserManager.ResetAccessFailedCount += ; } private IUser GetPerformingUser(int userId) @@ -138,16 +138,16 @@ namespace Umbraco.Web.Common.Security { if (disposing) { - //_backOfficeUserManager.AccountLocked -= ; - //_backOfficeUserManager.AccountUnlocked -= ; - _backOfficeUserManager.ForgotPasswordRequested -= OnForgotPasswordRequest; - _backOfficeUserManager.ForgotPasswordChangedSuccess -= OnForgotPasswordChange; - _backOfficeUserManager.LoginFailed -= OnLoginFailed; - //_backOfficeUserManager.LoginRequiresVerification -= ; - _backOfficeUserManager.LoginSuccess -= OnLoginSuccess; - _backOfficeUserManager.LogoutSuccess -= OnLogoutSuccess; - _backOfficeUserManager.PasswordChanged -= OnPasswordChanged; - _backOfficeUserManager.PasswordReset -= OnPasswordReset; + //BackOfficeUserManager.AccountLocked -= ; + //BackOfficeUserManager.AccountUnlocked -= ; + BackOfficeUserManager.ForgotPasswordRequested -= OnForgotPasswordRequest; + BackOfficeUserManager.ForgotPasswordChangedSuccess -= OnForgotPasswordChange; + BackOfficeUserManager.LoginFailed -= OnLoginFailed; + //BackOfficeUserManager.LoginRequiresVerification -= ; + BackOfficeUserManager.LoginSuccess -= OnLoginSuccess; + BackOfficeUserManager.LogoutSuccess -= OnLogoutSuccess; + BackOfficeUserManager.PasswordChanged -= OnPasswordChanged; + BackOfficeUserManager.PasswordReset -= OnPasswordReset; } _disposedValue = true; }