umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
MIT License
4.42k stars 2.67k forks source link

UmbracoAuthorizedController inherits from ControllerBase #15475

Closed fuzzyfuture closed 9 months ago

fuzzyfuture commented 9 months ago

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

10.6.1

Bug summary

UmbracoAuthorizedController is inheriting ControllerBase, when (I believe) it should be inheriting UmbracoController. ControllerBase (by design) lacks functionality for rendering views, such as the View() method and TempData.

Specifics

In Umbraco v8, UmbracoAuthorizedController inherits UmbracoController, which inherits Controller. This allows View() and TempData to be used in the controller's methods. In v10, UmbracoAuthorizedController has been changed to inherit ControllerBase, which does not support View() or TempData. I believe this change was unintentional for a few reasons:

Please correct me if I'm wrong and the change was actually intentional! The required code change is very simple, I just wanted to clarify before opening a pull request.

Steps to reproduce

  1. Create a new Umbraco 10.6.1 project
  2. Create a new controller class that inherits UmbracoAuthorizedController
  3. Add a method and attempt to return View() or access TempData

image

Expected result / actual result

Expected result

View() and TempData are accessible.

Actual result

Error CS0103.

github-actions[bot] commented 9 months ago

Hi there @nathangchay!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot :robot: :slightly_smiling_face:

andr317c commented 9 months ago

Hey! Thanks for creating this issue with really great information 💪! You are correct about the change being unintentional, but sadly it's not worth fixing since it could be a potentially massive breaking change to implement. We will update the docs so they do not mention the usage of UmbracoAuthorizedController since it is not used.

You can do this "workaround" instead of using the UmbracoAuthorizedController:

using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;
using Umbraco.Cms.Web.Common.Authorization;
using Umbraco.Cms.Web.Common.Controllers;
using Umbraco.Cms.Web.Common.Filters;

namespace v1061;

[Authorize(Policy = AuthorizationPolicies.BackOfficeAccess)]
[DisableBrowserCache]
public class TestController : UmbracoController
{
    public ActionResult Test()
    {
        TempData["Test"] = "Test";
        return View();
    }
}

I will close this issue, please reopen it if you have any additional questions or if the workaround did not fix your issue. 😄