Should my login logic be part of the controller or a service in MVC webapp

Question

I'm using Shiro as my security manager for a Spring MVC web application.

The login basically happens in these lines:

Subject user = SecurityUtils.getSubject();
user.login(new UsernamePasswordToken(username, password));

Where should I put this logic? login() can throw exceptions if the requested user doesn't exist or wrong password was provided, etc. Should I call the code from my controller and do the error handling there or should I call it from my service layer, catch exceptions and rethrow my own up to the controller?

Answer 1

Either of the two options you present is valid.

Services (the Model layer) are allowed to call other services. So having your service call the authentication service is acceptable.

Likewise, Controllers are allowed to call to services so that option is equally acceptable.

If Shiro is providing your only authentication mechanisms, then it's kind of a toss-up between either approach. Calling from the Controller would seem a little faster to develop and potentially have less error handling code.

However, if you intend to add additional checks / wrappings around that authentication call, then you should have your Model make the call. So if there is a likely additional call for authorization after the authentication check, then I would lean towards calling from the Model.

---

Quick note, loose definitions:
Authentication: "Am I who I say I am?"
Authorization: "Do I have the authority or permission to access this?"

Answer 2

It should be in a Service Layer.

public interface UserService {
 public User findByLogin(final String userName);
}

And in a Service implementation :

public class UserServiceImpl implements UserService {
 @Inject
 private UserRepository userRepository;
 public User findByLogin(final String userName, final String password) {
 Preconditions.checkArgument(!Strings.isNullOrEmpty(userName), "Username is mandatory !");
 return Preconditions.checkNotNull(userRepository.findByLogin(userName), "User not found !");
 }
}

And an Identity provider service :

public class InternalIdentityProviderImpl implements ... {
 @Inject
 private UserService userService;
 public void authenticate(final String login, final String password) throws BadCredentialsException, IdentityNotFoundException {
 try {
 Preconditions.checkArgument(!Strings.isNullOrEmpty(login), "Username is mandatory !");
 Preconditions.checkArgument(!Strings.isNullOrEmpty(password), "Username is mandatory !");
 catch(Exception e) {
 throw new BadCredentialsException(e);
 }
 // Check user 
 try {
 User internal = Preconditions.checkNotNull(userService.findByLogin(username));
 } catch(Exception e) {
 // logs
 throw new IdentityNotFoundException(e);
 }
 // Shiro authentication 
 Subject user = SecurityUtils.getSubject();
 user.login(new UsernamePasswordToken(username, password));
 }
}

You should decorelate how the user is retrieved from the identity provider, and how it is validated by your identification system.

Currently I used RuntimeException thrown by Guava Preconditions, but you can use a specific handcrafted Exception.

ie:

• IdentityNotFoundException : from your identity provider.
• BadCredentialsException : from your identification system.

This service could be called from the controller, and handle the exception to display the right message.

• Paul Richter: Minor issue: public class UserService should be public interface UserService
• Zenithar: Yep ! i fix it !
• Sotirios Delimanolis: When I do user.login(), it internally uses a configured Realm to check the database for the user. A Realm is like a specialized service class used by shiro. Your answer has good points but doesn't fit with the Shiro workflow.
• Mik378: Typo: public void findByLogin(final String userName); should be public User findByLogin(final String userName); ;)