[TASK] Refactor suggest wizard JS communication 44/50544/4
authorChristian Kuhn <lolli@schwarzbu.ch>
Tue, 8 Nov 2016 13:12:49 +0000 (14:12 +0100)
committerFrank Naegler <frank.naegler@typo3.org>
Wed, 9 Nov 2016 13:29:38 +0000 (14:29 +0100)
Transmitting the full $row in suggest wizard was introduced to
find the relevant flex form data structure in the ajax search action
to determine the suggest wizard field configuration

The patch adds the data structure identifier instead, the searchAction()
uses that to fetch the specified flex form data structure directly.

Additionally, field values relevant for the ajax call are now signed with
hmac and checked in searchAction().

Change-Id: Ibb5004d4d11487fe999072999c7ce2f847aac750
Resolves: #78616
Related: #78581
Releases: master
Reviewed-on: https://review.typo3.org/50544
Reviewed-by: Wouter Wolters <typo3@wouterwolters.nl>
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Michael Oehlhof <typo3@oehlhof.de>
Tested-by: Michael Oehlhof <typo3@oehlhof.de>
Reviewed-by: Frank Naegler <frank.naegler@typo3.org>
Tested-by: Frank Naegler <frank.naegler@typo3.org>
typo3/sysext/backend/Classes/Form/Element/AbstractFormElement.php
typo3/sysext/backend/Classes/Form/FormDataProvider/TcaFlexPrepare.php
typo3/sysext/backend/Classes/Form/Wizard/SuggestWizard.php
typo3/sysext/backend/Resources/Private/Templates/Wizards/SuggestWizard.html
typo3/sysext/backend/Resources/Public/JavaScript/FormEngineSuggest.js
typo3/sysext/backend/Tests/Unit/Form/FormDataProvider/TcaFlexPrepareTest.php
typo3/sysext/backend/Tests/Unit/Form/Wizard/SuggestWizardTest.php
typo3/sysext/core/Classes/Utility/GeneralUtility.php

index 49c28e5..4b851f3 100644 (file)
@@ -387,9 +387,15 @@ abstract class AbstractFormElement extends AbstractNode
                     if (!empty($PA['fieldTSConfig']['suggest.']['default.']['hide'])) {
                         break;
                     }
+                    // The suggest wizard needs to know if we're in flex form scope to use the dataStructureIdentifier.
+                    // If so, add the processedTca of the flex config as wizard argument.
+                    $flexFormConfig = [];
+                    if ($this->data['processedTca']['columns'][$field]['config']['type'] === 'flex') {
+                        $flexFormConfig = $this->data['processedTca']['columns'][$field];
+                    }
                     /** @var SuggestWizard $suggestWizard */
                     $suggestWizard = GeneralUtility::makeInstance(SuggestWizard::class);
-                    $otherWizards[] = $suggestWizard->renderSuggestSelector($PA['itemFormElName'], $table, $field, $row, $PA);
+                    $otherWizards[] = $suggestWizard->renderSuggestSelector($PA['itemFormElName'], $table, $field, $row, $PA, $flexFormConfig);
                     break;
             }
         }
index 339b811..054fd30 100644 (file)
@@ -70,6 +70,8 @@ class TcaFlexPrepare implements FormDataProviderInterface
             $fieldName,
             $result['databaseRow']
         );
+        // Add the identifier to TCA to use it later during rendering
+        $result['processedTca']['columns'][$fieldName]['config']['dataStructureIdentifier'] = $dataStructureIdentifier;
         $dataStructureArray = $flexFormTools->parseDataStructureByIdentifier($dataStructureIdentifier);
         if (!isset($dataStructureArray['meta']) || !is_array($dataStructureArray['meta'])) {
             $dataStructureArray['meta'] = [];
index d8ea1e5..b89c2b1 100644 (file)
@@ -38,30 +38,42 @@ class SuggestWizard
 
     /**
      * Construct
+     *
+     * @param StandaloneView $view
      */
-    public function __construct()
+    public function __construct(StandaloneView $view = null)
     {
-        $this->view = $this->getFluidTemplateObject('SuggestWizard.html');
+        $this->view = $view ?: $this->getFluidTemplateObject('SuggestWizard.html');
     }
 
     /**
      * Renders an ajax-enabled text field. Also adds required JS
      *
-     * @param string $fieldname The fieldname in the form
+     * @param string $fieldName The field name in the form
      * @param string $table The table we render this selector for
      * @param string $field The field we render this selector for
      * @param array $row The row which is currently edited
      * @param array $config The TSconfig of the field
+     * @param array $flexFormConfig If field is within flex form, this is the TCA config of the flex field
+     * @throws \RuntimeException for incomplete incoming arguments
      * @return string The HTML code for the selector
      */
-    public function renderSuggestSelector($fieldname, $table, $field, array $row, array $config)
+    public function renderSuggestSelector($fieldName, $table, $field, array $row, array $config, array $flexFormConfig = [])
     {
-        $isFlexFormField = $GLOBALS['TCA'][$table]['columns'][$field]['config']['type'] === 'flex';
-        if ($isFlexFormField) {
+        $dataStructureIdentifier = '';
+        if (!empty($flexFormConfig) && $flexFormConfig['config']['type'] === 'flex') {
             $fieldPattern = 'data[' . $table . '][' . $row['uid'] . '][';
-            $flexformField = str_replace($fieldPattern, '', $fieldname);
+            $flexformField = str_replace($fieldPattern, '', $fieldName);
             $flexformField = substr($flexformField, 0, -1);
             $field = str_replace([']['], '|', $flexformField);
+            if (!isset($flexFormConfig['config']['dataStructureIdentifier'])) {
+                throw new \RuntimeException(
+                    'A data structure identifier must be set in [\'config\'] part of a flex form.'
+                    . ' This is usually added by TcaFlexPrepare data processor',
+                    1478604742
+                );
+            }
+            $dataStructureIdentifier = $flexFormConfig['config']['dataStructureIdentifier'];
         }
 
         // Get minimumCharacters from TCA
@@ -69,7 +81,7 @@ class SuggestWizard
         if (isset($config['fieldConf']['config']['wizards']['suggest']['default']['minimumCharacters'])) {
             $minChars = (int)$config['fieldConf']['config']['wizards']['suggest']['default']['minimumCharacters'];
         }
-        // Overwrite it with minimumCharacters from TSConfig (TCEFORM) if given
+        // Overwrite it with minimumCharacters from TSConfig if given
         if (isset($config['fieldTSConfig']['suggest.']['default.']['minimumCharacters'])) {
             $minChars = (int)$config['fieldTSConfig']['suggest.']['default.']['minimumCharacters'];
         }
@@ -81,24 +93,23 @@ class SuggestWizard
             $type = $config['fieldConf']['config']['type'];
         }
 
-        $jsRow = '';
-        if ($isFlexFormField || !MathUtility::canBeInterpretedAsInteger($row['uid'])) {
-            // Ff we have a new record, we hand that row over to JS.
-            // This way we can properly retrieve the configuration of our wizard
-            // if it is shown in a flexform
-            $jsRow = serialize($row);
-        }
+        // Sign those parameters that come back in an ajax request to configure the search in searchAction()
+        $hmac = GeneralUtility::hmac(
+            (string)$table . (string)$field . (string)$row['uid'] . (string)$row['pid'] . (string)$dataStructureIdentifier,
+            'formEngineSuggest'
+        );
 
         $this->view->assignMultiple([
                 'placeholder' => 'LLL:EXT:lang/locallang_core.xlf:labels.findRecord',
-                'fieldname' => $fieldname,
+                'fieldname' => $fieldName,
                 'table' => $table,
                 'field' => $field,
                 'uid' => $row['uid'],
                 'pid' => (int)$row['pid'],
+                'dataStructureIdentifier' => $dataStructureIdentifier,
                 'fieldtype' => $type,
                 'minchars' => (int)$minChars,
-                'recorddata' => $jsRow
+                'hmac' => $hmac,
             ]
         );
 
@@ -110,36 +121,81 @@ class SuggestWizard
      *
      * @param ServerRequestInterface $request
      * @param ResponseInterface $response
+     * @throws \RuntimeException for incomplete or invalid arguments
      * @return ResponseInterface
      */
     public function searchAction(ServerRequestInterface $request, ResponseInterface $response)
     {
         $parsedBody = $request->getParsedBody();
-        $queryParams = $request->getQueryParams();
-
-        // Get parameters from $_GET/$_POST
-        $search = isset($parsedBody['value']) ? $parsedBody['value'] : $queryParams['value'];
-        $table = isset($parsedBody['table']) ? $parsedBody['table'] : $queryParams['table'];
-        $field = isset($parsedBody['field']) ? $parsedBody['field'] : $queryParams['field'];
-        $uid = isset($parsedBody['uid']) ? $parsedBody['uid'] : $queryParams['uid'];
-        $pageId = (int)(isset($parsedBody['pid']) ? $parsedBody['pid'] : $queryParams['pid']);
-        $newRecordRow = isset($parsedBody['newRecordRow']) ? $parsedBody['newRecordRow'] : $queryParams['newRecordRow'];
-        // If the $uid is numeric, we have an already existing element, so get the
-        // TSconfig of the page itself or the element container (for non-page elements)
-        // otherwise it's a new element, so use given id of parent page (i.e., don't modify it here)
-        if (is_numeric($uid)) {
-            $row = BackendUtility::getRecord($table, $uid);
-            if ($table === 'pages') {
-                $pageId = $uid;
-            } else {
-                $pageId = $row['pid'];
-            }
+
+        if (!isset($parsedBody['value'])
+            || !isset($parsedBody['table'])
+            || !isset($parsedBody['field'])
+            || !isset($parsedBody['uid'])
+            || !isset($parsedBody['dataStructureIdentifier'])
+            || !isset($parsedBody['hmac'])
+        ) {
+            throw new \RuntimeException(
+                'Missing at least one of the required arguments "value", "table", "field", "uid"'
+                . ', "dataStructureIdentifier" or "hmac"',
+                1478607036
+            );
+        }
+
+        $search = $parsedBody['value'];
+        $table = $parsedBody['table'];
+        $field = $parsedBody['field'];
+        $uid = $parsedBody['uid'];
+        $pid = (int)$parsedBody['pid'];
+
+        // flex form section container identifiers are created on js side dynamically "onClick". Those are
+        // not within the generated hmac ... the js side adds "idx{dateInMilliseconds}-", so this is removed here again.
+        // example outgoing in renderSuggestSelector():
+        // flex_1|data|sSuggestCheckCombination|lDEF|settings.subelements|el|ID-356586b0d3-form|item|el|content|vDEF
+        // incoming here:
+        // flex_1|data|sSuggestCheckCombination|lDEF|settings.subelements|el|ID-356586b0d3-idx1478611729574-form|item|el|content|vDEF
+        // Note: For existing containers, these parts are numeric, so "ID-356586b0d3-idx1478611729574-form" becomes 1 or 2, etc.
+        // @todo: This could be kicked is the flex form section containers are moved to an ajax call on creation
+        $fieldForHmac = preg_replace('/idx\d{13}-/', '', $field);
+
+        $dataStructureIdentifierString = '';
+        if (!empty($parsedBody['dataStructureIdentifier'])) {
+            $dataStructureIdentifierString = json_encode($parsedBody['dataStructureIdentifier']);
+        }
+
+        $incomingHmac = $parsedBody['hmac'];
+        $calculatedHmac = GeneralUtility::hmac(
+            $table . $fieldForHmac . $uid . $pid . $dataStructureIdentifierString,
+            'formEngineSuggest'
+        );
+        if ($incomingHmac !== $calculatedHmac) {
+            throw new \RuntimeException(
+                'Incoming and calculated hmac do not match',
+                1478608245
+            );
+        }
+
+        // If the $uid is numeric (existing page) and a suggest wizard in pages is handled, the effective
+        // pid is the uid of that page - important for page ts config configuration.
+        if (MathUtility::canBeInterpretedAsInteger($uid) && $table === 'pages') {
+            $pid = $uid;
+        }
+        $TSconfig = BackendUtility::getPagesTSconfig($pid);
+
+        // Determine TCA config of field
+        if (empty($dataStructureIdentifierString)) {
+            // Normal columns field
+            $fieldConfig = $GLOBALS['TCA'][$table]['columns'][$field]['config'];
         } else {
-            $row = unserialize($newRecordRow);
+            // A flex flex form field
+            $flexFormTools = GeneralUtility::makeInstance(FlexFormTools::class);
+            $dataStructureArray = $flexFormTools->parseDataStructureByIdentifier($dataStructureIdentifierString);
+            $parts = explode('|', $field);
+            $fieldConfig = $this->getFieldConfiguration($parts, $dataStructureArray);
+            // Flexform field name levels are separated with | instead of encapsulation in [];
+            // reverse this here to be compatible with regular field names.
+            $field = str_replace('|', '][', $field);
         }
-        $TSconfig = BackendUtility::getPagesTSconfig($pageId);
-        $fieldConfig = $GLOBALS['TCA'][$table]['columns'][$field]['config'];
-        $this->overrideFieldNameAndConfigurationForFlexform($table, $field, $row, $fieldConfig);
 
         $wizardConfig = $fieldConfig['wizards']['suggest'];
 
@@ -165,7 +221,7 @@ class SuggestWizard
             if (isset($config['addWhere'])) {
                 $replacement = [
                     '###THIS_UID###' => (int)$uid,
-                    '###CURRENT_PID###' => (int)$pageId
+                    '###CURRENT_PID###' => (int)$pid
                 ];
                 if (isset($TSconfig['TCEFORM.'][$table . '.'][$field . '.'])) {
                     $fieldTSconfig = $TSconfig['TCEFORM.'][$table . '.'][$field . '.'];
@@ -230,7 +286,7 @@ class SuggestWizard
      */
     protected function currentBackendUserMayAccessTable(array $tableConfig)
     {
-        if ($GLOBALS['BE_USER']->isAdmin()) {
+        if ($this->getBackendUser()->isAdmin()) {
             return true;
         }
 
@@ -244,53 +300,6 @@ class SuggestWizard
     }
 
     /**
-     * Checks if the query comes from a Flexform element and if yes, resolves the field configuration from the Flexform
-     * data structure.
-     *
-     * @param string $table
-     * @param string &$field The field identifier, either a simple table field or a Flexform field path separated with |
-     * @param array $row The row we're dealing with; optional (only required for Flexform records)
-     * @param array|NULL &$fieldConfig
-     */
-    protected function overrideFieldNameAndConfigurationForFlexform($table, &$field, array $row, &$fieldConfig)
-    {
-        // check if field is a flexform reference
-        if (strpos($field, '|') === false) {
-            $fieldConfig = $GLOBALS['TCA'][$table]['columns'][$field]['config'];
-        } else {
-            $parts = explode('|', $field);
-            if ($GLOBALS['TCA'][$table]['columns'][$parts[0]]['config']['type'] !== 'flex') {
-                return;
-            }
-
-            $flexfieldTCAConfig = $GLOBALS['TCA'][$table]['columns'][$parts[0]];
-            if (substr($row['uid'], 0, 3) === 'NEW') {
-                // We have to cleanup record information as they are coming from FormEngines DataProvider
-                $pointerFields = GeneralUtility::trimExplode(',', $flexfieldTCAConfig['ds_pointerField']);
-                foreach ($pointerFields as $pointerField) {
-                    if (is_array($row[$pointerField])) {
-                        $row[$pointerField] = $row[$pointerField][0];
-                    }
-                }
-            }
-            // @todo: Better hand around the data structure identifier. This would free us from $row usage
-            // @todo: and getDataStructureIdentifier() would not have to be called anymore at all.
-            $flexFormTools = GeneralUtility::makeInstance(FlexFormTools::class);
-            $dataStructureIdentifier = $flexFormTools->getDataStructureIdentifier(
-                $flexfieldTCAConfig,
-                $table,
-                $parts[0],
-                $row
-            );
-            $dataStructureArray = $flexFormTools->parseDataStructureByIdentifier($dataStructureIdentifier);
-            $fieldConfig = $this->getFieldConfiguration($parts, $dataStructureArray);
-            // Flexform field name levels are separated with | instead of encapsulation in [];
-            // reverse this here to be compatible with regular field names.
-            $field = str_replace('|', '][', $field);
-        }
-    }
-
-    /**
      * Get configuration for given field by traversing the flexform path to field
      * given in $parts
      *
@@ -546,4 +555,12 @@ class SuggestWizard
         $view->getRequest()->setControllerExtensionName('Backend');
         return $view;
     }
+
+    /**
+     * @return \TYPO3\CMS\Core\Authentication\BackendUserAuthentication
+     */
+    protected function getBackendUser()
+    {
+        return $GLOBALS['BE_USER'];
+    }
 }
index 4bfb8d4..7b485b2 100644 (file)
                data-field="{field -> f:format.htmlspecialchars()}"
                data-uid="{uid}"
                data-pid="{pid}"
+               data-datastructureidentifier="{dataStructureIdentifier}"
+               data-hmac="{hmac}"
                data-fieldtype="{fieldtype -> f:format.htmlspecialchars()}"
                data-minchars="{minchars}"
-               data-recorddata="{recorddata -> f:format.htmlspecialchars()}"
         />
     </div>
 </div>
index b9fdbe1..5e179e4 100644 (file)
@@ -22,7 +22,8 @@ define(['jquery', 'jquery/autocomplete'], function ($) {
                        field = $searchField.data('field'),
                        uid = $searchField.data('uid'),
                        pid = $searchField.data('pid'),
-                       newRecordRow = $searchField.data('recorddata'),
+                       dataStructureIdentifier = $searchField.data('datastructureidentifier'),
+                       hmac = $searchField.data('hmac'),
                        minimumCharacters = $searchField.data('minchars'),
                        url = TYPO3.settings.ajaxUrls['record_suggest'],
                        params = {
@@ -30,7 +31,8 @@ define(['jquery', 'jquery/autocomplete'], function ($) {
                                'field': field,
                                'uid': uid,
                                'pid': pid,
-                               'newRecordRow': newRecordRow
+                               'dataStructureIdentifier': dataStructureIdentifier,
+                               'hmac': hmac
                        },
                        insertValue = function(element) {
                                var insertData = '';
index e613795..0f48598 100644 (file)
@@ -111,6 +111,8 @@ class TcaFlexPrepareTest extends UnitTestCase
         $GLOBALS['TCA']['aTableName']['columns'] = $input['processedTca']['columns'];
 
         $expected = $input;
+        $expected['processedTca']['columns']['aField']['config']['dataStructureIdentifier']
+            = '{"type":"tca","tableName":"aTableName","fieldName":"aField","dataStructureKey":"default"}';
         $expected['processedTca']['columns']['aField']['config']['ds'] = [
             'sheets' => [
                 'sDEF' => [
@@ -187,6 +189,8 @@ class TcaFlexPrepareTest extends UnitTestCase
         $GLOBALS['TCA']['aTableName']['columns'] = $input['processedTca']['columns'];
 
         $expected = $input;
+        $expected['processedTca']['columns']['aField']['config']['dataStructureIdentifier']
+            = '{"type":"tca","tableName":"aTableName","fieldName":"aField","dataStructureKey":"default"}';
         $expected['processedTca']['columns']['aField']['config']['ds'] = [
             'sheets' => [
                 'sDEF' => [
@@ -240,6 +244,8 @@ class TcaFlexPrepareTest extends UnitTestCase
         $GLOBALS['TCA']['aTableName']['columns'] = $input['processedTca']['columns'];
 
         $expected = $input;
+        $expected['processedTca']['columns']['aField']['config']['dataStructureIdentifier']
+            = '{"type":"tca","tableName":"aTableName","fieldName":"aField","dataStructureKey":"default"}';
         $expected['processedTca']['columns']['aField']['config']['ds'] = [
             'ROOT' => '',
             'meta' => [],
@@ -318,6 +324,8 @@ class TcaFlexPrepareTest extends UnitTestCase
         $GLOBALS['TCA']['aTableName']['columns'] = $input['processedTca']['columns'];
 
         $expected = $input;
+        $expected['processedTca']['columns']['aField']['config']['dataStructureIdentifier']
+            = '{"type":"tca","tableName":"aTableName","fieldName":"aField","dataStructureKey":"default"}';
         $expected['processedTca']['columns']['aField']['config']['ds'] = [
             'sheets' => [
                 'sDEF' => [
@@ -422,6 +430,8 @@ class TcaFlexPrepareTest extends UnitTestCase
         $GLOBALS['TCA']['aTableName']['columns'] = $input['processedTca']['columns'];
 
         $expected = $input;
+        $expected['processedTca']['columns']['aField']['config']['dataStructureIdentifier']
+            = '{"type":"tca","tableName":"aTableName","fieldName":"aField","dataStructureKey":"default"}';
         $expected['processedTca']['columns']['aField']['config']['ds'] = [
             'sheets' => [
                 'sDEF' => [
index adc8362..2d0a79b 100644 (file)
@@ -14,9 +14,12 @@ namespace TYPO3\CMS\Backend\Tests\Unit\Form\Wizard;
  * The TYPO3 project - inspiring people to share!
  */
 
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\ServerRequestInterface;
 use TYPO3\CMS\Backend\Form\Wizard\SuggestWizard;
 use TYPO3\CMS\Core\Tests\AccessibleObjectInterface;
 use TYPO3\CMS\Core\Tests\UnitTestCase;
+use TYPO3\CMS\Fluid\View\StandaloneView;
 
 /**
  * Test case
@@ -26,6 +29,73 @@ class SuggestWizardTest extends UnitTestCase
     /**
      * @test
      */
+    public function renderSuggestSelectorThrowsExceptionIfFlexFieldDoesNotContainDataStructureIdentifier()
+    {
+        $viewProphecy = $this->prophesize(StandaloneView::class);
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionCode(1478604742);
+        (new SuggestWizard($viewProphecy->reveal()))->renderSuggestSelector(
+            'aFieldName',
+            'aTable',
+            'aField',
+            ['uid' => 42],
+            [],
+            [
+                'config' => [
+                        'type' => 'flex',
+                        // there should be a 'dataStructureIdentifier' here
+                ],
+            ]
+        );
+    }
+
+    /**
+     * @test
+     */
+    public function searchActionThrowsExceptionWithMissingArgument()
+    {
+        $viewProphecy = $this->prophesize(StandaloneView::class);
+        $responseProphecy = $this->prophesize(ResponseInterface::class);
+        $serverRequestProphecy = $this->prophesize(ServerRequestInterface::class);
+        $serverRequestProphecy->getParsedBody()->willReturn([
+            'value' => 'theSearchValue',
+            'table' => 'aTable',
+            'field' => 'aField',
+            'uid' => 'aUid',
+            'dataStructureIdentifier' => 'anIdentifier',
+            // hmac missing
+        ]);
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionCode(1478607036);
+        (new SuggestWizard($viewProphecy->reveal()))
+            ->searchAction($serverRequestProphecy->reveal(), $responseProphecy->reveal());
+    }
+
+    /**
+     * @test
+     */
+    public function searchActionThrowsExceptionWithWrongHmac()
+    {
+        $viewProphecy = $this->prophesize(StandaloneView::class);
+        $responseProphecy = $this->prophesize(ResponseInterface::class);
+        $serverRequestProphecy = $this->prophesize(ServerRequestInterface::class);
+        $serverRequestProphecy->getParsedBody()->willReturn([
+            'value' => 'theSearchValue',
+            'table' => 'aTable',
+            'field' => 'aField',
+            'uid' => 'aUid',
+            'dataStructureIdentifier' => 'anIdentifier',
+            'hmac' => 'wrongHmac'
+        ]);
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionCode(1478608245);
+        (new SuggestWizard($viewProphecy->reveal()))
+            ->searchAction($serverRequestProphecy->reveal(), $responseProphecy->reveal());
+    }
+
+    /**
+     * @test
+     */
     public function getFieldConfigurationFetchesConfigurationDependentOnTheFullPathToField()
     {
         $config = [
index afba26c..688260d 100644 (file)
@@ -732,7 +732,7 @@ class GeneralUtility
      * Returns a proper HMAC on a given input string and secret TYPO3 encryption key.
      *
      * @param string $input Input string to create HMAC from
-     * @param string $additionalSecret additionalSecret to prevent hmac beeing used in a different context
+     * @param string $additionalSecret additionalSecret to prevent hmac being used in a different context
      * @return string resulting (hexadecimal) HMAC currently with a length of 40 (HMAC-SHA-1)
      */
     public static function hmac($input, $additionalSecret = '')