diff --git a/src/Umbraco.Cms.Api.Management/Middleware/BackOfficeAuthorizationInitializationMiddleware.cs b/src/Umbraco.Cms.Api.Management/Middleware/BackOfficeAuthorizationInitializationMiddleware.cs index 60bab0d14e15..7b963fc9dd9d 100644 --- a/src/Umbraco.Cms.Api.Management/Middleware/BackOfficeAuthorizationInitializationMiddleware.cs +++ b/src/Umbraco.Cms.Api.Management/Middleware/BackOfficeAuthorizationInitializationMiddleware.cs @@ -18,7 +18,7 @@ namespace Umbraco.Cms.Api.Management.Middleware; public class BackOfficeAuthorizationInitializationMiddleware : IMiddleware { private SemaphoreSlim _firstBackOfficeRequestLocker = new(1); // this only works because this is a singleton - private ISet _knownHosts = new HashSet(); // this only works because this is a singleton + private ISet _knownHosts = new HashSet(StringComparer.OrdinalIgnoreCase); // this only works because this is a singleton private readonly UmbracoRequestPaths _umbracoRequestPaths; private readonly IServiceProvider _serviceProvider; @@ -79,30 +79,48 @@ private async Task InitializeBackOfficeAuthorizationOnceAsync(HttpContext contex await _firstBackOfficeRequestLocker.WaitAsync(); - // NOTE: _knownHosts is not thread safe; check again after entering the semaphore - if (_knownHosts.Add(host) is false) + try { - _firstBackOfficeRequestLocker.Release(); - return; - } + // NOTE: _knownHosts is not thread safe; check again after entering the semaphore. + if (_knownHosts.Contains(host)) + { + return; + } - // ensure we explicitly add UmbracoApplicationUrl if configured (https://github.com/umbraco/Umbraco-CMS/issues/16179) - if (_webRoutingSettings.UmbracoApplicationUrl.IsNullOrWhiteSpace() is false) - { - _knownHosts.Add(_webRoutingSettings.UmbracoApplicationUrl); - } + // Ensure we explicitly add UmbracoApplicationUrl if configured (https://github.com/umbraco/Umbraco-CMS/issues/16179). + var hostsToRegister = new HashSet(StringComparer.OrdinalIgnoreCase) { host }; + if (_webRoutingSettings.UmbracoApplicationUrl.IsNullOrWhiteSpace() is false) + { + hostsToRegister.Add(_webRoutingSettings.UmbracoApplicationUrl); + } - Uri[] backOfficeHosts = _knownHosts - .Select(host => Uri.TryCreate(host, UriKind.Absolute, out Uri? hostUri) - ? hostUri - : null) - .WhereNotNull() - .ToArray(); + // Merge with already-known hosts so the OpenIddict application includes all redirect URIs. + foreach (var knownHost in _knownHosts) + { + hostsToRegister.Add(knownHost); + } - using IServiceScope scope = _serviceProvider.CreateScope(); - IBackOfficeApplicationManager backOfficeApplicationManager = scope.ServiceProvider.GetRequiredService(); - await backOfficeApplicationManager.EnsureBackOfficeApplicationAsync(backOfficeHosts); + Uri[] backOfficeHosts = hostsToRegister + .Select(h => Uri.TryCreate(h, UriKind.Absolute, out Uri? hostUri) + ? hostUri + : null) + .WhereNotNull() + .ToArray(); - _firstBackOfficeRequestLocker.Release(); + using IServiceScope scope = _serviceProvider.CreateScope(); + IBackOfficeApplicationManager backOfficeApplicationManager = scope.ServiceProvider.GetRequiredService(); + await backOfficeApplicationManager.EnsureBackOfficeApplicationAsync(backOfficeHosts); + + // Only mark hosts as known after successful registration, so a transient failure + // (e.g. database contention during an unattended upgrade) is retried on the next request. + foreach (var registered in hostsToRegister) + { + _knownHosts.Add(registered); + } + } + finally + { + _firstBackOfficeRequestLocker.Release(); + } } } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Middleware/BackOfficeAuthorizationInitializationMiddlewareTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Middleware/BackOfficeAuthorizationInitializationMiddlewareTests.cs new file mode 100644 index 000000000000..1c60c6b02f57 --- /dev/null +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Middleware/BackOfficeAuthorizationInitializationMiddlewareTests.cs @@ -0,0 +1,163 @@ +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using Moq; +using NUnit.Framework; +using Umbraco.Cms.Api.Management.Middleware; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Hosting; +using Umbraco.Cms.Core.Routing; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Infrastructure.Security; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Cms.Api.Management.Middleware; + +[TestFixture] +public class BackOfficeAuthorizationInitializationMiddlewareTests +{ + private Mock _mockRuntimeState = null!; + private Mock _mockBackOfficeApplicationManager = null!; + private BackOfficeAuthorizationInitializationMiddleware _middleware = null!; + + [SetUp] + public void SetUp() + { + _mockRuntimeState = new Mock(); + _mockRuntimeState.Setup(x => x.Level).Returns(RuntimeLevel.Run); + + _mockBackOfficeApplicationManager = new Mock(); + + var serviceCollection = new ServiceCollection(); + serviceCollection.AddSingleton(_mockBackOfficeApplicationManager.Object); + ServiceProvider serviceProvider = serviceCollection.BuildServiceProvider(); + + UmbracoRequestPaths umbracoRequestPaths = CreateUmbracoRequestPaths(); + var webRoutingSettings = Options.Create(new WebRoutingSettings()); + + _middleware = new BackOfficeAuthorizationInitializationMiddleware( + umbracoRequestPaths, + serviceProvider, + _mockRuntimeState.Object, + webRoutingSettings); + } + + [Test] + public async Task Registration_Failure_Does_Not_Cache_Host_So_Next_Request_Retries() + { + // Arrange — first call throws (simulating database contention during upgrade). + var callCount = 0; + _mockBackOfficeApplicationManager + .Setup(x => x.EnsureBackOfficeApplicationAsync(It.IsAny>(), It.IsAny())) + .Returns(() => + { + callCount++; + if (callCount == 1) + { + throw new Exception("Database is locked"); + } + + return Task.CompletedTask; + }); + + RequestDelegate next = _ => Task.CompletedTask; + HttpContext context = CreateBackOfficeHttpContext(); + + // Act — first request: registration fails. + Assert.ThrowsAsync(async () => await _middleware.InvokeAsync(context, next)); + + // Act — second request: should retry (host was NOT cached). + await _middleware.InvokeAsync(context, next); + + // Assert — EnsureBackOfficeApplicationAsync was called twice (retried after failure). + Assert.That(callCount, Is.EqualTo(2)); + } + + [Test] + public async Task Successful_Registration_Caches_Host_So_Subsequent_Requests_Skip() + { + // Arrange + _mockBackOfficeApplicationManager + .Setup(x => x.EnsureBackOfficeApplicationAsync(It.IsAny>(), It.IsAny())) + .Returns(Task.CompletedTask); + + RequestDelegate next = _ => Task.CompletedTask; + HttpContext context = CreateBackOfficeHttpContext(); + + // Act + await _middleware.InvokeAsync(context, next); + await _middleware.InvokeAsync(context, next); + + // Assert — only called once (second request skipped because host was cached) + _mockBackOfficeApplicationManager.Verify( + x => x.EnsureBackOfficeApplicationAsync(It.IsAny>(), It.IsAny()), + Times.Once); + } + + [Test] + [Timeout(5000)] // Fail fast if the semaphore-release regression is reintroduced (deadlock). + public async Task Registration_Failure_Does_Not_Leak_Semaphore() + { + // Arrange — always throws + _mockBackOfficeApplicationManager + .Setup(x => x.EnsureBackOfficeApplicationAsync(It.IsAny>(), It.IsAny())) + .ThrowsAsync(new Exception("Database is locked")); + + RequestDelegate next = _ => Task.CompletedTask; + + // Act — multiple failing requests should not deadlock (semaphore released via finally). + for (var i = 0; i < 3; i++) + { + HttpContext context = CreateBackOfficeHttpContext(); + Assert.ThrowsAsync(async () => await _middleware.InvokeAsync(context, next)); + } + + // Assert — all 3 attempts reached EnsureBackOfficeApplicationAsync (no deadlock). + _mockBackOfficeApplicationManager.Verify( + x => x.EnsureBackOfficeApplicationAsync(It.IsAny>(), It.IsAny()), + Times.Exactly(3)); + } + + [Test] + public async Task RuntimeLevel_Below_Upgrade_Skips_Registration() + { + // Arrange + _mockRuntimeState.Setup(x => x.Level).Returns(RuntimeLevel.Install); + + RequestDelegate next = _ => Task.CompletedTask; + HttpContext context = CreateBackOfficeHttpContext(); + + // Act + await _middleware.InvokeAsync(context, next); + + // Assert + _mockBackOfficeApplicationManager.Verify( + x => x.EnsureBackOfficeApplicationAsync(It.IsAny>(), It.IsAny()), + Times.Never); + } + + /// + /// Creates an HttpContext with a path that recognizes. + /// The Management API path (/umbraco/management/api/) is a backoffice request. + /// + private static HttpContext CreateBackOfficeHttpContext() + { + var context = new DefaultHttpContext(); + context.Request.Scheme = "https"; + context.Request.Host = new HostString("localhost", 44339); + context.Request.Path = "/umbraco/management/api/v1/server/status"; + return context; + } + + private static UmbracoRequestPaths CreateUmbracoRequestPaths() + { + var mockHostingEnvironment = new Mock(); + mockHostingEnvironment.Setup(x => x.ApplicationVirtualPath).Returns("/"); + mockHostingEnvironment.Setup(x => x.ToAbsolute(It.IsAny())) + .Returns(path => path.TrimStart('~')); + + var options = Options.Create(new UmbracoRequestPathsOptions()); + + return new UmbracoRequestPaths(mockHostingEnvironment.Object, options); + } +}