From fd66f085201d24db4e126396804804629b80b250 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 26 Nov 2015 13:07:22 +0100 Subject: [PATCH] Fixes setting auth cookie during install, removes some try/catch/swallow with some error messaging, converts some String -> string and == false updates. --- src/Umbraco.Core/ApplicationContext.cs | 10 +++- .../Models/Identity/IdentityModelMappings.cs | 13 +++++ src/Umbraco.Core/Models/UserExtensions.cs | 1 + .../Install/HttpInstallAuthorizeAttribute.cs | 5 +- src/Umbraco.Web/Install/InstallHelper.cs | 14 ++--- .../Models/Mapping/UserModelMapper.cs | 2 +- src/Umbraco.Web/Security/WebAuthExtensions.cs | 6 +- src/Umbraco.Web/Security/WebSecurity.cs | 58 +++++++++++++++++-- .../webservices/CheckForUpgrade.asmx.cs | 9 ++- 9 files changed, 93 insertions(+), 25 deletions(-) diff --git a/src/Umbraco.Core/ApplicationContext.cs b/src/Umbraco.Core/ApplicationContext.cs index 4aef011d05..a05abe2a50 100644 --- a/src/Umbraco.Core/ApplicationContext.cs +++ b/src/Umbraco.Core/ApplicationContext.cs @@ -283,7 +283,12 @@ namespace Umbraco.Core { var configStatus = ConfigurationStatus; var currentVersion = UmbracoVersion.GetSemanticVersion(); - var ok = configStatus == currentVersion; + + var ok = + //we are not configured if this is null + string.IsNullOrWhiteSpace(configStatus) == false + //they must match + && configStatus == currentVersion; if (ok) { @@ -308,8 +313,9 @@ namespace Umbraco.Core return ok; } - catch + catch (Exception ex) { + LogHelper.Error("Error determining if application is configured, returning false", ex); return false; } diff --git a/src/Umbraco.Core/Models/Identity/IdentityModelMappings.cs b/src/Umbraco.Core/Models/Identity/IdentityModelMappings.cs index f57d6683a2..0dc95a8987 100644 --- a/src/Umbraco.Core/Models/Identity/IdentityModelMappings.cs +++ b/src/Umbraco.Core/Models/Identity/IdentityModelMappings.cs @@ -4,6 +4,7 @@ using AutoMapper; using Umbraco.Core.Models.Mapping; using Umbraco.Core.Models.Membership; +using Umbraco.Core.Security; namespace Umbraco.Core.Models.Identity { @@ -24,6 +25,18 @@ namespace Umbraco.Core.Models.Identity .ForMember(user => user.UserTypeAlias, expression => expression.MapFrom(user => user.UserType.Alias)) .ForMember(user => user.AccessFailedCount, expression => expression.MapFrom(user => user.FailedPasswordAttempts)) .ForMember(user => user.AllowedSections, expression => expression.MapFrom(user => user.AllowedSections.ToArray())); + + config.CreateMap() + .ConstructUsing((BackOfficeIdentityUser user) => new UserData(Guid.NewGuid().ToString("N"))) //this is the 'session id' + .ForMember(detail => detail.Id, opt => opt.MapFrom(user => user.Id)) + .ForMember(detail => detail.AllowedApplications, opt => opt.MapFrom(user => user.AllowedSections)) + .ForMember(detail => detail.RealName, opt => opt.MapFrom(user => user.Name)) + .ForMember(detail => detail.Roles, opt => opt.MapFrom(user => new[] { user.UserTypeAlias })) + .ForMember(detail => detail.StartContentNode, opt => opt.MapFrom(user => user.StartContentId)) + .ForMember(detail => detail.StartMediaNode, opt => opt.MapFrom(user => user.StartMediaId)) + .ForMember(detail => detail.Username, opt => opt.MapFrom(user => user.UserName)) + .ForMember(detail => detail.Culture, opt => opt.MapFrom(user => user.Culture)) + .ForMember(detail => detail.SessionId, opt => opt.MapFrom(user => user.SecurityStamp.IsNullOrWhiteSpace() ? Guid.NewGuid().ToString("N") : user.SecurityStamp)); } private string GetPasswordHash(string storedPass) diff --git a/src/Umbraco.Core/Models/UserExtensions.cs b/src/Umbraco.Core/Models/UserExtensions.cs index bd670f3836..5b9f63cf48 100644 --- a/src/Umbraco.Core/Models/UserExtensions.cs +++ b/src/Umbraco.Core/Models/UserExtensions.cs @@ -2,6 +2,7 @@ using System.Globalization; using System.Linq; using System.Threading; +using Umbraco.Core.Models.Identity; using Umbraco.Core.Models.Membership; using Umbraco.Core.Services; diff --git a/src/Umbraco.Web/Install/HttpInstallAuthorizeAttribute.cs b/src/Umbraco.Web/Install/HttpInstallAuthorizeAttribute.cs index bab2bddc40..4559a42740 100644 --- a/src/Umbraco.Web/Install/HttpInstallAuthorizeAttribute.cs +++ b/src/Umbraco.Web/Install/HttpInstallAuthorizeAttribute.cs @@ -5,6 +5,7 @@ using System.Web.Http.Controllers; using Umbraco.Core; using Umbraco.Web.Security; using umbraco.BasePages; +using Umbraco.Core.Logging; namespace Umbraco.Web.Install { @@ -52,6 +53,7 @@ namespace Umbraco.Web.Install return true; } var umbCtx = GetUmbracoContext(); + //otherwise we need to ensure that a user is logged in var isLoggedIn = GetUmbracoContext().Security.ValidateCurrentUser(); if (isLoggedIn) @@ -60,8 +62,9 @@ namespace Umbraco.Web.Install } return false; } - catch (Exception) + catch (Exception ex) { + LogHelper.Error("An error occurred determining authorization", ex); return false; } } diff --git a/src/Umbraco.Web/Install/InstallHelper.cs b/src/Umbraco.Web/Install/InstallHelper.cs index 59e073dc8c..143067f6a1 100644 --- a/src/Umbraco.Web/Install/InstallHelper.cs +++ b/src/Umbraco.Web/Install/InstallHelper.cs @@ -103,9 +103,9 @@ namespace Umbraco.Web.Install string userAgent = _umbContext.HttpContext.Request.UserAgent; // Check for current install Id - Guid installId = Guid.NewGuid(); - StateHelper.Cookies.Cookie installCookie = new StateHelper.Cookies.Cookie("umb_installId", 1); - if (!String.IsNullOrEmpty(installCookie.GetValue())) + var installId = Guid.NewGuid(); + var installCookie = new StateHelper.Cookies.Cookie("umb_installId", 1); + if (string.IsNullOrEmpty(installCookie.GetValue()) == false) { if (Guid.TryParse(installCookie.GetValue(), out installId)) { @@ -116,13 +116,13 @@ namespace Umbraco.Web.Install } installCookie.SetValue(installId.ToString()); - string dbProvider = String.Empty; - if (!IsBrandNewInstall) + string dbProvider = string.Empty; + if (IsBrandNewInstall == false) dbProvider = ApplicationContext.Current.DatabaseContext.DatabaseProvider.ToString(); org.umbraco.update.CheckForUpgrade check = new org.umbraco.update.CheckForUpgrade(); check.Install(installId, - !IsBrandNewInstall, + IsBrandNewInstall == false, isCompleted, DateTime.Now, UmbracoVersion.Current.Major, @@ -135,7 +135,7 @@ namespace Umbraco.Web.Install } catch (Exception ex) { - + LogHelper.Error("An error occurred in InstallStatus trying to check upgrades", ex); } } diff --git a/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs b/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs index afae0eb122..e1c3cc4f1e 100644 --- a/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs @@ -52,7 +52,7 @@ namespace Umbraco.Web.Models.Mapping .ForMember(detail => detail.Username, opt => opt.MapFrom(user => user.Username)) .ForMember(detail => detail.Culture, opt => opt.MapFrom(user => user.GetUserCulture(applicationContext.Services.TextService))) .ForMember(detail => detail.SessionId, opt => opt.MapFrom(user => user.SecurityStamp.IsNullOrWhiteSpace() ? Guid.NewGuid().ToString("N") : user.SecurityStamp)); - + } private static int GetIntId(object id) diff --git a/src/Umbraco.Web/Security/WebAuthExtensions.cs b/src/Umbraco.Web/Security/WebAuthExtensions.cs index ec43244b6e..52963d1852 100644 --- a/src/Umbraco.Web/Security/WebAuthExtensions.cs +++ b/src/Umbraco.Web/Security/WebAuthExtensions.cs @@ -50,14 +50,14 @@ namespace Umbraco.Web.Security /// This will set a an authenticated IPrincipal to the current request given the IUser object /// /// - /// + /// /// - internal static IPrincipal SetPrincipalForRequest(this HttpContextBase httpContext, IUser user) + internal static IPrincipal SetPrincipalForRequest(this HttpContextBase httpContext, UserData userData) { var principal = new ClaimsPrincipal( new UmbracoBackOfficeIdentity( new ClaimsIdentity(), - Mapper.Map(user))); + userData)); //It is actually not good enough to set this on the current app Context and the thread, it also needs // to be set explicitly on the HttpContext.Current !! This is a strange web api thing that is actually diff --git a/src/Umbraco.Web/Security/WebSecurity.cs b/src/Umbraco.Web/Security/WebSecurity.cs index 1e986dccfc..af6e53ca11 100644 --- a/src/Umbraco.Web/Security/WebSecurity.cs +++ b/src/Umbraco.Web/Security/WebSecurity.cs @@ -10,6 +10,7 @@ using Umbraco.Core.Logging; using Umbraco.Core.Models.Membership; using Umbraco.Core.Security; using Microsoft.AspNet.Identity.Owin; +using Microsoft.Owin; using umbraco.businesslogic.Exceptions; using Umbraco.Web.Models.ContentEditing; using GlobalSettings = Umbraco.Core.Configuration.GlobalSettings; @@ -79,6 +80,42 @@ namespace Umbraco.Web.Security } } + private BackOfficeSignInManager _signInManager; + private BackOfficeSignInManager SignInManager + { + get + { + if (_signInManager == null) + { + var mgr = _httpContext.GetOwinContext().Get(); + if (mgr == null) + { + throw new NullReferenceException("Could not resolve an instance of " + typeof(BackOfficeSignInManager) + " from the " + typeof(IOwinContext)); + } + _signInManager = mgr; + } + return _signInManager; + } + } + + private BackOfficeUserManager _userManager; + protected BackOfficeUserManager UserManager + { + get + { + if (_userManager == null) + { + var mgr = _httpContext.GetOwinContext().GetUserManager(); + if (mgr == null) + { + throw new NullReferenceException("Could not resolve an instance of " + typeof(BackOfficeUserManager) + " from the " + typeof(IOwinContext) + " GetUserManager method"); + } + _userManager = mgr; + } + return _userManager; + } + } + /// /// Logs a user in. /// @@ -87,25 +124,34 @@ namespace Umbraco.Web.Security public virtual double PerformLogin(int userId) { var owinCtx = _httpContext.GetOwinContext(); + //ensure it's done for owin too + owinCtx.Authentication.SignOut(Constants.Security.BackOfficeExternalAuthenticationType); - var user = _applicationContext.Services.UserService.GetUserById(userId); - var userDetail = Mapper.Map(user); - //update the userDetail and set their remaining seconds - userDetail.SecondsUntilTimeout = TimeSpan.FromMinutes(GlobalSettings.TimeOutInMinutes).TotalSeconds; - var principal = _httpContext.SetPrincipalForRequest(user); - owinCtx.Authentication.SignIn((UmbracoBackOfficeIdentity)principal.Identity); + var user = UserManager.FindByIdAsync(userId).Result; + var userData = Mapper.Map(user); + _httpContext.SetPrincipalForRequest(userData); + + SignInManager.SignInAsync(user, isPersistent: false, rememberBrowser: false).Wait(); return TimeSpan.FromMinutes(GlobalSettings.TimeOutInMinutes).TotalSeconds; } [Obsolete("This method should not be used, login is performed by the OWIN pipeline, use the overload that returns double and accepts a UserId instead")] public virtual FormsAuthenticationTicket PerformLogin(IUser user) { + //clear the external cookie - we do this first without owin context because we're writing cookies directly to httpcontext + // and cookie handling is different with httpcontext vs webapi and owin, normally we'd just do: + //_httpContext.GetOwinContext().Authentication.SignOut(Constants.Security.BackOfficeExternalAuthenticationType); + var externalLoginCookie = _httpContext.Request.Cookies.Get(Constants.Security.BackOfficeExternalCookieName); if (externalLoginCookie != null) { externalLoginCookie.Expires = DateTime.Now.AddYears(-1); _httpContext.Response.Cookies.Set(externalLoginCookie); } + + //ensure it's done for owin too + _httpContext.GetOwinContext().Authentication.SignOut(Constants.Security.BackOfficeExternalAuthenticationType); + var ticket = _httpContext.CreateUmbracoAuthTicket(Mapper.Map(user)); return ticket; } diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/webservices/CheckForUpgrade.asmx.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/webservices/CheckForUpgrade.asmx.cs index 2d6beced38..909b89b96b 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/webservices/CheckForUpgrade.asmx.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/webservices/CheckForUpgrade.asmx.cs @@ -56,9 +56,8 @@ namespace umbraco.presentation.webservices // Check for current install Id Guid installId = Guid.NewGuid(); - BusinessLogic.StateHelper.Cookies.Cookie installCookie = - new BusinessLogic.StateHelper.Cookies.Cookie("umb_installId", 1); - if (!String.IsNullOrEmpty(installCookie.GetValue())) + var installCookie = new BusinessLogic.StateHelper.Cookies.Cookie("umb_installId", 1); + if (string.IsNullOrEmpty(installCookie.GetValue()) == false) { if (Guid.TryParse(installCookie.GetValue(), out installId)) { @@ -70,8 +69,8 @@ namespace umbraco.presentation.webservices } installCookie.SetValue(installId.ToString()); - string dbProvider = String.Empty; - if (!String.IsNullOrEmpty(global::Umbraco.Core.Configuration.GlobalSettings.ConfigurationStatus)) + string dbProvider = string.Empty; + if (string.IsNullOrEmpty(global::Umbraco.Core.Configuration.GlobalSettings.ConfigurationStatus) == false) dbProvider = ApplicationContext.Current.DatabaseContext.DatabaseProvider.ToString(); var check = new global::Umbraco.Web.org.umbraco.update.CheckForUpgrade();