From 683170b04e08618dcb4c5b5887652c5c350c2d2a Mon Sep 17 00:00:00 2001 From: Zeegaan <70372949+Zeegaan@users.noreply.github.com> Date: Fri, 13 Aug 2021 10:51:50 +0200 Subject: [PATCH 1/4] Removed Unneccesary if check --- src/Umbraco.Web.Common/Security/PublicAccessChecker.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Umbraco.Web.Common/Security/PublicAccessChecker.cs b/src/Umbraco.Web.Common/Security/PublicAccessChecker.cs index 8e6174e5f1..ad9b39a7bb 100644 --- a/src/Umbraco.Web.Common/Security/PublicAccessChecker.cs +++ b/src/Umbraco.Web.Common/Security/PublicAccessChecker.cs @@ -39,11 +39,6 @@ namespace Umbraco.Cms.Web.Common.Security var username = currentMember.UserName; IList userRoles = await memberManager.GetRolesAsync(currentMember); - if (userRoles.Count == 0) - { - return PublicAccessStatus.AccessDenied; - } - if (!currentMember.IsApproved) { return PublicAccessStatus.NotApproved; From 18d670bc20814f6eb008f0142818e64afe193848 Mon Sep 17 00:00:00 2001 From: Zeegaan <70372949+Zeegaan@users.noreply.github.com> Date: Fri, 13 Aug 2021 12:17:20 +0200 Subject: [PATCH 2/4] Removed UnitTest --- .../Security/PublicAccessCheckerTests.cs | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/PublicAccessCheckerTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/PublicAccessCheckerTests.cs index 52c68b551f..f728314f81 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/PublicAccessCheckerTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/PublicAccessCheckerTests.cs @@ -115,23 +115,6 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security Assert.AreEqual(PublicAccessStatus.NotLoggedIn, result); } - [AutoMoqData] - [Test] - public async Task GivenMemberLoggedIn_WhenMemberHasNoRoles_ThenAccessDeniedResult( - IMemberManager memberManager, - IPublicAccessService publicAccessService, - IContentService contentService) - { - PublicAccessChecker sut = CreateSut(memberManager, publicAccessService, contentService, out HttpContext httpContext); - - httpContext.User = GetLoggedInUser(); - MockGetUserAsync(memberManager, new MemberIdentityUser()); - MockGetRolesAsync(memberManager, Enumerable.Empty()); - - var result = await sut.HasMemberAccessToContentAsync(123); - Assert.AreEqual(PublicAccessStatus.AccessDenied, result); - } - [AutoMoqData] [Test] public async Task GivenMemberLoggedIn_WhenMemberIsLockedOut_ThenLockedOutResult( From 8e2044ad4ee3a123f51f5fa7479bc7388ac858ce Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Fri, 13 Aug 2021 18:30:09 +0200 Subject: [PATCH 3/4] Revert "Removed UnitTest" This reverts commit 18d670bc20814f6eb008f0142818e64afe193848. --- .../Security/PublicAccessCheckerTests.cs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/PublicAccessCheckerTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/PublicAccessCheckerTests.cs index f728314f81..52c68b551f 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/PublicAccessCheckerTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/PublicAccessCheckerTests.cs @@ -115,6 +115,23 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security Assert.AreEqual(PublicAccessStatus.NotLoggedIn, result); } + [AutoMoqData] + [Test] + public async Task GivenMemberLoggedIn_WhenMemberHasNoRoles_ThenAccessDeniedResult( + IMemberManager memberManager, + IPublicAccessService publicAccessService, + IContentService contentService) + { + PublicAccessChecker sut = CreateSut(memberManager, publicAccessService, contentService, out HttpContext httpContext); + + httpContext.User = GetLoggedInUser(); + MockGetUserAsync(memberManager, new MemberIdentityUser()); + MockGetRolesAsync(memberManager, Enumerable.Empty()); + + var result = await sut.HasMemberAccessToContentAsync(123); + Assert.AreEqual(PublicAccessStatus.AccessDenied, result); + } + [AutoMoqData] [Test] public async Task GivenMemberLoggedIn_WhenMemberIsLockedOut_ThenLockedOutResult( From 8c42d563bd2ddaf3a03d8ee319698c4ab9363afe Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Fri, 13 Aug 2021 18:51:06 +0200 Subject: [PATCH 4/4] Updated unittest to match real usecase --- .../Security/PublicAccessCheckerTests.cs | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/PublicAccessCheckerTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/PublicAccessCheckerTests.cs index 52c68b551f..c2760e92f3 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/PublicAccessCheckerTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/PublicAccessCheckerTests.cs @@ -5,6 +5,7 @@ using System.Security.Claims; using System.Security.Principal; using System.Text; using System.Threading.Tasks; +using AutoFixture.NUnit3; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; using Moq; @@ -117,15 +118,28 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Security [AutoMoqData] [Test] - public async Task GivenMemberLoggedIn_WhenMemberHasNoRoles_ThenAccessDeniedResult( + public async Task GivenMemberLoggedIn_WhenMemberHasNoRolesAndWrongUsername_ThenAccessDeniedResult( IMemberManager memberManager, IPublicAccessService publicAccessService, - IContentService contentService) + IContentService contentService, + IContent protectedNode, + IContent loginNode, + IContent noAccessNode, + string username) { PublicAccessChecker sut = CreateSut(memberManager, publicAccessService, contentService, out HttpContext httpContext); + Mock.Get(publicAccessService).Setup(x => x.GetEntryForContent(It.IsAny())) + .Returns(new PublicAccessEntry(protectedNode, loginNode, noAccessNode, new [] + { + new PublicAccessRule(Guid.Empty, Guid.Empty) + { + RuleType = Constants.Conventions.PublicAccess.MemberUsernameRuleType, + RuleValue = "AnotherUsername" + } + })); httpContext.User = GetLoggedInUser(); - MockGetUserAsync(memberManager, new MemberIdentityUser()); + MockGetUserAsync(memberManager, new MemberIdentityUser(){IsApproved = true, UserName = username}); MockGetRolesAsync(memberManager, Enumerable.Empty()); var result = await sut.HasMemberAccessToContentAsync(123);