#610290 by dww and sun: system_run_authorized() shouldn't always drupal_goto() so it can be used in submit handlers.

3 files changed, 94 insertions, 23 deletions

diff --git a/authorize.php b/authorize.php
index 140c5a027..d9b437540 100644
--- a/authorize.php
+++ b/authorize.php
@@ -3,10 +3,12 @@
 
 /**
  * @file
- * Administrative script where the site owner (the user actually owning the
- * files on the webserver) can authorize certain file-related operations to
- * proceed with elevated privileges, for example to deploy and upgrade modules
- * or themes. Users should not visit this page directly, but instead use an
+ * Administrative script for running authorized file operations.
+ *
+ * Using this script, the site owner (the user actually owning the files on
+ * the webserver) can authorize certain file-related operations to proceed
+ * with elevated privileges, for example to deploy and upgrade modules or
+ * themes. Users should not visit this page directly, but instead use an
  * administrative user interface which knows how to redirect the user to this
  * script as part of a multistep process. This script actually performs the
  * selected operations without loading all of Drupal, to be able to more
@@ -14,7 +16,9 @@
  * global killswitch in settings.php ('allow_authorize_operations') and via
  * the 'administer software updates' permission.
  *
- * @see system_run_authorized()
+ * There are helper functions for setting up an operation to run via this
+ * system in modules/system/system.module. For more information, see:
+ * @link authorize Authorized operation helper functions @endlink
  */
 
 /**
diff --git a/modules/system/system.module b/modules/system/system.module
index bd4f2e2e9..41971c0eb 100644
--- a/modules/system/system.module
+++ b/modules/system/system.module
@@ -1450,14 +1450,56 @@ function _system_themes_access($theme) {
 }
 
 /**
- * Invoke a given callback via authorize.php to run with elevated privileges.
- *
- * To use authorize.php, certain variables must be stashed into
- * $_SESSION. This function sets up all the necessary $_SESSION variables,
- * then redirects to authorize.php to initiate the workflow that will
- * eventually lead to the callback being invoked. The callback will be invoked
- * at a low bootstrap level, without all modules being invoked, so it needs to
- * be careful not to assume any code exists.
+ * @defgroup authorize Authorized operations
+ * @{
+ * Functions to run operations with elevated privileges via authorize.php.
+ *
+ * Because of the Update manager functionality included in Drupal core, there
+ * is a mechanism for running operations with elevated file system privileges,
+ * the top-level authorize.php script. This script runs at a reduced Drupal
+ * bootstrap level so that it is not reliant on the entire site being
+ * functional. The operations use a FileTransfer class to manipulate code
+ * installed on the system as the user that owns the files, not the user that
+ * the httpd is running as.
+ *
+ * The first setup is to define a callback function that should be authorized
+ * to run with the elevated privileges. This callback should take a
+ * FileTransfer as its first argument, although you can define an array of
+ * other arguments it should be invoked with. The callback should be placed in
+ * a separate .inc file that will be included by authorize.php.
+ *
+ * To run the operation, certain data must be saved into the SESSION, and then
+ * the flow of control should be redirected to the authorize.php script. There
+ * are two ways to do this, either to call system_authorized_run() directly,
+ * or to call system_authorized_init() and then redirect to authorize.php,
+ * using the URL from system_authorized_get_url(). Redirecting yourself is
+ * necessary when your authorized operation is being triggered by a form
+ * submit handler, since calling drupal_goto() in a submit handler is a bad
+ * idea, and you should instead set $form_state['redirect'].
+ *
+ * Once the SESSION is setup for the operation and the user is redirected to
+ * authorize.php, they will be prompted for their connection credentials (core
+ * provides FTP and SSH by default, although other connection classes can be
+ * added via contributed modules). With valid credentials, authorize.php will
+ * instantiate the appropriate FileTransfer object, and then invoke the
+ * desired operation passing in that object. The authorize.php script can act
+ * as a Batch API processing page, if the operation requires a batch.
+ *
+ * @see authorize.php
+ * @see FileTransfer
+ * @see hook_filetransfer_backends()
+ */
+
+/**
+ * Setup a given callback to run via authorize.php with elevated privileges.
+ *
+ * To use authorize.php, certain variables must be stashed into $_SESSION.
+ * This function sets up all the necessary $_SESSION variables, then returns
+ * the full path to authorize.php so the caller can redirect to authorize.php.
+ * That initiates the workflow that will eventually lead to the callback being
+ * invoked. The callback will be invoked at a low bootstrap level, without all
+ * modules being invoked, so it needs to be careful not to assume any code
+ * exists.
  *
  * @param $callback
  *   The name of the function to invoke one the user authorizes the operation.
@@ -1470,11 +1512,9 @@ function _system_themes_access($theme) {
  * @param $page_title
  *   Optional string to use as the page title once redirected to authorize.php.
  * @return
- *   Nothing. This function redirects to authorize.php and does not return.
+ *   Nothing, this function just initializes variables in the user's session.
  */
-function system_run_authorized($callback, $file, $arguments = array(), $page_title = NULL) {
-  global $base_url;
-
+function system_authorized_init($callback, $file, $arguments = array(), $page_title = NULL) {
   // First, figure out what file transfer backends the site supports, and put
   // all of those in the SESSION so that authorize.php has access to all of
   // them via the class autoloader, even without a full bootstrap.
@@ -1490,12 +1530,31 @@ function system_run_authorized($callback, $file, $arguments = array(), $page_tit
   if (isset($page_title)) {
     $_SESSION['authorize_operation']['page_title'] = $page_title;
   }
+}
 
-  // Finally, redirect to authorize.php.
-  drupal_goto($base_url . '/authorize.php');
+/**
+ * Return the URL for the authorize.php script.
+ */
+function system_authorized_get_url() {
+  global $base_url;
+  return $base_url . '/authorize.php';
 }
 
 /**
+ * Setup and invoke an operation using authorize.php.
+ *
+ * @see system_authorized_init
+ */
+function system_authorized_run($callback, $file, $arguments = array(), $page_title = NULL) {
+  system_authorized_init($callback, $file, $arguments, $page_title);
+  drupal_goto(system_authorized_get_url());
+}
+
+/**
+ * @} End of "defgroup authorize".
+ */
+
+/**
  * Implement hook_updater_info().
  */
 function system_updater_info() {
diff --git a/modules/update/update.manager.inc b/modules/update/update.manager.inc
index c661a9593..1790e4176 100644
--- a/modules/update/update.manager.inc
+++ b/modules/update/update.manager.inc
@@ -396,9 +396,11 @@ function update_manager_confirm_update_form($form, &$form_state) {
  * update, do so now. Otherwise, pull information about all the required
  * updates out of the SESSION, figure out what Updater class is needed for
  * each one, generate an array of update operations to perform, and hand it
- * all off to system_run_authorized() where we redirect to authorize.php.
+ * all off to system_authorized_init(), then redirect to authorize.php.
  *
- * @see system_run_authorized()
+ * @see update_authorize_run_update()
+ * @see system_authorized_init()
+ * @see system_authorized_get_url()
  */
 function update_manager_confirm_update_form_submit($form, &$form_state) {
   if ($form_state['values']['site_offline'] == TRUE) {
@@ -440,7 +442,8 @@ function update_manager_confirm_update_form_submit($form, &$form_state) {
     // credentials and invoke update_authorize_run_update() indirectly with
     // whatever FileTransfer object authorize.php creates for us.
     else {
-      system_run_authorized('update_authorize_run_update', drupal_get_path('module', 'update') . '/update.authorize.inc', array($updates));
+      system_authorized_init('update_authorize_run_update', drupal_get_path('module', 'update') . '/update.authorize.inc', array($updates));
+      $form_state['redirect'] = system_authorized_get_url();
     }
   }
 }
@@ -536,6 +539,10 @@ function update_manager_install_form_validate($form, &$form_state) {
  * the live webroot. If everything is successful, setup an operation to run
  * via authorize.php which will copy the extracted files from the temporary
  * location into the live site.
+ *
+ * @see update_authorize_run_install()
+ * @see system_authorized_init()
+ * @see system_authorized_get_url()
  */
 function update_manager_install_form_submit($form, &$form_state) {
   if ($form_state['values']['project_url']) {
@@ -616,7 +623,8 @@ function update_manager_install_form_submit($form, &$form_state) {
   // credentials and invoke update_authorize_run_install() indirectly with
   // whatever FileTransfer object authorize.php creates for us.
   else {
-    system_run_authorized('update_authorize_run_install', drupal_get_path('module', 'update') . '/update.authorize.inc', $arguments);
+    system_authorized_init('update_authorize_run_install', drupal_get_path('module', 'update') . '/update.authorize.inc', $arguments);
+    $form_state['redirect'] = system_authorized_get_url();
   }
 }