From 46af75fea31bfd7ca58f3d3b539c94785b54cba6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Jaussoin?= Date: Tue, 23 Feb 2021 17:45:33 +0100 Subject: [PATCH] Add account expirations table Complete POST /accounts admin endpoints Handle expiration in email and phone endpoints Complete documentation Add related tests Bump package version --- flexiapi/app/Account.php | 23 ++++- flexiapi/app/ActivationExpiration.php | 28 ++++++ flexiapi/app/Events/AccountDeleting.php | 1 + .../Controllers/Api/AccountController.php | 7 +- .../Api/Admin/AccountController.php | 15 +++- ...53_create_activation_expirations_table.php | 23 +++++ .../resources/views/documentation.blade.php | 3 +- flexiapi/tests/Feature/AccountApiTest.php | 87 +++++++++++++++++-- flexisip-account-manager.spec | 2 +- 9 files changed, 176 insertions(+), 13 deletions(-) create mode 100644 flexiapi/app/ActivationExpiration.php create mode 100644 flexiapi/database/migrations/2021_03_02_151353_create_activation_expirations_table.php diff --git a/flexiapi/app/Account.php b/flexiapi/app/Account.php index 8e212a1..8486397 100644 --- a/flexiapi/app/Account.php +++ b/flexiapi/app/Account.php @@ -38,10 +38,10 @@ class Account extends Authenticatable use HasFactory; protected $connection = 'external'; - protected $with = ['passwords', 'admin', 'emailChanged', 'alias']; + protected $with = ['passwords', 'admin', 'emailChanged', 'alias', 'activationExpiration']; protected $hidden = ['alias', 'expire_time', 'confirmation_key']; protected $dateTimes = ['creation_time']; - protected $appends = ['realm', 'phone']; + protected $appends = ['realm', 'phone', 'confirmation_key_expires']; protected $casts = [ 'activated' => 'boolean', ]; @@ -97,6 +97,11 @@ class Account extends Authenticatable return $this->hasOne('App\Admin'); } + public function activationExpiration() + { + return $this->hasOne('App\ActivationExpiration'); + } + public function apiKey() { return $this->hasOne('App\ApiKey'); @@ -131,6 +136,20 @@ class Account extends Authenticatable return null; } + public function getConfirmationKeyExpiresAttribute() + { + if ($this->activationExpiration) { + return $this->activationExpiration->expires->format('Y-m-d H:i:s'); + } + + return null; + } + + public function activationExpired(): bool + { + return ($this->activationExpiration && $this->activationExpiration->isExpired()); + } + public function requestEmailUpdate(string $newEmail) { // Remove all the old requests diff --git a/flexiapi/app/ActivationExpiration.php b/flexiapi/app/ActivationExpiration.php new file mode 100644 index 0000000..fc5bd90 --- /dev/null +++ b/flexiapi/app/ActivationExpiration.php @@ -0,0 +1,28 @@ + 'datetime:Y-m-d H:i:s', + ]; + + public function account() + { + return $this->belongsTo('App\Account'); + } + + public function isExpired() + { + $now = Carbon::now(); + return $this->expires->lessThan($now); + } +} diff --git a/flexiapi/app/Events/AccountDeleting.php b/flexiapi/app/Events/AccountDeleting.php index 1099597..e878ccf 100644 --- a/flexiapi/app/Events/AccountDeleting.php +++ b/flexiapi/app/Events/AccountDeleting.php @@ -16,6 +16,7 @@ class AccountDeleting { $account->alias()->delete(); $account->passwords()->delete(); + $account->activationExpiration()->delete(); $account->nonces()->delete(); $account->admin()->delete(); $account->apiKey()->delete(); diff --git a/flexiapi/app/Http/Controllers/Api/AccountController.php b/flexiapi/app/Http/Controllers/Api/AccountController.php index c60cbc2..54b305f 100644 --- a/flexiapi/app/Http/Controllers/Api/AccountController.php +++ b/flexiapi/app/Http/Controllers/Api/AccountController.php @@ -100,9 +100,11 @@ class AccountController extends Controller $account = Account::sip($sip) ->where('confirmation_key', $request->get('code')) ->firstOrFail(); + + if ($account->activationExpired()) abort(403, 'Activation expired'); + $account->activated = true; $account->confirmation_key = null; - $account->save(); return $account; @@ -117,6 +119,9 @@ class AccountController extends Controller $account = Account::sip($sip) ->where('confirmation_key', $request->get('code')) ->firstOrFail(); + + if ($account->activationExpired()) abort(403, 'Activation expired'); + $account->activated = true; $account->confirmation_key = null; $account->save(); diff --git a/flexiapi/app/Http/Controllers/Api/Admin/AccountController.php b/flexiapi/app/Http/Controllers/Api/Admin/AccountController.php index ba80b50..fbf9baa 100644 --- a/flexiapi/app/Http/Controllers/Api/Admin/AccountController.php +++ b/flexiapi/app/Http/Controllers/Api/Admin/AccountController.php @@ -26,10 +26,9 @@ use Illuminate\Validation\Rule; use Carbon\Carbon; use App\Account; +use App\ActivationExpiration; use App\Admin; -use App\Password; use App\Rules\WithoutSpaces; -use App\Helpers\Utils; use App\Http\Controllers\Account\AuthenticateController as WebAuthenticateController; class AccountController extends Controller @@ -84,6 +83,10 @@ class AccountController extends Controller 'domain' => 'min:3', 'admin' => 'boolean|nullable', 'activated' => 'boolean|nullable', + 'confirmation_key_expires' => [ + 'date_format:Y-m-d H:i:s', + 'nullable', + ] ]); $account = new Account; @@ -105,6 +108,14 @@ class AccountController extends Controller $account->save(); + if ((!$request->has('activated') || !(bool)$request->get('activated')) + && $request->has('confirmation_key_expires')) { + $actionvationExpiration = new ActivationExpiration; + $actionvationExpiration->account_id = $account->id; + $actionvationExpiration->expires = $request->get('confirmation_key_expires'); + $actionvationExpiration->save(); + } + $account->updatePassword($request->get('password'), $request->get('algorithm')); if ($request->has('admin') && (bool)$request->get('admin')) { diff --git a/flexiapi/database/migrations/2021_03_02_151353_create_activation_expirations_table.php b/flexiapi/database/migrations/2021_03_02_151353_create_activation_expirations_table.php new file mode 100644 index 0000000..e72a770 --- /dev/null +++ b/flexiapi/database/migrations/2021_03_02_151353_create_activation_expirations_table.php @@ -0,0 +1,23 @@ +create('activation_expirations', function (Blueprint $table) { + $table->id(); + $table->integer('account_id')->unsigned(); + $table->dateTime('expires'); + $table->timestamps(); + }); + } + + public function down() + { + Schema::connection('local')->dropIfExists('activation_expirations'); + } +} diff --git a/flexiapi/resources/views/documentation.blade.php b/flexiapi/resources/views/documentation.blade.php index 12cad9e..f3a3b8b 100644 --- a/flexiapi/resources/views/documentation.blade.php +++ b/flexiapi/resources/views/documentation.blade.php @@ -157,7 +157,7 @@ For the moment only DIGEST-MD5 and DIGEST-SHA-256 are supported through the auth

POST /accounts

To create an account directly from the API.

-

If activated is set to false a random generated confirmation_key will be returned to allow further activation using the public endpoints.

+

If activated is set to false a random generated confirmation_key will be returned to allow further activation using the public endpoints. Check confirmation_key_expires to also set an expiration date on that confirmation_key.

JSON parameters:

GET /accounts

diff --git a/flexiapi/tests/Feature/AccountApiTest.php b/flexiapi/tests/Feature/AccountApiTest.php index dd2038c..3d5b8a7 100644 --- a/flexiapi/tests/Feature/AccountApiTest.php +++ b/flexiapi/tests/Feature/AccountApiTest.php @@ -21,8 +21,9 @@ namespace Tests\Feature; use App\Password; use App\Account; +use App\ActivationExpiration; use App\Admin; - +use Carbon\Carbon; use Illuminate\Foundation\Testing\RefreshDatabase; use Tests\TestCase; @@ -226,8 +227,7 @@ class AccountApiTest extends TestCase 'activated' => false, ]); - $response1 - ->assertStatus(200) + $response1->assertStatus(200) ->assertJson([ 'id' => 2, 'username' => $username, @@ -296,6 +296,11 @@ class AccountApiTest extends TestCase $password->account->activated = false; $password->account->save(); + $expiration = new ActivationExpiration; + $expiration->account_id = $password->account->id; + $expiration->expires = Carbon::now()->subYear(); + $expiration->save(); + $this->get($this->route.'/'.$password->account->identifier.'/info') ->assertStatus(200) ->assertJson([ @@ -308,20 +313,31 @@ class AccountApiTest extends TestCase ]) ->assertStatus(404); + $activateEmailRoute = $this->route.'/'.$password->account->identifier.'/activate/email'; + $this->keyAuthenticated($password->account) - ->json($this->method, $this->route.'/'.$password->account->identifier.'/activate/email', [ + ->json($this->method, $activateEmailRoute, [ 'code' => $confirmationKey.'longer' ]) ->assertStatus(422); $this->keyAuthenticated($password->account) - ->json($this->method, $this->route.'/'.$password->account->identifier.'/activate/email', [ + ->json($this->method, $activateEmailRoute, [ 'code' => 'X123456789abc' ]) ->assertStatus(404); + // Expired $this->keyAuthenticated($password->account) - ->json($this->method, $this->route.'/'.$password->account->identifier.'/activate/email', [ + ->json($this->method, $activateEmailRoute, [ + 'code' => $confirmationKey + ]) + ->assertStatus(403); + + $expiration->delete(); + + $this->keyAuthenticated($password->account) + ->json($this->method, $activateEmailRoute, [ 'code' => $confirmationKey ]) ->assertStatus(200); @@ -342,12 +358,26 @@ class AccountApiTest extends TestCase $password->account->activated = false; $password->account->save(); + $expiration = new ActivationExpiration; + $expiration->account_id = $password->account->id; + $expiration->expires = Carbon::now()->subYear(); + $expiration->save(); + $this->get($this->route.'/'.$password->account->identifier.'/info') ->assertStatus(200) ->assertJson([ 'activated' => false ]); + // Expired + $this->keyAuthenticated($password->account) + ->json($this->method, $this->route.'/'.$password->account->identifier.'/activate/phone', [ + 'code' => $confirmationKey + ]) + ->assertStatus(403); + + $expiration->delete(); + $this->keyAuthenticated($password->account) ->json($this->method, $this->route.'/'.$password->account->identifier.'/activate/phone', [ 'code' => $confirmationKey @@ -544,6 +574,51 @@ class AccountApiTest extends TestCase ]); } + public function testCodeExpires() + { + $admin = Admin::factory()->create(); + $admin->account->generateApiKey(); + + // Activated, no no confirmation_key + $this->keyAuthenticated($admin->account) + ->json($this->method, $this->route, [ + 'username' => 'foobar', + 'algorithm' => 'SHA-256', + 'password' => '123456', + 'activated' => true, + 'confirmation_key_expires' => '2040-12-12 12:12:12' + ]) + ->assertStatus(200) + ->assertJson([ + 'confirmation_key_expires' => null + ]); + + // Bad datetime format + $this->keyAuthenticated($admin->account) + ->json($this->method, $this->route, [ + 'username' => 'foobar2', + 'algorithm' => 'SHA-256', + 'password' => '123456', + 'activated' => false, + 'confirmation_key_expires' => 'abc' + ]) + ->assertStatus(422); + + // Bad datetime format + $this->keyAuthenticated($admin->account) + ->json($this->method, $this->route, [ + 'username' => 'foobar2', + 'algorithm' => 'SHA-256', + 'password' => '123456', + 'activated' => false, + 'confirmation_key_expires' => '2040-12-12 12:12:12' + ]) + ->assertStatus(200) + ->assertJson([ + 'confirmation_key_expires' => '2040-12-12 12:12:12' + ]);; + } + public function testDelete() { $password = Password::factory()->create(); diff --git a/flexisip-account-manager.spec b/flexisip-account-manager.spec index a00db15..191498d 100644 --- a/flexisip-account-manager.spec +++ b/flexisip-account-manager.spec @@ -8,7 +8,7 @@ #%define _datadir %{_datarootdir} #%define _docdir %{_datadir}/doc -%define build_number 52 +%define build_number 53 %define var_dir /var/opt/belledonne-communications %define opt_dir /opt/belledonne-communications/share/flexisip-account-manager %define env_file "$RPM_BUILD_ROOT/etc/flexisip-account-manager/flexiapi.env"