From e6fe43704cc2d2209c388fbae5fb645c90ff38f2 Mon Sep 17 00:00:00 2001 From: Claudiu Cristea Date: Sat, 3 Nov 2012 12:47:12 +0200 Subject: [PATCH] Issue #1420812 by claudiu.cristea: Fixed Files from private stream cannot be downloaded. --- file_entity.module | 45 ++++++++++++++++++++++++---------------- tests/file_entity.test | 53 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 18 deletions(-) diff --git a/file_entity.module b/file_entity.module index 8989699..e74fbef 100644 --- a/file_entity.module +++ b/file_entity.module @@ -1124,7 +1124,6 @@ function file_entity_access($op, $file = NULL, $account = NULL) { * Implements hook_file_entity_access(). */ function file_entity_file_entity_access($op, $file, $account) { - $grants = array(); // If the file URI is invalid, deny access. if (is_object($file) && !file_valid_uri($file->uri)) { @@ -1137,35 +1136,45 @@ function file_entity_file_entity_access($op, $file, $account) { } } + $is_own_file = is_object($file) && ($account->uid == $file->uid); + if ($op == 'update') { - if (user_access('edit any files', $account) || (is_object($file) && user_access('edit own files', $account) && ($account->uid == $file->uid))) { + if (user_access('edit any files', $account) || ($is_own_file && user_access('edit own files', $account))) { return FILE_ENTITY_ACCESS_ALLOW; } } if ($op == 'delete') { - if (user_access('delete any files', $account) || (is_object($file) && user_access('delete own files', $account) && ($account->uid == $file->uid))) { + if (user_access('delete any files', $account) || ($is_own_file && user_access('delete own files', $account))) { return FILE_ENTITY_ACCESS_ALLOW; } } - if ($op == 'view' && is_object($file) && file_uri_scheme($file->uri) == 'private') { - // When viewing private files, we can only invoke hook_file_download() - // if the $account user objet matches the current user. - if ($GLOBALS['user']->uid == $account->uid) { - foreach (module_implements('file_download') as $module) { - $access = module_invoke($module, 'file_download', $file->uri); - if ($access === -1) { - return FILE_ENTITY_ACCESS_DENY; - } - elseif (!empty($access)) { - $grants[] = $access; - } - } - } + return FILE_ENTITY_ACCESS_IGNORE; +} + +/** + * Implements hook_file_download(). + */ +function file_entity_file_download($uri) { + + // Get the File entity from $uri. + $file = reset(file_load_multiple(array(), array('uri' => $uri))); + + // We didn't find any managed record in the database. File entity module will + // not make any decision in this case. E.g. image derivatives will fall here. + if (!is_object($file) || empty($file->fid)) { + return NULL; } - return !empty($grants) ? FILE_ENTITY_ACCESS_ALLOW : FILE_ENTITY_ACCESS_IGNORE; + // If user can view file details, he's able also to download the file. + if (file_entity_access('view', $file)) { + return array( + 'Content-Type' => $file->filemime, + 'Content-Length' => $file->filesize, + ); + } + return -1; } /** diff --git a/tests/file_entity.test b/tests/file_entity.test index 52da7b6..dd02465 100644 --- a/tests/file_entity.test +++ b/tests/file_entity.test @@ -418,4 +418,57 @@ class FileEntityAccessTestCase extends FileEntityTestHelper { $this->drupalGet("file/{$file->fid}/delete"); $this->assertResponse(200, 'Users with access can access the file add page'); } + + /** + * Test download access. + */ + function testFileEntityPrivateDownloadAccess() { + // Define several cases for accesing private files. The associative array + // takes assertion messages as keys and an associative array, with the next + // keys as value: + // - "perms" array of permissions for user creation or NULL for anonymous. + // - "expect" expected HTTP response code. + // - "owner" Optional boolean indicating if the user will is the file owner. + $cases = array( + "Owner not granted with 'view own private files' cannot download his files." => array('perms' => array(), 'expect' => 403, 'owner' => TRUE), + 'Owner can download his files.' => array('perms' => array('view own private files'), 'expect' => 200, 'owner' => TRUE), + 'Anonymous cannot download private files.' => array('perms' => NULL, 'expect' => 403), + "Regular user cannot download other's private files." => array('perms' => array(), 'expect' => 403), + "User able to see everyone's public file details cannot download private files." => array('perms' => array('view files'), 'expect' => 403), + "Superuser can download everyone's files." => array('perms' => array('bypass file access'), 'expect' => 200), + ); + + foreach ($cases as $message => $case) { + // Create users and login only if non-anonymous. + $authenticated_user = !is_null($case['perms']); + if ($authenticated_user) { + $account = $this->drupalCreateUser($case['perms']); + $this->drupalLogin($account); + } + + // Create private, permanent files owned by this user only he's an owner. + if (!empty($case['owner'])) { + $file = next($this->files['text']); + $file->status = FILE_STATUS_PERMANENT; + $file->uid = $account->uid; + file_save($file); + $file = file_move($file, 'private://'); + + // Check if the physical file is there. + $arguments = array('%name' => $file->filename, '%username' => $account->name, '%uri' => $file->uri); + $this->assertTrue(is_file($file->uri), format_string('File %name owned by %username successfully created at %uri.', $arguments)); + $url = file_create_url($file->uri); + $message_file_info = ' ' . format_string('File %uri was checked.', array('%uri' => $file->uri)); + } + + // Try to download the file. + $this->drupalGet($url); + $this->assertResponse($case['expect'], $message . $message_file_info); + + // Logout authenticated users. + if ($authenticated_user) { + $this->drupalLogout(); + } + } + } } -- 1.7.9.6 (Apple Git-31.1)