Commit a9666b4a authored by catch's avatar catch

Issue #842620 by alexpott, hansfn, BondD, jurjenn, Skin, xjm, hitesh-jain,...

Issue #842620 by alexpott, hansfn, BondD, jurjenn, Skin, xjm, hitesh-jain, solidwebcode: Update manager can't install modules using FTP due broken FileTransferAuthorizeForm
parent fe6b6861
......@@ -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 @@ function authorize_access_allowed(Request $request) {
}
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.
......
......@@ -64,9 +64,8 @@ public function buildForm(array $form, FormStateInterface $form_state) {
}
// 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 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
// 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 @@ protected function getFiletransfer($backend, $settings = array()) {
* @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 @@ protected function runOperation($filetransfer) {
$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']));
}
......
......@@ -219,12 +219,17 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
'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));
......
<?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');
}
}
......@@ -2,19 +2,27 @@
namespace Drupal\update_test;
use Drupal\Core\FileTransfer\Local;
/**
* Mocks a FileTransfer object to test the settings form functionality.
* 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 MockFileTransfer {
class TestFileTransferWithSettingsForm extends Local {
/**
* Returns a Drupal\update_test\MockFileTransfer object.
* Returns a Drupal\update_test\TestFileTransferWithSettingsForm object.
*
* @return \Drupal\update_test\MockFileTransfer
* A new Drupal\update_test\MockFileTransfer object.
* @return \Drupal\update_test\TestFileTransferWithSettingsForm
* A new Drupal\update_test\TestFileTransferWithSettingsForm object.
*/
public static function factory() {
return new FileTransfer();
public static function factory($jail, $settings) {
return new static($jail);
}
/**
......
......@@ -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,
),
);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment