From 850f03b9be840cdd3bb00d6868f1232689d2c4ff Mon Sep 17 00:00:00 2001
From: xjm <xjm@65776.no-reply.drupal.org>
Date: Tue, 24 Oct 2017 12:14:02 -0500
Subject: [PATCH] Issue #2825204 by dawehner, BR0kEN, xjm, pcambra, Wim Leers,
tim.plunkett, tstoeckler, damiankloip, larowlan, effulgentsia, alexpott: REST
views: authentication is broken
---
core/modules/rest/rest.install | 36 +++
core/modules/rest/rest.module | 28 +++
.../src/Plugin/views/display/RestExport.php | 39 +++-
.../RestExportAuthCorrectionUpdateTest.php | 36 +++
...-export-with-authentication-correction.php | 63 ++++++
.../Functional/Views/RestExportAuthTest.php | 67 ++++++
.../src/Kernel/Views/RestExportAuthTest.php | 59 +++++
...t_export_with_authorization_correction.yml | 213 ++++++++++++++++++
8 files changed, 535 insertions(+), 6 deletions(-)
create mode 100644 core/modules/rest/src/Tests/Update/RestExportAuthCorrectionUpdateTest.php
create mode 100644 core/modules/rest/tests/fixtures/update/rest-export-with-authentication-correction.php
create mode 100644 core/modules/rest/tests/src/Functional/Views/RestExportAuthTest.php
create mode 100644 core/modules/rest/tests/src/Kernel/Views/RestExportAuthTest.php
create mode 100644 core/modules/views/tests/modules/views_test_config/test_views/views.view.rest_export_with_authorization_correction.yml
diff --git a/core/modules/rest/rest.install b/core/modules/rest/rest.install
index 32c5b1d27023..2113b0bbc3bd 100644
--- a/core/modules/rest/rest.install
+++ b/core/modules/rest/rest.install
@@ -84,3 +84,39 @@ function rest_update_8203() {
$rest_settings->set('bc_entity_resource_permissions', TRUE)
->save(TRUE);
}
+
+/**
+ * Ensure the right REST authentication method is used.
+ *
+ * This fixes the bug in https://www.drupal.org/node/2825204.
+ */
+function rest_update_8401() {
+ $config_factory = \Drupal::configFactory();
+ $auth_providers = \Drupal::service('authentication_collector')->getSortedProviders();
+ $process_auth = function ($auth_option) use ($auth_providers) {
+ foreach ($auth_providers as $provider_id => $provider_data) {
+ // The provider belongs to the module that declares it as a service.
+ if (strtok($provider_data->_serviceId, '.') === $auth_option) {
+ return $provider_id;
+ }
+ }
+
+ return $auth_option;
+ };
+
+ foreach ($config_factory->listAll('views.view.') as $view_config_name) {
+ $save = FALSE;
+ $view = $config_factory->getEditable($view_config_name);
+ $displays = $view->get('display');
+ foreach ($displays as $display_name => $display) {
+ if ('rest_export' === $display['display_plugin'] && !empty($display['display_options']['auth'])) {
+ $displays[$display_name]['display_options']['auth'] = array_map($process_auth, $display['display_options']['auth']);
+ $save = TRUE;
+ }
+ }
+ if ($save) {
+ $view->set('display', $displays);
+ $view->save(TRUE);
+ }
+ }
+}
diff --git a/core/modules/rest/rest.module b/core/modules/rest/rest.module
index edd74b9985e7..ba61a17feabd 100644
--- a/core/modules/rest/rest.module
+++ b/core/modules/rest/rest.module
@@ -6,6 +6,7 @@
*/
use Drupal\Core\Routing\RouteMatchInterface;
+use Drupal\views\ViewEntityInterface;
/**
* Implements hook_help().
@@ -27,3 +28,30 @@ function rest_help($route_name, RouteMatchInterface $route_match) {
return $output;
}
}
+
+/**
+ * Implements hook_view_presave().
+ *
+ * @see rest_update_8401()
+ */
+function rest_view_presave(ViewEntityInterface $view) {
+ // Fix the auth options on import, much like what rest_update_8401() does.
+ $auth_providers = \Drupal::service('authentication_collector')->getSortedProviders();
+ $process_auth = function ($auth_option) use ($auth_providers) {
+ foreach ($auth_providers as $provider_id => $provider_data) {
+ // The provider belongs to the module that declares it as a service.
+ if (strtok($provider_data->_serviceId, '.') === $auth_option) {
+ return $provider_id;
+ }
+ }
+
+ return $auth_option;
+ };
+
+ foreach (array_keys($view->get('display')) as $display_id) {
+ $display = &$view->getDisplay($display_id);
+ if ($display['display_plugin'] === 'rest_export' && !empty($display['display_options']['auth'])) {
+ $display['display_options']['auth'] = array_map($process_auth, $display['display_options']['auth']);
+ }
+ }
+}
diff --git a/core/modules/rest/src/Plugin/views/display/RestExport.php b/core/modules/rest/src/Plugin/views/display/RestExport.php
index 81e0b7dc5380..52daee05f040 100644
--- a/core/modules/rest/src/Plugin/views/display/RestExport.php
+++ b/core/modules/rest/src/Plugin/views/display/RestExport.php
@@ -87,7 +87,23 @@ class RestExport extends PathPluginBase implements ResponseDisplayPluginInterfac
protected $authenticationCollector;
/**
- * The authentication providers, keyed by ID.
+ * The authentication providers, like 'cookie' and 'basic_auth'.
+ *
+ * @var string[]
+ */
+ protected $authenticationProviderIds;
+
+ /**
+ * The authentication providers' modules, keyed by provider ID.
+ *
+ * Authentication providers like 'cookie' and 'basic_auth' are the array
+ * keys. The array values are the module names, e.g.:
+ * @code
+ * ['cookie' => 'user', 'basic_auth' => 'basic_auth']
+ * @endcode
+ *
+ * @deprecated as of 8.4.x, will be removed in before Drupal 9.0.0, see
+ * https://www.drupal.org/node/2825204.
*
* @var string[]
*/
@@ -124,6 +140,13 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition
parent::__construct($configuration, $plugin_id, $plugin_definition, $route_provider, $state);
$this->renderer = $renderer;
+ // $authentication_providers as defined in
+ // \Drupal\Core\DependencyInjection\Compiler\AuthenticationProviderPass
+ // and as such it is an array, with authentication providers (cookie,
+ // basic_auth) as keys and modules providing those as values (user,
+ // basic_auth).
+ $this->authenticationProviderIds = array_keys($authentication_providers);
+ // For BC reasons we keep around authenticationProviders as before.
$this->authenticationProviders = $authentication_providers;
$this->formatProviders = $serializer_format_providers;
}
@@ -238,7 +261,7 @@ public function getContentType() {
* An array to use as value for "#options" in the form element.
*/
public function getAuthOptions() {
- return array_combine($this->authenticationProviders, $this->authenticationProviders);
+ return array_combine($this->authenticationProviderIds, $this->authenticationProviderIds);
}
/**
@@ -472,10 +495,14 @@ public function calculateDependencies() {
$dependencies = parent::calculateDependencies();
$dependencies += ['module' => []];
- $modules = array_map(function ($authentication_provider) {
- return $this->authenticationProviders[$authentication_provider];
- }, $this->getOption('auth'));
- $dependencies['module'] = array_merge($dependencies['module'], $modules);
+ $dependencies['module'] = array_merge($dependencies['module'], array_filter(array_map(function ($provider) {
+ // During the update path the provider options might be wrong. This can
+ // happen when any update function, like block_update_8300() triggers a
+ // view to be saved.
+ return isset($this->authenticationProviderIds[$provider])
+ ? $this->authenticationProviderIds[$provider]
+ : NULL;
+ }, $this->getOption('auth'))));
return $dependencies;
}
diff --git a/core/modules/rest/src/Tests/Update/RestExportAuthCorrectionUpdateTest.php b/core/modules/rest/src/Tests/Update/RestExportAuthCorrectionUpdateTest.php
new file mode 100644
index 000000000000..aeca71f621de
--- /dev/null
+++ b/core/modules/rest/src/Tests/Update/RestExportAuthCorrectionUpdateTest.php
@@ -0,0 +1,36 @@
+<?php
+
+namespace Drupal\rest\Tests\Update;
+
+use Drupal\system\Tests\Update\UpdatePathTestBase;
+
+/**
+ * Ensures that update hook is run properly for REST Export config.
+ *
+ * @group Update
+ */
+class RestExportAuthCorrectionUpdateTest extends UpdatePathTestBase {
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function setDatabaseDumpFiles() {
+ $this->databaseDumpFiles = [
+ __DIR__ . '/../../../../system/tests/fixtures/update/drupal-8.bare.standard.php.gz',
+ __DIR__ . '/../../../tests/fixtures/update/rest-export-with-authentication-correction.php',
+ ];
+ }
+
+ /**
+ * Ensures that update hook is run for "rest" module.
+ */
+ public function testUpdate() {
+ $this->runUpdates();
+
+ // Get particular view.
+ $view = \Drupal::entityTypeManager()->getStorage('view')->load('rest_export_with_authorization_correction');
+ $displays = $view->get('display');
+ $this->assertIdentical($displays['rest_export_1']['display_options']['auth'], ['cookie'], 'Cookie is used for authentication');
+ }
+
+}
diff --git a/core/modules/rest/tests/fixtures/update/rest-export-with-authentication-correction.php b/core/modules/rest/tests/fixtures/update/rest-export-with-authentication-correction.php
new file mode 100644
index 000000000000..6cbd04e8b03e
--- /dev/null
+++ b/core/modules/rest/tests/fixtures/update/rest-export-with-authentication-correction.php
@@ -0,0 +1,63 @@
+<?php
+
+/**
+ * @file
+ * Test fixture for \Drupal\rest\Tests\Update\RestExportAuthCorrectionUpdateTest.
+ */
+
+use Drupal\Core\Database\Database;
+use Drupal\Core\Serialization\Yaml;
+
+$connection = Database::getConnection();
+
+// Set the schema version.
+$connection->insert('key_value')
+ ->fields([
+ 'collection' => 'system.schema',
+ 'name' => 'rest',
+ 'value' => 'i:8000;',
+ ])
+ ->execute();
+
+// Update core.extension.
+$extensions = $connection->select('config')
+ ->fields('config', ['data'])
+ ->condition('collection', '')
+ ->condition('name', 'core.extension')
+ ->execute()
+ ->fetchField();
+$extensions = unserialize($extensions);
+$extensions['module']['rest'] = 0;
+$extensions['module']['serialization'] = 0;
+$extensions['module']['basic_auth'] = 0;
+$connection->update('config')
+ ->fields([
+ 'data' => serialize($extensions),
+ ])
+ ->condition('collection', '')
+ ->condition('name', 'core.extension')
+ ->execute();
+
+$connection->insert('config')
+ ->fields([
+ 'name' => 'rest.settings',
+ 'data' => serialize([
+ 'link_domain' => '~',
+ ]),
+ 'collection' => '',
+ ])
+ ->execute();
+
+$connection->insert('config')
+ ->fields([
+ 'name' => 'views.view.rest_export_with_authorization_correction',
+ ])
+ ->execute();
+
+$connection->merge('config')
+ ->condition('name', 'views.view.rest_export_with_authorization_correction')
+ ->condition('collection', '')
+ ->fields([
+ 'data' => serialize(Yaml::decode(file_get_contents('core/modules/views/tests/modules/views_test_config/test_views/views.view.rest_export_with_authorization_correction.yml'))),
+ ])
+ ->execute();
diff --git a/core/modules/rest/tests/src/Functional/Views/RestExportAuthTest.php b/core/modules/rest/tests/src/Functional/Views/RestExportAuthTest.php
new file mode 100644
index 000000000000..24546089cd54
--- /dev/null
+++ b/core/modules/rest/tests/src/Functional/Views/RestExportAuthTest.php
@@ -0,0 +1,67 @@
+<?php
+
+namespace Drupal\Tests\rest\Functional\Views;
+
+use Drupal\Tests\views\Functional\ViewTestBase;
+use Drupal\views\Entity\View;
+
+/**
+ * Tests authentication for REST display.
+ *
+ * @group rest
+ */
+class RestExportAuthTest extends ViewTestBase {
+
+ /**
+ * {@inheritdoc}
+ */
+ public static $modules = ['rest', 'views_ui', 'basic_auth'];
+
+ /**
+ * {@inheritdoc}
+ */
+ public function setUp($import_test_views = TRUE) {
+ parent::setUp($import_test_views);
+
+ $this->drupalLogin($this->drupalCreateUser(['administer views']));
+ }
+
+ /**
+ * Checks that correct authentication providers are available for choosing.
+ *
+ * @link https://www.drupal.org/node/2825204
+ */
+ public function testAuthProvidersOptions() {
+ $view_id = 'test_view_rest_export';
+ $view_label = 'Test view (REST export)';
+ $view_display = 'rest_export_1';
+ $view_rest_path = 'test-view/rest-export';
+
+ // Create new view.
+ $this->drupalPostForm('admin/structure/views/add', [
+ 'id' => $view_id,
+ 'label' => $view_label,
+ 'show[wizard_key]' => 'users',
+ 'rest_export[path]' => $view_rest_path,
+ 'rest_export[create]' => TRUE,
+ ], t('Save and edit'));
+
+ $this->drupalGet("admin/structure/views/nojs/display/$view_id/$view_display/auth");
+ // The "basic_auth" will always be available since module,
+ // providing it, has the same name.
+ $this->assertField('edit-auth-basic-auth', 'Basic auth is available for choosing.');
+ // The "cookie" authentication provider defined by "user" module.
+ $this->assertField('edit-auth-cookie', 'Cookie-based auth can be chosen.');
+ // Wrong behavior in "getAuthOptions()" method makes this option available
+ // instead of "cookie".
+ // @see \Drupal\rest\Plugin\views\display\RestExport::getAuthOptions()
+ $this->assertNoField('edit-auth-user', 'Wrong authentication option is unavailable.');
+
+ $this->drupalPostForm(NULL, ['auth[basic_auth]' => 1, 'auth[cookie]' => 1], 'Apply');
+ $this->drupalPostForm(NULL, [], 'Save');
+
+ $view = View::load($view_id);
+ $this->assertEquals(['basic_auth', 'cookie'], $view->getDisplay('rest_export_1')['display_options']['auth']);
+ }
+
+}
diff --git a/core/modules/rest/tests/src/Kernel/Views/RestExportAuthTest.php b/core/modules/rest/tests/src/Kernel/Views/RestExportAuthTest.php
new file mode 100644
index 000000000000..9d32f7fcd12c
--- /dev/null
+++ b/core/modules/rest/tests/src/Kernel/Views/RestExportAuthTest.php
@@ -0,0 +1,59 @@
+<?php
+
+namespace Drupal\Tests\rest\Kernel\Views;
+
+use Drupal\Tests\views\Kernel\ViewsKernelTestBase;
+
+/**
+ * Tests the auth option of rest exports.
+ *
+ * @coversDefaultClass \Drupal\rest\Plugin\views\display\RestExport
+ *
+ * @group rest
+ */
+class RestExportAuthTest extends ViewsKernelTestBase {
+
+ /**
+ * {@inheritdoc}
+ */
+ public static $modules = [
+ 'node',
+ 'rest',
+ 'views_ui',
+ 'basic_auth',
+ 'serialization',
+ 'rest',
+ 'user',
+ ];
+
+ /**
+ * {@inheritdoc}
+ */
+ public static $testViews = ['rest_export_with_authorization_correction'];
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function setUp($import_test_views = TRUE) {
+ parent::setUp($import_test_views);
+
+ $this->installConfig(['user']);
+ }
+
+ /**
+ * Ensures that rest export auth settings are automatically corrected.
+ *
+ * @see rest_update_8401()
+ * @see rest_views_presave()
+ * @see \Drupal\rest\Tests\Update\RestExportAuthCorrectionUpdateTest
+ */
+ public function testAuthCorrection() {
+ // Get particular view.
+ $view = \Drupal::entityTypeManager()
+ ->getStorage('view')
+ ->load('rest_export_with_authorization_correction');
+ $displays = $view->get('display');
+ $this->assertSame($displays['rest_export_1']['display_options']['auth'], ['cookie'], 'Cookie is used for authentication');
+ }
+
+}
diff --git a/core/modules/views/tests/modules/views_test_config/test_views/views.view.rest_export_with_authorization_correction.yml b/core/modules/views/tests/modules/views_test_config/test_views/views.view.rest_export_with_authorization_correction.yml
new file mode 100644
index 000000000000..27130f2f74d0
--- /dev/null
+++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.rest_export_with_authorization_correction.yml
@@ -0,0 +1,213 @@
+langcode: en
+status: true
+dependencies:
+ config:
+ - user.role.authenticated
+ module:
+ - node
+ - rest
+ - user
+id: rest_export_with_authorization_correction
+label: 'Rest Export'
+module: views
+description: ''
+tag: ''
+base_table: node_field_data
+base_field: nid
+core: 8.x
+display:
+ default:
+ display_plugin: default
+ id: default
+ display_title: Master
+ position: 0
+ display_options:
+ access:
+ type: role
+ options:
+ role:
+ authenticated: authenticated
+ cache:
+ type: tag
+ options: { }
+ query:
+ type: views_query
+ options:
+ disable_sql_rewrite: false
+ distinct: false
+ replica: false
+ query_comment: ''
+ query_tags: { }
+ exposed_form:
+ type: basic
+ options:
+ submit_button: Filter
+ reset_button: false
+ reset_button_label: Reset
+ exposed_sorts_label: 'Sort by'
+ expose_sort_order: true
+ sort_asc_label: Asc
+ sort_desc_label: Desc
+ pager:
+ type: full
+ options:
+ items_per_page: 10
+ offset: 0
+ id: 0
+ total_pages: null
+ expose:
+ items_per_page: false
+ items_per_page_label: 'Items per page'
+ items_per_page_options: '5, 10, 25, 50'
+ items_per_page_options_all: false
+ items_per_page_options_all_label: '- All -'
+ offset: false
+ offset_label: Offset
+ tags:
+ previous: '‹ Previous'
+ next: 'Next ›'
+ first: '« First'
+ last: 'Last »'
+ quantity: 9
+ style:
+ type: default
+ row:
+ type: 'fields'
+ fields:
+ title:
+ id: title
+ table: node_field_data
+ field: title
+ entity_type: node
+ entity_field: title
+ label: ''
+ alter:
+ alter_text: false
+ make_link: false
+ absolute: false
+ trim: false
+ word_boundary: false
+ ellipsis: false
+ strip_tags: false
+ html: false
+ hide_empty: false
+ empty_zero: false
+ settings:
+ link_to_entity: true
+ plugin_id: field
+ relationship: none
+ group_type: group
+ admin_label: ''
+ exclude: false
+ element_type: ''
+ element_class: ''
+ element_label_type: ''
+ element_label_class: ''
+ element_label_colon: true
+ element_wrapper_type: ''
+ element_wrapper_class: ''
+ element_default_classes: true
+ empty: ''
+ hide_alter_empty: true
+ click_sort_column: value
+ type: string
+ group_column: value
+ group_columns: { }
+ group_rows: true
+ delta_limit: 0
+ delta_offset: 0
+ delta_reversed: false
+ delta_first_last: false
+ multi_type: separator
+ separator: ', '
+ field_api_classes: false
+ filters:
+ status:
+ id: status
+ table: node_field_data
+ field: status
+ relationship: none
+ group_type: group
+ admin_label: ''
+ operator: '='
+ value: '0'
+ group: 1
+ exposed: false
+ expose:
+ operator_id: ''
+ label: ''
+ description: ''
+ use_operator: false
+ operator: ''
+ identifier: ''
+ required: false
+ remember: false
+ multiple: false
+ remember_roles:
+ authenticated: authenticated
+ is_grouped: false
+ group_info:
+ label: ''
+ description: ''
+ identifier: ''
+ optional: true
+ widget: select
+ multiple: false
+ remember: false
+ default_group: All
+ default_group_multiple: { }
+ group_items: { }
+ plugin_id: boolean
+ entity_type: node
+ entity_field: status
+ sorts:
+ created:
+ id: created
+ table: node_field_data
+ field: created
+ order: DESC
+ entity_type: node
+ entity_field: created
+ plugin_id: date
+ relationship: none
+ group_type: group
+ admin_label: ''
+ exposed: false
+ expose:
+ label: ''
+ granularity: second
+ title: 'Rest Export'
+ header: { }
+ footer: { }
+ empty: { }
+ relationships: { }
+ arguments: { }
+ display_extenders: { }
+ cache_metadata:
+ max-age: -1
+ contexts:
+ - 'languages:language_content'
+ - 'languages:language_interface'
+ - url.query_args
+ - 'user.node_grants:view'
+ - user.roles
+ tags: { }
+ rest_export_1:
+ display_plugin: rest_export
+ id: rest_export_1
+ display_title: 'REST export'
+ position: 2
+ display_options:
+ display_extenders: { }
+ path: unpublished-content
+ auth:
+ - user
+ cache_metadata:
+ max-age: -1
+ contexts:
+ - 'languages:language_content'
+ - 'languages:language_interface'
+ - request_format
+ - 'user.node_grants:view'
+ - user.roles
+ tags: { }
--
GitLab