From f2b1558cb1b74db33d5b64fdef10a39f07e335ca Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Mon, 17 Feb 2020 09:04:15 +0100 Subject: [PATCH] Review fixes from comments on https://github.com/umbraco/Umbraco-CMS/pull/7664 --- src/Umbraco.Abstractions/HybridAccessorBase.cs | 2 -- src/Umbraco.Abstractions/UmbracoContextReference.cs | 9 ++++----- .../Install/InstallSteps/NewInstallStep.cs | 11 +++++------ src/Umbraco.Web/Net/AspNetHttpContextAccessor.cs | 8 +++++++- src/Umbraco.Web/Security/WebSecurity.cs | 7 ------- src/Umbraco.Web/UmbracoContextFactory.cs | 4 ++-- 6 files changed, 18 insertions(+), 23 deletions(-) diff --git a/src/Umbraco.Abstractions/HybridAccessorBase.cs b/src/Umbraco.Abstractions/HybridAccessorBase.cs index 3e17f7b757..ad33fbf067 100644 --- a/src/Umbraco.Abstractions/HybridAccessorBase.cs +++ b/src/Umbraco.Abstractions/HybridAccessorBase.cs @@ -24,8 +24,6 @@ namespace Umbraco.Web private static bool _registered; // ReSharper restore StaticMemberInGenericType - // private readonly IHttpContextAccessor _httpContextAccessor; - protected abstract string ItemKey { get; } // read diff --git a/src/Umbraco.Abstractions/UmbracoContextReference.cs b/src/Umbraco.Abstractions/UmbracoContextReference.cs index b282932d9f..3cd97cd77b 100644 --- a/src/Umbraco.Abstractions/UmbracoContextReference.cs +++ b/src/Umbraco.Abstractions/UmbracoContextReference.cs @@ -14,23 +14,22 @@ namespace Umbraco.Web /// public class UmbracoContextReference : IDisposable //fixme - should we inherit from DisposableObjectSlim? { - private readonly IUmbracoContextAccessor _umbracoContextAccessor; private bool _disposed; /// /// Initializes a new instance of the class. /// - internal UmbracoContextReference(bool isRoot, IUmbracoContextAccessor umbracoContextAccessor) + internal UmbracoContextReference(bool isRoot, IUmbracoContext umbracoContext) { IsRoot = isRoot; - _umbracoContextAccessor = umbracoContextAccessor; + UmbracoContext = umbracoContext; } /// /// Gets the . /// - public IUmbracoContext UmbracoContext => _umbracoContextAccessor.UmbracoContext; + public IUmbracoContext UmbracoContext { get; private set; } /// /// Gets a value indicating whether the reference is a root reference. @@ -50,7 +49,7 @@ namespace Umbraco.Web if (IsRoot) { UmbracoContext.Dispose(); - _umbracoContextAccessor.UmbracoContext = null; + UmbracoContext = null; } GC.SuppressFinalize(this); diff --git a/src/Umbraco.Web/Install/InstallSteps/NewInstallStep.cs b/src/Umbraco.Web/Install/InstallSteps/NewInstallStep.cs index 6122e2924a..c898532116 100644 --- a/src/Umbraco.Web/Install/InstallSteps/NewInstallStep.cs +++ b/src/Umbraco.Web/Install/InstallSteps/NewInstallStep.cs @@ -6,15 +6,12 @@ using System.Threading.Tasks; using System.Web; using Newtonsoft.Json; using Umbraco.Core; -using Umbraco.Web.Composing; using Umbraco.Core.Configuration; using Umbraco.Core.Migrations.Install; -using Umbraco.Core.Models.Identity; using Umbraco.Core.Services; using Umbraco.Web.Install.Models; -using Umbraco.Web.Models.Identity; -using Umbraco.Web.Security; using Umbraco.Core.Configuration.UmbracoSettings; +using Umbraco.Core.Cookie; namespace Umbraco.Web.Install.InstallSteps { @@ -37,8 +34,9 @@ namespace Umbraco.Web.Install.InstallSteps private readonly IUserPasswordConfiguration _passwordConfiguration; private readonly IUmbracoSettingsSection _umbracoSettingsSection; private readonly IConnectionStrings _connectionStrings; + private readonly ICookieManager _cookieManager; - public NewInstallStep(IHttpContextAccessor httpContextAccessor, IUserService userService, DatabaseBuilder databaseBuilder, IGlobalSettings globalSettings, IUserPasswordConfiguration passwordConfiguration, IUmbracoSettingsSection umbracoSettingsSection, IConnectionStrings connectionStrings) + public NewInstallStep(IHttpContextAccessor httpContextAccessor, IUserService userService, DatabaseBuilder databaseBuilder, IGlobalSettings globalSettings, IUserPasswordConfiguration passwordConfiguration, IUmbracoSettingsSection umbracoSettingsSection, IConnectionStrings connectionStrings, ICookieManager cookieManager) { _httpContextAccessor = httpContextAccessor ?? throw new ArgumentNullException(nameof(httpContextAccessor)); _userService = userService ?? throw new ArgumentNullException(nameof(userService)); @@ -47,6 +45,7 @@ namespace Umbraco.Web.Install.InstallSteps _passwordConfiguration = passwordConfiguration ?? throw new ArgumentNullException(nameof(passwordConfiguration)); _umbracoSettingsSection = umbracoSettingsSection ?? throw new ArgumentNullException(nameof(umbracoSettingsSection)); _connectionStrings = connectionStrings ?? throw new ArgumentNullException(nameof(connectionStrings)); + _cookieManager = cookieManager; } public override async Task ExecuteAsync(UserModel user) @@ -137,7 +136,7 @@ namespace Umbraco.Web.Install.InstallSteps // In this one case when it's a brand new install and nothing has been configured, make sure the // back office cookie is cleared so there's no old cookies lying around causing problems - _httpContextAccessor.HttpContext.ExpireCookie(_umbracoSettingsSection.Security.AuthCookieName); + _cookieManager.ExpireCookie(_umbracoSettingsSection.Security.AuthCookieName); return true; } diff --git a/src/Umbraco.Web/Net/AspNetHttpContextAccessor.cs b/src/Umbraco.Web/Net/AspNetHttpContextAccessor.cs index 5d306e0692..322e9f2d88 100644 --- a/src/Umbraco.Web/Net/AspNetHttpContextAccessor.cs +++ b/src/Umbraco.Web/Net/AspNetHttpContextAccessor.cs @@ -10,7 +10,13 @@ namespace Umbraco.Web get { var httpContext = System.Web.HttpContext.Current; - return httpContext is null ? null : new HttpContextWrapper(httpContext); + + if (httpContext is null) + { + throw new InvalidOperationException("HttpContext is not available"); + } + + return new HttpContextWrapper(httpContext); } set { diff --git a/src/Umbraco.Web/Security/WebSecurity.cs b/src/Umbraco.Web/Security/WebSecurity.cs index fd936eba39..a1b360bb83 100644 --- a/src/Umbraco.Web/Security/WebSecurity.cs +++ b/src/Umbraco.Web/Security/WebSecurity.cs @@ -101,13 +101,6 @@ namespace Umbraco.Web.Security Core.Constants.Security.BackOfficeExternalAuthenticationType); } - /// - /// Renews the user's login ticket - /// - public void RenewLoginTimeout() - { - _httpContextAccessor.HttpContext.RenewUmbracoAuthTicket(); - } /// /// Gets the current user's id. diff --git a/src/Umbraco.Web/UmbracoContextFactory.cs b/src/Umbraco.Web/UmbracoContextFactory.cs index fd41b2fca6..8a109e4c55 100644 --- a/src/Umbraco.Web/UmbracoContextFactory.cs +++ b/src/Umbraco.Web/UmbracoContextFactory.cs @@ -72,13 +72,13 @@ namespace Umbraco.Web { var currentUmbracoContext = _umbracoContextAccessor.UmbracoContext; if (currentUmbracoContext != null) - return new UmbracoContextReference(false, _umbracoContextAccessor); + return new UmbracoContextReference(false, currentUmbracoContext); var umbracoContext = CreateUmbracoContext(); _umbracoContextAccessor.UmbracoContext = umbracoContext; - return new UmbracoContextReference(true, _umbracoContextAccessor); + return new UmbracoContextReference(true, umbracoContext); } // dummy TextWriter that does not write