Is it bad design to make your business(service) layer dependant on a user session?

Question

In a common MVC designed applicaton, is it a bad idea to make the service layer dependant on a user session? Let's say there is a service method that fetches a few objects from a database, and you wish to return different results depending on who initializes the call - for example an administrator might get 10 rows of objects, while a normal user might only get 7 rows because the last 3 were "administrator-only" objects. A few ways to solve this would be:

• Introduce a new method parameter where you include the calling user. Dependancy-less but cumbersome to have to throw in user parameters in many methods.
• Make different methods for different user roles (with multiple results). Also dependancy-less but lots of methods which does basicly the same thing, which increases the risk of code duplication.
• Let the method read from a ThreadLocal variable in a static context storing the current user session. This variable is set before each request.

Lately, i've started using the last method more and more since it provides a clean interface and feels very practical to work with. A Filter makes sure that the current thread always has a user set. Is this bad design? I believe that some could see this as a dependancy from the service layer to the web layer, though I personally think that they are pretty uncoupled. The biggest consequence is that a methods behaviour will be different depending on the state of another class, which could be both a bad and a good thing.

What are your thoughts on this? If it's a bad solution, what would be a stronger one?

Answer 1

I strongly advise aginst ThreadLocal style approaches - it seems like too much of a "global variable" design smell. This has many issues, most notably:

• Your code gets harder to test as you go down the slippery slope of having more and more implicit global state to set up before testing
• You lose the ability to see clearly what parameters a certain piece of code is working with. This can be very confusing for maintainers or if you come back to the code after a few months.
• It can cause very nasty bugs / complexity if you ever hand off work to different threads. You've effectively made your code execution dependent on which thread it is running on.... what can possibly go wrong? :-)
• You are creating circular dependencies (service layer <-> user interface layer). Never a good idea, you should try to keep dependencies flowing one way only (almost always user interface layer -> service layer)

Between the other two approaches, I think "it depends":

• If the user is intrinsically part of the data model (e.g. you have a social graph database) then it would seem natural to pass the user as a parameter.
• If the user data is just used for front-end stuff like authentication etc. then I would tend towards minimising the dependency on specific user details and instead create different methods for different roles (or equivalently add a "role" parameter).

An alternative option is to pass a "context" object through to the service layer that contains a set of relevant session data (i.e. more than just the user name). This could make sense if you want to minimise parameter bloat. Just beware of it turning into a way to "get around" good layering principles. If it's just "data" then it's probably fine, but as soon as people start passing callback objects / UI components in the context then you are probably heading towards a bit of a mess....

Answer 2

Just use Java EE's role mechanism, it's already there.

As an example:

@Stateless
public class AService {

    @Resource
    private SessionContext sessionContext;

    @PersistenceContext
    private EntityManager entityManager;

    public List<AObject> fetchAFewObjects() {

        String query = "aUserQuery";      

        if (sessionContext.isCallerInRole("ADMIN")
            query = "aAdminQuery";

        return entityManager.createNamedQuery(query, AObject.class)
            .getResultList();    
    }  
}

On the web side, make sure you're doing a container login, so the server is aware of the logged-in user.

Answer 3

You don't have to implement either solution for your usecase.

In EJB, what you want is already supported by the framework. It's called the security context and it automatically propagates into every method you call (behind the scenes it's indeed often implemented by TLS, but that's an implementation detail).

This security context will give you access to the username and his/her roles. In your case all you seem to need is to check for the roles.

You can do this declaratively by using annotations on your methods (@RolesAllowed), or by injecting the EJB session context and asking it for the roles of the current user.