Do not rely on claims to figure out user access + prepare for claims removal (#16552)

* Do not rely on claims to figure out user access + prepare for claims removal

* Fix typos :)

* Ensure we fire all relevant notifications when creating and updating user groups

* Leave it to the cache refreshers to flush user cache data (start nodes)

Author: kjac

src/Umbraco.Cms.Api.Management/DependencyInjection/BackOfficeAuthBuilderExtensions.cs

@@ -27,11 +27,8 @@ public static IUmbracoBuilder AddBackOfficeAuthentication(this IUmbracoBuilder b
 
     public static IUmbracoBuilder AddTokenRevocation(this IUmbracoBuilder builder)
     {
-        builder.AddNotificationAsyncHandler<UserSavingNotification, RevokeUserAuthenticationTokensNotificationHandler>();
         builder.AddNotificationAsyncHandler<UserSavedNotification, RevokeUserAuthenticationTokensNotificationHandler>();
         builder.AddNotificationAsyncHandler<UserDeletedNotification, RevokeUserAuthenticationTokensNotificationHandler>();
-        builder.AddNotificationAsyncHandler<UserGroupDeletingNotification, RevokeUserAuthenticationTokensNotificationHandler>();
-        builder.AddNotificationAsyncHandler<UserGroupDeletedNotification, RevokeUserAuthenticationTokensNotificationHandler>();
         builder.AddNotificationAsyncHandler<UserLoginSuccessNotification, RevokeUserAuthenticationTokensNotificationHandler>();
 
         return builder;

src/Umbraco.Cms.Api.Management/DependencyInjection/BackOfficeAuthPolicyBuilderExtensions.cs

@@ -28,21 +28,20 @@ internal static IUmbracoBuilder AddAuthorizationPolicies(this IUmbracoBuilder bu
         builder.Services.AddSingleton<IAuthorizationHandler, MediaPermissionHandler>();
         builder.Services.AddSingleton<IAuthorizationHandler, UserGroupPermissionHandler>();
         builder.Services.AddSingleton<IAuthorizationHandler, UserPermissionHandler>();
+        builder.Services.AddSingleton<IAuthorizationHandler, AllowedApplicationHandler>();
 
         builder.Services.AddAuthorization(CreatePolicies);
         return builder;
     }
 
     private static void CreatePolicies(AuthorizationOptions options)
     {
-        void AddPolicy(string policyName, string claimType, params string[] allowedClaimValues)
-        {
-            options.AddPolicy(policyName, policy =>
+        void AddAllowedApplicationsPolicy(string policyName, params string[] allowedClaimValues)
+            => options.AddPolicy(policyName, policy =>
             {
                 policy.AuthenticationSchemes.Add(OpenIddictValidationAspNetCoreDefaults.AuthenticationScheme);
-                policy.RequireClaim(claimType, allowedClaimValues);
+                policy.Requirements.Add(new AllowedApplicationRequirement(allowedClaimValues));
             });
-        }
 
         options.AddPolicy(AuthorizationPolicies.BackOfficeAccess, policy =>
         {
@@ -56,39 +55,39 @@ void AddPolicy(string policyName, string claimType, params string[] allowedClaim
             policy.RequireRole(Constants.Security.AdminGroupAlias);
         });
 
-        AddPolicy(AuthorizationPolicies.SectionAccessContent, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Content);
-        AddPolicy(AuthorizationPolicies.SectionAccessContentOrMedia, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Content, Constants.Applications.Media);
-        AddPolicy(AuthorizationPolicies.SectionAccessForContentTree, Constants.Security.AllowedApplicationsClaimType,
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.SectionAccessContent, Constants.Applications.Content);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.SectionAccessContentOrMedia, Constants.Applications.Content, Constants.Applications.Media);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.SectionAccessForContentTree,
             Constants.Applications.Content, Constants.Applications.Media, Constants.Applications.Users,
             Constants.Applications.Settings, Constants.Applications.Packages, Constants.Applications.Members);
-        AddPolicy(AuthorizationPolicies.SectionAccessForMediaTree, Constants.Security.AllowedApplicationsClaimType,
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.SectionAccessForMediaTree,
             Constants.Applications.Content, Constants.Applications.Media, Constants.Applications.Users,
             Constants.Applications.Settings, Constants.Applications.Packages, Constants.Applications.Members);
-        AddPolicy(AuthorizationPolicies.SectionAccessForMemberTree, Constants.Security.AllowedApplicationsClaimType,
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.SectionAccessForMemberTree,
             Constants.Applications.Content, Constants.Applications.Media, Constants.Applications.Members);
-        AddPolicy(AuthorizationPolicies.SectionAccessMedia, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Media);
-        AddPolicy(AuthorizationPolicies.SectionAccessMembers, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Members);
-        AddPolicy(AuthorizationPolicies.SectionAccessPackages, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Packages);
-        AddPolicy(AuthorizationPolicies.SectionAccessSettings, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Settings);
-        AddPolicy(AuthorizationPolicies.SectionAccessUsers, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Users);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.SectionAccessMedia, Constants.Applications.Media);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.SectionAccessMembers, Constants.Applications.Members);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.SectionAccessPackages, Constants.Applications.Packages);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.SectionAccessSettings, Constants.Applications.Settings);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.SectionAccessUsers, Constants.Applications.Users);
 
-        AddPolicy(AuthorizationPolicies.TreeAccessDataTypes, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Settings);
-        AddPolicy(AuthorizationPolicies.TreeAccessDictionary, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Translation);
-        AddPolicy(AuthorizationPolicies.TreeAccessDictionaryOrTemplates, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Translation, Constants.Applications.Settings);
-        AddPolicy(AuthorizationPolicies.TreeAccessDocuments, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Content);
-        AddPolicy(AuthorizationPolicies.TreeAccessDocumentsOrDocumentTypes, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Content, Constants.Applications.Settings);
-        AddPolicy(AuthorizationPolicies.TreeAccessDocumentTypes, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Settings);
-        AddPolicy(AuthorizationPolicies.TreeAccessLanguages, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Settings);
-        AddPolicy(AuthorizationPolicies.TreeAccessMediaTypes, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Settings);
-        AddPolicy(AuthorizationPolicies.TreeAccessMediaOrMediaTypes, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Media, Constants.Applications.Settings);
-        AddPolicy(AuthorizationPolicies.TreeAccessMemberGroups, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Members);
-        AddPolicy(AuthorizationPolicies.TreeAccessMemberTypes, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Settings);
-        AddPolicy(AuthorizationPolicies.TreeAccessPartialViews, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Settings);
-        AddPolicy(AuthorizationPolicies.TreeAccessRelationTypes, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Settings);
-        AddPolicy(AuthorizationPolicies.TreeAccessScripts, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Settings);
-        AddPolicy(AuthorizationPolicies.TreeAccessStylesheets, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Settings);
-        AddPolicy(AuthorizationPolicies.TreeAccessTemplates, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Settings);
-        AddPolicy(AuthorizationPolicies.TreeAccessWebhooks, Constants.Security.AllowedApplicationsClaimType, Constants.Applications.Settings);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.TreeAccessDataTypes, Constants.Applications.Settings);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.TreeAccessDictionary, Constants.Applications.Translation);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.TreeAccessDictionaryOrTemplates, Constants.Applications.Translation, Constants.Applications.Settings);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.TreeAccessDocuments, Constants.Applications.Content);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.TreeAccessDocumentsOrDocumentTypes, Constants.Applications.Content, Constants.Applications.Settings);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.TreeAccessDocumentTypes, Constants.Applications.Settings);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.TreeAccessLanguages, Constants.Applications.Settings);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.TreeAccessMediaTypes, Constants.Applications.Settings);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.TreeAccessMediaOrMediaTypes, Constants.Applications.Media, Constants.Applications.Settings);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.TreeAccessMemberGroups, Constants.Applications.Members);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.TreeAccessMemberTypes, Constants.Applications.Settings);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.TreeAccessPartialViews, Constants.Applications.Settings);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.TreeAccessRelationTypes, Constants.Applications.Settings);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.TreeAccessScripts, Constants.Applications.Settings);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.TreeAccessStylesheets, Constants.Applications.Settings);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.TreeAccessTemplates, Constants.Applications.Settings);
+        AddAllowedApplicationsPolicy(AuthorizationPolicies.TreeAccessWebhooks, Constants.Applications.Settings);
 
         // Contextual permissions
         options.AddPolicy(AuthorizationPolicies.ContentPermissionByResource, policy =>

src/Umbraco.Cms.Api.Management/Handlers/RevokeUserAuthenticationTokensNotificationHandler.cs

@@ -13,77 +13,31 @@
 namespace Umbraco.Cms.Api.Management.Handlers;
 
 internal sealed class RevokeUserAuthenticationTokensNotificationHandler :
-        INotificationAsyncHandler<UserSavingNotification>,
         INotificationAsyncHandler<UserSavedNotification>,
         INotificationAsyncHandler<UserDeletedNotification>,
-        INotificationAsyncHandler<UserGroupDeletingNotification>,
-        INotificationAsyncHandler<UserGroupDeletedNotification>,
         INotificationAsyncHandler<UserLoginSuccessNotification>
 {
-    private const string NotificationStateKey = "Umbraco.Cms.Api.Management.Handlers.RevokeUserAuthenticationTokensNotificationHandler";
-
     private readonly IUserService _userService;
-    private readonly IUserGroupService _userGroupService;
     private readonly IOpenIddictTokenManager _tokenManager;
     private readonly ILogger<RevokeUserAuthenticationTokensNotificationHandler> _logger;
     private readonly SecuritySettings _securitySettings;
 
     public RevokeUserAuthenticationTokensNotificationHandler(
         IUserService userService,
-        IUserGroupService userGroupService,
         IOpenIddictTokenManager tokenManager,
         ILogger<RevokeUserAuthenticationTokensNotificationHandler> logger,
         IOptions<SecuritySettings> securitySettingsOptions)
     {
         _userService = userService;
-        _userGroupService = userGroupService;
         _tokenManager = tokenManager;
         _logger = logger;
         _securitySettings = securitySettingsOptions.Value;
     }
 
-    // We need to know the pre-saving state of the saved users in order to compare if their access has changed
-    public async Task HandleAsync(UserSavingNotification notification, CancellationToken cancellationToken)
-    {
-        try
-        {
-            var usersAccess = new Dictionary<Guid, UserStartNodesAndGroupAccess>();
-            foreach (IUser user in notification.SavedEntities)
-            {
-                UserStartNodesAndGroupAccess? priorUserAccess = await GetRelevantUserAccessDataByUserKeyAsync(user.Key);
-                if (priorUserAccess == null)
-                {
-                    continue;
-                }
-
-                usersAccess.Add(user.Key, priorUserAccess);
-            }
-
-            notification.State[NotificationStateKey] = usersAccess;
-        }
-        catch (DbException e)
-        {
-            _logger.LogWarning(e, "This is expected when we upgrade from < Umbraco 14. Otherwise it should not happen");
-        }
-    }
-
     public async Task HandleAsync(UserSavedNotification notification, CancellationToken cancellationToken)
     {
         try
         {
-            Dictionary<Guid, UserStartNodesAndGroupAccess>? preSavingUsersState = null;
-
-            if (notification.State.TryGetValue(NotificationStateKey, out var value))
-            {
-                preSavingUsersState = value as Dictionary<Guid, UserStartNodesAndGroupAccess>;
-            }
-
-            // If we have a new user, there is no token
-            if (preSavingUsersState is null || preSavingUsersState.Count == 0)
-            {
-                return;
-            }
-
             foreach (IUser user in notification.SavedEntities)
             {
                 if (user.IsSuper())
@@ -95,23 +49,6 @@ public async Task HandleAsync(UserSavedNotification notification, CancellationTo
                 if (user.IsLockedOut || user.IsApproved is false)
                 {
                     await RevokeTokensAsync(user);
-                    continue;
-                }
-
-                // Don't revoke admin tokens to prevent log out when accidental changes
-                if (user.IsAdmin())
-                {
-                    continue;
-                }
-
-                // Check if the user access has changed - we also need to revoke all tokens in this case
-                if (preSavingUsersState.TryGetValue(user.Key, out UserStartNodesAndGroupAccess? preSavingState))
-                {
-                    UserStartNodesAndGroupAccess postSavingState = MapToUserStartNodesAndGroupAccess(user);
-                    if (preSavingState.CompareAccess(postSavingState) == false)
-                    {
-                        await RevokeTokensAsync(user);
-                    }
                 }
             }
         }
@@ -131,49 +68,6 @@ public async Task HandleAsync(UserDeletedNotification notification, Cancellation
         }
     }
 
-    // We need to know the pre-deleting state of the users part of the deleted group to revoke their tokens
-    public async Task HandleAsync(UserGroupDeletingNotification notification, CancellationToken cancellationToken)
-    {
-        var usersInGroups = new Dictionary<Guid, IEnumerable<IUser>>();
-        foreach (IUserGroup userGroup in notification.DeletedEntities)
-        {
-            var users = await GetUsersByGroupKeyAsync(userGroup.Key);
-            if (users == null)
-            {
-                continue;
-            }
-
-            usersInGroups.Add(userGroup.Key, users);
-        }
-
-        notification.State[NotificationStateKey] = usersInGroups;
-    }
-
-    public async Task HandleAsync(UserGroupDeletedNotification notification, CancellationToken cancellationToken)
-    {
-        Dictionary<Guid, IEnumerable<IUser>>? preDeletingUsersInGroups = null;
-
-        if (notification.State.TryGetValue(NotificationStateKey, out var value))
-        {
-            preDeletingUsersInGroups = value as Dictionary<Guid, IEnumerable<IUser>>;
-        }
-
-        if (preDeletingUsersInGroups is null)
-        {
-            return;
-        }
-
-        // since the user group was deleted, we can only use the information we collected before the deletion
-        // this means that we will not be able to detect users in any groups that were eventually deleted (due to implementor/3th party supplier interference)
-        // that were not in the initial to be deleted list
-        foreach (IUser user in preDeletingUsersInGroups
-                     .Where(group => notification.DeletedEntities.Any(entity => group.Key == entity.Key))
-                     .SelectMany(group => group.Value))
-        {
-            await RevokeTokensAsync(user);
-        }
-    }
-
     public async Task HandleAsync(UserLoginSuccessNotification notification, CancellationToken cancellationToken)
     {
         if (_securitySettings.AllowConcurrentLogins is false)
@@ -190,29 +84,6 @@ public async Task HandleAsync(UserLoginSuccessNotification notification, Cancell
         }
     }
 
-    // Get data about the user before saving
-    private async Task<UserStartNodesAndGroupAccess?> GetRelevantUserAccessDataByUserKeyAsync(Guid userKey)
-    {
-        IUser? user = await _userService.GetAsync(userKey);
-
-        return user is null
-            ? null
-            : MapToUserStartNodesAndGroupAccess(user);
-    }
-
-    private UserStartNodesAndGroupAccess MapToUserStartNodesAndGroupAccess(IUser user)
-        => new(user.Groups.Select(g => g.Key), user.StartContentIds, user.StartMediaIds);
-
-    // Get data about the users part of a group before deleting it
-    private async Task<IEnumerable<IUser>?> GetUsersByGroupKeyAsync(Guid userGroupKey)
-    {
-        IUserGroup? userGroup = await _userGroupService.GetAsync(userGroupKey);
-
-        return userGroup is null
-            ? null
-            : _userService.GetAllInGroup(userGroup.Id);
-    }
-
     private async Task RevokeTokensAsync(IUser user)
     {
         _logger.LogInformation("Revoking active tokens for user with ID {id}", user.Id);
@@ -236,35 +107,4 @@ private async Task RevokeTokensAsync(IUser user)
 
         return null;
     }
-
-    private class UserStartNodesAndGroupAccess
-    {
-        public IEnumerable<Guid> GroupKeys { get; }
-
-        public int[]? StartContentIds { get; }
-
-        public int[]? StartMediaIds { get; }
-
-        public UserStartNodesAndGroupAccess(IEnumerable<Guid> groupKeys, int[]? startContentIds, int[]? startMediaIds)
-        {
-            GroupKeys = groupKeys;
-            StartContentIds = startContentIds;
-            StartMediaIds = startMediaIds;
-        }
-
-        public bool CompareAccess(UserStartNodesAndGroupAccess other)
-        {
-            var areContentStartNodesEqual = (StartContentIds == null && other.StartContentIds == null) ||
-                                            (StartContentIds != null && other.StartContentIds != null &&
-                                            StartContentIds.SequenceEqual(other.StartContentIds));
-
-            var areMediaStartNodesEqual = (StartMediaIds == null && other.StartMediaIds == null) ||
-                                          (StartMediaIds != null && other.StartMediaIds != null &&
-                                          StartMediaIds.SequenceEqual(other.StartMediaIds));
-
-            return areContentStartNodesEqual &&
-                   areMediaStartNodesEqual &&
-                   GroupKeys.SequenceEqual(other.GroupKeys);
-        }
-    }
 }