C# - Correct approach for method with various validations-CodePudding

I want to know what is the correct way of doing this: lets say I have a login method that receives username and password, and log ins the user or return invalid username/password or not enough permissions. What is the correct way of doing this?

Way #1: throwing exception and handling in the user interface to display the error

public void Login(string username, string password)
{
       if (SessionService.GetSession.IsLoggedIn)
       {
           throw new Exception("User is already logged in");
       }

       var user = GetByUsername(username);

       if (user == null)
       {
           throw new LoginException(LoginResultEnum.InvalidUsername);
       }

       var hashPass = EncryptionService.Hash(password);
       if (hashPass != user.password)
       {
           throw new LoginException(LoginResultEnum.InvalidPassword);
       }

       if (!user.HasPermission(PermissionTypeEnum.CanLogIn))
       {
           throw new MissingPermissionException(TipoPermisoEnum.CanLogIn);
       }

       SessionService.GetSession.Login(user);

}

Way #2: returning boolean true/false and handle the error in the UI (success or fail)

public bool Login(string username, string password)
{
       if (SessionService.GetSession.IsLoggedIn)
       {
           return false;
       }

       var user = GetByUsername(username);

       if (user == null)
       {
           return false;
       }

       var hashPass = EncryptionService.Hash(password);
       if (hashPass != user.password)
       {
           return false;
       }

       if (!user.HasPermission(PermissionTypeEnum.CanLogIn))
       {
           return false;
       }

       SessionService.GetSession.Login(user);
       return true;

}

Way #3: returning a LoginResult enum and handle in the UI

public LoginResult Login(string username, string password)
{
       if (SessionService.GetSession.IsLoggedIn)
       {
           return LoginResult.AlreadyLoggedIn;
       }

       var user = GetByUsername(username);

       if (user == null)
       {
           return LoginResult.InvalidUsername;
       }

       var hashPass = EncryptionService.Hash(password);
       if (hashPass != user.password)
       {
           return LoginResult.InvalidPassword;
       }

       if (!user.HasPermission(PermissionTypeEnum.CanLogIn))
       {
           return LoginResult.Forbidden;
       }

       SessionService.GetSession.Login(user);
       return LoginResult.OK;

}

CodePudding user response：

I would say #3 is the best way.

#1 you are using Exception for non-exceptional circumstances. The control path is expected, so don't use Exceptions.

#2 By using a bool you are discarding information, is it InvalidPassword or Forbidden?

#3 Returns all information, allowing the UI to surface that information to the User.

CodePudding user response：

In my view it better to create some dto if it is eligible for your case. So this dto will have the following properties:

public class LoginResponseDto
{
    public bool Success { get; set; }

    public string Error { get; set; }
}

And then you will return your response something like this:

public LoginResponseDto Login(string username, string password)
{
    if (SessionService.GetSession.IsLoggedIn)
    {   
        return new LoginResponseDto { Error = "User is already logged in" };
    }

    var user = GetByUsername(username);

    if (user == null)
    {
        return new LoginResponseDto { Error = "There is no such user" };
    }

    var hashPass = EncryptionService.Hash(password);
    if (hashPass != user.password)
    {
        return new LoginResponseDto { Error = "Incorrect password or username" };
    }

    if (!user.HasPermission(PermissionTypeEnum.CanLogIn))
    {
        return new LoginResponseDto { Error = "There is no permission to log in" };
    }

    SessionService.GetSession.Login(user);
    return new LoginResponseDto { Success = true };
}

It is possible to see this tutorial "Create a secure ASP.NET MVC 5 web app with log in, email confirmation and password reset". Author of article use ViewBag in this article to send errors from controller and Succeeded to check whether login is okay.

In addition, try to avoid to show message about what is exactly wrong username or password.