delight-im / PHP-Auth

Authentication for PHP. Simple, lightweight and secure.
MIT License
1.09k stars 235 forks source link

before login function #34

Closed mhmd1983 closed 7 years ago

mhmd1983 commented 7 years ago

Hello, Can we add a public function to execute before login ( true => continue, false => stop ), I'll use that to add google 2 step verification. but I think other people will find it useful for other things

ocram commented 7 years ago

Is the following the flow that you had in mind for using the proposed new callback with Google's 2FA?

 1)   The user enters their email address (or username) and password
 2a)  If the supplied credentials are correct:
      1a)  If the account has been activated already:
           1)   [!!!] The new callback is executed (e.g. run Google's 2FA here) [!!!]
           2a)  If the callback returns "true":
                1)  The login process continues
                2)  The time of the last login is updated in the database
                3)  The session is created
                4)  Authentication has succeeded
           2b)  If the callback returns "false":
                1) Authentication has failed
      1b)  If the account has not been activated yet
           1)  Authentication has failed
 2b)  If the supplied credentials are wrong:
      1)  Authentication has failed

That is, you hook right into the login process, as opposed to doing something that could be done either before or after the login. Is this what you want?

More specifically, the callback is only executed when the authentication itself would be successful, and it determines if authentication may indeed succeed or if it is cancelled.

mhmd1983 commented 7 years ago

Yes , That's the flow i want to use .

ocram commented 7 years ago

Implemented in https://github.com/delight-im/PHP-Auth/commit/0909291cf164313d2d8c04a54a49aed7b5e03382

Hope that helps!

fdiengdoh commented 7 years ago

It might be silly to ask this question and I'm not sure if it is right to ask something on a closed thread. But, I'm not an experienced php programmer, that's why I use libraries. I was confused about this $onBeforeSuccess = function ($userId),

For example, in this function I'm supposed to do something like this.

  1. check if a person has activated for TFA? If yes, display a form so that the user can input the security code.
  2. Then verify the code if correct return true else false.

Now, I've to store the user password/username in the form of hidden input variable while executing the above function. Then call the login to authenticate. Now my function looks something like this:

$onBeforeSuccess = function ($userId) {
        $tfa = checkTFAActivated($userId);
        //Check if f2a is added if yes prompt, if no then continue.
        return $tfa;
};
function checkTFAActivated($userid){
    //Connect to database//
    $db = new PDO('mysql:dbname=test;host=localhost;charset=utf8mb4', "root", "password");
    $sqlArray = array( $userid ) ;
    $stmt = $db->prepare("SELECT * FROM `users_fta` WHERE `userid` = ? LIMIT 1");
    $stmt->execute($sqlArray);
    $result = $stmt->fetch(PDO::FETCH_ASSOC);

    if($result){
        //IF activated then check for code verifying process or display form.
        if(isset($_POST['tfa'])){
            $totp = new TOTP( $_POST['email'], $result['secretkey'] );
            $totp->now(); // e.g. will return '123456'
            return $totp->verify($_POST['tfa']); // Will return true
        }else{
            (isset($_POST['remember'])) ? $remember = $_POST['remember'] : $remember = null; 
            showSecurityCodeForm($_POST['email'], $_POST['password'], $remember);
            exit;
        }
    }else{
        return true; //Here user not activated the TFA
    }
}
function showSecurityCodeForm($email, $password, $remember ){
    echo '<form action="" method="post" accept-charset="utf-8">';
    echo '<input type="hidden" name="action" value="login" />';
    echo '<input type="hidden" name="email" value="'. $email . '" /> ';
    echo '<input type="hidden" name="password" value="'. $password . '" /> ';
    echo '<input type="hidden" name="remember" value="'. $remember . '" />';
    echo '<input type="text" name="tfa" placeholder="Enter Security Code" /> ';
    echo '<button type="submit">Authenticate</button>';
    echo '</form>';
}

Now, my only concern is this secure or not?

ocram commented 7 years ago

First, that's a very legitimate question. I was already wondering if this should be explained in the documentation as well. Another conclusion might be that the new onBeforeSuccess callback creates more confusion than advantages. Perhaps, our implementation in this library must be improved. When the password is correct but another factor is required, we could also let authentication succeed instead, and keep the user in a state where they are just "half" logged in.

Second, while it's generally better to open separate issues for new questions or problems, it's okay here since this is a recent issue and the feature has just been introduced. And, by the way, using libraries and re-using existing solutions is not just something that less experienced programmers should do. Everybody should do that, if possible and adequate.

After having taken a quick glance at your implementation – and it's not that long, so that should suffice – it looks fine, overall. The process is as follows, and you got practically everything right:

These are all short-term solutions. As a long-term solution, we should also think about changing the way that this library handles multi-factor authentication. We could, for example, just send a token back to the user if the credentials were correct but a second factor was required. That token, consisting of random bytes and encoded as hex or base-64, for example, could then be valid for authentication within 5 minutes. Nobody would have to exchange the password a second time and the control flow could become simpler. That's just one quick idea, though.

fdiengdoh commented 7 years ago

Thank you for the clarification. In my site I use https only and also I use ajax call in my previous implementation. So I think I would use that when I implement this authentication library.

Also, I think it would help a lot if more information is added in the documentation.

ocram commented 7 years ago

Fine, your implementation should be secure then.

I have not decided yet whether to adjust the documentation for the current implementation or include a randomly generated token instead which is included in the AttemptCancelledException. You could then catch that exception, display a form to ask for the second factor, with the token inserted into a hidden field. And then you call a new method like loginWithToken instead, which will succeed if called within 5 minutes. That could be easier.

By the way, not every site or project does need multi-factor authentication right away. Of course, it's more secure and should definitely be considered, but perhaps not always on day one. You will also need to account for backup codes or another recovery mechanism, right?

fdiengdoh commented 7 years ago

By the way, not every site or project does need multi-factor authentication right away. Of course, it's more secure and should definitely be considered, but perhaps not always on day one. You will also need to account for backup codes or another recovery mechanism, right?

Yes, agree with you on that. I'm learning a lot from this too. thanks again.