From bfc6f74a9a921f063d3260e57b552e325ce2cf77 Mon Sep 17 00:00:00 2001 From: Thomas Shone Date: Sun, 1 Jan 2023 15:34:34 +0100 Subject: [PATCH 1/4] Added Passwordless Account check (#84) Accounts without passwords should be flagged to avoid unintended exposure. This goes doubly for accounts that are accessible outside of localhost. We don't currently case too much about what they have access to. We could examine if they only have read access (and whether it's limited to "monitoring" schemas like mysql or performance_schema) but that's a future refinement once we see what hits we get. Also required some refactoring of various bits of code and some hint at the newer structure for data objects (heading towards using readonly classes, using proper enums and named parameters which means this also bumps our requirements up to PHP 8.* as well). Ideally I want to be able to support multiple data sources (database, SQL definition files, etc) without massive refactoring. --- composer.json | 5 +- resources/mysql/sample.sql | 3 + src/Cli/Command/RunCommand.php | 2 + .../Check/Account/PasswordlessAccount.php | 70 ++++++++++ src/Engine/Entity/Account.php | 37 +++-- src/Engine/Entity/Account/User.php | 131 ++++++++++++++++++ src/Engine/Factory.php | 7 +- tests/Engine/BaseTest.php | 7 + .../Check/Account/NotConnectingTest.php | 5 +- .../NotProperlyClosingConnectionsTest.php | 5 +- .../Check/Account/PasswordlessAccountTest.php | 66 +++++++++ .../Check/Query/FunctionsOnIndexTest.php | 8 +- tests/Engine/Entity/AccountTest.php | 6 +- tests/Engine/Entity/DatabaseTest.php | 48 +++++-- 14 files changed, 363 insertions(+), 37 deletions(-) create mode 100644 src/Engine/Check/Account/PasswordlessAccount.php create mode 100644 src/Engine/Entity/Account/User.php create mode 100644 tests/Engine/Check/Account/PasswordlessAccountTest.php diff --git a/composer.json b/composer.json index d78ec15..c39715d 100644 --- a/composer.json +++ b/composer.json @@ -11,11 +11,12 @@ ], "minimum-stability": "stable", "require": { - "php": "^7.4 || ^8.0", + "php": "^8.0", "doctrine/dbal": "^2.10", "symfony/console": "^5.0", "monolog/monolog": "^2.1", - "greenlion/php-sql-parser": "^4.5" + "greenlion/php-sql-parser": "^4.5", + "ext-json": "*" }, "require-dev": { "squizlabs/php_codesniffer": "^3.5", diff --git a/resources/mysql/sample.sql b/resources/mysql/sample.sql index f1d1f8b..7c9769d 100644 --- a/resources/mysql/sample.sql +++ b/resources/mysql/sample.sql @@ -80,3 +80,6 @@ WITH RECURSIVE cte AS ) SELECT i, value FROM cte; + +# Create a passwordless account +CREATE USER IF NOT EXISTS 'localhost_passwordless_user'@'localhost'; \ No newline at end of file diff --git a/src/Cli/Command/RunCommand.php b/src/Cli/Command/RunCommand.php index d8d9cd8..8ad0be4 100644 --- a/src/Cli/Command/RunCommand.php +++ b/src/Cli/Command/RunCommand.php @@ -8,6 +8,7 @@ use Cadfael\Cli\Formatter\Json; use Cadfael\Engine\Check\Account\NotConnecting; use Cadfael\Engine\Check\Account\NotProperlyClosingConnections; +use Cadfael\Engine\Check\Account\PasswordlessAccount; use Cadfael\Engine\Check\Column\CorrectUtf8Encoding; use Cadfael\Engine\Check\Column\LowCardinalityExpensiveStorage; use Cadfael\Engine\Check\Column\ReservedKeywords; @@ -166,6 +167,7 @@ protected function runChecksAgainstSchema( new IndexPrefix(), new UUIDStorage(), new LowCardinalityExpensiveStorage(), + new PasswordlessAccount(), ); if ($load_performance_schema) { diff --git a/src/Engine/Check/Account/PasswordlessAccount.php b/src/Engine/Check/Account/PasswordlessAccount.php new file mode 100644 index 0000000..0a65bd4 --- /dev/null +++ b/src/Engine/Check/Account/PasswordlessAccount.php @@ -0,0 +1,70 @@ +getUser()->authentication_string) { + return new Report( + $this, + $entity, + Report::STATUS_OK + ); + } + + $status = Report::STATUS_WARNING; + $messages = [ + "Passwordless account detected.", + ]; + + // Can this user be accessed from outside the server localhost? + if ($entity->getUser()->host !== 'localhost' && $entity->getUser()->host !== '127.0.0.1') { + $status = Report::STATUS_CRITICAL; + $messages[] = "Account has access from outside the server."; + } + + return new Report( + $this, + $entity, + $status, + $messages + ); + } + + /** + * @codeCoverageIgnore + */ + public function getReferenceUri(): string + { + return 'https://github.com/xsist10/cadfael/wiki/Passwordless-Account'; + } + + /** + * @codeCoverageIgnore + */ + public function getName(): string + { + return 'Passwordless Account'; + } + + /** + * @codeCoverageIgnore + */ + public function getDescription(): string + { + return "Accounts that don't have a password set could be abused."; + } +} diff --git a/src/Engine/Entity/Account.php b/src/Engine/Entity/Account.php index ba17832..4d315bb 100644 --- a/src/Engine/Entity/Account.php +++ b/src/Engine/Entity/Account.php @@ -6,23 +6,32 @@ use Cadfael\Engine\Entity; use Cadfael\Engine\Entity\Account\NotClosedProperly; +use Cadfael\Engine\Entity\Account\User; class Account implements Entity { - protected string $username; - protected string $host; protected int $current_connections = 0; protected int $total_connections = 0; - + protected User $user; protected Database $database; public ?NotClosedProperly $account_not_closed_properly = null; - public function __construct(string $username, string $host) + protected function __construct() + { + } + + public static function withUser(User $user): Account + { + $account = new Account(); + $account->setUser($user); + return $account; + } + + public static function withRaw(string $username, string $host): Account { - $this->username = $username; - $this->host = $host; + return self::withUser(new User($username, $host)); } /** @@ -99,7 +108,7 @@ public function getDatabase(): Database */ public function getHost(): string { - return $this->host; + return $this->user->host; } /** @@ -110,7 +119,7 @@ public function getHost(): string */ public function getName(): string { - return $this->username; + return $this->user->user; } /** @@ -127,11 +136,21 @@ public function setAccountNotClosedProperly(NotClosedProperly $account_not_close */ public function __toString(): string { - return $this->username . '@' . $this->host; + return $this->user->user . '@' . $this->user->host; } public function isVirtual(): bool { return false; } + + public function getUser(): User + { + return $this->user; + } + + public function setUser(User $user): void + { + $this->user = $user; + } } diff --git a/src/Engine/Entity/Account/User.php b/src/Engine/Entity/Account/User.php new file mode 100644 index 0000000..a798001 --- /dev/null +++ b/src/Engine/Entity/Account/User.php @@ -0,0 +1,131 @@ + $payload This is a query from mysql.user + * @return User + */ + public static function createFromUser(array $payload): User + { + return new User( + $payload['User'], + $payload['Host'], + $payload['Select_priv'] === 'Y', + $payload['Insert_priv'] === 'Y', + $payload['Update_priv'] === 'Y', + $payload['Delete_priv'] === 'Y', + $payload['Create_priv'] === 'Y', + $payload['Drop_priv'] === 'Y', + $payload['Reload_priv'] === 'Y', + $payload['Shutdown_priv'] === 'Y', + $payload['Process_priv'] === 'Y', + $payload['File_priv'] === 'Y', + $payload['Grant_priv'] === 'Y', + $payload['References_priv'] === 'Y', + $payload['Index_priv'] === 'Y', + $payload['Alter_priv'] === 'Y', + $payload['Show_db_priv'] === 'Y', + $payload['Super_priv'] === 'Y', + $payload['Create_tmp_table_priv'] === 'Y', + $payload['Lock_tables_priv'] === 'Y', + $payload['Execute_priv'] === 'Y', + $payload['Repl_slave_priv'] === 'Y', + $payload['Repl_client_priv'] === 'Y', + $payload['Create_view_priv'] === 'Y', + $payload['Show_view_priv'] === 'Y', + $payload['Create_routine_priv'] === 'Y', + $payload['Alter_routine_priv'] === 'Y', + $payload['Create_user_priv'] === 'Y', + $payload['Event_priv'] === 'Y', + $payload['Trigger_priv'] === 'Y', + $payload['Create_tablespace_priv'] === 'Y', + $payload['ssl_type'], + $payload['ssl_cipher'], + $payload['x509_issuer'], + $payload['x509_subject'], + (int)$payload['max_questions'], + (int)$payload['max_updates'], + (int)$payload['max_connections'], + (int)$payload['max_user_connections'], + $payload['plugin'], + $payload['authentication_string'], + $payload['password_expired'] === 'Y', + (int)$payload['password_last_changed'], + (int)$payload['password_lifetime'], + $payload['account_locked'] === 'Y', + $payload['Create_role_priv'] === 'Y', + $payload['Drop_role_priv'] === 'Y', + (int)$payload['Password_reuse_history'], + (int)$payload['Password_reuse_time'], + $payload['Password_require_current'] === 'Y', + json_decode($payload['User_attributes'] ?? '{}', true) + ); + } +} diff --git a/src/Engine/Factory.php b/src/Engine/Factory.php index dabe778..f1fd98e 100644 --- a/src/Engine/Factory.php +++ b/src/Engine/Factory.php @@ -6,6 +6,7 @@ use Cadfael\Engine\Entity\Account; use Cadfael\Engine\Entity\Account\NotClosedProperly; +use Cadfael\Engine\Entity\Account\User; use Cadfael\Engine\Entity\Database; use Cadfael\Engine\Entity\Index\Statistics; use Cadfael\Engine\Entity\Query; @@ -355,7 +356,7 @@ public function getAccounts(Connection $connection): array $this->log()->info("Collecting MySQL user accounts."); $query = 'SELECT * FROM mysql.user'; foreach ($connection->fetchAllAssociative($query) as $row) { - $accounts[] = new Account($row['User'], $row['Host']); + $accounts[] = Account::withUser(User::createFromUser($row)); } } return $accounts; @@ -664,7 +665,7 @@ public function buildDatabase(Connection $connection, array $schema_names, bool $accountNotClosedProperly['host'] ); if (!$account) { - $account = new Account( + $account = Account::withRaw( $accountNotClosedProperly['user'], $accountNotClosedProperly['host'] ); @@ -682,7 +683,7 @@ public function buildDatabase(Connection $connection, array $schema_names, bool foreach ($accountConnections as $accountConnection) { $account = $database->getAccount($accountConnection['USER'], $accountConnection['HOST']); if (!$account) { - $account = new Account($accountConnection['USER'], $accountConnection['HOST']); + $account = Account::withRaw($accountConnection['USER'], $accountConnection['HOST']); $database->addAccount($account); } $account->setCurrentConnections((int)$accountConnection['CURRENT_CONNECTIONS']); diff --git a/tests/Engine/BaseTest.php b/tests/Engine/BaseTest.php index cffd03e..1072ef6 100644 --- a/tests/Engine/BaseTest.php +++ b/tests/Engine/BaseTest.php @@ -4,6 +4,8 @@ namespace Cadfael\Tests\Engine; +use Cadfael\Engine\Entity\Account; +use Cadfael\Engine\Entity\Account\User; use Cadfael\Engine\Entity\Database; use Cadfael\Engine\Entity\Schema; use Cadfael\Engine\Entity\Table; @@ -88,4 +90,9 @@ protected function createVirtualTable(array $override = []): Table ) ); } + + protected function createAccount(string $user, string $host): Account + { + return Account::withRaw($user, $host); + } } \ No newline at end of file diff --git a/tests/Engine/Check/Account/NotConnectingTest.php b/tests/Engine/Check/Account/NotConnectingTest.php index 70c4bd8..736b9b8 100644 --- a/tests/Engine/Check/Account/NotConnectingTest.php +++ b/tests/Engine/Check/Account/NotConnectingTest.php @@ -4,7 +4,6 @@ namespace Cadfael\Tests\Engine\Check\Account; use Cadfael\Engine\Check\Account\NotConnecting; -use Cadfael\Engine\Entity\Account; use Cadfael\Engine\Report; use Cadfael\Tests\Engine\BaseTest; @@ -14,10 +13,10 @@ class NotConnectingTest extends BaseTest public function setUp(): void { - $no_connection_account = new Account('no_connections', 'localhost'); + $no_connection_account = $this->createAccount('no_connections', 'localhost'); $no_connection_account->setTotalConnections(0); - $many_connection_account = new Account('many_connections', 'localhost'); + $many_connection_account = $this->createAccount('many_connections', 'localhost'); $many_connection_account->setTotalConnections(100); $this->accounts = [ diff --git a/tests/Engine/Check/Account/NotProperlyClosingConnectionsTest.php b/tests/Engine/Check/Account/NotProperlyClosingConnectionsTest.php index 030da0d..3188686 100644 --- a/tests/Engine/Check/Account/NotProperlyClosingConnectionsTest.php +++ b/tests/Engine/Check/Account/NotProperlyClosingConnectionsTest.php @@ -4,7 +4,6 @@ namespace Cadfael\Tests\Engine\Check\Account; use Cadfael\Engine\Check\Account\NotProperlyClosingConnections; -use Cadfael\Engine\Entity\Account; use Cadfael\Engine\Entity\Account\NotClosedProperly; use Cadfael\Engine\Report; use Cadfael\Tests\Engine\BaseTest; @@ -15,13 +14,13 @@ class NotProperlyClosingConnectionsTest extends BaseTest public function setUp(): void { - $not_closed_properly_account = new Account('not_closed_properly', 'localhost'); + $not_closed_properly_account = $this->createAccount('not_closed_properly', 'localhost'); $not_closed_properly_account->setAccountNotClosedProperly(NotClosedProperly::createFromEventSummary([ 'not_closed' => 1, 'not_closed_perc' => 5 ])); - $closed_properly_account = new Account('closed_properly_account', 'localhost'); + $closed_properly_account = $this->createAccount('closed_properly_account', 'localhost'); $closed_properly_account->setAccountNotClosedProperly(NotClosedProperly::createFromEventSummary([ 'not_closed' => 0, 'not_closed_perc' => 0 diff --git a/tests/Engine/Check/Account/PasswordlessAccountTest.php b/tests/Engine/Check/Account/PasswordlessAccountTest.php new file mode 100644 index 0000000..c9a1962 --- /dev/null +++ b/tests/Engine/Check/Account/PasswordlessAccountTest.php @@ -0,0 +1,66 @@ +createAccount('passwordless_account', 'localhost'); + $passwordless_account_non_local = $this->createAccount('passwordless_account', '%'); + $passworded_account = Account::withUser(new User('passworded_account', 'localhost', authentication_string: "randomJibberish")); + + $this->accounts = [ + $passwordless_account, + $passwordless_account_non_local, + $passworded_account + ]; + } + + public function testSupports() + { + $check = new PasswordlessAccount(); + + foreach ($this->accounts as $account) { + $this->assertTrue( + $check->supports($account), + "Ensure that we care about all accounts." + ); + } + } + + public function testRun() + { + $check = new PasswordlessAccount(); + + $account = $this->accounts[0]; + $this->assertEquals( + Report::STATUS_WARNING, + $check->run($account)->getStatus(), + $account->getName() . "@" . $account->getHost() . " has no password set for a localhost account." + ); + + $account = $this->accounts[1]; + $this->assertEquals( + Report::STATUS_CRITICAL, + $check->run($account)->getStatus(), + $account->getName() . "@" . $account->getHost() . " has no password set for a non-localhost account." + ); + + $account = $this->accounts[2]; + $this->assertEquals( + Report::STATUS_OK, + $check->run($account)->getStatus(), + $account->getName() . "@" . $account->getHost() . " has a password set." + ); + } +} diff --git a/tests/Engine/Check/Query/FunctionsOnIndexTest.php b/tests/Engine/Check/Query/FunctionsOnIndexTest.php index 728f231..e060934 100644 --- a/tests/Engine/Check/Query/FunctionsOnIndexTest.php +++ b/tests/Engine/Check/Query/FunctionsOnIndexTest.php @@ -53,7 +53,13 @@ protected function setUp(): void public function test__moo() { $check = new FunctionsOnIndex(); + $this->assertTrue($check->supports($this->query)); + $report = $check->run($this->query); - $this->assertEquals(Report::STATUS_WARNING, $report->getStatus(), "Query should identify modified INDEX column in WHERE statement."); + $this->assertEquals( + Report::STATUS_WARNING, + $report->getStatus(), + "Query should identify modified INDEX column in WHERE statement." + ); } } diff --git a/tests/Engine/Entity/AccountTest.php b/tests/Engine/Entity/AccountTest.php index 28b1278..1f924c6 100644 --- a/tests/Engine/Entity/AccountTest.php +++ b/tests/Engine/Entity/AccountTest.php @@ -5,15 +5,15 @@ use Cadfael\Engine\Entity\Account; use Cadfael\Engine\Entity\Database; -use PHPUnit\Framework\TestCase; +use Cadfael\Tests\Engine\BaseTest; -class AccountTest extends TestCase +class AccountTest extends BaseTest { protected Account $account; protected function setUp(): void { - $this->account = new Account("root", "localhost"); + $this->account = $this->createAccount("root", "localhost"); $this->account->setDatabase(new Database(null)); } diff --git a/tests/Engine/Entity/DatabaseTest.php b/tests/Engine/Entity/DatabaseTest.php index 95cd102..5eab8da 100644 --- a/tests/Engine/Entity/DatabaseTest.php +++ b/tests/Engine/Entity/DatabaseTest.php @@ -3,13 +3,12 @@ namespace Cadfael\Tests\Engine\Entity; -use Cadfael\Engine\Entity\Account; use Cadfael\Engine\Entity\Database; use Cadfael\Engine\Entity\Schema; +use Cadfael\Tests\Engine\BaseTest; use Doctrine\DBAL\DriverManager; -use PHPUnit\Framework\TestCase; -class DatabaseTest extends TestCase +class DatabaseTest extends BaseTest { protected Database $database; @@ -33,9 +32,9 @@ protected function setUp(): void $this->database = new Database($connection); $this->database->setVariables(self::VARIABLES); - $this->database->addAccount(new Account('root', 'localhost')); - $this->database->addAccount(new Account('bob', 'localhost')); - $this->database->addAccount(new Account('alice', '%')); + $this->database->addAccount($this->createAccount('root', 'localhost')); + $this->database->addAccount($this->createAccount('bob', 'localhost')); + $this->database->addAccount($this->createAccount('alice', '%')); } public function test__getName() @@ -50,24 +49,47 @@ public function test__toString() public function test__getAccount() { - $this->assertEquals(new Account('bob', 'localhost'), $this->database->getAccount('bob', 'localhost'), "Verify that we get the expected Account."); - $this->assertNull($this->database->getAccount('bob', '%'), "Our fuzzy domain should not match for Bob."); - $this->assertEquals(new Account('alice', '%'), $this->database->getAccount('alice', 'localhost'), "Our fuzzy domain should match for Alice."); + $this->assertEquals( + $this->createAccount('bob', 'localhost'), + $this->database->getAccount('bob', 'localhost'), + "Verify that we get the expected Account." + ); + $this->assertNull( + $this->database->getAccount('bob', '%'), + "Our fuzzy domain should not match for Bob." + ); + $this->assertEquals( + $this->createAccount('alice', '%'), + $this->database->getAccount('alice', 'localhost'), + "Our fuzzy domain should match for Alice." + ); } public function test__hasPerformanceSchema() { $this->database->setVariables([ 'performance_schema' => 'ON' ]); - $this->assertTrue($this->database->hasPerformanceSchema(), "Ensure we correctly detect that performance schema is on with a normal flag"); + $this->assertTrue( + $this->database->hasPerformanceSchema(), + "Ensure we correctly detect that performance schema is on with a normal flag" + ); $this->database->setVariables([ 'performance_schema' => 'OFF' ]); - $this->assertFalse($this->database->hasPerformanceSchema(), "Ensure we correctly detect that performance schema is off with a normal flag"); + $this->assertFalse( + $this->database->hasPerformanceSchema(), + "Ensure we correctly detect that performance schema is off with a normal flag" + ); $this->database->setVariables([ 'performance_schema_something' => '1' ]); - $this->assertTrue($this->database->hasPerformanceSchema(), "Ensure we correctly detect that performance schema is on with namespace variable."); + $this->assertTrue( + $this->database->hasPerformanceSchema(), + "Ensure we correctly detect that performance schema is on with namespace variable." + ); $this->database->setVariables([]); - $this->assertFalse($this->database->hasPerformanceSchema(), "Ensure we correctly detect that performance schema is off due to lack of namespace variable."); + $this->assertFalse( + $this->database->hasPerformanceSchema(), + "Ensure we correctly detect that performance schema is off due to lack of namespace variable." + ); } public function test__setSchema() From 873cd76885a00b410975528c0e7536e86cb6d258 Mon Sep 17 00:00:00 2001 From: Thomas Shone Date: Sun, 1 Jan 2023 20:57:37 +0100 Subject: [PATCH 2/4] Support all the cases listed in the issue --- .../Check/Account/PasswordlessAccount.php | 21 +- src/Engine/Entity/Account/User.php | 2 +- .../Check/Account/PasswordlessAccountTest.php | 238 +++++++++++++++--- 3 files changed, 222 insertions(+), 39 deletions(-) diff --git a/src/Engine/Check/Account/PasswordlessAccount.php b/src/Engine/Check/Account/PasswordlessAccount.php index 0a65bd4..35ebd8c 100644 --- a/src/Engine/Check/Account/PasswordlessAccount.php +++ b/src/Engine/Check/Account/PasswordlessAccount.php @@ -17,7 +17,26 @@ public function supports($entity): bool public function run($entity): ?Report { - if ($entity->getUser()->authentication_string) { + $pass = !is_null($entity->getUser()->authentication_string) + && $entity->getUser()->authentication_string; + + $version = $entity->getDatabase()->getVersion(); + + // If we're running MySQL 5.6.*, we only flag an issue if the plugin field is mysql_native_password + $pass |= ( + version_compare($version, '5.7', '<') + && version_compare($version, '5.6', '>=') + && $entity->getUser()->plugin !== 'mysql_native_password' + ); + + // If we're running MySQL 5.5.*, we only flag an issue if the plugin field is NULL or empty + $pass |= ( + version_compare($version, '5.6', '<') + && version_compare($version, '5.5', '>=') + && $entity->getUser()->plugin + ); + + if ($pass) { return new Report( $this, $entity, diff --git a/src/Engine/Entity/Account/User.php b/src/Engine/Entity/Account/User.php index a798001..a699763 100644 --- a/src/Engine/Entity/Account/User.php +++ b/src/Engine/Entity/Account/User.php @@ -53,7 +53,7 @@ public function __construct( public int $max_updates = 0, public int $max_connections = 0, public int $max_user_connections = 0, - public string $plugin = 'caching_sha2_password', + public string|null $plugin = 'caching_sha2_password', public string|null $authentication_string = null, public bool|null $password_expired = false, public int|null $password_last_changed = null, diff --git a/tests/Engine/Check/Account/PasswordlessAccountTest.php b/tests/Engine/Check/Account/PasswordlessAccountTest.php index c9a1962..54ae6f3 100644 --- a/tests/Engine/Check/Account/PasswordlessAccountTest.php +++ b/tests/Engine/Check/Account/PasswordlessAccountTest.php @@ -11,56 +11,220 @@ class PasswordlessAccountTest extends BaseTest { - private array $accounts; + public function providerAccountData() { - public function setUp(): void - { - $passwordless_account = $this->createAccount('passwordless_account', 'localhost'); - $passwordless_account_non_local = $this->createAccount('passwordless_account', '%'); - $passworded_account = Account::withUser(new User('passworded_account', 'localhost', authentication_string: "randomJibberish")); +// // Passwordless Local +// $passwordless_55 = $this->createAccount('passwordless_account', 'localhost'); +// $passwordless_55->setDatabase($database_55); +// +// $passwordless_56 = $this->createAccount('passwordless_account', 'localhost'); +// $passwordless_56->setDatabase($database_56); +// +// $passwordless_57 = $this->createAccount('passwordless_account', 'localhost'); +// $passwordless_57->setDatabase($database_57); +// +// $passwordless_81 = $this->createAccount('passwordless_account', 'localhost'); +// $passwordless_81->setDatabase($database_81); +// +// // Passwordless Remote +// $passwordless_remote_55 = $this->createAccount('passwordless_remote_account', '%'); +// $passwordless_remote_55->setDatabase($database_55); +// +// $passwordless_remote_56 = $this->createAccount('passwordless_remote_account', '%'); +// $passwordless_remote_56->setDatabase($database_56); +// +// $passwordless_remote_57 = $this->createAccount('passwordless_remote_account', '%'); +// $passwordless_remote_57->setDatabase($database_57); +// +// $passwordless_remote_81 = $this->createAccount('passwordless_remote_account', '%'); +// $passwordless_remote_81->setDatabase($database_81); +// +// // Password +// $passwordless_remote_55 = $this->createAccount('passwordless_remote_account', '%'); +// $passwordless_remote_55->setDatabase($database_55); +// +// $passwordless_remote_56 = $this->createAccount('passwordless_remote_account', '%'); +// $passwordless_remote_56->setDatabase($database_56); +// +// $passwordless_remote_57 = $this->createAccount('passwordless_remote_account', '%'); +// $passwordless_remote_57->setDatabase($database_57); +// +// $passwordless_remote_81 = $this->createAccount('passwordless_remote_account', '%'); +// $passwordless_remote_81->setDatabase($database_81); - $this->accounts = [ - $passwordless_account, - $passwordless_account_non_local, - $passworded_account + return [ + // Passwordless account with local access + [ + 'localhost', + '5.5.3', + 'not_empty', + null, + Report::STATUS_OK + ], + [ + 'localhost', + '5.5.3', + null, + null, + Report::STATUS_WARNING + ], + [ + 'localhost', + '5.6.0', + 'mysql_native_password', + null, + Report::STATUS_WARNING + ], + [ + 'localhost', + '5.7.2', + null, + null, + Report::STATUS_WARNING + ], + [ + 'localhost', + '5.7.2', + 'mysql_native_password', + null, + Report::STATUS_WARNING + ], + [ + 'localhost', + '8.1', + null, + null, + Report::STATUS_WARNING + ], + [ + 'localhost', + '8.1', + 'mysql_native_password', + null, + Report::STATUS_WARNING + ], + // Passwordless account with non-local access + [ + '%', + '5.5.3', + 'not_empty', + null, + Report::STATUS_OK + ], + [ + '%', + '5.5.3', + null, + null, + Report::STATUS_CRITICAL + ], + [ + '%', + '5.6.0', + 'mysql_native_password', + null, + Report::STATUS_CRITICAL + ], + [ + '%', + '5.7.2', + null, + null, + Report::STATUS_CRITICAL + ], + [ + '%', + '5.7.2', + 'mysql_native_password', + null, + Report::STATUS_CRITICAL + ], + [ + '%', + '8.1', + null, + null, + Report::STATUS_CRITICAL + ], + [ + '%', + '8.1', + 'mysql_native_password', + null, + Report::STATUS_CRITICAL + ], + // Passworded account + [ + 'localhost', + '5.5.3', + 'not_empty', + 'randomJibberish', + Report::STATUS_OK + ], + [ + 'localhost', + '5.5.3', + null, + 'randomJibberish', + Report::STATUS_OK + ], + [ + 'localhost', + '5.6.0', + 'mysql_native_password', + 'randomJibberish', + Report::STATUS_OK + ], + [ + 'localhost', + '5.7.2', + null, + 'randomJibberish', + Report::STATUS_OK + ], + [ + 'localhost', + '5.7.2', + 'mysql_native_password', + 'randomJibberish', + Report::STATUS_OK + ], + [ + 'localhost', + '8.1', + null, + 'randomJibberish', + Report::STATUS_OK + ], + [ + 'localhost', + '8.1', + 'mysql_native_password', + 'randomJibberish', + Report::STATUS_OK + ], ]; } - public function testSupports() + /** + * @dataProvider providerAccountData + */ + public function testRun($host, $version, $plugin, $password, $status) { $check = new PasswordlessAccount(); - foreach ($this->accounts as $account) { - $this->assertTrue( - $check->supports($account), - "Ensure that we care about all accounts." - ); - } - } - - public function testRun() - { - $check = new PasswordlessAccount(); + $account = Account::withUser(new User("test", $host, plugin: $plugin, authentication_string: $password)); + $account->setDatabase($this->createDatabase([ 'version' => $version ])); - $account = $this->accounts[0]; - $this->assertEquals( - Report::STATUS_WARNING, - $check->run($account)->getStatus(), - $account->getName() . "@" . $account->getHost() . " has no password set for a localhost account." - ); - - $account = $this->accounts[1]; - $this->assertEquals( - Report::STATUS_CRITICAL, - $check->run($account)->getStatus(), - $account->getName() . "@" . $account->getHost() . " has no password set for a non-localhost account." + $this->assertTrue( + $check->supports($account), + "Ensure that we care about all accounts." ); - $account = $this->accounts[2]; $this->assertEquals( - Report::STATUS_OK, + $status, $check->run($account)->getStatus(), - $account->getName() . "@" . $account->getHost() . " has a password set." + "test@$host on MySQL $version with plugin:$plugin and authentication_string:$password tests correctly: $status." ); } } From a378f6f15f8dcd742559758fe2e0786d553f0690 Mon Sep 17 00:00:00 2001 From: Thomas Shone Date: Sun, 1 Jan 2023 20:57:57 +0100 Subject: [PATCH 3/4] Updated composer.lock to reflect version bump to PHP 8.* --- composer.lock | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/composer.lock b/composer.lock index 9ab38f7..0f39e54 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "7b8d6684195b638e60f856317060cd42", + "content-hash": "7552e896758ab54f300900191d90c89a", "packages": [ { "name": "doctrine/cache", @@ -5819,7 +5819,8 @@ "prefer-stable": false, "prefer-lowest": false, "platform": { - "php": "^7.4 || ^8.0" + "php": "^8.0", + "ext-json": "*" }, "platform-dev": [], "plugin-api-version": "2.3.0" From 0b86583873344aa7a0c43c29a04a7ee07a94ba6a Mon Sep 17 00:00:00 2001 From: Thomas Shone Date: Tue, 3 Jan 2023 14:33:14 +0100 Subject: [PATCH 4/4] Removed commented out code. --- .../Check/Account/PasswordlessAccountTest.php | 40 ------------------- 1 file changed, 40 deletions(-) diff --git a/tests/Engine/Check/Account/PasswordlessAccountTest.php b/tests/Engine/Check/Account/PasswordlessAccountTest.php index 54ae6f3..dd5f66c 100644 --- a/tests/Engine/Check/Account/PasswordlessAccountTest.php +++ b/tests/Engine/Check/Account/PasswordlessAccountTest.php @@ -12,46 +12,6 @@ class PasswordlessAccountTest extends BaseTest { public function providerAccountData() { - -// // Passwordless Local -// $passwordless_55 = $this->createAccount('passwordless_account', 'localhost'); -// $passwordless_55->setDatabase($database_55); -// -// $passwordless_56 = $this->createAccount('passwordless_account', 'localhost'); -// $passwordless_56->setDatabase($database_56); -// -// $passwordless_57 = $this->createAccount('passwordless_account', 'localhost'); -// $passwordless_57->setDatabase($database_57); -// -// $passwordless_81 = $this->createAccount('passwordless_account', 'localhost'); -// $passwordless_81->setDatabase($database_81); -// -// // Passwordless Remote -// $passwordless_remote_55 = $this->createAccount('passwordless_remote_account', '%'); -// $passwordless_remote_55->setDatabase($database_55); -// -// $passwordless_remote_56 = $this->createAccount('passwordless_remote_account', '%'); -// $passwordless_remote_56->setDatabase($database_56); -// -// $passwordless_remote_57 = $this->createAccount('passwordless_remote_account', '%'); -// $passwordless_remote_57->setDatabase($database_57); -// -// $passwordless_remote_81 = $this->createAccount('passwordless_remote_account', '%'); -// $passwordless_remote_81->setDatabase($database_81); -// -// // Password -// $passwordless_remote_55 = $this->createAccount('passwordless_remote_account', '%'); -// $passwordless_remote_55->setDatabase($database_55); -// -// $passwordless_remote_56 = $this->createAccount('passwordless_remote_account', '%'); -// $passwordless_remote_56->setDatabase($database_56); -// -// $passwordless_remote_57 = $this->createAccount('passwordless_remote_account', '%'); -// $passwordless_remote_57->setDatabase($database_57); -// -// $passwordless_remote_81 = $this->createAccount('passwordless_remote_account', '%'); -// $passwordless_remote_81->setDatabase($database_81); - return [ // Passwordless account with local access [