diff --git a/src/Umbraco.Web.Common/Filters/UmbracoMemberAuthorizeFilter.cs b/src/Umbraco.Web.Common/Filters/UmbracoMemberAuthorizeFilter.cs index 95c4ae5cec00..2f56cdb51f17 100644 --- a/src/Umbraco.Web.Common/Filters/UmbracoMemberAuthorizeFilter.cs +++ b/src/Umbraco.Web.Common/Filters/UmbracoMemberAuthorizeFilter.cs @@ -60,14 +60,14 @@ public async Task OnAuthorizationAsync(AuthorizationFilterContext context) { context.HttpContext.SetReasonPhrase( "Resource restricted: the member is not of a permitted type or group."); + context.HttpContext.Response.StatusCode = 403; context.Result = new ForbidResult(); } } else { - context.HttpContext.SetReasonPhrase( - "Resource restricted: the member is not logged in."); - context.Result = new UnauthorizedResult(); + context.HttpContext.Response.StatusCode = 401; + context.Result = new ForbidResult(); } } diff --git a/src/Umbraco.Web.Common/Security/ConfigureMemberCookieOptions.cs b/src/Umbraco.Web.Common/Security/ConfigureMemberCookieOptions.cs index 66714be9e6dc..128737f76d3d 100644 --- a/src/Umbraco.Web.Common/Security/ConfigureMemberCookieOptions.cs +++ b/src/Umbraco.Web.Common/Security/ConfigureMemberCookieOptions.cs @@ -1,10 +1,12 @@ using Microsoft.AspNetCore.Authentication.Cookies; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Identity; +using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Routing; using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Web.Common.Controllers; using Umbraco.Extensions; namespace Umbraco.Cms.Web.Common.Security; @@ -58,7 +60,16 @@ public void Configure(CookieAuthenticationOptions options) }, OnRedirectToAccessDenied = ctx => { - ctx.Response.StatusCode = StatusCodes.Status403Forbidden; + // When the controller is an UmbracoAPIController, we want to return a StatusCode instead of a redirect. + // All other cases should use the default Redirect of the CookieAuthenticationEvent. + var controllerDescriptor = ctx.HttpContext.GetEndpoint()?.Metadata + .OfType() + .FirstOrDefault(); + + if (!controllerDescriptor?.ControllerTypeInfo.IsSubclassOf(typeof(UmbracoApiController)) ?? false) + { + new CookieAuthenticationEvents().OnRedirectToAccessDenied(ctx); + } return Task.CompletedTask; }, diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Web.Website/Security/MemberAuthorizeTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Web.Website/Security/MemberAuthorizeTests.cs new file mode 100644 index 000000000000..0fc1dfa85d19 --- /dev/null +++ b/tests/Umbraco.Tests.Integration/Umbraco.Web.Website/Security/MemberAuthorizeTests.cs @@ -0,0 +1,126 @@ +using System.Net; +using Microsoft.AspNetCore.Authentication.Cookies; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using Moq; +using NUnit.Framework; +using Umbraco.Cms.Core.Cache; +using Umbraco.Cms.Core.Logging; +using Umbraco.Cms.Core.Routing; +using Umbraco.Cms.Core.Security; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Web; +using Umbraco.Cms.Infrastructure.Persistence; +using Umbraco.Cms.Tests.Integration.TestServerTest; +using Umbraco.Cms.Web.Common.Controllers; +using Umbraco.Cms.Web.Common.Filters; +using Umbraco.Cms.Web.Common.Security; +using Umbraco.Cms.Web.Website.Controllers; + +namespace Umbraco.Cms.Tests.Integration.Umbraco.Web.Website.Security +{ + public class MemberAuthorizeTests : UmbracoTestServerTestBase + { + private Mock _memberManagerMock = new(); + + protected override void ConfigureTestServices(IServiceCollection services) + { + _memberManagerMock = new Mock(); + services.Remove(new ServiceDescriptor(typeof(IMemberManager), typeof(MemberManager), ServiceLifetime.Scoped)); + services.Remove(new ServiceDescriptor(typeof(MemberManager), ServiceLifetime.Scoped)); + services.AddScoped(_ => _memberManagerMock.Object); + } + + [Test] + public async Task Secure_SurfaceController_Should_Return_Redirect_WhenNotLoggedIn() + { + _memberManagerMock.Setup(x => x.IsLoggedIn()).Returns(false); + + var url = PrepareSurfaceControllerUrl(x => x.Secure()); + + var response = await Client.GetAsync(url); + + var cookieAuthenticationOptions = Services.GetService>(); + Assert.AreEqual(HttpStatusCode.Redirect, response.StatusCode); + Assert.AreEqual(cookieAuthenticationOptions.Value.AccessDeniedPath.ToString(), response.Headers.Location?.AbsolutePath); + } + + [Test] + public async Task Secure_SurfaceController_Should_Return_Redirect_WhenNotAuthorized() + { + _memberManagerMock.Setup(x => x.IsLoggedIn()).Returns(true); + _memberManagerMock.Setup(x => x.IsMemberAuthorizedAsync( + It.IsAny>(), + It.IsAny>(), + It.IsAny>())) + .ReturnsAsync(false); + + var url = PrepareSurfaceControllerUrl(x => x.Secure()); + + var response = await Client.GetAsync(url); + + var cookieAuthenticationOptions = Services.GetService>(); + Assert.AreEqual(HttpStatusCode.Redirect, response.StatusCode); + Assert.AreEqual(cookieAuthenticationOptions.Value.AccessDeniedPath.ToString(), response.Headers.Location?.AbsolutePath); + } + + + [Test] + public async Task Secure_ApiController_Should_Return_Unauthorized_WhenNotLoggedIn() + { + _memberManagerMock.Setup(x => x.IsLoggedIn()).Returns(false); + var url = PrepareApiControllerUrl(x => x.Secure()); + + var response = await Client.GetAsync(url); + + Assert.AreEqual(HttpStatusCode.Unauthorized, response.StatusCode); + } + + [Test] + public async Task Secure_ApiController_Should_Return_Forbidden_WhenNotAuthorized() + { + _memberManagerMock.Setup(x => x.IsLoggedIn()).Returns(true); + _memberManagerMock.Setup(x => x.IsMemberAuthorizedAsync( + It.IsAny>(), + It.IsAny>(), + It.IsAny>())) + .ReturnsAsync(false); + + var url = PrepareApiControllerUrl(x => x.Secure()); + + var response = await Client.GetAsync(url); + + Assert.AreEqual(HttpStatusCode.Forbidden, response.StatusCode); + } + } + + public class TestSurfaceController : SurfaceController + { + public TestSurfaceController( + IUmbracoContextAccessor umbracoContextAccessor, + IUmbracoDatabaseFactory databaseFactory, + ServiceContext services, + AppCaches appCaches, + IProfilingLogger profilingLogger, + IPublishedUrlProvider publishedUrlProvider) + : base( + umbracoContextAccessor, + databaseFactory, + services, + appCaches, + profilingLogger, + publishedUrlProvider) + { + } + + [UmbracoMemberAuthorize] + public IActionResult Secure() => NoContent(); + } + + public class TestApiController : UmbracoApiController + { + [UmbracoMemberAuthorize] + public IActionResult Secure() => NoContent(); + } +}