From 866db2e78b45c1b2611a8409e9427d02e007bee4 Mon Sep 17 00:00:00 2001 From: lucashedding <lucashedding@1463982.no-reply.drupal.org> Date: Fri, 10 May 2019 14:43:55 -0500 Subject: [PATCH] Issue #3052541 by heddn, mbaynton, catch: Checker: File owner and php script user are different --- automatic_updates.services.yml | 6 ++ src/ReadinessChecker/FileOwnership.php | 60 +++++++++++++++++++ .../ReadinessChecker/FileOwnershipTest.php | 57 ++++++++++++++++++ .../ReadinessChecker/ModifiedCodeTest.php | 2 - 4 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 src/ReadinessChecker/FileOwnership.php create mode 100644 tests/src/Kernel/ReadinessChecker/FileOwnershipTest.php diff --git a/automatic_updates.services.yml b/automatic_updates.services.yml index f8cdb18bb3..be10d60e87 100644 --- a/automatic_updates.services.yml +++ b/automatic_updates.services.yml @@ -55,3 +55,9 @@ services: - '@automatic_updates.drupal_finder' tags: - { name: readiness_checker, category: warning} + automatic_updates.file_ownership: + class: Drupal\automatic_updates\ReadinessChecker\FileOwnership + arguments: + - '@automatic_updates.drupal_finder' + tags: + - { name: readiness_checker, category: warning} diff --git a/src/ReadinessChecker/FileOwnership.php b/src/ReadinessChecker/FileOwnership.php new file mode 100644 index 0000000000..002a525e49 --- /dev/null +++ b/src/ReadinessChecker/FileOwnership.php @@ -0,0 +1,60 @@ +<?php + +namespace Drupal\automatic_updates\ReadinessChecker; + +use Drupal\Core\StringTranslation\StringTranslationTrait; +use DrupalFinder\DrupalFinder; + +/** + * File ownership checker. + */ +class FileOwnership extends Filesystem { + use StringTranslationTrait; + + /** + * FileOwnership constructor. + * + * @param \DrupalFinder\DrupalFinder $drupal_finder + * The Drupal finder. + */ + public function __construct(DrupalFinder $drupal_finder) { + $this->drupalFinder = $drupal_finder; + } + + /** + * {@inheritdoc} + */ + protected function doCheck() { + $file_path = $this->getRootPath() . '/core/core.api.php'; + return $this->ownerIsScriptUser($file_path); + } + + /** + * Check if file is owned by the same user as which is running the script. + * + * Helps identify scenarios when the check is run by web user and the files + * are owned by a non-web user. + * + * @param string $file_path + * The file path to check. + * + * @return array + * An array of translatable strings if there are file ownership issues. + */ + protected function ownerIsScriptUser($file_path) { + $messages = []; + if (function_exists('posix_getuid')) { + $file_owner_uid = fileowner($file_path); + $script_uid = posix_getuid(); + if ($file_owner_uid !== $script_uid) { + $messages[] = $this->t('Files are owned by uid "@owner" but PHP is running as uid "@actual". The file owner and PHP user should be the same during an update.', [ + '@owner' => $file_owner_uid, + '@file' => $file_path, + '@actual' => $script_uid, + ]); + } + } + return $messages; + } + +} diff --git a/tests/src/Kernel/ReadinessChecker/FileOwnershipTest.php b/tests/src/Kernel/ReadinessChecker/FileOwnershipTest.php new file mode 100644 index 0000000000..e5cfda0da9 --- /dev/null +++ b/tests/src/Kernel/ReadinessChecker/FileOwnershipTest.php @@ -0,0 +1,57 @@ +<?php + +namespace Drupal\Tests\automatic_updates\Kernel\ReadinessChecker; + +use Drupal\automatic_updates\ReadinessChecker\FileOwnership; +use Drupal\KernelTests\KernelTestBase; +use org\bovigo\vfs\vfsStream; + +/** + * Tests modified code readiness checking. + * + * @group automatic_updates + */ +class FileOwnershipTest extends KernelTestBase { + + /** + * {@inheritdoc} + */ + public static $modules = [ + 'automatic_updates', + ]; + + /** + * Tests the functionality of modified code readiness checks. + */ + public function testFileOwnership() { + // No ownership problems. + $file_ownership = new FileOwnership($this->container->get('automatic_updates.drupal_finder')); + $messages = $file_ownership->run(); + $this->assertEmpty($messages); + + // Ownership problems. + $file_ownership = new TestFileOwnership($this->container->get('automatic_updates.drupal_finder')); + $messages = $file_ownership->run(); + $this->assertCount(1, $messages); + $this->assertStringStartsWith('Files are owned by uid "23"', (string) $messages[0]); + $this->assertStringEndsWith('The file owner and PHP user should be the same during an update.', (string) $messages[0]); + } + +} + +/** + * Class TestFileOwnership. + */ +class TestFileOwnership extends FileOwnership { + + /** + * {@inheritdoc} + */ + protected function doCheck() { + $file_stream = vfsStream::setup('core', '755', ['core.api.php' => 'contents']); + $file = $file_stream->getChild('core.api.php'); + $file->chown(23)->chgrp(23); + return $this->ownerIsScriptUser($file->url()); + } + +} diff --git a/tests/src/Kernel/ReadinessChecker/ModifiedCodeTest.php b/tests/src/Kernel/ReadinessChecker/ModifiedCodeTest.php index fd6c86f13c..0606aa64ad 100644 --- a/tests/src/Kernel/ReadinessChecker/ModifiedCodeTest.php +++ b/tests/src/Kernel/ReadinessChecker/ModifiedCodeTest.php @@ -3,7 +3,6 @@ namespace Drupal\Tests\automatic_updates\Kernel\ReadinessChecker; use Drupal\automatic_updates\ReadinessChecker\ModifiedCode; -use Drupal\Core\StringTranslation\StringTranslationTrait; use Drupal\KernelTests\KernelTestBase; /** @@ -12,7 +11,6 @@ use Drupal\KernelTests\KernelTestBase; * @group automatic_updates */ class ModifiedCodeTest extends KernelTestBase { - use StringTranslationTrait; /** * {@inheritdoc} -- GitLab