fix(user:reset-password): generate policy-compliant random passwords

src/Command/ResetPasswordCommand.php

@@ -15,6 +15,10 @@
 namespace OpenCoreEMR\CLI\ManageUsers\Command;
 
 use OpenCoreEMR\CLI\ManageUsers\Exception\ManageUsersException;
+use OpenCoreEMR\CLI\ManageUsers\Service\OpenEMRConnector;
+use OpenCoreEMR\CLI\ManageUsers\Service\PasswordPolicy;
+use OpenCoreEMR\CLI\ManageUsers\Service\PasswordPolicyInterface;
+use OpenCoreEMR\CLI\ManageUsers\Service\UserManager;
 use Symfony\Component\Console\Attribute\AsCommand;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
@@ -24,7 +28,25 @@
 #[AsCommand(name: 'user:reset-password', description: "Reset an OpenEMR user's password")]
 class ResetPasswordCommand extends AbstractUserCommand
 {
-    private const RANDOM_PASSWORD_BYTES = 12;
+    private const RANDOM_PASSWORD_LENGTH = 20;
+    private const RANDOM_PASSWORD_MAX_ATTEMPTS = 32;
+    // Excludes look-alikes (0/O/o, 1/l/I) to reduce transcription errors.
+    private const RANDOM_LOWER = 'abcdefghijkmnpqrstuvwxyz';
+    private const RANDOM_UPPER = 'ABCDEFGHJKLMNPQRSTUVWXYZ';
+    private const RANDOM_DIGIT = '23456789';
+    // Shell-safe symbols: no quotes, backslash, backtick, $, parens, <, >, |, ;, &, space.
+    private const RANDOM_SYMBOL = '!@#%^*-_=+?';
+
+    private readonly PasswordPolicyInterface $policy;
+
+    public function __construct(
+        ?OpenEMRConnector $connector = null,
+        ?UserManager $users = null,
+        ?PasswordPolicyInterface $policy = null,
+    ) {
+        parent::__construct($connector, $users);
+        $this->policy = $policy ?? new PasswordPolicy();
+    }
 
     protected function configure(): void
     {
@@ -63,7 +85,7 @@ protected function doExecute(InputInterface $input, SymfonyStyle $io): int
             if ($password !== null && $password !== '') {
                 throw new ManageUsersException("--random cannot be combined with --password");
             }
-            $password = $this->generateRandomPassword();
+            $password = $this->generatePolicyCompliantPassword();
             $io->writeln("Generated password: <info>{$password}</info>");
         } elseif ($password === null || $password === '') {
             $question = new Question("New password for {$username}: ");
@@ -83,9 +105,59 @@ protected function doExecute(InputInterface $input, SymfonyStyle $io): int
         return self::SUCCESS;
     }
 
-    private function generateRandomPassword(): string
+    /**
+     * Generate a strong random password and verify it satisfies the install's
+     * configured policy. The generator already covers all four character
+     * classes at length 20, so a single attempt almost always suffices —
+     * retries exist only to absorb a future stricter policy without surprising
+     * the operator with a "successful" reset to an unusable account.
+     */
+    private function generatePolicyCompliantPassword(): string
+    {
+        $lastError = null;
+        for ($i = 0; $i < self::RANDOM_PASSWORD_MAX_ATTEMPTS; $i++) {
+            $candidate = $this->generateRandomCandidate();
+            $lastError = $this->policy->validate($candidate);
+            if ($lastError === null) {
+                return $candidate;
+            }
+        }
+        throw new ManageUsersException(
+            "Could not generate a policy-compliant random password after "
+            . self::RANDOM_PASSWORD_MAX_ATTEMPTS . " attempts: {$lastError}"
+        );
+    }
+
+    private function generateRandomCandidate(): string
+    {
+        // Guarantee at least one of each class so the strictest default OpenEMR
+        // policy (secure_password) accepts the result on the first try.
+        $chars = [
+            $this->pick(self::RANDOM_LOWER),
+            $this->pick(self::RANDOM_UPPER),
+            $this->pick(self::RANDOM_DIGIT),
+            $this->pick(self::RANDOM_SYMBOL),
+        ];
+
+        $alphabet = self::RANDOM_LOWER . self::RANDOM_UPPER . self::RANDOM_DIGIT . self::RANDOM_SYMBOL;
+        for ($i = count($chars); $i < self::RANDOM_PASSWORD_LENGTH; $i++) {
+            $chars[] = $this->pick($alphabet);
+        }
+
+        // Fisher-Yates shuffle with a CSPRNG; str_shuffle is not cryptographic.
+        for ($i = count($chars) - 1; $i > 0; $i--) {
+            $j = random_int(0, $i);
+            [$chars[$i], $chars[$j]] = [$chars[$j], $chars[$i]];
+        }
+
+        return implode('', $chars);
+    }
+
+    /**
+     * @param non-empty-string $alphabet
+     */
+    private function pick(string $alphabet): string
     {
-        // Hex of N bytes -> 2N chars; safely printable, easy to copy/paste.
-        return bin2hex(random_bytes(self::RANDOM_PASSWORD_BYTES));
+        return $alphabet[random_int(0, strlen($alphabet) - 1)];
     }
 }

src/Service/PasswordPolicy.php

@@ -0,0 +1,68 @@
+<?php
+
+/**
+ * Mirror of OpenEMR's runtime password validation.
+ *
+ * OpenEMR's actual checks live in AuthUtils::testMinimumPasswordLength,
+ * testMaximumPasswordLength, and testPasswordStrength — all private, and the
+ * only public entry point (AuthUtils::updatePassword) couples validation with
+ * persistence and process-killing edge cases. So we cannot delegate; we
+ * reapply the same checks against the install's own $GLOBALS so the policy
+ * tracks whatever the operator configured.
+ *
+ * If OpenEMR ever exposes a side-effect-free validator, replace this class
+ * with a thin wrapper around it.
+ *
+ * @see vendor/openemr/openemr/src/Common/Auth/AuthUtils.php
+ *      (testMinimumPasswordLength, testMaximumPasswordLength, testPasswordStrength)
+ *
+ * @package   OpenCoreEMR\CLI\ManageUsers
+ * @link      https://opencoreemr.com
+ * @author    Michael A. Smith <michael@opencoreemr.com>
+ * @copyright Copyright (c) 2026 OpenCoreEMR Inc
+ * @license   https://github.com/openCoreEMR/oce-cli-manage-users/blob/main/LICENSE GNU General Public License 3
+ */
+
+declare(strict_types=1);
+
+namespace OpenCoreEMR\CLI\ManageUsers\Service;
+
+final class PasswordPolicy implements PasswordPolicyInterface
+{
+    /**
+     * @return string|null Null on pass; an explanatory message on fail.
+     */
+    public function validate(string $password): ?string
+    {
+        /** @var mixed $rawMin */
+        $rawMin = $GLOBALS['gbl_minimum_password_length'] ?? 0;
+        $min = is_numeric($rawMin) ? (int)$rawMin : 0;
+        if ($min > 0 && strlen($password) < $min) {
+            return "Password too short (minimum {$min} characters)";
+        }
+
+        /** @var mixed $rawMax */
+        $rawMax = $GLOBALS['gbl_maximum_password_length'] ?? 0;
+        $max = is_numeric($rawMax) ? (int)$rawMax : 0;
+        if ($max > 0 && strlen($password) > $max) {
+            return "Password too long (maximum {$max} characters)";
+        }
+
+        $secure = $GLOBALS['secure_password'] ?? null;
+        if ($secure) {
+            $classes = [
+                '/[a-z]/',    // lowercase
+                '/[A-Z]/',    // uppercase
+                '/\d/',       // digit
+                '/[\W_]/',    // symbol (non-word or underscore)
+            ];
+            foreach ($classes as $regex) {
+                if (preg_match($regex, $password) !== 1) {
+                    return "Password must contain a lowercase letter, uppercase letter, digit, and symbol";
+                }
+            }
+        }
+
+        return null;
+    }
+}

src/Service/PasswordPolicyInterface.php

@@ -0,0 +1,27 @@
+<?php
+
+/**
+ * Contract for runtime password policy validation.
+ *
+ * Exists so callers (commands, tests) depend on the abstraction rather than
+ * the concrete PasswordPolicy. Tests mock this interface; the concrete
+ * implementation stays final.
+ *
+ * @package   OpenCoreEMR\CLI\ManageUsers
+ * @link      https://opencoreemr.com
+ * @author    Michael A. Smith <michael@opencoreemr.com>
+ * @copyright Copyright (c) 2026 OpenCoreEMR Inc
+ * @license   https://github.com/openCoreEMR/oce-cli-manage-users/blob/main/LICENSE GNU General Public License 3
+ */
+
+declare(strict_types=1);
+
+namespace OpenCoreEMR\CLI\ManageUsers\Service;
+
+interface PasswordPolicyInterface
+{
+    /**
+     * @return string|null Null on pass; an explanatory message on fail.
+     */
+    public function validate(string $password): ?string;
+}

tests/Unit/Command/ResetPasswordCommandTest.php

@@ -16,6 +16,7 @@
 
 use OpenCoreEMR\CLI\ManageUsers\Command\ResetPasswordCommand;
 use OpenCoreEMR\CLI\ManageUsers\Service\OpenEMRConnector;
+use OpenCoreEMR\CLI\ManageUsers\Service\PasswordPolicyInterface;
 use OpenCoreEMR\CLI\ManageUsers\Service\UserManager;
 use PHPUnit\Framework\Attributes\Test;
 use PHPUnit\Framework\MockObject\MockObject;
@@ -27,14 +28,16 @@ class ResetPasswordCommandTest extends TestCase
 {
     private OpenEMRConnector&MockObject $connector;
     private UserManager&MockObject $users;
+    private PasswordPolicyInterface&MockObject $policy;
     private CommandTester $tester;
 
     protected function setUp(): void
     {
         $this->connector = $this->createMock(OpenEMRConnector::class);
         $this->users = $this->createMock(UserManager::class);
+        $this->policy = $this->createMock(PasswordPolicyInterface::class);
 
-        $command = new ResetPasswordCommand($this->connector, $this->users);
+        $command = new ResetPasswordCommand($this->connector, $this->users, $this->policy);
         $app = new Application();
         $app->add($command);
 
@@ -72,14 +75,22 @@ public function resetWithExplicitPassword(): void
     }
 
     #[Test]
-    public function randomGeneratesAndPrintsPassword(): void
+    public function randomGeneratesPolicyCompliantPassword(): void
     {
+        $this->policy->expects(self::atLeastOnce())
+            ->method('validate')
+            ->willReturn(null);
+
         $captured = null;
         $this->users->expects(self::once())
             ->method('resetPassword')
             ->with('bob', self::callback(function (string $password) use (&$captured): bool {
                 $captured = $password;
-                return strlen($password) >= 16;
+                return strlen($password) === 20
+                    && preg_match('/[a-z]/', $password) === 1
+                    && preg_match('/[A-Z]/', $password) === 1
+                    && preg_match('/\d/', $password) === 1
+                    && preg_match('/[\W_]/', $password) === 1;
             }));
 
         $exit = $this->tester->execute(['--user' => 'bob', '--random' => true]);
@@ -89,6 +100,32 @@ public function randomGeneratesAndPrintsPassword(): void
         self::assertStringContainsString("Generated password: {$captured}", $this->tester->getDisplay());
     }
 
+    #[Test]
+    public function randomRetriesUntilPolicyAccepts(): void
+    {
+        $this->policy->expects(self::exactly(3))
+            ->method('validate')
+            ->willReturnOnConsecutiveCalls('too weak', 'too weak', null);
+
+        $this->users->expects(self::once())->method('resetPassword');
+
+        $exit = $this->tester->execute(['--user' => 'bob', '--random' => true]);
+
+        self::assertSame(0, $exit);
+    }
+
+    #[Test]
+    public function randomFailsWhenPolicyKeepsRejecting(): void
+    {
+        $this->policy->method('validate')->willReturn('arbitrary policy says no');
+        $this->users->expects(self::never())->method('resetPassword');
+
+        $exit = $this->tester->execute(['--user' => 'bob', '--random' => true]);
+
+        self::assertSame(1, $exit);
+        self::assertStringContainsString('policy-compliant', $this->tester->getDisplay());
+    }
+
     #[Test]
     public function randomCannotBeCombinedWithPassword(): void
     {

tests/Unit/Service/PasswordPolicyTest.php

@@ -0,0 +1,80 @@
+<?php
+
+/**
+ * Unit tests for PasswordPolicy.
+ *
+ * PasswordPolicy reads $GLOBALS, so each test sets the relevant keys and
+ * tearDown clears them to keep tests isolated.
+ *
+ * @package   OpenCoreEMR\CLI\ManageUsers\Tests
+ * @link      https://opencoreemr.com
+ * @author    Michael A. Smith <michael@opencoreemr.com>
+ * @copyright Copyright (c) 2026 OpenCoreEMR Inc
+ * @license   https://github.com/openCoreEMR/oce-cli-manage-users/blob/main/LICENSE GNU General Public License 3
+ */
+
+declare(strict_types=1);
+
+namespace OpenCoreEMR\CLI\ManageUsers\Tests\Unit\Service;
+
+use OpenCoreEMR\CLI\ManageUsers\Service\PasswordPolicy;
+use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\TestCase;
+
+class PasswordPolicyTest extends TestCase
+{
+    private PasswordPolicy $policy;
+
+    protected function setUp(): void
+    {
+        $this->policy = new PasswordPolicy();
+    }
+
+    protected function tearDown(): void
+    {
+        unset(
+            $GLOBALS['secure_password'],
+            $GLOBALS['gbl_minimum_password_length'],
+            $GLOBALS['gbl_maximum_password_length'],
+        );
+    }
+
+    #[Test]
+    public function passesWhenPolicyDisabled(): void
+    {
+        self::assertNull($this->policy->validate('hex'));
+    }
+
+    #[Test]
+    public function rejectsShortPassword(): void
+    {
+        $GLOBALS['gbl_minimum_password_length'] = 12;
+        $error = $this->policy->validate('short');
+        self::assertNotNull($error);
+        self::assertStringContainsString('too short', $error);
+    }
+
+    #[Test]
+    public function rejectsLongPassword(): void
+    {
+        $GLOBALS['gbl_maximum_password_length'] = 8;
+        $error = $this->policy->validate('this-is-too-long');
+        self::assertNotNull($error);
+        self::assertStringContainsString('too long', $error);
+    }
+
+    #[Test]
+    public function securePasswordRequiresAllFourClasses(): void
+    {
+        $GLOBALS['secure_password'] = 1;
+
+        // hex-only string (the bug from issue #12) should be rejected
+        self::assertNotNull($this->policy->validate('a1b2c3d4e5f6a1b2c3d4'));
+
+        // missing symbol
+        self::assertNotNull($this->policy->validate('Abcdefgh1234'));
+
+        // all four classes present
+        self::assertNull($this->policy->validate('Abcdefgh1234!'));
+    }
+}