diff --git a/CHANGELOG.md b/CHANGELOG.md index 440b5e4..902788c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/flexiapi/app/Account.php b/flexiapi/app/Account.php index 68d0fd5..55fafb5 100644 --- a/flexiapi/app/Account.php +++ b/flexiapi/app/Account.php @@ -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) { diff --git a/flexiapi/app/Alias.php b/flexiapi/app/Alias.php deleted file mode 100644 index 4b5f071..0000000 --- a/flexiapi/app/Alias.php +++ /dev/null @@ -1,45 +0,0 @@ -. -*/ - -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); - } -} diff --git a/flexiapi/app/Http/Controllers/Account/AuthenticateController.php b/flexiapi/app/Http/Controllers/Account/AuthenticateController.php index aa0b6ea..b77140a 100644 --- a/flexiapi/app/Http/Controllers/Account/AuthenticateController.php +++ b/flexiapi/app/Http/Controllers/Account/AuthenticateController.php @@ -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) { diff --git a/flexiapi/app/Http/Controllers/Account/RecoveryController.php b/flexiapi/app/Http/Controllers/Account/RecoveryController.php index a2a9eed..08497e7 100644 --- a/flexiapi/app/Http/Controllers/Account/RecoveryController.php +++ b/flexiapi/app/Http/Controllers/Account/RecoveryController.php @@ -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(); } } diff --git a/flexiapi/app/Http/Controllers/Admin/AccountImportController.php b/flexiapi/app/Http/Controllers/Admin/AccountImportController.php index d22d54b..a7132cc 100644 --- a/flexiapi/app/Http/Controllers/Admin/AccountImportController.php +++ b/flexiapi/app/Http/Controllers/Admin/AccountImportController.php @@ -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 '); diff --git a/flexiapi/app/Http/Controllers/Api/Account/AccountController.php b/flexiapi/app/Http/Controllers/Api/Account/AccountController.php index ad41366..85c9b79 100644 --- a/flexiapi/app/Http/Controllers/Api/Account/AccountController.php +++ b/flexiapi/app/Http/Controllers/Api/Account/AccountController.php @@ -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; diff --git a/flexiapi/app/Http/Requests/Account/Create/Request.php b/flexiapi/app/Http/Requests/Account/Create/Request.php index d822cc4..7b09f86 100644 --- a/flexiapi/app/Http/Requests/Account/Create/Request.php +++ b/flexiapi/app/Http/Requests/Account/Create/Request.php @@ -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:+' ] diff --git a/flexiapi/app/Http/Requests/Account/Update/Request.php b/flexiapi/app/Http/Requests/Account/Update/Request.php index 5083bcd..a821d91 100644 --- a/flexiapi/app/Http/Requests/Account/Update/Request.php +++ b/flexiapi/app/Http/Requests/Account/Update/Request.php @@ -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:+' ] ]; diff --git a/flexiapi/app/Services/AccountService.php b/flexiapi/app/Services/AccountService.php index 1184caf..1bf93c2 100644 --- a/flexiapi/app/Services/AccountService.php +++ b/flexiapi/app/Services/AccountService.php @@ -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(); diff --git a/flexiapi/database/migrations/2024_04_09_140622_drop_aliases_table.php b/flexiapi/database/migrations/2024_04_09_140622_drop_aliases_table.php new file mode 100644 index 0000000..899636b --- /dev/null +++ b/flexiapi/database/migrations/2024_04_09_140622_drop_aliases_table.php @@ -0,0 +1,50 @@ +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'); + }); + } +}; diff --git a/flexiapi/database/seeds/LiblinphoneTesterAccoutSeeder.php b/flexiapi/database/seeds/LiblinphoneTesterAccoutSeeder.php index 2d2e12f..6f9f442 100644 --- a/flexiapi/database/seeds/LiblinphoneTesterAccoutSeeder.php +++ b/flexiapi/database/seeds/LiblinphoneTesterAccoutSeeder.php @@ -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": , "username": "", "domain": "", - * "passwords": [{ "hash": "", "algorithm": ""}], - * "alias": { "alias": "", "domain": ""} + * "passwords": [{ "hash": "", "algorithm": ""}] * }, * {"type": "range", "id": "%id%", "username": "user_%usernamePostfix%", "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 - ]; - } } diff --git a/flexiapi/resources/views/account/documentation_markdown.blade.php b/flexiapi/resources/views/account/documentation_markdown.blade.php index 1a69b46..50b2f0f 100644 --- a/flexiapi/resources/views/account/documentation_markdown.blade.php +++ b/flexiapi/resources/views/account/documentation_markdown.blade.php @@ -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 diff --git a/flexiapi/tests/Feature/ApiAccountTest.php b/flexiapi/tests/Feature/ApiAccountTest.php index e770579..9394489 100644 --- a/flexiapi/tests/Feature/ApiAccountTest.php +++ b/flexiapi/tests/Feature/ApiAccountTest.php @@ -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') ]); }