improve UX for unvalidated accounts

elabftw · Jul 28, 2023 · e80df9e · e80df9e
1 parent 1fa3c13
commit e80df9e
Show file tree

Hide file tree

Showing 18 changed files with 90 additions and 64 deletions.
diff --git a/src/Auth/Anon.php b/src/Auth/Anon.php
@@ -28,6 +28,7 @@ public function __construct(array $configArr, int $team)
         $this->AuthResponse = new AuthResponse();
         $this->AuthResponse->userid = 0;
         $this->AuthResponse->isAnonymous = true;
+        $this->AuthResponse->isValidated = true;
         $this->AuthResponse->selectedTeam = $team;
     }
 

diff --git a/src/Auth/Cookie.php b/src/Auth/Cookie.php
@@ -35,7 +35,7 @@ public function tryAuth(): AuthResponse
     {
         // compare the provided token with the token saved in SQL database
         $sql = sprintf(
-            'SELECT userid, mfa_secret, auth_service
+            'SELECT userid, mfa_secret, auth_service, validated
             FROM users WHERE token = :token AND token_created_at + INTERVAL %d MINUTE > NOW() LIMIT 1',
             $this->validityMinutes
         );
@@ -57,6 +57,7 @@ public function tryAuth(): AuthResponse
 
         $this->AuthResponse->userid = $userid;
         $this->AuthResponse->mfaSecret = $res['mfa_secret'];
+        $this->AuthResponse->isValidated = (bool) $res['validated'];
         $this->AuthResponse->selectedTeam = $this->team;
 
         // Force user to login again to activate MFA if it is enforced for local auth and there is no mfaSecret

diff --git a/src/Auth/External.php b/src/Auth/External.php
@@ -65,6 +65,7 @@ public function tryAuth(): AuthResponse
             $this->log->info('New user (' . $email . ') autocreated from external auth');
         }
         $this->AuthResponse->userid = (int) $Users->userData['userid'];
+        $this->AuthResponse->isValidated = true;
         $this->AuthResponse->setTeams();
 
         return $this->AuthResponse;

diff --git a/src/Auth/Ldap.php b/src/Auth/Ldap.php
@@ -112,6 +112,7 @@ public function tryAuth(): AuthResponse
 
         $this->AuthResponse->userid = (int) $Users->userData['userid'];
         $this->AuthResponse->mfaSecret = $Users->userData['mfa_secret'];
+        $this->AuthResponse->isValidated = (bool) $Users->userData['validated'];
         $this->AuthResponse->setTeams();
 
         return $this->AuthResponse;

diff --git a/src/Auth/Local.php b/src/Auth/Local.php
@@ -116,7 +116,7 @@ private function getUseridFromEmail(): int
     {
         try {
             $Users = ExistingUser::fromEmail($this->email);
-        } catch (ResourceNotFoundException $e) {
+        } catch (ResourceNotFoundException) {
             // here we rethrow an quantum exception because we don't want to let the user know if the email exists or not
             throw new QuantumException(_('Invalid email/password combination.'));
         }
@@ -175,8 +175,7 @@ private function authWithSha(): void
 
     private function authWithModernAlgo(): void
     {
-        $sql = 'SELECT `users`.`password_hash`, `users`.`mfa_secret`
-            FROM `users` WHERE `userid` = :userid;';
+        $sql = 'SELECT password_hash, mfa_secret, validated FROM users WHERE userid = :userid;';
         $req = $this->Db->prepare($sql);
         $req->bindParam(':userid', $this->userid, PDO::PARAM_INT);
         $this->Db->execute($req);
@@ -197,6 +196,7 @@ private function authWithModernAlgo(): void
 
         $this->AuthResponse->userid = $this->userid;
         $this->AuthResponse->mfaSecret = $res['mfa_secret'];
+        $this->AuthResponse->isValidated = (bool) $res['validated'];
         $this->AuthResponse->setTeams();
     }
 }
diff --git a/src/Auth/Mfa.php b/src/Auth/Mfa.php
@@ -37,6 +37,7 @@ public function tryAuth(): AuthResponse
 
         $this->AuthResponse->hasVerifiedMfa = true;
         $this->AuthResponse->mfaSecret = $Users->userData['mfa_secret'];
+        $this->AuthResponse->isValidated = (bool) $Users->userData['validated'];
         $this->AuthResponse->userid = $this->MfaHelper->userid;
         $this->AuthResponse->setTeams();
 

diff --git a/src/Auth/Saml.php b/src/Auth/Saml.php
@@ -160,6 +160,7 @@ public function assertIdpResponse(): AuthResponse
 
         $this->AuthResponse->userid = $userid;
         $this->AuthResponse->mfaSecret = $Users->userData['mfa_secret'];
+        $this->AuthResponse->isValidated = (bool) $Users->userData['validated'];
 
         // synchronize the teams from the IDP
         // because teams can change since the time the user was created

diff --git a/src/Auth/Team.php b/src/Auth/Team.php
@@ -25,6 +25,7 @@ public function __construct(int $userid, int $team)
     {
         $this->AuthResponse = new AuthResponse();
         $this->AuthResponse->userid = $userid;
+        $this->AuthResponse->isValidated = true;
         $this->AuthResponse->selectedTeam = $team;
     }
 

diff --git a/src/classes/AuthResponse.php b/src/classes/AuthResponse.php
@@ -29,6 +29,8 @@ class AuthResponse
     // when user needs to request access to a team
     public bool $initTeamRequired = false;
 
+    public bool $isValidated = false;
+
     // info (email/name) about user that needs to request a team
     public array $initTeamUserInfo = array();
 

diff --git a/src/controllers/LoginController.php b/src/controllers/LoginController.php
@@ -94,6 +94,33 @@ public function getResponse(): Response
         // TRY TO AUTHENTICATE
         $AuthResponse = $this->getAuthService($authType)->tryAuth();
 
+        ////////////////////
+        // TEAM SELECTION //
+        ////////////////////
+        // if the user is in several teams, we need to redirect to the team selection
+        if ($AuthResponse->isInSeveralTeams) {
+            $this->App->Session->set('team_selection_required', true);
+            $this->App->Session->set('team_selection', $AuthResponse->selectableTeams);
+            $this->App->Session->set('auth_userid', $AuthResponse->userid);
+            // carry over the cookie
+            $this->App->Session->set('rememberme', $icanhazcookies);
+            return new RedirectResponse('/login.php');
+        }
+
+        // no team was found so user must select one
+        if ($AuthResponse->initTeamRequired) {
+            $this->App->Session->set('initial_team_selection_required', true);
+            $this->App->Session->set('teaminit_email', $AuthResponse->initTeamUserInfo['email']);
+            $this->App->Session->set('teaminit_firstname', $AuthResponse->initTeamUserInfo['firstname']);
+            $this->App->Session->set('teaminit_lastname', $AuthResponse->initTeamUserInfo['lastname']);
+            return new RedirectResponse('/login.php');
+        }
+
+        // send a helpful message if account requires validation, needs to be after team selection
+        if ($AuthResponse->isValidated === false) {
+            throw new ImproperActionException(_('Your account is not validated. An admin of your team needs to validate it!'));
+        }
+
         /////////////////
         // ENFORCE MFA //
         /////////////////
@@ -116,7 +143,7 @@ public function getResponse(): Response
             $this->App->Session->set('auth_userid', $AuthResponse->userid);
             $this->App->Session->set('rememberme', $icanhazcookies);
 
-            return new RedirectResponse('../../login.php');
+            return new RedirectResponse('/login.php');
         }
 
         /////////
@@ -129,35 +156,14 @@ public function getResponse(): Response
             // remember which user is authenticated
             $this->App->Session->set('auth_userid', $AuthResponse->userid);
             $this->App->Session->set('rememberme', $icanhazcookies);
-            return new RedirectResponse('../../login.php');
+            return new RedirectResponse('/login.php');
         }
         if ($AuthResponse->hasVerifiedMfa) {
             $this->App->Session->remove('enforce_mfa');
             $this->App->Session->remove('mfa_auth_required');
             $this->App->Session->remove('mfa_secret');
         }
 
-        ////////////////////
-        // TEAM SELECTION //
-        ////////////////////
-        // if the user is in several teams, we need to redirect to the team selection
-        if ($AuthResponse->isInSeveralTeams) {
-            $this->App->Session->set('team_selection_required', true);
-            $this->App->Session->set('team_selection', $AuthResponse->selectableTeams);
-            $this->App->Session->set('auth_userid', $AuthResponse->userid);
-            // carry over the cookie
-            $this->App->Session->set('rememberme', $icanhazcookies);
-            return new RedirectResponse('../../login.php');
-        }
-
-        // no team was found so user must select one
-        if ($AuthResponse->initTeamRequired) {
-            $this->App->Session->set('initial_team_selection_required', true);
-            $this->App->Session->set('teaminit_email', $AuthResponse->initTeamUserInfo['email']);
-            $this->App->Session->set('teaminit_firstname', $AuthResponse->initTeamUserInfo['firstname']);
-            $this->App->Session->set('teaminit_lastname', $AuthResponse->initTeamUserInfo['lastname']);
-            return new RedirectResponse('../../login.php');
-        }
         // All good now we can login the user
         $LoginHelper = new LoginHelper($AuthResponse, $this->App->Session);
         $LoginHelper->login((bool) $icanhazcookies);

diff --git a/src/models/ExistingUser.php b/src/models/ExistingUser.php
@@ -9,42 +9,19 @@
 
 namespace Elabftw\Models;
 
-use Elabftw\Elabftw\Db;
-use Elabftw\Exceptions\ResourceNotFoundException;
-
 /**
- * A user that exists in the db, so we have a userid but not necessarily a team
+ * A user that exists in the db, so we have a userid but not necessarily a team, and they might not be validated
  */
 class ExistingUser extends Users
 {
     public static function fromEmail(string $email): Users
     {
-        $Db = Db::getConnection();
-        $sql = 'SELECT userid FROM users
-            WHERE email = :email AND archived = 0 AND validated = 1 LIMIT 1';
-        $req = $Db->prepare($sql);
-        $req->bindParam(':email', $email);
-        $Db->execute($req);
-        $res = $req->fetchColumn();
-        if ($res === false) {
-            throw new ResourceNotFoundException();
-        }
-        return new self((int) $res);
+        return self::search('email', $email);
     }
 
     public static function fromOrgid(string $orgid): Users
     {
-        $Db = Db::getConnection();
-        $sql = 'SELECT userid FROM users
-            WHERE orgid = :orgid AND archived = 0 AND validated = 1 LIMIT 1';
-        $req = $Db->prepare($sql);
-        $req->bindParam(':orgid', $orgid);
-        $Db->execute($req);
-        $res = $req->fetchColumn();
-        if ($res === false) {
-            throw new ResourceNotFoundException();
-        }
-        return new self((int) $res);
+        return self::search('orgid', $orgid);
     }
 
     public static function fromScratch(

diff --git a/src/models/Users.php b/src/models/Users.php
@@ -18,6 +18,7 @@
 use Elabftw\Exceptions\IllegalActionException;
 use Elabftw\Exceptions\ImproperActionException;
 use Elabftw\Exceptions\InvalidCredentialsException;
+use Elabftw\Exceptions\ResourceNotFoundException;
 use Elabftw\Interfaces\RestInterface;
 use Elabftw\Models\Notifications\SelfIsValidated;
 use Elabftw\Models\Notifications\SelfNeedValidation;
@@ -442,6 +443,28 @@ public function checkCurrentPasswordOrExplode(?string $currentPassword): void
         }
     }
 
+    protected static function search(string $column, string $term, bool $validated = false): self
+    {
+        $searchColumn = 'email';
+        if ($column === 'orgid') {
+            $searchColumn = 'orgid';
+        }
+        $validatedFilter = '';
+        if ($validated) {
+            $validatedFilter = ' AND validated = 1 ';
+        }
+        $Db = Db::getConnection();
+        $sql = sprintf('SELECT userid FROM users WHERE %s = :term AND archived = 0 %s LIMIT 1', $searchColumn, $validatedFilter);
+        $req = $Db->prepare($sql);
+        $req->bindParam(':term', $term);
+        $Db->execute($req);
+        $res = $req->fetchColumn();
+        if ($res === false) {
+            throw new ResourceNotFoundException();
+        }
+        return new self((int) $res);
+    }
+
     /**
      * Remove sensitives values from readFromQuery()
      */

diff --git a/src/models/ValidatedUser.php b/src/models/ValidatedUser.php
@@ -14,6 +14,16 @@
  */
 class ValidatedUser extends ExistingUser
 {
+    public static function fromEmail(string $email): Users
+    {
+        return self::search('email', $email, true);
+    }
+
+    public static function fromOrgid(string $orgid): Users
+    {
+        return self::search('orgid', $orgid, true);
+    }
+
     public static function fromExternal(string $email, array $teams, string $firstname, string $lastname): Users
     {
         return parent::fromScratch($email, $teams, $firstname, $lastname, null, true);

diff --git a/src/services/ResetPasswordKey.php b/src/services/ResetPasswordKey.php
@@ -13,8 +13,9 @@
 use Defuse\Crypto\Key;
 use Elabftw\Exceptions\IllegalActionException;
 use Elabftw\Exceptions\ImproperActionException;
-use Elabftw\Models\ExistingUser;
 use Elabftw\Models\Users;
+use Elabftw\Models\ValidatedUser;
+
 use function explode;
 use function implode;
 use function sprintf;
@@ -66,7 +67,7 @@ public function validate(string $key): Users
             throw new ImproperActionException(sprintf(_('This link has expired! Password reset links are only valid for %s minutes.'), self::LINK_LIFETIME));
         }
 
-        // if the key is correct, we now have an ExistingUser here
-        return ExistingUser::fromEmail($email);
+        // if the key is correct, we now have a ValidatedUser here
+        return ValidatedUser::fromEmail($email);
     }
 }
diff --git a/src/templates/login.html b/src/templates/login.html
@@ -14,6 +14,7 @@ <h5 class='modal-title' id='resetModalLabel'>{{ 'Reset your password'|trans }}</
       </div>
       <div class='modal-body' data-wait='{{ 'Please wait…' }}'>
         <form name='resetPass' method='post' action='app/controllers/ResetPasswordController.php'>
+          {{ App.Session.get('csrf')|csrf }}
           <input class='form-control' placeholder='{{ 'Enter your email address'|trans }}' name='email' type='email' required />
           <div class='text-center'>
             <button class='btn btn-primary mt-2' type="submit" name="Submit">{{ 'Send reset link'|trans }}</button>

diff --git a/src/templates/sysconfig.html b/src/templates/sysconfig.html
@@ -731,7 +731,7 @@ <h4 data-action='toggle-next' class='d-inline togglable-section-title'><i class=
             <hr>
 
             <div class='d-flex justify-content-between'>
-              <label for='ssoBinding_{{ idp.id }}' class='col-form-label'>Single Sign-On binding (only Redirect is supported)</label>
+              <label for='ssoBinding_{{ idp.id }}' class='col-form-label'>Single Sign-On binding</label>
               <select class='form-control col-md-3' id='ssoBinding_{{ idp.id }}' data-trigger='change' data-model='idps/{{ idp.id }}' data-target='sso_binding'>
                 <option value='urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect' {{ idp.sso_binding == 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect' ? 'selected' }}>Redirect</option>
                 <option value='urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST' {{ idp.sso_binding == 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST' ? 'selected' }}>POST</option>

diff --git a/web/app/controllers/RegisterController.php b/web/app/controllers/RegisterController.php
@@ -1,12 +1,11 @@
-<?php
+<?php declare(strict_types=1);
 /**
  * @author Nicolas CARPi <nico-git@deltablot.email>
  * @copyright 2012 Nicolas CARPi
  * @see https://www.elabftw.net Official website
  * @license AGPL-3.0
  * @package elabftw
  */
-declare(strict_types=1);
 
 namespace Elabftw\Elabftw;
 
@@ -20,7 +19,7 @@
 require_once dirname(__DIR__) . '/init.inc.php';
 
 // default location to redirect to
-$location = '../../login.php';
+$location = '/login.php';
 
 try {
     // check for disabled local register
@@ -63,16 +62,16 @@
 } catch (ImproperActionException $e) {
     // show message to user
     $App->Session->getFlashBag()->add('ko', $e->getMessage());
-    $location = '../../register.php';
+    $location = '/register.php';
 } catch (IllegalActionException $e) {
     $App->Log->notice('', array(array('userid' => $App->Session->get('userid')), array('IllegalAction', $e->getMessage())));
     $App->Session->getFlashBag()->add('ko', Tools::error(true));
-    $location = '../../register.php';
+    $location = '/register.php';
 } catch (Exception $e) {
     // log error and show general error message
     $App->Log->error('', array('Exception' => $e));
     $App->Session->getFlashBag()->add('ko', Tools::error());
-    $location = '../../register.php';
+    $location = '/register.php';
 } finally {
     $Response = new RedirectResponse($location);
     $Response->send();

diff --git a/web/app/controllers/ResetPasswordController.php b/web/app/controllers/ResetPasswordController.php
@@ -65,7 +65,7 @@
         // If user is not validated, the password reset form won't work
         // this is because often users don't understand that their account needs to be
         // validated and just reset their password twenty times
-        if ($Users->userData['validated'] === '0') {
+        if ($Users->userData['validated'] === 0) {
             throw new ImproperActionException(_('Your account is not validated. An admin of your team needs to validate it!'));
         }