Binding action parameters to request cookies in ASP.NET MVC - what happened?

Question

In several early previews of ASP.NET MVC, arguments to controller methods would be resolved by inspecting the query string, then the form, then the cookies and server variables collections, as documented in this post from Stephen Walther.

For example, this code used to work:

public class MyController : Controller {

    // This should bind to Request.Cookies["userId"].Value
    public ActionResult Welcome(int userId) {

        WebUser wu = WebUser.Load(userId);
        ViewData["greeting"] = "Welcome, " + wu.Name;
        return(View());
    }
}

but now running against the release candidate, it throws an exception because it can't find a value for userId, even though userId definitely appears in the request cookies.

Was this change covered anywhere in the release notes? If this is a change to the framework, is there now a recommended alternative to binding cookies and server variables in this way?

EDIT: Thanks to those of you who have responded so far. I may have picked a bad example to demonstrate this; our code uses cookies for various forms of "convenient" but non-essential persistence (remembering ordering of search results, that kind of thing), so it's by no means purely an authentication issue. The security implications of relying on user cookies are well documented; I'm more interested in current recommendations for flexible, easily testable techniques for retrieving cookie values. (As I'm sure you can appreciate, the above example may have security implications, but is very, very easy to test!)

Answer 1

Besides the obvious security implications, why would you need to pass the cookie through in the route anyway?

Surely you would be better off having an Authorize attribute on the action, indicating that the user should be authenticated before the action is executed.

At the end of the day, I think (hope) Microsoft has closed this as it's quite a large security issue. If this is the case, you should consider rewriting your application to comply with this.

Answer 2

Hi,

I don't believe the cookies are checked anymore, and I'm not sure if it is intentional or not.

In an app against the RC I wrote recently I used the CookieContainer code from this post and a custom authorize attribute on classes like this:

public class LoginRequiredAttribute : AuthorizeAttribute
{
    protected override bool AuthorizeCore(HttpContextBase httpContext)
    {
        IAppCookies a = new AppCookies(new CookieContainer());
        return a.UserId != null; /* Better checks here */
    }
}

My AppCookies.cs just has a method for UserId like this (auto resolves to int/null for you):

public int? UserId
{
    get { return _cookieContainer.GetValue<int?>("UserId"); }
    set { _cookieContainer.SetValue("UserId", value, DateTime.Now.AddDays(10)); }
}

Then just make sure your web.config is setup to point to your login page like this:

<authentication mode="Forms">
<forms loginUrl="~/Login"/>
</authentication>

This means in my controller to get a UserId I need to do something like this to retrieve my cookie:

[LoginRequiredAttribute]
public class RandomController : Controller
{
    BaseDataContext dbContext = new BaseDataContext();
    private readonly IAppCookies _cookies = new AppCookies(new CookieContainer());

    public ActionResult Index()
    {
        return View(new RandomViewData(dbContext, _cookies.UserId));
    }
}

Answer 3

I believe it was the security implications that persuaded them to take these out:

The comments in Stephen Walther's post ASP.NET MVC Tip 15, leading to Phil Haack's posting User Input in Sheep's Clothing, especially his comment here:

@Troy - Step one is to dissuade devs from that line of thinking in the first place. ;) Step one prime (in parallel) is for us to remove the possibility of this line of thinking in this case.

The larger point still stands, we can make this change (after discussing it, we probably will), but that doesn't mean that it's suddenly safe to trust action method parameters.

Coupled with the complications of how you would call these methods from the various action builder classes.

I can't seem to find any explicit documentation one way or another about the controllers behaving like this other than Stephen's post, so I guess it was "quietly dropped".