maartenba / MvcSiteMapProvider

An ASP.NET MVC SiteMapProvider implementation for the ASP.NET MVC framework.
Microsoft Public License
539 stars 218 forks source link

preservedRouteParameters is no longer an attribute in SiteMapNodeModel #263

Closed vishnu4 closed 10 years ago

vishnu4 commented 10 years ago

I just upgraded from 4.4.5 to 4.4.7, and now everything that i had that used preservedRouteParameters is broken.
Used to be that i would step through my nodes to get the correct URL for that node, and i would check each node to see if it had preservedRouteParameters as a key, and if so i would add that value to the routevaluedictionary. if (node.Attributes.Keys.Contains("preservedRouteParameters")) { string preParam = node.Attributes["preservedRouteParameters"].ToString(); if ((!string.IsNullOrEmpty(preParam)) && (data.Values.ContainsKey(preParam))) { rtDict.Add(preParam, data.Values[preParam]); } } This no longer works, because preservedRouteParameters is no longer in the node attributes. I can't find where preservedRouteParameters is supposed to be in the node at all anymore. Any guidance on where to look? I can make my own attribute, but i'd rather not. I see that you mention this breaking change at https://github.com/maartenba/MvcSiteMapProvider/releases. You mention using the properties instead, but I don't see anywhere in the SiteMapNode object where i can find this value. Mind pointing me to where i should be looking?

NightOwl888 commented 10 years ago

I am not quite certain what you are trying to do here because MvcSiteMapProvider automatically copies the values from preservedRouteParameters to the current node.RouteValues dictionary for the current request. So any value you are trying to get should already be in there if you set the preservedRouteParemeters value in your configuration.

However, in case I missed your point, you can access the preserved route parameters through the PreservedRouteParameters property of the node.

string preParam = node.PreservedRouteParameters[0];

Do note that PreservedRouteParameters is an IList<string> so if there is more than one value, you will likely need a loop to access them.

I apologize for the confusion - the reason why we removed the values from the Attributes collection is because they were all just duplicates of what were going into the node's local properties. This was taking up extra memory and providing no real benefit.

vishnu4 commented 10 years ago

I'm not using SiteMapNode (which does have preservedrouteparameters), I'm using MvcSiteMapProvider.Web.Html.Models.SiteMapNodeModel , which does not have that property. In your MenuHelperModel, you have: @model MvcSiteMapProvider.Web.Html.Models.MenuHelperModel @using System.Web.Mvc.Html @using MvcSiteMapProvider.Web.Html.Models

and each 'var node' is of type SiteMapNodeModel. I'm using your Display template as a guide for my own menu cshtml. Should i be using something else? I have to go through each node because i need to check parameters on each one to see if that user has permission to see/use that node (using the 'roles' property). Additionally, some nodes shouldn't be seen unless a certain querystring parameter exists.

NightOwl888 commented 10 years ago

Only "known" attributes are being filtered out. If you need to add a custom one to do something specific, you still can.

<mvcSiteMapNode title="Home" someCustomAttribute="some-value"/>

You can then read "someCustomAttribute" in the attributes collection of the SiteMapNodeModel.

var someCustomAttribute = node.Attibutes["someCustomAttribute"];

To make sure it is not unintentionally added to the RouteValues collection (which will add the attribute to the URL), set the name of the attribute in the MvcSiteMapProvider_AttributesToIgnore web.config value.

  <appSettings>
    <add key="MvcSiteMapProvider_AttributesToIgnore" value="someCustomAttribute"/>
  </appSettings>

MvcSiteMapProvider Security

Do note that the roles attribute/property is for supporting interop with ASP.NET only.

Microsoft recommends using the [Authorize] attribute (which supports roles and in MVC5 it supports custom "claims" attributes) as the one and only security to use for MVC. This is because since MVC isn't based on files and routing makes it nearly impossible to ensure only 1 route can reach a resource, the best way to secure the resource is to add security at the resource itself (the action method and/or controller).

MvcSiteMapProvider supports the [Authorize] attribute natively (as well as classes that inherit it), and there are no special configuration to add to make it show or hide nodes when you use [Authorize] in MVC. The only thing you need to do is enable security trimming and decorate your action methods/controllers with the [Authorize] attribute. Optionally, you can add roles to the [Authorize] attribute.

Internal DI (web.config):

    <add key="MvcSiteMapProvider_SecurityTrimmingEnabled" value="true"/>

External DI (in MvcSiteMapProvider module):

    bool securityTrimmingEnabled = true; // First line in the module

For MVC4 and higher, it is best to register the AuthorizeAttribute as a global filter and then selectively allow access using the [AllowAnonymous] attribute. This means that by default when you create an action method, it will at minimum require the user to be logged in to view it unless the [AllowAnonymous] attribute is applied - secure by default.

    public class FilterConfig
    {
        public static void RegisterGlobalFilters(GlobalFilterCollection filters)
        {
            filters.Add(new HandleErrorAttribute());
            filters.Add(new AuthorizeAttribute());
        }
    }

See this post for more examples of using the [Authorize] attribute with MvcSiteMapProvider.

vishnu4 commented 10 years ago

You're right of course about the Authorize attribute, which i am actually using. I didn't realize that your sitemapprovider played well with that too. Turning the bool securityTrimmingEnabled = true absolutely did work, so i was able to trim out some code that way. Thanks! I tried the customattribute method previously, but for some reason once i did that the MvcSiteMapProvider.ISiteMapNode Parnd = MvcSiteMapProvider.SiteMaps.Current.CurrentNode; that i have at the top of each page came out as null for some reason.

and, sorry to throw another thing at you, but when testing the securitytrimmingenabled above, when i logged off of my site, the @Html.MvcSiteMap().CanonicalTag() @Html.MvcSiteMap().MetaRobotsTag() methods both caused null errors in my layout page. I'm not really using them, so i just commented them out, but i thought you might want to know. they worked fine when i was a valid user.

NightOwl888 commented 10 years ago

I tried the customattribute method previously, but for some reason once i did that the

MvcSiteMapProvider.ISiteMapNode Parnd = MvcSiteMapProvider.SiteMaps.Current.CurrentNode;

that i have at the top of each page came out as null for some reason.

This is likely because when you added the custom attribute, it was automatically added to your RouteValues collection, which caused it to no longer match your current route. You need also add it to the MvcSiteMapProvider_AttributesToIgnore so it is only added to Attributes and not also in RouteValues.

See How to Make MvcSiteMapProvider Remember a User's Position for a detailed look at how the matching process works.

and, sorry to throw another thing at you, but when testing the securitytrimmingenabled above, when i logged off of my site, the

@Html.MvcSiteMap().CanonicalTag() @Html.MvcSiteMap().MetaRobotsTag()

methods both caused null errors in my layout page. I'm not really using them, so i just commented them out, but i thought you might want to know. they worked fine when i was a valid user.

Ditto. Your route no longer matches, so the current node is null. They should be more robust and not throw exceptions in this case, though. Thanks for the feedback.

vishnu4 commented 10 years ago

Ok, you've definitely given me enough to work on, thanks!

vishnu4 commented 10 years ago

Actually, i just attempt to use the "AttributesToIgnore" suggestion above to user a custom parameter and avoid the null error when i use: MvcSiteMapProvider.ISiteMapNode Parnd = MvcSiteMapProvider.SiteMaps.Current.CurrentNode; But for some reason i couldn't get it to work. I'm actually using the multiple sitemap/ExternalDIContainer configuration, so i put the following: this.For().Use() .Ctor<IEnumerable>("attributesToIgnore").Is(new string[] { "KeepParameterInURL" }); in MVCSitemapProvderRegistry. This doesn't seem to fix the problem though.

NightOwl888 commented 10 years ago

Your example got munged, so I am not sure if this is what you have - but this should fix it so your custom attributes are not added to RouteValues:

this.For<ISiteMapXmlReservedAttributeNameProvider>().Use<SiteMapXmlReservedAttributeNameProvider>()
                .Ctor<IEnumerable<string>>("attributesToIgnore").Is(new string { "KeepParameterInURL" });

But then, this is not to keep the parameter in the URL, but to exclude it from the URL (for that you need to leave this setting alone).

If you are doing it the way you intend and are still getting a null current node, I would suggest stepping through the comparison on the node you intend to match to see why your node.RouteValues doesn't match your current request's RouteValues. Keep in mind, the current node will also be null if you don't have permission according to the [Authorize] attribute.

FYI - I added a null reference guard clause to all of the latest CanonicalTag, MetaRobotsTag, and Title HTML helpers, which should fix the error. Unfortunately, there is no way for NuGet to merge the changes into changed templates so you will need to copy the changes into your templates manually.

vishnu4 commented 10 years ago

You're right, my example was messed up, but what i have is what you wrote above. I'm not entirely sure what I'm doing to be honest, i'm only looking to exclude the value from routevalues based on your suggestion earlier up this thread. I'll go through the debugging you mentioned above, thanks for pointing me there, and all your very fast responses!

NightOwl888 commented 10 years ago

Well, one thing to keep in mind is that there are 2 ways to configure your custom route values to match a node and both of them can be used at the same time on the same node.

Given this action method and the default route...

        public ActionResult MyAction(int id)
        {
            return View();
        }

By default, you are expected to create a node for each possible value of the parameter.

<mvcSiteMapNode title="MyActionNode 1" action="MyAction" controller="MyController" id="1"/>
<mvcSiteMapNode title="MyActionNode 2" action="MyAction" controller="MyController" id="2"/>
<mvcSiteMapNode title="MyActionNode 3" action="MyAction" controller="MyController" id="3"/>
<mvcSiteMapNode title="MyActionNode 4" action="MyAction" controller="MyController" id="4"/>

This will make MvcSiteMapProvider match a different node for each ID. You can override this default and make MvcSiteMapProvider match any ID by using preservedRouteParameters. So in the example below, this node will always match regardless of what you pass into the URL for ID.

<mvcSiteMapNode title="MyActionNode" action="MyAction" controller="MyController" preservedRouteParamters="id"/>

On the other hand, AttributesToIgnore is for taking ID out of the matching equation altogether, so you would never pass ID in your request and it would always match this node.

<mvcSiteMapNode title="MyActionNode" action="MyAction" controller="MyController" id="SomeCustomValue"/>
        public ActionResult MyAction()
        {
            return View();
        }

The purpose of this is to allow you to define custom attributes that do not affect your route matching (so they can be used in HTML helpers, for example).

vishnu4 commented 10 years ago

Thanks, your walkthrough ended up getting me through it. I was trying to replace preservedRouteParameters, which i should have been using parameters in conjunction with it!