diff --git authorize.php authorize.php index c107260..f3db04b 100644 --- authorize.php +++ authorize.php @@ -125,7 +125,7 @@ if (authorize_access_allowed()) { // Clear the session out. unset($_SESSION['authorize_results']); unset($_SESSION['authorize_operation']); - unset($_SESSION['authorize_filetransfer_backends']); + unset($_SESSION['authorize_filetransfer_info']); if (!empty($results['page_title'])) { drupal_set_title(check_plain($results['page_title'])); @@ -153,7 +153,7 @@ if (authorize_access_allowed()) { $output = _batch_page(); } else { - if (empty($_SESSION['authorize_operation']) || empty($_SESSION['authorize_filetransfer_backends'])) { + if (empty($_SESSION['authorize_operation']) || empty($_SESSION['authorize_filetransfer_info'])) { $output = t('It appears you have reached this page in error.'); } elseif (!$batch = batch_get()) { diff --git includes/authorize.inc includes/authorize.inc index c5ba691..9ce7004 100644 --- includes/authorize.inc +++ includes/authorize.inc @@ -21,12 +21,11 @@ function authorize_filetransfer_form($form_state) { $form['#attached']['js'][] = $base_url . '/misc/authorize.js'; // Get all the available ways to transfer files. - if (empty($_SESSION['authorize_filetransfer_backends'])) { + if (empty($_SESSION['authorize_filetransfer_info'])) { drupal_set_message(t('Unable to continue, no available methods of file transfer'), 'error'); return array(); } - $available_backends = $_SESSION['authorize_filetransfer_backends']; - uasort($available_backends, 'drupal_sort_weight'); + $available_backends = $_SESSION['authorize_filetransfer_info']; if (!$is_https) { drupal_set_message(t('WARNING: You are not using an encrypted connection, so your password will be sent in plain text. Learn more.', array('@https-link' => 'http://drupal.org/https-information')), 'error'); @@ -87,8 +86,7 @@ function authorize_filetransfer_form($form_state) { '#title' => t('@backend connection settings', array('@backend' => $backend['title'])), ); - $current_settings = variable_get('authorize_filetransfer_connection_settings_' . $name, array()); - $form['connection_settings'][$name] += system_get_filetransfer_settings_form($name, $current_settings); + $form['connection_settings'][$name] += _authorize_filetransfer_connection_settings($name); // Start non-JS code. if (isset($form_state['values']['connection_settings']['authorize_filetransfer_default']) && $form_state['values']['connection_settings']['authorize_filetransfer_default'] == $name) { @@ -122,6 +120,65 @@ function authorize_filetransfer_form($form_state) { } /** + * Generate the Form API array for the settings for a given connection backend. + * + * @param $backend + * The name of the backend (e.g. 'ftp', 'ssh', etc). + * @return + * Form API array of connection settings for the given backend. + * + * @see hook_filetransfer_backends() + */ +function _authorize_filetransfer_connection_settings($backend) { + $defaults = variable_get('authorize_filetransfer_connection_settings_' . $backend, array()); + $form = array(); + + // Create an instance of the file transfer class to get its settings form. + $filetransfer = authorize_get_filetransfer($backend); + if ($filetransfer) { + $form = $filetransfer->getSettingsForm(); + } + // Fill in the defaults based on the saved settings, if any. + _authorize_filetransfer_connection_settings_set_defaults($form, NULL, $defaults); + return $form; +} + +/** + * Recursively fill in the default settings on a file transfer connection form. + * + * The default settings for the file transfer connection forms are saved in + * the database. The settings are stored as a nested array in the case of a + * settings form that has fieldsets or otherwise uses a nested structure. + * Therefore, to properly add defaults, we need to walk through all the + * children form elements and process those defaults recursively. + * + * @param &$element + * Reference to the Form API form element we're operating on. + * @param $key + * The key for our current form element, if any. + * @param array $defaults + * The default settings for the file transfer backend we're operating on. + * @return + * Nothing, this function just sets $element['#default_value'] if needed. + */ +function _authorize_filetransfer_connection_settings_set_defaults(&$element, $key, array $defaults) { + // If we're operating on a form element which isn't a fieldset, and we have + // a default setting saved, stash it in #default_value. + if (!empty($key) && isset($defaults[$key]) && isset($element['#type']) && $element['#type'] != 'fieldset') { + $element['#default_value'] = $defaults[$key]; + } + // Now, we walk through all the child elements, and recursively invoke + // ourself on each one. Since the $defaults settings array can be nested + // (because of #tree, any values inside fieldsets will be nested), if + // there's a subarray of settings for the form key we're currently + // processing, pass in that subarray to the recursive call. Otherwise, just + // pass on the whole $defaults array. + foreach (element_children($element) as $child_key) { + _authorize_filetransfer_connection_settings_set_defaults($element[$child_key], $child_key, ((isset($defaults[$key]) && is_array($defaults[$key])) ? $defaults[$key] : $defaults)); + } +} + +/** * Validate callback for the filetransfer authorization form. * * @see authorize_filetransfer_form() @@ -235,9 +292,16 @@ function authorize_run_operation($filetransfer) { */ function authorize_get_filetransfer($backend, $settings = array()) { $filetransfer = FALSE; - if (!empty($_SESSION['authorize_filetransfer_backends'][$backend])) { - $filetransfer = call_user_func_array(array($_SESSION['authorize_filetransfer_backends'][$backend]['class'], 'factory'), array(DRUPAL_ROOT, $settings)); + if (!empty($_SESSION['authorize_filetransfer_info'][$backend])) { + $backend_info = $_SESSION['authorize_filetransfer_info'][$backend]; + if (!empty($backend_info['file'])) { + $file = $backend_info['file path'] . '/' . $backend_info['file']; + require_once $file; + } + if (class_exists($backend_info['class'])) { + $jail = DRUPAL_ROOT; + $filetransfer = $backend_info['class']::factory($jail, $settings); + } } return $filetransfer; } - diff --git includes/common.inc includes/common.inc index 651eb6a..a507acf 100644 --- includes/common.inc +++ includes/common.inc @@ -7639,3 +7639,39 @@ function drupal_get_updaters() { } return $updaters; } + +/** + * Drupal FileTransfer registry. + * + * @return + * Returns the Drupal FileTransfer class registry. + * + * @see FileTransfer + * @see hook_filetransfer_info() + * @see hook_filetransfer_info_alter() + */ +function drupal_get_filetransfer_info() { + $info = &drupal_static(__FUNCTION__); + if (!isset($info)) { + // Since we have to manually set the 'form path' default for each + // module separately, we can't use module_invoke_all(). + $info = array(); + foreach (module_implements('filetransfer_info') as $module) { + $function = $module . '_filetransfer_info'; + if (function_exists($function)) { + $result = $function(); + if (isset($result) && is_array($result)) { + foreach ($result as &$values) { + if (empty($values['file path'])) { + $values['file path'] = drupal_get_path('module', $module); + } + } + $info = array_merge_recursive($info, $result); + } + } + } + drupal_alter('filetransfer_info', $info); + uasort($info, 'drupal_sort_weight'); + } + return $info; +} diff --git includes/filetransfer/filetransfer.inc includes/filetransfer/filetransfer.inc index 7849ddb..04ed35e 100644 --- includes/filetransfer/filetransfer.inc +++ includes/filetransfer/filetransfer.inc @@ -313,6 +313,42 @@ abstract class FileTransfer { $this->chroot = $this->findChroot(); $this->jail = $this->fixRemotePath($this->jail); } + + /** + * Returns a form to collect connection settings credentials. + * + * Implementing classes can either extend this form with fields collecting the + * specific information they need, or override it entirely. + */ + public function getSettingsForm() { + $form['username'] = array( + '#type' => 'textfield', + '#title' => t('Username'), + ); + $form['password'] = array( + '#type' => 'password', + '#title' => t('Password'), + '#description' => t('Your password is not saved in the database and is only used to establish a connection.'), + ); + $form['advanced'] = array( + '#type' => 'fieldset', + '#title' => t('Advanced settings'), + '#collapsible' => TRUE, + '#collapsed' => TRUE, + ); + $form['advanced']['hostname'] = array( + '#type' => 'textfield', + '#title' => t('Host'), + '#default_value' => 'localhost', + '#description' => t('The connection will be created between your web server and the machine hosting the web server files. In the vast majority of cases, this will be the same machine, and "localhost" is correct.'), + ); + $form['advanced']['port'] = array( + '#type' => 'textfield', + '#title' => t('Port'), + '#default_value' => NULL, + ); + return $form; + } } /** diff --git includes/filetransfer/ftp.inc includes/filetransfer/ftp.inc index 6258f76..a850f91 100644 --- includes/filetransfer/ftp.inc +++ includes/filetransfer/ftp.inc @@ -24,6 +24,8 @@ abstract class FileTransferFTP extends FileTransfer { * options. If the FTP PHP extension is available, use it. */ static function factory($jail, $settings) { + $settings['username'] = empty($settings['username']) ? '' : $settings['username']; + $settings['password'] = empty($settings['password']) ? '' : $settings['password']; $settings['hostname'] = empty($settings['hostname']) ? 'localhost' : $settings['hostname']; $settings['port'] = empty($settings['port']) ? 21 : $settings['port']; @@ -36,6 +38,15 @@ abstract class FileTransferFTP extends FileTransfer { return new $class($jail, $settings['username'], $settings['password'], $settings['hostname'], $settings['port']); } + + /** + * Returns the form to configure the FileTransfer class for FTP. + */ + public function getSettingsForm() { + $form = parent::getSettingsForm(); + $form['advanced']['port']['#default_value'] = 21; + return $form; + } } class FileTransferFTPExtension extends FileTransferFTP implements FileTransferChmodInterface { diff --git includes/filetransfer/ssh.inc includes/filetransfer/ssh.inc index 64fea3a..e8893b1 100644 --- includes/filetransfer/ssh.inc +++ includes/filetransfer/ssh.inc @@ -25,6 +25,8 @@ class FileTransferSSH extends FileTransfer implements FileTransferChmodInterface } static function factory($jail, $settings) { + $settings['username'] = empty($settings['hostname']) ? '' : $settings['username']; + $settings['password'] = empty($settings['password']) ? '' : $settings['password']; $settings['hostname'] = empty($settings['hostname']) ? 'localhost' : $settings['hostname']; $settings['port'] = empty($settings['port']) ? 22 : $settings['port']; return new FileTransferSSH($jail, $settings['username'], $settings['password'], $settings['hostname'], $settings['port']); @@ -95,4 +97,13 @@ class FileTransferSSH extends FileTransfer implements FileTransferChmodInterface throw new FileTransferException('Cannot change permissions of @path.', NULL, array('@path' => $path)); } } + + /** + * Returns the form to configure the FileTransfer class for SSH. + */ + public function getSettingsForm() { + $form = parent::getSettingsForm(); + $form['advanced']['port']['#default_value'] = 22; + return $form; + } } diff --git modules/simpletest/tests/system_test.module modules/simpletest/tests/system_test.module index f7833c4..9bcfafe 100644 --- modules/simpletest/tests/system_test.module +++ modules/simpletest/tests/system_test.module @@ -16,6 +16,12 @@ function system_test_menu() { 'access callback' => TRUE, 'type' => MENU_CALLBACK, ); + $items['system-test/authorize-init/%'] = array( + 'page callback' => 'system_test_authorize_init_page', + 'page arguments' => array(2), + 'access arguments' => array('administer software updates'), + 'type' => MENU_CALLBACK, + ); $items['system-test/redirect/%'] = array( 'title' => 'Redirect', 'page callback' => 'system_test_redirect', @@ -311,3 +317,45 @@ function _system_test_second_shutdown_function($arg1, $arg2) { throw new Exception('Drupal is awesome.'); } +/** + * Implements hook_filetransfer_info(). + */ +function system_test_filetransfer_info() { + return array( + 'system_test' => array( + 'title' => t('System Test FileTransfer'), + 'file' => 'system_test.module', // Should be a .inc, but for test, ok. + 'class' => 'SystemTestFileTransfer', + 'weight' => -10, + ), + ); +} + +/** + * Mock FileTransfer object to test the settings form functionality. + */ +class SystemTestFileTransfer { + public static function factory() { + return new SystemTestFileTransfer; + } + + public function getSettingsForm() { + $form = array(); + $form['system_test_username'] = array( + '#type' => 'textfield', + '#title' => t('System Test Username'), + ); + return $form; + } +} + +/** + * Page callback to initialize authorize.php during testing. + * + * @see system_authorized_init(). + */ +function system_test_authorize_init_page($page_title) { + $authorize_url = $GLOBALS['base_url'] . '/authorize.php'; + system_authorized_init('system_test_authorize_run', drupal_get_path('module', 'system_test') . '/system_test.module', array(), $page_title); + drupal_goto($authorize_url); +} diff --git modules/system/system.api.php modules/system/system.api.php index f7ab69e..0327f6d 100644 --- modules/system/system.api.php +++ modules/system/system.api.php @@ -4272,52 +4272,6 @@ function hook_countries_alter(&$countries) { } /** - * Provide information on available file transfer backends. - * - * File transfer backends are used by modules to transfer files from remote - * locations to Drupal sites. For instance, update.module uses a file transfer - * backend to download new versions of modules and themes from drupal.org. - * - * @return - * An associative array of information about the file transfer backend(s). - * being provided. This array can contain the following keys: - * - title: Title of the backend to be shown to the end user. - * - class: Name of the PHP class which implements this backend. - * - settings_form: An optional callback function that provides additional - * configuration information required by this backend (for instance a port - * number.) - * - weight: Controls what order the backends are presented to the user. - * - * @see authorize.php - * @see FileTransfer - */ -function hook_filetransfer_backends() { - $backends = array(); - - // This is the default, will be available on most systems. - if (function_exists('ftp_connect')) { - $backends['ftp'] = array( - 'title' => t('FTP'), - 'class' => 'FileTransferFTP', - 'settings_form' => 'system_filetransfer_backend_form_ftp', - 'weight' => 0, - ); - } - - // SSH2 lib connection is only available if the proper PHP extension is - // installed. - if (function_exists('ssh2_connect')) { - $backends['ssh'] = array( - 'title' => t('SSH'), - 'class' => 'FileTransferSSH', - 'settings_form' => 'system_filetransfer_backend_form_ssh', - 'weight' => 20, - ); - } - return $backends; -} - -/** * Control site status before menu dispatching. * * The hook is called after checking whether the site is offline but before @@ -4343,5 +4297,68 @@ function hook_menu_site_status_alter(&$menu_site_status, $path) { } /** + * Register information about FileTransfer classes provided by a module. + * + * The FileTransfer class allows transfering files over a specific type of + * connection. Core provides classes for FTP and SSH. Contributed modules are + * free to extend the FileTransfer base class to add other connection types, + * and if these classes are registered via hook_filetransfer_info(), those + * connection types will be available to site administrators using the Update + * manager when they are redirected to the authorize.php script to authorize + * the file operations. + * + * @return array + * Nested array of information about FileTransfer classes. Each key is a + * FileTransfer type (not human readable, used for form elements and + * variable names, etc), and the values are subarrays that define properties + * of that type. The keys in each subarray are: + * - 'title': Required. The human-readable name of the connection type. + * - 'class': Required. The name of the FileTransfer class. The constructor + * will always be passed the full path to the root of the site that should + * be used to restrict where file transfer operations can occur (the $jail) + * and an array of settings values returned by the settings form. + * - 'file': Required. The include file containing the FileTransfer class. + * This should be a separate .inc file, not just the .module file, so that + * the minimum possible code is loaded when authorize.php is running. + * - 'file path': Optional. The directory (relative to the Drupal root) + * where the include file lives. If not defined, defaults to the base + * directory of the module implementing the hook. + * - 'weight': Optional. Integer weight used for sorting connection types on + * the authorize.php form. + * + * @see FileTransfer + * @see authorize.php + * @see hook_filetransfer_info_alter() + * @see drupal_get_filetransfer_info() + */ +function hook_filetransfer_info() { + $info['sftp'] = array( + 'title' => t('SFTP (Secure FTP)'), + 'file' => 'sftp.filetransfer.inc', + 'class' => 'FileTransferSFTP', + 'weight' => 10, + ); + return $info; +} + +/** + * Alter the FileTransfer class registry. + * + * @param array $filetransfer_info + * Reference to a nested array containing information about the FileTransfer + * class registry. + * + * @see hook_filetransfer_info() + */ +function hook_filetransfer_info_alter(&$filetransfer_info) { + if (variable_get('paranoia', FALSE)) { + // Remove the FTP option entirely. + unset($filetransfer_info['ftp']); + // Make sure the SSH option is listed first. + $filetransfer_info['ssh']['weight'] = -10; + } +} + +/** * @} End of "addtogroup hooks". */ diff --git modules/system/system.module modules/system/system.module index 0db5752..f0a3230 100644 --- modules/system/system.module +++ modules/system/system.module @@ -1626,7 +1626,7 @@ function _system_themes_access($theme) { * * @see authorize.php * @see FileTransfer - * @see hook_filetransfer_backends() + * @see hook_filetransfer_info() */ /** @@ -1657,7 +1657,7 @@ function system_authorized_init($callback, $file, $arguments = array(), $page_ti // 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. - $_SESSION['authorize_filetransfer_backends'] = module_invoke_all('filetransfer_backends'); + $_SESSION['authorize_filetransfer_info'] = drupal_get_filetransfer_info(); // Now, define the callback to invoke. $_SESSION['authorize_operation'] = array( @@ -1732,9 +1732,9 @@ function system_updater_info() { } /** - * Implements hook_filetransfer_backends(). + * Implements hook_filetransfer_info(). */ -function system_filetransfer_backends() { +function system_filetransfer_info() { $backends = array(); // This is the default, will be available on most systems. @@ -1742,7 +1742,8 @@ function system_filetransfer_backends() { $backends['ftp'] = array( 'title' => t('FTP'), 'class' => 'FileTransferFTP', - 'settings_form' => 'system_filetransfer_backend_form_ftp', + 'file' => 'ftp.inc', + 'file path' => 'includes/filetransfer', 'weight' => 0, ); } @@ -1753,7 +1754,8 @@ function system_filetransfer_backends() { $backends['ssh'] = array( 'title' => t('SSH'), 'class' => 'FileTransferSSH', - 'settings_form' => 'system_filetransfer_backend_form_ssh', + 'file' => 'ssh.inc', + 'file path' => 'includes/filetransfer', 'weight' => 20, ); } @@ -1761,78 +1763,6 @@ function system_filetransfer_backends() { } /** - * Helper function to return a form for configuring a filetransfer backend. - * - * @param string $filetransfer_backend_name - * The name of the backend to return a form for. - * - * @param string $defaults - * An associative array of settings to pre-populate the form with. - */ -function system_get_filetransfer_settings_form($filetransfer_backend_name, $defaults) { - $available_backends = module_invoke_all('filetransfer_backends'); - $form = call_user_func($available_backends[$filetransfer_backend_name]['settings_form']); - - foreach ($form as $name => &$element) { - if (isset($defaults[$name])) { - $element['#default_value'] = $defaults[$name]; - } - } - return $form; -} - -/** - * Returns the form to configure the filetransfer class for FTP - */ -function system_filetransfer_backend_form_ftp() { - $form = _system_filetransfer_backend_form_common(); - $form['advanced']['port']['#default_value'] = 21; - return $form; -} - -/** - * Returns the form to configure the filetransfer class for SSH - */ -function system_filetransfer_backend_form_ssh() { - $form = _system_filetransfer_backend_form_common(); - $form['advanced']['port']['#default_value'] = 22; - return $form; -} - -/** - * Helper function because SSH and FTP backends share the same elements - */ -function _system_filetransfer_backend_form_common() { - $form['username'] = array( - '#type' => 'textfield', - '#title' => t('Username'), - ); - $form['password'] = array( - '#type' => 'password', - '#title' => t('Password'), - '#description' => t('Your password is not saved in the database and is only used to establish a connection.'), - ); - $form['advanced'] = array( - '#type' => 'fieldset', - '#title' => t('Advanced settings'), - '#collapsible' => TRUE, - '#collapsed' => TRUE, - ); - $form['advanced']['hostname'] = array( - '#type' => 'textfield', - '#title' => t('Host'), - '#default_value' => 'localhost', - '#description' => t('The connection will be created between your web server and the machine hosting the web server files. In the vast majority of cases, this will be the same machine, and "localhost" is correct.'), - ); - $form['advanced']['port'] = array( - '#type' => 'textfield', - '#title' => t('Port'), - '#default_value' => NULL, - ); - return $form; -} - -/** * Implements hook_init(). */ function system_init() { diff --git modules/system/system.test modules/system/system.test index bc542b8..0b59588 100644 --- modules/system/system.test +++ modules/system/system.test @@ -2092,3 +2092,56 @@ class SystemAdminTestCase extends DrupalWebTestCase { $this->assertTrue($this->cookies['Drupal.visitor.admin_compact_mode']['value'], t('Compact mode persists on new requests.')); } } + +/** + * Tests authorize.php and related hooks. + */ +class SystemAuthorizeCase extends DrupalWebTestCase { + public static function getInfo() { + return array( + 'name' => 'Authorize API', + 'description' => 'Tests the authorize.php script and related API.', + 'group' => 'System', + ); + } + + function setUp() { + parent::setUp(array('system_test')); + + variable_set('allow_authorize_operations', TRUE); + + // Create an administrator user. + $this->admin_user = $this->drupalCreateUser(array('administer software updates')); + $this->drupalLogin($this->admin_user); + } + + /** + * Helper function to initialize authorize.php and load it via drupalGet(). + * + * Initializing authorize.php needs to happen in the child Drupal + * installation, not the parent. So, we visit a menu callback provided by + * system_test.module which calls system_authorized_init() to initialize the + * $_SESSION inside the test site, not the framework site. This callback + * redirects to authorize.php when it's done initializing. + * + * @see system_authorized_init(). + */ + function drupalGetAuthorizePHP($page_title = 'system-test-auth') { + $this->drupalGet('system-test/authorize-init/' . $page_title); + } + + /** + * Tests the FileTransfer hooks + */ + function testFileTransferHooks() { + $page_title = $this->randomName(16); + $this->drupalGetAuthorizePHP($page_title); + $this->assertTitle(strtr('@title | Drupal', array('@title' => $page_title)), 'authorize.php page title is correct.'); + $this->assertNoText('It appears you have reached this page in error.'); + $this->assertText('To continue, provide your server connection details'); + // Make sure we see the new connection method added by system_test. + $this->assertRaw('System Test FileTransfer'); + // Make sure the settings form callback works. + $this->assertText('System Test Username'); + } +}