MAGETWO-99796: [Backport for 2.1.x]User roles not correctly enforced for Stores->Settings->Configuration. Can change any configuration option with config_path

Author: OlgaVasyltsun

app/code/Magento/Config/Controller/Adminhtml/System/AbstractConfig.php

@@ -68,7 +68,7 @@ protected function _isAllowed()
     {
         $sectionId = $this->_request->getParam('section');
         return parent::_isAllowed()
-            && $this->_configStructure->getElement($sectionId)->isAllowed();
+            || $this->_configStructure->getElement($sectionId)->isAllowed();
     }
 
     /**

app/code/Magento/Config/Controller/Adminhtml/System/Config/Save.php

@@ -55,6 +55,68 @@ public function __construct(
         $this->string = $string;
     }
 
+    /**
+     * @inheritdoc
+     */
+    protected function _isAllowed()
+    {
+        return parent::_isAllowed() && $this->isSectionAllowed();
+    }
+
+    /**
+     * Checks if user has access to section.
+     *
+     * @return bool
+     */
+    private function isSectionAllowed()
+    {
+        $sectionId = $this->_request->getParam('section');
+        $isAllowed = $this->_configStructure->getElement($sectionId)->isAllowed();
+        if (!$isAllowed) {
+            $groups = $this->getRequest()->getPost('groups');
+            $fieldPath = $this->getFirstFieldPath($groups, $sectionId);
+
+            $fieldPaths = $this->_configStructure->getFieldPaths();
+            $fieldPath = !empty($fieldPaths[$fieldPath][0]) ? $fieldPaths[$fieldPath][0] : $sectionId;
+            $explodedConfigPath = explode('/', $fieldPath);
+            $configSectionId = !empty($explodedConfigPath[0]) ? $explodedConfigPath[0] : $sectionId;
+
+            $isAllowed = $this->_configStructure->getElement($configSectionId)->isAllowed();
+        }
+
+        return $isAllowed;
+    }
+
+    /**
+     * Return field path as string.
+     *
+     * @param array $elements
+     * @param string $fieldPath
+     * @return string
+     */
+    private function getFirstFieldPath($elements, $fieldPath)
+    {
+        $groupData = [];
+        foreach ($elements as $elementName => $element) {
+            if (!empty($element)) {
+                $fieldPath .= '/' . $elementName;
+
+                if (!empty($element['fields'])) {
+                    $groupData = $element['fields'];
+                } elseif (!empty($element['groups'])) {
+                    $groupData = $element['groups'];
+                }
+
+                if (!empty($groupData)) {
+                    $fieldPath = $this->getFirstFieldPath($groupData, $fieldPath);
+                }
+                break;
+            }
+        }
+
+        return $fieldPath;
+    }
+
     /**
      * Get groups for save
      *

dev/tests/integration/testsuite/Magento/Paypal/Controller/Adminhtml/System/ConfigTest.php

@@ -0,0 +1,80 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+namespace Magento\Paypal\Controller\Adminhtml\System;
+
+use Magento\Framework\App\Config\ScopeConfigInterface;
+use Magento\Framework\App\Request\Http as HttpRequest;
+use Magento\TestFramework\Helper\Bootstrap;
+
+/**
+ * @magentoAppArea adminhtml
+ */
+class ConfigTest extends \Magento\TestFramework\TestCase\AbstractBackendController
+{
+    /**
+     * @magentoAppIsolation enabled
+     * @magentoDbIsolation enabled
+     *
+     * @dataProvider saveMerchantCountryDataProvider
+     *
+     * @param string $section
+     * @param array $groups
+     * @return void
+     */
+    public function testSaveMerchantCountry($section, $groups)
+    {
+        /** @var ScopeConfigInterface $scopeConfig */
+        $scopeConfig = Bootstrap::getObjectManager()->get(ScopeConfigInterface::class);
+
+        $request = $this->getRequest();
+        $request->setPostValue($groups)
+            ->setParam('section', $section)
+            ->setMethod(HttpRequest::METHOD_POST);
+
+        $this->dispatch('backend/admin/system_config/save');
+
+        $this->assertSessionMessages($this->equalTo(['You saved the configuration.']));
+
+        $this->assertEquals(
+            'GB',
+            $scopeConfig->getValue('paypal/general/merchant_country')
+        );
+    }
+
+    /**
+     * @return array
+     */
+    public function saveMerchantCountryDataProvider()
+    {
+        return [
+            [
+                'section' => 'paypal',
+                'groups' => [
+                    'groups' => [
+                        'general' => [
+                            'fields' => [
+                                'merchant_country' => ['value' => 'GB'],
+                            ],
+                        ],
+                    ],
+                ],
+            ],
+            [
+                'section' => 'payment',
+                'groups' => [
+                    'groups' => [
+                        'account' => [
+                            'fields' => [
+                                'merchant_country' => ['value' => 'GB'],
+                            ],
+                        ],
+                    ],
+                ],
+            ],
+        ];
+    }
+}