summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNathaniel Catchpole2016-05-16 12:05:48 (GMT)
committerNathaniel Catchpole2016-05-16 12:05:48 (GMT)
commita9666b4ae85300ff9106144421a6582732c8f6cb (patch)
tree18826e4fc8dcd2ff9ec9ca578acdf6b96a99c4c2
parentfe6b686153f5e31b974118d6887faf79cfa7e94e (diff)
Issue #842620 by alexpott, hansfn, BondD, jurjenn, Skin, xjm, hitesh-jain, solidwebcode: Update manager can't install modules using FTP due broken FileTransferAuthorizeForm
-rw-r--r--core/authorize.php9
-rw-r--r--core/lib/Drupal/Core/FileTransfer/Form/FileTransferAuthorizeForm.php31
-rw-r--r--core/modules/update/src/Form/UpdateManagerInstall.php7
-rw-r--r--core/modules/update/src/Tests/FileTransferAuthorizeFormTest.php58
-rw-r--r--core/modules/update/tests/modules/update_test/src/MockFileTransfer.php32
-rw-r--r--core/modules/update/tests/modules/update_test/src/TestFileTransferWithSettingsForm.php40
-rw-r--r--core/modules/update/tests/modules/update_test/update_test.module6
7 files changed, 119 insertions, 64 deletions
diff --git a/core/authorize.php b/core/authorize.php
index 4752799..073770a 100644
--- a/core/authorize.php
+++ b/core/authorize.php
@@ -21,6 +21,7 @@
*/
use Drupal\Core\DrupalKernel;
+use Drupal\Core\Form\EnforcedResponseException;
use Drupal\Core\Url;
use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface;
use Symfony\Component\HttpFoundation\Request;
@@ -169,7 +170,13 @@ if ($is_allowed) {
}
elseif (!$batch = batch_get()) {
// We have a batch to process, show the filetransfer form.
- $content = \Drupal::formBuilder()->getForm('Drupal\Core\FileTransfer\Form\FileTransferAuthorizeForm');
+ try {
+ $content = \Drupal::formBuilder()->getForm('Drupal\Core\FileTransfer\Form\FileTransferAuthorizeForm');
+ }
+ catch (EnforcedResponseException $e) {
+ $e->getResponse()->send();
+ exit;
+ }
}
}
// We defer the display of messages until all operations are done.
diff --git a/core/lib/Drupal/Core/FileTransfer/Form/FileTransferAuthorizeForm.php b/core/lib/Drupal/Core/FileTransfer/Form/FileTransferAuthorizeForm.php
index cb4cae2..647463a 100644
--- a/core/lib/Drupal/Core/FileTransfer/Form/FileTransferAuthorizeForm.php
+++ b/core/lib/Drupal/Core/FileTransfer/Form/FileTransferAuthorizeForm.php
@@ -64,9 +64,8 @@ class FileTransferAuthorizeForm extends FormBase {
}
// Decide on a default backend.
- if ($authorize_filetransfer_default = $form_state->getValue(array('connection_settings', 'authorize_filetransfer_default')));
- elseif ($authorize_filetransfer_default = $this->config('system.authorize')->get('filetransfer_default'));
- else {
+ $authorize_filetransfer_default = $form_state->getValue(array('connection_settings', 'authorize_filetransfer_default'));
+ if (!$authorize_filetransfer_default) {
$authorize_filetransfer_default = key($available_backends);
}
@@ -198,27 +197,6 @@ class FileTransferAuthorizeForm extends FormBase {
// likely) be called during the installation process, before the
// database is set up.
try {
- $connection_settings = array();
- foreach ($form_connection_settings[$filetransfer_backend] as $key => $value) {
- // We do *not* want to store passwords in the database, unless the
- // backend explicitly says so via the magic #filetransfer_save form
- // property. Otherwise, we store everything that's not explicitly
- // marked with #filetransfer_save set to FALSE.
- if (!isset($form['connection_settings'][$filetransfer_backend][$key]['#filetransfer_save'])) {
- if ($form['connection_settings'][$filetransfer_backend][$key]['#type'] != 'password') {
- $connection_settings[$key] = $value;
- }
- }
- // The attribute is defined, so only save if set to TRUE.
- elseif ($form['connection_settings'][$filetransfer_backend][$key]['#filetransfer_save']) {
- $connection_settings[$key] = $value;
- }
- }
- // Set this one as the default authorize method.
- $this->config('system.authorize')->set('filetransfer_default', $filetransfer_backend);
- // Save the connection settings minus the password.
- $this->config('system.authorize')->set('filetransfer_connection_settings_' . $filetransfer_backend, $connection_settings);
-
$filetransfer = $this->getFiletransfer($filetransfer_backend, $form_connection_settings[$filetransfer_backend]);
// Now run the operation.
@@ -280,8 +258,7 @@ class FileTransferAuthorizeForm extends FormBase {
* @see hook_filetransfer_backends()
*/
protected function addConnectionSettings($backend) {
- $auth_connection_config = $this->config('system.authorize')->get('filetransfer_connection_settings_' . $backend);
- $defaults = $auth_connection_config ? $auth_connection_config : array();
+ $defaults = array();
$form = array();
// Create an instance of the file transfer class to get its settings form.
@@ -342,7 +319,7 @@ class FileTransferAuthorizeForm extends FormBase {
$operation = $_SESSION['authorize_operation'];
unset($_SESSION['authorize_operation']);
- require_once $this->root . '/' . $operation['file'];
+ require_once $operation['file'];
return call_user_func_array($operation['callback'], array_merge(array($filetransfer), $operation['arguments']));
}
diff --git a/core/modules/update/src/Form/UpdateManagerInstall.php b/core/modules/update/src/Form/UpdateManagerInstall.php
index f06b53a..3bc2c78 100644
--- a/core/modules/update/src/Form/UpdateManagerInstall.php
+++ b/core/modules/update/src/Form/UpdateManagerInstall.php
@@ -219,12 +219,17 @@ class UpdateManagerInstall extends FormBase {
'local_url' => $project_real_location,
);
+ // This process is inherently difficult to test therefore use a state flag.
+ $test_authorize = FALSE;
+ if (drupal_valid_test_ua()) {
+ $test_authorize = \Drupal::state()->get('test_uploaders_via_prompt', FALSE);
+ }
// If the owner of the directory we extracted is the same as the owner of
// our configuration directory (e.g. sites/default) where we're trying to
// install the code, there's no need to prompt for FTP/SSH credentials.
// Instead, we instantiate a Drupal\Core\FileTransfer\Local and invoke
// update_authorize_run_install() directly.
- if (fileowner($project_real_location) == fileowner($this->sitePath)) {
+ if (fileowner($project_real_location) == fileowner($this->sitePath) && !$test_authorize) {
$this->moduleHandler->loadInclude('update', 'inc', 'update.authorize');
$filetransfer = new Local($this->root);
$response = call_user_func_array('update_authorize_run_install', array_merge(array($filetransfer), $arguments));
diff --git a/core/modules/update/src/Tests/FileTransferAuthorizeFormTest.php b/core/modules/update/src/Tests/FileTransferAuthorizeFormTest.php
new file mode 100644
index 0000000..0d0989b
--- /dev/null
+++ b/core/modules/update/src/Tests/FileTransferAuthorizeFormTest.php
@@ -0,0 +1,58 @@
+<?php
+
+namespace Drupal\update\Tests;
+
+/**
+ * Tests the Update Manager module upload via authorize.php functionality.
+ *
+ * @group update
+ */
+class FileTransferAuthorizeFormTest extends UpdateTestBase {
+
+ /**
+ * Modules to enable.
+ *
+ * @var array
+ */
+ public static $modules = array('update', 'update_test');
+
+ protected function setUp() {
+ parent::setUp();
+ $admin_user = $this->drupalCreateUser(array('administer modules', 'administer software updates', 'administer site configuration'));
+ $this->drupalLogin($admin_user);
+
+ // Create a local cache so the module is not downloaded from drupal.org.
+ $cache_directory = _update_manager_cache_directory(TRUE);
+ $validArchiveFile = __DIR__ . '/../../tests/update_test_new_module/8.x-1.0/update_test_new_module.tar.gz';
+ copy($validArchiveFile, $cache_directory . '/update_test_new_module.tar.gz');
+ }
+
+ /**
+ * Tests the Update Manager module upload via authorize.php functionality.
+ */
+ public function testViaAuthorize() {
+ // Ensure the that we can select which file transfer backend to use.
+ \Drupal::state()->set('test_uploaders_via_prompt', TRUE);
+
+ // Ensure the module does not already exist.
+ $this->drupalGet('admin/modules');
+ $this->assertNoText('Update test new module');
+
+ $edit = [
+ // This project has been cached in the test's setUp() method.
+ 'project_url' => 'https://ftp.drupal.org/files/projects/update_test_new_module.tar.gz',
+ ];
+ $this->drupalPostForm('admin/modules/install', $edit, t('Install'));
+ $edit = [
+ 'connection_settings[authorize_filetransfer_default]' => 'system_test',
+ 'connection_settings[system_test][update_test_username]' => $this->randomMachineName(),
+ ];
+ $this->drupalPostForm(NULL, $edit, t('Continue'));
+ $this->assertText(t('Installation was completed successfully.'));
+
+ // Ensure the module is available to install.
+ $this->drupalGet('admin/modules');
+ $this->assertText('Update test new module');
+ }
+
+}
diff --git a/core/modules/update/tests/modules/update_test/src/MockFileTransfer.php b/core/modules/update/tests/modules/update_test/src/MockFileTransfer.php
deleted file mode 100644
index 2194b43..0000000
--- a/core/modules/update/tests/modules/update_test/src/MockFileTransfer.php
+++ /dev/null
@@ -1,32 +0,0 @@
-<?php
-
-namespace Drupal\update_test;
-
-/**
- * Mocks a FileTransfer object to test the settings form functionality.
- */
-class MockFileTransfer {
-
- /**
- * Returns a Drupal\update_test\MockFileTransfer object.
- *
- * @return \Drupal\update_test\MockFileTransfer
- * A new Drupal\update_test\MockFileTransfer object.
- */
- public static function factory() {
- return new FileTransfer();
- }
-
- /**
- * Returns a settings form with a text field to input a username.
- */
- public function getSettingsForm() {
- $form = array();
- $form['update_test_username'] = array(
- '#type' => 'textfield',
- '#title' => t('Update Test Username'),
- );
- return $form;
- }
-
-}
diff --git a/core/modules/update/tests/modules/update_test/src/TestFileTransferWithSettingsForm.php b/core/modules/update/tests/modules/update_test/src/TestFileTransferWithSettingsForm.php
new file mode 100644
index 0000000..5737c09
--- /dev/null
+++ b/core/modules/update/tests/modules/update_test/src/TestFileTransferWithSettingsForm.php
@@ -0,0 +1,40 @@
+<?php
+
+namespace Drupal\update_test;
+
+use Drupal\Core\FileTransfer\Local;
+
+/**
+ * Provides an object to test the settings form functionality.
+ *
+ * This class extends \Drupal\Core\FileTransfer\Local to make module install
+ * testing via \Drupal\Core\FileTransfer\Form\FileTransferAuthorizeForm and
+ * authorize.php possible.
+ *
+ * @see \Drupal\update\Tests\FileTransferAuthorizeFormTest
+ */
+class TestFileTransferWithSettingsForm extends Local {
+
+ /**
+ * Returns a Drupal\update_test\TestFileTransferWithSettingsForm object.
+ *
+ * @return \Drupal\update_test\TestFileTransferWithSettingsForm
+ * A new Drupal\update_test\TestFileTransferWithSettingsForm object.
+ */
+ public static function factory($jail, $settings) {
+ return new static($jail);
+ }
+
+ /**
+ * Returns a settings form with a text field to input a username.
+ */
+ public function getSettingsForm() {
+ $form = array();
+ $form['update_test_username'] = array(
+ '#type' => 'textfield',
+ '#title' => t('Update Test Username'),
+ );
+ return $form;
+ }
+
+}
diff --git a/core/modules/update/tests/modules/update_test/update_test.module b/core/modules/update/tests/modules/update_test/update_test.module
index 04a77e9..90b6cdc 100644
--- a/core/modules/update/tests/modules/update_test/update_test.module
+++ b/core/modules/update/tests/modules/update_test/update_test.module
@@ -59,13 +59,13 @@ function update_test_update_status_alter(&$projects) {
* Implements hook_filetransfer_info().
*/
function update_test_filetransfer_info() {
- // Define a mock file transfer method, to ensure that there will always be
- // at least one method available in the user interface (regardless of the
+ // Define a test file transfer method, to ensure that there will always be at
+ // least one method available in the user interface (regardless of the
// environment in which the update manager tests are run).
return array(
'system_test' => array(
'title' => t('Update Test FileTransfer'),
- 'class' => 'Drupal\update_test\MockFileTransfer',
+ 'class' => 'Drupal\update_test\TestFileTransferWithSettingsForm',
'weight' => -20,
),
);