.../modules/page_cache/src/Tests/PageCacheTest.php | 4 -- core/modules/rest/config/install/rest.settings.yml | 8 +++ core/modules/rest/config/schema/rest.schema.yml | 3 ++ core/modules/rest/rest.install | 20 +++++++ core/modules/rest/src/Plugin/ResourceBase.php | 47 +++++++++++++--- core/modules/rest/src/Plugin/ResourceInterface.php | 4 ++ .../src/Plugin/rest/resource/EntityResource.php | 60 +++++++++++++++++++++ core/modules/rest/src/RequestHandler.php | 10 ++++ core/modules/rest/src/Tests/AuthTest.php | 1 - core/modules/rest/src/Tests/CreateTest.php | 10 ++-- core/modules/rest/src/Tests/CsrfTest.php | 1 - core/modules/rest/src/Tests/DeleteTest.php | 1 - core/modules/rest/src/Tests/NodeTest.php | 1 - core/modules/rest/src/Tests/PageCacheTest.php | 4 -- core/modules/rest/src/Tests/ReadTest.php | 8 --- .../Update/EntityResourcePermissionsUpdateTest.php | 56 +++++++++++++++++++ core/modules/rest/src/Tests/UpdateTest.php | 3 -- .../update/drupal-8.rest-rest_update_8201.php | 63 ++++++++++++++++++++++ .../src/Tests/System/ResponseGeneratorTest.php | 1 - 19 files changed, 269 insertions(+), 36 deletions(-) diff --git a/core/modules/page_cache/src/Tests/PageCacheTest.php b/core/modules/page_cache/src/Tests/PageCacheTest.php index 9194e65..79b69f3 100644 --- a/core/modules/page_cache/src/Tests/PageCacheTest.php +++ b/core/modules/page_cache/src/Tests/PageCacheTest.php @@ -136,10 +136,6 @@ function testQueryParameterFormatRequests() { $node = $this->drupalCreateNode(['type' => 'article']); $node_uri = $node->urlInfo(); $node_url_with_hal_json_format = $node->urlInfo('canonical')->setRouteParameter('_format', 'hal_json'); - /** @var \Drupal\user\RoleInterface $role */ - $role = Role::load('anonymous'); - $role->grantPermission('restful get entity:node'); - $role->save(); $this->drupalGet($node_uri); $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS'); diff --git a/core/modules/rest/config/install/rest.settings.yml b/core/modules/rest/config/install/rest.settings.yml index d3aa82b..5090f9f 100644 --- a/core/modules/rest/config/install/rest.settings.yml +++ b/core/modules/rest/config/install/rest.settings.yml @@ -48,3 +48,11 @@ resources: # Set the domain for REST type and relation links. # If left blank, the site's domain will be used. link_domain: ~ + +# Before Drupal 8.2, EntityResource used permissions as well as the entity +# access system for access checking. This was confusing, and it only did this +# for historical reasons. New Drupal installations opt out from this by default +# (hence this is set to false), existing installations opt in to it. +# @see rest_update_8201() +# @see https://www.drupal.org/node/2664780 +bc_entity_resource_permissions: false diff --git a/core/modules/rest/config/schema/rest.schema.yml b/core/modules/rest/config/schema/rest.schema.yml index 8014b31..fbeb127 100644 --- a/core/modules/rest/config/schema/rest.schema.yml +++ b/core/modules/rest/config/schema/rest.schema.yml @@ -13,6 +13,9 @@ rest.settings: link_domain: type: string label: 'Domain of the relation' + bc_entity_resource_permissions: + type: boolean + label: 'Whether the pre Drupal 8.2.x behavior of having permissions for EntityResource is enabled or not.' rest_resource: type: mapping diff --git a/core/modules/rest/rest.install b/core/modules/rest/rest.install index 4bca69b..9a08e34 100644 --- a/core/modules/rest/rest.install +++ b/core/modules/rest/rest.install @@ -21,3 +21,23 @@ function rest_requirements($phase) { } return $requirements; } + +/** + * @defgroup updates-8.1.x-to-8.2.x Updates from 8.1.x to 8.2.x + * @{ + * Update functions from 8.1.x to 8.2.x. + */ + +/** + * Enable BC for EntityResource: continue to use permissions. + */ +function rest_update_8201() { + $config_factory = \Drupal::configFactory(); + $rest_settings = $config_factory->getEditable('rest.settings'); + $rest_settings->set('bc_entity_resource_permissions', TRUE) + ->save(TRUE); +} + +/** + * @} End of "defgroup updates-8.1.x-to-8.2.x". + */ diff --git a/core/modules/rest/src/Plugin/ResourceBase.php b/core/modules/rest/src/Plugin/ResourceBase.php index 33cb3aa..176a857 100644 --- a/core/modules/rest/src/Plugin/ResourceBase.php +++ b/core/modules/rest/src/Plugin/ResourceBase.php @@ -12,6 +12,11 @@ /** * Common base class for resource plugins. * + * Note that this base class' implementation of the permissions() method + * generates a permission for every method for a resource. If your resource + * already has its own access control mechanism, you should opt out from this + * default permissions() method by overriding it. + * * @see \Drupal\rest\Annotation\RestResource * @see \Drupal\rest\Plugin\Type\ResourcePluginManager * @see \Drupal\rest\Plugin\ResourceInterface @@ -179,7 +184,7 @@ public function availableMethods() { } /** - * Setups the base route for all HTTP methods. + * Gets the base route for a particular method. * * @param string $canonical_path * The canonical path for the resource. @@ -190,22 +195,50 @@ public function availableMethods() { * The created base route. */ protected function getBaseRoute($canonical_path, $method) { - $lower_method = strtolower($method); - - $route = new Route($canonical_path, array( + return new Route($canonical_path, array( '_controller' => 'Drupal\rest\RequestHandler::handle', // Pass the resource plugin ID along as default property. '_plugin' => $this->pluginId, - ), array( - '_permission' => "restful $lower_method $this->pluginId", ), + $this->getBaseRouteRequirements($method), array(), '', array(), // The HTTP method is a requirement for this route. array($method) ); - return $route; + } + + /** + * Gets the base route requirements for a particular method. + * + * @param $method + * The HTTP method to be used for the route. + * + * @return array + * An array of requirements for parameters. + */ + protected function getBaseRouteRequirements($method) { + $lower_method = strtolower($method); + // Every route MUST have requirements that result in the access manager + // having access checks to check. If it does not, the route is made + // inaccessible. So, we default to granting access to everyone. If a + // permission exists, then we add that below. The access manager requires + // that ALL access checks must grant access, so this still results in + // correct behavior. + $requirements = [ + '_access' => 'TRUE', + ]; + + // Only specify route requirements if the default permission exists. For any + // more advanced route definition, resource plugins extending this base + // class must override this method. + $permission = "restful $lower_method $this->pluginId"; + if (isset($this->permissions()[$permission])) { + $requirements['_permission'] = $permission; + } + + return $requirements; } } diff --git a/core/modules/rest/src/Plugin/ResourceInterface.php b/core/modules/rest/src/Plugin/ResourceInterface.php index 0bc2bfb..7e92c57 100644 --- a/core/modules/rest/src/Plugin/ResourceInterface.php +++ b/core/modules/rest/src/Plugin/ResourceInterface.php @@ -33,6 +33,10 @@ public function routes(); * A resource plugin can define a set of user permissions that are used on the * routes for this resource or for other purposes. * + * It is not required for a resource plugin to specify permissions: if they + * have their own access control mechanism, they can use that, and return the + * empty array. + * * @return array * The permission array. */ diff --git a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php index 63857e4..56fd298 100644 --- a/core/modules/rest/src/Plugin/rest/resource/EntityResource.php +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php @@ -2,11 +2,14 @@ namespace Drupal\rest\Plugin\rest\resource; +use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityStorageException; use Drupal\rest\Plugin\ResourceBase; use Drupal\rest\ResourceResponse; use Drupal\rest\ModifiedResourceResponse; +use Psr\Log\LoggerInterface; +use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; use Symfony\Component\HttpKernel\Exception\HttpException; @@ -30,6 +33,48 @@ class EntityResource extends ResourceBase { /** + * The config factory. + * + * @var \Drupal\Core\Config\ConfigFactoryInterface + */ + protected $configFactory; + + /** + * Constructs an EntityResource object. + * + * @param array $configuration + * A configuration array containing information about the plugin instance. + * @param string $plugin_id + * The plugin_id for the plugin instance. + * @param mixed $plugin_definition + * The plugin implementation definition. + * @param array $serializer_formats + * The available serialization formats. + * @param \Psr\Log\LoggerInterface $logger + * A logger instance. + * @param \Drupal\Core\Config\ConfigFactoryInterface + * The config factory. + */ + public function __construct(array $configuration, $plugin_id, $plugin_definition, array $serializer_formats, LoggerInterface $logger, ConfigFactoryInterface $config_factory) { + parent::__construct($configuration, $plugin_id, $plugin_definition, $serializer_formats, $logger); + $this->configFactory = $config_factory; + } + + /** + * {@inheritdoc} + */ + public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) { + return new static( + $configuration, + $plugin_id, + $plugin_definition, + $container->getParameter('serializer.formats'), + $container->get('logger.factory')->get('rest'), + $container->get('config.factory') + ); + } + + /** * Responds to entity GET requests. * * @param \Drupal\Core\Entity\EntityInterface $entity @@ -245,6 +290,21 @@ protected function validate(EntityInterface $entity) { /** * {@inheritdoc} */ + public function permissions() { + // @see https://www.drupal.org/node/2664780 + if ($this->configFactory->get('rest.settings')->get('bc_entity_resource_permissions')) { + // The default Drupal 8.0.x and 8.1.x behavior. + return parent::permissions(); + } + else { + // The default Drupal 8.2.x behavior. + return []; + } + } + + /** + * {@inheritdoc} + */ protected function getBaseRoute($canonical_path, $method) { $route = parent::getBaseRoute($canonical_path, $method); $definition = $this->getPluginDefinition(); diff --git a/core/modules/rest/src/RequestHandler.php b/core/modules/rest/src/RequestHandler.php index 2aa3673..0c0d05b 100644 --- a/core/modules/rest/src/RequestHandler.php +++ b/core/modules/rest/src/RequestHandler.php @@ -91,6 +91,16 @@ public function handle(RouteMatchInterface $route_match, Request $request) { $response = call_user_func_array(array($resource, $method), array_merge($parameters, array($unserialized, $request))); } catch (HttpException $e) { + // In case a response is forbidden for the current user, we must not + // convert it into a response, because that would rob + // \Drupal\Core\EventSubscriber\AuthenticationSubscriber::onExceptionSendChallenge() + // from the opportunity to convert it into a 401 response, to challenge + // the user to authenticate. + // @see \Drupal\Core\Authentication\AuthenticationProviderChallengeInterface + if ($e->getStatusCode() == 403) { + throw $e; + } + $error['error'] = $e->getMessage(); $content = $serializer->serialize($error, $format); // Add the default content type, but only if the headers from the diff --git a/core/modules/rest/src/Tests/AuthTest.php b/core/modules/rest/src/Tests/AuthTest.php index 08db9ab..89838df 100644 --- a/core/modules/rest/src/Tests/AuthTest.php +++ b/core/modules/rest/src/Tests/AuthTest.php @@ -43,7 +43,6 @@ public function testRead() { // resources via the REST API, but the request is authenticated // with session cookies. $permissions = $this->entityPermissions($entity_type, 'view'); - $permissions[] = 'restful get entity:' . $entity_type; $account = $this->drupalCreateUser($permissions); $this->drupalLogin($account); diff --git a/core/modules/rest/src/Tests/CreateTest.php b/core/modules/rest/src/Tests/CreateTest.php index 035480b..60ff6af 100644 --- a/core/modules/rest/src/Tests/CreateTest.php +++ b/core/modules/rest/src/Tests/CreateTest.php @@ -49,8 +49,6 @@ public function testCreateResourceRestApiNotEnabled() { // Get the necessary user permissions to create the current entity type. $permissions = $this->entityPermissions($entity_type, 'create'); - // POST method must be allowed for the current entity type. - $permissions[] = 'restful post entity:' . $entity_type; // Create the user. $account = $this->drupalCreateUser($permissions); @@ -77,7 +75,11 @@ public function testCreateResourceRestApiNotEnabled() { /** * Ensure that an entity cannot be created without the restful permission. */ - public function testCreateWithoutPermission() { + public function testCreateWithoutPermissionIfBcFlagIsOn() { + $rest_settings = $this->config('rest.settings'); + $rest_settings->set('bc_entity_resource_permissions', TRUE) + ->save(TRUE); + $entity_type = 'entity_test'; // Enables the REST service for 'entity_test' entity type. $this->enableService('entity:' . $entity_type, 'POST'); @@ -331,8 +333,6 @@ public function createAccountPerEntity($entity_type) { $accounts = array(); // Get the necessary user permissions for the current $entity_type creation. $permissions = $this->entityPermissions($entity_type, 'create'); - // POST method must be allowed for the current entity type. - $permissions[] = 'restful post entity:' . $entity_type; // Create user without administrative permissions. $accounts[] = $this->drupalCreateUser($permissions); // Add administrative permissions for nodes and users. diff --git a/core/modules/rest/src/Tests/CsrfTest.php b/core/modules/rest/src/Tests/CsrfTest.php index f98abad..26121a4 100644 --- a/core/modules/rest/src/Tests/CsrfTest.php +++ b/core/modules/rest/src/Tests/CsrfTest.php @@ -43,7 +43,6 @@ protected function setUp() { // Create a user account that has the required permissions to create // resources via the REST API. $permissions = $this->entityPermissions($this->testEntityType, 'create'); - $permissions[] = 'restful post entity:' . $this->testEntityType; $this->account = $this->drupalCreateUser($permissions); // Serialize an entity to a string to use in the content body of the POST diff --git a/core/modules/rest/src/Tests/DeleteTest.php b/core/modules/rest/src/Tests/DeleteTest.php index ccba38e..50a427b 100644 --- a/core/modules/rest/src/Tests/DeleteTest.php +++ b/core/modules/rest/src/Tests/DeleteTest.php @@ -31,7 +31,6 @@ public function testDelete() { // Create a user account that has the required permissions to delete // resources via the REST API. $permissions = $this->entityPermissions($entity_type, 'delete'); - $permissions[] = 'restful delete entity:' . $entity_type; $account = $this->drupalCreateUser($permissions); $this->drupalLogin($account); diff --git a/core/modules/rest/src/Tests/NodeTest.php b/core/modules/rest/src/Tests/NodeTest.php index 7f0ed81..a560d7e 100644 --- a/core/modules/rest/src/Tests/NodeTest.php +++ b/core/modules/rest/src/Tests/NodeTest.php @@ -32,7 +32,6 @@ class NodeTest extends RESTTestBase { protected function enableNodeConfiguration($method, $operation) { $this->enableService('entity:node', $method); $permissions = $this->entityPermissions('node', $operation); - $permissions[] = 'restful ' . strtolower($method) . ' entity:node'; $account = $this->drupalCreateUser($permissions); $this->drupalLogin($account); } diff --git a/core/modules/rest/src/Tests/PageCacheTest.php b/core/modules/rest/src/Tests/PageCacheTest.php index 0ce66da..6715b8b 100644 --- a/core/modules/rest/src/Tests/PageCacheTest.php +++ b/core/modules/rest/src/Tests/PageCacheTest.php @@ -47,10 +47,6 @@ public function testConfigChangePageCache() { $this->enableService('entity:entity_test', 'POST'); $permissions = [ 'administer entity_test content', - 'restful post entity:entity_test', - 'restful get entity:entity_test', - 'restful patch entity:entity_test', - 'restful delete entity:entity_test', ]; $account = $this->drupalCreateUser($permissions); diff --git a/core/modules/rest/src/Tests/ReadTest.php b/core/modules/rest/src/Tests/ReadTest.php index 31b5db8..af5e862 100644 --- a/core/modules/rest/src/Tests/ReadTest.php +++ b/core/modules/rest/src/Tests/ReadTest.php @@ -31,7 +31,6 @@ public function testRead() { // Create a user account that has the required permissions to read // resources via the REST API. $permissions = $this->entityPermissions($entity_type, 'view'); - $permissions[] = 'restful get entity:' . $entity_type; $account = $this->drupalCreateUser($permissions); $this->drupalLogin($account); @@ -71,12 +70,6 @@ public function testRead() { $data = Json::decode($response); $this->assertFalse(isset($data['field_test_text']), 'Field access protected field is not visible in the response.'); } - - // Try to read an entity without proper permissions. - $this->drupalLogout(); - $response = $this->httpRequest($entity->urlInfo()->setRouteParameter('_format', $this->defaultFormat), 'GET'); - $this->assertResponse(403); - $this->assertIdentical('{"message":""}', $response); } // Try to read a resource which is not REST API enabled. $account = $this->drupalCreateUser(); @@ -102,7 +95,6 @@ public function testResourceStructure() { // Create a user account that has the required permissions to read // resources via the REST API. $permissions = $this->entityPermissions('node', 'view'); - $permissions[] = 'restful get entity:node'; $account = $this->drupalCreateUser($permissions); $this->drupalLogin($account); diff --git a/core/modules/rest/src/Tests/Update/EntityResourcePermissionsUpdateTest.php b/core/modules/rest/src/Tests/Update/EntityResourcePermissionsUpdateTest.php new file mode 100644 index 0000000..ff20c1e --- /dev/null +++ b/core/modules/rest/src/Tests/Update/EntityResourcePermissionsUpdateTest.php @@ -0,0 +1,56 @@ +databaseDumpFiles = [ + __DIR__ . '/../../../../system/tests/fixtures/update/drupal-8.bare.standard.php.gz', + __DIR__ . '/../../../../rest/tests/fixtures/update/drupal-8.rest-rest_update_8201.php', + ]; + } + + /** + * Tests rest_update_8201(). + */ + public function testAllowedHtmlUpdate() { + $permission_handler = $this->container->get('user.permissions'); + + // Make sure we have the expected values before the update. + $rest_settings = $this->config('rest.settings'); + $this->assertFalse(array_key_exists('bc_entity_resource_permissions', $rest_settings->getRawData())); + $this->assertFalse($permission_handler->moduleProvidesPermissions('rest')); + + $this->runUpdates(); + + // Make sure we have the expected values after the update. + $rest_settings = $this->config('rest.settings'); + $this->assertTrue(array_key_exists('bc_entity_resource_permissions', $rest_settings->getRawData())); + $this->assertTrue($rest_settings->get('bc_entity_resource_permissions')); + $this->assertTrue($permission_handler->moduleProvidesPermissions('rest')); + $permissions = $permission_handler->getPermissions(); + $rest_permissions = array_keys(array_filter($permissions, function ($permission) { + return $permission['provider'] === 'rest'; + })); + $this->assertEqual(['restful delete entity:node', 'restful get entity:node', 'restful patch entity:node', 'restful post entity:node'], $rest_permissions); + } + +} diff --git a/core/modules/rest/src/Tests/UpdateTest.php b/core/modules/rest/src/Tests/UpdateTest.php index 98df739..01eb194 100644 --- a/core/modules/rest/src/Tests/UpdateTest.php +++ b/core/modules/rest/src/Tests/UpdateTest.php @@ -45,7 +45,6 @@ public function testPatchUpdate() { // Create a user account that has the required permissions to create // resources via the REST API. $permissions = $this->entityPermissions($entity_type, 'update'); - $permissions[] = 'restful patch entity:' . $entity_type; $account = $this->drupalCreateUser($permissions); $this->drupalLogin($account); @@ -183,7 +182,6 @@ public function testUpdateUser() { // Enables the REST service for 'user' entity type. $this->enableService('entity:' . $entity_type, 'PATCH'); $permissions = $this->entityPermissions($entity_type, 'update'); - $permissions[] = 'restful patch entity:' . $entity_type; $account = $this->drupalCreateUser($permissions); $account->set('mail', 'old-email@example.com'); $this->drupalLogin($account); @@ -245,7 +243,6 @@ public function testUpdateComment() { // Enables the REST service for 'comment' entity type. $this->enableService('entity:' . $entity_type, 'PATCH', ['hal_json', 'json']); $permissions = $this->entityPermissions($entity_type, 'update'); - $permissions[] = 'restful patch entity:' . $entity_type; $account = $this->drupalCreateUser($permissions); $account->set('mail', 'old-email@example.com'); $this->drupalLogin($account); diff --git a/core/modules/rest/tests/fixtures/update/drupal-8.rest-rest_update_8201.php b/core/modules/rest/tests/fixtures/update/drupal-8.rest-rest_update_8201.php new file mode 100644 index 0000000..f035e94 --- /dev/null +++ b/core/modules/rest/tests/fixtures/update/drupal-8.rest-rest_update_8201.php @@ -0,0 +1,63 @@ +insert('key_value') + ->fields([ + 'collection' => 'system.schema', + 'name' => 'rest', + 'value' => 'i:8000;', + ]) + ->fields([ + 'collection' => 'system.schema', + 'name' => 'serialization', + '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'] = 8000; +$extensions['module']['serialization'] = 8000; +$connection->update('config') + ->fields([ + 'data' => serialize($extensions), + ]) + ->condition('collection', '') + ->condition('name', 'core.extension') + ->execute(); + +// Install the rest configuration. +$config = [ + 'resources' => [ + 'entity:node' => [ + 'GET' => [ + 'supported_formats' => ['json'], + 'supported_auth' => [], + ], + ], + ], + 'link_domain' => '~', +]; +$data = $connection->insert('config') + ->fields([ + 'name' => 'rest.settings', + 'data' => serialize($config), + 'collection' => '' + ]) + ->execute(); diff --git a/core/modules/system/src/Tests/System/ResponseGeneratorTest.php b/core/modules/system/src/Tests/System/ResponseGeneratorTest.php index 9d6cf4f..09c238d 100644 --- a/core/modules/system/src/Tests/System/ResponseGeneratorTest.php +++ b/core/modules/system/src/Tests/System/ResponseGeneratorTest.php @@ -26,7 +26,6 @@ protected function setUp() { $this->drupalCreateContentType(array('type' => 'page', 'name' => 'Basic page')); $permissions = $this->entityPermissions('node', 'view'); - $permissions[] = 'restful get entity:node'; $account = $this->drupalCreateUser($permissions); $this->drupalLogin($account); }