From 141ee541cad11846d3df8e1aa336ceb8fca89a11 Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Tue, 14 Mar 2017 12:17:30 +0800 Subject: [PATCH] MDL-58219 repository: Change how controlled links work Files are copied to the system user as soon as they are uploaded. Write access is then controlled when serving links to the file. Part of MDL-58220 --- lib/classes/oauth2/api.php | 2 +- lib/classes/oauth2/rest.php | 3 ++- lib/db/install.xml | 1 - lib/db/upgrade.php | 1 - lib/filelib.php | 24 ++++++----------- lib/filestorage/file_storage.php | 35 ++----------------------- lib/oauthlib.php | 6 +---- lib/upgrade.txt | 3 +-- mod/assign/submission/file/locallib.php | 13 --------- mod/data/field/file/field.class.php | 2 -- mod/forum/lib.php | 2 -- mod/workshop/locallib.php | 7 ----- repository/lib.php | 18 +++---------- repository/repository_ajax.php | 4 --- 14 files changed, 19 insertions(+), 102 deletions(-) diff --git a/lib/classes/oauth2/api.php b/lib/classes/oauth2/api.php index 3ca18ce7034..c886a1aaecb 100644 --- a/lib/classes/oauth2/api.php +++ b/lib/classes/oauth2/api.php @@ -107,7 +107,7 @@ class api { 'name' => 'alternatename', 'last_name' => 'lastname', 'email' => 'email', - 'id' => 'username', + 'third_party_id' => 'username', 'first_name' => 'firstname', 'picture-data-url' => 'picture', 'link' => 'url', diff --git a/lib/classes/oauth2/rest.php b/lib/classes/oauth2/rest.php index f7049e6f54c..c790429c891 100644 --- a/lib/classes/oauth2/rest.php +++ b/lib/classes/oauth2/rest.php @@ -74,6 +74,7 @@ abstract class rest { $method = $functions[$functionname]['method']; $endpoint = $functions[$functionname]['endpoint']; + $responsetype = $functions[$functionname]['response']; if (!in_array($method, $supportedmethods)) { throw new coding_exception('unsupported api method: ' . $method); @@ -112,7 +113,7 @@ abstract class rest { $json = json_decode($response); if (!empty($json->error)) { - throw new rest_exception($json->error->message, $json->error->code); + throw new rest_exception($json->error->code . ': ' . $json->error->message); } return $json; } diff --git a/lib/db/install.xml b/lib/db/install.xml index 7610a685c53..adb86a4f4ed 100755 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -3531,7 +3531,6 @@ - diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 5deb30d322b..c5878f7b3a0 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2719,7 +2719,6 @@ function xmldb_main_upgrade($oldversion) { // Adding keys to table oauth2_user_field_mapping. $table->add_key('primary', XMLDB_KEY_PRIMARY, array('id')); $table->add_key('issuerkey', XMLDB_KEY_FOREIGN, array('issuerid'), 'oauth2_issuer', array('id')); - $table->add_key('uniqexternal', XMLDB_KEY_UNIQUE, array('issuerid', 'externalfield')); $table->add_key('uniqinternal', XMLDB_KEY_UNIQUE, array('issuerid', 'internalfield')); // Conditionally launch create table for oauth2_user_field_mapping. diff --git a/lib/filelib.php b/lib/filelib.php index df71196dbba..d9e0b0bb414 100644 --- a/lib/filelib.php +++ b/lib/filelib.php @@ -256,21 +256,6 @@ function file_postupdate_standard_editor($data, $field, array $options, $context return $data; } -/** - * For all files in this file area - walk the file list and copy each to a system owned account, making them read-only. - * - * @category files - * @param int $contextid context id - must already exist - * @param string $component - * @param string $filearea file area name - * @param int $itemid - * @return bool - */ -function file_prevent_changes_to_external_files($contextid, $component, $filearea, $itemid=false) { - $fs = get_file_storage(); - return $fs->prevent_changes_to_external_files($contextid, $component, $filearea, $itemid); -} - /** * Saves text and files modified by Editor formslib element * @@ -968,8 +953,15 @@ function file_save_draft_area_files($draftitemid, $contextid, $component, $filea if ($file->is_external_file()) { $repoid = $file->get_repository_id(); if (!empty($repoid)) { + $context = context::instance_by_id($contextid, MUST_EXIST); + $repo = repository::get_repository_by_id($repoid, $context); + $file_record['repositoryid'] = $repoid; - $file_record['reference'] = $file->get_reference(); + // This hook gives the repo a place to do some house cleaning, and update the $reference before it's saved + // to the file store. E.g. transfer ownership of the file to a system account etc. + $reference = $repo->reference_file_selected($file->get_reference(), $context, $component, $filearea, $itemid); + + $file_record['reference'] = $reference; } } diff --git a/lib/filestorage/file_storage.php b/lib/filestorage/file_storage.php index bdc0c6cb100..7fd1eaaa4ac 100644 --- a/lib/filestorage/file_storage.php +++ b/lib/filestorage/file_storage.php @@ -1128,9 +1128,8 @@ class file_storage { // creating a new file from an existing alias creates new alias implicitly. // here we just check the database consistency. if (!empty($newrecord->repositoryid)) { - if ($newrecord->referencefileid != $this->get_referencefileid($newrecord->repositoryid, $newrecord->reference, MUST_EXIST)) { - throw new file_reference_exception($newrecord->repositoryid, $newrecord->reference, $newrecord->referencefileid); - } + // It is OK if the current reference does not exist. It may have been altered by a repository plugin when the files where saved from a draft area. + $newrecord->referencefileid = $this->get_or_create_referencefileid($newrecord->repositoryid, $newrecord->reference); } try { @@ -2324,34 +2323,4 @@ class file_storage { $DB->update_record('files_reference', (object)$data); } - /** - * For an entire file area - walk through the files and for each one that is a controlled link, - * call prevent_changes on the repository. Typically this will copy the external file to a system - * account controlled by Moodle, remove all write access and update the file reference. - * - * @param int $contextid - * @param string $component - * @param string $filearea - * @param int $itemid - */ - public function prevent_changes_to_external_files($contextid, $component, $filearea, $itemid = false) { - global $DB; - - $transaction = $DB->start_delegated_transaction(); - - $files = $this->get_area_files($contextid, $component, $filearea, $itemid, 'id', false); - - foreach ($files as $file) { - if ($file->is_external_file()) { - // Note that this function uses a cache, so we don't need to - // double cache these. - $repo = repository::get_repository_by_id($file->get_repository_id(), SYSCONTEXTID); - - // We expect this function to throw exceptions on failure. - $repo->prevent_changes_to_external_file($file); - } - } - $transaction->allow_commit(); - return true; - } } diff --git a/lib/oauthlib.php b/lib/oauthlib.php index cf62393b61d..c684c9c911a 100644 --- a/lib/oauthlib.php +++ b/lib/oauthlib.php @@ -576,11 +576,7 @@ abstract class oauth2_client extends curl { // Expires 10 seconds before actual expiry. $accesstoken->expires = (time() + ($r->expires_in - 10)); } - if (isset($r->scope)) { - $accesstoken->scope = $r->scope; - } else { - $accesstoken->scope = $this->scope; - } + $accesstoken->scope = $this->scope; // Also add the scopes. self::$upgradedcodes[] = $code; $this->store_token($accesstoken); diff --git a/lib/upgrade.txt b/lib/upgrade.txt index b1293039d5e..088454b38bb 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -2,8 +2,7 @@ This files describes API changes in core libraries and APIs, information provided here is intended especially for developers. === 3.3 === * Support added for a new type of external file: FILE_CONTROLLED_LINK. This is an external file that Moodle can control - the permissions. Moodle can make files read-only or grant temporary write access. - To make all the files in file area read only (owned by Moodle) - use file_prevent_changes_to_external_files(). + the permissions. Moodle makes files read-only but can grant temporary write access. When accessing a URL, the info from file_browser::get_file_info will be checked to determine if the user has write access, if they do - the remote file will have access controls set to allow editing. * The method moodleform::after_definition() has been added and can now be used to add some logic diff --git a/mod/assign/submission/file/locallib.php b/mod/assign/submission/file/locallib.php index ba222f490d7..cd54142cb34 100644 --- a/mod/assign/submission/file/locallib.php +++ b/mod/assign/submission/file/locallib.php @@ -552,19 +552,6 @@ class assign_submission_file extends assign_submission_plugin { ); } - /** - * Make any controlled links in the submission area read-only for the student. - * - * @param stdClass $submission the assign_submission record being submitted. - * @return void - */ - public function submit_for_grading($submission) { - file_prevent_changes_to_external_files($this->assignment->get_context()->id, - 'assignsubmission_file', - ASSIGNSUBMISSION_FILE_FILEAREA, - $submission->id); - } - /** * Return the plugin configs for external functions. * diff --git a/mod/data/field/file/field.class.php b/mod/data/field/file/field.class.php index 0263432c21d..251fa7c93dc 100644 --- a/mod/data/field/file/field.class.php +++ b/mod/data/field/file/field.class.php @@ -185,8 +185,6 @@ class data_field_file extends data_field_base { $usercontext = context_user::instance($USER->id); $files = $fs->get_area_files($this->context->id, 'mod_data', 'content', $content->id, 'itemid, filepath, filename', false); - file_prevent_changes_to_external_files($this->context->id, 'mod_data', 'content', $content->id); - // We expect no or just one file (maxfiles = 1 option is set for the form_filemanager). if (count($files) == 0) { $content->content = null; diff --git a/mod/forum/lib.php b/mod/forum/lib.php index cef2dd0e197..a14d1eddb80 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -559,9 +559,7 @@ function forum_cron() { } } - // We need to prevent changes to controlled links in attachments. $modcontext = context_module::instance($coursemodules[$forumid]->id); - file_prevent_changes_to_external_files($modcontext->id, 'mod_forum', 'attachment', $pid); // Save the Inbound Message datakey here to reduce DB queries later. $messageinboundgenerator->set_data($pid); diff --git a/mod/workshop/locallib.php b/mod/workshop/locallib.php index c19a0c02d6b..af0dca3c231 100644 --- a/mod/workshop/locallib.php +++ b/mod/workshop/locallib.php @@ -1843,13 +1843,6 @@ class workshop { workshop_update_grades($workshop); } - if (self::PHASE_ASSESSMENT == $newphase) { - file_prevent_changes_to_external_files($this->context->id, 'mod_workshop', 'submission_content'); - } - if (self::PHASE_EVALUATION == $newphase) { - file_prevent_changes_to_external_files($this->context->id, 'mod_workshop', 'overallfeedback_attachment'); - } - $DB->set_field('workshop', 'phase', $newphase, array('id' => $this->id)); $this->phase = $newphase; $eventdata = array( diff --git a/repository/lib.php b/repository/lib.php index f30c5cb7e01..7522ca9247f 100644 --- a/repository/lib.php +++ b/repository/lib.php @@ -1290,9 +1290,12 @@ abstract class repository implements cacheable_object { * @param string $reference this reference is generated by * repository::get_file_reference() * @param context $context the target context for this new file. + * @param string $component the target component for this new file. + * @param string $filearea the target filearea for this new file. + * @param string $itemid the target itemid for this new file. * @return string updated reference (final one before it's saved to db). */ - public function reference_file_selected($reference, $context) { + public function reference_file_selected($reference, $context, $component, $filearea, $itemid) { return $reference; } @@ -2680,19 +2683,6 @@ abstract class repository implements cacheable_object { 'Use repository::sync_reference instead.'); } - /** - * Update an external file so only Moodle has write access to it. - * This function must be implemented by all repositories supporting FILE_CONTROLLED_LINK return types. - * - * Throw exceptions on error and the transaction will be rolled back - * (because it is called on an entire filearea at a time). - * - * @param stored_file $file - */ - public function prevent_changes_to_external_file(stored_file $file) { - return; - } - /** * Performs synchronisation of an external file if the previous one has expired. * diff --git a/repository/repository_ajax.php b/repository/repository_ajax.php index 20ae194f2fc..9cda39912ba 100644 --- a/repository/repository_ajax.php +++ b/repository/repository_ajax.php @@ -228,10 +228,6 @@ switch ($action) { $record->filesize = $sourcefile->get_filesize(); } - // This hook gives the repo a place to do some house cleaning, and update the $reference before it's saved - // to the file store. E.g. transfer ownership of the file to a system account etc. - $reference = $repo->reference_file_selected($reference, $context); - // Check if file exists. if (repository::draftfile_exists($itemid, $saveas_path, $saveas_filename)) { // File name being used, rename it.