From 2514de1754736ea1f6035cecb3efb7e371fcbcf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Jaussoin?= Date: Wed, 3 May 2023 13:20:26 +0000 Subject: [PATCH] Fix #94 Implement the deprecated endpoint changes + tests + documentation for 1.4 --- flexiapi/app/Alias.php | 12 ++++ .../Controllers/Api/AccountController.php | 36 ++++++---- flexiapi/app/Rules/AccountCreationToken.php | 21 ++++++ .../api/documentation_markdown.blade.php | 7 ++ flexiapi/tests/Feature/ApiAccountTest.php | 72 ++++++++++++++++++- 5 files changed, 133 insertions(+), 15 deletions(-) create mode 100644 flexiapi/app/Rules/AccountCreationToken.php diff --git a/flexiapi/app/Alias.php b/flexiapi/app/Alias.php index bf58ce5..99c238d 100644 --- a/flexiapi/app/Alias.php +++ b/flexiapi/app/Alias.php @@ -30,4 +30,16 @@ class Alias extends Model { return $this->belongsTo('App\Account'); } + + public function scopeSip($query, string $sip) + { + if (\str_contains($sip, '@')) { + list($usernane, $domain) = explode('@', $sip); + + return $query->where('alias', $usernane) + ->where('domain', $domain); + }; + + return $query->where('id', '<', 0); + } } diff --git a/flexiapi/app/Http/Controllers/Api/AccountController.php b/flexiapi/app/Http/Controllers/Api/AccountController.php index f4cd8ae..e91c24d 100644 --- a/flexiapi/app/Http/Controllers/Api/AccountController.php +++ b/flexiapi/app/Http/Controllers/Api/AccountController.php @@ -34,6 +34,7 @@ use App\Alias; use App\Http\Controllers\Account\AuthenticateController as WebAuthenticateController; use App\Libraries\OvhSMS; use App\Mail\RegisterConfirmation; +use App\Rules\AccountCreationToken as RulesAccountCreationToken; use App\Rules\BlacklistedUsername; use App\Rules\IsNotPhoneNumber; use App\Rules\NoUppercase; @@ -70,11 +71,13 @@ class AccountController extends Controller $alias = Alias::where('alias', $phone)->first(); $account = $alias ? $alias->account - : Account::sip($phone)->firstOrFail(); + // Injecting the default sip domain to try to resolve the account + : Account::sip($phone . '@' . config('app.sip_domain'))->firstOrFail(); return \response()->json([ 'activated' => $account->activated, - 'realm' => $account->realm + 'realm' => $account->realm, + 'phone' => (bool)$alias ]); } @@ -113,6 +116,10 @@ class AccountController extends Controller 'unique:aliases,alias', 'unique:accounts,username', new WithoutSpaces, 'starts_with:+' + ], + 'account_creation_token' => [ + 'required', + new RulesAccountCreationToken ] ]); @@ -178,13 +185,17 @@ class AccountController extends Controller $request->validate([ 'phone' => [ 'required', new WithoutSpaces, 'starts_with:+' + ], + 'account_creation_token' => [ + 'required', + new RulesAccountCreationToken ] ]); $alias = Alias::where('alias', $request->get('phone'))->first(); $account = $alias ? $alias->account - : Account::sip($request->get('phone'))->firstOrFail(); + : Account::sip($request->get('phone') . '@' . config('app.sip_domain'))->firstOrFail(); $account->confirmation_key = generatePin(); $account->save(); @@ -202,9 +213,12 @@ class AccountController extends Controller { if (!config('app.dangerous_endpoints')) return abort(404); - $account = Account::sip($sip) - ->where('confirmation_key', $recoveryKey) - ->firstOrFail(); + $alias = Alias::sip($sip)->first(); + $account = $alias + ? $alias->account + : Account::sip($sip)->firstOrFail(); + + if ($account->confirmation_key != $recoveryKey) abort(404); if ($account->activationExpired()) abort(403, 'Activation expired'); @@ -241,10 +255,7 @@ class AccountController extends Controller 'dtmf_protocol' => 'nullable|in:' . Account::dtmfProtocolsRule(), 'account_creation_token' => [ 'required_without:token', - Rule::exists('account_creation_tokens', 'token')->where(function ($query) { - $query->where('used', false); - }), - 'size:' . WebAuthenticateController::$emailCodeSize + new RulesAccountCreationToken ], 'email' => config('app.account_email_unique') ? 'nullable|email|unique:accounts,email' @@ -252,10 +263,7 @@ class AccountController extends Controller // For retro-compatibility 'token' => [ 'required_without:account_creation_token', - Rule::exists('account_creation_tokens', 'token')->where(function ($query) { - $query->where('used', false); - }), - 'size:' . WebAuthenticateController::$emailCodeSize + new RulesAccountCreationToken ], ]); diff --git a/flexiapi/app/Rules/AccountCreationToken.php b/flexiapi/app/Rules/AccountCreationToken.php new file mode 100644 index 0000000..39679fb --- /dev/null +++ b/flexiapi/app/Rules/AccountCreationToken.php @@ -0,0 +1,21 @@ +where('used', false)->exists() + && strlen($value) == AuthenticateController::$emailCodeSize; + } + + public function message() + { + return 'Please provide a valid account_creation_token'; + } +} diff --git a/flexiapi/resources/views/api/documentation_markdown.blade.php b/flexiapi/resources/views/api/documentation_markdown.blade.php index d03f0a3..b7cf3b7 100644 --- a/flexiapi/resources/views/api/documentation_markdown.blade.php +++ b/flexiapi/resources/views/api/documentation_markdown.blade.php @@ -134,6 +134,7 @@ JSON parameters: * `domain` if not set the value is enforced to the default registration domain set in the global configuration * `email` optional if `phone` set, an email, set an email to the account, must be unique if `ACCOUNT_EMAIL_UNIQUE` is set to `true` * `phone` required if `username` not set, optional if `email` set, a phone number, set a phone number to the account +* `account_creation_token` the unique `account_creation_token` ### `POST /accounts/with-account-creation-token` Public @@ -162,6 +163,8 @@ Return `404` if the account doesn't exists. Retrieve public information about the account. Return `404` if the account doesn't exists. +Return `phone: true` if the returned account has a phone number. + ### `POST /accounts/recover-by-phone` @if(config('app.dangerous_endpoints'))**Enabled on this instance**@else**Not enabled on this instance**@endif @@ -174,6 +177,7 @@ Return `404` if the account doesn't exists. JSON parameters: * `phone` required the phone number to send the SMS to +* `account_creation_token` the unique `account_creation_token` ### `GET /accounts/{sip}/recover/{recover_key}` @if(config('app.dangerous_endpoints'))**Enabled on this instance**@else**Not enabled on this instance**@endif @@ -182,6 +186,9 @@ JSON parameters: Public Unsecure endpoint Activate the account if the correct `recover_key` is provided. + +The `sip` parameter can be the default SIP account or the phone based one. + Return the account information (including the hashed password) if valid. Return `404` if the account doesn't exists. diff --git a/flexiapi/tests/Feature/ApiAccountTest.php b/flexiapi/tests/Feature/ApiAccountTest.php index 042e621..b777d02 100644 --- a/flexiapi/tests/Feature/ApiAccountTest.php +++ b/flexiapi/tests/Feature/ApiAccountTest.php @@ -21,6 +21,7 @@ namespace Tests\Feature; use App\Password; use App\Account; +use App\AccountCreationToken; use App\AccountTombstone; use App\ActivationExpiration; use App\Admin; @@ -565,8 +566,31 @@ class ApiAccountTest extends TestCase $this->assertDatabaseHas('accounts', [ 'username' => $password->account->username, 'domain' => $password->account->domain, + 'confirmation_key' => null, 'activated' => true ]); + + // Recover by alias + + $newConfirmationKey = '1345'; + + $password->account->confirmation_key = $newConfirmationKey; + $password->account->save(); + + $phone = '+1234'; + + $alias = new AppAlias; + $alias->alias = $phone; + $alias->domain = $password->account->domain; + $alias->account_id = $password->account->id; + $alias->save(); + + $this->get($this->route . '/' . $phone . '@' . $alias->domain . '/recover/' . $newConfirmationKey) + ->assertJson(['passwords' => [[ + 'password' => $password->password, + 'algorithm' => $password->algorithm + ]]]) + ->assertStatus(200); } /** @@ -591,6 +615,22 @@ class ApiAccountTest extends TestCase $this->json($this->method, $this->route . '/recover-by-phone', [ 'phone' => $phone + ]) + ->assertStatus(422) + ->assertJsonValidationErrors(['account_creation_token']); + + + $this->json($this->method, $this->route . '/recover-by-phone', [ + 'phone' => $phone, + 'account_creation_token' => 'wrong' + ]) + ->assertStatus(422) + ->assertJsonValidationErrors(['account_creation_token']); + $token = AccountCreationToken::factory()->create(); + + $this->json($this->method, $this->route . '/recover-by-phone', [ + 'phone' => $phone, + 'account_creation_token' => $token->token ]) ->assertStatus(200); @@ -605,7 +645,8 @@ class ApiAccountTest extends TestCase $this->get($this->route . '/' . $phone . '/info-by-phone') ->assertStatus(200) ->assertJson([ - 'activated' => true + 'activated' => true, + 'phone' => true ]); $this->get($this->route . '/+1234/info-by-phone') @@ -614,11 +655,25 @@ class ApiAccountTest extends TestCase $this->json('GET', $this->route . '/' . $password->account->identifier . '/info-by-phone') ->assertStatus(422) ->assertJsonValidationErrors(['phone']); + + // Check the mixed username/phone resolution... + $password->account->username = $phone; + $password->account->save(); + + $alias->delete(); + + $this->get($this->route . '/' . $phone . '/info-by-phone') + ->assertStatus(200) + ->assertJson([ + 'activated' => true, + 'phone' => false + ]); } /** * /!\ Dangerous endpoints */ + public function testCreatePublic() { $username = 'publicuser'; @@ -639,6 +694,18 @@ class ApiAccountTest extends TestCase 'algorithm' => 'SHA-256', 'password' => '2', 'email' => 'john@doe.tld', + ]) + ->assertStatus(422) + ->assertJsonValidationErrors(['account_creation_token']); + + $token = AccountCreationToken::factory()->create(); + + $this->json($this->method, $this->route . '/public', [ + 'username' => $username, + 'algorithm' => 'SHA-256', + 'password' => '2', + 'email' => 'john@doe.tld', + 'account_creation_token' => $token->token ]) ->assertStatus(200) ->assertJson([ @@ -700,11 +767,14 @@ class ApiAccountTest extends TestCase ->assertStatus(422) ->assertJsonValidationErrors(['phone']); + $token = AccountCreationToken::factory()->create(); + $this->json($this->method, $this->route . '/public', [ 'phone' => $phone, 'algorithm' => 'SHA-256', 'password' => '2', 'email' => 'john@doe.tld', + 'account_creation_token' => $token->token ]) ->assertStatus(200) ->assertJson([