views:

132

answers:

6

In my typical app, the user clicks a button in an aspx page, invokes a C# business object, then runs a stored procedure.

Should role checks be done at the top of the stack, the bottom of the stack or at every level? It seems that if a malicious user can invoke one method, he could invoke any, so for effective security, you'd need a check on every method (and that's a lot of extra code to write).

Here is a typical call stack to illustrate my question:

Page_Load()
{
  if(p.IsInRole("Managers"))  //or equivalent attribute
  {
    AddAccount.Visible =true;
  }
}

AddAccount_OnClick()
{
  if(p.IsInRole("Managers"))  //or equivalent attribute
  {
    //Add the account
    Account.Add(...);  //and maybe another role check...
  }
}

-- TSQL doesn't understand .NET authorization, this call is in a 'trusted' subsystem
create proc Add_Account @user, @account_name
If @user in (Select user from role_table where role='manager')
-- Add the account
+2  A: 

I would place the role access checks in the business objects actually performing the potentially dangerous/sensitive stuff.

Adding the role check to your page load and button click events would be extraneous, IMHO. Besides, if you're going to protect the page, protect the page using the declarative location directives in your web.config ... e.g. put all of your "admin" pages in a separate folder and protect the entire folder, declaratively.

But, you should always have the checks on your business object methods, as a minimum.

Chris W. Rea
+2  A: 

You need to put it at the method level. You cannot assume that you reach that method in any specific way. That method can be called by the button handler or it can be called in normal code as the result of any type of logic. How many times have you seen something like this calling a button handler ...

private void MyBypassingCall()
{
  if( myLogic )
  {
    AddAccount_OnClick();
  }
}

So putting it on Page_Load is not enough. You should also check out decorating the method with a PrincipalPermissionAttribute. That cuts down on a lot of code.

JP Alioto
+1  A: 

Business objects.

But at construction time. Let each instance capture a very specific role.

Edit: More succinct this way.

hythlodayr
+2  A: 

This is just an anecdotal comment in how important it can be to do security validation on the business side. It wasn't good enough in our case to be optimistic about low expectations of request hacking. We didn't have any sort of validation in our business objects and got burned in an unexpected way. A client built a script to automate their usage of our site. It ended up following expected links in their script that weren't actually rendered. It ended up corrupting data. Of course, this is more of a system state and data integrity issue for us rather than a security breach, but I suppose both really apply.

s_t_e_v_e
+2  A: 

From an implementation perspective it would be the best solution to implement the checks as far down the stack as possible because there is the lowest number of functions that require protection, hence the lowest number of things to mess up, and all user inputs have to pass through the protected layer. If your foundation is protected, you needn't care about everything build on that foundation.

This solution has one obviouse drawback - the user interface knows nothing about authentication, authorization, data verification, and all the other stuff. So every input goes down the stack, might cause an error, and goes up the stack again to inform the user. This will cause an unpleasant user experience because errors that could be easily detected in the user interface are only detected after passing the data to the backend systems. In consequence you will add many checks to the userinterface, too, in order to give a better user experience.

If you use interface based programming, this is no problem at all, because you can share the verification code between application layers. This enables you further to easily add verification code to all application layers and this will give you defense in depth - an bug in one application layer can be caught by another layer. Of course, if the verification code itself is erroneous and you share it accross application layers, the bug and hance a error will probably probagate through all application layers.

Daniel Brückner
+2  A: 

In my opinion, you should put it as close to the data as possible. The closer you are to the data, the better you can ensure that it isn't possible to take some circuitous route through your code base to circumvent an access check.

That argument would call for security checks being placed in either the data source itself, if it supports it (like your favorite RDBMS), or the data access layer.

However, some security constraints have a strong odour of business logic; e.g. "if the user is in this role and attempting to modify data that meet these specifications, the operation should be allowed; otherwise not". That sounds to me like a policy, and something that belongs either in the Business Logic layer og a separate Rules Engine of sorts.

I wrote about something similar in the context of WCF some time ago.

Mark Seemann