From 8211a50e1d3e0a4ffe39fdfef2f10ca1a3055b10 Mon Sep 17 00:00:00 2001
From: Laurent Laffont <llaffont@afi-sa.fr>
Date: Fri, 19 Jun 2020 17:01:16 +0200
Subject: [PATCH] hotline #106221 article forms : check that required input
 have value in post

---
 VERSIONS_HOTLINE/106221                       |   1 +
 .../opac/controllers/FormulaireController.php |  36 ++----
 library/Class/Formulaire/Post.php             | 109 ++++++++++++++++++
 .../controllers/FormulaireControllerTest.php  |  60 +++++++++-
 .../controllers/FormulaireControllerTest.php  |   3 +-
 5 files changed, 175 insertions(+), 34 deletions(-)
 create mode 100644 VERSIONS_HOTLINE/106221
 create mode 100644 library/Class/Formulaire/Post.php

diff --git a/VERSIONS_HOTLINE/106221 b/VERSIONS_HOTLINE/106221
new file mode 100644
index 00000000000..076f7b900e0
--- /dev/null
+++ b/VERSIONS_HOTLINE/106221
@@ -0,0 +1 @@
+ - ticket #106221 : Formulaires dans les articles : renforcement de la détection des bots
\ No newline at end of file
diff --git a/application/modules/opac/controllers/FormulaireController.php b/application/modules/opac/controllers/FormulaireController.php
index fb16a0ca484..472374edcc8 100644
--- a/application/modules/opac/controllers/FormulaireController.php
+++ b/application/modules/opac/controllers/FormulaireController.php
@@ -24,9 +24,17 @@ class FormulaireController extends ZendAfi_Controller_Action {
 
     $post = $this->_request->getPost();
 
-    if ($this->_isPostFromBot($post, $article))
+    $form_post = new Class_Formulaire_Post($post, $article);
+    if ($form_post->isBot())
       return $this->_redirect('/');
 
+    if (!$form_post->isValid()) {
+      array_map([$this->_helper, 'notify'],
+                $form_post->getErrors());
+      $this->_redirectToReferer();
+    }
+
+
     unset($post['website']);
     $formulaire = new Class_Formulaire();
     $formulaire->setData(serialize($post))
@@ -44,32 +52,6 @@ class FormulaireController extends ZendAfi_Controller_Action {
   }
 
 
-  protected function _isPostFromBot($post, $article) {
-    if ($this->_getParam('website'))
-      return true;
-
-    $quote = '[\"\']';
-    $no_quotes = '([^\"\']+)';
-    $quoted_value = $quote.$no_quotes.$quote;
-
-    preg_match_all('/ name='.$quoted_value.'/',
-                   $article->getContenu(),
-                   $all_inputs);
-
-    $clean_input = array_map(function($input)
-                             {
-                               return str_replace(['.',' ','['],
-                                                  '_',
-                                                  $input);
-                             },
-                             $all_inputs[1]);
-    if (array_diff(array_keys($post), $clean_input))
-      return true;
-
-    return false;
-  }
-
-
   protected function _sendFormEmail($address, $body) {
     $mail = new ZendAfi_Mail('utf8');
     $mail->setFrom(Class_Profil::getCurrentProfil()->getMailSiteOrPortail())
diff --git a/library/Class/Formulaire/Post.php b/library/Class/Formulaire/Post.php
new file mode 100644
index 00000000000..1163f2b2679
--- /dev/null
+++ b/library/Class/Formulaire/Post.php
@@ -0,0 +1,109 @@
+<?php
+/**
+ * Copyright (c) 2012-2020, Agence Française Informatique (AFI). All rights reserved.
+ *
+ * BOKEH is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE as published by
+ * the Free Software Foundation.
+ *
+ * There are special exceptions to the terms and conditions of the AGPL as it
+ * is applied to this software (see README file).
+ *
+ * BOKEH is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU AFFERO GENERAL PUBLIC LICENSE for more details.
+ *
+ * You should have received a copy of the GNU AFFERO GENERAL PUBLIC LICENSE
+ * along with BOKEH; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301  USA
+ */
+
+
+class Class_Formulaire_Post {
+  use Trait_Translator;
+
+  protected
+    $_article,
+    $_post,
+    $_regex_quoted_value,
+    $_errors = [];
+
+  public function __construct($post, $article) {
+    $this->_post = $post;
+    $this->_article = $article;
+
+    $quote = '[\"\']';
+    $no_quotes = '([^\"\']+)';
+    $this->_regex_quoted_value = $quote.$no_quotes.$quote;
+  }
+
+
+  public function isBot() {
+    return !($this->_isInvisibleCaptchaValid()
+             &&
+             $this->_allPostValuesDefinedInArticle());
+  }
+
+
+  public function isValid() {
+    $missing_required = $this->_allMissingRequiredInputInPost();
+    foreach($missing_required as $input_name)
+      $this->_errors []= $this->_('Le champ %s est requis', $input_name);
+    return empty($this->_errors);
+  }
+
+
+  public function getErrors() {
+    return $this->_errors;
+  }
+
+
+  protected function _allMissingRequiredInputInPost() {
+    preg_match_all('/ name='.$this->_regex_quoted_value.'[^>]+required=/',
+                   $this->_article->getContenu(),
+                   $all_required_inputs);
+
+    return  array_diff($this->_normalizePhpInputNames($all_required_inputs[1]),
+                       array_keys($this->_post));
+  }
+
+
+  protected function _allPostValuesDefinedInArticle() {
+    preg_match_all('/ name='.$this->_regex_quoted_value.'/',
+                   $this->_article->getContenu(),
+                   $all_inputs);
+
+
+    return array_diff(array_keys($this->_post),
+                      $this->_normalizePhpInputNames($all_inputs[1]))
+      ? false
+      : true;
+  }
+
+
+  /**
+   * @see https://www.php.net/manual/en/language.variables.external.php
+   * Dots and spaces and [ in variable names are converted to underscores. For example <input name="a.b" /> becomes $_REQUEST["a_b"].
+   */
+  protected function _normalizePhpInputNames($input_names) {
+    return array_map(function($input)
+                     {
+                       return str_replace(['.',' ','['],
+                                          '_',
+                                          $input);
+                     },
+                     $input_names);
+  }
+
+
+  protected function _isInvisibleCaptchaValid() {
+    if (!isset($this->_post['website']))
+      return false;
+
+    if ($this->_post['website'])
+      return false;
+
+    return true;
+  }
+}
\ No newline at end of file
diff --git a/tests/application/modules/opac/controllers/FormulaireControllerTest.php b/tests/application/modules/opac/controllers/FormulaireControllerTest.php
index 7eea1070deb..7d16714b713 100644
--- a/tests/application/modules/opac/controllers/FormulaireControllerTest.php
+++ b/tests/application/modules/opac/controllers/FormulaireControllerTest.php
@@ -68,7 +68,9 @@ class FormulaireControllerWithEmailPostActionTest extends FormulaireControllerPo
 
     $this->postDispatch('/formulaire/add/id_article/45',
                         ['nom' => 'Tinguette' ,
-                         'prenom' => 'Quentin' ],
+                         'prenom' => 'Quentin',
+                         'website' => '',
+                         'Dejeuner' => 'Libre'],
                         true);
 
     $this->new_formulaire = Class_Formulaire::find(2);
@@ -124,7 +126,8 @@ class FormulaireControllerPostActionTest extends FormulaireControllerPostActionT
     $this->postDispatch('/formulaire/add/id_article/45',
                         ['nom' => 'Tinguette' ,
                          'prenom' => 'Quentin',
-                         'website' => ''],
+                         'website' => '',
+                         'Dejeuner' => 'Libre'],
                         true);
 
     $this->new_formulaire = Class_Formulaire::find(2);
@@ -144,9 +147,10 @@ class FormulaireControllerPostActionTest extends FormulaireControllerPostActionT
 
 
   /** @test */
-  public function getDataShouldAnswerSerializedNomPrenom() {
+  public function getDataShouldAnswerSerializedNomPrenomDejeuner() {
     $this->assertEquals(serialize(['nom' => 'Tinguette' ,
-                                   'prenom' => 'Quentin']),
+                                   'prenom' => 'Quentin',
+                                   'Dejeuner' => 'Libre']),
                                   $this->new_formulaire->getData());
   }
 
@@ -192,7 +196,9 @@ class FormulaireControllerWithoutConnectedUserPostActionTest extends FormulaireC
     ZendAfi_Auth::getInstance()->clearIdentity();
     $this->postDispatch('/formulaire/add/id_article/45',
                         ['nom' => 'Tinguette' ,
-                         'prenom' => 'Quentin' ],
+                         'prenom' => 'Quentin',
+                         'website' => '',
+                         'Dejeuner' => 'Libre'],
                         true);
 
     $this->new_formulaire = Class_Formulaire::find(2);
@@ -236,6 +242,7 @@ class FormulaireControllerPostAsBotTest extends FormulaireControllerPostActionTe
     $this->postDispatch('/formulaire/add/id_article/45',
                         ['nom' => 'Tinguette' ,
                          'prenom' => 'Quentin',
+                         'Dejeuner' => 'libre',
                          'whoareyou' => 'i am a bot'],
                         true);
 
@@ -255,7 +262,42 @@ class FormulaireControllerPostAsBotTest extends FormulaireControllerPostActionTe
                         true);
     $this->assertNotRedirectTo('/');
   }
+}
+
+
+
+
+class FormulaireControllerPostWithoutRequiredTest extends FormulaireControllerPostActionTestCase {
+  public function setUp() {
+    parent::setUp();
+    $_SERVER['HTTP_REFERER'] = '/viewarticle/id/45';
+    $this->postDispatch('/formulaire/add/id_article/45',
+                        ['nom' => 'Tinguette' ,
+                         'prenom' => 'Quentin',
+                         'option1' => 'cafe',
+                         'option2' => 'wifi',
+                         'website' => ''],
+                        true);
 
+  }
+
+
+  /** @test */
+  public function responseShouldNotifyFieldRequired() {
+    $this->assertFlashMessengerContentContains('Le champ Dejeuner est requis');
+  }
+
+
+  /** @test */
+  public function pageShouldRedirectToReferer() {
+    $this->assertRedirectTo('/viewarticle/id/45');
+  }
+
+
+  public function tearDown() {
+    unset($_SERVER['HTTP_REFERER']);
+    parent::tearDown();
+  }
 }
 
 
@@ -338,10 +380,16 @@ class FormulaireControllerPostActionDebugTestCase extends AbstractControllerTest
                         true);
   }
 
-    /** @test */
+  /** @test */
   public function responseShouldNotRedirect() {
     $this->assertNotRedirectTo('/');
   }
+
+
+  /** @test */
+  public function pageShouldContainsShouldFormRecorded() {
+    $this->assertXPathContentContains('//h2', 'Merci. Le formulaire a bien été enregistré');
+  }
 }
 
 ?>
\ No newline at end of file
diff --git a/tests/application/modules/telephone/controllers/FormulaireControllerTest.php b/tests/application/modules/telephone/controllers/FormulaireControllerTest.php
index 5e544e8a1c0..bd2d8b457d7 100644
--- a/tests/application/modules/telephone/controllers/FormulaireControllerTest.php
+++ b/tests/application/modules/telephone/controllers/FormulaireControllerTest.php
@@ -36,7 +36,8 @@ class Telephone_FormulaireControllerPostActionTestCase extends TelephoneAbstract
 
     $this->postDispatch('/formulaire/add/id_article/45',
                         ['nom' => 'Tinguette' ,
-                         'prenom' => 'Quentin' ]
+                         'prenom' => 'Quentin',
+                         'website' => '' ]
                         ,true);
 
     $this->new_formulaire = Class_Formulaire::find(1);
-- 
GitLab