From e516ae788c3f2117267a2a8e8f5459e6feaf788b 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/RecoverByCode.php | 2 +- .../views/admin/account/create_edit.blade.php | 2 +- .../documentation_markdown.blade.php | 12 +++- flexiapi/routes/web.php | 3 +- .../tests/Feature/AccountProvisioningTest.php | 10 +-- 6 files changed, 49 insertions(+), 41 deletions(-) diff --git a/flexiapi/app/Http/Controllers/Account/ProvisioningController.php b/flexiapi/app/Http/Controllers/Account/ProvisioningController.php index a637892..f4cb89e 100644 --- a/flexiapi/app/Http/Controllers/Account/ProvisioningController.php +++ b/flexiapi/app/Http/Controllers/Account/ProvisioningController.php @@ -53,7 +53,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()) @@ -80,7 +80,7 @@ class ProvisioningController extends Controller $account = $authToken->account; $authToken->delete(); - return $this->show($request, null, $account); + return $this->generateProvisioning($request, $account); } abort(404); @@ -91,10 +91,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'); @@ -139,17 +167,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)); @@ -164,7 +181,7 @@ class ProvisioningController extends Controller $config->appendChild($section); - if ($account && !$account->activationExpired()) { + if ($account) { $externalAccount = $account->externalAccount; $section = $dom->createElement('section'); @@ -231,18 +248,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/RecoverByCode.php b/flexiapi/app/Mail/RecoverByCode.php index 9d63dd8..188ae82 100644 --- a/flexiapi/app/Mail/RecoverByCode.php +++ b/flexiapi/app/Mail/RecoverByCode.php @@ -46,7 +46,7 @@ class RecoverByCode extends Mailable : 'mails.authentication_text') ->with([ 'recovery_code' => $this->account->recovery_code, - '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 08b92eb..d71d6ad 100644 --- a/flexiapi/resources/views/admin/account/create_edit.blade.php +++ b/flexiapi/resources/views/admin/account/create_edit.blade.php @@ -178,7 +178,7 @@
+ value="{{ route('provisioning.provision', $account->provisioning_token) }}"> The following link can only be visited once
diff --git a/flexiapi/resources/views/provisioning/documentation_markdown.blade.php b/flexiapi/resources/views/provisioning/documentation_markdown.blade.php index b606128..de7df5d 100644 --- a/flexiapi/resources/views/provisioning/documentation_markdown.blade.php +++ b/flexiapi/resources/views/provisioning/documentation_markdown.blade.php @@ -41,14 +41,20 @@ When an account is having an available `provisioning_token` it can be provisione Return the provisioning information available in the liblinphone configuration file (if correctly configured). +### `GET /provisioning/auth_token/{auth_token}` + +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 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). +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 and the `provisioning_token` is valid the account will be activated. +If the account is not activated the account will be activated. The account is then considered as "provisioned". URL parameters: diff --git a/flexiapi/routes/web.php b/flexiapi/routes/web.php index 8a27155..d39a8b0 100644 --- a/flexiapi/routes/web.php +++ b/flexiapi/routes/web.php @@ -63,7 +63,8 @@ Route::name('provisioning.')->prefix('provisioning')->controller(ProvisioningCon 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?}', 'show')->name('show'); + Route::get('{provisioning_token}', 'provision')->name('provision'); + Route::get('/', 'show')->name('show'); }); if (publicRegistrationEnabled()) { 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();