From ec1bdba37679b22a8c909b9eba9c7754ce0997bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Jaussoin?= Date: Thu, 25 May 2023 15:15:50 +0000 Subject: [PATCH] Fix #95 PUT /accounts admin endpoint implementation --- flexiapi/app/Account.php | 37 +++++++- flexiapi/app/Helpers/Utils.php | 11 +++ .../Controllers/Admin/AccountController.php | 89 ++----------------- .../Api/Admin/AccountController.php | 83 ++++++++--------- flexiapi/app/Http/Controllers/Controller.php | 10 --- .../app/Http/Middleware/AuthenticateAdmin.php | 2 +- .../Http/Requests/CreateAccountRequest.php | 7 +- .../Http/Requests/UpdateAccountRequest.php | 7 +- .../resources/views/account/panel.blade.php | 2 +- .../api/documentation_markdown.blade.php | 15 ++++ flexiapi/routes/api.php | 5 +- flexiapi/tests/Feature/ApiAccountTest.php | 49 +++++++++- 12 files changed, 164 insertions(+), 153 deletions(-) diff --git a/flexiapi/app/Account.php b/flexiapi/app/Account.php index ccf1e4f..1c58c6b 100644 --- a/flexiapi/app/Account.php +++ b/flexiapi/app/Account.php @@ -32,6 +32,7 @@ use App\Password; use App\EmailChanged; use App\Mail\ChangingEmail; use Carbon\Carbon; +use Illuminate\Http\Request; class Account extends Authenticatable { @@ -202,6 +203,19 @@ class Account extends Authenticatable return null; } + public function setPhoneAttribute(?string $phone) + { + $this->alias()->delete(); + + if (!empty($phone)) { + $alias = new Alias; + $alias->alias = $phone; + $alias->domain = config('app.sip_domain'); + $alias->account_id = $this->id; + $alias->save(); + } + } + public function getConfirmationKeyExpiresAttribute() { if ($this->activationExpiration) { @@ -302,9 +316,20 @@ class Account extends Authenticatable return $this->provisioning_token; } - public function isAdmin() + public function getAdminAttribute(): bool { - return ($this->admin); + return ($this->admin()->exists()); + } + + public function setAdminAttribute(bool $isAdmin) + { + $this->admin()->delete(); + + if ($isAdmin) { + $admin = new Admin; + $admin->account_id = $this->id; + $admin->save(); + } } public function hasTombstone() @@ -325,6 +350,14 @@ 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 7333e94..d4ce249 100644 --- a/flexiapi/app/Helpers/Utils.php +++ b/flexiapi/app/Helpers/Utils.php @@ -22,6 +22,7 @@ use Illuminate\Support\Str; use App\Account; use App\DigestNonce; use App\ExternalAccount; +use Illuminate\Http\Request; use Illuminate\Support\Facades\Schema; use League\CommonMark\CommonMarkConverter; use League\CommonMark\Extension\HeadingPermalink\HeadingPermalinkExtension; @@ -119,3 +120,13 @@ function isRegularExpression($string): bool return $isRegularExpression; } + +function resolveDomain(Request $request): string +{ + return $request->has('domain') + && $request->user() + && $request->user()->admin + && config('app.admins_manage_multi_domains') + ? $request->get('domain') + : config('app.sip_domain'); +} diff --git a/flexiapi/app/Http/Controllers/Admin/AccountController.php b/flexiapi/app/Http/Controllers/Admin/AccountController.php index b67313d..7bc1d01 100644 --- a/flexiapi/app/Http/Controllers/Admin/AccountController.php +++ b/flexiapi/app/Http/Controllers/Admin/AccountController.php @@ -30,12 +30,6 @@ use App\Alias; use App\ExternalAccount; use App\Http\Requests\CreateAccountRequest; use App\Http\Requests\UpdateAccountRequest; -use App\Rules\BlacklistedUsername; -use App\Rules\IsNotPhoneNumber; -use App\Rules\NoUppercase; -use App\Rules\SIPUsername; -use App\Rules\WithoutSpaces; -use Illuminate\Validation\Rule; class AccountController extends Controller { @@ -71,45 +65,19 @@ class AccountController extends Controller public function store(CreateAccountRequest $request) { - $request->validate([ - 'username' => [ - 'required', - new NoUppercase, - new IsNotPhoneNumber, - new BlacklistedUsername, - new SIPUsername, - Rule::unique('accounts', 'username')->where(function ($query) use ($request) { - $query->where('domain', $this->resolveDomain($request)); - }), - 'filled', - ], - 'dtmf_protocol' => 'nullable|in:' . Account::dtmfProtocolsRule(), - 'email' => [ - 'nullable', - 'email', - config('app.account_email_unique') ? Rule::unique('accounts', 'email') : null - ], - 'phone' => [ - 'nullable', - 'unique:aliases,alias', - 'unique:accounts,username', - new WithoutSpaces, 'starts_with:+' - ] - ]); - $account = new Account; $account->username = $request->get('username'); $account->email = $request->get('email'); $account->display_name = $request->get('display_name'); - $account->domain = $this->resolveDomain($request); + $account->domain = resolveDomain($request); $account->ip_address = $request->ip(); $account->creation_time = Carbon::now(); $account->user_agent = config('app.name'); $account->dtmf_protocol = $request->get('dtmf_protocol'); $account->save(); - $this->fillPassword($request, $account); - $this->fillPhone($request, $account); + $account->phone = $request->get('phone'); + $account->fillPassword($request); Log::channel('events')->info('Web Admin: Account created', ['id' => $account->identifier]); @@ -126,32 +94,6 @@ class AccountController extends Controller public function update(UpdateAccountRequest $request, $id) { - $request->validate([ - 'username' => [ - 'required', - new NoUppercase, - new IsNotPhoneNumber, - new BlacklistedUsername, - new SIPUsername, - Rule::unique('accounts', 'username')->where(function ($query) use ($request) { - $query->where('domain', $this->resolveDomain($request)); - })->ignore($id), - 'filled', - ], - 'dtmf_protocol' => 'nullable|in:' . Account::dtmfProtocolsRule(), - 'email' => [ - 'nullable', - 'email', - config('app.account_email_unique') ? Rule::unique('accounts', 'email')->ignore($id) : null - ], - 'phone' => [ - 'nullable', - 'unique:aliases,alias', - 'unique:accounts,username', - new WithoutSpaces, 'starts_with:+' - ] - ]); - $account = Account::findOrFail($id); $account->username = $request->get('username'); $account->email = $request->get('email'); @@ -159,8 +101,8 @@ class AccountController extends Controller $account->dtmf_protocol = $request->get('dtmf_protocol'); $account->save(); - $this->fillPassword($request, $account); - $this->fillPhone($request, $account); + $account->phone = $request->get('phone'); + $account->fillPassword($request); Log::channel('events')->info('Web Admin: Account updated', ['id' => $account->identifier]); @@ -262,25 +204,4 @@ class AccountController extends Controller return redirect()->route('admin.account.index'); } - - private function fillPassword(Request $request, Account $account) - { - if ($request->filled('password')) { - $algorithm = $request->has('password_sha256') ? 'SHA-256' : 'MD5'; - $account->updatePassword($request->get('password'), $algorithm); - } - } - - private function fillPhone(Request $request, Account $account) - { - if ($request->filled('phone')) { - $account->alias()->delete(); - - $alias = new Alias; - $alias->alias = $request->get('phone'); - $alias->domain = config('app.sip_domain'); - $alias->account_id = $account->id; - $alias->save(); - } - } } diff --git a/flexiapi/app/Http/Controllers/Api/Admin/AccountController.php b/flexiapi/app/Http/Controllers/Api/Admin/AccountController.php index e7a6df7..897696c 100644 --- a/flexiapi/app/Http/Controllers/Api/Admin/AccountController.php +++ b/flexiapi/app/Http/Controllers/Api/Admin/AccountController.php @@ -22,7 +22,6 @@ namespace App\Http\Controllers\Api\Admin; use App\Http\Controllers\Controller; use Illuminate\Http\Request; use Illuminate\Support\Str; -use Illuminate\Validation\Rule; use Illuminate\Support\Facades\Log; use Carbon\Carbon; @@ -33,12 +32,9 @@ use App\ActivationExpiration; use App\Admin; use App\Alias; use App\Http\Controllers\Account\AuthenticateController as WebAuthenticateController; +use App\Http\Requests\CreateAccountRequest; +use App\Http\Requests\UpdateAccountRequest; use App\Mail\PasswordAuthentication; -use App\Rules\BlacklistedUsername; -use App\Rules\IsNotPhoneNumber; -use App\Rules\NoUppercase; -use App\Rules\SIPUsername; -use App\Rules\WithoutSpaces; use Illuminate\Support\Facades\Mail; class AccountController extends Controller @@ -112,36 +108,15 @@ class AccountController extends Controller return $account->makeVisible(['provisioning_token']); } - public function store(Request $request) + public function store(CreateAccountRequest $request) { $request->validate([ - 'username' => [ - 'required', - new NoUppercase, - new IsNotPhoneNumber, - new BlacklistedUsername, - new SIPUsername, - Rule::unique('accounts', 'username')->where(function ($query) use ($request) { - $query->where('domain', $this->resolveDomain($request)); - }), - 'filled', - ], 'algorithm' => 'required|in:SHA-256,MD5', - 'password' => 'required|filled', 'admin' => 'boolean|nullable', 'activated' => 'boolean|nullable', - 'dtmf_protocol' => 'nullable|in:' . Account::dtmfProtocolsRule(), 'confirmation_key_expires' => [ 'date_format:Y-m-d H:i:s', 'nullable', - ], - 'email' => config('app.account_email_unique') - ? 'nullable|email|unique:accounts,email' - : 'nullable|email', - 'phone' => [ - 'unique:aliases,alias', - 'unique:accounts,username', - new WithoutSpaces, 'starts_with:+' ] ]); @@ -153,7 +128,7 @@ class AccountController extends Controller $account->ip_address = $request->ip(); $account->dtmf_protocol = $request->get('dtmf_protocol'); $account->creation_time = Carbon::now(); - $account->domain = $this->resolveDomain($request); + $account->domain = resolveDomain($request); $account->user_agent = $request->header('User-Agent') ?? config('app.name'); if (!$request->has('activated') || !(bool)$request->get('activated')) { @@ -172,27 +147,45 @@ class AccountController extends Controller } $account->updatePassword($request->get('password'), $request->get('algorithm')); - - if ($request->has('admin') && (bool)$request->get('admin')) { - $admin = new Admin; - $admin->account_id = $account->id; - $admin->save(); - } - - if ($request->has('phone')) { - $alias = new Alias; - $alias->alias = $request->get('phone'); - $alias->domain = config('app.sip_domain'); - $alias->account_id = $account->id; - $alias->save(); - } + $account->admin = $request->has('admin') && (bool)$request->get('admin'); + $account->phone = $request->get('phone'); // Full reload $account = Account::withoutGlobalScopes()->find($account->id); Log::channel('events')->info('API Admin: Account created', ['id' => $account->identifier]); - return response()->json($account->makeVisible(['confirmation_key', 'provisioning_token'])); + return $account->makeVisible(['confirmation_key', 'provisioning_token']); + } + + public function update(UpdateAccountRequest $request, int $accountId) + { + $request->validate([ + 'algorithm' => 'required|in:SHA-256,MD5', + 'admin' => 'boolean|nullable', + 'activated' => 'boolean|nullable' + ]); + + $account = Account::findOrFail($accountId); + $account->username = $request->get('username'); + $account->email = $request->get('email'); + $account->display_name = $request->get('display_name'); + $account->dtmf_protocol = $request->get('dtmf_protocol'); + $account->domain = resolveDomain($request); + $account->user_agent = $request->header('User-Agent') ?? config('app.name'); + + $account->save(); + + $account->updatePassword($request->get('password'), $request->get('algorithm')); + $account->admin = $request->has('admin') && (bool)$request->get('admin'); + $account->phone = $request->get('phone'); + + // Full reload + $account = Account::withoutGlobalScopes()->find($account->id); + + Log::channel('events')->info('API Admin: Account updated', ['id' => $account->identifier]); + + return $account->makeVisible(['confirmation_key', 'provisioning_token']); } public function typeAdd(int $id, int $typeId) @@ -226,6 +219,6 @@ class AccountController extends Controller Mail::to($account)->send(new PasswordAuthentication($account)); - return response()->json($account->makeVisible(['confirmation_key', 'provisioning_token'])); + return $account->makeVisible(['confirmation_key', 'provisioning_token']); } } diff --git a/flexiapi/app/Http/Controllers/Controller.php b/flexiapi/app/Http/Controllers/Controller.php index 675ca4d..459afe8 100644 --- a/flexiapi/app/Http/Controllers/Controller.php +++ b/flexiapi/app/Http/Controllers/Controller.php @@ -11,14 +11,4 @@ use Illuminate\Routing\Controller as BaseController; class Controller extends BaseController { use AuthorizesRequests, DispatchesJobs, ValidatesRequests; - - protected function resolveDomain(Request $request): string - { - return $request->has('domain') - && $request->user() - && $request->user()->isAdmin() - && config('app.admins_manage_multi_domains') - ? $request->get('domain') - : config('app.sip_domain'); - } } diff --git a/flexiapi/app/Http/Middleware/AuthenticateAdmin.php b/flexiapi/app/Http/Middleware/AuthenticateAdmin.php index afe4b01..e145cbd 100644 --- a/flexiapi/app/Http/Middleware/AuthenticateAdmin.php +++ b/flexiapi/app/Http/Middleware/AuthenticateAdmin.php @@ -19,7 +19,7 @@ class AuthenticateAdmin return redirect()->route('account.login'); } - if (!$request->user()->isAdmin()) { + if (!$request->user()->admin) { return abort(403, 'Unauthorized area'); } diff --git a/flexiapi/app/Http/Requests/CreateAccountRequest.php b/flexiapi/app/Http/Requests/CreateAccountRequest.php index 40f6b3a..25119b4 100644 --- a/flexiapi/app/Http/Requests/CreateAccountRequest.php +++ b/flexiapi/app/Http/Requests/CreateAccountRequest.php @@ -29,13 +29,14 @@ class CreateAccountRequest extends FormRequest new BlacklistedUsername, new SIPUsername, Rule::unique('accounts', 'username')->where(function ($query) { - $query->where('domain', config('app.sip_domain')); + $query->where('domain', resolveDomain($this)); }), 'filled', ], - 'domain' => config('app.admins_manage_multi_domains') ? 'required' : '', 'password' => 'required|min:3', - 'email' => 'nullable|email', + 'email' => config('app.account_email_unique') + ? 'nullable|email|unique:accounts,email' + : 'nullable|email', 'dtmf_protocol' => 'nullable|in:' . Account::dtmfProtocolsRule(), 'phone' => [ 'nullable', diff --git a/flexiapi/app/Http/Requests/UpdateAccountRequest.php b/flexiapi/app/Http/Requests/UpdateAccountRequest.php index 7133a70..2b2b12a 100644 --- a/flexiapi/app/Http/Requests/UpdateAccountRequest.php +++ b/flexiapi/app/Http/Requests/UpdateAccountRequest.php @@ -33,8 +33,11 @@ class UpdateAccountRequest extends FormRequest })->ignore($this->route('id'), 'id'), 'filled', ], - 'domain' => config('app.admins_manage_multi_domains') ? 'required' : '', - 'email' => 'nullable|email', + 'email' => [ + 'nullable', + 'email', + config('app.account_email_unique') ? Rule::unique('accounts', 'email')->ignore($this->route('id')) : null + ], 'password_sha256' => 'nullable|min:3', 'dtmf_protocol' => 'nullable|in:' . Account::dtmfProtocolsRule(), 'phone' => [ diff --git a/flexiapi/resources/views/account/panel.blade.php b/flexiapi/resources/views/account/panel.blade.php index 3553ed5..9a5d45e 100644 --- a/flexiapi/resources/views/account/panel.blade.php +++ b/flexiapi/resources/views/account/panel.blade.php @@ -45,7 +45,7 @@ -@if($account->isAdmin()) +@if($account->admin)

Admin area

diff --git a/flexiapi/resources/views/api/documentation_markdown.blade.php b/flexiapi/resources/views/api/documentation_markdown.blade.php index a75df97..470b5a3 100644 --- a/flexiapi/resources/views/api/documentation_markdown.blade.php +++ b/flexiapi/resources/views/api/documentation_markdown.blade.php @@ -291,6 +291,21 @@ JSON parameters: * `dtmf_protocol` optional, values must be `sipinfo`, `sipmessage` or `rfc2833` * `confirmation_key_expires` optional, a datetime of this format: Y-m-d H:i:s. Only used when `activated` is not used or `false`. Enforces an expiration date on the returned `confirmation_key`. After that datetime public email or phone activation endpoints will return `403`. +### `PUT /accounts/{id}` +Admin +Update an existing account. + +JSON parameters: + +* `username` unique username, minimum 6 characters +* `password` required minimum 6 characters +* `algorithm` required, values can be `SHA-256` or `MD5` +* `display_name` optional, string +* `email` optional, must be an email, must be unique if `ACCOUNT_EMAIL_UNIQUE` is set to `true` +* `admin` optional, a boolean, set to `false` by default, create an admin account +* `phone` optional, a phone number, set a phone number to the account +* `dtmf_protocol` optional, values must be `sipinfo`, `sipmessage` or `rfc2833` + ### `GET /accounts` Admin Retrieve all the accounts, paginated. diff --git a/flexiapi/routes/api.php b/flexiapi/routes/api.php index e8508f8..0413d29 100644 --- a/flexiapi/routes/api.php +++ b/flexiapi/routes/api.php @@ -88,11 +88,12 @@ Route::group(['middleware' => ['auth.digest_or_key']], function () { Route::post('accounts/{id}/recover-by-email', 'Api\Admin\AccountController@recoverByEmail'); Route::post('accounts', 'Api\Admin\AccountController@store'); + Route::put('accounts/{id}', 'Api\Admin\AccountController@update'); Route::get('accounts', 'Api\Admin\AccountController@index'); - Route::get('accounts/{sip}/search', 'Api\Admin\AccountController@search'); - Route::get('accounts/{email}/search-by-email', 'Api\Admin\AccountController@searchByEmail'); Route::get('accounts/{id}', 'Api\Admin\AccountController@show'); Route::delete('accounts/{id}', 'Api\Admin\AccountController@destroy'); + Route::get('accounts/{sip}/search', 'Api\Admin\AccountController@search'); + Route::get('accounts/{email}/search-by-email', 'Api\Admin\AccountController@searchByEmail'); // Account actions Route::get('accounts/{id}/actions', 'Api\Admin\AccountActionController@index'); diff --git a/flexiapi/tests/Feature/ApiAccountTest.php b/flexiapi/tests/Feature/ApiAccountTest.php index 0d9fd44..2f3a807 100644 --- a/flexiapi/tests/Feature/ApiAccountTest.php +++ b/flexiapi/tests/Feature/ApiAccountTest.php @@ -319,7 +319,7 @@ class ApiAccountTest extends TestCase ->json($this->method, $this->route, [ 'username' => $username, 'algorithm' => 'SHA-256', - 'password' => '2', + 'password' => 'blabla', 'admin' => true, ]); @@ -349,7 +349,7 @@ class ApiAccountTest extends TestCase ->json($this->method, $this->route, [ 'username' => $username, 'algorithm' => 'SHA-256', - 'password' => '2', + 'password' => 'blabla', 'activated' => true, ]); @@ -379,7 +379,7 @@ class ApiAccountTest extends TestCase ->json($this->method, $this->route, [ 'username' => $username, 'algorithm' => 'SHA-256', - 'password' => '2', + 'password' => 'blabla', 'activated' => false, ]); @@ -533,6 +533,49 @@ class ApiAccountTest extends TestCase ->assertJsonValidationErrors(['email']); } + public function testEditAdmin() + { + $password = Password::factory()->create(); + $account = $password->account; + + $admin = Admin::factory()->create(); + $admin->account->generateApiKey(); + $admin->account->save(); + + $username = 'changed'; + $algorithm = 'MD5'; + $password = 'other'; + + $this->keyAuthenticated($admin->account) + ->json('PUT', $this->route. '/1234') + ->assertStatus(422) + ->assertJsonValidationErrors(['username']); + + $this->keyAuthenticated($admin->account) + ->json('PUT', $this->route. '/1234', [ + 'username' => 'good' + ]) + ->assertStatus(422); + + $this->keyAuthenticated($admin->account) + ->json('PUT', $this->route. '/'. $account->id, [ + 'username' => $username, + 'algorithm' => $algorithm, + 'password' => $password, + ]) + ->assertStatus(200); + + $this->assertDatabaseHas('accounts', [ + 'id' => $account->id, + 'username' => $username + ]); + + $this->assertDatabaseHas('passwords', [ + 'account_id' => $account->id, + 'algorithm' => $algorithm + ]); + } + /** * /!\ Dangerous endpoints */