From 97ddbdb1f05939aa8c74dbc8e3085fa2c4755380 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 18 Sep 2017 16:17:54 +1000 Subject: [PATCH] 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(); + } } }