FIX(sign up form): improve anti-spam

src/Controller/Api/V1/UsersController.php

@@ -472,9 +472,10 @@ class UsersController extends AppController
      * - save tokens in user session
      * - return token for HTTP GET method
      *
+     * @param false $cleanBeforeCreateTokens
      * @return string token for HTTP GET method
      */
-    private function addUserCreateTokens()
+    private function addUserCreateTokens($cleanBeforeCreateTokens = false)
     {
         $prefixToken = bin2hex(openssl_random_pseudo_bytes(12));
         $getSuffixToken = bin2hex(openssl_random_pseudo_bytes(12));
@@ -482,7 +483,7 @@ class UsersController extends AppController
         $formSuffixToken = bin2hex(openssl_random_pseudo_bytes(12));
 
         $dataToken = $this->request->session()->read('CreateUserAntiSpam');
-        if (is_null($dataToken)) {
+        if (is_null($dataToken) | $cleanBeforeCreateTokens === true) {
             $dataToken = [];
         }
         $dataToken["$prefixToken"] = [
@@ -596,8 +597,12 @@ class UsersController extends AppController
         }
 
         // Force token-based URL (HTTP GET and POST methods), expect for json API
-        if (!$this->request->is('json') && $this->addUserIsValidTokenInUrl() === false) {
-            return $this->redirect("/$lang/users/add?t1=". $this->addUserCreateTokens());
+        if (!$this->request->is('json')) {
+            if ($this->request->method() === 'GET' && count($this->request->query) === 0) { // default page: /users/add
+                return $this->redirect("/$lang/users/add?t1=". $this->addUserCreateTokens(false));
+            } elseif ($this->addUserIsValidTokenInUrl() === false) {
+                return $this->redirect("/$lang/users/add?t1=". $this->addUserCreateTokens(true));
+            }
         }
 
         // Process form data
@@ -607,7 +612,7 @@ class UsersController extends AppController
                 $expectedFormToken = $this->addUserGetTokenByType('form');
                 $formToken = $this->request->data("formToken");
                 if ($expectedFormToken !== $formToken | $this->addUserIsFormSentTooQuickly()) {
-                    return $this->redirect("/$lang/users/add?t1=". $this->addUserCreateTokens());
+                    return $this->redirect("/$lang/users/add?t1=". $this->addUserCreateTokens(true));
                 }
             }
 

tests/TestCase/Controller/Api/V1/UsersControllerTest.php

@@ -196,6 +196,7 @@ class UsersControllerTest extends ApiIntegrationTestCase
      * @group public
      * @group user
      * @group createUser
+     * @group wip
      *
      * @return void
      */
@@ -247,17 +248,19 @@ class UsersControllerTest extends ApiIntegrationTestCase
         // Add form token to fixtures
         $userFixture1['formToken'] = $tokenForm1;
 
-        // Try to display form with a wrong token in the URL
-        // --> redirect to empty form (with new generated token)
-        $this->session($backupSession);  // Use backuped SESSION
-        $this->get($expectedUrlPrefix ."bad45-token45");
-        $this->commonCheckForAddHtmlForm($expectedUrlPrefix);
-
         // Try to display form without a token in the URL
-        // --> redirect to empty form (with new generated token)
+        // --> create a new token and add it to the token list
+        // --> redirect to empty form (with the new generated token)
         $this->session($backupSession);  // Use backuped SESSION
         $this->get($testeddUrl);
-        $this->commonCheckForAddHtmlForm($expectedUrlPrefix);
+        $this->commonCheckForAddHtmlForm($expectedUrlPrefix, 2);
+
+        // Try to display form with a wrong token in the URL
+        // --> delete all tokens and create a new token
+        // --> redirect to empty form (with the new generated token)
+        $this->session($backupSession);  // Use backuped SESSION
+        $this->get($expectedUrlPrefix ."bad45-token45");
+        $this->commonCheckForAddHtmlForm($expectedUrlPrefix, 1);
 
         // Check that form contains the tokens
         // for the form processing URL and the hidden field
@@ -270,52 +273,57 @@ class UsersControllerTest extends ApiIntegrationTestCase
         $this->assertContains("id=\"formtoken\" value=\"$tokenForm1\"", $html);
 
         // Try to send form with a wrong token in the URL
-        // --> redirect to empty form (with new generated token)
-        // --> new user has not been created
+        // --> new user hasn't been created
+        // --> delete all tokens and create a new token
+        // --> redirect to empty form (with the new generated token)
         $this->session($backupSession);  // Use backuped SESSION
         $this->assertEquals(0, $this->Users->find()->where(['email' => $emailFixture1])->count()); // not exist in DB
         $this->post("/fr/users/add?t2=$tokenPostMethod1"."badTokenSuffix", $userFixture1);
-        $this->commonCheckForAddHtmlForm($expectedUrlPrefix);
+        $this->commonCheckForAddHtmlForm($expectedUrlPrefix, 1);
         $this->assertEquals(0, $this->Users->find()->where(['email' => $emailFixture1])->count()); // not exist in DB
 
         // Try to send form without a token in the URL
-        // --> redirect to empty form (with new generated token)
-        // --> new user has not been created
+        // --> new user hasn't been created
+        // --> delete all tokens and create a new token
+        // --> redirect to empty form (with the new generated token)
         $this->session($backupSession);  // Use backuped SESSION
         $this->assertEquals(0, $this->Users->find()->where(['email' => $emailFixture1])->count()); // not exist in DB
         $this->post("/fr/users/add", $userFixture1);
-        $this->commonCheckForAddHtmlForm($expectedUrlPrefix);
+        $this->commonCheckForAddHtmlForm($expectedUrlPrefix, 1);
         $this->assertEquals(0, $this->Users->find()->where(['email' => $emailFixture1])->count()); // not exist in DB
 
         // Try to send form with a valid token in the URL, but without a token input in form
-        // --> redirect to empty form (with new generated token)
-        // --> new user has not been created
+        // --> new user hasn't been created
+        // --> delete all tokens and create a new token
+        // --> redirect to empty form (with the new generated token)
         $this->session($backupSession);  // Use backuped SESSION
         $this->assertEquals(0, $this->Users->find()->where(['email' => $emailFixture1])->count()); // not exist in DB
         $userFixture1WithoutFormToken =  $userFixture1;
         unset($userFixture1WithoutFormToken['formToken']);
         $this->post("/fr/users/add?t2=$tokenPostMethod1", $userFixture1WithoutFormToken);
-        $this->commonCheckForAddHtmlForm($expectedUrlPrefix);
+        $this->commonCheckForAddHtmlForm($expectedUrlPrefix, 1);
         $this->assertEquals(0, $this->Users->find()->where(['email' => $emailFixture1])->count()); // not exist in DB
 
         // Try to send form with a valid token in the URL, but with a wrong token input in form
-        // --> redirect to empty form (with new generated token)
-        // --> new user has not been created
+        // --> new user hasn't been created
+        // --> delete all tokens and create a new token
+        // --> redirect to empty form (with the new generated token)
         $this->session($backupSession);  // Use backuped SESSION
         $this->assertEquals(0, $this->Users->find()->where(['email' => $emailFixture1])->count()); // not exist in DB
         $userFixture1WithBadFormToken =  $userFixture1;
         $userFixture1WithBadFormToken['formToken'] = 'bad45-token45';
         $this->post("/fr/users/add?t2=$tokenPostMethod1", $userFixture1WithBadFormToken);
-        $this->commonCheckForAddHtmlForm($expectedUrlPrefix);
+        $this->commonCheckForAddHtmlForm($expectedUrlPrefix, 1);
         $this->assertEquals(0, $this->Users->find()->where(['email' => $emailFixture1])->count()); // not exist in DB
 
         // Try to send form with a valid token in the URL, but too quickly (as a bot)
-        // --> redirect to empty form (with new generated token)
-        // --> new user has not been created
+        // --> new user hasn't been created
+        // --> delete all tokens and create a new token
+        // --> redirect to empty form (with the new generated token)
         $this->session($backupSession);  // Use backuped SESSION
         $this->assertEquals(0, $this->Users->find()->where(['email' => $emailFixture1])->count()); // not exist in DB
         $this->post("/fr/users/add?t2=$tokenPostMethod1", $userFixture1);
-        $this->commonCheckForAddHtmlForm($expectedUrlPrefix);
+        $this->commonCheckForAddHtmlForm($expectedUrlPrefix, 1);
         $this->assertEquals(0, $this->Users->find()->where(['email' => $emailFixture1])->count()); // not exist in DB
 
         // Add delay to look like a real user and not a robot
@@ -337,13 +345,14 @@ class UsersControllerTest extends ApiIntegrationTestCase
      * -> redirect to empty form (with new generated token)
      *
      * @param $expectedUrlPrefix
+     * @param int $expectedNumberOfTokens
      */
-    private function commonCheckForAddHtmlForm($expectedUrlPrefix)
+    private function commonCheckForAddHtmlForm($expectedUrlPrefix, $expectedNumberOfTokens = 1)
     {
         $this->assertResponseCode(302);
         $this->assertRedirectContains($expectedUrlPrefix);
         $this->assertArrayHasKey('CreateUserAntiSpam', $_SESSION);
-        $this->assertEquals(2, \count($_SESSION['CreateUserAntiSpam']));
+        $this->assertEquals($expectedNumberOfTokens, \count($_SESSION['CreateUserAntiSpam']));
         $tokenPrefix = array_key_last($_SESSION['CreateUserAntiSpam']);
         $tokenGetMethod = $_SESSION['CreateUserAntiSpam'][$tokenPrefix]['getToken'];
         $expectedUrl = $expectedUrlPrefix . $tokenGetMethod;