diff options
author | Angie Byron <webchick@24967.no-reply.drupal.org> | 2010-08-23 14:53:50 +0000 |
---|---|---|
committer | Angie Byron <webchick@24967.no-reply.drupal.org> | 2010-08-23 14:53:50 +0000 |
commit | aa235ac59cff6134190da5c853594ad779f36949 (patch) | |
tree | f78b49ac58b33c45a12a6c090b03a42cd963c2fe /modules/file | |
parent | 943d9c06b0cfd9ccee5d05d3d86e67bbd48de912 (diff) | |
download | brdo-aa235ac59cff6134190da5c853594ad779f36949.tar.gz brdo-aa235ac59cff6134190da5c853594ad779f36949.tar.bz2 |
#846296 by Berdir, ridgerunner, agentrickard: Fixed file_file_download() only implements access checks for nodes and users.
Diffstat (limited to 'modules/file')
-rw-r--r-- | modules/file/file.api.php | 67 | ||||
-rw-r--r-- | modules/file/file.module | 85 | ||||
-rw-r--r-- | modules/file/tests/file.test | 64 |
3 files changed, 184 insertions, 32 deletions
diff --git a/modules/file/file.api.php b/modules/file/file.api.php new file mode 100644 index 000000000..b5cf7931e --- /dev/null +++ b/modules/file/file.api.php @@ -0,0 +1,67 @@ +<?php +// $Id$ + +/** + * @file + * Hooks for file module. + */ + +/** + * Control download access to files. + * + * The hook is typically implemented to limit access based on the entity the + * file is referenced, e.g., only users with access to a node should be allowed + * to download files attached to that node. + * + * @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. + * + * @return + * TRUE is access should be allowed by this entity or FALSE if denied. Note + * that denial may be overridden by another entity controller, making this + * grant permissive rather than restrictive. + * + * @see hook_field_access(). + */ +function hook_file_download_access($field, $entity_type, $entity) { + if ($entity_type == 'node') { + return node_access('view', $entity); + } +} + +/** + * Alter the access rules applied to a file download. + * + * Entities that implement file management set the access rules for their + * individual files. Module may use this hook to create custom access rules + * for file downloads. + * + * @see hook_file_download_access(). + * + * @param &$grants + * 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. + * + * @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. + */ +function hook_file_download_access_alter(&$grants, $field, $entity_type, $entity) { + // 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/modules/file/file.module b/modules/file/file.module index a69a21dca..f5f642ef2 100644 --- a/modules/file/file.module +++ b/modules/file/file.module @@ -141,46 +141,69 @@ function file_file_download($uri, $field_type = 'file') { // Find out which (if any) file fields contain this file. $references = file_get_file_references($file, NULL, FIELD_LOAD_REVISION, $field_type); - // TODO: Check field-level access if available here. - - $denied = $file->status ? NULL : FALSE; - // Check access to content containing the file fields. If access is allowed - // to any of this content, allow the download. + // Default to allow access. + $denied = FALSE; + // Loop through all references of this file. If a reference explicitly allows + // access to the field to which this file belongs, no further checks are done + // and download access is granted. If a reference denies access, eventually + // existing additional references are checked. If all references were checked + // and no reference denied access, access is granted as well. If at least one + // reference denied access, access is denied. foreach ($references as $field_name => $field_references) { foreach ($field_references as $entity_type => $type_references) { - foreach ($type_references as $reference) { - // If access is allowed to any object, immediately stop and grant - // access. If access is denied, continue through in case another object - // grants access. - // TODO: Switch this to a universal access check mechanism if available. - if ($entity_type == 'node' && ($node = node_load($reference->nid))) { - if (node_access('view', $node)) { - $denied = FALSE; - break 3; - } - else { - $denied = TRUE; + foreach ($type_references as $id => $reference) { + // Try to load $entity and $field. + $entity = reset(entity_load($entity_type, array($id))); + $field = NULL; + if ($entity) { + // Load all fields 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 ($field_item['uri'] == $uri) { + $field = $field_item; + break; + } } } - if ($entity_type == 'user') { - if (user_access('access user profiles') || $user->uid == $reference->uid) { - $denied = FALSE; - break 3; - } - else { - $denied = TRUE; - } + + // 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)) { + $denied = TRUE; + break; + } + + // Invoke hook and collect grants/denies for download access. + // 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))); + } + // Allow other modules to alter the returned grants/denies. + drupal_alter('file_download_access', $grants, $field, $entity_type, $entity); + + if (in_array(TRUE, $grants)) { + // If TRUE is returned, access is granted and no further checks are + // necessary. + $denied = FALSE; + break 3; + } + + if (in_array(FALSE, $grants)) { + // If an implementation returns FALSE, access to this entity is denied + // but the file could belong to another entity to which the user might + // have access. Continue with these. + $denied = TRUE; } } } } - // No access was denied or granted. - if (!isset($denied)) { - return; - } - // Access specifically denied and not granted elsewhere. - elseif ($denied == TRUE) { + // Access specifically denied. + if ($denied) { return -1; } diff --git a/modules/file/tests/file.test b/modules/file/tests/file.test index cc275e202..278aed332 100644 --- a/modules/file/tests/file.test +++ b/modules/file/tests/file.test @@ -14,7 +14,7 @@ class FileFieldTestCase extends DrupalWebTestCase { function setUp() { parent::setUp('file'); - $this->admin_user = $this->drupalCreateUser(array('access content', 'access administration pages', 'administer site configuration', 'administer users', 'administer content types', 'administer nodes', 'bypass node access')); + $this->admin_user = $this->drupalCreateUser(array('access content', 'access administration pages', 'administer site configuration', 'administer users', 'administer permissions', 'administer content types', 'administer nodes', 'bypass node access')); $this->drupalLogin($this->admin_user); } @@ -301,6 +301,68 @@ class FileFieldWidgetTestCase extends FileFieldTestCase { $this->drupalGet("admin/structure/types/manage/$type_name/fields/$field_name"); $this->assertFieldByXpath('//input[@id="edit-field-settings-uri-scheme-public" and not(@disabled)]', 'public', t('Upload destination setting enabled.')); } + + /** + * Tests that download restrictions on private files work on comments. + */ + function testPrivateFileComment() { + $user = $this->drupalCreateUser(array('access comments')); + + // Remove access comments permission from anon user. + $edit = array( + '1[access comments]' => FALSE, + ); + $this->drupalPost('admin/people/permissions', $edit, t('Save permissions')); + + // Create a new field. + $edit = array( + '_add_new_field[label]' => $label = $this->randomName(), + '_add_new_field[field_name]' => $name = strtolower($this->randomName()), + '_add_new_field[type]' => 'file', + '_add_new_field[widget_type]' => 'file_generic', + ); + $this->drupalPost('admin/structure/types/manage/article/comment/fields', $edit, t('Save')); + $edit = array('field[settings][uri_scheme]' => 'private'); + $this->drupalPost(NULL, $edit, t('Save field settings')); + $this->drupalPost(NULL, array(), t('Save settings')); + + // Create node. + $text_file = $this->getTestFile('text'); + $edit = array( + 'title' => $this->randomName(), + ); + $this->drupalPost('node/add/article', $edit, t('Save')); + + // Add a comment with a file. + $text_file = $this->getTestFile('text'); + $edit = array( + 'files[field_' . $name . '_' . LANGUAGE_NONE . '_' . 0 . ']' => realpath($text_file->uri), + 'comment_body[' . LANGUAGE_NONE . '][0][value]' => $comment_body = $this->randomName(), + ); + $this->drupalPost(NULL, $edit, t('Save')); + + // Get the comment ID. + preg_match('/comment-([0-9]+)/', $this->getUrl(), $matches); + $cid = $matches[1]; + + // Log in as normal user. + $this->drupalLogin($user); + + $comment = comment_load($cid); + $comment_file = (object) $comment->{'field_' . $name}[LANGUAGE_NONE][0]; + $this->assertFileExists($comment_file, t('New file saved to disk on node creation.')); + // Test authenticated file download. + $url = file_create_url($comment_file->uri); + $this->assertNotEqual($url, NULL, t('Confirmed that the URL is valid')); + $this->drupalGet(file_create_url($comment_file->uri)); + $this->assertResponse(200, t('Confirmed that the generated URL is correct by downloading the shipped file.')); + + // Test anonymous file download. + $this->drupalLogout(); + $this->drupalGet(file_create_url($comment_file->uri)); + $this->assertResponse(403, t('Confirmed that access is denied for the file without the needed permission.')); + } + } /** |