Skip to content

Commit

Permalink
minor #52421 [PasswordHasher] [Tests] Do not invoke methods with addi…
Browse files Browse the repository at this point in the history
…tional arguments in tests (OskarStark)

This PR was merged into the 5.4 branch.

Discussion
----------

[PasswordHasher] [Tests] Do not invoke methods with additional arguments in tests

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | ---
| License       | MIT

Extracted from #52402

I think its worth to merge this in 5.4

Commits
-------

fdaefc4 [PasswordHasher][Tests] Do not invoke methods with additional arguments in tests
  • Loading branch information
nicolas-grekas committed Nov 2, 2023
2 parents fc22954 + fdaefc4 commit 6765a43
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 48 deletions.
Expand Up @@ -51,25 +51,25 @@ public function testValidation()
{
$hasher = new NativePasswordHasher();
$result = $hasher->hash('password', null);
$this->assertTrue($hasher->verify($result, 'password', null));
$this->assertFalse($hasher->verify($result, 'anotherPassword', null));
$this->assertFalse($hasher->verify($result, '', null));
$this->assertTrue($hasher->verify($result, 'password'));
$this->assertFalse($hasher->verify($result, 'anotherPassword'));
$this->assertFalse($hasher->verify($result, ''));
}

public function testNonArgonValidation()
{
$hasher = new NativePasswordHasher();
$this->assertTrue($hasher->verify('$5$abcdefgh$ZLdkj8mkc2XVSrPVjskDAgZPGjtj1VGVaa1aUkrMTU/', 'password', null));
$this->assertFalse($hasher->verify('$5$abcdefgh$ZLdkj8mkc2XVSrPVjskDAgZPGjtj1VGVaa1aUkrMTU/', 'anotherPassword', null));
$this->assertTrue($hasher->verify('$6$abcdefgh$yVfUwsw5T.JApa8POvClA1pQ5peiq97DUNyXCZN5IrF.BMSkiaLQ5kvpuEm/VQ1Tvh/KV2TcaWh8qinoW5dhA1', 'password', null));
$this->assertFalse($hasher->verify('$6$abcdefgh$yVfUwsw5T.JApa8POvClA1pQ5peiq97DUNyXCZN5IrF.BMSkiaLQ5kvpuEm/VQ1Tvh/KV2TcaWh8qinoW5dhA1', 'anotherPassword', null));
$this->assertTrue($hasher->verify('$5$abcdefgh$ZLdkj8mkc2XVSrPVjskDAgZPGjtj1VGVaa1aUkrMTU/', 'password'));
$this->assertFalse($hasher->verify('$5$abcdefgh$ZLdkj8mkc2XVSrPVjskDAgZPGjtj1VGVaa1aUkrMTU/', 'anotherPassword'));
$this->assertTrue($hasher->verify('$6$abcdefgh$yVfUwsw5T.JApa8POvClA1pQ5peiq97DUNyXCZN5IrF.BMSkiaLQ5kvpuEm/VQ1Tvh/KV2TcaWh8qinoW5dhA1', 'password'));
$this->assertFalse($hasher->verify('$6$abcdefgh$yVfUwsw5T.JApa8POvClA1pQ5peiq97DUNyXCZN5IrF.BMSkiaLQ5kvpuEm/VQ1Tvh/KV2TcaWh8qinoW5dhA1', 'anotherPassword'));
}

public function testConfiguredAlgorithm()
{
$hasher = new NativePasswordHasher(null, null, null, \PASSWORD_BCRYPT);
$result = $hasher->hash('password', null);
$this->assertTrue($hasher->verify($result, 'password', null));
$result = $hasher->hash('password');
$this->assertTrue($hasher->verify($result, 'password'));
$this->assertStringStartsWith('$2', $result);
}

Expand All @@ -84,8 +84,8 @@ public function testDefaultAlgorithm()
public function testConfiguredAlgorithmWithLegacyConstValue()
{
$hasher = new NativePasswordHasher(null, null, null, '1');
$result = $hasher->hash('password', null);
$this->assertTrue($hasher->verify($result, 'password', null));
$result = $hasher->hash('password');
$this->assertTrue($hasher->verify($result, 'password'));
$this->assertStringStartsWith('$2', $result);
}

Expand All @@ -94,17 +94,17 @@ public function testBcryptWithLongPassword()
$hasher = new NativePasswordHasher(null, null, 4, \PASSWORD_BCRYPT);
$plainPassword = str_repeat('a', 100);

$this->assertFalse($hasher->verify(password_hash($plainPassword, \PASSWORD_BCRYPT, ['cost' => 4]), $plainPassword, 'salt'));
$this->assertTrue($hasher->verify($hasher->hash($plainPassword), $plainPassword, 'salt'));
$this->assertFalse($hasher->verify(password_hash($plainPassword, \PASSWORD_BCRYPT, ['cost' => 4]), $plainPassword));
$this->assertTrue($hasher->verify($hasher->hash($plainPassword), $plainPassword));
}

public function testBcryptWithNulByte()
{
$hasher = new NativePasswordHasher(null, null, 4, \PASSWORD_BCRYPT);
$plainPassword = "a\0b";

$this->assertFalse($hasher->verify(password_hash($plainPassword, \PASSWORD_BCRYPT, ['cost' => 4]), $plainPassword, 'salt'));
$this->assertTrue($hasher->verify($hasher->hash($plainPassword), $plainPassword, 'salt'));
$this->assertFalse($hasher->verify(password_hash($plainPassword, \PASSWORD_BCRYPT, ['cost' => 4]), $plainPassword));
$this->assertTrue($hasher->verify($hasher->hash($plainPassword), $plainPassword));
}

public function testNeedsRehash()
Expand All @@ -113,7 +113,7 @@ public function testNeedsRehash()

$this->assertTrue($hasher->needsRehash('dummyhash'));

$hash = $hasher->hash('foo', 'salt');
$hash = $hasher->hash('foo');
$this->assertFalse($hasher->needsRehash($hash));

$hasher = new NativePasswordHasher(5, 11000, 5);
Expand Down
Expand Up @@ -110,7 +110,7 @@ public function testGetNamedHasherForHasherAware()
'hasher_name' => new MessageDigestPasswordHasher('sha1'),
]);

$hasher = $factory->getPasswordHasher(new HasherAwareUser('user', 'pass'));
$hasher = $factory->getPasswordHasher(new HasherAwareUser());
$expectedHasher = new MessageDigestPasswordHasher('sha1');
$this->assertEquals($expectedHasher->hash('foo', ''), $hasher->hash('foo', ''));
}
Expand All @@ -122,7 +122,7 @@ public function testGetNullNamedHasherForHasherAware()
'hasher_name' => new MessageDigestPasswordHasher('sha256'),
]);

$user = new HasherAwareUser('mathilde', 'krogulec');
$user = new HasherAwareUser();
$user->hasherName = null;
$hasher = $factory->getPasswordHasher($user);
$expectedHasher = new MessageDigestPasswordHasher('sha1');
Expand All @@ -137,7 +137,7 @@ public function testGetInvalidNamedHasherForHasherAware()
'hasher_name' => new MessageDigestPasswordHasher('sha256'),
]);

$user = new HasherAwareUser('user', 'pass');
$user = new HasherAwareUser();
$user->hasherName = 'invalid_hasher_name';
$factory->getPasswordHasher($user);
}
Expand Down Expand Up @@ -168,9 +168,9 @@ public function testMigrateFrom()
$hasher = $factory->getPasswordHasher(SomeUser::class);
$this->assertInstanceOf(MigratingPasswordHasher::class, $hasher);

$this->assertTrue($hasher->verify((new SodiumPasswordHasher())->hash('foo', null), 'foo', null));
$this->assertTrue($hasher->verify((new NativePasswordHasher(null, null, null, \PASSWORD_BCRYPT))->hash('foo', null), 'foo', null));
$this->assertTrue($hasher->verify($digest->hash('foo', null), 'foo', null));
$this->assertTrue($hasher->verify((new SodiumPasswordHasher())->hash('foo'), 'foo', null));
$this->assertTrue($hasher->verify((new NativePasswordHasher(null, null, null, \PASSWORD_BCRYPT))->hash('foo'), 'foo', null));
$this->assertTrue($hasher->verify($digest->hash('foo'), 'foo', null));
$this->assertStringStartsWith(\SODIUM_CRYPTO_PWHASH_STRPREFIX, $hasher->hash('foo', null));
}

Expand All @@ -190,8 +190,8 @@ public function testMigrateFromWithCustomInstance()
$hasher = $factory->getPasswordHasher(SomeUser::class);
$this->assertInstanceOf(MigratingPasswordHasher::class, $hasher);

$this->assertTrue($hasher->verify((new SodiumPasswordHasher())->hash('foo', null), 'foo', null));
$this->assertTrue($hasher->verify((new NativePasswordHasher(null, null, null, \PASSWORD_BCRYPT))->hash('foo', null), 'foo', null));
$this->assertTrue($hasher->verify((new SodiumPasswordHasher())->hash('foo'), 'foo', null));
$this->assertTrue($hasher->verify((new NativePasswordHasher(null, null, null, \PASSWORD_BCRYPT))->hash('foo'), 'foo', null));
$this->assertTrue($hasher->verify($digest->hash('foo', null), 'foo', null));
$this->assertStringStartsWith(\SODIUM_CRYPTO_PWHASH_STRPREFIX, $hasher->hash('foo', null));
}
Expand All @@ -213,8 +213,8 @@ public function testMigrateFromLegacy()
$hasher = $factory->getPasswordHasher(SomeUser::class);
$this->assertInstanceOf(MigratingPasswordHasher::class, $hasher);

$this->assertTrue($hasher->verify((new SodiumPasswordHasher())->hash('foo', null), 'foo', null));
$this->assertTrue($hasher->verify((new NativePasswordHasher(null, null, null, \PASSWORD_BCRYPT))->hash('foo', null), 'foo', null));
$this->assertTrue($hasher->verify((new SodiumPasswordHasher())->hash('foo'), 'foo', null));
$this->assertTrue($hasher->verify((new NativePasswordHasher(null, null, null, \PASSWORD_BCRYPT))->hash('foo'), 'foo', null));
$this->assertTrue($hasher->verify($plaintext->encodePassword('foo', null), 'foo', null));
$this->assertStringStartsWith(\SODIUM_CRYPTO_PWHASH_STRPREFIX, $hasher->hash('foo', null));
}
Expand Down
Expand Up @@ -28,65 +28,65 @@ protected function setUp(): void
public function testValidation()
{
$hasher = new SodiumPasswordHasher();
$result = $hasher->hash('password', null);
$this->assertTrue($hasher->verify($result, 'password', null));
$this->assertFalse($hasher->verify($result, 'anotherPassword', null));
$this->assertFalse($hasher->verify($result, '', null));
$result = $hasher->hash('password');
$this->assertTrue($hasher->verify($result, 'password'));
$this->assertFalse($hasher->verify($result, 'anotherPassword'));
$this->assertFalse($hasher->verify($result, ''));
}

public function testBcryptValidation()
{
$hasher = new SodiumPasswordHasher();
$this->assertTrue($hasher->verify('$2y$04$M8GDODMoGQLQRpkYCdoJh.lbiZPee3SZI32RcYK49XYTolDGwoRMm', 'abc', null));
$this->assertTrue($hasher->verify('$2y$04$M8GDODMoGQLQRpkYCdoJh.lbiZPee3SZI32RcYK49XYTolDGwoRMm', 'abc'));
}

public function testNonArgonValidation()
{
$hasher = new SodiumPasswordHasher();
$this->assertTrue($hasher->verify('$5$abcdefgh$ZLdkj8mkc2XVSrPVjskDAgZPGjtj1VGVaa1aUkrMTU/', 'password', null));
$this->assertFalse($hasher->verify('$5$abcdefgh$ZLdkj8mkc2XVSrPVjskDAgZPGjtj1VGVaa1aUkrMTU/', 'anotherPassword', null));
$this->assertTrue($hasher->verify('$6$abcdefgh$yVfUwsw5T.JApa8POvClA1pQ5peiq97DUNyXCZN5IrF.BMSkiaLQ5kvpuEm/VQ1Tvh/KV2TcaWh8qinoW5dhA1', 'password', null));
$this->assertFalse($hasher->verify('$6$abcdefgh$yVfUwsw5T.JApa8POvClA1pQ5peiq97DUNyXCZN5IrF.BMSkiaLQ5kvpuEm/VQ1Tvh/KV2TcaWh8qinoW5dhA1', 'anotherPassword', null));
$this->assertTrue($hasher->verify('$5$abcdefgh$ZLdkj8mkc2XVSrPVjskDAgZPGjtj1VGVaa1aUkrMTU/', 'password'));
$this->assertFalse($hasher->verify('$5$abcdefgh$ZLdkj8mkc2XVSrPVjskDAgZPGjtj1VGVaa1aUkrMTU/', 'anotherPassword'));
$this->assertTrue($hasher->verify('$6$abcdefgh$yVfUwsw5T.JApa8POvClA1pQ5peiq97DUNyXCZN5IrF.BMSkiaLQ5kvpuEm/VQ1Tvh/KV2TcaWh8qinoW5dhA1', 'password'));
$this->assertFalse($hasher->verify('$6$abcdefgh$yVfUwsw5T.JApa8POvClA1pQ5peiq97DUNyXCZN5IrF.BMSkiaLQ5kvpuEm/VQ1Tvh/KV2TcaWh8qinoW5dhA1', 'anotherPassword'));
}

public function testHashLength()
{
$this->expectException(InvalidPasswordException::class);
$hasher = new SodiumPasswordHasher();
$hasher->hash(str_repeat('a', 4097), 'salt');
$hasher->hash(str_repeat('a', 4097));
}

public function testCheckPasswordLength()
{
$hasher = new SodiumPasswordHasher();
$result = $hasher->hash(str_repeat('a', 4096), null);
$this->assertFalse($hasher->verify($result, str_repeat('a', 4097), null));
$this->assertTrue($hasher->verify($result, str_repeat('a', 4096), null));
$result = $hasher->hash(str_repeat('a', 4096));
$this->assertFalse($hasher->verify($result, str_repeat('a', 4097)));
$this->assertTrue($hasher->verify($result, str_repeat('a', 4096)));
}

public function testBcryptWithLongPassword()
{
$hasher = new SodiumPasswordHasher(null, null, 4);
$hasher = new SodiumPasswordHasher(null, null);
$plainPassword = str_repeat('a', 100);

$this->assertFalse($hasher->verify(password_hash($plainPassword, \PASSWORD_BCRYPT, ['cost' => 4]), $plainPassword, 'salt'));
$this->assertTrue($hasher->verify((new NativePasswordHasher(null, null, 4, \PASSWORD_BCRYPT))->hash($plainPassword), $plainPassword, 'salt'));
$this->assertFalse($hasher->verify(password_hash($plainPassword, \PASSWORD_BCRYPT, ['cost' => 4]), $plainPassword));
$this->assertTrue($hasher->verify((new NativePasswordHasher(null, null, 4, \PASSWORD_BCRYPT))->hash($plainPassword), $plainPassword));
}

public function testBcryptWithNulByte()
{
$hasher = new SodiumPasswordHasher(null, null, 4);
$hasher = new SodiumPasswordHasher(null, null);
$plainPassword = "a\0b";

$this->assertFalse($hasher->verify(password_hash($plainPassword, \PASSWORD_BCRYPT, ['cost' => 4]), $plainPassword, 'salt'));
$this->assertTrue($hasher->verify((new NativePasswordHasher(null, null, 4, \PASSWORD_BCRYPT))->hash($plainPassword), $plainPassword, 'salt'));
$this->assertFalse($hasher->verify(password_hash($plainPassword, \PASSWORD_BCRYPT, ['cost' => 4]), $plainPassword));
$this->assertTrue($hasher->verify((new NativePasswordHasher(null, null, 4, \PASSWORD_BCRYPT))->hash($plainPassword), $plainPassword));
}

public function testUserProvidedSaltIsNotUsed()
{
$hasher = new SodiumPasswordHasher();
$result = $hasher->hash('password', 'salt');
$this->assertTrue($hasher->verify($result, 'password', 'anotherSalt'));
$result = $hasher->hash('password');
$this->assertTrue($hasher->verify($result, 'password'));
}

public function testNeedsRehash()
Expand All @@ -95,7 +95,7 @@ public function testNeedsRehash()

$this->assertTrue($hasher->needsRehash('dummyhash'));

$hash = $hasher->hash('foo', 'salt');
$hash = $hasher->hash('foo');
$this->assertFalse($hasher->needsRehash($hash));

$hasher = new SodiumPasswordHasher(5, 11000);
Expand Down

0 comments on commit 6765a43

Please sign in to comment.