diff --git a/CHANGELOG.md b/CHANGELOG.md index 48f6dbf..646f9fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ v1.5 ---- +- Fix FLEXIAPI-168 Add POST /accounts/me/email to confirm the email change - Fix FLEXIAPI-166 Reimplement the deprecated email validation URL - Fix FLEXIAPI-165 Remove for now text/vcard header constraint - Fix FLEXIAPI-164 Add vcards-storage endpoints diff --git a/flexiapi/app/Http/Controllers/Api/Account/EmailController.php b/flexiapi/app/Http/Controllers/Api/Account/EmailController.php index d61a92c..4aa650c 100644 --- a/flexiapi/app/Http/Controllers/Api/Account/EmailController.php +++ b/flexiapi/app/Http/Controllers/Api/Account/EmailController.php @@ -33,6 +33,15 @@ class EmailController extends Controller return abort(403, 'Account blocked'); } + if (!$request->user()->accountCreationToken?->consumed()) { + return abort(403, 'Account unvalidated'); + } + (new AccountService)->requestEmailChange($request); } + + public function update(Request $request) + { + return (new AccountService)->updateEmail($request); + } } diff --git a/flexiapi/composer.lock b/flexiapi/composer.lock index abfc521..e4c5c5e 100644 --- a/flexiapi/composer.lock +++ b/flexiapi/composer.lock @@ -466,16 +466,16 @@ }, { "name": "doctrine/dbal", - "version": "3.8.3", + "version": "3.8.4", "source": { "type": "git", "url": "https://github.com/doctrine/dbal.git", - "reference": "db922ba9436b7b18a23d1653a0b41ff2369ca41c" + "reference": "b05e48a745f722801f55408d0dbd8003b403dbbd" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/doctrine/dbal/zipball/db922ba9436b7b18a23d1653a0b41ff2369ca41c", - "reference": "db922ba9436b7b18a23d1653a0b41ff2369ca41c", + "url": "https://api.github.com/repos/doctrine/dbal/zipball/b05e48a745f722801f55408d0dbd8003b403dbbd", + "reference": "b05e48a745f722801f55408d0dbd8003b403dbbd", "shasum": "" }, "require": { @@ -559,7 +559,7 @@ ], "support": { "issues": "https://github.com/doctrine/dbal/issues", - "source": "https://github.com/doctrine/dbal/tree/3.8.3" + "source": "https://github.com/doctrine/dbal/tree/3.8.4" }, "funding": [ { @@ -575,7 +575,7 @@ "type": "tidelift" } ], - "time": "2024-03-03T15:55:06+00:00" + "time": "2024-04-25T07:04:44+00:00" }, { "name": "doctrine/deprecations", @@ -9975,16 +9975,16 @@ }, { "name": "squizlabs/php_codesniffer", - "version": "3.9.1", + "version": "3.9.2", "source": { "type": "git", "url": "https://github.com/PHPCSStandards/PHP_CodeSniffer.git", - "reference": "267a4405fff1d9c847134db3a3c92f1ab7f77909" + "reference": "aac1f6f347a5c5ac6bc98ad395007df00990f480" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/PHPCSStandards/PHP_CodeSniffer/zipball/267a4405fff1d9c847134db3a3c92f1ab7f77909", - "reference": "267a4405fff1d9c847134db3a3c92f1ab7f77909", + "url": "https://api.github.com/repos/PHPCSStandards/PHP_CodeSniffer/zipball/aac1f6f347a5c5ac6bc98ad395007df00990f480", + "reference": "aac1f6f347a5c5ac6bc98ad395007df00990f480", "shasum": "" }, "require": { @@ -10051,7 +10051,7 @@ "type": "open_collective" } ], - "time": "2024-03-31T21:03:09+00:00" + "time": "2024-04-23T20:25:34+00:00" }, { "name": "symfony/config", diff --git a/flexiapi/database/factories/AccountFactory.php b/flexiapi/database/factories/AccountFactory.php index 1bf0ad1..5959f97 100644 --- a/flexiapi/database/factories/AccountFactory.php +++ b/flexiapi/database/factories/AccountFactory.php @@ -38,7 +38,6 @@ class AccountFactory extends Factory 'username' => $this->faker->username, 'display_name' => $this->faker->name, 'domain' => config('app.sip_domain'), - 'email' => $this->faker->email, 'user_agent' => $this->faker->userAgent, 'confirmation_key' => Str::random(WebAuthenticateController::$emailCodeSize), 'ip_address' => $this->faker->ipv4, @@ -56,6 +55,13 @@ class AccountFactory extends Factory ]); } + public function withEmail() + { + return $this->state(fn (array $attributes) => [ + 'email' => $this->faker->email, + ]); + } + public function withConsumedAccountCreationToken() { return $this->state(fn (array $attributes) => [])->afterCreating(function (Account $account) { diff --git a/flexiapi/database/factories/EmailChangeCodeFactory.php b/flexiapi/database/factories/EmailChangeCodeFactory.php new file mode 100644 index 0000000..8bb4be4 --- /dev/null +++ b/flexiapi/database/factories/EmailChangeCodeFactory.php @@ -0,0 +1,41 @@ +. +*/ + +namespace Database\Factories; + +use App\Account; +use App\EmailChangeCode; +use Illuminate\Database\Eloquent\Factories\Factory; + +class EmailChangeCodeFactory extends Factory +{ + protected $model = EmailChangeCode::class; + + public function definition() + { + $account = Account::factory()->create(); + $account->generateApiKey(); + + return [ + 'account_id' => $account->id, + 'code' => generatePin(), + 'email' => $this->faker->email, + ]; + } +} \ No newline at end of file diff --git a/flexiapi/resources/views/api/documentation_markdown.blade.php b/flexiapi/resources/views/api/documentation_markdown.blade.php index 393d2a1..7827bb4 100644 --- a/flexiapi/resources/views/api/documentation_markdown.blade.php +++ b/flexiapi/resources/views/api/documentation_markdown.blade.php @@ -381,18 +381,32 @@ Provision an account by generating a fresh `provisioning_token`. ### `POST /accounts/me/email/request` User -Change the account email. An email will be sent to the new email address to confirm the operation. +Request to change the account email. An email will be sent to the new email address to confirm the operation. + +Will return `403` if the account doesn't have a validated Account Creation Token attached to it. JSON parameters: * `email` the new email address, must be unique if `ACCOUNT_EMAIL_UNIQUE` is set to `true` +### `POST /accounts/me/email` +User + +Confirm the code received and change the email. +Activate the account. + +JSON parameters: + +* `code` the received SMS code + +Return the updated account. + ## Accounts phone number ### `POST /accounts/me/phone/request` User -Request a specific code by SMS. +Request a specific code by SMS to change the phone number. Will return `403` if the account doesn't have a validated Account Creation Token attached to it. diff --git a/flexiapi/routes/api.php b/flexiapi/routes/api.php index 8918f1b..fc158d3 100644 --- a/flexiapi/routes/api.php +++ b/flexiapi/routes/api.php @@ -74,6 +74,7 @@ Route::group(['middleware' => ['auth.jwt', 'auth.digest_or_key', 'auth.check_blo Route::delete('devices/{uuid}', 'Api\Account\DeviceController@destroy'); Route::post('email/request', 'Api\Account\EmailController@requestUpdate'); + Route::post('email', 'Api\Account\EmailController@update'); Route::post('password', 'Api\Account\PasswordController@update'); diff --git a/flexiapi/tests/Feature/ApiAccountEmailChangeTest.php b/flexiapi/tests/Feature/ApiAccountEmailChangeTest.php new file mode 100644 index 0000000..3d736aa --- /dev/null +++ b/flexiapi/tests/Feature/ApiAccountEmailChangeTest.php @@ -0,0 +1,128 @@ +. +*/ + +namespace Tests\Feature; + +use App\Account; +use App\EmailChangeCode; +use Tests\TestCase; + +class ApiAccountEmailChangeTest extends TestCase +{ + protected $route = '/api/accounts/me/email'; + protected $method = 'POST'; + + public function testRequest() + { + $account = Account::factory()->withConsumedAccountCreationToken()->create(); + $account->generateApiKey(); + $otherAccount = Account::factory()->withEmail()->create(); + $account->generateApiKey(); + $newEmail = 'test@test.com'; + + $this->keyAuthenticated($account) + ->json($this->method, $this->route.'/request', [ + 'email' => 'blabla' + ]) + ->assertStatus(422); + + $this->keyAuthenticated($account) + ->json($this->method, $this->route.'/request', [ + 'email' => $newEmail + ]) + ->assertStatus(200); + + // Same email + $this->keyAuthenticated($account) + ->json($this->method, $this->route.'/request', [ + 'email' => $account->email + ]) + ->assertStatus(422); + + $this->keyAuthenticated($account) + ->get('/api/accounts/me') + ->assertStatus(200) + ->assertJson([ + 'username' => $account->username, + 'email_change_code' => [ + 'email' => $newEmail + ] + ]); + + // Email already exists + config()->set('app.account_email_unique', true); + + $this->keyAuthenticated($account) + ->json($this->method, $this->route . '/request', [ + 'email' => $otherAccount->email + ])->assertJsonValidationErrors(['email']); + } + + public function testUnvalidatedAccount() + { + $account = Account::factory()->create(); + $account->generateApiKey(); + + $this->keyAuthenticated($account) + ->json($this->method, $this->route.'/request', [ + 'email' => 'test@test.com' + ]) + ->assertStatus(403); + } + + public function testConfirmWrongCode() + { + $emailChange = EmailChangeCode::factory()->create(); + + $this->keyAuthenticated($emailChange->account) + ->json($this->method, $this->route, [ + 'code' => 'wrong' + ]) + ->assertStatus(422); + } + + public function testConfirmGoodCode() + { + $emailChange = EmailChangeCode::factory()->create(); + $email = $emailChange->email; + + $this->keyAuthenticated($emailChange->account) + ->get('/api/accounts/me') + ->assertStatus(200) + ->assertJson([ + 'email' => null + ]); + + $this->keyAuthenticated($emailChange->account) + ->json($this->method, $this->route, [ + 'code' => $emailChange->code + ]) + ->assertStatus(200) + ->assertJson([ + 'email' => $email, + ]); + + $this->keyAuthenticated($emailChange->account) + ->get('/api/accounts/me') + ->assertStatus(200) + ->assertJson([ + 'email' => $email + ]); + } +} diff --git a/flexiapi/tests/Feature/ApiAccountPhoneChangeTest.php b/flexiapi/tests/Feature/ApiAccountPhoneChangeTest.php index e126c3f..4a6f811 100644 --- a/flexiapi/tests/Feature/ApiAccountPhoneChangeTest.php +++ b/flexiapi/tests/Feature/ApiAccountPhoneChangeTest.php @@ -59,7 +59,7 @@ class ApiAccountPhoneChangeTest extends TestCase ->assertStatus(403); } - public function testConfirmLongCode() + public function testConfirmWrongCode() { $phoneChange = PhoneChangeCode::factory()->create(); diff --git a/flexiapi/tests/Feature/ApiAccountTest.php b/flexiapi/tests/Feature/ApiAccountTest.php index 9394489..0db9efa 100644 --- a/flexiapi/tests/Feature/ApiAccountTest.php +++ b/flexiapi/tests/Feature/ApiAccountTest.php @@ -963,55 +963,6 @@ class ApiAccountTest extends TestCase ]); } - public function testChangeEmail() - { - $this->disableBlockingService(); - - $password = Password::factory()->create(); - $otherAccount = Password::factory()->create(); - $password->account->generateApiKey(); - $newEmail = 'new_email@test.com'; - - // Bad email - $this->keyAuthenticated($password->account) - ->json($this->method, $this->route . '/me/email/request', [ - 'email' => 'gnap' - ]) - ->assertStatus(422); - - // Same email - $this->keyAuthenticated($password->account) - ->json($this->method, $this->route . '/me/email/request', [ - 'email' => $password->account->email - ]) - ->assertStatus(422); - - // Correct email - $this->keyAuthenticated($password->account) - ->json($this->method, $this->route . '/me/email/request', [ - 'email' => $newEmail - ]) - ->assertStatus(200); - - $this->keyAuthenticated($password->account) - ->get($this->route . '/me') - ->assertStatus(200) - ->assertJson([ - 'username' => $password->account->username, - 'email_change_code' => [ - 'email' => $newEmail - ] - ]); - - // Email already exists - config()->set('app.account_email_unique', true); - - $this->keyAuthenticated($password->account) - ->json($this->method, $this->route . '/me/email/request', [ - 'email' => $otherAccount->account->email - ])->assertJsonValidationErrors(['email']); - } - public function testChangePassword() { $account = Account::factory()->create(); @@ -1085,35 +1036,35 @@ class ApiAccountTest extends TestCase public function testActivateDeactivate() { - $password = Password::factory()->create(); + $account = Account::factory()->withEmail()->create(); $admin = Account::factory()->admin()->create(); $admin->generateApiKey(); // deactivate $this->keyAuthenticated($admin) - ->post($this->route . '/' . $password->account->id . '/deactivate') + ->post($this->route . '/' . $account->id . '/deactivate') ->assertStatus(200) ->assertJson([ 'activated' => false ]); $this->keyAuthenticated($admin) - ->get($this->route . '/' . $password->account->id) + ->get($this->route . '/' . $account->id) ->assertStatus(200) ->assertJson([ 'activated' => false ]); $this->keyAuthenticated($admin) - ->post($this->route . '/' . $password->account->id . '/activate') + ->post($this->route . '/' . $account->id . '/activate') ->assertStatus(200) ->assertJson([ 'activated' => true ]); $this->keyAuthenticated($admin) - ->get($this->route . '/' . $password->account->id) + ->get($this->route . '/' . $account->id) ->assertStatus(200) ->assertJson([ 'activated' => true @@ -1121,18 +1072,18 @@ class ApiAccountTest extends TestCase // Search feature $this->keyAuthenticated($admin) - ->get($this->route . '/' . $password->account->identifier . '/search') + ->get($this->route . '/' . $account->identifier . '/search') ->assertStatus(200) ->assertJson([ - 'id' => $password->account->id, + 'id' => $account->id, 'activated' => true ]); $this->keyAuthenticated($admin) - ->get($this->route . '/' . $password->account->email . '/search-by-email') + ->get($this->route . '/' . $account->email . '/search-by-email') ->assertStatus(200) ->assertJson([ - 'id' => $password->account->id, + 'id' => $account->id, 'activated' => true ]);