Security Trimming

[murali]

Hello All,

I have upgraded my project to 2012.1.214 from 2011.3.1115. 

My menu's are bound to sitemap and are shown based on the authorization permissions. As soon as I upgraded all the menu items show up irrespective of the authorization attribute. Am I missing anything between these versions. 

Appreciate your help.

Thank you,
mm

[Georgi Krustev]

Hello Murali,

 
From Q1 2012 (2012.1.214) release of Telerik Extensions for ASP.NET MVC the security trimming uses OnAuthorization method instead of AuthorizeCore method directly. I will need more information about the current implementation in order to help you further.

Regards,
Georgi Krustev
the Telerik team

[murali]

Hello Georgi,

I am having trouble with the sitemap binding. And here is the sitemap file.

<siteMap xmlns="http://schemas.microsoft.com/AspNet/SiteMap-File-1.0" >

  <siteMapNode title="Home">

<siteMapNode title="Home" controller="Home" action="Index"/> 

<siteMapNode title="Admin" >
<siteMapNode title="Accounts" controller="Account" action="Index">

</siteMapNode> 

 </siteMapNode> 

</siteMap>

I render the menu from the site.master file.

<!-- Site map -->
        <div>
            <%
                if (Request.IsAuthenticated)
                {
            %>
                <% Html.RenderPartial("SiteMapMenu"); %>
            <%
                }
            %>
        </div>

And the partial...

<div>

        <%= Html.Telerik().Menu()

            .Name("Menu")

            .HtmlAttributes(new { @class="menu" })

            .BindTo("SiteMapper")   

         %>

</div>

This setting worked with earlier versions. After the upgrade, when I login as a non admin user, I can still see the Admin menu options, it fails with my authorization error when I click on it, but I would like to not show the options at all.
I tried to override OnAuthorization method, still didn't help.

Appreciate your help.
mm

[Georgi Krustev]

Hello Murali, 

I have attached a simple test project which uses Q1 2012 release of Telerik Extensions for ASP.NET MVC. Everything works on my end. I will ask you to check and modify it in order to replicate the issue.
 

Regards,
Georgi Krustev
the Telerik team

[murali]

Hello Georgi, thank you for the sample project, it helped me to figure out the issue.

First couple of things.

1. The sample is using Q4 release of 2011, I changed it to Q1 2012 release. 
2. Its built for MVC 3, but I am using MVC 2 (sorry I should have mentioned). I changed the project to MVC 2.

Here is my question. 

I have to comment out HandleUnauthorizedRequest method from both my authorization attribute classes in order it to work. The way it is set up is Admin can see all the menu items, but a user can see only the menus assigned for him. 

Everything works if I comment out this method, I am overriding this method to show a custom screen to redirect the user back. Please advice a work around. I am attaching the modified project files. Also I had to create two new users admin2 and user2 with same passwords you had for admin.

Also, when I logged in as "user2", I can still see the main menu item [About (Admin authorized only)], is there a way to hide this for regular users?

Thank you,
mm

[Georgi Krustev]

Hello Murali,

 
The Menu UI component, as other navigational components, uses the AuthorizationContext.Result to determine whether the item is allowed or not. When the result is HttpUnauthorizedResult then the item is not rendered. If the result is normal ActionResult then the item is considered as allowed and is rendered. That is why in your case the items are not shown when you comment the HandleUnauthorizedRequest method.
In order to use the menu and also override the HandleUnauthorizedRequest method, then you will need to stop security trimming and hide/show the items manually. You can stop security trimming using this method:

@(
   Html.Telerik().Menu()
            .Name("menu")
            .SecurityTrimming(false)
            ...
)

Regards,
Georgi Krustev
the Telerik team

[murali]

Georgi, is this something changed in the newer version. It will be tedious to manually hide items based on permissions.

Murali.

[Georgi Krustev]

Hello Murali,

Yes, this change was introduced after Q1 2012 release of Telerik Extensions for ASP.NET MVC. As I mentioned the security trimming depends on the AuthorizationContext.Result and it should be of HttpUnauthorizedResult type to hide the menu item. Currently, the only acheive your goal is to hide the menu items manually.

Regards,
Georgi Krustev
the Telerik team

[Ian McCarty]

This is a breaking change that wasn’t mention in the “Changes and Backward Compatibility” section of the documentation.

 

I have exactly the same problem as mentioned in this post. I’m using security trimming based on the Authorize attribute but if the user is not authorized I redirect to a custom error page.

 

I’m now faced with a choice:

1. Enable security trimming but it the user goes directly to a page an empty view is returned.
2. Disable security trimming and show a nice error page if not authorized.

 

Manually disabling items based on permissions is not practical.

 

What were the reasons for moving to OnAuthorization instead of AuthorizeCore?

Ian

[Georgi Krustev]

Hello Ian, 

Please excuse us for not pointing this change in the “Changes and Backward Compatibility” section. We will update the section as soon as possible to avoid any confusion.

Regarding question, we decide to support the OnAuthorization method because:

1. We do not need to IL generate the custom AuthorizationAttribute (we need this because the Telerik.Web.Mvc.dll does not reference custom assembly). The drawback of the IL generated attribute was that you cannot have a custom AuthorizeAttribute with different Roles property.
2. Now global authorize attributes are supported correctly.
3. Also in the OnAuthorization method you have access to the AuthorizationContext (ActionDescriptor in particular) object and have more variations to validate the user.

Currently the limitation of this approach is that we expect to return HttpUnauthorizedResult as a AuthorizationContext.Result to hide the menu item. We will further investigate the issue and if possible will provide a better solution in some of the next releases of the Telerik Extensions for ASP.NET MVC.

Regards,
Georgi Krustev
the Telerik team

[Nick]

Hi,

Has any progress been made on this at all?  I'm also using a custom Authorize filter to redirect unauthorised users and as such I'm stuck using 2011.3.1115 which was the last version before this change was made.

The issue is there was no way of discerning whether the authorize call was being made by the user choosing an action or the menu determining which items to trim.

Being stuck on 2011.3.1115 means that I'm prevented from upgrading jQuery to 1.7

For reference my authorize attribute is as follows:

Public Overrides Sub OnAuthorization(filterContext As System.Web.Mvc.AuthorizationContext)
  MyBase.OnAuthorization(filterContext)

  If filterContext.Result Is Nothing Then ' User is authorised, check if they are enabled
    Dim usr = _UserService.GetUser(filterContext.HttpContext.User.Identity.Name)
    If usr.Enabled = False Then
      SetRedirect(filterContext)
    End If
  Else
    SetRedirect(filterContext)
  End If
End Sub

where the SetRedirect function logs the unauthorised access attempt and redirects to an appropriate action dependant on the action result type (viewresult, partialviewresult, jsonresult, etc).

I raised a ticket about this a while ago (536786) but wondered if anything had changed in recent releases.

[Georgi Krustev]

Hello Nick,

 
After further investigation, we decided that the best way to handle the this requirement is to inherit HttpUnauthorizedResult and override the ExecuteReult method. Thus the OnAuthorization method will return an instance of HttpUnauthorizedResult type and also will be able to redirect to a custom page. Check the attached test project, which uses the Q2 2012 release. Please note that there is a known issue in this release, which is fixed in the current internal build. 

All the best,
Georgi Krustev
the Telerik team

[Chris]

I gotta say, that's kinda lame. That's a backward breaking change. Granted that's already been discussed. But to require HttpUnauthorizedResult as the AuthorizationContext result type is just....silly. A common workaround with Forms Auth to prevent redirecting back to the login page is to implement a 403 error instead of a 401 error. This requirement of yours breaks a lot of code out there that works perfectly fine right now.

It seems to me if you're concerned about showing menu items based off of whether or not something is accessible (such as with a HttpNotFoundResult) that you're missing the boat. Why not require the result type to be of type HttpStatusCodeResult? Or even better, use that and require certain status codes within it? Or, even better, allow the status codes that DON'T show menu items to be configurable in the config files???

It seems to me HttpUnauthorizedResult is unduly restrictive. While you have a great API, I think you dropped the ball here on this one and implemented something without considering all the ramifications first.