summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorcatch2012-08-21 17:19:09 +0200
committercatch2012-08-21 17:19:09 +0200
commite900366cd5827da67244c328820c6d402b2b5243 (patch)
treea35fa4d4b9e69af8e95354224250d615b2f3f621
parent02e4cab661c86821528bded644b973d2857da54b (diff)
Issue #1245220 by Berdir, plach, dpolant, xjm, WorldFallz, bfroehle, David_Rothstein, effulgentsia, corvus_ch: Fixed file_file_download() passed bogus $field to field_access().
-rw-r--r--core/modules/comment/comment.module7
-rw-r--r--core/modules/file/file.api.php25
-rw-r--r--core/modules/file/file.module33
-rw-r--r--core/modules/file/lib/Drupal/file/Tests/FilePrivateTest.php15
-rw-r--r--core/modules/file/tests/file_module_test.module6
-rw-r--r--core/modules/node/node.module6
-rw-r--r--core/modules/user/user.module5
7 files changed, 63 insertions, 34 deletions
diff --git a/core/modules/comment/comment.module b/core/modules/comment/comment.module
index 9f5b2cf..4a75f93 100644
--- a/core/modules/comment/comment.module
+++ b/core/modules/comment/comment.module
@@ -10,7 +10,8 @@
*/
use Drupal\node\Node;
-
+use Drupal\Core\File\File;
+use Drupal\entity\EntityInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
use Symfony\Component\HttpKernel\HttpKernelInterface;
@@ -2136,8 +2137,8 @@ function comment_rdf_mapping() {
/**
* Implements hook_file_download_access().
*/
-function comment_file_download_access($field, $entity_type, $entity) {
- if ($entity_type == 'comment') {
+function comment_file_download_access($field, EntityInterface $entity, File $file) {
+ if ($entity->entityType() == 'comment') {
if (user_access('access comments') && $entity->status == COMMENT_PUBLISHED || user_access('administer comments')) {
$node = node_load($entity->nid);
return node_access('view', $node);
diff --git a/core/modules/file/file.api.php b/core/modules/file/file.api.php
index 7f20d83..e407b43 100644
--- a/core/modules/file/file.api.php
+++ b/core/modules/file/file.api.php
@@ -14,10 +14,10 @@
*
* @param $field
* The field to which the file belongs.
- * @param $entity_type
- * The type of $entity; for example, 'node' or 'user'.
- * @param $entity
+ * @param Drupal\entity\EntityInterface $entity
* The $entity to which $file is referenced.
+ * @param Drupal\Core\File\File $file
+ * The file entity that is being requested.
*
* @return
* TRUE is access should be allowed by this entity or FALSE if denied. Note
@@ -26,8 +26,8 @@
*
* @see hook_field_access().
*/
-function hook_file_download_access($field, $entity_type, $entity) {
- if ($entity_type == 'node') {
+function hook_file_download_access($field, Drupal\entity\EntityInterface $entity, Drupal\Core\File\File $file) {
+ if ($entity->entityType() == 'node') {
return node_access('view', $entity);
}
}
@@ -45,20 +45,21 @@ function hook_file_download_access($field, $entity_type, $entity) {
* An array of grants gathered by hook_file_download_access(). The array is
* keyed by the module that defines the entity type's access control; the
* values are Boolean grant responses for each module.
- * @param $field
- * The field to which the file belongs.
- * @param $entity_type
- * The type of $entity; for example, 'node' or 'user'.
- * @param $entity
- * The $entity to which $file is referenced.
+ * @param array $context
+ * An associative array containing the following key-value pairs:
+ * - entity: The entity to which the field item is referenced.
+ * - field: The field info of the field the field_item belongs to.
+ * - file: The file entity that is being requested.
*
* @return
* An array of grants, keyed by module name, each with a Boolean grant value.
* Return an empty array to assert FALSE. You may choose to return your own
* module's value in addition to other grants or to overwrite the values set
* by other modules.
+ *
+ * @see hook_file_download_access().
*/
-function hook_file_download_access_alter(&$grants, $field, $entity_type, $entity) {
+function hook_file_download_access_alter(&$grants, $context) {
// For our example module, we always enforce the rules set by node module.
if (isset($grants['node'])) {
$grants = array('node' => $grants['node']);
diff --git a/core/modules/file/file.module b/core/modules/file/file.module
index 24dd94f..c46199c 100644
--- a/core/modules/file/file.module
+++ b/core/modules/file/file.module
@@ -167,24 +167,28 @@ function file_file_download($uri, $field_type = 'file') {
foreach ($type_references as $id => $reference) {
// Try to load $entity and $field.
$entity = entity_load($entity_type, $id);
- $field = NULL;
+ $field = field_info_field($field_name);
+
+ // Load the field item that references the file.
+ $match = FALSE;
if ($entity) {
- // Load all fields for that entity.
+ // Load all fields items for that entity.
$field_items = field_get_items($entity_type, $entity, $field_name);
- // Find the field item with the matching URI.
- foreach ($field_items as $field_item) {
- if (file_load($field_item['fid'])->uri == $uri) {
- $field = field_info_field($field_name);
+ // Try to find the field item that references the given file.
+ foreach ($field_items as $item) {
+ if ($file->fid == $item['fid']) {
+ $match = TRUE;
break;
}
}
}
- // Check that $entity and $field were loaded successfully and check if
- // access to that field is not disallowed. If any of these checks fail,
- // stop checking access for this reference.
- if (empty($entity) || empty($field) || !field_access('view', $field, $entity_type, $entity)) {
+ // Check that $entity and $field were loaded successfully, a field
+ // item that references the file exists and check if access to that
+ // field is not disallowed. If any of these checks fail, stop checking
+ // access for this reference.
+ if (empty($entity) || empty($field) || !$match || !field_access('view', $field, $entity_type, $entity)) {
$denied = TRUE;
break;
}
@@ -193,10 +197,15 @@ function file_file_download($uri, $field_type = 'file') {
// Default to FALSE and let entities overrule this ruling.
$grants = array('system' => FALSE);
foreach (module_implements('file_download_access') as $module) {
- $grants = array_merge($grants, array($module => module_invoke($module, 'file_download_access', $field, $entity_type, $entity)));
+ $grants = array_merge($grants, array($module => module_invoke($module, 'file_download_access', $field, $entity, $file)));
}
// Allow other modules to alter the returned grants/denies.
- drupal_alter('file_download_access', $grants, $field, $entity_type, $entity);
+ $context = array(
+ 'entity' => $entity,
+ 'field' => $field,
+ 'file' => $file,
+ );
+ drupal_alter('file_download_access', $grants, $context);
if (in_array(TRUE, $grants)) {
// If TRUE is returned, access is granted and no further checks are
diff --git a/core/modules/file/lib/Drupal/file/Tests/FilePrivateTest.php b/core/modules/file/lib/Drupal/file/Tests/FilePrivateTest.php
index 03226ca..bf66e2d 100644
--- a/core/modules/file/lib/Drupal/file/Tests/FilePrivateTest.php
+++ b/core/modules/file/lib/Drupal/file/Tests/FilePrivateTest.php
@@ -17,7 +17,7 @@ class FilePrivateTest extends FileFieldTestBase {
*
* @var array
*/
- public static $modules = array('node_access_test');
+ public static $modules = array('node_access_test', 'field_test');
public static function getInfo() {
return array(
@@ -45,6 +45,10 @@ class FilePrivateTest extends FileFieldTestBase {
$field_name = strtolower($this->randomName());
$this->createFileField($field_name, $type_name, array('uri_scheme' => 'private'));
+ // Create a field with no view access - see field_test_field_access().
+ $no_access_field_name = 'field_no_view_access';
+ $this->createFileField($no_access_field_name, $type_name, array('uri_scheme' => 'private'));
+
$test_file = $this->getTestFile('text');
$nid = $this->uploadNodeFile($test_file, $field_name, $type_name, TRUE, array('private' => TRUE));
$node = node_load($nid, NULL, TRUE);
@@ -55,5 +59,14 @@ class FilePrivateTest extends FileFieldTestBase {
$this->drupalLogOut();
$this->drupalGet(file_create_url($node_file->uri));
$this->assertResponse(403, t('Confirmed that access is denied for the file without the needed permission.'));
+
+ // Test with the field that should deny access through field access.
+ $this->drupalLogin($this->admin_user);
+ $nid = $this->uploadNodeFile($test_file, $no_access_field_name, $type_name, TRUE, array('private' => TRUE));
+ $node = node_load($nid, NULL, TRUE);
+ $node_file = file_load($node->{$no_access_field_name}[LANGUAGE_NOT_SPECIFIED][0]['fid']);
+ // Ensure the file cannot be downloaded.
+ $this->drupalGet(file_create_url($node_file->uri));
+ $this->assertResponse(403, t('Confirmed that access is denied for the file without view field access permission.'));
}
}
diff --git a/core/modules/file/tests/file_module_test.module b/core/modules/file/tests/file_module_test.module
index 216b10f..662d2b8 100644
--- a/core/modules/file/tests/file_module_test.module
+++ b/core/modules/file/tests/file_module_test.module
@@ -5,6 +5,8 @@
* Provides File module pages for testing purposes.
*/
+use Drupal\entity\EntityInterface;
+
/**
* Implements hook_menu().
*/
@@ -72,8 +74,8 @@ function file_module_test_form_submit($form, &$form_state) {
/**
* Implements hook_file_download_access().
*/
-function file_module_test_file_download_access($field, $entity_type, $entity) {
- $instance = field_info_instance($entity_type, $field['field_name'], $entity->bundle());
+function file_module_test_file_download_access($field, EntityInterface $entity, $field_item) {
+ $instance = field_info_instance($entity->entityType(), $field['field_name'], $entity->bundle());
// Allow the file to be downloaded only if the given arguments are correct.
// If any are wrong, $instance will be NULL.
if (empty($instance)) {
diff --git a/core/modules/node/node.module b/core/modules/node/node.module
index cf10c07..5e2b54b 100644
--- a/core/modules/node/node.module
+++ b/core/modules/node/node.module
@@ -14,6 +14,8 @@ use Drupal\Core\Database\Query\AlterableInterface;
use Drupal\Core\Database\Query\SelectExtender;
use Drupal\Core\Database\Query\SelectInterface;
use Drupal\node\Node;
+use Drupal\Core\File\File;
+use Drupal\entity\EntityInterface;
/**
* Denotes that the node is not published.
@@ -3979,8 +3981,8 @@ function node_modules_disabled($modules) {
/**
* Implements hook_file_download_access().
*/
-function node_file_download_access($field, $entity_type, $entity) {
- if ($entity_type == 'node') {
+function node_file_download_access($field, EntityInterface $entity, File $file) {
+ if ($entity->entityType() == 'node') {
return node_access('view', $entity);
}
}
diff --git a/core/modules/user/user.module b/core/modules/user/user.module
index 535a579..bd80875 100644
--- a/core/modules/user/user.module
+++ b/core/modules/user/user.module
@@ -2,6 +2,7 @@
use Drupal\Core\Database\Query\SelectInterface;
use Drupal\Core\File\File;
+use Drupal\entity\EntityInterface;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
/**
@@ -3291,8 +3292,8 @@ function user_rdf_mapping() {
/**
* Implements hook_file_download_access().
*/
-function user_file_download_access($field, $entity_type, $entity) {
- if ($entity_type == 'user') {
+function user_file_download_access($field, EntityInterface $entity, File $file) {
+ if ($entity->entityType() == 'user') {
return user_view_access($entity);
}
}