Don't test only with superusers

One common pitfall that keeps poping up in my life is doing tests always with superusers/admins/roots, and then having problems with normal accounts.

This is utterly wrong, because admins are the smallest segment of your users, and thus should be the last ones to take into account for tests.

A typical scenario is disallowing doing stuff, accessing content, etc.:

In a screen of a "delete content" control you might want to optimize the code by not doing anything if you are not supposed to view it; The typical "if (user.canDelete(Item) { //renderdelete button } else { //do nothing }".


But what happens if you forget to start the rendering code/markup with a "if (user.canDelete(Item) { ..." ?

Either everybody will see the delete button, or something will break further in (hopefully, as allowing everyone to delete content is catastrophic).

It is very important to either have a mechanism/architecture to add this checks automatically, or to double and triple check they are present, and in the correct spot.

Another example of bad practices:

Imagine you are building an item deletion feature: A user can delete something if he was the creator, or if he is an admin.
First approach could be to implement a user.CanDelete(item) that checks user.IsOwner(item) || user.IsAdmin(item) and this will work until we add another category of user (for example, moderator). Now we need a user.IsModerator(content) because is not an admin.

I've seen people do similar approaches as:

public bool CanDelete(object Item)
{
bool canDelete
= true;
if
(!this.IsOwner(Item) && !this
.IsAdmin(Item))
{
canDelete
= false;
}
return canDelete
;
}

A basic refactor to just one line would avoid the false positive problem, but wouldn't fix the false negative that might arise. Plus it places all users in two flags: owners/no-owners and admins/no-admins. Adding a moderator would need a first rewrite. Adding special users would need more. Adding visitors (maybe read-only access to every part of the site/application) might need even more checks.

The solution is quite easy: Build a role +permissions system, doing more "meta-data" coding (user.IsInRole(xxx), role.CanDelete(xxx), etcetera). It is way more better to have a role-based security, with HasPermissions(action) methods, or a permissions manager and then send both the action and subject to it.

Any solution that tries to simplify the logic as much as possible, encapsulating the logic and adding reusability in the process; providing as less entry points to security checks as possible, so that you know you have to only check and modify security logic in one or two places.

It is critical to test this delicate conditionals, so if they are either by convention in a method or all in a separate class make sure to see the logic and try to break it in the tests that fully cover all cases.

Also, by testing with superusers we might get side-effects:

  • Longer response times: Because as an admin you might get additional data, a slower perception, longer loading times...
  • Specific admin errors: Mere verbosing for admins can create new errors or generate Out of Memory exceptions by the extra amount or data or broken special features.
  • Wrong privacy tests: Being an admin you will probably see everything, be able to comment everywhere, etc. Use a normal account to see what the user exactly sees.
  • Beta or different menus/features: If your beta features are activate for admins (without a finer control) you can forget that what you see is the new version but that there is already an existing one, maybe with different algorithms, security or behaviour.
  • Relaxed or removed limitations: Rate limiting removed? Premium content? Those should be different tests, not the normal behaviour.

Just think what is better: To have your website broken for the two administrators, or for thousands of users and visitors? First make sure your site works for the common scenario and majority of users, then go with the non-standard cases or the rest of the roles.

Posted by Kartones on 2011-07-17

Comments?

Share via: Twitter Linkedin Google+ Facebook