From faf06be61807f045782e8e3531e6257ad97d9d49 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 2 Jun 2022 12:19:22 +0200 Subject: [PATCH] Changes to Basic Auth to support external logins (#12434) * Fixed issues with basic auth middleware to support Umbraco Cloud usecase * Fix redirects to return url, now allows website urls * Strip potential domain part of returnPath * Fix default value in appsettings schema * Reintroduce check of basic auth enabled. * Fix wrong negation introduced in #12349 * Fixed issues with redirects * Also check external login cookie, while authenticating backoffice --- .../Configuration/Models/BasicAuthSettings.cs | 13 +++++ src/Umbraco.Core/Services/BasicAuthService.cs | 15 +++++ .../Services/IBasicAuthService.cs | 4 ++ .../Controllers/BackOfficeController.cs | 2 +- .../Extensions/HttpContextExtensions.cs | 7 +++ .../application/umblogin.directive.js | 9 ++- src/Umbraco.Web.UI.Client/src/init.js | 4 +- .../src/views/common/login.controller.js | 11 ++-- .../BasicAuthenticationMiddleware.cs | 56 ++++++++++++++++--- 9 files changed, 102 insertions(+), 19 deletions(-) diff --git a/src/Umbraco.Core/Configuration/Models/BasicAuthSettings.cs b/src/Umbraco.Core/Configuration/Models/BasicAuthSettings.cs index 054619d843..aa82f69d2e 100644 --- a/src/Umbraco.Core/Configuration/Models/BasicAuthSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/BasicAuthSettings.cs @@ -23,5 +23,18 @@ namespace Umbraco.Cms.Core.Configuration.Models public string[] AllowedIPs { get; set; } = Array.Empty(); + public SharedSecret SharedSecret { get; set; } = new SharedSecret(); + + public bool RedirectToLoginPage { get; set; } = false; + + } + + public class SharedSecret + { + private const string StaticHeaderName = "X-Authentication-Shared-Secret"; + + [DefaultValue(StaticHeaderName)] + public string? HeaderName { get; set; } = StaticHeaderName; + public string? Value { get; set; } } } diff --git a/src/Umbraco.Core/Services/BasicAuthService.cs b/src/Umbraco.Core/Services/BasicAuthService.cs index 3021768bfe..d81469fac0 100644 --- a/src/Umbraco.Core/Services/BasicAuthService.cs +++ b/src/Umbraco.Core/Services/BasicAuthService.cs @@ -2,6 +2,7 @@ using System; 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 @@ namespace Umbraco.Cms.Core.Services.Implement } public bool IsBasicAuthEnabled() => _basicAuthSettings.Enabled; + public bool IsRedirectToLoginPageEnabled() => _basicAuthSettings.RedirectToLoginPage; public bool IsIpAllowListed(IPAddress clientIpAddress) { @@ -44,5 +46,18 @@ namespace Umbraco.Cms.Core.Services.Implement 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 84173a629a..82e48e1180 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 @@ namespace Umbraco.Cms.Core.Services { bool IsBasicAuthEnabled(); bool IsIpAllowListed(IPAddress clientIpAddress); + bool HasCorrectSharedSecret(IDictionary headers) => false; + + bool IsRedirectToLoginPageEnabled() => false; } } diff --git a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs index 9dce470b3e..26f146cfb0 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs @@ -322,7 +322,7 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers [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.Common/Extensions/HttpContextExtensions.cs b/src/Umbraco.Web.Common/Extensions/HttpContextExtensions.cs index 34af34fe45..226755039e 100644 --- a/src/Umbraco.Web.Common/Extensions/HttpContextExtensions.cs +++ b/src/Umbraco.Web.Common/Extensions/HttpContextExtensions.cs @@ -52,6 +52,13 @@ public static class HttpContextExtensions AuthenticateResult result = await httpContext.AuthenticateAsync(Constants.Security.BackOfficeAuthenticationType); + + if (!result.Succeeded) + { + result = + await httpContext.AuthenticateAsync(Constants.Security.BackOfficeExternalAuthenticationType); + } + return result; } 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 d1c8d1ac85..425ebf3a10 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.UI.Client/src/init.js b/src/Umbraco.Web.UI.Client/src/init.js index 92ed2c4944..1f8a29ee2f 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 86132fe8f3..b33d707c94 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,10 @@ angular.module('umbraco').controller("Umbraco.LoginController", function (events path = decodeURIComponent(locationObj.returnPath); } - $location.url(path); + // Ensure path is not absolute + path = path.replace(/^.*\/\/[^\/]+/, '') + + window.location.href = path; }); $scope.$on('$destroy', function () { diff --git a/src/Umbraco.Web.Website/Middleware/BasicAuthenticationMiddleware.cs b/src/Umbraco.Web.Website/Middleware/BasicAuthenticationMiddleware.cs index 0ad7b9e259..e2f4b2b09a 100644 --- a/src/Umbraco.Web.Website/Middleware/BasicAuthenticationMiddleware.cs +++ b/src/Umbraco.Web.Website/Middleware/BasicAuthenticationMiddleware.cs @@ -1,11 +1,16 @@ 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; 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 +23,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 + || !_basicAuthService.IsBasicAuthEnabled() + || context.Request.IsBackOfficeRequest() + || AllowedClientRequest(context) + || _basicAuthService.HasCorrectSharedSecret(context.Request.Headers)) { await next(context); return; @@ -67,24 +93,36 @@ public class BasicAuthenticationMiddleware : IMiddleware } 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={WebUtility.UrlEncode(context.Request.GetEncodedPathAndQuery())}" , false); + } + else + { + context.Response.StatusCode = 401; + context.Response.Headers.Add("WWW-Authenticate", "Basic realm=\"Umbraco login\""); + } } }