hackzilla / password-generator

PHP Library to generate random passwords
https://hackzilla.org
MIT License
293 stars 37 forks source link

RequirementPasswordGenerator returns password with different length #27

Closed adamsafr closed 6 years ago

adamsafr commented 6 years ago

Hi! I have a problem with RequirementPasswordGenerator. It generates password with different length on production server with OPcache. I set length = 8, but it sometimes generates password with length 6, 2, etc.

I couldn't find any reason why this happens. On dev environment everything works fine.

Symfony: 3.3.9 PHP 7.1.14-1 with FPM

public function generatePassword(): string
    {
        $this->passwordGenerator
            ->setLength(self::PASS_MIN_LENGTH) // must have a minimum of eight characters,
            ->setOptionValue(RequirementPasswordGenerator::OPTION_NUMBERS, true) // must be composed of numbers...
            ->setMinimumCount(RequirementPasswordGenerator::OPTION_NUMBERS, 1) // ...at least one number...
            ->setOptionValue(RequirementPasswordGenerator::OPTION_SYMBOLS, true) // ...and special characters
            ->setMinimumCount(RequirementPasswordGenerator::OPTION_SYMBOLS, 1)
            ->setOptionValue(RequirementPasswordGenerator::OPTION_UPPER_CASE, true) // at least one upper
            ->setMinimumCount(RequirementPasswordGenerator::OPTION_UPPER_CASE, 1)
            ->setOptionValue(RequirementPasswordGenerator::OPTION_LOWER_CASE, true) // ...and one lower
            ->setMinimumCount(RequirementPasswordGenerator::OPTION_LOWER_CASE, 1);

       // I've written this code to generate pass with required length (FIX)
        do {
            $password = $this->passwordGenerator->generatePassword();
        } while (\strlen($password) !== self::PASS_MIN_LENGTH);

        return $password;
    }
hackzilla commented 6 years ago

Does the test suite pass in your production server?

adamsafr commented 6 years ago

All tests pass well except ReadMeTest. I think it's OK because I don't have file /usr/share/dict/words. password-generator-bundle tests pass 100%

There were 3 errors:

1) Hackzilla\PasswordGenerator\Tests\ReadMeTest::testHumanPasswordGeneratorUsage
Hackzilla\PasswordGenerator\Exception\FileNotFoundException: File not found

/var/www/addon/code/vendor/hackzilla/password-generator/Generator/HumanPasswordGenerator.php:329
/var/www/addon/code/vendor/hackzilla/password-generator/Tests/ReadMeTest.php:62
/var/www/addon/code/vendor/symfony/phpunit-bridge/bin/.phpunit/phpunit-5.7/phpunit:5

2) Hackzilla\PasswordGenerator\Tests\ReadMeTest::testPhp5RandomGeneratorUsage
Hackzilla\PasswordGenerator\Exception\FileNotFoundException: File not found

/var/www/addon/code/vendor/hackzilla/password-generator/Generator/HumanPasswordGenerator.php:329
/var/www/addon/code/vendor/hackzilla/password-generator/Tests/ReadMeTest.php:80
/var/www/addon/code/vendor/symfony/phpunit-bridge/bin/.phpunit/phpunit-5.7/phpunit:5

3) Hackzilla\PasswordGenerator\Tests\ReadMeTest::testPhp7RandomGeneratorUsage
Hackzilla\PasswordGenerator\Exception\FileNotFoundException: File not found

/var/www/addon/code/vendor/hackzilla/password-generator/Generator/HumanPasswordGenerator.php:329
/var/www/addon/code/vendor/hackzilla/password-generator/Tests/ReadMeTest.php:100
/var/www/addon/code/vendor/symfony/phpunit-bridge/bin/.phpunit/phpunit-5.7/phpunit:5

ERRORS!
Tests: 325, Assertions: 1573, Errors: 3.

I'll try to write tests for service which uses password-generator and check on production server

hackzilla commented 6 years ago

I was hoping that I'd have a test fail, that would make this easy to fix. And, as you say, there doesn't seem to be a reason for it.

Try running this basic script a few times on your production server and see if it produces different length passwords.

<?php

require "vendor/autoload.php";

use Hackzilla\PasswordGenerator\Generator\RequirementPasswordGenerator;

$passwordGenerator = new RequirementPasswordGenerator();

$passwordGenerator
            ->setLength(8) // must have a minimum of eight characters,
            ->setOptionValue(RequirementPasswordGenerator::OPTION_NUMBERS, true) // must be composed of numbers...
            ->setMinimumCount(RequirementPasswordGenerator::OPTION_NUMBERS, 1) // ...at least one number...
            ->setOptionValue(RequirementPasswordGenerator::OPTION_SYMBOLS, true) // ...and special characters
            ->setMinimumCount(RequirementPasswordGenerator::OPTION_SYMBOLS, 1)
            ->setOptionValue(RequirementPasswordGenerator::OPTION_UPPER_CASE, true) // at least one upper
            ->setMinimumCount(RequirementPasswordGenerator::OPTION_UPPER_CASE, 1)
            ->setOptionValue(RequirementPasswordGenerator::OPTION_LOWER_CASE, true) // ...and one lower
            ->setMinimumCount(RequirementPasswordGenerator::OPTION_LOWER_CASE, 1);

       // I've written this code to generate pass with required length (FIX)
#        do {
            $password = $passwordGenerator->generatePassword();
#        } while (\strlen($password) !== 8);

echo $password . " (" . strlen($password) . ")" . PHP_EOL;
adamsafr commented 6 years ago

Finally I've found the reason 🤦‍♂️ I should have used escaping for html tags for generated password because password can contain opening tag syntax (for example "<U") Thank you for help 👍

hackzilla commented 6 years ago

I'm glad you got to the bottom of it.