From eb2e93f77f0652c4f2e30f6af8fba23be8127a63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Jaussoin?= Date: Wed, 27 Sep 2023 14:31:04 +0000 Subject: [PATCH] Fix #124 Return 404 when the account is already provisioned or the provisioning_token not valid --- .../Account/ProvisioningController.php | 61 ++++++++++--------- flexiapi/app/Mail/PasswordAuthentication.php | 2 +- .../views/admin/account/create_edit.blade.php | 2 +- .../api/documentation_markdown.blade.php | 28 +++++++-- flexiapi/routes/web.php | 12 +++- .../tests/Feature/AccountProvisioningTest.php | 10 +-- 6 files changed, 69 insertions(+), 46 deletions(-) diff --git a/flexiapi/app/Http/Controllers/Account/ProvisioningController.php b/flexiapi/app/Http/Controllers/Account/ProvisioningController.php index 7a8e3c8..1e5a7f9 100644 --- a/flexiapi/app/Http/Controllers/Account/ProvisioningController.php +++ b/flexiapi/app/Http/Controllers/Account/ProvisioningController.php @@ -46,7 +46,7 @@ class ProvisioningController extends Controller $params['reset_password'] = true; } - $url = route('provisioning.show', $params); + $url = route('provisioning.provision', $params); $result = Builder::create() ->writer(new PngWriter()) @@ -73,7 +73,7 @@ class ProvisioningController extends Controller $account = $authToken->account; $authToken->delete(); - return $this->show($request, null, $account); + return $this->generateProvisioning($request, $account); } abort(404); @@ -84,10 +84,38 @@ class ProvisioningController extends Controller */ public function me(Request $request) { - return $this->show($request, null, $request->user()); + return $this->generateProvisioning($request, $request->user()); } - public function show(Request $request, $provisioningToken = null, Account $requestAccount = null) + /** + * Get the base provisioning, with authentication + */ + public function show(Request $request) + { + return $this->generateProvisioning($request); + } + + /** + * Provisioning Token based provisioning + */ + public function provision(Request $request, string $provisioningToken) + { + $account = Account::withoutGlobalScopes() + ->where('provisioning_token', $provisioningToken) + ->firstOrFail(); + + if ($account->activationExpired() || ($provisioningToken != $account->provisioning_token)) { + abort(404); + } + + $account->activated = true; + $account->provisioning_token = null; + $account->save(); + + return $this->generateProvisioning($request, $account); + } + + private function generateProvisioning(Request $request, Account $account = null) { // Load the hooks if they exists $provisioningHooks = config_path('provisioning_hooks.php'); @@ -132,17 +160,6 @@ class ProvisioningController extends Controller } } - $account = null; - - // Account handling - if ($requestAccount) { - $account = $requestAccount; - } elseif ($provisioningToken) { - $account = Account::withoutGlobalScopes() - ->where('provisioning_token', $provisioningToken) - ->first(); - } - // Password reset if ($account && $request->has('reset_password')) { $account->updatePassword(Str::random(10)); @@ -157,7 +174,7 @@ class ProvisioningController extends Controller $config->appendChild($section); - if ($account && !$account->activationExpired()) { + if ($account) { $externalAccount = $account->externalAccount; $section = $dom->createElement('section'); @@ -224,18 +241,6 @@ class ProvisioningController extends Controller $authInfoIndex++; } - if ($provisioningToken) { - // Activate the account - if ($account->activated == false - && $provisioningToken == $account->provisioning_token - ) { - $account->activated = true; - } - - $account->provisioning_token = null; - $account->save(); - } - $proxyConfigIndex++; // External Account handling diff --git a/flexiapi/app/Mail/PasswordAuthentication.php b/flexiapi/app/Mail/PasswordAuthentication.php index 667ca33..a11484c 100644 --- a/flexiapi/app/Mail/PasswordAuthentication.php +++ b/flexiapi/app/Mail/PasswordAuthentication.php @@ -46,7 +46,7 @@ class PasswordAuthentication extends Mailable : 'mails.authentication_text') ->with([ 'link' => route('account.authenticate.email_confirm', [$this->account->confirmation_key]), - 'provisioning_link' => route('provisioning.show', [ + 'provisioning_link' => route('provisioning.provision', [ 'provisioning_token' => $this->account->provisioning_token, 'reset_password' => true ]), diff --git a/flexiapi/resources/views/admin/account/create_edit.blade.php b/flexiapi/resources/views/admin/account/create_edit.blade.php index 1d9abf3..5ef82c9 100644 --- a/flexiapi/resources/views/admin/account/create_edit.blade.php +++ b/flexiapi/resources/views/admin/account/create_edit.blade.php @@ -95,4 +95,4 @@ {!! Form::submit(($account->id) ? 'Update' : 'Create', ['class' => 'btn btn-success btn-centered']) !!} {!! Form::close() !!} -@endsection \ No newline at end of file +@endsection diff --git a/flexiapi/resources/views/api/documentation_markdown.blade.php b/flexiapi/resources/views/api/documentation_markdown.blade.php index 2bb5963..5f1163d 100644 --- a/flexiapi/resources/views/api/documentation_markdown.blade.php +++ b/flexiapi/resources/views/api/documentation_markdown.blade.php @@ -497,25 +497,37 @@ The following URLs are **not API endpoints** they are not returning `JSON` conte ## Provisioning -When an account is having an available `provisioning_token` it can be provisioned using the two following URL. +When an account is having an available `provisioning_token` it can be provisioned using the following URLs. ### `GET /provisioning` + Public + Return the provisioning information available in the liblinphone configuration file (if correctly configured). -### `GET /provisioning/{provisioning_token}?reset_password` -Public -Return the provisioning information available in the liblinphone configuration file. -If the `provisioning_token` is valid the related account information are added to the returned XML. The account is then considered as "provisioned" and those account related information will be removed in the upcoming requests (the content will be the same as the previous url). +### `GET /provisioning/auth_token/{auth_token}` -If the account is not activated and the `provisioning_token` is valid. The account will be activated. +Public + +Return the provisioning information available linked to the account that was attached to the `auth_token`. + +### `GET /provisioning/{provisioning_token}?reset_password` + +Public + +Return the provisioning information available linked to the account related to the `provisioning_token`. +Return `404` if the `provisioning_token` provided is not valid or expired otherwise. + +If the account is not activated the account will be activated. The account is then considered as "provisioned". URL parameters: * `reset_password` optional, reset the password while doing the provisioning ### `GET /provisioning/qrcode/{provisioning_token}?reset_password` + Public + Return a QRCode that points to the provisioning URL. URL parameters: @@ -523,7 +535,11 @@ URL parameters: * `reset_password` optional, reset the password while doing the provisioning ### `GET /provisioning/me` + User + +Authenticated endpoint, see [API About & Auth]({{ route('api') }}#about--auth) + Return the same base content as the previous URL and the account related information, similar to the `provisioning_token` endpoint. However this endpoint will always return those information. ## Contacts list diff --git a/flexiapi/routes/web.php b/flexiapi/routes/web.php index 6c12197..2e5362b 100644 --- a/flexiapi/routes/web.php +++ b/flexiapi/routes/web.php @@ -17,6 +17,8 @@ along with this program. If not, see . */ +use \Illuminate\Support\Facades\Route; + Route::get('/', 'Account\AccountController@home')->name('account.home'); Route::get('documentation', 'Account\AccountController@documentation')->name('account.documentation'); @@ -49,9 +51,13 @@ Route::group(['middleware' => 'auth.digest_or_key'], function () { Route::get('contacts/vcard', 'Account\ContactVcardController@index')->name('account.contacts.vcard.index'); }); -Route::get('provisioning/auth_token/{auth_token}', 'Account\ProvisioningController@authToken')->name('provisioning.auth_token'); -Route::get('provisioning/qrcode/{provisioning_token}', 'Account\ProvisioningController@qrcode')->name('provisioning.qrcode'); -Route::get('provisioning/{provisioning_token?}', 'Account\ProvisioningController@show')->name('provisioning.show'); +Route::name('provisioning.')->prefix('provisioning')->controller('Account\ProvisioningController')->group(function () { + Route::get('documentation', 'documentation')->name('documentation'); + Route::get('auth_token/{auth_token}', 'authToken')->name('auth_token'); + Route::get('qrcode/{provisioning_token}', 'qrcode')->name('qrcode'); + Route::get('{provisioning_token}', 'provision')->name('provision'); + Route::get('/', 'show')->name('show'); +}); if (publicRegistrationEnabled()) { if (config('app.phone_authentication')) { diff --git a/flexiapi/tests/Feature/AccountProvisioningTest.php b/flexiapi/tests/Feature/AccountProvisioningTest.php index 189b84f..9a72713 100644 --- a/flexiapi/tests/Feature/AccountProvisioningTest.php +++ b/flexiapi/tests/Feature/AccountProvisioningTest.php @@ -104,7 +104,7 @@ class AccountProvisioningTest extends TestCase $currentPassword = $password->password; $provioningUrl = route( - 'provisioning.show', + 'provisioning.provision', [ 'provisioning_token' => $password->account->provisioning_token, 'reset_password' => true @@ -132,9 +132,7 @@ class AccountProvisioningTest extends TestCase public function testConfirmationKeyProvisioning() { $response = $this->get($this->route . '/1234'); - $response->assertStatus(200); - $response->assertHeader('Content-Type', 'application/xml'); - $response->assertDontSee('ha1'); + $response->assertStatus(404); $password = Password::factory()->create(); $password->account->generateApiKey(); @@ -152,9 +150,7 @@ class AccountProvisioningTest extends TestCase // And then twice $response = $this->get($this->route . '/' . $password->account->provisioning_token) - ->assertStatus(200) - ->assertHeader('Content-Type', 'application/xml') - ->assertDontSee('ha1'); + ->assertStatus(404); $password->account->refresh();