From d50b3a521e9a086652ba98392638f08dffc3105d Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Wed, 10 Apr 2024 12:39:22 +0200 Subject: [PATCH] Simplify user group authorization failure statuses (#16009) Co-authored-by: Elitsa --- .../UserGroup/UserGroupsControllerBase.cs | 22 ++------------- src/Umbraco.Core/Extensions/EnumExtensions.cs | 28 ------------------- .../UserGroupOperationStatus.cs | 7 +---- src/Umbraco.Core/Services/UserGroupService.cs | 15 +++++----- 4 files changed, 12 insertions(+), 60 deletions(-) diff --git a/src/Umbraco.Cms.Api.Management/Controllers/UserGroup/UserGroupsControllerBase.cs b/src/Umbraco.Cms.Api.Management/Controllers/UserGroup/UserGroupsControllerBase.cs index 023435ea8b..9ad5cead4d 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/UserGroup/UserGroupsControllerBase.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/UserGroup/UserGroupsControllerBase.cs @@ -76,25 +76,9 @@ public class UserGroupControllerBase : ManagementApiControllerBase UserGroupOperationStatus.UserNotInGroup => BadRequest(problemDetailsBuilder .WithTitle("User not in group") .WithDetail("The user is not in the group.")), - UserGroupOperationStatus.UnauthorizedMissingAllowedSectionAccess => Unauthorized(problemDetailsBuilder - .WithTitle("Unauthorized section access") - .WithDetail("The performing user does not have access to all sections specified as allowed for this user group.") - .Build()), - UserGroupOperationStatus.UnauthorizedMissingContentStartNodeAccess => Unauthorized(problemDetailsBuilder - .WithTitle("Unauthorized content start node access") - .WithDetail("The performing user does not have access to the specified content start node item.") - .Build()), - UserGroupOperationStatus.UnauthorizedMissingMediaStartNodeAccess => Unauthorized(problemDetailsBuilder - .WithTitle("Unauthorized media start node access") - .WithDetail("The performing user does not have access to the specified media start node item.") - .Build()), - UserGroupOperationStatus.UnauthorizedMissingUserGroupAccess => Unauthorized(problemDetailsBuilder - .WithTitle("Unauthorized user group access") - .WithDetail("The performing user does not have access to the specified user group(s).") - .Build()), - UserGroupOperationStatus.UnauthorizedMissingUsersSectionAccess => Unauthorized(problemDetailsBuilder - .WithTitle("Unauthorized access to Users section") - .WithDetail("The performing user does not have access to the Users section.") + UserGroupOperationStatus.Unauthorized => Unauthorized(problemDetailsBuilder + .WithTitle("Unauthorized access") + .WithDetail("The performing user does not have the necessary access to perform this operation. Check the log for details.") .Build()), _ => StatusCode(StatusCodes.Status500InternalServerError, problemDetailsBuilder .WithTitle("Unknown user group operation status.") diff --git a/src/Umbraco.Core/Extensions/EnumExtensions.cs b/src/Umbraco.Core/Extensions/EnumExtensions.cs index 4e0856a4b6..4d07c1d382 100644 --- a/src/Umbraco.Core/Extensions/EnumExtensions.cs +++ b/src/Umbraco.Core/Extensions/EnumExtensions.cs @@ -1,10 +1,6 @@ // Copyright (c) Umbraco. // See LICENSE for more details. -using System; -using Umbraco.Cms.Core.Services.AuthorizationStatus; -using Umbraco.Cms.Core.Services.OperationStatus; - namespace Umbraco.Extensions { /// @@ -29,29 +25,5 @@ namespace Umbraco.Extensions return (v & f) > 0; } - - /// - /// Converts from to . - /// - /// The authorization status to convert from. - /// The corresponding operation status. - /// Thrown if an authorization status does not have a corresponding operation status. - internal static UserGroupOperationStatus ToUserGroupOperationStatus(this UserGroupAuthorizationStatus from) => - from switch - { - UserGroupAuthorizationStatus.Success - => UserGroupOperationStatus.Success, - UserGroupAuthorizationStatus.UnauthorizedMissingAllowedSectionAccess - => UserGroupOperationStatus.UnauthorizedMissingAllowedSectionAccess, - UserGroupAuthorizationStatus.UnauthorizedMissingContentStartNodeAccess - => UserGroupOperationStatus.UnauthorizedMissingContentStartNodeAccess, - UserGroupAuthorizationStatus.UnauthorizedMissingMediaStartNodeAccess - => UserGroupOperationStatus.UnauthorizedMissingMediaStartNodeAccess, - UserGroupAuthorizationStatus.UnauthorizedMissingUserGroupAccess - => UserGroupOperationStatus.UnauthorizedMissingUserGroupAccess, - UserGroupAuthorizationStatus.UnauthorizedMissingUsersSectionAccess - => UserGroupOperationStatus.UnauthorizedMissingUsersSectionAccess, - _ => throw new NotImplementedException("UserGroupAuthorizationStatus does not map to a corresponding UserGroupOperationStatus") - }; } } diff --git a/src/Umbraco.Core/Services/OperationStatus/UserGroupOperationStatus.cs b/src/Umbraco.Core/Services/OperationStatus/UserGroupOperationStatus.cs index b086b36e49..f7dc1f76e0 100644 --- a/src/Umbraco.Core/Services/OperationStatus/UserGroupOperationStatus.cs +++ b/src/Umbraco.Core/Services/OperationStatus/UserGroupOperationStatus.cs @@ -1,6 +1,5 @@ namespace Umbraco.Cms.Core.Services.OperationStatus; -// FIXME: Move all authorization statuses to public enum UserGroupOperationStatus { Success, @@ -18,11 +17,7 @@ public enum UserGroupOperationStatus NameTooLong, AliasTooLong, MissingName, - UnauthorizedMissingAllowedSectionAccess, - UnauthorizedMissingContentStartNodeAccess, - UnauthorizedMissingMediaStartNodeAccess, - UnauthorizedMissingUserGroupAccess, - UnauthorizedMissingUsersSectionAccess, + Unauthorized, AdminGroupCannotBeEmpty, UserNotInGroup, } diff --git a/src/Umbraco.Core/Services/UserGroupService.cs b/src/Umbraco.Core/Services/UserGroupService.cs index 54001d7e57..78c049afe5 100644 --- a/src/Umbraco.Core/Services/UserGroupService.cs +++ b/src/Umbraco.Core/Services/UserGroupService.cs @@ -24,6 +24,7 @@ internal sealed class UserGroupService : RepositoryService, IUserGroupService private readonly IUserGroupPermissionService _userGroupPermissionService; private readonly IEntityService _entityService; private readonly IUserService _userService; + private readonly ILogger _logger; public UserGroupService( ICoreScopeProvider provider, @@ -32,13 +33,15 @@ internal sealed class UserGroupService : RepositoryService, IUserGroupService IUserGroupRepository userGroupRepository, IUserGroupPermissionService userGroupPermissionService, IEntityService entityService, - IUserService userService) + IUserService userService, + ILogger logger) : base(provider, loggerFactory, eventMessagesFactory) { _userGroupRepository = userGroupRepository; _userGroupPermissionService = userGroupPermissionService; _entityService = entityService; _userService = userService; + _logger = logger; } /// @@ -262,9 +265,8 @@ internal sealed class UserGroupService : RepositoryService, IUserGroupService await _userGroupPermissionService.AuthorizeCreateAsync(performingUser, userGroup); if (isAuthorized != UserGroupAuthorizationStatus.Success) { - // Convert from UserGroupAuthorizationStatus to UserGroupOperationStatus - UserGroupOperationStatus operationStatus = isAuthorized.ToUserGroupOperationStatus(); - return Attempt.FailWithStatus(operationStatus, userGroup); + _logger.LogInformation("The performing user is not allowed to create the user group. The authorization status returned was: {AuthorizationStatus}", isAuthorized); + return Attempt.FailWithStatus(UserGroupOperationStatus.Unauthorized, userGroup); } EventMessages eventMessages = EventMessagesFactory.Get(); @@ -342,9 +344,8 @@ internal sealed class UserGroupService : RepositoryService, IUserGroupService await _userGroupPermissionService.AuthorizeUpdateAsync(performingUser, userGroup); if (isAuthorized != UserGroupAuthorizationStatus.Success) { - // Convert from UserGroupAuthorizationStatus to UserGroupOperationStatus - UserGroupOperationStatus operationStatus = isAuthorized.ToUserGroupOperationStatus(); - return Attempt.FailWithStatus(operationStatus, userGroup); + _logger.LogInformation("The performing user is not allowed to update the user group. The authorization status returned was: {AuthorizationStatus}", isAuthorized); + return Attempt.FailWithStatus(UserGroupOperationStatus.Unauthorized, userGroup); } EventMessages eventMessages = EventMessagesFactory.Get();