Role-Based Access Control: Check Deny Rules Before Admin Bypass

Role-Based Access Control: Check Deny Rules Before Admin Bypass

Most permission checks start the same way:

if User.IsAdmin then
  Exit(True);

It is quick, and it saves giving administrators every right one by one. It is also wrong as soon as the system has a rule that should apply to everyone - data that is read-only because it is a database view, a record locked after it was invoiced, an entity nobody has written rules for yet. The admin check runs first, returns True, and the rule is never looked at.

These rules are about the data, not the user. "This is a view, it cannot be written" is true for an administrator too. Writing to a view fails at the database at best; a record that should be locked gets changed without complaint.

The Order That Works

1. Deny rules - things nobody may do. Checked first, and final.
2. Admin bypass - administrators may do everything that is left.
3. Normal rights - everyone else needs the right for this operation.
4. Deny by default - anything without a rule is refused.

Example

Each entity class declares, with an attribute, the right needed to view it and the right needed to change it. Two special values carry the rules that are not about any one user:

• rNoEdits is a right granted to nobody. An entity whose edit right is rNoEdits is read-only.
• rNone means "no right exists for this", so only administrators may do it.

An entity with no attribute gets rNoEdits for every operation, so forgetting the attribute closes it rather than opening it.

unit Access_Rules;

interface

uses
  System.SysUtils, System.Rtti;

type
  TRight = (rNone, rLogin, rViewOrders, rEditOrders, rViewReports,
            rNoEdits);   // granted to no one: marks data that is read-only
  TRights = set of TRight;

  TOperation = (opView, opCreate, opUpdate, opDelete);

  // Declares which right each operation on an entity needs
  EntityRightsAttribute = class(TCustomAttribute)
  private
    FView: TRight;
    FEdit: TRight;
  public
    constructor Create(const AView, AEdit: TRight);
    function Required(const AOperation: TOperation): TRight;
  end;

  TUser = record
    Name: string;
    IsAdmin: Boolean;
    Rights: TRights;
  end;

  // Entities
  [EntityRights(rViewOrders, rEditOrders)]
  TOrder = class
  end;

  // A database view - can be read, never written
  [EntityRights(rViewReports, rNoEdits)]
  TSalesSummary = class
  end;

  // Anyone logged in may read it; only admins may change it
  [EntityRights(rLogin, rNone)]
  TNotice = class
  end;

  // No attribute at all
  TUntagged = class
  end;

function RequiredRight(const AClass: TClass; const AOperation: TOperation): TRight;

// Admin check first - the usual way, and wrong
function IsAllowed_AdminFirst(const AUser: TUser; const AClass: TClass;
  const AOperation: TOperation): Boolean;

// Deny rules first, then the admin bypass, then the normal rights check
function IsAllowed(const AUser: TUser; const AClass: TClass;
  const AOperation: TOperation): Boolean;

implementation

{ EntityRightsAttribute }

constructor EntityRightsAttribute.Create(const AView, AEdit: TRight);
begin
  inherited Create;
  FView := AView;
  FEdit := AEdit;
end;

function EntityRightsAttribute.Required(const AOperation: TOperation): TRight;
begin
  if AOperation = opView then
    Result := FView
  else
    Result := FEdit;
end;

{ Rules }

// rNoEdits when the class has no attribute, so untagged entities are closed
function RequiredRight(const AClass: TClass; const AOperation: TOperation): TRight;
var
  LContext: TRttiContext;
  LAttr: TCustomAttribute;
begin
  Result := rNoEdits;
  LContext := TRttiContext.Create;
  try
    for LAttr in LContext.GetType(AClass).GetAttributes do
      if LAttr is EntityRightsAttribute then
        Exit(EntityRightsAttribute(LAttr).Required(AOperation));
  finally
    LContext.Free;
  end;
end;

function IsAllowed_AdminFirst(const AUser: TUser; const AClass: TClass;
  const AOperation: TOperation): Boolean;
var
  LRight: TRight;
begin
  if AUser.IsAdmin then
    Exit(True);

  LRight := RequiredRight(AClass, AOperation);
  if LRight = rNone then
    Exit(False);
  Result := LRight in AUser.Rights;
end;

function IsAllowed(const AUser: TUser; const AClass: TClass;
  const AOperation: TOperation): Boolean;
var
  LRight: TRight;
begin
  LRight := RequiredRight(AClass, AOperation);

  // 1. Deny rules - nobody gets past these, admins included
  if LRight = rNoEdits then
    Exit(False);

  // 2. Admin bypass
  if AUser.IsAdmin then
    Exit(True);

  // 3. Normal check; rNone means "admins only"
  if LRight = rNone then
    Exit(False);
  Result := LRight in AUser.Rights;
end;

end.

The only difference between the two functions is where the admin check sits.

Testing Every Combination

Permission code is easy to test completely, because the number of combinations is small. Three users - an administrator, a clerk with order and report rights, and a viewer who may only view orders - against the four entities and four operations is 48 cases. The expected answer for each was written out by hand, and both functions were run against the full table:

  IsAllowed_AdminFirst: Admin Create TSalesSummary gave True, expected False
  IsAllowed_AdminFirst: Admin Update TSalesSummary gave True, expected False
  IsAllowed_AdminFirst: Admin Delete TSalesSummary gave True, expected False
  IsAllowed_AdminFirst: Admin View TUntagged gave True, expected False
  IsAllowed_AdminFirst: Admin Create TUntagged gave True, expected False
  IsAllowed_AdminFirst: Admin Update TUntagged gave True, expected False
  IsAllowed_AdminFirst: Admin Delete TUntagged gave True, expected False
IsAllowed_AdminFirst: 7 wrong out of 48
IsAllowed: 0 wrong out of 48

Every error in the admin-first version is the same mistake: an administrator allowed to do something the data itself forbids. Ordinary users were handled correctly by both versions, which is why this kind of bug survives testing done with a normal login.

Where the Check Belongs

Call it in one place on the server that every request passes through - an event that fires before each read, insert, update and delete of an entity, or the start of each service method. A check in the client only hides buttons; the server must refuse the request as well.

Watch for code paths that do their own checking. A generic "delete any entity" service that tests the admin flag itself, or a second event handler that calls the rights check without the deny rule, quietly reopens the hole. One function, called from every path, keeps the order in one place.

Rules of Thumb

• Rules about the data come before rules about the user.
• "Admin" means "every right", not "every action". Nothing a right cannot grant should be granted by being an administrator.
• Deny by default: no rule found means no access.
• Test with an administrator as well as with an ordinary user. This class of bug only shows up for the administrator.
• Write the expected table by hand and keep the test. When a new entity or right is added, add it to the table first.