big boy refactors

src/web/backend/app/Http/Controllers/UsersController.php

@@ -85,7 +85,11 @@ class UsersController extends ApiController
 
         $presenter = new CreateUserAPIPresenter();
 
-        $createUser->create($this->request->all(), $presenter);
+        $attributes = $this->request->all();
+
+        $attributes['salt'] = config('app.key');
+
+        $createUser->create($attributes, $presenter);
 
         return $this->respondWithData($presenter->getViewModel());
     }
@@ -115,7 +119,11 @@ class UsersController extends ApiController
 
         $presenter = new UpdateUserAPIPresenter();
 
-        $updateUser->update($userId, $this->request->all(), $presenter);
+        $attributes = $this->request->all();
+
+        $attributes['salt'] = config('app.key');
+
+        $updateUser->update($userId, $attributes, $presenter);
 
         if ($presenter->hasError()) {
             return $this->respondWithError($presenter->getViewModel()['message']);

src/web/backend/database/seeds/UsersSeeder.php

@@ -26,8 +26,8 @@ class UsersSeeder extends Seeder
             'admin',
             '',
             'Admin User',
-            null,
             'admin@admin.user',
+            null,
             'Default Admin Password',
             null
         ));

src/web/backend/src/Entities/Doorcode.php

+<?php
+
+
+namespace Source\Entities;
+
+class Doorcode
+{
+    /**
+     * @var string
+     */
+    protected string $hash;
+
+    /**
+     * Construct from existing Doorcode hash
+     *
+     * @param string $hash
+     */
+    public function __construct(string $hash)
+    {
+        $this->hash = $hash;
+    }
+
+    /**
+     * @param string      $salt
+     * @param string|null $plaintext
+     * @return static|null
+     */
+    public static function hash(string $salt, ?string $plaintext): ?self
+    {
+        if (!$plaintext) {
+            return null;
+        }
+
+        // Two rounds of sha512 each salted
+        // As of PHP 7 the salt parameter to password_hash has been depreciated.
+        // As we need to be able to search for users by doorcode they either all need to have the same salt
+        // or no salt (I'd prefer a salt). This way the doorcode can be hashed and
+        // then easily searched for in the database without having to check against every user and rehash every
+        // time with the new salt. As such it is not as easy to use BCRYPT :(
+        // So I'll just shred it twice using sha512 with with (probably) the application key.
+        return new static(
+            hash(
+                'sha512',
+                hash('sha512', $plaintext . $salt)
+                . $salt
+            )
+        );
+    }
+
+    /**
+     * @return string
+     */
+    public function getHash(): string
+    {
+        return $this->hash;
+    }
+}

src/web/backend/src/Entities/Password.php

+<?php
+
+
+namespace Source\Entities;
+
+class Password
+{
+    protected const HASH_ALGORITHM = PASSWORD_BCRYPT;
+
+    /**
+     * @var string
+     */
+    protected string $hash;
+
+    /**
+     * Construct from existing password hash.
+     *
+     * @param string $hash
+     */
+    public function __construct(string $hash)
+    {
+        $this->hash = $hash;
+    }
+
+    /**
+     * @param string $plaintext
+     * @return static
+     */
+    public static function hash(?string $plaintext): ?self
+    {
+        if (!$plaintext) {
+            return null;
+        }
+
+        $hashed = password_hash($plaintext, self::HASH_ALGORITHM);
+        return new static($hashed);
+    }
+
+    /**
+     * @param string|null $plaintext
+     * @return bool
+     */
+    public function matches(?string $plaintext): bool
+    {
+        // If either is null, they do not match
+        if (!$plaintext || !$this->hash) {
+            return false;
+        }
+
+        return password_verify($plaintext, $this->hash);
+    }
+
+    /**
+     * @return string
+     */
+    public function getHash(): string
+    {
+        return $this->hash;
+    }
+}

src/web/backend/src/Entities/User.php

@@ -38,14 +38,14 @@ class User
     protected string $email;
 
     /**
-     * @var string|null
+     * @var \Source\Entities\Password|null
      */
-    protected ?string $password;
+    protected ?Password $password;
 
     /**
-     * @var string|null
+     * @var \Source\Entities\Doorcode|null
      */
-    protected ?string $doorcode;
+    protected ?Doorcode $doorcode;
 
     /**
      * @var Carbon|null
@@ -63,27 +63,27 @@ class User
     protected ?Carbon $updatedAt;
 
     /**
-     * @param int         $id
-     * @param string      $firstName
-     * @param string      $lastName
-     * @param string      $displayName
-     * @param string|null $emplid
-     * @param string      $email
-     * @param string|null $password
-     * @param string|null $doorcode
-     * @param Carbon|null $expiresAt
-     * @param Carbon|null $createdAt
-     * @param Carbon|null $updatedAt
+     * @param int                            $id
+     * @param string                         $firstName
+     * @param string                         $lastName
+     * @param string                         $displayName
+     * @param string                         $email
+     * @param string|null                    $emplid
+     * @param \Source\Entities\Password|null $password
+     * @param \Source\Entities\Doorcode|null $doorcode
+     * @param Carbon|null                    $expiresAt
+     * @param Carbon|null                    $createdAt
+     * @param Carbon|null                    $updatedAt
      */
     public function __construct(
         int $id,
         string $firstName,
         string $lastName,
         string $displayName,
-        ?string $emplid,
         string $email,
-        ?string $password,
-        ?string $doorcode,
+        ?string $emplid = null,
+        ?Password $password = null,
+        ?Doorcode $doorcode = null,
         ?Carbon $expiresAt = null,
         ?Carbon $createdAt = null,
         ?Carbon $updatedAt = null
@@ -186,13 +186,18 @@ class User
         $this->id = $id;
     }
 
+    /**
+     * @param string|null $email
+     * @param string|null $password
+     * @return bool
+     */
     public function matchCredentials(?string $email, ?string $password): bool
     {
-        if (!$password || !$email) {
+        if (!$this->password) {
             return false;
         }
 
-        return $this->getEmail() === $email && $this->getPassword() === $password;
+        return $this->hasEmailOf($email) && $this->password->matches($password);
     }
 
     /**
@@ -204,9 +209,9 @@ class User
     }
 
     /**
-     * @return string|null
+     * @return \Source\Entities\Password|null
      */
-    public function getPassword(): ?string
+    public function getPassword(): ?Password
     {
         return $this->password;
     }
@@ -215,28 +220,40 @@ class User
      * @param string $doorcode
      * @return bool
      */
-    public function hasDoorcodeOf(?string $doorcode): bool
+    public function hasDoorcodeOf(?Doorcode $doorcode): bool
     {
-        if (!$doorcode) {
+        if (!$doorcode || !$this->getDoorcode()) {
             return false;
         }
 
-        return $this->getDoorcode() === $doorcode;
+        return $this->getDoorcode()->getHash() === $doorcode;
     }
 
     /**
-     * @return string|null
+     * @return \Source\Entities\Doorcode|null
      */
-    public function getDoorcode(): ?string
+    public function getDoorcode(): ?Doorcode
     {
         return $this->doorcode;
     }
 
+    /**
+     * @param string|null $email
+     * @return bool
+     */
     public function hasEmailOf(?string $email): bool
     {
+        if (!$email) {
+            return false;
+        }
+
         return $this->getEmail() === strtolower($email);
     }
 
+    /**
+     * @param string|null $name
+     * @return bool
+     */
     public function hasFirstNameOf(?string $name): bool
     {
         if (!$name) {
@@ -246,6 +263,10 @@ class User
         return $this->getFirstName() === $name;
     }
 
+    /**
+     * @param \Source\Entities\User|null $user
+     * @return bool
+     */
     public function is(?User $user): bool
     {
         if (!$user) {

src/web/backend/src/Gateways/Users/DatabaseUsersRepository.php

@@ -4,6 +4,8 @@
 namespace Source\Gateways\Users;
 
 use Source\Entities\User;
+use Source\Entities\Doorcode;
+use Source\Entities\Password;
 
 class DatabaseUsersRepository implements UsersRepository
 {
@@ -32,10 +34,10 @@ class DatabaseUsersRepository implements UsersRepository
             $user->first_name,
             $user->last_name,
             $user->display_name,
-            $user->emplid,
             $user->email,
-            $user->password,
-            $user->doorcode,
+            $user->emplid,
+            $user->password === null ? null : new Password($user->password),
+            $user->doorcode === null ? null : new Doorcode($user->doorcode),
             $user->expires_at,
             $user->created_at,
             $user->updated_at
@@ -82,41 +84,12 @@ class DatabaseUsersRepository implements UsersRepository
         $dbUser->emplid = $user->getEmplid();
         $dbUser->email = $user->getEmail();
         $dbUser->expires_at = $user->getExpiresAt();
-
-        // If the password exists and is the same as provided, don't change
-        // Else regenerate
-        if (!isset($dbUser->password) || (isset($dbUser->password) && $dbUser->password !== $user->getPassword())) {
-            $dbUser->password = bcrypt($user->getPassword());
-        }
-
-        // If the doorcode exists and is the same as provided, don't change
-        // Else regenerate
-        if (!isset($dbUser->doorcode) || (isset($dbUser->doorcode) && $dbUser->doorcode !== $user->getDoorcode())) {
-            $dbUser->doorcode = self::secureDoorcode($user->getDoorcode());
-        }
+        $dbUser->password = $user->getPassword() === null ? null : $user->getPassword()->getHash();
+        $dbUser->doorcode = $user->getDoorcode() === null ? null : $user->getDoorcode()->getHash();
 
         return $dbUser;
     }
 
-    /**
-     * @param string|null $doorcode
-     * @return string|null
-     */
-    public static function secureDoorcode(?string $doorcode): ?string
-    {
-        if (!$doorcode) {
-            return null;
-        }
-
-        // As of PHP 7 the salt parameter to password_hash has been depreciated.
-        // As we need to be able to search for users by doorcode they either all need to have the same salt
-        // (in this case, the app key) or no salt (I'd prefer a salt). This way the doorcode can be hashed and
-        // then easily searched for in the database without having to check against every user and rehash every
-        // time with the new salt. As such it is not as easy to use BCRYPT :(
-        // So I'll just shred it twice using sha512 with with the application key.
-        return hash('sha512', hash('sha512', $doorcode . config('app.key')));
-    }
-
     /**
      * @inheritDoc
      */
@@ -154,29 +127,13 @@ class DatabaseUsersRepository implements UsersRepository
     /**
      * @inheritDoc
      */
-    public function findByCredentials(string $email, string $password): ?User
+    public function findByDoorcode(?Doorcode $doorcode): ?User
     {
-        $user = \App\User::where('email', $email)->first();
-
-        if (!$user) {
-            return null;
-        }
-
-        if (!password_verify($password, $user->password)) {
+        if (!$doorcode) {
             return null;
         }
 
-        return self::makeUserFromDbUser($user);
-    }
-
-    /**
-     * @inheritDoc
-     */
-    public function findByDoorcode(string $doorcode): ?User
-    {
-        $doorcodeSearch = self::secureDoorcode($doorcode);
-
-        $user = \App\User::where('doorcode', $doorcodeSearch)->first();
+        $user = \App\User::where('doorcode', $doorcode->getHash())->first();
 
         if (!$user) {
             return null;

src/web/backend/src/Gateways/Users/InMemoryUsersRepository.php

@@ -4,6 +4,7 @@
 namespace Source\Gateways\Users;
 
 use Source\Entities\User;
+use Source\Entities\Doorcode;
 
 class InMemoryUsersRepository implements UsersRepository
 {
@@ -97,21 +98,7 @@ class InMemoryUsersRepository implements UsersRepository
     /**
      * @inheritDoc
      */
-    public function findByCredentials(string $email, string $password): ?User
-    {
-        foreach ($this->users as $user) {
-            if ($user->matchCredentials($email, $password)) {
-                return $user;
-            }
-        }
-
-        return null;
-    }
-
-    /**
-     * @inheritDoc
-     */
-    public function findByDoorcode(string $doorcode): ?User
+    public function findByDoorcode(?Doorcode $doorcode): ?User
     {
         foreach ($this->users as $user) {
             if ($user->hasDoorcodeOf($doorcode)) {

src/web/backend/src/Gateways/Users/LocalUsersRepository.php

@@ -32,8 +32,8 @@ class LocalUsersRepository extends InMemoryUsersRepository
             'Sheev',
             'Palpatine',
             'The Emperor',
-            'execute order 66',
             'sithL0rd@senate.com',
+            'execute order 66',
             'I am the senate',
             '123456',
             Carbon::now()->addDays(3),
@@ -53,13 +53,13 @@ class LocalUsersRepository extends InMemoryUsersRepository
             'Kobe',
             'Bryant',
             'Cobain Bryant',
-            '299012',
             'he ded',
+            '299012',
             'kobe didn\' miss his last shot',
             '12453',
             new Carbon('2020-01-26 09:06:00'),
             null,
-            null,
+            null
         );
     }
 
@@ -74,8 +74,8 @@ class LocalUsersRepository extends InMemoryUsersRepository
             'Jacob',
             'Priddy',
             'JD Priddy',
-            '201565',
             'email idk',
+            '201565',
             'not gonna be plaintext just placeholder here',
             '123866',
             null,
@@ -95,8 +95,8 @@ class LocalUsersRepository extends InMemoryUsersRepository
             'Jarod',
             'Owen',
             'JJ Obob',
-            '177013',
             'jarod.owen@wallawalla.edu',
+            '177013',
             'not rlly plaintext password',
             '42069',
             null,

src/web/backend/src/Gateways/Users/UsersRepository.php

@@ -4,6 +4,7 @@
 namespace Source\Gateways\Users;
 
 use Source\Entities\User;
+use Source\Entities\Doorcode;
 
 interface UsersRepository
 {
@@ -43,22 +44,13 @@ interface UsersRepository
      */
     public function exists(string $userId): bool;
 
-    /**
-     * Find a user by username and password
-     *
-     * @param string $email
-     * @param string $password
-     * @return User|null
-     */
-    public function findByCredentials(string $email, string $password): ?User;
-
     /**
      * Find a user by doorcode
      *
-     * @param string $doorcode
+     * @param \Source\Entities\Doorcode|null $doorcode
      * @return User|null
      */
-    public function findByDoorcode(string $doorcode): ?User;
+    public function findByDoorcode(?Doorcode $doorcode): ?User;
 
     /**
      * Find a user by email

src/web/backend/src/UseCases/Users/Authenticate/Authenticate.php

@@ -52,9 +52,11 @@ class Authenticate implements AuthenticateUseCase
             throw new AuthenticationException();
         }
 
-        $user = $this->users->findByCredentials(strtolower($email), $password);
+        $user = $this->users->findByEmail(strtolower($email));
 
-        if (!$user) {
+        if (!$user ||
+            !$user->getPassword() ||
+            !$user->getPassword()->matches($password)) {
             throw new AuthenticationException();
         }
 
@@ -103,8 +105,8 @@ class Authenticate implements AuthenticateUseCase
                     $samlUser->getFirstName(),
                     $samlUser->getLastName(),
                     $samlUser->getDisplayName(),
-                    $samlUser->getEmplid(),
                     $samlUser->getEmail(),
+                    $samlUser->getEmplid(),
                     null,
                     null
                 )

src/web/backend/src/UseCases/Users/CreateUser/CreateUser.php

@@ -5,6 +5,8 @@ namespace Source\UseCases\Users\CreateUser;
 use Exception;
 use Carbon\Carbon;
 use Source\Entities\User;
+use Source\Entities\Doorcode;
+use Source\Entities\Password;
 use Source\Gateways\Users\UsersRepository;
 use Source\Exceptions\EntityExistsException;
 
@@ -40,13 +42,13 @@ class CreateUser implements CreateUserUseCase
             $attributes['first_name'],
             $attributes['last_name'],
             $attributes['display_name'],
-            $emplid,
             $attributes['email'],
-            $password,
-            $attributes['doorcode'],
+            $emplid,
+            Password::hash($password),
+            Doorcode::hash($attributes['salt'], $attributes['doorcode']),
             $expires,
             null,
-            null,
+            null
         );
 
 

src/web/backend/src/UseCases/Users/CreateUser/CreateUserUseCase.php

@@ -15,6 +15,7 @@ interface CreateUserUseCase
      *  email
      *  password
      *  doorcode
+     *  salt (The salt to hash the doorcode with)
      * Optional attributes
      *  emplid
      *  expires_at

src/web/backend/src/UseCases/Users/UpdateUser/UpdateUser.php

@@ -5,6 +5,8 @@ namespace Source\UseCases\Users\UpdateUser;
 use Exception;
 use Carbon\Carbon;
 use Source\Entities\User;
+use Source\Entities\Doorcode;
+use Source\Entities\Password;
 use Source\Authorization\Authorizer;
 use Source\Gateways\Users\UsersRepository;
 use Source\Exceptions\EntityNotFoundException;
@@ -22,7 +24,8 @@ class UpdateUser implements UpdateUserUseCase
     protected Authorizer $authorizer;
 
     /**
-     * @param UsersRepository $usersRepository
+     * @param \Source\Authorization\Authorizer       $authorizer
+     * @param \Source\Gateways\Users\UsersRepository $usersRepository
      */
     public function __construct(Authorizer $authorizer, UsersRepository $usersRepository)
     {
@@ -54,13 +57,13 @@ class UpdateUser implements UpdateUserUseCase
             $attributes['first_name'],
             $attributes['last_name'],