From 6c395db5a5b1a15406407eccc9200846524455a6 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Wed, 12 Feb 2020 10:56:23 +0100 Subject: [PATCH 1/7] Removed legacy way of handling alternative templates.. Today we use `ContentFinderByUrlAndTemplate` --- .../Constants-Conventions.cs | 13 +--- src/Umbraco.Web/Routing/PublishedRouter.cs | 73 ++----------------- src/Umbraco.Web/Templates/TemplateRenderer.cs | 11 +-- 3 files changed, 10 insertions(+), 87 deletions(-) diff --git a/src/Umbraco.Abstractions/Constants-Conventions.cs b/src/Umbraco.Abstractions/Constants-Conventions.cs index 37275b156a..6672f22239 100644 --- a/src/Umbraco.Abstractions/Constants-Conventions.cs +++ b/src/Umbraco.Abstractions/Constants-Conventions.cs @@ -137,7 +137,7 @@ namespace Umbraco.Core public static readonly string UmbracoMemberProviderName = "UmbracoMembershipProvider"; public static readonly string UmbracoRoleProviderName = "UmbracoRoleProvider"; - + /// /// Property alias for the Comments on a Member /// @@ -208,17 +208,6 @@ namespace Umbraco.Core public const string AllMembersListId = "all-members"; } - /// - /// Constants for Umbraco URLs/Querystrings. - /// - public static class Url - { - /// - /// Querystring parameter name used for Umbraco's alternative template functionality. - /// - public const string AltTemplate = "altTemplate"; - } - /// /// Defines the alias identifiers for built-in Umbraco relation types. /// diff --git a/src/Umbraco.Web/Routing/PublishedRouter.cs b/src/Umbraco.Web/Routing/PublishedRouter.cs index 9148ce2e31..74fe9f93e6 100644 --- a/src/Umbraco.Web/Routing/PublishedRouter.cs +++ b/src/Umbraco.Web/Routing/PublishedRouter.cs @@ -20,7 +20,6 @@ namespace Umbraco.Web.Routing /// public class PublishedRouter : IPublishedRouter { - private readonly IWebRoutingSection _webRoutingSection; private readonly ContentFinderCollection _contentFinders; private readonly IContentLastChanceFinder _contentLastChanceFinder; private readonly ServiceContext _services; @@ -34,7 +33,6 @@ namespace Umbraco.Web.Routing /// Initializes a new instance of the class. /// public PublishedRouter( - IWebRoutingSection webRoutingSection, ContentFinderCollection contentFinders, IContentLastChanceFinder contentLastChanceFinder, IVariationContextAccessor variationContextAccessor, @@ -43,7 +41,6 @@ namespace Umbraco.Web.Routing IUmbracoSettingsSection umbracoSettingsSection, IUserService userService) { - _webRoutingSection = webRoutingSection ?? throw new ArgumentNullException(nameof(webRoutingSection)); _contentFinders = contentFinders ?? throw new ArgumentNullException(nameof(contentFinders)); _contentLastChanceFinder = contentLastChanceFinder ?? throw new ArgumentNullException(nameof(contentLastChanceFinder)); _services = services ?? throw new ArgumentNullException(nameof(services)); @@ -644,74 +641,14 @@ namespace Umbraco.Web.Routing return; } - // read the alternate template alias, from querystring, form, cookie or server vars, - // only if the published content is the initial once, else the alternate template - // does not apply - // + optionally, apply the alternate template on internal redirects - var useAltTemplate = request.IsInitialPublishedContent - || (_webRoutingSection.InternalRedirectPreservesTemplate && request.IsInternalRedirectPublishedContent); - var altTemplate = useAltTemplate - ? request.UmbracoContext.HttpContext.Request[Constants.Conventions.Url.AltTemplate] - : null; - - if (string.IsNullOrWhiteSpace(altTemplate)) + if (request.HasTemplate) { - // we don't have an alternate template specified. use the current one if there's one already, - // which can happen if a content lookup also set the template (LookupByNiceUrlAndTemplate...), - // else lookup the template id on the document then lookup the template with that id. - - if (request.HasTemplate) - { - _logger.Debug("FindTemplate: Has a template already, and no alternate template."); - return; - } - - // TODO: When we remove the need for a database for templates, then this id should be irrelevant, - // not sure how were going to do this nicely. - - // TODO: We need to limit altTemplate to only allow templates that are assigned to the current document type! - // if the template isn't assigned to the document type we should log a warning and return 404 - - var templateId = request.PublishedContent.TemplateId; - request.TemplateModel = GetTemplateModel(templateId); + _logger.Debug("FindTemplate: Has a template already, and no alternate template."); + return; } - else - { - // we have an alternate template specified. lookup the template with that alias - // this means the we override any template that a content lookup might have set - // so /path/to/page/template1?altTemplate=template2 will use template2 - // ignore if the alias does not match - just trace - - if (request.HasTemplate) - _logger.Debug("FindTemplate: Has a template already, but also an alternative template."); - _logger.Debug("FindTemplate: Look for alternative template alias={AltTemplate}", altTemplate); - - // IsAllowedTemplate deals both with DisableAlternativeTemplates and ValidateAlternativeTemplates settings - if (request.PublishedContent.IsAllowedTemplate(altTemplate)) - { - // allowed, use - var template = _services.FileService.GetTemplate(altTemplate); - - if (template != null) - { - request.TemplateModel = template; - _logger.Debug("FindTemplate: Got alternative template id={TemplateId} alias={TemplateAlias}", template.Id, template.Alias); - } - else - { - _logger.Debug("FindTemplate: The alternative template with alias={AltTemplate} does not exist, ignoring.", altTemplate); - } - } - else - { - _logger.Warn("FindTemplate: Alternative template {TemplateAlias} is not allowed on node {NodeId}, ignoring.", altTemplate, request.PublishedContent.Id); - - // no allowed, back to default - var templateId = request.PublishedContent.TemplateId; - request.TemplateModel = GetTemplateModel(templateId); - } - } + var templateId = request.PublishedContent.TemplateId; + request.TemplateModel = GetTemplateModel(templateId); if (request.HasTemplate == false) { diff --git a/src/Umbraco.Web/Templates/TemplateRenderer.cs b/src/Umbraco.Web/Templates/TemplateRenderer.cs index b13719f6e9..8e8d1b5068 100644 --- a/src/Umbraco.Web/Templates/TemplateRenderer.cs +++ b/src/Umbraco.Web/Templates/TemplateRenderer.cs @@ -96,7 +96,7 @@ namespace Umbraco.Web.Templates //First, save all of the items locally that we know are used in the chain of execution, we'll need to restore these //after this page has rendered. - SaveExistingItems(out var oldPublishedRequest, out var oldAltTemplate); + SaveExistingItems(out var oldPublishedRequest); try { @@ -109,7 +109,7 @@ namespace Umbraco.Web.Templates finally { //restore items on context objects to continuing rendering the parent template - RestoreItems(oldPublishedRequest, oldAltTemplate); + RestoreItems(oldPublishedRequest); } } @@ -172,28 +172,25 @@ namespace Umbraco.Web.Templates private void SetNewItemsOnContextObjects(PublishedRequest request) { //now, set the new ones for this page execution - _umbracoContextAccessor.UmbracoContext.HttpContext.Items[Core.Constants.Conventions.Url.AltTemplate] = null; _umbracoContextAccessor.UmbracoContext.PublishedRequest = request; } /// /// Save all items that we know are used for rendering execution to variables so we can restore after rendering /// - private void SaveExistingItems(out PublishedRequest oldPublishedRequest, out object oldAltTemplate) + private void SaveExistingItems(out PublishedRequest oldPublishedRequest) { //Many objects require that these legacy items are in the http context items... before we render this template we need to first //save the values in them so that we can re-set them after we render so the rest of the execution works as per normal oldPublishedRequest = _umbracoContextAccessor.UmbracoContext.PublishedRequest; - oldAltTemplate = _umbracoContextAccessor.UmbracoContext.HttpContext.Items[Core.Constants.Conventions.Url.AltTemplate]; } /// /// Restores all items back to their context's to continue normal page rendering execution /// - private void RestoreItems(PublishedRequest oldPublishedRequest, object oldAltTemplate) + private void RestoreItems(PublishedRequest oldPublishedRequest) { _umbracoContextAccessor.UmbracoContext.PublishedRequest = oldPublishedRequest; - _umbracoContextAccessor.UmbracoContext.HttpContext.Items[Core.Constants.Conventions.Url.AltTemplate] = oldAltTemplate; } } } From 0f6c945fe0e3338e32e2d9bc44fddfbc565241c8 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Wed, 12 Feb 2020 11:12:09 +0100 Subject: [PATCH 2/7] Fix tests --- src/Umbraco.Tests/TestHelpers/BaseWebTest.cs | 12 +----------- src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs | 2 +- src/Umbraco.Tests/Web/Mvc/UmbracoViewPageTests.cs | 2 +- 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/Umbraco.Tests/TestHelpers/BaseWebTest.cs b/src/Umbraco.Tests/TestHelpers/BaseWebTest.cs index 176664b467..01e2c66e3c 100644 --- a/src/Umbraco.Tests/TestHelpers/BaseWebTest.cs +++ b/src/Umbraco.Tests/TestHelpers/BaseWebTest.cs @@ -40,10 +40,6 @@ namespace Umbraco.Tests.TestHelpers // need to specify a custom callback for unit tests // AutoPublishedContentTypes generates properties automatically - var dataTypeService = new TestObjects.TestDataTypeService( - new DataType(new VoidEditor(Mock.Of(), Mock.Of(), Mock.Of(),Mock.Of(), Mock.Of())) { Id = 1 }); - - var factory = new PublishedContentTypeFactory(Mock.Of(), new PropertyValueConverterCollection(Array.Empty()), dataTypeService); var type = new AutoPublishedContentType(0, "anything", new PublishedPropertyType[] { }); ContentTypesCache.GetPublishedContentTypeByAlias = alias => GetPublishedContentTypeByAlias(alias) ?? type; } @@ -87,15 +83,9 @@ namespace Umbraco.Tests.TestHelpers "; } - internal PublishedRouter CreatePublishedRouter(IFactory container = null, ContentFinderCollection contentFinders = null) - { - return CreatePublishedRouter(TestObjects.GetUmbracoSettings().WebRouting, container ?? Factory, contentFinders); - } - - internal static PublishedRouter CreatePublishedRouter(IWebRoutingSection webRoutingSection, IFactory container = null, ContentFinderCollection contentFinders = null) + internal static PublishedRouter CreatePublishedRouter(IFactory container = null, ContentFinderCollection contentFinders = null) { return new PublishedRouter( - webRoutingSection, contentFinders ?? new ContentFinderCollection(Enumerable.Empty()), new TestLastChanceFinder(), new TestVariationContextAccessor(), diff --git a/src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs b/src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs index c54192f869..76a1d0e23d 100644 --- a/src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs +++ b/src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs @@ -160,7 +160,7 @@ namespace Umbraco.Tests.Web.Mvc var content = Mock.Of(publishedContent => publishedContent.Id == 12345); var contextBase = umbracoContext.HttpContext; - var publishedRouter = BaseWebTest.CreatePublishedRouter(TestObjects.GetUmbracoSettings().WebRouting); + var publishedRouter = BaseWebTest.CreatePublishedRouter(); var frequest = publishedRouter.CreateRequest(umbracoContext, new Uri("http://localhost/test")); frequest.PublishedContent = content; diff --git a/src/Umbraco.Tests/Web/Mvc/UmbracoViewPageTests.cs b/src/Umbraco.Tests/Web/Mvc/UmbracoViewPageTests.cs index 942666c7b0..638189b963 100644 --- a/src/Umbraco.Tests/Web/Mvc/UmbracoViewPageTests.cs +++ b/src/Umbraco.Tests/Web/Mvc/UmbracoViewPageTests.cs @@ -391,7 +391,7 @@ namespace Umbraco.Tests.Web.Mvc logger, settings, "/dang", 0); - var publishedRouter = BaseWebTest.CreatePublishedRouter(TestObjects.GetUmbracoSettings().WebRouting); + var publishedRouter = BaseWebTest.CreatePublishedRouter(); var frequest = publishedRouter.CreateRequest(umbracoContext, new Uri("http://localhost/dang")); frequest.Culture = CultureInfo.InvariantCulture; From 99045eaaa48ea06411fcce0928737965d04f7668 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 13 Feb 2020 09:17:57 +0100 Subject: [PATCH 3/7] Reintroduced some code that could not be removed --- .../Constants-Conventions.cs | 12 +++ src/Umbraco.Web/Routing/PublishedRouter.cs | 77 +++++++++++++++++-- 2 files changed, 82 insertions(+), 7 deletions(-) diff --git a/src/Umbraco.Abstractions/Constants-Conventions.cs b/src/Umbraco.Abstractions/Constants-Conventions.cs index 6672f22239..0bfb890abd 100644 --- a/src/Umbraco.Abstractions/Constants-Conventions.cs +++ b/src/Umbraco.Abstractions/Constants-Conventions.cs @@ -208,6 +208,17 @@ namespace Umbraco.Core public const string AllMembersListId = "all-members"; } + /// + /// Constants for Umbraco URLs/Querystrings. + /// + public static class Url + { + /// + /// Querystring parameter name used for Umbraco's alternative template functionality. + /// + public const string AltTemplate = "altTemplate"; + } + /// /// Defines the alias identifiers for built-in Umbraco relation types. /// @@ -274,6 +285,7 @@ namespace Umbraco.Core //TODO: return a list of built in types so we can use that to prevent deletion in the uI } + } } } diff --git a/src/Umbraco.Web/Routing/PublishedRouter.cs b/src/Umbraco.Web/Routing/PublishedRouter.cs index 74fe9f93e6..9148ce2e31 100644 --- a/src/Umbraco.Web/Routing/PublishedRouter.cs +++ b/src/Umbraco.Web/Routing/PublishedRouter.cs @@ -20,6 +20,7 @@ namespace Umbraco.Web.Routing /// public class PublishedRouter : IPublishedRouter { + private readonly IWebRoutingSection _webRoutingSection; private readonly ContentFinderCollection _contentFinders; private readonly IContentLastChanceFinder _contentLastChanceFinder; private readonly ServiceContext _services; @@ -33,6 +34,7 @@ namespace Umbraco.Web.Routing /// Initializes a new instance of the class. /// public PublishedRouter( + IWebRoutingSection webRoutingSection, ContentFinderCollection contentFinders, IContentLastChanceFinder contentLastChanceFinder, IVariationContextAccessor variationContextAccessor, @@ -41,6 +43,7 @@ namespace Umbraco.Web.Routing IUmbracoSettingsSection umbracoSettingsSection, IUserService userService) { + _webRoutingSection = webRoutingSection ?? throw new ArgumentNullException(nameof(webRoutingSection)); _contentFinders = contentFinders ?? throw new ArgumentNullException(nameof(contentFinders)); _contentLastChanceFinder = contentLastChanceFinder ?? throw new ArgumentNullException(nameof(contentLastChanceFinder)); _services = services ?? throw new ArgumentNullException(nameof(services)); @@ -641,14 +644,74 @@ namespace Umbraco.Web.Routing return; } - if (request.HasTemplate) - { - _logger.Debug("FindTemplate: Has a template already, and no alternate template."); - return; - } + // read the alternate template alias, from querystring, form, cookie or server vars, + // only if the published content is the initial once, else the alternate template + // does not apply + // + optionally, apply the alternate template on internal redirects + var useAltTemplate = request.IsInitialPublishedContent + || (_webRoutingSection.InternalRedirectPreservesTemplate && request.IsInternalRedirectPublishedContent); + var altTemplate = useAltTemplate + ? request.UmbracoContext.HttpContext.Request[Constants.Conventions.Url.AltTemplate] + : null; - var templateId = request.PublishedContent.TemplateId; - request.TemplateModel = GetTemplateModel(templateId); + if (string.IsNullOrWhiteSpace(altTemplate)) + { + // we don't have an alternate template specified. use the current one if there's one already, + // which can happen if a content lookup also set the template (LookupByNiceUrlAndTemplate...), + // else lookup the template id on the document then lookup the template with that id. + + if (request.HasTemplate) + { + _logger.Debug("FindTemplate: Has a template already, and no alternate template."); + return; + } + + // TODO: When we remove the need for a database for templates, then this id should be irrelevant, + // not sure how were going to do this nicely. + + // TODO: We need to limit altTemplate to only allow templates that are assigned to the current document type! + // if the template isn't assigned to the document type we should log a warning and return 404 + + var templateId = request.PublishedContent.TemplateId; + request.TemplateModel = GetTemplateModel(templateId); + } + else + { + // we have an alternate template specified. lookup the template with that alias + // this means the we override any template that a content lookup might have set + // so /path/to/page/template1?altTemplate=template2 will use template2 + + // ignore if the alias does not match - just trace + + if (request.HasTemplate) + _logger.Debug("FindTemplate: Has a template already, but also an alternative template."); + _logger.Debug("FindTemplate: Look for alternative template alias={AltTemplate}", altTemplate); + + // IsAllowedTemplate deals both with DisableAlternativeTemplates and ValidateAlternativeTemplates settings + if (request.PublishedContent.IsAllowedTemplate(altTemplate)) + { + // allowed, use + var template = _services.FileService.GetTemplate(altTemplate); + + if (template != null) + { + request.TemplateModel = template; + _logger.Debug("FindTemplate: Got alternative template id={TemplateId} alias={TemplateAlias}", template.Id, template.Alias); + } + else + { + _logger.Debug("FindTemplate: The alternative template with alias={AltTemplate} does not exist, ignoring.", altTemplate); + } + } + else + { + _logger.Warn("FindTemplate: Alternative template {TemplateAlias} is not allowed on node {NodeId}, ignoring.", altTemplate, request.PublishedContent.Id); + + // no allowed, back to default + var templateId = request.PublishedContent.TemplateId; + request.TemplateModel = GetTemplateModel(templateId); + } + } if (request.HasTemplate == false) { From 04b1e3734814337decd01e17454b709875741110 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 13 Feb 2020 09:38:16 +0100 Subject: [PATCH 4/7] Reintroduced some code that could not be removed --- src/Umbraco.Tests/TestHelpers/BaseWebTest.cs | 8 +++++++- src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs | 2 +- src/Umbraco.Tests/Web/Mvc/UmbracoViewPageTests.cs | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Tests/TestHelpers/BaseWebTest.cs b/src/Umbraco.Tests/TestHelpers/BaseWebTest.cs index 01e2c66e3c..3fa61e8961 100644 --- a/src/Umbraco.Tests/TestHelpers/BaseWebTest.cs +++ b/src/Umbraco.Tests/TestHelpers/BaseWebTest.cs @@ -83,9 +83,15 @@ namespace Umbraco.Tests.TestHelpers "; } - internal static PublishedRouter CreatePublishedRouter(IFactory container = null, ContentFinderCollection contentFinders = null) + internal PublishedRouter CreatePublishedRouter(IFactory container = null, ContentFinderCollection contentFinders = null) + { + return CreatePublishedRouter(TestObjects.GetUmbracoSettings().WebRouting, container ?? Factory, contentFinders); + } + + internal static PublishedRouter CreatePublishedRouter(IWebRoutingSection webRoutingSection, IFactory container = null, ContentFinderCollection contentFinders = null) { return new PublishedRouter( + webRoutingSection, contentFinders ?? new ContentFinderCollection(Enumerable.Empty()), new TestLastChanceFinder(), new TestVariationContextAccessor(), diff --git a/src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs b/src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs index 76a1d0e23d..c54192f869 100644 --- a/src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs +++ b/src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs @@ -160,7 +160,7 @@ namespace Umbraco.Tests.Web.Mvc var content = Mock.Of(publishedContent => publishedContent.Id == 12345); var contextBase = umbracoContext.HttpContext; - var publishedRouter = BaseWebTest.CreatePublishedRouter(); + var publishedRouter = BaseWebTest.CreatePublishedRouter(TestObjects.GetUmbracoSettings().WebRouting); var frequest = publishedRouter.CreateRequest(umbracoContext, new Uri("http://localhost/test")); frequest.PublishedContent = content; diff --git a/src/Umbraco.Tests/Web/Mvc/UmbracoViewPageTests.cs b/src/Umbraco.Tests/Web/Mvc/UmbracoViewPageTests.cs index 638189b963..942666c7b0 100644 --- a/src/Umbraco.Tests/Web/Mvc/UmbracoViewPageTests.cs +++ b/src/Umbraco.Tests/Web/Mvc/UmbracoViewPageTests.cs @@ -391,7 +391,7 @@ namespace Umbraco.Tests.Web.Mvc logger, settings, "/dang", 0); - var publishedRouter = BaseWebTest.CreatePublishedRouter(); + var publishedRouter = BaseWebTest.CreatePublishedRouter(TestObjects.GetUmbracoSettings().WebRouting); var frequest = publishedRouter.CreateRequest(umbracoContext, new Uri("http://localhost/dang")); frequest.Culture = CultureInfo.InvariantCulture; From 31d71ad86e955713e9fb16833691a9cae7105113 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 14 Feb 2020 12:30:56 +1100 Subject: [PATCH 5/7] Removes methods/props from interfaces that don't need to be there, removes unused code, obsoletes some code we need to refactor --- src/Umbraco.Abstractions/IUmbracoContext.cs | 2 - .../Routing/IPublishedRequest.cs | 8 -- .../Routing/IPublishedUrlProvider.cs | 7 +- .../Security/IWebSecurity.cs | 36 +------ src/Umbraco.Web/Routing/UrlProvider.cs | 7 +- src/Umbraco.Web/Security/WebSecurity.cs | 85 ++------------- src/Umbraco.Web/Umbraco.Web.csproj | 1 - .../UmbracoAuthorizedHttpHandler.cs | 100 ------------------ 8 files changed, 17 insertions(+), 229 deletions(-) delete mode 100644 src/Umbraco.Web/UmbracoAuthorizedHttpHandler.cs diff --git a/src/Umbraco.Abstractions/IUmbracoContext.cs b/src/Umbraco.Abstractions/IUmbracoContext.cs index 3bc51f224f..b38a031f88 100644 --- a/src/Umbraco.Abstractions/IUmbracoContext.cs +++ b/src/Umbraco.Abstractions/IUmbracoContext.cs @@ -88,8 +88,6 @@ namespace Umbraco.Web /// bool InPreviewMode { get; } - string PreviewToken { get; } - /// /// Gets the url of a content identified by its identifier. /// diff --git a/src/Umbraco.Abstractions/Routing/IPublishedRequest.cs b/src/Umbraco.Abstractions/Routing/IPublishedRequest.cs index 6856b09127..f357108a4e 100644 --- a/src/Umbraco.Abstractions/Routing/IPublishedRequest.cs +++ b/src/Umbraco.Abstractions/Routing/IPublishedRequest.cs @@ -123,14 +123,6 @@ namespace Umbraco.Web.Routing /// should use the specified description. The description will or will not be used, in due time. string ResponseStatusDescription { get; } - /// - /// Gets or sets the System.Web.HttpCacheability - /// -// Note: we used to set a default value here but that would then be the default -// for ALL requests, we shouldn't overwrite it though if people are using [OutputCache] for example -// see: https://our.umbraco.com/forum/using-umbraco-and-getting-started/79715-output-cache-in-umbraco-752 - //HttpCacheability Cacheability { get; set; } - /// /// Gets or sets a list of Extensions to append to the Response.Cache object. /// diff --git a/src/Umbraco.Abstractions/Routing/IPublishedUrlProvider.cs b/src/Umbraco.Abstractions/Routing/IPublishedUrlProvider.cs index a26f3efaff..45faf76772 100644 --- a/src/Umbraco.Abstractions/Routing/IPublishedUrlProvider.cs +++ b/src/Umbraco.Abstractions/Routing/IPublishedUrlProvider.cs @@ -12,11 +12,6 @@ namespace Umbraco.Web.Routing /// UrlMode Mode { get; set; } - UrlMode GetMode(bool absolute); - IPublishedContent GetDocument(int id); - IPublishedContent GetDocument(Guid id); - IPublishedContent GetMedia(Guid id); - /// /// Gets the url of a published content. /// @@ -107,4 +102,4 @@ namespace Umbraco.Web.Routing /// string GetMediaUrl(IPublishedContent content, UrlMode mode = UrlMode.Default, string culture = null, string propertyAlias = Constants.Conventions.Media.File, Uri current = null); } -} \ No newline at end of file +} diff --git a/src/Umbraco.Abstractions/Security/IWebSecurity.cs b/src/Umbraco.Abstractions/Security/IWebSecurity.cs index cc268b87b4..0822b5cb69 100644 --- a/src/Umbraco.Abstractions/Security/IWebSecurity.cs +++ b/src/Umbraco.Abstractions/Security/IWebSecurity.cs @@ -1,3 +1,4 @@ +using System; using Umbraco.Core; using Umbraco.Core.Models.Membership; @@ -11,41 +12,18 @@ namespace Umbraco.Web.Security /// The current user. IUser CurrentUser { get; } - /// - /// Logs a user in. - /// - /// The user Id - /// returns the number of seconds until their session times out + [Obsolete("This needs to be removed, ASP.NET Identity should always be used for this operation, this is currently only used in the installer which needs to be updated")] double PerformLogin(int userId); - /// - /// Clears the current login for the currently logged in user - /// + [Obsolete("This needs to be removed, ASP.NET Identity should always be used for this operation, this is currently only used in the installer which needs to be updated")] void ClearCurrentLogin(); - /// - /// Validates credentials for a back office user - /// - /// - /// - /// - /// - /// This uses ASP.NET Identity to perform the validation - /// - bool ValidateBackOfficeCredentials(string username, string password); - /// /// Gets the current user's id. /// /// Attempt GetUserId(); - /// - /// Returns the current user's unique session id - used to mitigate csrf attacks or any other reason to validate a request - /// - /// - string GetSessionId(); - /// /// Validates the currently logged in user and ensures they are not timed out /// @@ -75,14 +53,6 @@ namespace Umbraco.Web.Security /// bool UserHasSectionAccess(string section, IUser user); - /// - /// Checks if the specified user by username as access to the app - /// - /// - /// - /// - bool UserHasSectionAccess(string section, string username); - /// /// Ensures that a back office user is logged in /// diff --git a/src/Umbraco.Web/Routing/UrlProvider.cs b/src/Umbraco.Web/Routing/UrlProvider.cs index 693482db76..2ce673dcce 100644 --- a/src/Umbraco.Web/Routing/UrlProvider.cs +++ b/src/Umbraco.Web/Routing/UrlProvider.cs @@ -73,10 +73,9 @@ namespace Umbraco.Web.Routing #region GetUrl - public UrlMode GetMode(bool absolute) => absolute ? UrlMode.Absolute : Mode; - public IPublishedContent GetDocument(int id) => _umbracoContext.Content.GetById(id); - public IPublishedContent GetDocument(Guid id) => _umbracoContext.Content.GetById(id); - public IPublishedContent GetMedia(Guid id) => _umbracoContext.Media.GetById(id); + private IPublishedContent GetDocument(int id) => _umbracoContext.Content.GetById(id); + private IPublishedContent GetDocument(Guid id) => _umbracoContext.Content.GetById(id); + private IPublishedContent GetMedia(Guid id) => _umbracoContext.Media.GetById(id); /// /// Gets the url of a published content. diff --git a/src/Umbraco.Web/Security/WebSecurity.cs b/src/Umbraco.Web/Security/WebSecurity.cs index 9a5bfb2437..c809838c73 100644 --- a/src/Umbraco.Web/Security/WebSecurity.cs +++ b/src/Umbraco.Web/Security/WebSecurity.cs @@ -10,9 +10,7 @@ using Microsoft.Owin; using Umbraco.Core.Configuration; using Umbraco.Core.IO; using Umbraco.Core.Models; -using Umbraco.Core.Models.Identity; using Umbraco.Web.Models.Identity; -using Current = Umbraco.Web.Composing.Current; namespace Umbraco.Web.Security { @@ -41,7 +39,7 @@ namespace Umbraco.Web.Security /// Gets the current user. /// /// The current user. - public virtual IUser CurrentUser + public IUser CurrentUser { get { @@ -78,12 +76,8 @@ namespace Umbraco.Web.Security protected BackOfficeUserManager UserManager => _userManager ?? (_userManager = _httpContext.GetOwinContext().GetBackOfficeUserManager()); - /// - /// Logs a user in. - /// - /// The user Id - /// returns the number of seconds until their session times out - public virtual double PerformLogin(int userId) + [Obsolete("This needs to be removed, ASP.NET Identity should always be used for this operation, this is currently only used in the installer which needs to be updated")] + public double PerformLogin(int userId) { var owinCtx = _httpContext.GetOwinContext(); //ensure it's done for owin too @@ -98,10 +92,8 @@ namespace Umbraco.Web.Security return TimeSpan.FromMinutes(_globalSettings.TimeOutInMinutes).TotalSeconds; } - /// - /// Clears the current login for the currently logged in user - /// - public virtual void ClearCurrentLogin() + [Obsolete("This needs to be removed, ASP.NET Identity should always be used for this operation, this is currently only used in the installer which needs to be updated")] + public void ClearCurrentLogin() { _httpContext.UmbracoLogout(); _httpContext.GetOwinContext().Authentication.SignOut( @@ -112,67 +104,26 @@ namespace Umbraco.Web.Security /// /// Renews the user's login ticket /// - public virtual void RenewLoginTimeout() + public void RenewLoginTimeout() { _httpContext.RenewUmbracoAuthTicket(); } - /// - /// Validates credentials for a back office user - /// - /// - /// - /// - /// - /// This uses ASP.NET Identity to perform the validation - /// - public virtual bool ValidateBackOfficeCredentials(string username, string password) - { - //find the user by username - var user = UserManager.FindByNameAsync(username).Result; - return user != null && UserManager.CheckPasswordAsync(user, password).Result; - } - - /// - /// Validates the current user to see if they have access to the specified app - /// - /// - /// - internal bool ValidateUserApp(string app) - { - //if it is empty, don't validate - if (app.IsNullOrWhiteSpace()) - { - return true; - } - return CurrentUser.AllowedSections.Any(uApp => uApp.InvariantEquals(app)); - } - /// /// Gets the current user's id. /// /// - public virtual Attempt GetUserId() + public Attempt GetUserId() { var identity = _httpContext.GetCurrentIdentity(false); return identity == null ? Attempt.Fail() : Attempt.Succeed(Convert.ToInt32(identity.Id)); } - /// - /// Returns the current user's unique session id - used to mitigate csrf attacks or any other reason to validate a request - /// - /// - public virtual string GetSessionId() - { - var identity = _httpContext.GetCurrentIdentity(false); - return identity?.SessionId; - } - /// /// Validates the currently logged in user and ensures they are not timed out /// /// - public virtual bool ValidateCurrentUser() + public bool ValidateCurrentUser() { return ValidateCurrentUser(false, true) == ValidateRequestAttempt.Success; } @@ -183,7 +134,7 @@ namespace Umbraco.Web.Security /// set to true if you want exceptions to be thrown if failed /// If true requires that the user is approved to be validated /// - public virtual ValidateRequestAttempt ValidateCurrentUser(bool throwExceptions, bool requiresApproval = true) + public ValidateRequestAttempt ValidateCurrentUser(bool throwExceptions, bool requiresApproval = true) { //This will first check if the current user is already authenticated - which should be the case in nearly all circumstances // since the authentication happens in the Module, that authentication also checks the ticket expiry. We don't @@ -235,27 +186,11 @@ namespace Umbraco.Web.Security /// /// /// - public virtual bool UserHasSectionAccess(string section, IUser user) + public bool UserHasSectionAccess(string section, IUser user) { return user.HasSectionAccess(section); } - /// - /// Checks if the specified user by username as access to the app - /// - /// - /// - /// - public bool UserHasSectionAccess(string section, string username) - { - var user = _userService.GetByUsername(username); - if (user == null) - { - return false; - } - return user.HasSectionAccess(section); - } - /// /// Ensures that a back office user is logged in /// diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index d0936f0d6f..c4e82142f6 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -705,7 +705,6 @@ True Reference.map - Component diff --git a/src/Umbraco.Web/UmbracoAuthorizedHttpHandler.cs b/src/Umbraco.Web/UmbracoAuthorizedHttpHandler.cs deleted file mode 100644 index 394c25dc6f..0000000000 --- a/src/Umbraco.Web/UmbracoAuthorizedHttpHandler.cs +++ /dev/null @@ -1,100 +0,0 @@ -using System; -using System.Security; -using Umbraco.Core; -using Umbraco.Core.Cache; -using Umbraco.Core.Logging; -using Umbraco.Web.Security; -using Umbraco.Core.Models.Membership; -using Umbraco.Core.Services; - -namespace Umbraco.Web -{ - public abstract class UmbracoAuthorizedHttpHandler : UmbracoHttpHandler - { - protected UmbracoAuthorizedHttpHandler() - { - } - - protected UmbracoAuthorizedHttpHandler(IUmbracoContextAccessor umbracoContextAccessor, UmbracoHelper umbracoHelper, ServiceContext service, IProfilingLogger plogger) : base(umbracoContextAccessor, umbracoHelper, service, plogger) - { - } - - /// - /// Checks if the umbraco context id is valid - /// - /// - /// - protected bool ValidateUserContextId(string currentUmbracoUserContextId) - { - return Security.ValidateCurrentUser(); - } - - /// - /// Checks if the username/password credentials are valid - /// - /// - /// - /// - protected bool ValidateCredentials(string username, string password) - { - return Security.ValidateBackOfficeCredentials(username, password); - } - - /// - /// Validates the user for access to a certain application - /// - /// The application alias. - /// true if an exception should be thrown if authorization fails - /// - protected bool AuthorizeRequest(string app, bool throwExceptions = false) - { - //ensure we have a valid user first! - if (!AuthorizeRequest(throwExceptions)) return false; - - //if it is empty, don't validate - if (app.IsNullOrWhiteSpace()) - { - return true; - } - var hasAccess = UserHasAppAccess(app, Security.CurrentUser); - if (!hasAccess && throwExceptions) - throw new SecurityException("The user does not have access to the required application"); - return hasAccess; - } - - /// - /// Checks if the specified user as access to the app - /// - /// - /// - /// - protected bool UserHasAppAccess(string app, IUser user) - { - return Security.UserHasSectionAccess(app, user); - } - - /// - /// Checks if the specified user by username as access to the app - /// - /// - /// - /// - protected bool UserHasAppAccess(string app, string username) - { - return Security.UserHasSectionAccess(app, username); - } - - /// - /// Returns true if there is a valid logged in user and that ssl is enabled if required - /// - /// true if an exception should be thrown if authorization fails - /// - protected bool AuthorizeRequest(bool throwExceptions = false) - { - var result = Security.AuthorizeRequest(throwExceptions); - return result == ValidateRequestAttempt.Success; - } - - - } -} From e89096eeb67064760e060de15434a3f5df1f285b Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 14 Feb 2020 12:36:22 +1100 Subject: [PATCH 6/7] Dont call Thread.GetDomain() more than needed --- src/Umbraco.Web/UmbracoContextFactory.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Web/UmbracoContextFactory.cs b/src/Umbraco.Web/UmbracoContextFactory.cs index 50d450112e..4c7ca2c2a3 100644 --- a/src/Umbraco.Web/UmbracoContextFactory.cs +++ b/src/Umbraco.Web/UmbracoContextFactory.cs @@ -90,7 +90,8 @@ namespace Umbraco.Web public static HttpContextBase EnsureHttpContext(HttpContextBase httpContext = null) { - if (Thread.GetDomain().GetData(".appPath") is null || Thread.GetDomain().GetData(".appVPath") is null) + var domain = Thread.GetDomain(); + if (domain.GetData(".appPath") is null || domain.GetData(".appVPath") is null) { return httpContext ?? new HttpContextWrapper(HttpContext.Current ?? new HttpContext(new SimpleWorkerRequest("", "", "null.aspx", "", NullWriterInstance))); From 6ccd63694dce1229990fb4ab7cc20387061326e1 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Fri, 14 Feb 2020 08:51:17 +0100 Subject: [PATCH 7/7] Fix merge issue --- src/Umbraco.Web/Templates/TemplateRenderer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Web/Templates/TemplateRenderer.cs b/src/Umbraco.Web/Templates/TemplateRenderer.cs index 4b9e9fa479..da5de1055c 100644 --- a/src/Umbraco.Web/Templates/TemplateRenderer.cs +++ b/src/Umbraco.Web/Templates/TemplateRenderer.cs @@ -181,7 +181,7 @@ namespace Umbraco.Web.Templates /// /// Save all items that we know are used for rendering execution to variables so we can restore after rendering /// - private void SaveExistingItems(out PublishedRequest oldPublishedRequest) + private void SaveExistingItems(out IPublishedRequest oldPublishedRequest) { //Many objects require that these legacy items are in the http context items... before we render this template we need to first //save the values in them so that we can re-set them after we render so the rest of the execution works as per normal @@ -191,7 +191,7 @@ namespace Umbraco.Web.Templates /// /// Restores all items back to their context's to continue normal page rendering execution /// - private void RestoreItems(PublishedRequest oldPublishedRequest) + private void RestoreItems(IPublishedRequest oldPublishedRequest) { _umbracoContextAccessor.UmbracoContext.PublishedRequest = oldPublishedRequest; }