Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UML-2725 implement check for pwned passwords #2342

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions service-front/app/src/Actor/src/Form/CreateAccount.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Common\Form\AbstractForm;
use Common\Form\Element\Email;
use Common\Validator\CommonPasswordValidator;
use Common\Validator\EmailAddressValidator;
use Laminas\Filter\StringToLower;
use Laminas\Filter\StringTrim;
Expand Down Expand Up @@ -93,6 +94,9 @@ public function getInputFilterSpecification(): array
],
],
],
[
'name' => CommonPasswordValidator::class,
],
[
'name' => StringLength::class,
'options' => [
Expand Down
4 changes: 4 additions & 0 deletions service-front/app/src/Actor/src/Form/PasswordChange.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Actor\Form;

use Common\Form\AbstractForm;
use Common\Validator\CommonPasswordValidator;
use Laminas\InputFilter\InputFilterProviderInterface;
use Laminas\Validator\NotEmpty;
use Laminas\Validator\StringLength;
Expand Down Expand Up @@ -88,6 +89,9 @@ public function getInputFilterSpecification()
],
],
],
[
'name' => CommonPasswordValidator::class,
],
],
],
];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
{% trans %}Passwords do not match{% context %}error{% notes %}Create account{% endtrans %}
{% trans %}Confirm your password{% context %}error{% notes %}Create account, password reset{% endtrans %}
{% trans %}You must accept the terms of use to create an account{% context %}error{% notes %}Create account{% endtrans %}
{% trans %}Password is too common{% context %}error{% notes %}Create account, change password{% endtrans %}


{# Triage page #}
{% trans %}Select yes if you have a Use a lasting power of attorney account{% context %}error{% notes %}Triage page{% endtrans %}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
<?php

namespace Common\Validator;

use Laminas\Validator\AbstractValidator;
use RuntimeException;

class CommonPasswordValidator extends AbstractValidator
{
public const COMMON_PASSWORD = 'common';

/**
* @var string[]
*/
protected array $messageTemplates = [
self::COMMON_PASSWORD => 'Password is too common',
];


const TMP_ROOT_PATH = '/tmp/';
const PWNED_PW_URL = 'https://www.ncsc.gov.uk/static-assets/documents/PwnedPasswordsTop100k.txt';

private string $filePathCommonPasswords;

private string $pwnedPasswordsUrl;

public function __construct()
{
parent::__construct();
$this->filePathCommonPasswords = self::TMP_ROOT_PATH.'commonpasswords.txt';
$this->pwnedPasswordsUrl = self::PWNED_PW_URL;
}

/**
* @param mixed $value
* @return bool
*/
public function isValid($value): bool
{
$isValid = true;
$this->checkCommonPasswordsFileExists($this->filePathCommonPasswords);

if ($this->passwordMatchesCommonPasswords($value, $this->filePathCommonPasswords)) {
$this->error(self::COMMON_PASSWORD);
$isValid = false;
}

return $isValid;
}

protected function passwordMatchesCommonPasswords(string $searchTerm, string $filePath): bool
{
$matches = [];
$handle = @fopen($filePath, 'r');
if ($handle && strlen($searchTerm) > 0) {
while (!feof($handle)) {
$buffer = fgets($handle);
if (false !== strpos($buffer, $searchTerm)) {
$matches[] = $buffer;
}
}
fclose($handle);
}
//show results:
if (count($matches) > 0) {
return true;
} else {
return false;
}
}

protected function checkCommonPasswordsFileExists(string $filePath): void
{
if (file_exists($filePath) & (time() - filemtime($filePath) < 24 * 3600)) {
return;
} else {
$fp = fopen($this->pwnedPasswordsUrl, 'r');
if (false !== $fp) {
$written = file_put_contents(
"$filePath",
$fp
);
if (false === $written) {
throw new RuntimeException(sprintf('Unable to download or write common password file to disk'));
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php

declare(strict_types=1);

namespace CommonTest\Validator;

use Common\Validator\CommonPasswordValidator;
use PHPUnit\Framework\TestCase;

class CommonPasswordValidatorTest extends TestCase
{
const PW_FILE_PATH = '/tmp/commonpasswords.txt';
const PWNED_PW_URL = 'https://www.ncsc.gov.uk/static-assets/documents/PwnedPasswordsTop100k.txt';

public function setUp(): void
{
file_put_contents(
self::PW_FILE_PATH,
fopen(self::PWNED_PW_URL, 'r')
);

$this->validator = new CommonPasswordValidator();
}

/**
* Verify a constraint message is triggered when value is invalid.
*/
public function testValidateOnInvalid()
{
$this->assertFalse($this->validator->isValid('Password123'));
print(count($this->validator->getMessages()));
$this->assertArrayHasKey(CommonPasswordValidator::COMMON_PASSWORD, $this->validator->getMessages());
}


/**
* Verify no constraint message is triggered when value is valid.
*/
public function testValidateOnValid()
{
$this->assertTrue($this->validator->isValid('Aformidablepw876!'));
$this->assertCount(0, $this->validator->getMessages());

}
}
4 changes: 4 additions & 0 deletions service-front/docker/app/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ FROM php-base AS app
COPY service-front/app /app
COPY --from=dependency /app/vendor /app/vendor

# Add common passwords file
RUN wget -q -O /tmp/commonpasswords.txt "https://www.ncsc.gov.uk/static-assets/documents/PwnedPasswordsTop100k.txt" \
&& chown www-data /tmp/commonpasswords.txt

#
# Install development dependencies and tools into production image
#
Expand Down
Loading