From e5e87c96db21ed5c99beefdd0e170be51915e110 Mon Sep 17 00:00:00 2001 From: Sven Geusens Date: Mon, 27 May 2024 07:17:45 +0200 Subject: [PATCH] [v14] Fix multiple upgrader issues (#16339) * Made previewhubupdater work with full cache refreshes * Make signout_Async available on coreSignInManager * Allow Migrations to signout the logged in user * Adding a guid to a user requires resignin * Added a token revoke mechanism during migrations * Revert "Make signout_Async available on coreSignInManager" This reverts commit b103cf119a505e61de659dc206f6c85c2a27f2d5. * Revert add allRefreshed on preview hub Clarified with a comment * Updated failing test setups --------- Co-authored-by: Sven Geusens --- .../Migrations/MigrationBase.cs | 5 ++ .../Migrations/MigrationPlanExecutor.cs | 55 +++++++++++++++++-- .../Upgrade/V_14_0_0/AddGuidsToUsers.cs | 1 + .../Migrations/AdvancedMigrationTests.cs | 5 +- .../Migrations/MigrationPlanTests.cs | 3 +- 5 files changed, 63 insertions(+), 6 deletions(-) diff --git a/src/Umbraco.Infrastructure/Migrations/MigrationBase.cs b/src/Umbraco.Infrastructure/Migrations/MigrationBase.cs index ab6d079a96..cbdccc1ca4 100644 --- a/src/Umbraco.Infrastructure/Migrations/MigrationBase.cs +++ b/src/Umbraco.Infrastructure/Migrations/MigrationBase.cs @@ -91,6 +91,11 @@ public abstract partial class MigrationBase : IDiscoverable /// public bool RebuildCache { get; set; } + /// + /// If this is set to true, all backoffice client tokens will be revoked upon successful completion of the migration. + /// + public bool InvalidateBackofficeUserAccess { get; set; } + /// /// Runs the migration. /// diff --git a/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs b/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs index 31a9a3fa5f..bf2d734305 100644 --- a/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs +++ b/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs @@ -1,11 +1,15 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using OpenIddict.Abstractions; +using Org.BouncyCastle.Utilities; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Migrations; +using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.PublishedCache; using Umbraco.Cms.Core.Scoping; +using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Infrastructure.Persistence; using Umbraco.Cms.Infrastructure.Scoping; @@ -42,10 +46,12 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor private readonly IUmbracoDatabaseFactory _databaseFactory; private readonly IPublishedSnapshotService _publishedSnapshotService; private readonly IKeyValueService _keyValueService; + private readonly IServiceScopeFactory _serviceScopeFactory; private readonly DistributedCache _distributedCache; private readonly IScopeAccessor _scopeAccessor; private readonly ICoreScopeProvider _scopeProvider; private bool _rebuildCache; + private bool _invalidateBackofficeUserAccess; public MigrationPlanExecutor( ICoreScopeProvider scopeProvider, @@ -55,7 +61,8 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor IUmbracoDatabaseFactory databaseFactory, IPublishedSnapshotService publishedSnapshotService, DistributedCache distributedCache, - IKeyValueService keyValueService) + IKeyValueService keyValueService, + IServiceScopeFactory serviceScopeFactory) { _scopeProvider = scopeProvider; _scopeAccessor = scopeAccessor; @@ -64,6 +71,7 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor _databaseFactory = databaseFactory; _publishedSnapshotService = publishedSnapshotService; _keyValueService = keyValueService; + _serviceScopeFactory = serviceScopeFactory; _distributedCache = distributedCache; _logger = _loggerFactory.CreateLogger(); } @@ -85,7 +93,8 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor StaticServiceProvider.Instance.GetRequiredService(), StaticServiceProvider.Instance.GetRequiredService(), StaticServiceProvider.Instance.GetRequiredService(), - StaticServiceProvider.Instance.GetRequiredService()) + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService()) { } @@ -103,8 +112,8 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor StaticServiceProvider.Instance.GetRequiredService(), StaticServiceProvider.Instance.GetRequiredService(), StaticServiceProvider.Instance.GetRequiredService(), - StaticServiceProvider.Instance.GetRequiredService() - ) + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService()) { } @@ -135,6 +144,12 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor RebuildCache(); } + // If any completed migration requires us to sign out the user we'll do that. + if (_invalidateBackofficeUserAccess) + { + RevokeBackofficeTokens().GetAwaiter().GetResult(); // should async all the way up at some point + } + return result; } @@ -320,6 +335,11 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor { _rebuildCache = true; } + + if (migration.InvalidateBackofficeUserAccess) + { + _invalidateBackofficeUserAccess = true; + } } private void RebuildCache() @@ -327,4 +347,31 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor _publishedSnapshotService.RebuildAll(); _distributedCache.RefreshAllPublishedSnapshot(); } + + private async Task RevokeBackofficeTokens() + { + using IServiceScope scope = _serviceScopeFactory.CreateScope(); + + IOpenIddictApplicationManager openIddictApplicationManager = scope.ServiceProvider.GetRequiredService(); + var backOfficeClient = await openIddictApplicationManager.FindByClientIdAsync(Constants.OAuthClientIds.BackOffice); + if (backOfficeClient is null) + { + _logger.LogWarning("Could not get the openIddict Application for {backofficeClientId}. Canceling token revocation. Users might have to manually log out to get proper access to the backoffice", Constants.OAuthClientIds.BackOffice); + return; + } + + var backOfficeClientId = await openIddictApplicationManager.GetIdAsync(backOfficeClient); + if (backOfficeClientId is null) + { + _logger.LogWarning("Could not extract the clientId from the openIddict backofficelient Application. Canceling token revocation. Users might have to manually log out to get proper access to the backoffice", Constants.OAuthClientIds.BackOffice); + return; + } + + IOpenIddictTokenManager tokenManager = scope.ServiceProvider.GetRequiredService(); + var tokens = await tokenManager.FindByApplicationIdAsync(backOfficeClientId).ToArrayAsync(); + foreach (var token in tokens) + { + await tokenManager.DeleteAsync(token); + } + } } diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/AddGuidsToUsers.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/AddGuidsToUsers.cs index e5edce8e87..fe730fd2b8 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/AddGuidsToUsers.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/AddGuidsToUsers.cs @@ -26,6 +26,7 @@ internal class AddGuidsToUsers : UnscopedMigrationBase protected override void Migrate() { + InvalidateBackofficeUserAccess = true; using IScope scope = _scopeProvider.CreateScope(); using IDisposable notificationSuppression = scope.Notifications.Suppress(); ScopeDatabase(scope); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/AdvancedMigrationTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/AdvancedMigrationTests.cs index 084efb2371..cd138283eb 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/AdvancedMigrationTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/AdvancedMigrationTests.cs @@ -2,6 +2,7 @@ // See LICENSE for more details. using System.Linq; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Moq; using NUnit.Framework; @@ -35,6 +36,7 @@ public class AdvancedMigrationTests : UmbracoIntegrationTest private IMigrationBuilder MigrationBuilder => GetRequiredService(); private IUmbracoDatabaseFactory UmbracoDatabaseFactory => GetRequiredService(); private IPublishedSnapshotService PublishedSnapshotService => GetRequiredService(); + private IServiceScopeFactory ServiceScopeFactory => GetRequiredService(); private DistributedCache DistributedCache => GetRequiredService(); private IMigrationPlanExecutor MigrationPlanExecutor => new MigrationPlanExecutor( CoreScopeProvider, @@ -44,7 +46,8 @@ public class AdvancedMigrationTests : UmbracoIntegrationTest UmbracoDatabaseFactory, PublishedSnapshotService, DistributedCache, - Mock.Of()); + Mock.Of(), + ServiceScopeFactory); [Test] public void CreateTableOfTDto() diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/MigrationPlanTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/MigrationPlanTests.cs index fd279b5b3a..b70d036227 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/MigrationPlanTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/MigrationPlanTests.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using Moq; @@ -76,7 +77,7 @@ public class MigrationPlanTests loggerFactory, migrationBuilder, databaseFactory, - Mock.Of(), distributedCache, Mock.Of()); + Mock.Of(), distributedCache, Mock.Of(), Mock.Of()); var plan = new MigrationPlan("default") .From(string.Empty)