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 caaebf8f42..990160964d 100644 --- a/src/Umbraco.Core/Security/BackOfficeUserManager.cs +++ b/src/Umbraco.Core/Security/BackOfficeUserManager.cs @@ -5,6 +5,7 @@ using System.Linq; using System.Text; using System.Threading.Tasks; using System.Web.Security; +using System.Web; using Microsoft.AspNet.Identity; using Microsoft.AspNet.Identity.Owin; using Microsoft.Owin.Security.DataProtection; @@ -506,52 +507,23 @@ 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); } - /// - /// Clears a lock so that the membership user can be validated. - /// - /// The IMemberService to user for unlocking - /// The membership user to clear the lock status for. - /// - /// true if the membership user was successfully unlocked; otherwise, false. - /// - public bool UnlockUser(IMembershipMemberService memberService, string username) where TEntity : class, IMembershipUser - { - var member = memberService.GetByUsername(username); - - if (member == null) - { - throw new ProviderException(string.Format("No member with the username '{0}' found", username)); - } - - // Non need to update - if (member.IsLockedOut == false) return true; - - member.IsLockedOut = false; - member.FailedPasswordAttempts = 0; - - memberService.Save(member); - - RaiseAccountUnlockedEvent(member.Id); - - return true; - } + public override Task AccessFailedAsync(int userId) { @@ -566,98 +538,61 @@ 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; @@ -726,5 +661,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(); + } } } diff --git a/src/Umbraco.Web/Editors/CurrentUserController.cs b/src/Umbraco.Web/Editors/CurrentUserController.cs index f75576e1ee..3d8ec01937 100644 --- a/src/Umbraco.Web/Editors/CurrentUserController.cs +++ b/src/Umbraco.Web/Editors/CurrentUserController.cs @@ -94,6 +94,18 @@ namespace Umbraco.Web.Editors 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(Services.TextService.Localize("user/password"), Services.TextService.Localize("user/passwordChanged")); diff --git a/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs b/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs index e0f7bf9a7f..334c2dd013 100644 --- a/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs +++ b/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs @@ -17,7 +17,7 @@ using Umbraco.Core.Models.Identity; namespace Umbraco.Web.Security.Providers { - + /// /// Abstract Membership Provider that users any implementation of IMembershipMemberService{TEntity} service @@ -28,7 +28,7 @@ namespace Umbraco.Web.Security.Providers { protected IMembershipMemberService MemberService { get; private set; } - + protected UmbracoMembershipProvider(IMembershipMemberService memberService) { MemberService = memberService; @@ -60,7 +60,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; @@ -88,7 +88,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); @@ -141,7 +141,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 @@ -166,11 +166,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; @@ -214,7 +214,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) { @@ -259,7 +259,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)); @@ -292,7 +292,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) { @@ -414,7 +414,7 @@ namespace Umbraco.Web.Security.Providers // UpdateFailureCount(username, "passwordAnswer"); // throw new ProviderException("Password answer required for password reset."); //} - + var m = MemberService.GetByUsername(username); if (m == null) { @@ -438,7 +438,7 @@ namespace Umbraco.Web.Security.Providers m.RawPasswordValue = FormatPasswordForStorage(encodedPassword, salt); m.LastPasswordChangeDate = DateTime.Now; MemberService.Save(m); - + return generatedPassword; } @@ -451,15 +451,21 @@ namespace Umbraco.Web.Security.Providers /// public override bool UnlockUser(string username) { - var userManager = GetBackofficeUserManager(); - return userManager != null && userManager.UnlockUser(MemberService, 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; } /// @@ -485,7 +491,7 @@ namespace Umbraco.Web.Security.Providers } } m.Email = user.Email; - + m.IsApproved = user.IsApproved; m.IsLockedOut = user.IsLockedOut; if (user.IsLockedOut) @@ -497,15 +503,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); @@ -517,7 +517,10 @@ namespace Umbraco.Web.Security.Providers username, GetCurrentRequestIpAddress())); - return false; + return new ValidateUserResult + { + Authenticated = false + }; } if (member.IsApproved == false) @@ -528,7 +531,11 @@ namespace Umbraco.Web.Security.Providers username, GetCurrentRequestIpAddress())); - return false; + return new ValidateUserResult + { + Member = member, + Authenticated = false + }; } if (member.IsLockedOut) { @@ -538,11 +545,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) { @@ -561,10 +571,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 { @@ -581,8 +588,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; @@ -591,10 +596,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 @@ -607,7 +609,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 fdebf78480..45ac9b16c8 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; @@ -115,5 +117,49 @@ namespace Umbraco.Web.Security.Providers return _defaultMemberTypeAlias; } } + + /// + /// 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