Giter Club home page Giter Club logo

Comments (8)

ocram avatar ocram commented on May 22, 2024 1

Implemented in 0909291

Hope that helps!

from php-auth.

ocram avatar ocram commented on May 22, 2024 1

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?

from php-auth.

ocram avatar ocram commented on May 22, 2024

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.

from php-auth.

mhmd1983 avatar mhmd1983 commented on May 22, 2024

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

from php-auth.

fdiengdoh avatar fdiengdoh commented on May 22, 2024

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?

from php-auth.

ocram avatar ocram commented on May 22, 2024

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:

  • You receive a username and password and execute the login method with those credentials. This is how it has always been.
  • Then, in the new onBeforeSuccess callback, you first check if 2FA is enabled at all (for the specific user). If it's not, things are as easy as they were before, you just let the login finish by returning true. If 2FA is enabled, you "pause" authentication by returning false and you will want to proceed a little bit later.
    • (Instead of the exit statement, you could also return false there and ask for the second factor in the AttemptCancelledException. But both approaches are fine. By the way, you often don't need to construct another database instance in the callback, and you can simply import external variables into the closure with a use clause.)
  • Now if you returned false from onBeforeSuccess (or handled it inside that function and stopped via exit), you have to check the second factor for authentication yourself, which usually requires another form to be sent, received and processed. And only if that second factor is correct, you proceed with the login.
  • If proceeding with (and eventually finishing) login, you actually have to supply the username and password again. This is what could be confusing and what might make you think your solution is not secure, right? Well, we could implement this in a different way in the library. But as it is now, you have at least four solutions available:
    • You could just ask the user for their username and password again. This is obviously secure, but the least user-friendly way of dealing with this situation. And since the user has to enter their password again, some "attacker" could get another chance at "shoulder surfing". Oh, and as users have to type their password more often than really necessary, that could make them choose shorter (and weaker) passwords on your service.
    • When receiving the username and password for the first time, you could keep them in the user's session (on the server) for a short time, until they will return from the verification of their second factor a few moments later. They wouldn't have to re-send the username and password then, as you would still have those in the session. I wouldn't do that.
    • You receive the username and the password in cleartext on the server, as always. But then you could send back both the username and the password in cleartext to the client, e.g. embedding those values in the HTML that is sent to the user for the additional form. All that should be done over HTTPS, of course. Otherwise, receiving the password for the first time alone is not secure already. So sending back over HTTPS is just as secure as receiving it a moment earlier, and presenting the password to the user in a hidden field is the same situation as the user having entered the password into the form themselves. So this approach is not inherently insecure. It's acceptable, if done over HTTPS, but it increases the exposure time of the credentials, which are sent and received multiple times. This is what you did.
    • If you submit the login form via XHR/AJAX instead, the user can keep their filled-out form. If you require a second factor for authentication, you then just send back and append an additional form field asking for the additional factor. The rest of the form (with username and password) is still there and filled out. After the user has entered the second factor, the form is submitted again, including the old information entered before (i.e. the username and password), and you receive all the parts that are required. This frees you from having to send the password back to the user. I would probably do that.

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.

from php-auth.

fdiengdoh avatar fdiengdoh commented on May 22, 2024

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.

from php-auth.

fdiengdoh avatar fdiengdoh commented on May 22, 2024

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.

from php-auth.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.