-
Notifications
You must be signed in to change notification settings - Fork 218
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
Security Trimming fails when using AttributeRouting and MVC 5.1.x #316
Comments
Thanks for the report. Are you using the MVC 5 integrated AttributeRouting, or are you using AttributeRouting from the open source project with an earlier version of MVC? |
Hi, I'm using the MVC 5 integrated AttributeRouting. |
I was able to reproduce this problem. But after going through the msdn documentation to setup attribute routing, I noticed that it starts working when you include the public class RouteConfig
{
public static void RegisterRoutes(RouteCollection routes)
{
routes.IgnoreRoute("{resource}.axd/{*pathInfo}");
routes.MapMvcAttributeRoutes(); // <-- This line is required to make it work.
routes.MapRoute(
name: "Default",
url: "{controller}/{action}/{id}",
defaults: new { controller = "Home", action = "Index", id = UrlParameter.Optional }
);
}
} So it looks like you will need to add that line to your configuration. However, do let me know if you see anything else out of the ordinary with MVC 5 attribute routing because I have not tested it thoroughly. |
It's is done. The probleme it's not on navigation but on security trimming. If I don't add the attribute routing to the methode the node menu is not visible, but when I add it the node is visible. |
I have posted a demo project on GitHub showing this configuration in functioning order. I didn't use the role because that is a part of the AuthorizeAttribute and really has nothing to do with the functionality that MvcSiteMapProvider provides. However, if the user that is logging in doesn't have the appropriate role assigned (in your case), the About node should not be visible. If you don't use Git, you can also download the solution directly as a zip file. The About link will be invisible when you first launch the application. You can register a new account, and upon logging in the About link will become visible. |
If you add a role to the Autorize attribute and upon logging with an account without this role the About link is visible. |
I am unable to reproduce the behavior you are seeing. I made only a single change to the demo project: [Authorize(Roles="Admin")] // <- I added "Admin"
[MvcSiteMapNode(Title="About", Key="Home.About", ParentKey="Home.Index", Order=1 )]
[Route("~/About")] // Attribute routing
public ActionResult About()
{
ViewBag.Message = "Your application description page.";
return View();
} Now when I login, the About link is not visible (as expected), because the user isn't in the Admin role. Of course, to assign roles you need some custom code or manual intervention. I suggest you look at how your roles are implemented, because this is clearly not an issue with MvcSiteMapProvider. AuthorizeAttribute is what contains the logic that checks roles, which is part of MVC - however if you are using a custom AuthorizeAttribute subclass it might contain logic that is causing the problem. This post and this question might be helpful in resolving this issue. |
The problem appear with the version 5.1.1 of Microsoft.AspNet.Mvc |
Thanks. I have confirmed that there is a problem with using version 5.1.1 of MVC. However, it looks like Microsoft redid the plumbing completely in 5.1 for looking up an action descriptor, which is the primary reason why it isn't working. I will have to continue researching this, but it could take a while, not to mention I currently don't have much time to work on the problem. If you need an immediate fix, downgrade to 5.0.x and it will work. Alternatively, use native routing instead of attribute routing. |
I would appreciate your help on this one. I don't really know the path you took to come up with the original solution for AuthorizeAttributeAclModule, so I am in the dark a little on this. I discovered other people trying to figure it out here and here, but neither of them has a solution posted. However, someone did break down the steps that are involved (and there are a lot of them). Unfortunately, the classes involved in this process are now marked internal, which pretty much means the functionality will need to be copied instead of relying on the MVC library. I am fairly certain that AttributeRoutingMapper.GetActionDescriptors() contains the solution, but as previously mentioned, properties and classes needed to duplicate that code are marked internal and/or private. One other thing that is a bit confusing to me is that it is working for MVC 5.0 with AttributeRouting (and I don't think it should be). Basically, there is a child RouteData object stored in the main RouteData object with the key "MS_DirectRouteMatches" that is added when AttributeRouting is being utilized. To use the actual RouteData, you need to use the inner object from the dictionary. Therefore, the action name is unavailable when using AttributeRouting unless you step into that internal dictionary. Somehow it works in MVC 5.0 even though the action name is blank. So, I think to fix the blank action name part, the VerifyControllerAttributes method needs to be changed to this: protected virtual bool VerifyControllerAttributes(RouteData routes, Type controllerType, ControllerContext controllerContext)
{
// Get controller descriptor
var controllerDescriptor = controllerDescriptorFactory.Create(controllerType);
if (controllerDescriptor == null)
return true;
var actionName = routes.GetOptionalString("action");
// Get the actionName from AttributeRouting
if (string.IsNullOrEmpty(actionName) && routes != null && routes.Values.ContainsKey("MS_DirectRouteMatches"))
{
actionName = ((IEnumerable<RouteData>)routes.Values["MS_DirectRouteMatches"]).First().GetOptionalString("action");
}
// Get action descriptor
var actionDescriptor = this.GetActionDescriptor(actionName, controllerDescriptor, controllerContext);
if (actionDescriptor == null)
return true;
// Verify security
var authorizeAttributes = this.GetAuthorizeAttributes(actionDescriptor, controllerContext);
return this.VerifyAuthorizeAttributes(authorizeAttributes, controllerContext, actionDescriptor);
} That fixes the empty string for action name, but I still get a null result for the GetActionDescriptor method, which is where things are going wrong. I have upgraded the demo project to MVC 5.1.1 and it is now failing as the OP described. You can use it to tell if any progress is being made on a solution for getting the action descriptor in MVC 5.1.x. |
It occurred to me that since this functionality has fundamentally changed and there doesn't appear to be an alternative way to look up action descriptors that this is probably a bug in MVC 5.1.x. I have opened a new issue with the MVC team. |
Thanks @NightOwl888, I'll cross post this to the ASP.NET insiders mailing list. |
Hi,
When you use this code the node "About" is not show in menu :
[Authorize(Roles = "Admin")]
[MvcSiteMapNode(Title = "About", Key = "Home.About", ParentKey = "Home.Index")]
public virtual ActionResult About()
{
return View();
}
but when you use this code the node "About" is show in menu :
[Authorize(Roles = "Admin")]
[MvcSiteMapNode(Title = "About", Key = "Home.About", ParentKey = "Home.Index")]
[Route("~/About")] // Attribute routing
public virtual ActionResult About()
{
return View();
}
Thank's for your job
The text was updated successfully, but these errors were encountered: