diff --git a/src/Umbraco.Core/HttpContextExtensions.cs b/src/Umbraco.Core/HttpContextExtensions.cs new file mode 100644 index 0000000000..b4e420dc42 --- /dev/null +++ b/src/Umbraco.Core/HttpContextExtensions.cs @@ -0,0 +1,45 @@ +using System.Web; + +namespace Umbraco.Core +{ + public static class HttpContextExtensions + { + public static string GetCurrentRequestIpAddress(this HttpContextBase httpContext) + { + if (httpContext == null) + { + return "Unknown, httpContext is null"; + } + if (httpContext.Request == null) + { + return "Unknown, httpContext.Request is null"; + } + if (httpContext.Request.ServerVariables == null) + { + return "Unknown, httpContext.Request.ServerVariables is null"; + } + + // From: http://stackoverflow.com/a/740431/5018 + + try + { + var ipAddress = httpContext.Request.ServerVariables["HTTP_X_FORWARDED_FOR"]; + + if (string.IsNullOrEmpty(ipAddress)) + return httpContext.Request.ServerVariables["REMOTE_ADDR"]; + + var addresses = ipAddress.Split(','); + if (addresses.Length != 0) + return addresses[0]; + + return httpContext.Request.ServerVariables["REMOTE_ADDR"]; + } + catch (System.Exception ex) + { + //This try catch is to just always ensure that no matter what we're not getting any exceptions caused since + // that would cause people to not be able to login + return string.Format("Unknown, exception occurred trying to resolve IP {0}", ex); + } + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Security/MembershipProviderBase.cs b/src/Umbraco.Core/Security/MembershipProviderBase.cs index ebcd967cc2..d12b9a952e 100644 --- a/src/Umbraco.Core/Security/MembershipProviderBase.cs +++ b/src/Umbraco.Core/Security/MembershipProviderBase.cs @@ -4,6 +4,7 @@ using System.Configuration.Provider; using System.Security.Cryptography; using System.Text; using System.Text.RegularExpressions; +using System.Web; using System.Web.Configuration; using System.Web.Hosting; using System.Web.Security; @@ -893,5 +894,15 @@ namespace Umbraco.Core.Security return sb.ToString(); } + /// + /// Returns the current request IP address for logging if there is one + /// + /// + protected string GetCurrentRequestIpAddress() + { + var httpContext = HttpContext.Current == null ? (HttpContextBase) null : new HttpContextWrapper(HttpContext.Current); + return httpContext.GetCurrentRequestIpAddress(); + } + } } \ No newline at end of file diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index bc272d351e..070090d8c7 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -313,6 +313,7 @@ + diff --git a/src/Umbraco.Web/Editors/AuthenticationController.cs b/src/Umbraco.Web/Editors/AuthenticationController.cs index 378306fdb4..43236226a1 100644 --- a/src/Umbraco.Web/Editors/AuthenticationController.cs +++ b/src/Umbraco.Web/Editors/AuthenticationController.cs @@ -107,8 +107,6 @@ namespace Umbraco.Web.Editors if (http.Success == false) throw new InvalidOperationException("This method requires that an HttpContext be active"); - var ipAddress = GetIPAddress(http.Result); - if (UmbracoContext.Security.ValidateBackOfficeCredentials(loginModel.Username, loginModel.Password)) { var user = Security.GetBackOfficeUser(loginModel.Username); @@ -121,16 +119,13 @@ namespace Umbraco.Web.Editors //set their remaining seconds result.SecondsUntilTimeout = ticket.GetRemainingAuthSeconds(); - LogHelper.Info(string.Format("Login attempt succeeded for username {0} from IP address {1}", loginModel.Username, ipAddress)); return result; } //return BadRequest (400), we don't want to return a 401 because that get's intercepted // by our angular helper because it thinks that we need to re-perform the request once we are // authorized and we don't want to return a 403 because angular will show a warning msg indicating - // that the user doesn't have access to perform this function, we just want to return a normal invalid msg. - - LogHelper.Info(string.Format("Login attempt failed for username {0} from IP address {1}", loginModel.Username, ipAddress)); + // that the user doesn't have access to perform this function, we just want to return a normal invalid msg. throw new HttpResponseException(HttpStatusCode.BadRequest); } @@ -147,19 +142,5 @@ namespace Umbraco.Web.Editors return Request.CreateResponse(HttpStatusCode.OK); } - // From: http://stackoverflow.com/a/740431/5018 - protected string GetIPAddress(HttpContextBase httpContext) - { - var ipAddress = httpContext.Request.ServerVariables["HTTP_X_FORWARDED_FOR"]; - - if (string.IsNullOrEmpty(ipAddress)) - return httpContext.Request.ServerVariables["REMOTE_ADDR"]; - - var addresses = ipAddress.Split(','); - if (addresses.Length != 0) - return addresses[0]; - - return httpContext.Request.ServerVariables["REMOTE_ADDR"]; - } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs b/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs index 16641e5f91..65f90d8127 100644 --- a/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs +++ b/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs @@ -511,16 +511,35 @@ namespace Umbraco.Web.Security.Providers { var member = MemberService.GetByUsername(username); - if (member == null) return false; + if (member == null) + { + LogHelper.Info( + string.Format( + "Login attempt failed for username {0} from IP address {1}, the user does not exist", + username, + GetCurrentRequestIpAddress())); + + return false; + } if (member.IsApproved == false) { - LogHelper.Info>("Cannot validate member " + username + " because they are not approved"); + LogHelper.Info( + string.Format( + "Login attempt failed for username {0} from IP address {1}, the user is not approved", + username, + GetCurrentRequestIpAddress())); + return false; } if (member.IsLockedOut) { - LogHelper.Info>("Cannot validate member " + username + " because they are currently locked out"); + LogHelper.Info( + string.Format( + "Login attempt failed for username {0} from IP address {1}, the user is locked", + username, + GetCurrentRequestIpAddress())); + return false; } @@ -538,18 +557,39 @@ namespace Umbraco.Web.Security.Providers { member.IsLockedOut = true; member.LastLockoutDate = DateTime.Now; - LogHelper.Info>("Member " + username + " is now locked out, max invalid password attempts exceeded"); + + LogHelper.Info( + 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())); + } + else + { + LogHelper.Info( + string.Format( + "Login attempt failed for username {0} from IP address {1}", + username, + GetCurrentRequestIpAddress())); } } else { member.FailedPasswordAttempts = 0; member.LastLoginDate = DateTime.Now; + + LogHelper.Info( + string.Format( + "Login attempt succeeded for username {0} from IP address {1}", + username, + GetCurrentRequestIpAddress())); } //don't raise events for this! It just sets the member dates, if we do raise events this will // cause all distributed cache to execute - which will clear out some caches we don't want. // http://issues.umbraco.org/issue/U4-3451 + //TODO: In v8 we aren't going to have an overload to disable events, so we'll need to make a different method + // for this type of thing (i.e. UpdateLastLogin or similar). MemberService.Save(member, false); return authenticated; diff --git a/src/umbraco.providers/UsersMembershipProvider.cs b/src/umbraco.providers/UsersMembershipProvider.cs index df190a8ac0..93f5327cbb 100644 --- a/src/umbraco.providers/UsersMembershipProvider.cs +++ b/src/umbraco.providers/UsersMembershipProvider.cs @@ -9,6 +9,8 @@ using umbraco.BusinessLogic; using System.Web.Util; using System.Configuration.Provider; using System.Linq; +using Umbraco.Core.Logging; + #endregion namespace umbraco.providers @@ -491,10 +493,33 @@ namespace umbraco.providers { if (user.Disabled) { + LogHelper.Info( + string.Format( + "Login attempt failed for username {0} from IP address {1}, the user is locked", + username, + GetCurrentRequestIpAddress())); + return false; } - return CheckPassword(password, user.Password); + var result = CheckPassword(password, user.Password); + if (result == false) + { + LogHelper.Info( + string.Format( + "Login attempt failed for username {0} from IP address {1}", + username, + GetCurrentRequestIpAddress())); + } + else + { + LogHelper.Info( + string.Format( + "Login attempt succeeded for username {0} from IP address {1}", + username, + GetCurrentRequestIpAddress())); + } + return result; } } return false;