Skip to content

Commit 1c877ad

Browse files
alexweissmanalexweissman
committed
implements #429
Sends new users a password creation link. Also, removes the requirement for supplying the username to reset account password.
1 parent 95ff0b0 commit 1c877ad

17 files changed

Lines changed: 318 additions & 206 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,6 @@
1616
- Switch to jQueryValidation from FormValidation
1717
- Implement basic interface for modifying group authorization rules
1818
- User events - timestamps for things like sign-in, sign-up, password reset, etc are now stored in a `user_event` table
19-
- Wrapper class Notification for sending emails, other notifications to users
19+
- Wrapper class Notification for sending emails, other notifications to users
20+
- Remove username requirement for password reset. It is more likely that an attacker would know the user's username, than the user themselves. For the next version, we can try to implement some real multi-factor authentication.
21+
- When a user creates another user, they don't need to set a password. Instead, an email is sent out to the new user, with a token allowing them to set their own password.

‎public/index.php‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,12 +88,13 @@
8888
switch ($action) {
8989
case "login": return $controller->pageLogin();
9090
case "logout": return $controller->logout(true);
91-
case "register": return $controller->pageRegister();
92-
case "activate": return $controller->activate();
91+
case "register": return $controller->pageRegister();
9392
case "resend-activation": return $controller->pageResendActivation();
9493
case "forgot-password": return $controller->pageForgotPassword();
94+
case "activate": return $controller->activate();
95+
case "set-password": return $controller->pageSetPassword(true);
9596
case "reset-password": if (isset($get['confirm']) && $get['confirm'] == "true")
96-
return $controller->pageResetPassword();
97+
return $controller->pageSetPassword(false);
9798
else
9899
return $controller->denyResetPassword();
99100
case "captcha": return $controller->captcha();
@@ -110,7 +111,8 @@
110111
case "register": return $controller->register();
111112
case "resend-activation": return $controller->resendActivation();
112113
case "forgot-password": return $controller->forgotPassword();
113-
case "reset-password": return $controller->resetPassword();
114+
case "set-password": return $controller->setPassword(true);
115+
case "reset-password": return $controller->setPassword(false);
114116
case "settings": return $controller->accountSettings();
115117
default: $app->notFound();
116118
}

‎userfrosting/controllers/AccountController.php‎

Lines changed: 100 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ public function pageRegister($can_register = false){
9292
}
9393

9494
/**
95-
* Render the "lost password" page.
95+
* Render the "forgot password" page.
9696
*
9797
* This creates a simple form to allow users who forgot their password to have a time-limited password reset link emailed to them.
9898
* By default, this is a "public page" (does not require authentication).
@@ -107,24 +107,33 @@ public function pageForgotPassword(){
107107
'validators' => $this->_app->jsValidator->rules()
108108
]);
109109
}
110-
110+
111111
/**
112-
* Render the "reset password" page.
112+
* Render the "set password" page.
113113
*
114-
* This is the actual page that is linked to in the "forgot password" email.
115-
* By default, this is a "public page" (does not require authentication).
114+
* If $flag_new_user is set to true, this renders the page where new users who have had accounts created
115+
* for them by another user, can set their password. If set to false, this renders the new password page for password reset requests.
116+
* By default, this is a "public page" (does not require authentication).
116117
* Request type: GET
117-
*/
118-
public function pageResetPassword(){
118+
* @param bool $flag_new_user Set to true if this is for a new user who doesn't yet have a password.
119+
*/
120+
public function pageSetPassword($flag_new_user = false){
121+
// Look up the user for the secret token
122+
$token = $this->_app->request->get()['secret_token'];
119123

120-
$schema = new \Fortress\RequestSchema($this->_app->config('schema.path') . "/forms/reset-password.json");
121-
$this->_app->jsValidator->setSchema($schema);
124+
$schema = new \Fortress\RequestSchema($this->_app->config('schema.path') . "/forms/set-password.json");
125+
$this->_app->jsValidator->setSchema($schema);
126+
127+
if ($flag_new_user)
128+
$template = 'account/create-password.twig';
129+
else
130+
$template = 'account/reset-password.twig';
122131

123-
$this->_app->render('account/reset-password.twig', [
124-
'secret_token' => $this->_app->request->get()['secret_token'],
132+
$this->_app->render($template, [
133+
'secret_token' => $token,
125134
'validators' => $this->_app->jsValidator->rules()
126135
]);
127-
}
136+
}
128137

129138
/**
130139
* Render the "resend account activation link" page.
@@ -241,6 +250,12 @@ public function login(){
241250
}
242251
}
243252

253+
// Check that the user has a password set (so, rule out newly created accounts without a password)
254+
if (!$user->password) {
255+
$ms->addMessageTranslated("danger", "ACCOUNT_USER_OR_PASS_INVALID");
256+
$this->_app->halt(403);
257+
}
258+
244259
// Check that the user's account is enabled
245260
if ($user->flag_enabled == 0){
246261
$ms->addMessageTranslated("danger", "ACCOUNT_DISABLED");
@@ -293,21 +308,7 @@ public function login(){
293308
* Request type: POST
294309
*/
295310
public function logout($complete = false){
296-
if ($complete){
297-
$storage = new \Birke\Rememberme\Storage\PDO($this->_app->remember_me_table);
298-
$storage->setConnection(\Illuminate\Database\Capsule\Manager::connection()->getPdo());
299-
$storage->cleanAllTriplets($this->_app->user->id);
300-
}
301-
// Change cookie path
302-
$cookie = $this->_app->remember_me->getCookie();
303-
$cookie->setPath("/");
304-
$this->_app->remember_me->setCookie($cookie);
305-
306-
if ($this->_app->remember_me->clearCookie())
307-
error_log("Cleared cookie");
308-
309-
session_regenerate_id(true);
310-
session_destroy();
311+
$this->_app->user->logout($complete);
311312
$this->_app->redirect($this->_app->site->uri['public']);
312313
}
313314

@@ -435,12 +436,18 @@ public function register(){
435436
foreach ($defaultGroups as $group_id => $group)
436437
$user->addGroup($group_id);
437438

439+
// Store new user to database
440+
$user->save();
441+
438442
// Create sign-up event
439443
$sign_up_event = $user->newEventSignUp();
444+
$sign_up_event->save();
440445

441446
if ($this->_app->site->require_activation) {
442-
// Create verification request
447+
// Create verification request event
443448
$verification_request = $user->newEventVerificationRequest();
449+
$user->save(); // Re-save is needed to update user's secret token
450+
$verification_request->save();
444451

445452
// Create and send verification email
446453
$twig = $this->_app->view()->getEnvironment();
@@ -460,15 +467,10 @@ public function register(){
460467
}
461468

462469
$ms->addMessageTranslated("success", "ACCOUNT_REGISTRATION_COMPLETE_TYPE2");
463-
$verification_request->save();
464470
} else
465471
// No activation required
466472
$ms->addMessageTranslated("success", "ACCOUNT_REGISTRATION_COMPLETE_TYPE1");
467-
468-
$sign_up_event->save();
469-
// Store new user to database
470-
$user->store();
471-
}
473+
}
472474

473475
/**
474476
* Processes an new account activation request.
@@ -516,12 +518,14 @@ public function activate(){
516518
* Processes a request to email a forgotten password reset link to the user.
517519
*
518520
* Processes the request from the form on the "forgot password" page, checking that:
519-
* 1. The provided username exists;
520-
* 2. The provided email address matches the username;
521-
* 3. The user doesn't already have an outstanding password reset request;
522-
* 4. The submitted data is valid.
521+
* 1. The provided email address belongs to a registered account;
522+
* 2. The submitted data is valid.
523+
* Note that we have removed the requirement that a password reset request not already be in progress.
524+
* This is because we need to allow users to re-request a reset, even if they lose the first reset email.
523525
* This route is "public access".
524-
* Request type: POST
526+
* Request type: POST
527+
* @todo rate-limit forgotten password requests, to prevent password-reset spamming
528+
* @todo require additional user information
525529
*/
526530
public function forgotPassword(){
527531
$data = $this->_app->request->post();
@@ -540,26 +544,17 @@ public function forgotPassword(){
540544
$this->_app->halt(400);
541545
}
542546

543-
// Check that the username exists
544-
if(!UserLoader::exists($data['user_name'], 'user_name')) {
545-
$ms->addMessageTranslated("danger", "ACCOUNT_INVALID_USERNAME");
546-
$this->_app->halt(400);
547-
}
547+
// Load the user, by the specified email address
548+
$user = User::where('email', $data['email'])->first();
548549

549-
// Load the user, by username
550-
$user = UserLoader::fetch($data['user_name'], 'user_name');
551-
552-
// Check that the specified email is correct
553-
if (strtolower($user->email) != strtolower($data['email'])){
554-
$ms->addMessageTranslated("danger", "ACCOUNT_USER_OR_EMAIL_INVALID");
555-
$this->_app->halt(400);
550+
// Check that the email exists.
551+
// On failure, we should still pretend like we succeeded, to prevent account enumeration
552+
if(!$user) {
553+
$ms->addMessageTranslated("success", "FORGOTPASS_REQUEST_SUCCESS");
554+
$this->_app->halt(200);
556555
}
557556

558-
// Check if the user has any outstanding lost password requests
559-
if($user->flag_password_reset == 1) {
560-
$ms->addMessageTranslated("danger", "FORGOTPASS_REQUEST_EXISTS");
561-
$this->_app->halt(403);
562-
}
557+
// TODO: rate-limit the number of password reset requests for a given user
563558

564559
// Generate a new password reset request. This will also generate a new secret token for the user.
565560
$event = $user->newEventPasswordReset();
@@ -588,22 +583,21 @@ public function forgotPassword(){
588583
}
589584

590585
/**
591-
* Processes a request to reset a user's password.
586+
* Processes a request to reset a user's password, or set the password for a new user.
592587
*
593-
* Processes the request from the password reset form, which should have the reset token embedded in it, checking that:
594-
* 1. The provided activation token is associated with an existing user account;
595-
* 2. The provided username matches the activation token;
596-
* 3. The user has a lost password request in progress;
597-
* 4. The token has not expired;
598-
* 5. The submitted data (new password) is valid.
588+
* Processes the request from the password create/reset form, which should have the secret token embedded in it, checking that:
589+
* 1. The provided secret token is associated with an existing user account;
590+
* 2. The user has a lost password request in progress;
591+
* 3. The token has not expired;
592+
* 4. The submitted data (new password) is valid.
599593
* This route is "public access".
600594
* Request type: POST
601595
*/
602-
public function resetPassword(){
596+
public function setPassword($flag_new_user = false){
603597
$data = $this->_app->request->post();
604598

605599
// Load the request schema
606-
$requestSchema = new \Fortress\RequestSchema($this->_app->config('schema.path') . "/forms/reset-password.json");
600+
$requestSchema = new \Fortress\RequestSchema($this->_app->config('schema.path') . "/forms/set-password.json");
607601

608602
// Get the alert message stream
609603
$ms = $this->_app->alerts;
@@ -616,20 +610,15 @@ public function resetPassword(){
616610
$this->_app->halt(400);
617611
}
618612

619-
// Fetch the user, by looking up the submitted activation token
620-
$user = UserLoader::fetch($data['secret_token'], 'secret_token');
613+
// Fetch the user, by looking up the submitted secret token
614+
$user = User::where('secret_token', $data['secret_token'])->first();
621615

616+
// If no user exists for this token, just say the token is invalid.
622617
if (!$user){
623618
$ms->addMessageTranslated("danger", "FORGOTPASS_INVALID_TOKEN");
624619
$this->_app->halt(400);
625620
}
626621

627-
// Check that the username matches the submitted username
628-
if (strtolower($user->user_name) != strtolower($data['user_name'])){
629-
$ms->addMessageTranslated("danger", "ACCOUNT_INVALID_USERNAME");
630-
$this->_app->halt(400);
631-
}
632-
633622
// Get the most recent password reset request time
634623
$last_password_reset_time = $user->lastEventTime('password_reset_request');
635624

@@ -639,14 +628,19 @@ public function resetPassword(){
639628
$this->_app->halt(400);
640629
}
641630

642-
// Check the time to see if the token is still valid based on the timeout value. If not valid make the user restart the password request
631+
// Check the time to see if the token is still valid based on the timeout value. If not valid, make the user restart the password request
643632
$current_time = new \DateTime("now");
644633
$last_password_reset_datetime = new \DateTime($last_password_reset_time);
645634
$current_token_life = $current_time->getTimestamp() - $last_password_reset_datetime->getTimestamp();
646635

647-
if($current_token_life >= $this->_app->site->reset_password_timeout || $current_token_life < 0){
648-
// Reset the password reset flag
649-
// TODO: should we do this here, or just when there is a new reset request?
636+
// Compare to appropriate expiration time
637+
if ($flag_new_user)
638+
$expiration = $this->_app->site->create_password_expiration;
639+
else
640+
$expiration = $this->_app->site->reset_password_timeout;
641+
642+
if($current_token_life >= $expiration|| $current_token_life < 0){
643+
// Reset the password reset flag so that they'll be able to submit another request
650644
$user->flag_password_reset = "0";
651645
$user->store();
652646
$ms->addMessageTranslated("danger", "FORGOTPASS_OLD_TOKEN");
@@ -656,7 +650,7 @@ public function resetPassword(){
656650
// Reset the password flag
657651
$user->flag_password_reset = "0";
658652

659-
// Hash the user's password and update
653+
// Hash the user's new password and update
660654
$user->password = Authentication::hashPassword($data['password']);
661655

662656
if (!$user->password){
@@ -665,16 +659,41 @@ public function resetPassword(){
665659
}
666660

667661
// Store the updated info
668-
$user->store();
669-
$ms->addMessageTranslated("success", "ACCOUNT_PASSWORD_UPDATED");
662+
$user->store();
663+
664+
// Log out any existing user, and create a new session
665+
if (!$this->_app->user->isGuest()) {
666+
$this->_app->user->logout(true);
667+
// Use native PHP sessions
668+
session_cache_limiter(false);
669+
session_name("UserFrosting");
670+
// First, initialize the PHP session
671+
session_start();
672+
}
673+
674+
// Log in the user
675+
$user->login();
676+
session_regenerate_id();
677+
678+
// Assume identity
679+
$this->_app->user = $user;
680+
681+
// Store user id in session
682+
$_SESSION["userfrosting"]["user_id"] = $user->id;
683+
684+
// Setup logged in user environment
685+
$this->_app->setupAuthenticatedEnvironment();
686+
$ms = $this->_app->alerts;
687+
$ms->addMessageTranslated("success", "ACCOUNT_WELCOME", $this->_app->user->export());
688+
$ms->addMessageTranslated("success", "ACCOUNT_PASSWORD_UPDATED");
670689
}
671690

672691
/**
673692
* Processes a request to cancel a password reset request.
674693
*
675694
* This is provided so that users can cancel a password reset request, if they made it in error or if it was not initiated by themselves.
676695
* Processes the request from the password reset link, checking that:
677-
* 1. The provided activation token is associated with an existing user account.
696+
* 1. The provided secret token is associated with an existing user account.
678697
* Request type: GET
679698
*/
680699
public function denyResetPassword(){

0 commit comments

Comments
 (0)