Skip to content

Commit

Permalink
Tests checked in for password-strength checker.
Browse files Browse the repository at this point in the history
Adding password-strength checker functionality.
Modified docs to support option.
  • Loading branch information
Saeven committed Mar 10, 2017
1 parent dabc284 commit 052851d
Show file tree
Hide file tree
Showing 9 changed files with 123 additions and 17 deletions.
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ configure it like so:
],

Writing your own should be very simple, see provided tests.

## Pluggable Password Strength Checker

You can use the built-in support for [paragonie/passwdqc](https://github.com/paragonie/passwdqc) by uncommenting the password_strength_checker config key. You can also roll your own if you have more complex needs; uncomment the key and specify your own implementation of [PasswordCheckerInterface](src/CirclicalUser/Provider/PasswordCheckerInterface.php). This will cause the
password input routines to throw WeakPasswordExceptions when weak input is received.

## Creating Access For Your Users
Expand Down
28 changes: 28 additions & 0 deletions bundle/Spec/CirclicalUser/Service/AuthenticationServiceSpec.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use CirclicalUser\Entity\Authentication;
use CirclicalUser\Exception\PersistedUserRequiredException;
use CirclicalUser\Exception\WeakPasswordException;
use CirclicalUser\Provider\AuthenticationRecordInterface;
use CirclicalUser\Provider\UserInterface as User;
use CirclicalUser\Exception\BadPasswordException;
Expand All @@ -14,6 +15,7 @@
use CirclicalUser\Mapper\AuthenticationMapper;
use CirclicalUser\Mapper\UserMapper;
use CirclicalUser\Service\AuthenticationService;
use CirclicalUser\Service\PasswordChecker\Passwdqc;
use ParagonIE\Halite\KeyFactory;
use ParagonIE\Halite\Symmetric\Crypto;
use ParagonIE\Halite\Symmetric\EncryptionKey;
Expand Down Expand Up @@ -415,4 +417,30 @@ public function it_requires_that_users_have_id_during_creation(User $otherUser)
$otherUser->getId()->willReturn(null);
$this->shouldThrow(PersistedUserRequiredException::class)->during('create', [$otherUser, 'whoami', 'nobody']);
}

public function it_will_create_new_auth_records_with_strong_passwords($authenticationMapper, User $user5, AuthenticationRecordInterface $newAuth, $userMapper)
{
$this->beConstructedWith($authenticationMapper, $userMapper, $this->systemEncryptionKey->getRawKeyMaterial(), false, false, new Passwdqc());

$newAuth->getSessionKey()->willReturn(KeyFactory::generateEncryptionKey()->getRawKeyMaterial());
$newAuth->getUsername()->willReturn('email');
$newAuth->getUserId()->willReturn(5);
$user5->getId()->willReturn(5);

$authenticationMapper->save(Argument::type(AuthenticationRecordInterface::class))->shouldBeCalled();
$authenticationMapper->create(Argument::type('integer'), Argument::type('string'), Argument::type('string'), Argument::type('string'))->willReturn($newAuth);
$this->create($user5, 'userC', 'beestring')->shouldBeAnInstanceOf(AuthenticationRecordInterface::class);
}

public function it_wont_create_new_auth_records_with_weak_passwords($authenticationMapper, User $user5, AuthenticationRecordInterface $newAuth, $userMapper)
{
$this->beConstructedWith($authenticationMapper, $userMapper, $this->systemEncryptionKey->getRawKeyMaterial(), false, false, new Passwdqc());

$newAuth->getSessionKey()->willReturn(KeyFactory::generateEncryptionKey()->getRawKeyMaterial());
$newAuth->getUsername()->willReturn('email');
$newAuth->getUserId()->willReturn(5);
$user5->getId()->willReturn(5);

$this->shouldThrow(WeakPasswordException::class)->during('create', [$user5, 'userC', '123456']);
}
}
6 changes: 5 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@
"require-dev": {
"phpspec/phpspec": "^3.1",
"henrikbjorn/phpspec-code-coverage": "^3.0",
"codacy/coverage": "^1.0"
"codacy/coverage": "^1.0",
"paragonie/passwdqc": "dev-master"
},
"suggest":{
"paragonie/passwdqc": "Add built-in support for password-strength checking! See README for configuration details."
},
"autoload": {
"psr-4": {
Expand Down
2 changes: 2 additions & 0 deletions config/module.config.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use CirclicalUser\Service\AccessService;
use CirclicalUser\Service\AuthenticationService;
use CirclicalUser\Factory\Service\AuthenticationServiceFactory;
use CirclicalUser\Service\PasswordChecker\Passwdqc;
use CirclicalUser\Strategy\RedirectStrategy;

return [
Expand All @@ -38,6 +39,7 @@
'user' => UserPermissionMapper::class,
],
],
//'password_strength_checker' => Passwdqc::class,
],
],

Expand Down
8 changes: 8 additions & 0 deletions src/CirclicalUser/Exception/WeakPasswordException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

namespace CirclicalUser\Exception;

class WeakPasswordException extends \Exception
{

}
17 changes: 14 additions & 3 deletions src/CirclicalUser/Factory/Service/AuthenticationServiceFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace CirclicalUser\Factory\Service;

use CirclicalUser\Provider\PasswordCheckerInterface;
use Interop\Container\ContainerInterface;
use Interop\Container\Exception\ContainerException;
use Zend\ServiceManager\Exception\ServiceNotCreatedException;
Expand Down Expand Up @@ -31,15 +32,25 @@ public function __invoke(ContainerInterface $container, $requestedName, array $o
$config = $container->get('config');
$userConfig = $config['circlical']['user'];

$userProvider = isset($userConfig['providers']['user']) ? $userConfig['providers']['user'] : UserMapper::class;
$authMapper = isset($userConfig['providers']['auth']) ? $userConfig['providers']['auth'] : AuthenticationMapper::class;
$userProvider = $userConfig['providers']['user'] ?? UserMapper::class;
$authMapper = $userConfig['providers']['auth'] ?? AuthenticationMapper::class;
$passwordChecker = null;

if( !empty( $userConfig['password_strength_checker'] ) ){
$checkerImplementation = new $userConfig['password_strength_checker'];
if( $checkerImplementation instanceof PasswordCheckerInterface ){
$passwordChecker = $checkerImplementation;
}
}


return new AuthenticationService(
$container->get($authMapper),
$container->get($userProvider),
base64_decode($userConfig['auth']['crypto_key']),
$userConfig['auth']['transient'],
false
false,
$passwordChecker
);
}
}
8 changes: 8 additions & 0 deletions src/CirclicalUser/Provider/PasswordCheckerInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

namespace CirclicalUser\Provider;

interface PasswordCheckerInterface
{
public function isStrongPassword(string $clearPassword): bool;
}
49 changes: 36 additions & 13 deletions src/CirclicalUser/Service/AuthenticationService.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@


use CirclicalUser\Exception\PersistedUserRequiredException;
use CirclicalUser\Exception\WeakPasswordException;
use CirclicalUser\Provider\AuthenticationProviderInterface;
use CirclicalUser\Provider\AuthenticationRecordInterface;
use CirclicalUser\Provider\PasswordCheckerInterface;
use CirclicalUser\Provider\UserInterface as User;
use CirclicalUser\Exception\BadPasswordException;
use CirclicalUser\Exception\EmailUsernameTakenException;
Expand Down Expand Up @@ -82,6 +84,11 @@ class AuthenticationService
private $secure;


/**
* @var PasswordCheckerInterface
*/
private $passwordChecker;

/**
* AuthenticationService constructor.
*
Expand All @@ -90,15 +97,16 @@ class AuthenticationService
* @param string $systemEncryptionKey The raw material of a Halite-generated encryption key, stored in config.
* @param bool $transient True if cookies should expire at the end of the session (zero value, for expiry)
* @param bool $secure True if cookies should be marked as 'Secure', enforced as 'true' in production by this service's Factory
* @param PasswordCheckerInterface $passwordChecker Optional, a password checker implementation
*/
public function __construct(AuthenticationProviderInterface $authenticationProvider, UserProviderInterface $userProvider, $systemEncryptionKey, $transient, $secure)
public function __construct(AuthenticationProviderInterface $authenticationProvider, UserProviderInterface $userProvider, string $systemEncryptionKey, bool $transient, bool $secure, $passwordChecker = null)
{
$this->authenticationProvider = $authenticationProvider;
$this->userProvider = $userProvider;
$this->systemEncryptionKey = $systemEncryptionKey;
$this->transient = $transient;
$this->secure = $secure;
$this->identity = null;
$this->passwordChecker = $passwordChecker;
}

/**
Expand All @@ -107,7 +115,7 @@ public function __construct(AuthenticationProviderInterface $authenticationProvi
*/
public function hasIdentity(): bool
{
return $this->getIdentity() != null;
return $this->getIdentity() !== null;
}

/**
Expand All @@ -133,7 +141,7 @@ private function setIdentity(User $user)
* @throws BadPasswordException Thrown when the password doesn't work
* @throws NoSuchUserException Thrown when the user can't be identified
*/
public function authenticate($username, $password): User
public function authenticate(string $username, string $password): User
{
$auth = $this->authenticationProvider->findByUsername($username);
$user = null;
Expand Down Expand Up @@ -185,7 +193,7 @@ public function authenticate($username, $password): User
* @throws NoSuchUserException Thrown when the user's authentication records couldn't be found
* @throws UsernameTakenException
*/
public function changeUsername(User $user, $newUsername): AuthenticationRecordInterface
public function changeUsername(User $user, string $newUsername): AuthenticationRecordInterface
{
/** @var AuthenticationRecordInterface $auth */
$auth = $this->authenticationProvider->findByUserId($user->getId());
Expand Down Expand Up @@ -220,7 +228,7 @@ private function setSessionCookies(AuthenticationRecordInterface $authentication
$systemKey = new EncryptionKey($this->systemEncryptionKey);
$userKey = new EncryptionKey($authentication->getSessionKey());
$hashCookieName = hash_hmac('sha256', $authentication->getSessionKey() . $authentication->getUsername(), $systemKey);
$userTuple = base64_encode(Crypto::encrypt($authentication->getUserId() . ":" . $hashCookieName, $systemKey));
$userTuple = base64_encode(Crypto::encrypt($authentication->getUserId() . ':' . $hashCookieName, $systemKey));
$hashCookieContents = base64_encode(Crypto::encrypt(time() . ':' . $authentication->getUserId() . ':' . $authentication->getUsername(), $userKey));

//
Expand Down Expand Up @@ -262,7 +270,7 @@ private function setSessionCookies(AuthenticationRecordInterface $authentication
* @param $name
* @param $value
*/
private function setCookie($name, $value)
private function setCookie(string $name, $value)
{
$expiry = $this->transient ? 0 : (time() + 2629743);
$sessionParameters = session_get_cookie_params();
Expand Down Expand Up @@ -332,7 +340,7 @@ public function getIdentity()

// paranoid, make sure we have everything we need
@list($cookieUserId, $hashCookieSuffix) = @explode(":", $userTuple, 2);
if (!isset($cookieUserId) || !isset($hashCookieSuffix) || !is_numeric($cookieUserId) || !trim($hashCookieSuffix)) {
if (!isset($cookieUserId, $hashCookieSuffix) || !is_numeric($cookieUserId) || !trim($hashCookieSuffix)) {
throw new \Exception();
}

Expand Down Expand Up @@ -364,7 +372,7 @@ public function getIdentity()
// 3. Decrypt the hash cookie with the user key
//
$hashedCookieContents = Crypto::decrypt(base64_decode($_COOKIE[$hashCookieName]), $userKey);
if (!substr_count($hashedCookieContents, ':') == 2) {
if (!substr_count($hashedCookieContents, ':') === 2) {
throw new \Exception();
}

Expand Down Expand Up @@ -405,12 +413,19 @@ private function purgeHashCookies(string $skipCookie = null)
{
$sp = session_get_cookie_params();
foreach ($_COOKIE as $cookieName => $value) {
if ($cookieName != $skipCookie && strpos($cookieName, self::COOKIE_HASH_PREFIX) !== false) {
if ($cookieName !== $skipCookie && strpos($cookieName, self::COOKIE_HASH_PREFIX) !== false) {
setcookie($cookieName, null, null, '/', $sp['domain'], false, true);
}
}
}

private function enforcePasswordStrength(string $password)
{
if ($this->passwordChecker && !$this->passwordChecker->isStrongPassword($password)) {
throw new WeakPasswordException();
}
}


/**
* Reset this user's password
Expand All @@ -419,9 +434,12 @@ private function purgeHashCookies(string $skipCookie = null)
* @param string $newPassword Cleartext password that's being hashed
*
* @throws NoSuchUserException
* @throws WeakPasswordException
*/
public function resetPassword(User $user, $newPassword)
public function resetPassword(User $user, string $newPassword)
{
$this->enforcePasswordStrength($newPassword);

$auth = $this->authenticationProvider->findByUserId($user->getId());
if (!$auth) {
throw new NoSuchUserException();
Expand All @@ -442,9 +460,12 @@ public function resetPassword(User $user, $newPassword)
* @return bool
*
* @throws NoSuchUserException
* @throws WeakPasswordException
*/
public function verifyPassword(User $user, string $password): bool
{
$this->enforcePasswordStrength($password);

$auth = $this->authenticationProvider->findByUserId($user->getId());
if (!$auth) {
throw new NoSuchUserException();
Expand All @@ -467,6 +488,8 @@ public function verifyPassword(User $user, string $password): bool
*/
public function create(User $user, string $username, string $password): AuthenticationRecordInterface
{
$this->enforcePasswordStrength($password);

$auth = $this->registerAuthenticationRecord($user, $username, $password);
$this->setSessionCookies($auth);
$this->setIdentity($user);
Expand Down Expand Up @@ -500,12 +523,12 @@ public function registerAuthenticationRecord(User $user, string $username, strin
}

if (filter_var($username, FILTER_VALIDATE_EMAIL)) {
if ($user->getEmail() != $username) {
if ($user->getEmail() !== $username) {
throw new MismatchedEmailsException();
}

if ($emailUser = $this->userProvider->findByEmail($username)) {
if ($emailUser != $user) {
if ($emailUser !== $user) {
throw new EmailUsernameTakenException();
}
}
Expand Down
16 changes: 16 additions & 0 deletions src/CirclicalUser/Service/PasswordChecker/Passwdqc.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

namespace CirclicalUser\Service\PasswordChecker;

use CirclicalUser\Provider\PasswordCheckerInterface;

class Passwdqc implements PasswordCheckerInterface
{

public function isStrongPassword(string $clearPassword): bool
{
$implementation = new \ParagonIE\Passwdqc\Passwdqc();

return $implementation->check($clearPassword);
}
}

0 comments on commit 052851d

Please sign in to comment.