Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent false redirect to "Page Not Found" from UmbracoMemberAuthorize attribute handling #14036

Merged
merged 1 commit into from
Apr 11, 2023

Conversation

elit0451
Copy link
Member

Fixes #13946

Details

[UmbracoMemberAuthorize] attribute checks if the current member is authorized for a given request. When the member wasn't authorized we set the result to Forbidden and then we were redirected to a 404 page which wasn't the intention. This PR fixes the behaviour to return 401 Unauthorized when the member is not logged in and 403 Forbidden when the member is not of a permitted type or group.

Test

  • You can follow the issue's "Steps to reproduce" or you can use the code below:
using Microsoft.AspNetCore.Mvc;
using Umbraco.Cms.Web.Common.Controllers;
using Umbraco.Cms.Web.Common.Filters;

namespace Umbraco.Cms.Web.UI;

public class TestController : UmbracoApiController
{
    [UmbracoMemberAuthorize("", "All", "")]
    // /umbraco/api/test/test
    public IActionResult Test()
    {
        return Ok("Working");
    }
}
  • In the BO, create a member group called "All", as well as a few members and assign some of them to that group;
  • Create 2 Partial View Macro Files from snippet - LoginStatus.cshtml and Login.cshtml;
  • In the Macros folder in the Settings section, enable the 2 macros to be used in rich text editor and the grid;
  • Create a doc type with a Richtext editor and create a content page where you insert the 2 macros;
  • Remember to display the value of the RTE property in the template of your doc type - add @Model.Value("RTEaliasHERE");
  • Open the Network dev tools and verify:
    • Login with a member of the "All" group and verify you get "Working" as a response when visiting https://localhost:44331/umbraco/api/test/test;
    • Login with a member not part of the "All" group and verify you get a Forbidden resource response for the same link ⬆️ ;
    • And finally, visiting the same link when no member is logged in returns an Unauthorized response, like:

Screenshot

Copy link
Member

@bergmania bergmania left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 💪

@bergmania bergmania merged commit 2232859 into v10/dev Apr 11, 2023
@bergmania bergmania deleted the v10/bugfix/fix-member-authorize-filter-redirect branch April 11, 2023 13:41
mwillebrands added a commit to mwillebrands/Umbraco-CMS that referenced this pull request Jun 16, 2023
elit0451 added a commit that referenced this pull request Jul 4, 2023
…pi requests (#14399)

* Fix broken CookieAuthenticationRedirect caused by PR #14036 when not in an API controller

* Added Integration Tests for the MemberAuthorizationFilter

* Fix merge conflict

---------

Co-authored-by: Elitsa <elm@umbraco.dk>
nul800sebastiaan pushed a commit that referenced this pull request Jul 4, 2023
…pi requests (#14399)

* Fix broken CookieAuthenticationRedirect caused by PR #14036 when not in an API controller

* Added Integration Tests for the MemberAuthorizationFilter

* Fix merge conflict

---------

Co-authored-by: Elitsa <elm@umbraco.dk>
(cherry picked from commit 1d239a3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants