From 97ddbdb1f05939aa8c74dbc8e3085fa2c4755380 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 18 Sep 2017 16:17:54 +1000 Subject: [PATCH 1/5] Makes IdentityAuditEventArgs immutable and injects all required values, makes the events strongly typed. --- .../Auditing/IdentityAuditEventArgs.cs | 32 ++++---- .../Security/BackOfficeUserManager.cs | 78 +++++++------------ 2 files changed, 42 insertions(+), 68 deletions(-) diff --git a/src/Umbraco.Core/Auditing/IdentityAuditEventArgs.cs b/src/Umbraco.Core/Auditing/IdentityAuditEventArgs.cs index e4fed75e33..14445d461f 100644 --- a/src/Umbraco.Core/Auditing/IdentityAuditEventArgs.cs +++ b/src/Umbraco.Core/Auditing/IdentityAuditEventArgs.cs @@ -13,7 +13,7 @@ namespace Umbraco.Core.Auditing /// /// The action that got triggered from the audit event /// - public AuditEvent Action { get; set; } + public AuditEvent Action { get; private set; } /// /// Current date/time in UTC format @@ -23,27 +23,27 @@ namespace Umbraco.Core.Auditing /// /// The source IP address of the user performing the action /// - public string IpAddress { get; set; } + public string IpAddress { get; private set; } /// /// The user affected by the event raised /// - public int AffectedUser { get; set; } + public int AffectedUser { get; private set; } /// /// If a user is perfoming an action on a different user, then this will be set. Otherwise it will be -1 /// - public int PerformingUser { get; set; } + public int PerformingUser { get; private set; } /// /// An optional comment about the action being logged /// - public string Comment { get; set; } + public string Comment { get; private set; } /// /// This property is always empty except in the LoginFailed event for an unknown user trying to login /// - public string Username { get; set; } + public string Username { get; private set; } /// /// Sets the properties on the event being raised, all parameters are optional except for the action being performed @@ -51,28 +51,26 @@ namespace Umbraco.Core.Auditing /// An action based on the AuditEvent enum /// The client's IP address. This is usually automatically set but could be overridden if necessary /// The Id of the user performing the action (if different from the user affected by the action) - public IdentityAuditEventArgs(AuditEvent action, string ipAddress = "", int performingUser = -1) + public IdentityAuditEventArgs(AuditEvent action, string ipAddress, int performingUser = -1) { DateTimeUtc = DateTime.UtcNow; Action = action; - IpAddress = string.IsNullOrWhiteSpace(ipAddress) - ? GetCurrentRequestIpAddress() - : ipAddress; + IpAddress = ipAddress; PerformingUser = performingUser == -1 ? GetCurrentRequestBackofficeUserId() : performingUser; } - /// - /// Returns the current request IP address for logging if there is one - /// - /// - protected string GetCurrentRequestIpAddress() + public IdentityAuditEventArgs(AuditEvent action, string ipAddress, string username, string comment) { - var httpContext = HttpContext.Current == null ? (HttpContextBase)null : new HttpContextWrapper(HttpContext.Current); - return httpContext.GetCurrentRequestIpAddress(); + DateTimeUtc = DateTime.UtcNow; + Action = action; + + IpAddress = ipAddress; + Username = username; + Comment = comment; } /// diff --git a/src/Umbraco.Core/Security/BackOfficeUserManager.cs b/src/Umbraco.Core/Security/BackOfficeUserManager.cs index 40eec5c420..b5b8f1bf4d 100644 --- a/src/Umbraco.Core/Security/BackOfficeUserManager.cs +++ b/src/Umbraco.Core/Security/BackOfficeUserManager.cs @@ -1,6 +1,7 @@ using System; using System.Configuration.Provider; using System.Threading.Tasks; +using System.Web; using Microsoft.AspNet.Identity; using Microsoft.AspNet.Identity.Owin; using Microsoft.Owin.Security.DataProtection; @@ -317,99 +318,63 @@ namespace Umbraco.Core.Security internal void RaiseAccountLockedEvent(int userId) { - OnAccountLocked(new IdentityAuditEventArgs(AuditEvent.AccountLocked) - { - AffectedUser = userId - }); + OnAccountLocked(new IdentityAuditEventArgs(AuditEvent.AccountLocked, GetCurrentRequestIpAddress(), userId)); } internal void RaiseAccountUnlockedEvent(int userId) { - OnAccountUnlocked(new IdentityAuditEventArgs(AuditEvent.AccountUnlocked) - { - AffectedUser = userId - }); + OnAccountUnlocked(new IdentityAuditEventArgs(AuditEvent.AccountUnlocked, GetCurrentRequestIpAddress(), userId)); } internal void RaiseForgotPasswordRequestedEvent(int userId) { - OnForgotPasswordRequested(new IdentityAuditEventArgs(AuditEvent.ForgotPasswordRequested) - { - AffectedUser = userId - }); + OnForgotPasswordRequested(new IdentityAuditEventArgs(AuditEvent.ForgotPasswordRequested, GetCurrentRequestIpAddress(), userId)); } internal void RaiseForgotPasswordChangedSuccessEvent(int userId) { - OnForgotPasswordChangedSuccess(new IdentityAuditEventArgs(AuditEvent.ForgotPasswordChangedSuccess) - { - AffectedUser = userId - }); + OnForgotPasswordChangedSuccess(new IdentityAuditEventArgs(AuditEvent.ForgotPasswordChangedSuccess, GetCurrentRequestIpAddress(), userId)); } - public void RaiseLoginFailedEvent(int userId) + internal void RaiseLoginFailedEvent(int userId) { - OnLoginFailed(new IdentityAuditEventArgs(AuditEvent.LoginFailed) - { - AffectedUser = userId - }); + OnLoginFailed(new IdentityAuditEventArgs(AuditEvent.LoginFailed, GetCurrentRequestIpAddress(), userId)); } - public void RaiseInvalidLoginAttemptEvent(string username) + internal void RaiseInvalidLoginAttemptEvent(string username) { - OnLoginFailed(new IdentityAuditEventArgs(AuditEvent.LoginFailed) - { - Username = username, - Comment = string.Format("Attempted login for username '{0}' failed", username) - }); + OnLoginFailed(new IdentityAuditEventArgs(AuditEvent.LoginFailed, GetCurrentRequestIpAddress(), username, string.Format("Attempted login for username '{0}' failed", username))); } internal void RaiseLoginRequiresVerificationEvent(int userId) { - OnLoginRequiresVerification(new IdentityAuditEventArgs(AuditEvent.LoginRequiresVerification) - { - AffectedUser = userId - }); + OnLoginRequiresVerification(new IdentityAuditEventArgs(AuditEvent.LoginRequiresVerification, GetCurrentRequestIpAddress(), userId)); } internal void RaiseLoginSuccessEvent(int userId) { - OnLoginSuccess(new IdentityAuditEventArgs(AuditEvent.LoginSucces) - { - AffectedUser = userId - }); + OnLoginSuccess(new IdentityAuditEventArgs(AuditEvent.LoginSucces, GetCurrentRequestIpAddress(), userId)); } internal void RaiseLogoutSuccessEvent(int userId) { - OnLogoutSuccess(new IdentityAuditEventArgs(AuditEvent.LogoutSuccess) - { - AffectedUser = userId - }); + OnLogoutSuccess(new IdentityAuditEventArgs(AuditEvent.LogoutSuccess, GetCurrentRequestIpAddress(), userId)); } internal void RaisePasswordChangedEvent(int userId) { - OnPasswordChanged(new IdentityAuditEventArgs(AuditEvent.PasswordChanged) - { - AffectedUser = userId - }); + OnPasswordChanged(new IdentityAuditEventArgs(AuditEvent.PasswordChanged, GetCurrentRequestIpAddress(), userId)); } internal void RaisePasswordResetEvent(int userId) { - OnPasswordReset(new IdentityAuditEventArgs(AuditEvent.PasswordReset) - { - AffectedUser = userId - }); + OnPasswordReset(new IdentityAuditEventArgs(AuditEvent.PasswordReset, GetCurrentRequestIpAddress(), userId)); } internal void RaiseResetAccessFailedCountEvent(int userId) { - OnResetAccessFailedCount(new IdentityAuditEventArgs(AuditEvent.ResetAccessFailedCount) - { - AffectedUser = userId - }); + OnResetAccessFailedCount(new IdentityAuditEventArgs(AuditEvent.ResetAccessFailedCount, GetCurrentRequestIpAddress(), userId)); } + public static event EventHandler AccountLocked; public static event EventHandler AccountUnlocked; public static event EventHandler ForgotPasswordRequested; @@ -476,5 +441,16 @@ namespace Umbraco.Core.Security { if (ResetAccessFailedCount != null) ResetAccessFailedCount(this, e); } + + /// + /// Returns the current request IP address for logging if there is one + /// + /// + protected virtual string GetCurrentRequestIpAddress() + { + //TODO: inject a service to get this value, we should not be relying on the old HttpContext.Current especially in the ASP.NET Identity world. + var httpContext = HttpContext.Current == null ? (HttpContextBase)null : new HttpContextWrapper(HttpContext.Current); + return httpContext.GetCurrentRequestIpAddress(); + } } } From 26034fa33f3263b2af6978fe1d94c238f2014992 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 18 Sep 2017 16:24:23 +1000 Subject: [PATCH 2/5] Changes ResetAccessFailedCountAsync in BackOfficeUserManager to use Identity APIs --- .../Security/BackOfficeUserManager.cs | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/Umbraco.Core/Security/BackOfficeUserManager.cs b/src/Umbraco.Core/Security/BackOfficeUserManager.cs index b5b8f1bf4d..a849134783 100644 --- a/src/Umbraco.Core/Security/BackOfficeUserManager.cs +++ b/src/Umbraco.Core/Security/BackOfficeUserManager.cs @@ -273,21 +273,20 @@ namespace Umbraco.Core.Security public override async Task ResetAccessFailedCountAsync(int userId) { - var user = ApplicationContext.Current.Services.UserService.GetUserById(userId); - + var lockoutStore = (IUserLockoutStore)Store; + var user = await FindByIdAsync(userId); if (user == null) - { - throw new ProviderException(string.Format("No user with the id {0} found", userId)); - } + throw new InvalidOperationException("No user found by user id " + userId); - if (user.FailedPasswordAttempts > 0) - { - user.FailedPasswordAttempts = 0; - ApplicationContext.Current.Services.UserService.Save(user); - RaiseResetAccessFailedCountEvent(userId); - } + var accessFailedCount = await GetAccessFailedCountAsync(user.Id); - return await Task.FromResult(IdentityResult.Success); + if (accessFailedCount == 0) + return IdentityResult.Success; + + await lockoutStore.ResetAccessFailedCountAsync(user); + //raise the event now that it's reset + RaiseResetAccessFailedCountEvent(userId); + return await UpdateAsync(user); } /// From 4ec04c779a4a97e706620d505cb9cda64b0424ae Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 18 Sep 2017 16:29:29 +1000 Subject: [PATCH 3/5] Adds notes about the UnlockUser method as we'll need to change this in 7.7 --- src/Umbraco.Core/Security/BackOfficeUserManager.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Core/Security/BackOfficeUserManager.cs b/src/Umbraco.Core/Security/BackOfficeUserManager.cs index a849134783..e93744838f 100644 --- a/src/Umbraco.Core/Security/BackOfficeUserManager.cs +++ b/src/Umbraco.Core/Security/BackOfficeUserManager.cs @@ -286,7 +286,7 @@ namespace Umbraco.Core.Security await lockoutStore.ResetAccessFailedCountAsync(user); //raise the event now that it's reset RaiseResetAccessFailedCountEvent(userId); - return await UpdateAsync(user); + return await UpdateAsync(user); } /// @@ -296,7 +296,13 @@ namespace Umbraco.Core.Security /// /// true if the membership user was successfully unlocked; otherwise, false. /// - public bool UnlockUser(string username) + /// + /// TODO: This is only currently used for membership provider compatibility so that an event is raised when the + /// membership provider's UnlockUser method is called, this functionality changes in 7.7 since we use ASP.NET Identity APIs everwhere + /// and the BackOfficeUserManager shouldn't be using singletons or know about the UserService since it should only need to know about + /// the UserStore - we will fix this up for 7.7 + /// + internal bool UnlockUser(string username) { var user = ApplicationContext.Current.Services.UserService.GetByUsername(username); if (user == null) From 2a10eed059a5fa7cc5b28a27f488606f58c5b51d Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 18 Sep 2017 16:56:43 +1000 Subject: [PATCH 4/5] Moves raising events for reseting/changing password out of the MembershipHelper ChangePassword method and to the places where it is being called --- .../Editors/CurrentUserController.cs | 13 ++++++++++ src/Umbraco.Web/Security/MembershipHelper.cs | 24 ++----------------- .../umbraco/users/EditUser.aspx.cs | 12 ++++++++++ 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/Umbraco.Web/Editors/CurrentUserController.cs b/src/Umbraco.Web/Editors/CurrentUserController.cs index aacad99fb7..b0b0377a21 100644 --- a/src/Umbraco.Web/Editors/CurrentUserController.cs +++ b/src/Umbraco.Web/Editors/CurrentUserController.cs @@ -18,6 +18,7 @@ using umbraco; using legacyUser = umbraco.BusinessLogic.User; using System.Net.Http; using System.Collections.Specialized; +using Umbraco.Core.Security; using Constants = Umbraco.Core.Constants; @@ -60,6 +61,18 @@ namespace Umbraco.Web.Editors var passwordChangeResult = Members.ChangePassword(Security.CurrentUser.Username, data, userProvider); if (passwordChangeResult.Success) { + var userMgr = this.TryGetOwinContext().Result.GetBackOfficeUserManager(); + + //raise the appropriate event + if (data.Reset.HasValue && data.Reset.Value) + { + userMgr.RaisePasswordResetEvent(Security.CurrentUser.Id); + } + else + { + userMgr.RaisePasswordChangedEvent(Security.CurrentUser.Id); + } + //even if we weren't resetting this, it is the correct value (null), otherwise if we were resetting then it will contain the new pword var result = new ModelWithNotifications(passwordChangeResult.Result.ResetPassword); result.AddSuccessNotification(ui.Text("user", "password"), ui.Text("user", "passwordChanged")); diff --git a/src/Umbraco.Web/Security/MembershipHelper.cs b/src/Umbraco.Web/Security/MembershipHelper.cs index 729a8fa45b..ca82852717 100644 --- a/src/Umbraco.Web/Security/MembershipHelper.cs +++ b/src/Umbraco.Web/Security/MembershipHelper.cs @@ -717,21 +717,7 @@ namespace Umbraco.Web.Security if (passwordModel == null) throw new ArgumentNullException("passwordModel"); if (membershipProvider == null) throw new ArgumentNullException("membershipProvider"); - - BackOfficeUserManager backofficeUserManager = null; - var userId = -1; - - if (membershipProvider.IsUmbracoUsersProvider()) - { - backofficeUserManager = _httpContext.GetOwinContext().GetBackOfficeUserManager(); - if (backofficeUserManager != null) - { - var profile = _applicationContext.Services.UserService.GetProfileByUserName(username); - if (profile != null) - int.TryParse(profile.Id.ToString(), out userId); - } - } - + //Are we resetting the password?? if (passwordModel.Reset.HasValue && passwordModel.Reset.Value) { @@ -751,9 +737,6 @@ namespace Umbraco.Web.Security username, membershipProvider.RequiresQuestionAndAnswer ? passwordModel.Answer : null); - if (membershipProvider.IsUmbracoUsersProvider() && backofficeUserManager != null && userId >= 0) - backofficeUserManager.RaisePasswordResetEvent(userId); - //return the generated pword return Attempt.Succeed(new PasswordChangedModel { ResetPassword = newPass }); } @@ -804,10 +787,7 @@ namespace Umbraco.Web.Security try { var result = membershipProvider.ChangePassword(username, passwordModel.OldPassword, passwordModel.NewPassword); - - if (result && backofficeUserManager != null && userId >= 0) - backofficeUserManager.RaisePasswordChangedEvent(userId); - + return result == false ? Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Could not change password, invalid username or password", new[] { "value" }) }) : Attempt.Succeed(new PasswordChangedModel()); diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/users/EditUser.aspx.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/users/EditUser.aspx.cs index 9c4a744341..c66c094961 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/users/EditUser.aspx.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/users/EditUser.aspx.cs @@ -481,6 +481,18 @@ namespace umbraco.cms.presentation.user if (changePassResult.Success) { + var userMgr = Context.GetOwinContext().GetBackOfficeUserManager(); + + //raise the appropriate event + if (changePasswordModel.Reset.HasValue && changePasswordModel.Reset.Value) + { + userMgr.RaisePasswordResetEvent(UmbracoUser.Id); + } + else + { + userMgr.RaisePasswordChangedEvent(UmbracoUser.Id); + } + //if it is successful, we need to show the generated password if there was one, so set //that back on the control passwordChangerControl.ChangingPasswordModel.GeneratedPassword = changePassResult.Result.ResetPassword; From d468e346f16e06a30e7fc5016d23f450852dde03 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 18 Sep 2017 19:01:00 +1000 Subject: [PATCH 5/5] reverts changes made to UmbracoMembershipProvider and adds another virtual internal method that the UsersMembershipProvider overrides, so now all of the event raising is done in the user specific provider. --- .../Providers/UmbracoMembershipProvider.cs | 118 +++++++++++------- .../Providers/UsersMembershipProvider.cs | 62 +++++++++ 2 files changed, 134 insertions(+), 46 deletions(-) diff --git a/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs b/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs index d4b0ae2721..67c37d9f04 100644 --- a/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs +++ b/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs @@ -21,7 +21,7 @@ using Umbraco.Core.Security; namespace Umbraco.Web.Security.Providers { - + /// /// Abstract Membership Provider that users any implementation of IMembershipMemberService{TEntity} service @@ -32,7 +32,7 @@ namespace Umbraco.Web.Security.Providers { protected IMembershipMemberService MemberService { get; private set; } - + protected UmbracoMembershipProvider(IMembershipMemberService memberService) { MemberService = memberService; @@ -64,7 +64,7 @@ namespace Umbraco.Web.Security.Providers /// The name of the provider has a length of zero. public override void Initialize(string name, NameValueCollection config) { - if (config == null) {throw new ArgumentNullException("config");} + if (config == null) { throw new ArgumentNullException("config"); } if (string.IsNullOrEmpty(name)) name = ProviderName; @@ -92,7 +92,7 @@ namespace Umbraco.Web.Security.Providers // in order to support updating passwords from the umbraco core, we can't validate the old password var m = MemberService.GetByUsername(username); if (m == null) return false; - + string salt; var encodedPassword = EncryptOrHashNewPassword(newPassword, out salt); @@ -145,7 +145,7 @@ namespace Umbraco.Web.Security.Providers /// /// A object populated with the information for the newly created user. /// - protected override MembershipUser PerformCreateUser(string memberTypeAlias, string username, string password, string email, string passwordQuestion, + protected override MembershipUser PerformCreateUser(string memberTypeAlias, string username, string password, string email, string passwordQuestion, string passwordAnswer, bool isApproved, object providerUserKey, out MembershipCreateStatus status) { // See if the user already exists @@ -170,11 +170,11 @@ namespace Umbraco.Web.Security.Providers var member = MemberService.CreateWithIdentity( username, - email, - FormatPasswordForStorage(encodedPassword, salt), + email, + FormatPasswordForStorage(encodedPassword, salt), memberTypeAlias, isApproved); - + member.PasswordQuestion = passwordQuestion; member.RawPasswordAnswerValue = EncryptString(passwordAnswer); member.LastLoginDate = DateTime.Now; @@ -218,7 +218,7 @@ namespace Umbraco.Web.Security.Providers public override MembershipUserCollection FindUsersByEmail(string emailToMatch, int pageIndex, int pageSize, out int totalRecords) { var byEmail = MemberService.FindByEmail(emailToMatch, pageIndex, pageSize, out totalRecords, StringPropertyMatchType.Wildcard).ToArray(); - + var collection = new MembershipUserCollection(); foreach (var m in byEmail) { @@ -263,7 +263,7 @@ namespace Umbraco.Web.Security.Providers var membersList = new MembershipUserCollection(); var pagedMembers = MemberService.GetAll(pageIndex, pageSize, out totalRecords); - + foreach (var m in pagedMembers) { membersList.Add(ConvertToMembershipUser(m)); @@ -296,7 +296,7 @@ namespace Umbraco.Web.Security.Providers /// The password for the specified user name. /// protected override string PerformGetPassword(string username, string answer) - { + { var m = MemberService.GetByUsername(username); if (m == null) { @@ -419,7 +419,7 @@ namespace Umbraco.Web.Security.Providers // throw new ProviderException("Password answer required for password reset."); //} - + var m = MemberService.GetByUsername(username); if (m == null) { @@ -443,7 +443,7 @@ namespace Umbraco.Web.Security.Providers m.RawPasswordValue = FormatPasswordForStorage(encodedPassword, salt); m.LastPasswordChangeDate = DateTime.Now; MemberService.Save(m); - + return generatedPassword; } @@ -456,15 +456,21 @@ namespace Umbraco.Web.Security.Providers /// public override bool UnlockUser(string username) { - var userManager = GetBackofficeUserManager(); - return userManager != null && userManager.UnlockUser(username); - } + var member = MemberService.GetByUsername(username); + if (member == null) + { + throw new ProviderException(string.Format("No member with the username '{0}' found", username)); + } - internal BackOfficeUserManager GetBackofficeUserManager() - { - return HttpContext.Current == null - ? null - : HttpContext.Current.GetOwinContext().GetBackOfficeUserManager(); + // Non need to update + if (member.IsLockedOut == false) return true; + + member.IsLockedOut = false; + member.FailedPasswordAttempts = 0; + + MemberService.Save(member); + + return true; } /// @@ -490,7 +496,7 @@ namespace Umbraco.Web.Security.Providers } } m.Email = user.Email; - + m.IsApproved = user.IsApproved; m.IsLockedOut = user.IsLockedOut; if (user.IsLockedOut) @@ -502,15 +508,9 @@ namespace Umbraco.Web.Security.Providers MemberService.Save(m); } - /// - /// Verifies that the specified user name and password exist in the data source. - /// - /// The name of the user to validate. - /// The password for the specified user. - /// - /// true if the specified username and password are valid; otherwise, false. - /// - public override bool ValidateUser(string username, string password) + + + internal virtual ValidateUserResult PerformValidateUser(string username, string password) { var member = MemberService.GetByUsername(username); @@ -522,7 +522,10 @@ namespace Umbraco.Web.Security.Providers username, GetCurrentRequestIpAddress())); - return false; + return new ValidateUserResult + { + Authenticated = false + }; } if (member.IsApproved == false) @@ -533,7 +536,11 @@ namespace Umbraco.Web.Security.Providers username, GetCurrentRequestIpAddress())); - return false; + return new ValidateUserResult + { + Member = member, + Authenticated = false + }; } if (member.IsLockedOut) { @@ -543,11 +550,14 @@ namespace Umbraco.Web.Security.Providers username, GetCurrentRequestIpAddress())); - return false; + return new ValidateUserResult + { + Member = member, + Authenticated = false + }; } var authenticated = CheckPassword(password, member.RawPasswordValue); - var backofficeUserManager = GetBackofficeUserManager(); if (authenticated == false) { @@ -566,10 +576,7 @@ namespace Umbraco.Web.Security.Providers string.Format( "Login attempt failed for username {0} from IP address {1}, the user is now locked out, max invalid password attempts exceeded", username, - GetCurrentRequestIpAddress())); - - if(backofficeUserManager != null) - backofficeUserManager.RaiseAccountLockedEvent(member.Id); + GetCurrentRequestIpAddress())); } else { @@ -586,8 +593,6 @@ namespace Umbraco.Web.Security.Providers { //we have successfully logged in, reset the AccessFailedCount member.FailedPasswordAttempts = 0; - if (backofficeUserManager != null) - backofficeUserManager.RaiseResetAccessFailedCountEvent(member.Id); } member.LastLoginDate = DateTime.Now; @@ -596,10 +601,7 @@ namespace Umbraco.Web.Security.Providers string.Format( "Login attempt succeeded for username {0} from IP address {1}", username, - GetCurrentRequestIpAddress())); - - if (backofficeUserManager != null) - backofficeUserManager.RaiseLoginSuccessEvent(member.Id); + GetCurrentRequestIpAddress())); } //don't raise events for this! It just sets the member dates, if we do raise events this will @@ -612,7 +614,31 @@ namespace Umbraco.Web.Security.Providers if (UmbracoVersion.Current >= new Version(7, 3, 0, 0)) MemberService.Save(member, false); - return authenticated; + return new ValidateUserResult + { + Authenticated = authenticated, + Member = member + }; + } + + /// + /// Verifies that the specified user name and password exist in the data source. + /// + /// The name of the user to validate. + /// The password for the specified user. + /// + /// true if the specified username and password are valid; otherwise, false. + /// + public override bool ValidateUser(string username, string password) + { + var result = PerformValidateUser(username, password); + return result.Authenticated; + } + + internal class ValidateUserResult + { + public TEntity Member { get; set; } + public bool Authenticated { get; set; } } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Security/Providers/UsersMembershipProvider.cs b/src/Umbraco.Web/Security/Providers/UsersMembershipProvider.cs index 267bc0b433..6cbde80da3 100644 --- a/src/Umbraco.Web/Security/Providers/UsersMembershipProvider.cs +++ b/src/Umbraco.Web/Security/Providers/UsersMembershipProvider.cs @@ -2,9 +2,11 @@ using System.Configuration.Provider; using System.Security.Cryptography; using System.Text; +using System.Web; using System.Web.Security; using Umbraco.Core; using Umbraco.Core.Configuration; +using Umbraco.Core.Models.Identity; using Umbraco.Core.Models.Membership; using Umbraco.Core.Security; using Umbraco.Core.Services; @@ -80,5 +82,65 @@ namespace Umbraco.Web.Security.Providers return _defaultMemberTypeAlias; } } + + /// + /// Overridden in order to call the BackOfficeUserManager.UnlockUser method in order to raise the user audit events + /// + /// + /// + public override bool UnlockUser(string username) + { + var userManager = GetBackofficeUserManager(); + + //if this is null it means it's not in a web context so we'll revert to calling the base class + if (userManager == null) + return base.UnlockUser(username); + + return userManager.UnlockUser(username); + } + + /// + /// Override in order to raise appropriate events via the + /// + /// + /// + /// + internal override ValidateUserResult PerformValidateUser(string username, string password) + { + var result = base.PerformValidateUser(username, password); + + var userManager = GetBackofficeUserManager(); + + if (userManager == null) return result; + + if (result.Authenticated == false) + { + var count = result.Member.FailedPasswordAttempts; + if (count >= MaxInvalidPasswordAttempts) + { + userManager.RaiseAccountLockedEvent(result.Member.Id); + } + } + else + { + if (result.Member.FailedPasswordAttempts > 0) + { + //we have successfully logged in, if the failed password attempts was modified it means it was reset + if (result.Member.WasPropertyDirty("FailedPasswordAttempts")) + { + userManager.RaiseResetAccessFailedCountEvent(result.Member.Id); + } + } + } + + return result; + } + + internal BackOfficeUserManager GetBackofficeUserManager() + { + return HttpContext.Current == null + ? null + : HttpContext.Current.GetOwinContext().GetBackOfficeUserManager(); + } } } \ No newline at end of file