ASP.NET MVC - Authorization Refactor

Question

The problem i see with this code, is that it's going to be reused a lot; anything being edited/created by a authenticated user (except for Site administrators) will only have access to a their "studios" objects.

My question to you all; how would you re-factor this so the service layer can be abstracted away from the knowledge of the client. I intend to reuse the service layer in a stand-alone desktop application later.

Please shed some light on my erroneous ways! I greatly appreciate it.

AuthorizeOwnerAttribute.cs (AuthorizeAttribute)

protected override bool AuthorizeCore(HttpContextBase httpContext)
{
    // Get the authentication cookie
    string cookieName = FormsAuthentication.FormsCookieName;
    HttpCookie authCookie = httpContext.Request.Cookies[cookieName];

    // If the cookie can't be found, don't issue the ticket
    if (authCookie == null) return false;

    // Get the authentication ticket and rebuild the principal & identity
    FormsAuthenticationTicket authTicket = FormsAuthentication.Decrypt(authCookie.Value);
    string[] userData = authTicket.UserData.Split(new[] { '|' });

    int userId = Int32.Parse(userData[0]);
    int studioID = Int32.Parse(userData[1]);
    GenericIdentity userIdentity = new GenericIdentity(authTicket.Name);
    WebPrincipal userPrincipal = new WebPrincipal(userIdentity, userId, studioID);
    httpContext.User = userPrincipal;

    return true;
}

Inside of my "User" Controller attach this attribute to any method that requires an owner

    [AuthorizeOwner]
    public ActionResult Edit(int Id)
    {
        IUser user = userService.GetById(HttpContext.User, Id);
        return View(user);
    }

Now, in my Service layer is where I'm checking the passed down IPrincipal if it has access to the object being requested. This is where it's getting smelly:

UserService.cs

    public IUser GetById(IPrincipal authUser, int id)
    {
        if (authUser == null) throw new ArgumentException("user");

        WebPrincipal webPrincipal = authUser as WebPrincipal;
        if (webPrincipal == null) throw new AuthenticationException("User is not logged in");

        IUser user = repository.GetByID(id).FirstOrDefault();
        if (user != null)
        {
            if (user.StudioID != webPrincipal.StudioID) throw new AuthenticationException("User does not have ownership of this object");
            return user;
        }

        throw new ArgumentException("Couldn't find a user by the id specified", "id");
    }

Answer 1

I'm not sure I'd be storing the actual IDs in the cookie, that's a little too exposed. I'd be more inclined to use the Session hash to store that data thus keeping it on the server and not exposed.

I'd also use the Model (by passing the userID) to determine which objects to return, i.e. those that have a matching studioID. That way your controller would only ever have to call "GetObjects(int id)", if they don't have access to anything then you get a null or empty collection back. That feels cleaner to me.