Issue #2825204 by dawehner, BR0kEN, pcambra, Wim Leers, tim.plunkett,...

Issue #2825204 by dawehner, BR0kEN, pcambra, Wim Leers, tim.plunkett, tstoeckler, damiankloip: REST views: authentication is broken

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);
+    }
+  }
+}

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']);
+    }
+  }
+}

core/modules/rest/src/Plugin/views/display/RestExport.php

@@ -124,7 +124,9 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition
     parent::__construct($configuration, $plugin_id, $plugin_definition, $route_provider, $state);
 
     $this->renderer = $renderer;
-    $this->authenticationProviders = $authentication_providers;
+    // Values of "$this->authenticationProviders" - are module names, defining
+    // authentication providers. Only provider IDs should be used for choosing.
+    $this->authenticationProviders = array_keys($authentication_providers);
     $this->formatProviders = $serializer_format_providers;
   }
 
@@ -472,10 +474,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->authenticationProviders[$provider])
+        ? $this->authenticationProviders[$provider]
+        : NULL;
+    }, $this->getOption('auth'))));
 
     return $dependencies;
   }

core/modules/rest/src/Tests/Update/RestExportAuthCorrectionUpdateTest.php

+<?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');
+  }
+
+}

core/modules/rest/tests/fixtures/update/rest-export-with-authentication-correction.php

+<?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();

core/modules/rest/tests/src/Functional/Views/RestExportAuthTest.php

+<?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']);
+  }
+
+}

core/modules/rest/tests/src/Kernel/Views/RestExportAuthTest.php

+<?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');
+  }
+
+}

core/modules/views/tests/modules/views_test_config/test_views/views.view.rest_export_with_authorization_correction.yml

+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: {  }