Fix #135 Refactor the password algorithms code

CHANGELOG.md

@@ -2,6 +2,19 @@
 
 v1.5
 ----
+- Fix #135 Refactor the password algorithms code
+- Fix #134 Create an Activity view in the Admin > Accounts panel
+- Fix #133 Make the MySQL connection unstrict
+- Fix #132 Move the provisioning_tokens and recovery_codes to dedicated table
+- Fix #130 Drop the group column in the Accounts table
+
+v1.4.2
+------
+- Fix #135 Refactor the password algorithms code
+
+v1.4.1
+------
+- Fix #133 Make the MySQL connection unstrict
 
 v1.4
 ----

flexiapi/.env.example

@@ -26,6 +26,7 @@ ACCOUNT_EMAIL_UNIQUE=false # Emails are unique between all the accounts
 ACCOUNT_CONSUME_EXTERNAL_ACCOUNT_ON_CREATE=false
 ACCOUNT_BLACKLISTED_USERNAMES=
 ACCOUNT_USERNAME_REGEX="^[a-z0-9+_.-]*$"
+ACCOUNT_DEFAULT_PASSWORD_ALGORITHM=SHA-256 # Can ONLY be MD5 or SHA-256 in capital, default to SHA-256
 
 # Account provisioning
 ACCOUNT_PROVISIONING_RC_FILE=

flexiapi/app/Account.php

@@ -416,8 +416,10 @@ class Account extends Authenticatable
         return !empty($this->recovery_code) && $this->updated_at->greaterThan($oneHourAgo);
     }
 
-    public function updatePassword($newPassword, string $algorithm = 'SHA-256')
+    public function updatePassword($newPassword, ?string $algorithm = null)
     {
+        $algorithm = $algorithm ?? config('app.account_default_password_algorithm');
+
         $this->passwords()->delete();
 
         $password = new Password;
@@ -427,14 +429,6 @@ class Account extends Authenticatable
         $password->save();
     }
 
-    public function fillPassword(Request $request)
-    {
-        if ($request->filled('password')) {
-            $this->algorithm = $request->has('password_sha256') ? 'SHA-256' : 'MD5';
-            $this->updatePassword($request->get('password'), $this->algorithm);
-        }
-    }
-
     public function toVcard4()
     {
         $vcard = 'BEGIN:VCARD

flexiapi/app/Helpers/Utils.php

@@ -28,6 +28,14 @@ use League\CommonMark\Extension\HeadingPermalink\HeadingPermalinkExtension;
 use League\CommonMark\Extension\TableOfContents\TableOfContentsExtension;
 use Illuminate\Support\Facades\DB;
 
+function passwordAlgorithms(): array
+{
+    return [
+        'MD5'     => 'md5',
+        'SHA-256' => 'sha256',
+    ];
+}
+
 function generateNonce(): string
 {
     return Str::random(32);
@@ -45,9 +53,7 @@ function generateValidNonce(Account $account): string
 
 function bchash(string $username, string $domain, string $password, string $algorithm = 'MD5')
 {
-    $algos = ['MD5' => 'md5', 'SHA-256' => 'sha256'];
-
-    return hash($algos[$algorithm], $username . ':' . $domain . ':' . $password);
+    return hash(passwordAlgorithms()[$algorithm], $username . ':' . $domain . ':' . $password);
 }
 
 function generatePin()

flexiapi/app/Http/Controllers/Account/PasswordController.php

@@ -45,7 +45,7 @@ class PasswordController extends Controller
         $account->activated = true;
         $account->save();
 
-        $account->updatePassword($request->get('password'), 'SHA-256');
+        $account->updatePassword($request->get('password'));
 
         if ($account->passwords()->count() > 0) {
             Log::channel('events')->info('Web: Password changed', ['id' => $account->identifier]);

flexiapi/app/Http/Controllers/Admin/AccountController.php

@@ -98,7 +98,7 @@ class AccountController extends Controller
         $account->save();
 
         $account->phone = $request->get('phone');
-        $account->fillPassword($request);
+        $account->updatePassword($request->get('password'));
 
         $account->refresh();
         $account->setRole($request->get('role'));
@@ -136,10 +136,7 @@ class AccountController extends Controller
 
         $account->phone = $request->get('phone');
 
-        if ($request->filled('password') && $request->filled('password_confirmation')) {
-            $account->fillPassword($request);
-        }
-
+        $account->updatePassword($request->get('password'));
         $account->setRole($request->get('role'));
 
         Log::channel('events')->info('Web Admin: Account updated', ['id' => $account->identifier]);

flexiapi/app/Http/Controllers/Api/Account/AccountController.php

@@ -31,15 +31,19 @@ use App\Account;
 use App\AccountCreationToken;
 use App\AccountTombstone;
 use App\Alias;
+
 use App\Http\Controllers\Account\AuthenticateController as WebAuthenticateController;
 use App\Http\Requests\CreateAccountRequest;
 use App\Libraries\OvhSMS;
 use App\Mail\RegisterConfirmation;
+
 use App\Rules\AccountCreationToken as RulesAccountCreationToken;
 use App\Rules\BlacklistedUsername;
 use App\Rules\NoUppercase;
 use App\Rules\SIPUsername;
 use App\Rules\WithoutSpaces;
+use App\Rules\PasswordAlgorithm;
+
 use App\Services\AccountService;
 
 class AccountController extends Controller
@@ -104,7 +108,7 @@ class AccountController extends Controller
                 }),
                 'filled',
             ],
-            'algorithm' => 'required|in:SHA-256,MD5',
+            'algorithm' => ['required', new PasswordAlgorithm],
             'password' => 'required|filled',
             'domain' => 'min:3',
             'email' => config('app.account_email_unique')

flexiapi/app/Http/Controllers/Api/Account/PasswordController.php

@@ -19,11 +19,13 @@
 
 namespace App\Http\Controllers\Api\Account;
 
-use App\Http\Controllers\Controller;
 use Illuminate\Http\Request;
 use Illuminate\Support\Facades\Mail;
 use Illuminate\Support\Facades\Log;
 
+use App\Http\Controllers\Controller;
+use App\Rules\PasswordAlgorithm;
+
 use App\Mail\ConfirmedRegistration;
 
 class PasswordController extends Controller
@@ -31,7 +33,7 @@ class PasswordController extends Controller
     public function update(Request $request)
     {
         $request->validate([
-            'algorithm' => 'required|in:SHA-256,MD5',
+            'algorithm' => ['required', new PasswordAlgorithm],
             'password' => 'required',
         ]);
 

flexiapi/app/Http/Controllers/Api/Admin/AccountController.php

@@ -33,6 +33,7 @@ use App\ContactsList;
 use App\Http\Controllers\Account\AuthenticateController as WebAuthenticateController;
 use App\Http\Requests\CreateAccountRequest;
 use App\Http\Requests\UpdateAccountRequest;
+use App\Rules\PasswordAlgorithm;
 use App\Services\AccountService;
 
 class AccountController extends Controller
@@ -109,7 +110,7 @@ class AccountController extends Controller
     public function store(CreateAccountRequest $request)
     {
         $request->validate([
-            'algorithm' => 'required|in:SHA-256,MD5',
+            'algorithm' => ['required', new PasswordAlgorithm],
             'admin' => 'boolean|nullable',
             'activated' => 'boolean|nullable',
             'confirmation_key_expires' => [
@@ -158,7 +159,7 @@ class AccountController extends Controller
     public function update(UpdateAccountRequest $request, int $accountId)
     {
         $request->validate([
-            'algorithm' => 'required|in:SHA-256,MD5',
+            'algorithm' => ['required', new PasswordAlgorithm],
             'admin' => 'boolean|nullable',
             'activated' => 'boolean|nullable'
         ]);

flexiapi/app/Http/Middleware/AuthenticateDigestOrKey.php

@@ -30,11 +30,6 @@ use Validator;
 
 class AuthenticateDigestOrKey
 {
-    const ALGORITHMS = [
-        'MD5'     => 'md5',
-        'SHA-256' => 'sha256',
-    ];
-
     /**
      * Handle an incoming request.
      *
@@ -106,7 +101,7 @@ class AuthenticateDigestOrKey
                 'cnonce'    => 'required',
                 'algorithm' => [
                     'required',
-                    Rule::in(array_keys(self::ALGORITHMS)),
+                    Rule::in(array_keys(passwordAlgorithms())),
                 ],
                 'username'  => 'required|in:'.$username,
             ])->validate();
@@ -130,7 +125,7 @@ class AuthenticateDigestOrKey
                 return $this->generateUnauthorizedResponse($account, 'Wrong algorithm');
             }
 
-            $hash = self::ALGORITHMS[$auth['algorithm']];
+            $hash = passwordAlgorithms()[$auth['algorithm']];
 
             // Hashing and checking
             $a1 = $password->algorithm == 'CLRTXT'
@@ -208,14 +203,14 @@ class AuthenticateDigestOrKey
 
         foreach ($account->passwords as $password) {
             if ($password->algorithm == 'CLRTXT') {
-                foreach (array_keys(self::ALGORITHMS) as $algorithm) {
+                foreach (array_keys(passwordAlgorithms()) as $algorithm) {
                     array_push(
                         $headers,
                         $this->generateAuthHeader($resolvedRealm, $algorithm, $nonce)
                     );
                 }
                 break;
-            } else if (\in_array($password->algorithm, array_keys(self::ALGORITHMS))) {
+            } else if (\in_array($password->algorithm, array_keys(passwordAlgorithms()))) {
                 array_push(
                     $headers,
                     $this->generateAuthHeader($resolvedRealm, $password->algorithm, $nonce)

flexiapi/app/Http/Requests/UpdateAccountRequest.php

@@ -39,7 +39,6 @@ class UpdateAccountRequest extends FormRequest
                 config('app.account_email_unique') ? Rule::unique('accounts', 'email')->ignore($this->route('account_id')) : null
             ],
             'role' => 'in:admin,end_user',
-            'password_sha256' => 'nullable|min:3',
             'dtmf_protocol' => 'nullable|in:' . Account::dtmfProtocolsRule(),
             'phone' => [
                 'nullable',

flexiapi/app/Rules/PasswordAlgorithm.php

@@ -0,0 +1,18 @@
+<?php
+
+namespace App\Rules;
+
+use Illuminate\Contracts\Validation\Rule;
+
+class PasswordAlgorithm implements Rule
+{
+    public function passes($attribute, $value)
+    {
+        return in_array($value, array_keys(passwordAlgorithms()));
+    }
+
+    public function message()
+    {
+        return 'The password algorithm must be in ' . implode(', ', array_keys(passwordAlgorithms()));
+    }
+}

flexiapi/app/Services/AccountService.php

@@ -72,7 +72,7 @@ class AccountService
         $account->confirmation_key = generatePin();
         $account->save();
 
-        $account->updatePassword($request->get('password'), $request->has('algorithm') ? $request->get('algorithm') : 'SHA-256');
+        $account->updatePassword($request->get('password'), $request->get('algorithm'));
 
         if ($this->api) {
             $token = AccountCreationToken::where('token', $request->get('account_creation_token'))->first();

flexiapi/config/app.php

@@ -32,6 +32,7 @@ return [
     'account_email_unique' => env('ACCOUNT_EMAIL_UNIQUE', false),
     'blacklisted_usernames' => env('ACCOUNT_BLACKLISTED_USERNAMES', ''),
     'account_username_regex' => env('ACCOUNT_USERNAME_REGEX', '^[a-z0-9+_.-]*$'),
+    'account_default_password_algorithm' => env('ACCOUNT_DEFAULT_PASSWORD_ALGORITHM', 'SHA-256'),
 
     /**
      * Time limit before the API Key and related cookie are expired

flexiapi/resources/views/account/password.blade.php

@@ -13,7 +13,7 @@
         @endif
 
         <a href="{{ route('account.dashboard') }}" class="btn btn-secondary oppose">Cancel</a>
-        <input form="password_updated" class="btn" type="submit" value="Change">
+        <input form="password_update" class="btn" type="submit" value="Change">
     </header>
 
     <form id="password_update" method="POST" action="{{ route('account.password.update') }}" accept-charset="UTF-8">
@@ -24,7 +24,7 @@
             <label for="password">New password</label>
         </div>
         <div>
-            <input type="password_confirmation" name="password_confirmation" required>
+            <input type="password" name="password_confirmation" required>
             <label for="password_confirmation">Password confirmation</label>
         </div>
     </form>

flexiapi/resources/views/admin/account/parts/breadcrumb_accounts_index.blade.php

@@ -0,0 +1,3 @@
+<li class="breadcrumb-item">
+    <a href="{{ route('admin.account.index') }}">Accounts</a>
+</li>

flexiapi/tests/TestCase.php

@@ -28,8 +28,6 @@ abstract class TestCase extends BaseTestCase
 {
     use CreatesApplication;
 
-    const ALGORITHMS = ['md5' => 'MD5', 'sha256' => 'SHA-256'];
-
     protected $route = '/api/accounts/me';
     protected $method = 'GET';
 
@@ -88,7 +86,7 @@ abstract class TestCase extends BaseTestCase
             $extractedChallenge['qop'],
             $response,
             $extractedChallenge['opaque'],
-            self::ALGORITHMS[$hash],
+            array_flip(passwordAlgorithms())[$hash],
         );
 
         return 'Digest ' . $digest;