From be429be82a3829eb97814cf94ca1f80d53b81e63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Jaussoin?= Date: Tue, 29 Aug 2023 09:52:45 +0000 Subject: [PATCH] Fix #117 Redeem properly the tokens to prevent reuse --- .../Api/Account/AccountController.php | 13 +++++++++ flexiapi/app/Services/AccountService.php | 14 +++++---- .../Feature/ApiAccountCreationTokenTest.php | 6 ++++ flexiapi/tests/Feature/ApiAccountTest.php | 29 +++++++++++++++++++ 4 files changed, 56 insertions(+), 6 deletions(-) diff --git a/flexiapi/app/Http/Controllers/Api/Account/AccountController.php b/flexiapi/app/Http/Controllers/Api/Account/AccountController.php index b934a59..33f6c6a 100644 --- a/flexiapi/app/Http/Controllers/Api/Account/AccountController.php +++ b/flexiapi/app/Http/Controllers/Api/Account/AccountController.php @@ -28,6 +28,7 @@ use App\Http\Controllers\Controller; use Carbon\Carbon; use App\Account; +use App\AccountCreationToken; use App\AccountTombstone; use App\Alias; use App\Http\Controllers\Account\AuthenticateController as WebAuthenticateController; @@ -139,6 +140,12 @@ class AccountController extends Controller $account->updatePassword($request->get('password'), $request->get('algorithm')); + $token = AccountCreationToken::where('token', $request->get('account_creation_token'))->first(); + $token->used = true; + $token->account_id = $account->id; + $token->save(); + + Log::channel('events')->info('API: AccountCreationToken redeemed', ['token' => $request->get('account_creation_token')]); Log::channel('events')->info('API: Account created using the public endpoint', ['id' => $account->identifier]); // Send validation by phone @@ -200,6 +207,12 @@ class AccountController extends Controller $account->confirmation_key = generatePin(); $account->save(); + $token = AccountCreationToken::where('token', $request->get('account_creation_token'))->first(); + $token->used = true; + $token->account_id = $account->id; + $token->save(); + + Log::channel('events')->info('API: AccountCreationToken redeemed', ['token' => $request->get('account_creation_token')]); Log::channel('events')->info('API: Account recovery by phone', ['id' => $account->identifier]); $ovhSMS = new OvhSMS; diff --git a/flexiapi/app/Services/AccountService.php b/flexiapi/app/Services/AccountService.php index 3f6e385..f9a875f 100644 --- a/flexiapi/app/Services/AccountService.php +++ b/flexiapi/app/Services/AccountService.php @@ -61,12 +61,6 @@ class AccountService $request->validate($rules); - if ($this->api) { - $token = AccountCreationToken::where('token', $request->get('account_creation_token'))->first(); - $token->used = true; - $token->save(); - } - $account = new Account; $account->username = $request->get('username'); $account->activated = false; @@ -81,6 +75,14 @@ class AccountService $account->updatePassword($request->get('password'), $request->has('algorithm') ? $request->get('algorithm') : 'SHA-256'); + if ($this->api) { + $token = AccountCreationToken::where('token', $request->get('account_creation_token'))->first(); + $token->used = true; + $token->account_id = $account->id; + $token->save(); + } + + Log::channel('events')->info('API: AccountCreationToken redeemed', ['token' => $request->get('account_creation_token')]); Log::channel('events')->info('Account Service: Account created', ['id' => $account->identifier]); if (!$this->api) { diff --git a/flexiapi/tests/Feature/ApiAccountCreationTokenTest.php b/flexiapi/tests/Feature/ApiAccountCreationTokenTest.php index d524013..cbac38b 100644 --- a/flexiapi/tests/Feature/ApiAccountCreationTokenTest.php +++ b/flexiapi/tests/Feature/ApiAccountCreationTokenTest.php @@ -19,6 +19,7 @@ namespace Tests\Feature; +use App\Account; use App\AccountCreationRequestToken; use Illuminate\Foundation\Testing\RefreshDatabase; use Tests\TestCase; @@ -102,6 +103,11 @@ class ApiAccountCreationTokenTest extends TestCase 'account_creation_token' => $token->token ]); $response->assertStatus(422); + + $this->assertDatabaseHas('account_creation_tokens', [ + 'used' => true, + 'account_id' => Account::where('username', 'username')->first()->id, + ]); } public function testBlacklistedUsername() diff --git a/flexiapi/tests/Feature/ApiAccountTest.php b/flexiapi/tests/Feature/ApiAccountTest.php index c78732b..2e0bf84 100644 --- a/flexiapi/tests/Feature/ApiAccountTest.php +++ b/flexiapi/tests/Feature/ApiAccountTest.php @@ -685,6 +685,13 @@ class ApiAccountTest extends TestCase $password->account->refresh(); + // Use the token a second time + $this->json($this->method, $this->route . '/recover-by-phone', [ + 'phone' => $phone, + 'account_creation_token' => $token->token + ]) + ->assertStatus(422); + $this->get($this->route . '/' . $password->account->identifier . '/recover/' . $password->account->confirmation_key) ->assertStatus(200) ->assertJson([ @@ -717,6 +724,11 @@ class ApiAccountTest extends TestCase 'activated' => true, 'phone' => false ]); + + $this->assertDatabaseHas('account_creation_tokens', [ + 'used' => true, + 'account_id' => $password->account->id, + ]); } /** @@ -764,6 +776,18 @@ class ApiAccountTest extends TestCase 'activated' => false ]); + // Re-use the token + $this->withHeaders([ + 'User-Agent' => $userAgent, + ])->json($this->method, $this->route . '/public', [ + 'username' => $username . 'foo', + 'algorithm' => 'SHA-256', + 'password' => '2', + 'email' => 'john@doe.tld', + 'account_creation_token' => $token->token + ]) + ->assertStatus(422); + // Already created $this->json($this->method, $this->route . '/public', [ 'username' => $username, @@ -791,6 +815,11 @@ class ApiAccountTest extends TestCase 'domain' => config('app.sip_domain'), 'user_agent' => $userAgent ]); + + $this->assertDatabaseHas('account_creation_tokens', [ + 'used' => true, + 'account_id' => Account::where('username', $username)->first()->id, + ]); } public function testCreatePublicPhone()