views:

78

answers:

2

OK. So here's my simplified scenario. We have a system which handles orders for a number of clients. We want staff users to be able to view all orders and we want client user to only be able to view orders which relate to them.

When attempting to view a particular record we make use of the following function in our OrderSecurity class:

Public Function CanViewOrder(order)
    If currentUser.MemberOfStaff() Then
        CanViewOrder = True
    Else
        CanViewOrder = (order.ClientId = currentUser.ClientId)
    End If
End Function

At points when we want to display a list of orders to a user we can the following function defined in a OrderService class

Public Function GetOrders()
    If currentUser.MemberOfStaff() Then
        GetOrders = GetAllOrders()
    Else
        GetOrders = GetAllOrdersForClient(currentUser.ClientId)
    End If
End Function

This is OK for the above but doesn't hold up well as the rules get more complicated. Say, for example, we add another user type which represents a less trusted staff member who can only view orders from a sub-set of clients. We'd then have to add logic to the CanViewOrder and GetOrders functions (and potentially in the data access classes) which in my mind violates the DRY principle.

So, my question is: Am I missing a trick here - is there some way I can combine the business logic for permission to view orders in one place which both of these functions can use?

Or am I worrying too much and should just get on and have the logic in two places?

(In this particular application I'm using ASP Classic - don't hate the player, hate the game - but I'd be interested in how you solve this problem in any language)

+1  A: 

IMO you are not repeating yourself here. The business logic in CanViewOrder is not the same as the business logic in GetOrders. There is a superficial resemblance, it's true, but the two rules could theoretically evolve differently.

Christian Hayter
+1  A: 

You could concentrate the access policy and make it more general (probably at the expense of efficiency) by keeping it in a predicate like you have, just slightly generalised to take both subject (user) and object (order) into account:

Public Function CanView(user, order)
    (magic)
End Function

Then avoid repeating your access policy by implementing GetOrders(user) as a filter applying CanView(user, order) over the set of orders.

Having gone this route you can define other "queries", too, once and for all, in a way independent of the policy and how it may change. For example: GetUsersWhoCanView(order), CanViewSameOrders(user1, user2), CanAnybodyView(order),...

For a simple and relatively static policy with only a small number of functions relying on it, the "direct approach", well documented, gives you the best efficiency and least headache. If your policy could become complex or could change often or could end up being used by many other functions in the future, using the modular approach I outlined above avoids incurring technical debt.

Thomas Herlea