Skip to content

Commit

Permalink
Refactoring and adding the option to include other input
Browse files Browse the repository at this point in the history
  • Loading branch information
@REBELinBLUE
REBELinBLUE committed Oct 27, 2017
1 parent 7ef552b commit e4436e2
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 77 deletions.
10 changes: 0 additions & 10 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,11 @@ phpmd:
@echo "${GREEN}PHP Mess Detector${RESET}"
@php vendor/bin/phpmd src/ text cleancode,codesize,naming,design,controversial,unusedcode

## Run Behat
#behat:
# @echo "${GREEN}Behat${RESET}"
# @php vendor/bin/behat --format progress

## Run PHP code sniffer
phpcs:
@echo "${GREEN}PHP Code Sniffer${RESET}"
@php vendor/bin/phpcs -p --standard=psr2 --colors src/

## Run PHPStan
#phpstan:
# @echo "${GREEN}PHPStan${RESET}"
# @php vendor/bin/phpstan analyse -l 0 src/

## PHP Parallel Lint
lint:
@echo "${GREEN}PHP Parallel Lint${RESET}"
Expand Down
5 changes: 3 additions & 2 deletions resources/lang/en/validation.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
'predictable' => 'Predictable substitutions such as "@" for "a" or "$" for "s" are easy to guess',
'sequence' => 'Sequences like "ABC" or "123" are easy to guess',
'repeat' => 'Repeating characters like "AAA" or "111" are easy to guess',
'dates' => 'Dates are often easy to guess',
'years' => 'Recent years are easy to guess',
'date' => 'Dates are often easy to guess',
'year' => 'Recent years are easy to guess',
'straight_spatial' => 'Short keyboard patterns are easy to guess',
'spatial_with_turns' => 'Straight rows of keys are easy to guess',
'names' => 'Names are easy to guess',
Expand All @@ -15,5 +15,6 @@
'top_10' => 'This is in the top-10 most common passwords',
'top_100' => 'This is in the top-100 most common passwords',
'digits' => 'Adding a series of digits does not improve security',
'reused' => 'Re-using information such as your name, username or email in the password is not secure',

];
92 changes: 48 additions & 44 deletions src/ZxcvbnValidator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace REBELinBLUE\Zxcvbn;

use Illuminate\Translation\Translator;
use Illuminate\Validation\Validator;
use InvalidArgumentException;
use ZxcvbnPhp\Zxcvbn;

Expand All @@ -29,18 +30,17 @@ public function __construct(Translator $translator)

public function validate(...$args)
{
/** @var string $value */
$value = trim($args[1]);

/** @var array $parameters */
$parameters = $args[2] ? $args[2] : [];

/** @var \Illuminate\Validation\Validator $validator */
$validator = $args[3];
$otherInput = []; // FIXME: Get this

$desiredScore = isset($parameters[0]) ? $parameters[0] : self::DEFAULT_MINIMUM_STRENGTH;

if ($desiredScore < 0 || $desiredScore > 4 || !is_numeric($desiredScore)) {
throw new InvalidArgumentException('The required password score must be between 0 and 4');
}
$desiredScore = $this->getDesiredScore($parameters);
$otherInput = $this->getAdditionalInput($validator, $parameters);

$zxcvbn = new Zxcvbn();
$strength = $zxcvbn->passwordStrength($value, $otherInput);
Expand All @@ -60,6 +60,31 @@ public function validate(...$args)
return false;
}

private function getDesiredScore(array $parameters = [])
{
$desiredScore = isset($parameters[0]) ? $parameters[0] : self::DEFAULT_MINIMUM_STRENGTH;

if ($desiredScore < 0 || $desiredScore > 4 || !ctype_digit($desiredScore)) {
throw new InvalidArgumentException('The required password score must be between 0 and 4');
}

return $desiredScore;
}

private function getAdditionalInput(Validator $validator, array $parameters = [])
{
$input = $validator->getData();

$otherInput = [];
foreach (array_slice($parameters, 1) as $attribute) {
if (isset($input[$attribute])) {
$otherInput[] = $input[$attribute];
}
}

return $otherInput;
}

private function getFeedbackTranslation()
{
$isOnlyMatch = count($this->result['match_sequence']) === 1;
Expand All @@ -68,9 +93,7 @@ private function getFeedbackTranslation()
$longestMatch->token = '';

foreach ($this->result['match_sequence'] as $match) {
if (strlen($match->token) > strlen($longestMatch->token) &&
preg_match('/Match$/', get_class($match)) // FIXME: HORRIBLE, BECAUSE OF THE BRUTE FORCE MATCHER
) {
if (strlen($match->token) > strlen($longestMatch->token)) {
$longestMatch = $match;
}
}
Expand All @@ -80,25 +103,29 @@ private function getFeedbackTranslation()

private function getMatchFeedback($match, $isOnlyMatch)
{
$strategy = 'get' . ucfirst($match->pattern) . 'Warning';
$pattern = strtolower($match->pattern);
$strategy = 'get' . ucfirst($pattern) . 'Warning';

if (method_exists($this, $strategy)) {
return $this->$strategy($match, $isOnlyMatch);
}

// FIXME: This should not happen
return $strategy;
// ['digits', 'year', 'date', 'repeat', 'sequence']
return strtolower($pattern);
}

/**
* @SuppressWarnings(PHPMD.UnusedPrivateMethod)
*/
private function getDictionaryWarning($match, $isOnlyMatch)
{
$warning = ''; // FIXME: Should it be possible for this to happen?
if ($match->dictionaryName == 'passwords') {
$warning = 'common'; // $match->dictionaryName == 'english'
if ($match->dictionaryName === 'passwords') {
$warning = $this->getPasswordWarning($match, $isOnlyMatch);
} elseif ($match->dictionaryName == 'english') {
$warning = 'common';
} elseif (in_array($match->dictionaryName, ['surnames', 'male_names', 'female_names'])) {
$warning = 'names';
} elseif ($match->dictionaryName === 'user_inputs') {
$warning = 'reused'; // FIXME: Think of a way to detail which field is reused
}

if (isset($match->l33t)) {
Expand All @@ -124,38 +151,15 @@ private function getPasswordWarning($match, $isOnlyMatch)
return $warning;
}

private function getSequenceWarning()
{
return 'sequence';
}

/**
* @SuppressWarnings(PHPMD.UnusedPrivateMethod)
*/
private function getSpatialWarning($match)
{
$translation = 'spatial_with_turns';
if ($match->turns === 1) {
$translation = 'straight_spatial';
return 'straight_spatial';
}

return $translation;
}

private function getRepeatWarning()
{
return 'repeat';
}

private function getDateWarning()
{
return 'dates';
}

private function getYearWarning()
{
return 'years';
}

private function getDigitWarning()
{
return 'digits';
return 'spatial_with_turns';
}
}
61 changes: 40 additions & 21 deletions tests/ZxcvbnTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,7 @@ public function testItThrowsAnExceptionWhenConfiguredScoreIsInvalid($score)

public function scoreDataProvider()
{
return [
[-1],
[5],
['invalid-score'],
];
return array_chunk(['-1', -10, 5, 3.4, '2.1', 'invalid-score', 0x1A, 0b111111], 1);
}

public function testItFailsOnGuessablePassword()
Expand All @@ -100,7 +96,7 @@ public function testItPassesOnSecurePassword()
}

/**
* @dataProvider validationDataProvider
* @dataProvider invalidPasswordDataProvider
*/
public function testItSetsTheCorrectErrorOnFailure($value, $expected)
{
Expand All @@ -120,23 +116,46 @@ public function testItSetsTheCorrectErrorOnFailure($value, $expected)
);
}

public function validationDataProvider()
public function invalidPasswordDataProvider()
{
return [
['test123456', 'common'], // Common
['poiuytghjkl', 'spatial_with_turns'], // Simple keyboard pattern
['poiuyt`', 'straight_spatial'], // Straight row of keys
['98761234', 'sequence'], // Sequence of characters
['30/09/1983', 'dates'], // Date
['StephenBall', 'names'], // Name
['aaaaaaaaa', 'repeat'], // Repeating characters
['password', 'top_10'], // Top 10 password
['trustno1', 'top_100'], // Top 100 password
['drowssap', 'very_common'], // Simple reversal of one of the top passwords
['P4$$w0rd', 'predictable'], // Predictable "l33t" substitutions
['seriously', 'common'], // Dictionary word
[date('Y'), 'years'] // Recent year
// ['crkuw297', 'digits'],
['test123456', 'common'], // Common
['poiuytghjkl', 'spatial_with_turns'], // Simple keyboard pattern
['poiuyt`', 'straight_spatial'], // Straight row of keys
['98761234', 'sequence'], // Sequence of characters
['30/09/1983', 'date'], // Date
['StephenBall', 'names'], // Name
['aaaaaaaaa', 'repeat'], // Repeating characters
['password', 'top_10'], // Top 10 password
['trustno1', 'top_100'], // Top 100 password
['drowssap', 'very_common'], // Simple reversal of one of the top passwords
['P4$$w0rd', 'predictable'], // Predictable "l33t" substitutions
['seriously', 'common'], // Dictionary word
[date('Y'), 'year'] // Recent year
];
}

public function testItAddsOtherInputToTheDictionary()
{
$data = [
'password' => 'rebelinblue',
'username' => 'REBELinBLUE',
'email' => '[email protected]',
'gender' => 'male'
];

$validator = $this->factory->make($data, [
'password' => 'zxcvbn:4,username,email,name',
]);

$this->assertFalse($validator->passes());


$translator = $this->app->make('translator');

$this->assertSame(
$translator->get('zxcvbn::validation.reused'),
$validator->errors()->first()
);
}
}

0 comments on commit e4436e2

Please sign in to comment.