From 2741b84fdc34708845b2dd41ba9956bca5a1b421 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Wed, 18 May 2022 14:58:22 +0200 Subject: [PATCH 1/8] Fixed issues with basic auth middleware to support Umbraco Cloud usecase --- .../Configuration/Models/BasicAuthSettings.cs | 10 ++++ src/Umbraco.Core/Services/BasicAuthService.cs | 15 +++++ .../Services/IBasicAuthService.cs | 4 ++ .../BasicAuthenticationMiddleware.cs | 55 ++++++++++++++++--- 4 files changed, 75 insertions(+), 9 deletions(-) diff --git a/src/Umbraco.Core/Configuration/Models/BasicAuthSettings.cs b/src/Umbraco.Core/Configuration/Models/BasicAuthSettings.cs index 054619d84353..6541de66b5b1 100644 --- a/src/Umbraco.Core/Configuration/Models/BasicAuthSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/BasicAuthSettings.cs @@ -23,5 +23,15 @@ public class BasicAuthSettings public string[] AllowedIPs { get; set; } = Array.Empty(); + public SharedSecret SharedSecret { get; set; } = new SharedSecret(); + + public bool RedirectToLoginPage { get; set; } = false; + + } + + public class SharedSecret + { + public string? HeaderName { get; set; } = "X-Authentication-Shared-Secret"; + public string? Value { get; set; } } } diff --git a/src/Umbraco.Core/Services/BasicAuthService.cs b/src/Umbraco.Core/Services/BasicAuthService.cs index 3021768bfe1d..d81469fac0f2 100644 --- a/src/Umbraco.Core/Services/BasicAuthService.cs +++ b/src/Umbraco.Core/Services/BasicAuthService.cs @@ -2,6 +2,7 @@ using System.Net; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; +using Microsoft.Extensions.Primitives; using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Web.Common.DependencyInjection; @@ -31,6 +32,7 @@ public BasicAuthService(IOptionsMonitor optionsMonitor, IIpAd } public bool IsBasicAuthEnabled() => _basicAuthSettings.Enabled; + public bool IsRedirectToLoginPageEnabled() => _basicAuthSettings.RedirectToLoginPage; public bool IsIpAllowListed(IPAddress clientIpAddress) { @@ -44,5 +46,18 @@ public bool IsIpAllowListed(IPAddress clientIpAddress) return false; } + + public bool HasCorrectSharedSecret(IDictionary headers) + { + var headerName = _basicAuthSettings.SharedSecret.HeaderName; + var sharedSecret = _basicAuthSettings.SharedSecret.Value; + + if (string.IsNullOrWhiteSpace(headerName) || string.IsNullOrWhiteSpace(sharedSecret)) + { + return false; + } + + return headers.TryGetValue(headerName, out var value) && value.Equals(sharedSecret); + } } } diff --git a/src/Umbraco.Core/Services/IBasicAuthService.cs b/src/Umbraco.Core/Services/IBasicAuthService.cs index 84173a629aeb..82e48e11802f 100644 --- a/src/Umbraco.Core/Services/IBasicAuthService.cs +++ b/src/Umbraco.Core/Services/IBasicAuthService.cs @@ -1,4 +1,5 @@ using System.Net; +using Microsoft.Extensions.Primitives; namespace Umbraco.Cms.Core.Services { @@ -6,5 +7,8 @@ public interface IBasicAuthService { bool IsBasicAuthEnabled(); bool IsIpAllowListed(IPAddress clientIpAddress); + bool HasCorrectSharedSecret(IDictionary headers) => false; + + bool IsRedirectToLoginPageEnabled() => false; } } diff --git a/src/Umbraco.Web.Website/Middleware/BasicAuthenticationMiddleware.cs b/src/Umbraco.Web.Website/Middleware/BasicAuthenticationMiddleware.cs index 0ad7b9e2598b..887c499182a0 100644 --- a/src/Umbraco.Web.Website/Middleware/BasicAuthenticationMiddleware.cs +++ b/src/Umbraco.Web.Website/Middleware/BasicAuthenticationMiddleware.cs @@ -3,9 +3,13 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Hosting; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Web.BackOffice.Security; +using Umbraco.Cms.Web.Common.DependencyInjection; using Umbraco.Extensions; namespace Umbraco.Cms.Web.Common.Middleware; @@ -18,20 +22,41 @@ public class BasicAuthenticationMiddleware : IMiddleware { private readonly IBasicAuthService _basicAuthService; private readonly IRuntimeState _runtimeState; + private readonly string _backOfficePath; public BasicAuthenticationMiddleware( IRuntimeState runtimeState, - IBasicAuthService basicAuthService) + IBasicAuthService basicAuthService, + IOptionsMonitor globalSettings, + IHostingEnvironment hostingEnvironment) { _runtimeState = runtimeState; _basicAuthService = basicAuthService; + + _backOfficePath = globalSettings.CurrentValue.GetBackOfficePath(hostingEnvironment); + } + + [Obsolete("Use Ctor with all methods. This will be removed in Umbraco 12")] + public BasicAuthenticationMiddleware( + IRuntimeState runtimeState, + IBasicAuthService basicAuthService) : this( + runtimeState, + basicAuthService, + StaticServiceProvider.Instance.GetRequiredService>(), + StaticServiceProvider.Instance.GetRequiredService() + ) + { + } /// public async Task InvokeAsync(HttpContext context, RequestDelegate next) { - if (_runtimeState.Level < RuntimeLevel.Run || context.Request.IsBackOfficeRequest() || - !_basicAuthService.IsBasicAuthEnabled()) + if (_runtimeState.Level < RuntimeLevel.Run + || context.Request.IsBackOfficeRequest() + || context.Request.IsClientSideRequest() + || AllowedClientRequest(context) + || _basicAuthService.HasCorrectSharedSecret(context.Request.Headers)) { await next(context); return; @@ -67,24 +92,36 @@ public async Task InvokeAsync(HttpContext context, RequestDelegate next) } else { - SetUnauthorizedHeader(context); + HandleUnauthorized(context); } } else { - SetUnauthorizedHeader(context); + HandleUnauthorized(context); } } else { // no authorization header - SetUnauthorizedHeader(context); + HandleUnauthorized(context); } } - private static void SetUnauthorizedHeader(HttpContext context) + private bool AllowedClientRequest(HttpContext context) { - context.Response.StatusCode = 401; - context.Response.Headers.Add("WWW-Authenticate", "Basic realm=\"Umbraco login\""); + return context.Request.IsClientSideRequest() && _basicAuthService.IsRedirectToLoginPageEnabled(); + } + + private void HandleUnauthorized(HttpContext context) + { + if (_basicAuthService.IsRedirectToLoginPageEnabled()) + { + context.Response.Redirect($"{_backOfficePath}#/login/false?returnPath=%252F" , false); + } + else + { + context.Response.StatusCode = 401; + context.Response.Headers.Add("WWW-Authenticate", "Basic realm=\"Umbraco login\""); + } } } From 789a58c1377d433ab3400a40890001f5776a18ce Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 19 May 2022 11:37:32 +0200 Subject: [PATCH 2/8] Fix redirects to return url, now allows website urls --- src/Umbraco.Web.UI.Client/src/init.js | 4 ++-- .../src/views/common/login.controller.js | 9 +++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/init.js b/src/Umbraco.Web.UI.Client/src/init.js index 92ed2c4944d8..1f8a29ee2fcf 100644 --- a/src/Umbraco.Web.UI.Client/src/init.js +++ b/src/Umbraco.Web.UI.Client/src/init.js @@ -8,7 +8,7 @@ app.run(['$rootScope', '$route', '$location', '$cookies', 'urlHelper', 'appState $.ajaxSetup({ beforeSend: function (xhr) { xhr.setRequestHeader("X-UMB-XSRF-TOKEN", $cookies["UMB-XSRF-TOKEN"]); - // This is a standard header that should be sent for all ajax requests and is required for + // This is a standard header that should be sent for all ajax requests and is required for // how the server handles auth rejections, etc... see https://github.com/dotnet/aspnetcore/blob/a2568cbe1e8dd92d8a7976469100e564362f778e/src/Security/Authentication/Cookies/src/CookieAuthenticationEvents.cs#L106-L107 xhr.setRequestHeader("X-Requested-With", "XMLHttpRequest"); var queryStrings = urlHelper.getQueryStringParams(); @@ -120,7 +120,7 @@ app.run(['$rootScope', '$route', '$location', '$cookies', 'urlHelper', 'appState var returnPath = null; if (rejection.path == "/login" || rejection.path.startsWith("/login/")) { //Set the current path before redirecting so we know where to redirect back to - returnPath = encodeURIComponent($location.url()); + returnPath = encodeURIComponent(window.location.href.replace(window.location.origin,'')); } $location.path(rejection.path) if (returnPath) { diff --git a/src/Umbraco.Web.UI.Client/src/views/common/login.controller.js b/src/Umbraco.Web.UI.Client/src/views/common/login.controller.js index 86132fe8f309..a7c1c157e536 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/login.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/common/login.controller.js @@ -1,8 +1,8 @@ /** This controller is simply here to launch the login dialog when the route is explicitly changed to /login */ -angular.module('umbraco').controller("Umbraco.LoginController", function (eventsService, $scope, userService, $location, $rootScope) { +angular.module('umbraco').controller("Umbraco.LoginController", function (eventsService, $scope, userService, $location, $rootScope, $window) { + + userService._showLoginDialog(); - userService._showLoginDialog(); - var evtOn = eventsService.on("app.ready", function(evt, data){ $scope.avatar = "assets/img/application/logo.png"; @@ -14,7 +14,8 @@ angular.module('umbraco').controller("Umbraco.LoginController", function (events path = decodeURIComponent(locationObj.returnPath); } - $location.url(path); + //TODO: check path validity. + window.location.href = path; }); $scope.$on('$destroy', function () { From 85f752b34d1e1366728a5e20c2b25b34bf120d95 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 19 May 2022 12:38:15 +0200 Subject: [PATCH 3/8] Strip potential domain part of returnPath --- .../src/views/common/login.controller.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Web.UI.Client/src/views/common/login.controller.js b/src/Umbraco.Web.UI.Client/src/views/common/login.controller.js index a7c1c157e536..b33d707c9478 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/login.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/common/login.controller.js @@ -14,7 +14,9 @@ angular.module('umbraco').controller("Umbraco.LoginController", function (events path = decodeURIComponent(locationObj.returnPath); } - //TODO: check path validity. + // Ensure path is not absolute + path = path.replace(/^.*\/\/[^\/]+/, '') + window.location.href = path; }); From e655188c04c59ae009c630777cb75f52644df071 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 19 May 2022 13:26:54 +0200 Subject: [PATCH 4/8] Fix default value in appsettings schema --- src/Umbraco.Core/Configuration/Models/BasicAuthSettings.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Configuration/Models/BasicAuthSettings.cs b/src/Umbraco.Core/Configuration/Models/BasicAuthSettings.cs index 6541de66b5b1..aa82f69d2e2d 100644 --- a/src/Umbraco.Core/Configuration/Models/BasicAuthSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/BasicAuthSettings.cs @@ -31,7 +31,10 @@ public class BasicAuthSettings public class SharedSecret { - public string? HeaderName { get; set; } = "X-Authentication-Shared-Secret"; + private const string StaticHeaderName = "X-Authentication-Shared-Secret"; + + [DefaultValue(StaticHeaderName)] + public string? HeaderName { get; set; } = StaticHeaderName; public string? Value { get; set; } } } From a1178b7a467a5aca5748ff536328d8abf87d0550 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 19 May 2022 13:46:31 +0200 Subject: [PATCH 5/8] Reintroduce check of basic auth enabled. --- .../Middleware/BasicAuthenticationMiddleware.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Umbraco.Web.Website/Middleware/BasicAuthenticationMiddleware.cs b/src/Umbraco.Web.Website/Middleware/BasicAuthenticationMiddleware.cs index 887c499182a0..b791c20556e0 100644 --- a/src/Umbraco.Web.Website/Middleware/BasicAuthenticationMiddleware.cs +++ b/src/Umbraco.Web.Website/Middleware/BasicAuthenticationMiddleware.cs @@ -53,6 +53,7 @@ public BasicAuthenticationMiddleware( public async Task InvokeAsync(HttpContext context, RequestDelegate next) { if (_runtimeState.Level < RuntimeLevel.Run + || !_basicAuthService.IsBasicAuthEnabled() || context.Request.IsBackOfficeRequest() || context.Request.IsClientSideRequest() || AllowedClientRequest(context) From 1332584fb28dd58ba16f0cc1dac47142bd58145a Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Mon, 30 May 2022 14:35:42 +0200 Subject: [PATCH 6/8] Fix wrong negation introduced in #12349 --- src/Umbraco.Web.Common/Security/UmbracoSignInManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Web.Common/Security/UmbracoSignInManager.cs b/src/Umbraco.Web.Common/Security/UmbracoSignInManager.cs index 263343b8174f..0f7a7f5ba2ae 100644 --- a/src/Umbraco.Web.Common/Security/UmbracoSignInManager.cs +++ b/src/Umbraco.Web.Common/Security/UmbracoSignInManager.cs @@ -86,7 +86,7 @@ public override async Task PasswordSignInAsync(TUser user, string var providerKey = auth.Principal.FindFirstValue(ClaimTypes.NameIdentifier); var provider = items[UmbracoSignInMgrLoginProviderKey]; - if (providerKey == null || provider is not null) + if (providerKey == null || provider is null) { return null; } From 29bde2d309f3512e694e16e27aaaf1f4cbc27a25 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Wed, 1 Jun 2022 15:58:10 +0200 Subject: [PATCH 7/8] Fixed issues with redirects --- .../Controllers/BackOfficeController.cs | 2 +- .../components/application/umblogin.directive.js | 9 ++++++--- .../Middleware/BasicAuthenticationMiddleware.cs | 4 ++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs index 9dce470b3ec4..26f146cfb0f3 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs @@ -322,7 +322,7 @@ public async Task ServerVariables() [AllowAnonymous] public ActionResult ExternalLogin(string provider, string? redirectUrl = null) { - if (redirectUrl == null) + if (redirectUrl == null || Uri.TryCreate(redirectUrl, UriKind.Absolute, out _)) { redirectUrl = Url.Action(nameof(Default), this.GetControllerName()); } diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umblogin.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umblogin.directive.js index d1c8d1ac8534..425ebf3a10da 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umblogin.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umblogin.directive.js @@ -45,7 +45,10 @@ vm.allowPasswordReset = Umbraco.Sys.ServerVariables.umbracoSettings.canSendRequiredEmail && Umbraco.Sys.ServerVariables.umbracoSettings.allowPasswordReset; vm.errorMsg = ""; - vm.externalLoginFormAction = Umbraco.Sys.ServerVariables.umbracoUrls.externalLoginsUrl; + const tempUrl = new URL(Umbraco.Sys.ServerVariables.umbracoUrls.externalLoginsUrl, window.location.origin); + tempUrl.searchParams.append("redirectUrl", $location.search().returnPath ?? "") + + vm.externalLoginFormAction = tempUrl.pathname + tempUrl.search; vm.externalLoginProviders = externalLoginInfoService.getLoginProviders(); vm.externalLoginProviders.forEach(x => { x.customView = externalLoginInfoService.getLoginProviderView(x); @@ -224,8 +227,8 @@ if (formHelper.submitForm({ scope: $scope, formCtrl: vm.loginForm })) { //if the login and password are not empty we need to automatically - // validate them - this is because if there are validation errors on the server - // then the user has to change both username & password to resubmit which isn't ideal, + // validate them - this is because if there are validation errors on the server + // then the user has to change both username & password to resubmit which isn't ideal, // so if they're not empty, we'll just make sure to set them to valid. if (vm.login && vm.password && vm.login.length > 0 && vm.password.length > 0) { vm.loginForm.username.$setValidity('auth', true); diff --git a/src/Umbraco.Web.Website/Middleware/BasicAuthenticationMiddleware.cs b/src/Umbraco.Web.Website/Middleware/BasicAuthenticationMiddleware.cs index b791c20556e0..e2f4b2b09a52 100644 --- a/src/Umbraco.Web.Website/Middleware/BasicAuthenticationMiddleware.cs +++ b/src/Umbraco.Web.Website/Middleware/BasicAuthenticationMiddleware.cs @@ -1,6 +1,7 @@ using System.Net; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Extensions; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; @@ -55,7 +56,6 @@ public async Task InvokeAsync(HttpContext context, RequestDelegate next) if (_runtimeState.Level < RuntimeLevel.Run || !_basicAuthService.IsBasicAuthEnabled() || context.Request.IsBackOfficeRequest() - || context.Request.IsClientSideRequest() || AllowedClientRequest(context) || _basicAuthService.HasCorrectSharedSecret(context.Request.Headers)) { @@ -117,7 +117,7 @@ private void HandleUnauthorized(HttpContext context) { if (_basicAuthService.IsRedirectToLoginPageEnabled()) { - context.Response.Redirect($"{_backOfficePath}#/login/false?returnPath=%252F" , false); + context.Response.Redirect($"{_backOfficePath}#/login/false?returnPath={WebUtility.UrlEncode(context.Request.GetEncodedPathAndQuery())}" , false); } else { From caff24bb4806fe974e0be4064a7200518394bcdf Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 2 Jun 2022 09:29:16 +0200 Subject: [PATCH 8/8] Also check external login cookie, while authenticating backoffice --- src/Umbraco.Web.Common/Extensions/HttpContextExtensions.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Umbraco.Web.Common/Extensions/HttpContextExtensions.cs b/src/Umbraco.Web.Common/Extensions/HttpContextExtensions.cs index 34af34fe4506..226755039e8a 100644 --- a/src/Umbraco.Web.Common/Extensions/HttpContextExtensions.cs +++ b/src/Umbraco.Web.Common/Extensions/HttpContextExtensions.cs @@ -52,6 +52,13 @@ public static async Task AuthenticateBackOfficeAsync(this Ht AuthenticateResult result = await httpContext.AuthenticateAsync(Constants.Security.BackOfficeAuthenticationType); + + if (!result.Succeeded) + { + result = + await httpContext.AuthenticateAsync(Constants.Security.BackOfficeExternalAuthenticationType); + } + return result; }