diff --git a/src/Umbraco.Web.BackOffice/Authorization/AdminUsersHandler.cs b/src/Umbraco.Web.BackOffice/Authorization/AdminUsersHandler.cs index 13711b825c..3a4e125a28 100644 --- a/src/Umbraco.Web.BackOffice/Authorization/AdminUsersHandler.cs +++ b/src/Umbraco.Web.BackOffice/Authorization/AdminUsersHandler.cs @@ -13,10 +13,11 @@ using Umbraco.Web.Editors; namespace Umbraco.Web.BackOffice.Authorization { + /// /// if the users being edited is an admin then we must ensure that the current user is also an admin /// - public class AdminUsersHandler : AuthorizationHandler + public class AdminUsersHandler : MustSatisfyRequirementAuthorizationHandler { private readonly IHttpContextAccessor _httpContextAcessor; private readonly IUserService _userService; @@ -34,14 +35,13 @@ namespace Umbraco.Web.BackOffice.Authorization _userEditorAuthorizationHelper = userEditorAuthorizationHelper; } - protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, AdminUsersRequirement requirement) + protected override Task IsAuthorized(AuthorizationHandlerContext context, AdminUsersRequirement requirement) { var queryString = _httpContextAcessor.HttpContext?.Request.Query[requirement.QueryStringName]; if (!queryString.HasValue) { // must succeed this requirement since we cannot process it - context.Succeed(requirement); - return Task.CompletedTask; + return Task.FromResult(true); } int[] userIds; @@ -55,8 +55,7 @@ namespace Umbraco.Web.BackOffice.Authorization if (ids.Count == 0) { // must succeed this requirement since we cannot process it - context.Succeed(requirement); - return Task.CompletedTask; + return Task.FromResult(true); } userIds = ids.Select(x => x.Value.TryConvertTo()).Where(x => x.Success).Select(x => x.Result).ToArray(); } @@ -64,23 +63,13 @@ namespace Umbraco.Web.BackOffice.Authorization if (userIds.Length == 0) { // must succeed this requirement since we cannot process it - context.Succeed(requirement); - return Task.CompletedTask; + return Task.FromResult(true); } var users = _userService.GetUsersById(userIds); var isAuth = users.All(user => _userEditorAuthorizationHelper.IsAuthorized(_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, user, null, null, null) != false); - if (isAuth) - { - context.Succeed(requirement); - } - else - { - context.Fail(); - } - - return Task.CompletedTask; + return Task.FromResult(isAuth); } } } diff --git a/src/Umbraco.Web.BackOffice/Authorization/BackOfficeHandler.cs b/src/Umbraco.Web.BackOffice/Authorization/BackOfficeHandler.cs index 8c1da3b204..6cee04deae 100644 --- a/src/Umbraco.Web.BackOffice/Authorization/BackOfficeHandler.cs +++ b/src/Umbraco.Web.BackOffice/Authorization/BackOfficeHandler.cs @@ -9,7 +9,7 @@ namespace Umbraco.Web.BackOffice.Authorization /// /// Ensures authorization is successful for a back office user. /// - public class BackOfficeHandler : AuthorizationHandler + public class BackOfficeHandler : MustSatisfyRequirementAuthorizationHandler { private readonly IBackOfficeSecurityAccessor _backOfficeSecurity; private readonly IRuntimeState _runtimeState; @@ -20,34 +20,22 @@ namespace Umbraco.Web.BackOffice.Authorization _runtimeState = runtimeState; } - protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, BackOfficeRequirement requirement) - { - if (!IsAuthorized(requirement)) - { - context.Fail(); - } - else - { - context.Succeed(requirement); - } - - return Task.CompletedTask; - } - - private bool IsAuthorized(BackOfficeRequirement requirement) + protected override Task IsAuthorized(AuthorizationHandlerContext context, BackOfficeRequirement requirement) { try { // if not configured (install or upgrade) then we can continue // otherwise we need to ensure that a user is logged in - return _runtimeState.Level == RuntimeLevel.Install + var isAuth = _runtimeState.Level == RuntimeLevel.Install || _runtimeState.Level == RuntimeLevel.Upgrade || _backOfficeSecurity.BackOfficeSecurity?.ValidateCurrentUser(false, requirement.RequireApproval) == ValidateRequestAttempt.Success; + return Task.FromResult(isAuth); } catch (Exception) { - return false; + return Task.FromResult(false); } } + } } diff --git a/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionsPublishBranchHandler.cs b/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionsPublishBranchHandler.cs index 89e72f6cba..265e34fe66 100644 --- a/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionsPublishBranchHandler.cs +++ b/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionsPublishBranchHandler.cs @@ -13,7 +13,7 @@ namespace Umbraco.Web.BackOffice.Authorization /// /// The user must have access to all descendant nodes of the content item in order to continue /// - public class ContentPermissionsPublishBranchHandler : AuthorizationHandler + public class ContentPermissionsPublishBranchHandler : MustSatisfyRequirementAuthorizationHandler { private readonly IEntityService _entityService; private readonly ContentPermissions _contentPermissions; @@ -29,7 +29,7 @@ namespace Umbraco.Web.BackOffice.Authorization _backOfficeSecurityAccessor = backOfficeSecurityAccessor; } - protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, ContentPermissionsPublishBranchRequirement requirement, IContent resource) + protected override Task IsAuthorized(AuthorizationHandlerContext context, ContentPermissionsPublishBranchRequirement requirement, IContent resource) { var denied = new List(); var page = 0; @@ -55,16 +55,7 @@ namespace Umbraco.Web.BackOffice.Authorization } } - if (denied.Count == 0) - { - context.Succeed(requirement); - } - else - { - context.Fail(); - } - - return Task.CompletedTask; + return Task.FromResult(denied.Count == 0); } } } diff --git a/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionsQueryStringHandler.cs b/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionsQueryStringHandler.cs index bba502e78f..b947014dbc 100644 --- a/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionsQueryStringHandler.cs +++ b/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionsQueryStringHandler.cs @@ -15,7 +15,7 @@ namespace Umbraco.Web.BackOffice.Authorization /// /// Used to authorize if the user has the correct permission access to the content for the content id specified in a query string /// - public class ContentPermissionsQueryStringHandler : AuthorizationHandler + public class ContentPermissionsQueryStringHandler : MustSatisfyRequirementAuthorizationHandler { private readonly IBackOfficeSecurityAccessor _backofficeSecurityAccessor; private readonly IHttpContextAccessor _httpContextAccessor; @@ -34,7 +34,7 @@ namespace Umbraco.Web.BackOffice.Authorization _contentPermissions = contentPermissions; } - protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, ContentPermissionsQueryStringRequirement requirement) + protected override Task IsAuthorized(AuthorizationHandlerContext context, ContentPermissionsQueryStringRequirement requirement) { int nodeId; if (requirement.NodeId.HasValue == false) @@ -42,8 +42,7 @@ namespace Umbraco.Web.BackOffice.Authorization if (!_httpContextAccessor.HttpContext.Request.Query.TryGetValue(requirement.QueryStringName, out var routeVal)) { // must succeed this requirement since we cannot process it - context.Succeed(requirement); - return Task.CompletedTask; + return Task.FromResult(true); } else { @@ -72,29 +71,21 @@ namespace Umbraco.Web.BackOffice.Authorization } var permissionResult = _contentPermissions.CheckPermissions(nodeId, - _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, + _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, out IContent contentItem, new[] { requirement.PermissionToCheck }); - switch (permissionResult) - { - case ContentPermissions.ContentAccess.Denied: - context.Fail(); - break; - case ContentPermissions.ContentAccess.NotFound: - default: - context.Succeed(requirement); - break; - } - if (contentItem != null) { //store the content item in request cache so it can be resolved in the controller without re-looking it up _httpContextAccessor.HttpContext.Items[typeof(IContent).ToString()] = contentItem; } - return Task.CompletedTask; + return permissionResult switch + { + ContentPermissions.ContentAccess.Denied => Task.FromResult(false), + _ => Task.FromResult(true), + }; } - } } diff --git a/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionsResourceHandler.cs b/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionsResourceHandler.cs index b2aeecb0ac..fe956c7360 100644 --- a/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionsResourceHandler.cs +++ b/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionsResourceHandler.cs @@ -9,7 +9,7 @@ namespace Umbraco.Web.BackOffice.Authorization /// /// Used to authorize if the user has the correct permission access to the content for the specified /// - public class ContentPermissionsResourceHandler : AuthorizationHandler + public class ContentPermissionsResourceHandler : MustSatisfyRequirementAuthorizationHandler { private readonly IBackOfficeSecurityAccessor _backofficeSecurityAccessor; private readonly ContentPermissions _contentPermissions; @@ -22,22 +22,13 @@ namespace Umbraco.Web.BackOffice.Authorization _contentPermissions = contentPermissions; } - protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, ContentPermissionsResourceRequirement requirement, IContent resource) + protected override Task IsAuthorized(AuthorizationHandlerContext context, ContentPermissionsResourceRequirement requirement, IContent resource) { var permissionResult = _contentPermissions.CheckPermissions(resource, _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, requirement.PermissionsToCheck); - if (permissionResult == ContentPermissions.ContentAccess.Denied) - { - context.Fail(); - } - else - { - context.Succeed(requirement); - } - - return Task.CompletedTask; + return Task.FromResult(permissionResult != ContentPermissions.ContentAccess.Denied); } } } diff --git a/src/Umbraco.Web.BackOffice/Authorization/DenyLocalLoginHandler.cs b/src/Umbraco.Web.BackOffice/Authorization/DenyLocalLoginHandler.cs index 70656e202f..771f462b8c 100644 --- a/src/Umbraco.Web.BackOffice/Authorization/DenyLocalLoginHandler.cs +++ b/src/Umbraco.Web.BackOffice/Authorization/DenyLocalLoginHandler.cs @@ -8,7 +8,7 @@ namespace Umbraco.Web.BackOffice.Authorization /// /// Ensures the resource cannot be accessed if returns true /// - public class DenyLocalLoginHandler : AuthorizationHandler + public class DenyLocalLoginHandler : MustSatisfyRequirementAuthorizationHandler { private readonly IBackOfficeExternalLoginProviders _externalLogins; @@ -17,18 +17,9 @@ namespace Umbraco.Web.BackOffice.Authorization _externalLogins = externalLogins; } - protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, DenyLocalLoginRequirement requirement) + protected override Task IsAuthorized(AuthorizationHandlerContext context, DenyLocalLoginRequirement requirement) { - if (_externalLogins.HasDenyLocalLogin()) - { - context.Fail(); - } - else - { - context.Succeed(requirement); - } - - return Task.CompletedTask; + return Task.FromResult(!_externalLogins.HasDenyLocalLogin()); } } } diff --git a/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionsQueryStringHandler.cs b/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionsQueryStringHandler.cs index 319d5969fd..a8d3b72918 100644 --- a/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionsQueryStringHandler.cs +++ b/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionsQueryStringHandler.cs @@ -10,7 +10,7 @@ using Umbraco.Core.Services; namespace Umbraco.Web.BackOffice.Authorization { - public class MediaPermissionsQueryStringHandler : AuthorizationHandler + public class MediaPermissionsQueryStringHandler : MustSatisfyRequirementAuthorizationHandler { private readonly IBackOfficeSecurityAccessor _backofficeSecurityAccessor; private readonly IHttpContextAccessor _httpContextAccessor; @@ -29,13 +29,12 @@ namespace Umbraco.Web.BackOffice.Authorization _mediaPermissions = mediaPermissions; } - protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, MediaPermissionsQueryStringRequirement requirement) + protected override Task IsAuthorized(AuthorizationHandlerContext context, MediaPermissionsQueryStringRequirement requirement) { if (!_httpContextAccessor.HttpContext.Request.Query.TryGetValue(requirement.QueryStringName, out var routeVal)) { // must succeed this requirement since we cannot process it - context.Succeed(requirement); - return Task.CompletedTask; + return Task.FromResult(true); } int nodeId; @@ -63,24 +62,17 @@ namespace Umbraco.Web.BackOffice.Authorization nodeId, out var mediaItem); - switch (permissionResult) - { - case MediaPermissions.MediaAccess.Denied: - context.Fail(); - break; - case MediaPermissions.MediaAccess.NotFound: - default: - context.Succeed(requirement); - break; - } - if (mediaItem != null) { //store the content item in request cache so it can be resolved in the controller without re-looking it up _httpContextAccessor.HttpContext.Items[typeof(IMedia).ToString()] = mediaItem; } - return Task.CompletedTask; + return permissionResult switch + { + MediaPermissions.MediaAccess.Denied => Task.FromResult(false), + _ => Task.FromResult(true), + }; } } } diff --git a/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionsResourceHandler.cs b/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionsResourceHandler.cs index 34451ade11..8b016bf466 100644 --- a/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionsResourceHandler.cs +++ b/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionsResourceHandler.cs @@ -8,7 +8,7 @@ namespace Umbraco.Web.BackOffice.Authorization /// /// Used to authorize if the user has the correct permission access to the content for the specified /// - public class MediaPermissionsResourceHandler : AuthorizationHandler + public class MediaPermissionsResourceHandler : MustSatisfyRequirementAuthorizationHandler { private readonly IBackOfficeSecurityAccessor _backofficeSecurityAccessor; private readonly MediaPermissions _mediaPermissions; @@ -21,7 +21,7 @@ namespace Umbraco.Web.BackOffice.Authorization _mediaPermissions = mediaPermissions; } - protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, MediaPermissionsResourceRequirement requirement, IMedia resource) + protected override Task IsAuthorized(AuthorizationHandlerContext context, MediaPermissionsResourceRequirement requirement, IMedia resource) { var permissionResult = MediaPermissions.MediaAccess.NotFound; @@ -39,16 +39,7 @@ namespace Umbraco.Web.BackOffice.Authorization out _); } - if (permissionResult == MediaPermissions.MediaAccess.Denied) - { - context.Fail(); - } - else - { - context.Succeed(requirement); - } - - return Task.CompletedTask; + return Task.FromResult(permissionResult != MediaPermissions.MediaAccess.Denied); } } } diff --git a/src/Umbraco.Web.BackOffice/Authorization/MustSatisfyRequirementAuthorizationHandler.cs b/src/Umbraco.Web.BackOffice/Authorization/MustSatisfyRequirementAuthorizationHandler.cs new file mode 100644 index 0000000000..98baacf5ee --- /dev/null +++ b/src/Umbraco.Web.BackOffice/Authorization/MustSatisfyRequirementAuthorizationHandler.cs @@ -0,0 +1,70 @@ +using Microsoft.AspNetCore.Authorization; +using System.Threading.Tasks; + +namespace Umbraco.Web.BackOffice.Authorization +{ + /// + /// Abstract handler that must satisfy the requirement so Succeed or Fail will be called no matter what + /// + /// + /// + /// aspnetcore Authz handlers are not required to satisfy the requirement and generally don't explicitly call Fail when the requirement + /// isn't satisfied, however in many simple cases explicitly calling Succeed or Fail is what we want which is what this class is used for. + /// + public abstract class MustSatisfyRequirementAuthorizationHandler : AuthorizationHandler where T : IAuthorizationRequirement + { + protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, T requirement) + { + var isAuth = await IsAuthorized(context, requirement); + if (isAuth) + { + context.Succeed(requirement); + } + else + { + context.Fail(); + } + } + + /// + /// Return true if the requirement is succeeded or ignored, return false if the requirement is explicitly not met + /// + /// + /// + /// + protected abstract Task IsAuthorized(AuthorizationHandlerContext context, T requirement); + } + + /// + /// Abstract handler that must satisfy the requirement so Succeed or Fail will be called no matter what + /// + /// + /// + /// + /// aspnetcore Authz handlers are not required to satisfy the requirement and generally don't explicitly call Fail when the requirement + /// isn't satisfied, however in many simple cases explicitly calling Succeed or Fail is what we want which is what this class is used for. + /// + public abstract class MustSatisfyRequirementAuthorizationHandler : AuthorizationHandler where T : IAuthorizationRequirement + { + protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, T requirement, TResource resource) + { + var isAuth = await IsAuthorized(context, requirement, resource); + if (isAuth) + { + context.Succeed(requirement); + } + else + { + context.Fail(); + } + } + + /// + /// Return true if the requirement is succeeded or ignored, return false if the requirement is explicitly not met + /// + /// + /// + /// + protected abstract Task IsAuthorized(AuthorizationHandlerContext context, T requirement, TResource resource); + } +} diff --git a/src/Umbraco.Web.BackOffice/Authorization/SectionHandler.cs b/src/Umbraco.Web.BackOffice/Authorization/SectionHandler.cs index 91ed79c685..ffa6db220f 100644 --- a/src/Umbraco.Web.BackOffice/Authorization/SectionHandler.cs +++ b/src/Umbraco.Web.BackOffice/Authorization/SectionHandler.cs @@ -12,7 +12,7 @@ namespace Umbraco.Web.BackOffice.Authorization /// /// The user only needs access to one of the sections specified, not all of the sections. /// - public class SectionHandler : AuthorizationHandler + public class SectionHandler : MustSatisfyRequirementAuthorizationHandler { private readonly IBackOfficeSecurityAccessor _backofficeSecurityAccessor; @@ -21,27 +21,13 @@ namespace Umbraco.Web.BackOffice.Authorization _backofficeSecurityAccessor = backofficeSecurityAccessor; } - protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, SectionRequirement requirement) - { - if (IsAuthorized(requirement)) - { - context.Succeed(requirement); - } - else - { - context.Fail(); - } - - return Task.CompletedTask; - } - - private bool IsAuthorized(SectionRequirement requirement) + protected override Task IsAuthorized(AuthorizationHandlerContext context, SectionRequirement requirement) { var authorized = _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser != null && requirement.SectionAliases.Any(app => _backofficeSecurityAccessor.BackOfficeSecurity.UserHasSectionAccess( app, _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser)); - return authorized; + return Task.FromResult(authorized); } } } diff --git a/src/Umbraco.Web.BackOffice/Authorization/TreeHandler.cs b/src/Umbraco.Web.BackOffice/Authorization/TreeHandler.cs index 932f467ac4..f151247850 100644 --- a/src/Umbraco.Web.BackOffice/Authorization/TreeHandler.cs +++ b/src/Umbraco.Web.BackOffice/Authorization/TreeHandler.cs @@ -16,7 +16,7 @@ namespace Umbraco.Web.BackOffice.Authorization /// This would allow a tree to be moved between sections. /// The user only needs access to one of the trees specified, not all of the trees. /// - public class TreeHandler : AuthorizationHandler + public class TreeHandler : MustSatisfyRequirementAuthorizationHandler { private readonly ITreeService _treeService; @@ -37,21 +37,7 @@ namespace Umbraco.Web.BackOffice.Authorization _backofficeSecurityAccessor = backofficeSecurityAccessor ?? throw new ArgumentNullException(nameof(backofficeSecurityAccessor)); } - protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, TreeRequirement requirement) - { - if (IsAuthorized(requirement)) - { - context.Succeed(requirement); - } - else - { - context.Fail(); - } - - return Task.CompletedTask; - } - - private bool IsAuthorized(TreeRequirement requirement) + protected override Task IsAuthorized(AuthorizationHandlerContext context, TreeRequirement requirement) { var apps = requirement.TreeAliases.Select(x => _treeService .GetByAlias(x)) @@ -60,9 +46,12 @@ namespace Umbraco.Web.BackOffice.Authorization .Distinct() .ToArray(); - return _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser != null + var isAuth = _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser != null && apps.Any(app => _backofficeSecurityAccessor.BackOfficeSecurity.UserHasSectionAccess( app, _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser)); + + return Task.FromResult(isAuth); } + } } diff --git a/src/Umbraco.Web.BackOffice/Authorization/UserGroupHandler.cs b/src/Umbraco.Web.BackOffice/Authorization/UserGroupHandler.cs index 5a9f21df81..697f60ec41 100644 --- a/src/Umbraco.Web.BackOffice/Authorization/UserGroupHandler.cs +++ b/src/Umbraco.Web.BackOffice/Authorization/UserGroupHandler.cs @@ -12,7 +12,7 @@ namespace Umbraco.Web.BackOffice.Authorization /// /// Authorizes that the current user has access to the user group Id in the request /// - public class UserGroupHandler : AuthorizationHandler + public class UserGroupHandler : MustSatisfyRequirementAuthorizationHandler { private readonly IHttpContextAccessor _httpContextAcessor; private readonly IUserService _userService; @@ -36,22 +36,7 @@ namespace Umbraco.Web.BackOffice.Authorization _backofficeSecurityAccessor = backofficeSecurityAccessor; } - protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, UserGroupRequirement requirement) - { - var isAuth = IsAuthorized(requirement); - if (!isAuth.HasValue || isAuth.Value) - { - context.Succeed(requirement); - } - else - { - context.Fail(); - } - - return Task.CompletedTask; - } - - private bool? IsAuthorized(UserGroupRequirement requirement) + protected override Task IsAuthorized(AuthorizationHandlerContext context, UserGroupRequirement requirement) { var currentUser = _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser; @@ -69,8 +54,11 @@ namespace Umbraco.Web.BackOffice.Authorization _contentService, _mediaService, _entityService); - return authHelper.AuthorizeGroupAccess(currentUser, intIds); + var isAuth = authHelper.AuthorizeGroupAccess(currentUser, intIds); + + return Task.FromResult(isAuth.Success); } + } }