Fix FLEXIAPI-162 Drop the aliases table

This commit is contained in:
Timothée Jaussoin 2024-04-22 13:38:51 +00:00
parent 7418d79b41
commit faf33f5ac3
14 changed files with 98 additions and 194 deletions

View file

@ -2,6 +2,7 @@
v1.5
----
- Fix FLEXIAPI-162 Drop the aliases table and migrate the data to the phone column
- Fix FLEXIAPI-161 Complete the Dictionary tests to cover the collection accessor
- Fix FLEXIAPI-158 Restrict the phone number change API endpoint to return 403 if the account doesn't have a validated Account Creation Token
- Fix FLEXIAPI-156 Disable the Phone change web form when PHONE_AUTHENTICATION is disabled

View file

@ -35,9 +35,9 @@ class Account extends Authenticatable
use HasFactory;
use Compoships;
protected $with = ['passwords', 'alias', 'activationExpiration', 'emailChangeCode', 'types', 'actions', 'dictionaryEntries'];
protected $hidden = ['alias', 'expire_time', 'confirmation_key', 'pivot', 'currentProvisioningToken', 'currentRecoveryCode', 'dictionaryEntries'];
protected $appends = ['realm', 'phone', 'confirmation_key_expires', 'provisioning_token', 'dictionary'];
protected $with = ['passwords', 'activationExpiration', 'emailChangeCode', 'types', 'actions', 'dictionaryEntries'];
protected $hidden = ['expire_time', 'confirmation_key', 'pivot', 'currentProvisioningToken', 'currentRecoveryCode', 'dictionaryEntries'];
protected $appends = ['realm', 'confirmation_key_expires', 'provisioning_token', 'dictionary'];
protected $casts = [
'activated' => 'boolean',
];
@ -84,9 +84,9 @@ class Account extends Authenticatable
public function scopeSip($query, string $sip)
{
if (\str_contains($sip, '@')) {
list($usernane, $domain) = explode('@', $sip);
list($username, $domain) = explode('@', $sip);
return $query->where('username', $usernane)
return $query->where('username', $username)
->where('domain', $domain);
};
@ -120,11 +120,6 @@ class Account extends Authenticatable
return $this->hasOne(ActivationExpiration::class);
}
public function alias()
{
return $this->hasOne(Alias::class);
}
public function apiKey()
{
return $this->hasOne(ApiKey::class);
@ -302,28 +297,6 @@ class Account extends Authenticatable
return config('app.realm') ?? $this->domain;
}
public function getPhoneAttribute()
{
if ($this->alias) {
return $this->alias->alias;
}
return null;
}
public function setPhoneAttribute(?string $phone)
{
$this->alias()->delete();
if (!empty($phone)) {
$alias = new Alias;
$alias->alias = $phone;
$alias->domain = config('app.sip_domain');
$alias->account_id = $this->id;
$alias->save();
}
}
public function getConfirmationKeyExpiresAttribute()
{
if ($this->activationExpiration) {

View file

@ -1,45 +0,0 @@
<?php
/*
Flexisip Account Manager is a set of tools to manage SIP accounts.
Copyright (C) 2020 Belledonne Communications SARL, All rights reserved.
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU Affero General Public License as
published by the Free Software Foundation, either version 3 of the
License, or (at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU Affero General Public License for more details.
You should have received a copy of the GNU Affero General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
namespace App;
use Illuminate\Database\Eloquent\Model;
class Alias extends Model
{
protected $table = 'aliases';
public $timestamps = false;
public function account()
{
return $this->belongsTo(Account::class);
}
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);
}
}

View file

@ -25,7 +25,6 @@ use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Str;
use App\Account;
use App\Alias;
use App\AuthToken;
class AuthenticateController extends Controller
@ -53,13 +52,8 @@ class AuthenticateController extends Controller
$account = Account::where('username', $request->get('username'))
->first();
// Try alias
if (!$account) {
$alias = Alias::where('alias', $request->get('username'))->first();
if ($alias) {
$account = $alias->account;
}
$account = Account::where('phone', $request->get('username'))->first();
}
if (!$account) {

View file

@ -20,7 +20,6 @@
namespace App\Http\Controllers\Account;
use App\Account;
use App\Alias;
use Illuminate\Http\Request;
use App\Http\Controllers\Controller;
use App\Services\AccountService;
@ -73,24 +72,16 @@ class RecoveryController extends Controller
$account = $account->first();
// Try alias
if (!$account) {
$alias = Alias::where('alias', $request->get('username'))->first();
if ($alias && $alias->account->email == $request->get('email')) {
$account = $alias->account;
}
$account = Account::where('phone', $request->get('username'))
->where('email', $request->get('email'))
->first();
}
} elseif ($request->get('phone')) {
$account = Account::where('username', $request->get('phone'))->first();
// Try alias
if (!$account) {
$alias = Alias::where('alias', $request->get('phone'))->first();
if ($alias) {
$account = $alias->account;
}
$account = Account::where('phone', $request->get('phone'))->first();
}
}

View file

@ -20,11 +20,9 @@
namespace App\Http\Controllers\Admin;
use App\Account;
use App\Alias;
use App\Http\Controllers\Controller;
use Illuminate\Support\Collection;
use Illuminate\Http\Request;
use Illuminate\Http\UploadedFile;
use Illuminate\Support\Facades\Storage;
use Illuminate\Validation\Rules\File;
@ -114,8 +112,8 @@ class AccountImportController extends Controller
}
}
$existingPhones = Alias::whereIn('alias', $lines->pluck('phone')->all())
->pluck('alias');
$existingPhones = Account::whereIn('phone', $lines->pluck('phone')->all())
->pluck('phone');
if ($existingPhones->isNotEmpty()) {
$this->errors['Those phones numbers already exists'] = $existingPhones->join(', ', ' and ');

View file

@ -29,7 +29,6 @@ use Carbon\Carbon;
use App\Account;
use App\AccountCreationToken;
use App\Alias;
use App\Http\Controllers\Account\AuthenticateController as WebAuthenticateController;
use App\Http\Requests\Account\Create\Api\Request as ApiRequest;
@ -72,16 +71,16 @@ class AccountController extends Controller
'phone' => ['required', new WithoutSpaces, 'starts_with:+']
]);
$alias = Alias::where('alias', $phone)->first();
$account = $alias
? $alias->account
// Injecting the default sip domain to try to resolve the account
: Account::sip($phone . '@' . config('app.sip_domain'))->firstOrFail();
$account = Account::where('domain', config('app.sip_domain'))
->where(function ($query) use ($phone) {
$query->where('username', $phone)
->orWhere('phone', $phone);
})->firstOrFail();
return \response()->json([
'activated' => $account->activated,
'realm' => $account->realm,
'phone' => (bool)$alias
'phone' => (bool)$account->phone
]);
}
@ -116,7 +115,7 @@ class AccountController extends Controller
'phone' => [
'required_without:email',
'required_without:username',
'unique:aliases,alias',
'unique:accounts,phone',
'unique:accounts,username',
new WithoutSpaces, 'starts_with:+'
],
@ -152,12 +151,7 @@ class AccountController extends Controller
// Send validation by phone
if ($request->has('phone')) {
$alias = new Alias;
$alias->alias = $request->get('phone');
$alias->domain = config('app.sip_domain');
$alias->account_id = $account->id;
$alias->save();
$account->phone = $request->get('phone');
$account->confirmation_key = generatePin();
$account->save();
@ -201,11 +195,7 @@ class AccountController extends Controller
]
]);
$alias = Alias::where('alias', $request->get('phone'))->first();
$account = $alias
? $alias->account
: Account::sip($request->get('phone') . '@' . config('app.sip_domain'))->firstOrFail();
$account = Account::where('phone', $request->get('phone'))->first();
$account->confirmation_key = generatePin();
$account->save();
@ -228,10 +218,13 @@ class AccountController extends Controller
{
if (!config('app.dangerous_endpoints')) return abort(404);
$alias = Alias::sip($sip)->first();
$account = $alias
? $alias->account
: Account::sip($sip)->firstOrFail();
list($username, $domain) = explode('@', $sip);
$account = Account::where('domain', $domain)
->where(function ($query) use ($username) {
$query->where('username', $username)
->orWhere('phone', $username);
})->firstOrFail();
$confirmationKey = $account->confirmation_key;
$account->confirmation_key = null;

View file

@ -45,7 +45,7 @@ class Request extends FormRequest
'dtmf_protocol' => 'nullable|in:' . Account::dtmfProtocolsRule(),
'phone' => [
'nullable',
'unique:aliases,alias',
'unique:accounts,phone',
'unique:accounts,username',
new WithoutSpaces(), 'starts_with:+'
]

View file

@ -45,7 +45,7 @@ class Request extends FormRequest
Rule::unique('accounts', 'username')->where(function ($query) {
$query->where('domain', resolveDomain($this));
})->ignore($this->route('id'), 'id'),
Rule::unique('aliases', 'alias')->ignore($this->route('account_id'), 'account_id'),
Rule::unique('accounts', 'phone')->ignore($this->route('account_id'), 'id'),
new WithoutSpaces, 'starts_with:+'
]
];

View file

@ -22,7 +22,6 @@ namespace App\Services;
use App\Account;
use App\AccountCreationToken;
use App\ActivationExpiration;
use App\Alias;
use App\EmailChangeCode;
use App\Http\Controllers\Account\AuthenticateController as WebAuthenticateController;
use App\Http\Requests\Account\Create\Request as CreateRequest;
@ -98,6 +97,7 @@ class AccountService
}
$account->phone = $request->get('phone');
$account->save();
}
$account->updatePassword($request->get('password'), $request->get('algorithm'));
@ -170,9 +170,8 @@ class AccountService
);
}
$account->save();
$account->phone = $request->get('phone');
$account->save();
}
Log::channel('events')->info(
@ -214,7 +213,7 @@ class AccountService
{
$request->validate([
'phone' => [
'required', 'unique:aliases,alias',
'required', 'unique:accounts,phone',
'unique:accounts,username',
new WithoutSpaces(), 'starts_with:+'
]
@ -254,19 +253,12 @@ class AccountService
$phoneChangeCode = $account->phoneChangeCode()->firstOrFail();
if ($phoneChangeCode->code == $code) {
$account->alias()->delete();
$alias = new Alias();
$alias->alias = $phoneChangeCode->phone;
$alias->domain = config('app.sip_domain');
$alias->account_id = $account->id;
$alias->save();
Log::channel('events')->info('Account Service: Account phone changed using SMS', ['id' => $account->identifier]);
$account->phone = $phoneChangeCode->phone;
$account->activated = true;
$account->save();
Log::channel('events')->info('Account Service: Account phone changed using SMS', ['id' => $account->identifier]);
$account->refresh();
$phoneChangeCode->consume();

View file

@ -0,0 +1,50 @@
<?php
use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Schema;
return new class extends Migration
{
public function up()
{
Schema::table('accounts', function (Blueprint $table) {
$table->string('phone', 64)->nullable();
});
DB::table('accounts')->update([
'phone' => DB::raw('(select alias from aliases where aliases.account_id = accounts.id)')
]);
Schema::dropIfExists('aliases');
}
public function down()
{
Schema::create('aliases', function (Blueprint $table) {
$table->id();
$table->integer('account_id')->unsigned();
$table->string('alias', 64);
$table->string('domain', 64);
$table->foreign('account_id')->references('id')
->on('accounts')->onDelete('cascade');
$table->unique(['alias', 'domain']);
});
DB::table('aliases')
->insertUsing(
['account_id','alias', 'domain'],
DB::table('accounts')
->select('id','phone','domain')
->whereNotNull('phone')
);
Schema::table('accounts', function (Blueprint $table) {
$table->dropColumn('phone');
});
}
};

View file

@ -9,11 +9,10 @@ use Illuminate\Support\Facades\DB;
/**
* This seeder is only used for liblinphone related tests
* The JSON MUST respect the following format. type: range requires a range object.
* alias and passwords are optionnal.
* passwords is optionnal.
* [
* {"type": "account", "id": <id>, "username": "<username>", "domain": "<domain>",
* "passwords": [{ "hash": "<hash>", "algorithm": "<algorithm>"}],
* "alias": { "alias": "<alias>", "domain": "<domain>"}
* "passwords": [{ "hash": "<hash>", "algorithm": "<algorithm>"}]
* },
* {"type": "range", "id": "%id%", "username": "user_%usernamePostfix%", "domain": "<domain>",
* "iteration": 2000,
@ -29,7 +28,6 @@ class LiblinphoneTesterAccoutSeeder extends Seeder
{
$accounts = [];
$passwords = [];
$aliases = [];
foreach ($json as $element) {
if ($element->type == 'account') {
@ -56,17 +54,6 @@ class LiblinphoneTesterAccoutSeeder extends Seeder
);
}
}
if (isset($element->alias)) {
array_push(
$aliases,
$this->generateAliasArray(
$element->id,
$element->alias->alias,
$element->alias->domain
)
);
}
}
if ($element->type == 'range') {
@ -98,7 +85,6 @@ class LiblinphoneTesterAccoutSeeder extends Seeder
// And seed the fresh ones
DB::table('accounts')->insert($accounts);
DB::table('passwords')->insert($passwords);
DB::table('aliases')->insert($aliases);
}
private function generateAccountArray(
@ -130,16 +116,4 @@ class LiblinphoneTesterAccoutSeeder extends Seeder
'algorithm' => $algorythm
];
}
private function generateAliasArray(
int $accountId,
string $alias,
string $domain
): array {
return [
'account_id' => $accountId,
'alias' => $alias,
'domain' => $domain
];
}
}

View file

@ -98,7 +98,7 @@ Administrators can create and edit accounts directly from the admin panel. Durin
### Delete an account
The deletion of an account is definitive, all the database related data (password, aliases) will be destroyed after the deletion.
The deletion of an account is definitive, all the database related data (password) will be destroyed after the deletion.
### Create, edit and delete account types

View file

@ -23,7 +23,6 @@ use App\Account;
use App\AccountCreationToken;
use App\AccountTombstone;
use App\ActivationExpiration;
use App\Alias as AppAlias;
use App\Password;
use Carbon\Carbon;
use Tests\TestCase;
@ -687,22 +686,16 @@ class ApiAccountTest extends TestCase
'activated' => true
]);
// Recover by alias
// Recover by phone
$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();
$password->account->confirmation_key = $newConfirmationKey;
$password->account->phone = $phone;
$password->account->save();
$this->get($this->route . '/' . $phone . '@' . $alias->domain . '/recover/' . $newConfirmationKey)
$this->get($this->route . '/' . $phone . '@' . $password->account->domain . '/recover/' . $newConfirmationKey)
->assertJson(['passwords' => [[
'password' => $password->password,
'algorithm' => $password->algorithm
@ -737,16 +730,11 @@ class ApiAccountTest extends TestCase
$password = Password::factory()->create();
$password->account->generateApiKey();
$password->account->activated = false;
$password->account->phone = $phone;
$password->account->save();
config()->set('app.dangerous_endpoints', true);
$alias = new AppAlias();
$alias->alias = $phone;
$alias->domain = $password->account->domain;
$alias->account_id = $password->account->id;
$alias->save();
$this->json($this->method, $this->route . '/recover-by-phone', [
'phone' => $phone
])->assertJsonValidationErrors(['account_creation_token']);
@ -792,10 +780,9 @@ class ApiAccountTest extends TestCase
// Check the mixed username/phone resolution...
$password->account->username = $phone;
$password->account->phone = null;
$password->account->save();
$alias->delete();
$this->get($this->route . '/' . $phone . '/info-by-phone')
->assertStatus(200)
->assertJson([
@ -929,11 +916,7 @@ class ApiAccountTest extends TestCase
$this->assertDatabaseHas('accounts', [
'username' => $phone,
'domain' => config('app.sip_domain')
]);
$this->assertDatabaseHas('aliases', [
'alias' => $phone,
'phone' => $phone,
'domain' => config('app.sip_domain')
]);
}