1

I am designing an MVC 3 application where multiple tenants reside in a single database.

What is the best way to prevent users from editing/viewing other tenants data in MVC? (i.e. someone could type in '/People/Edit/1' and edit the person with Id of 1- regardless of wether they are part of the tenants data or not).

I know I can override 'OnActionExecuting(ActionExecutingContext filterContext)' for each controller- but it sounds crazy to have to handle each action seperately, get the ID or OBJECT depending on if its a POST or GET and then check if the operation is allowed.

Any better ideas?

Also, I do not want to go down the route of creating a different database or schema for each tenant.

Thanks in advance.

Improve this question

3 Answers 3

Reset to default
1

Instead of passing ids to your controller actions write a custom model binder for your entities which will fetch it from the database. So for example let's assume that you have the following model:

public class Person
{
    public string Id { get; set; }
    ... some other properties
}

Now instead of having:

[HttpPost]
public ActionResult Edit(string id)
{
    ...
}

write:

[HttpPost]
public ActionResult Edit(Person person)
{
    ...
}

and then write a custom model binder for Person:

public class PersonModelBinder : IModelBinder
{
    public object BindModel(ControllerContext controllerContext, ModelBindingContext bindingContext)
    {
        var id = bindingContext.ValueProvider.GetValue("id");
        // check if an id was provided and if the user is authenticated
        if (!controllerContext.HttpContext.User.Identity.IsAuthenticated || id == null)
        {
            throw new HttpException(403, "Forbidden");
        }
        var currentUser = controllerContext.HttpContext.User.Identity.Name;
        // fetch the person from your repository given the id and belonging 
        // to the currently authenticated user
        var person = _repository.GetPerson(id.AttemptedValue, currentUser);
        if (person == null)
        {
            // no person found matching
            throw new HttpException(403, "Forbidden");
        }
        return person;
    }
}

which you would register in Application_Start:

ModelBinders.Binders.Add(typeof(Person), new PersonModelBinder());
Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

Thanks Darin. This sounds like a good idea but I don't have to replace all my logic for Geting and Posting objects.
0

Original answer

To solve the problem quickly use guids instead of a auto-increasing integer. However this is just delaying the problem.

One of the things you can do is to role your own authorize attribuut http://msdn.microsoft.com/en-us/library/system.web.mvc.authorizeattribute.aspx Or you can chose to create a global actionfilter. http://www.asp.net/mvc/tutorials/understanding-action-filters-cs

Addition information on how you could do it based on request in comment

public class MySuperFilter : ActionFilterAttribute
    {
        //Called by the MVC framework before the action method executes.
        public override void OnActionExecuting(ActionExecutingContext filterContext)
        {
            String user = filterContext.HttpContext.User.Identity.Name;
            int id = int.Parse(filterContext.RouteData.GetRequiredString("Id"));
            if (!IsValidUser(user,id))
            {
                filterContext.Result = new RedirectToRouteResult(
                        new RouteValueDictionary {{ "Controller", "YourController" },
                                      { "Action", "YourAction" } });


            }

            base.OnActionExecuting(filterContext);
        }

        private bool IsValidUser(string user,int id)
        {
            //Check if the user has acces to the page
            return true;
        }
    }
Improve this answer

6 Comments

This method makes sense and sounds like the easiest to implement. One question: I want to view a person (say, Id = 3). The Details action has my new filter in place [CanViewPerson]. How can I let the overriding function know that I want to view person with Id = 3?
Thanks for your updated answer. It makes sense now. Am I correct in saying that this will take care of security issues for all my GET requests and my POST reqs will be okay if I add the [ValidateAntiForgeryToken] attribute and token to my forms?
That token only checks if someone did not alternated the page. To prevent addons.mozilla.org/en-US/firefox/addon/tamper-data type of attack. You should be safe but I think its still possible to get access to your post-request by php.net/manual/en/intro.curl.php
Cheers Dvd, any idea how to prevent against the post-request attack?
@ViperMan place the attribute on post and get method
|
0

It's a non-elegant solution but depending on scope it could be easiest. Having designed a similar system that has relatively few points of entry for multiple tenants data we just merely check that the CurrentUser is the Owner of the object that is being queried. Our use objects a common base interface that has the owner field so the check is made not carrying about the specific object but just from the interface. If there's a mismatch we throw a security exception and log that a user is probably playing around the query string seeing if they get our website to leak data the way many production websites do.

Improve this answer

3 Comments

it looks like you are right. I was hoping for a more elegant solution but sometimes you got to get roll with what you have.
@ViperMAN being an architect myself I love elegant solutions, however many times its far easier to end up doubling and tripling the amount of work, time, effort and testing validation required for an elegant solution if you're really just targeting a handful of things. If you have dozens of actions that need similarly secured I'd definitely look at using an ActionFilter to do it in a more AOP way. And ActionFilters are why I develop solely in MVC3 since they allow true AOP that can achieve almost PostSharp level flexibility but without the build time overhead.
very true. I went down the ActionFilter route- as you said, it is much better for AOP.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.