Tests checked in for password-strength checker.

Adding password-strength checker functionality. Modified docs to support option.
Saeven · Mar 10, 2017 · 052851d · 052851d
1 parent dabc284
commit 052851d
Show file tree

Hide file tree

Showing 9 changed files with 123 additions and 17 deletions.
diff --git a/README.md b/README.md
@@ -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

diff --git a/bundle/Spec/CirclicalUser/Service/AuthenticationServiceSpec.php b/bundle/Spec/CirclicalUser/Service/AuthenticationServiceSpec.php
@@ -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;
@@ -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;
@@ -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']);
+    }
 }
diff --git a/composer.json b/composer.json
@@ -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": {

diff --git a/config/module.config.php b/config/module.config.php
@@ -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 [
@@ -38,6 +39,7 @@
                     'user' => UserPermissionMapper::class,
                 ],
             ],
+            //'password_strength_checker' => Passwdqc::class,
         ],
     ],
 

diff --git a/src/CirclicalUser/Exception/WeakPasswordException.php b/src/CirclicalUser/Exception/WeakPasswordException.php
@@ -0,0 +1,8 @@
+<?php
+
+namespace CirclicalUser\Exception;
+
+class WeakPasswordException extends \Exception
+{
+
+}
diff --git a/src/CirclicalUser/Factory/Service/AuthenticationServiceFactory.php b/src/CirclicalUser/Factory/Service/AuthenticationServiceFactory.php
@@ -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;
@@ -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
         );
     }
 }
diff --git a/src/CirclicalUser/Provider/PasswordCheckerInterface.php b/src/CirclicalUser/Provider/PasswordCheckerInterface.php
@@ -0,0 +1,8 @@
+<?php
+
+namespace CirclicalUser\Provider;
+
+interface PasswordCheckerInterface
+{
+    public function isStrongPassword(string $clearPassword): bool;
+}
diff --git a/src/CirclicalUser/Service/AuthenticationService.php b/src/CirclicalUser/Service/AuthenticationService.php
@@ -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;
@@ -82,6 +84,11 @@ class AuthenticationService
     private $secure;
 
 
+    /**
+     * @var PasswordCheckerInterface
+     */
+    private $passwordChecker;
+
     /**
      * AuthenticationService constructor.
      *
@@ -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;
     }
 
     /**
@@ -107,7 +115,7 @@ public function __construct(AuthenticationProviderInterface $authenticationProvi
      */
     public function hasIdentity(): bool
     {
-        return $this->getIdentity() != null;
+        return $this->getIdentity() !== null;
     }
 
     /**
@@ -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;
@@ -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());
@@ -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));
 
         //
@@ -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();
@@ -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();
             }
 
@@ -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();
             }
 
@@ -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
@@ -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();
@@ -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();
@@ -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);
@@ -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();
                 }
             }

diff --git a/src/CirclicalUser/Service/PasswordChecker/Passwdqc.php b/src/CirclicalUser/Service/PasswordChecker/Passwdqc.php
@@ -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);
+    }
+}