From c2ebe29d77ba1f072bf0b5966559613bae70097a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Jaussoin?= Date: Tue, 19 Dec 2023 09:30:21 +0000 Subject: [PATCH] Fix #135 Refactor the password algorithms code --- CHANGELOG.md | 13 +++++++++++++ flexiapi/.env.example | 1 + flexiapi/app/Account.php | 12 +++--------- flexiapi/app/Helpers/Utils.php | 12 +++++++++--- .../Controllers/Account/PasswordController.php | 2 +- .../Controllers/Admin/AccountController.php | 7 ++----- .../Api/Account/AccountController.php | 6 +++++- .../Api/Account/PasswordController.php | 6 ++++-- .../Api/Admin/AccountController.php | 5 +++-- .../Middleware/AuthenticateDigestOrKey.php | 13 ++++--------- .../app/Http/Requests/UpdateAccountRequest.php | 1 - flexiapi/app/Rules/PasswordAlgorithm.php | 18 ++++++++++++++++++ flexiapi/app/Services/AccountService.php | 2 +- flexiapi/config/app.php | 1 + .../resources/views/account/password.blade.php | 4 ++-- .../parts/breadcrumb_accounts_index.blade.php | 3 +++ flexiapi/tests/TestCase.php | 4 +--- 17 files changed, 71 insertions(+), 39 deletions(-) create mode 100644 flexiapi/app/Rules/PasswordAlgorithm.php create mode 100644 flexiapi/resources/views/admin/account/parts/breadcrumb_accounts_index.blade.php diff --git a/CHANGELOG.md b/CHANGELOG.md index a87e33a..7081c98 100644 --- a/CHANGELOG.md +++ b/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 ---- diff --git a/flexiapi/.env.example b/flexiapi/.env.example index 1d94e98..5e6edd5 100644 --- a/flexiapi/.env.example +++ b/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= diff --git a/flexiapi/app/Account.php b/flexiapi/app/Account.php index d06a7bf..c22820e 100644 --- a/flexiapi/app/Account.php +++ b/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 diff --git a/flexiapi/app/Helpers/Utils.php b/flexiapi/app/Helpers/Utils.php index cca810a..20cece1 100644 --- a/flexiapi/app/Helpers/Utils.php +++ b/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() diff --git a/flexiapi/app/Http/Controllers/Account/PasswordController.php b/flexiapi/app/Http/Controllers/Account/PasswordController.php index d00d40d..c213bfc 100644 --- a/flexiapi/app/Http/Controllers/Account/PasswordController.php +++ b/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]); diff --git a/flexiapi/app/Http/Controllers/Admin/AccountController.php b/flexiapi/app/Http/Controllers/Admin/AccountController.php index 0e8f260..80f1482 100644 --- a/flexiapi/app/Http/Controllers/Admin/AccountController.php +++ b/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]); diff --git a/flexiapi/app/Http/Controllers/Api/Account/AccountController.php b/flexiapi/app/Http/Controllers/Api/Account/AccountController.php index 17e7732..71db563 100644 --- a/flexiapi/app/Http/Controllers/Api/Account/AccountController.php +++ b/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') diff --git a/flexiapi/app/Http/Controllers/Api/Account/PasswordController.php b/flexiapi/app/Http/Controllers/Api/Account/PasswordController.php index 103205d..da299f0 100644 --- a/flexiapi/app/Http/Controllers/Api/Account/PasswordController.php +++ b/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', ]); diff --git a/flexiapi/app/Http/Controllers/Api/Admin/AccountController.php b/flexiapi/app/Http/Controllers/Api/Admin/AccountController.php index 975ba0e..4944149 100644 --- a/flexiapi/app/Http/Controllers/Api/Admin/AccountController.php +++ b/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' ]); diff --git a/flexiapi/app/Http/Middleware/AuthenticateDigestOrKey.php b/flexiapi/app/Http/Middleware/AuthenticateDigestOrKey.php index b5a1fbb..73a6a39 100644 --- a/flexiapi/app/Http/Middleware/AuthenticateDigestOrKey.php +++ b/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) diff --git a/flexiapi/app/Http/Requests/UpdateAccountRequest.php b/flexiapi/app/Http/Requests/UpdateAccountRequest.php index 3f58367..da381bc 100644 --- a/flexiapi/app/Http/Requests/UpdateAccountRequest.php +++ b/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', diff --git a/flexiapi/app/Rules/PasswordAlgorithm.php b/flexiapi/app/Rules/PasswordAlgorithm.php new file mode 100644 index 0000000..00a42d8 --- /dev/null +++ b/flexiapi/app/Rules/PasswordAlgorithm.php @@ -0,0 +1,18 @@ +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(); diff --git a/flexiapi/config/app.php b/flexiapi/config/app.php index 6694d68..2e6149f 100644 --- a/flexiapi/config/app.php +++ b/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 diff --git a/flexiapi/resources/views/account/password.blade.php b/flexiapi/resources/views/account/password.blade.php index cc5a02a..260be1f 100644 --- a/flexiapi/resources/views/account/password.blade.php +++ b/flexiapi/resources/views/account/password.blade.php @@ -13,7 +13,7 @@ @endif Cancel - +
@@ -24,7 +24,7 @@
- +
diff --git a/flexiapi/resources/views/admin/account/parts/breadcrumb_accounts_index.blade.php b/flexiapi/resources/views/admin/account/parts/breadcrumb_accounts_index.blade.php new file mode 100644 index 0000000..a7a1266 --- /dev/null +++ b/flexiapi/resources/views/admin/account/parts/breadcrumb_accounts_index.blade.php @@ -0,0 +1,3 @@ +
  • + Accounts +
    • \ No newline at end of file diff --git a/flexiapi/tests/TestCase.php b/flexiapi/tests/TestCase.php index 4e1076f..df74a88 100644 --- a/flexiapi/tests/TestCase.php +++ b/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;