From 49d324ac33f1ec6cc1536000bc5f2f1819699028 Mon Sep 17 00:00:00 2001 From: Mohammad Alavi Date: Sat, 16 Apr 2022 13:09:50 +0430 Subject: [PATCH] fix(email-verification): make email verification actually work! hopefully! --- .../ApiLoginProxyForWebClientAction.php | 3 +- .../Actions/RegisterUserAction.php | 2 +- .../Actions/SendVerificationEmailAction.php | 2 +- .../Configs/appSection-authentication.php | 1 + .../Notifications/VerifyEmail.php | 17 +++- .../Tasks/SendVerificationEmailTask.php | 8 +- .../Tests/Unit/RegisterUserActionTest.php | 33 ++++++- .../Unit/SendVerificationEmailActionTest.php | 33 ------- .../Unit/SendVerificationEmailTaskTest.php | 40 ++++++++ .../Controllers/RegisterUserController.php | 6 +- .../SendVerificationEmail.v1.public.php | 12 ++- .../UI/API/Routes/VerifyEmail.v1.public.php | 11 ++- .../API/Tests/Functional/RegisterUserTest.php | 61 +++++++----- .../Functional/SendVerificationEmailTest.php | 95 +++++++++++++++++++ .../API/Tests/Functional/VerifyEmailTest.php | 47 ++++++++- .../AppSection/User/Models/User.php | 16 +++- app/Ship/Contracts/MustVerifyEmail.php | 13 +++ 17 files changed, 314 insertions(+), 86 deletions(-) delete mode 100644 app/Containers/AppSection/Authentication/Tests/Unit/SendVerificationEmailActionTest.php create mode 100644 app/Containers/AppSection/Authentication/Tests/Unit/SendVerificationEmailTaskTest.php create mode 100644 app/Containers/AppSection/Authentication/UI/API/Tests/Functional/SendVerificationEmailTest.php create mode 100644 app/Ship/Contracts/MustVerifyEmail.php diff --git a/app/Containers/AppSection/Authentication/Actions/ApiLoginProxyForWebClientAction.php b/app/Containers/AppSection/Authentication/Actions/ApiLoginProxyForWebClientAction.php index d0e651dcc..d132cbcaf 100644 --- a/app/Containers/AppSection/Authentication/Actions/ApiLoginProxyForWebClientAction.php +++ b/app/Containers/AppSection/Authentication/Actions/ApiLoginProxyForWebClientAction.php @@ -14,7 +14,7 @@ class ApiLoginProxyForWebClientAction extends Action { use LoginAttributeCaseSensitivityTrait; - + /** * @param LoginProxyPasswordGrantRequest $request * @return array @@ -49,6 +49,7 @@ private function enrichSanitizedData(string $username, array $sanitizedData): ar $sanitizedData['client_secret'] = config('appSection-authentication.clients.web.secret'); $sanitizedData['grant_type'] = 'password'; $sanitizedData['scope'] = ''; + return $sanitizedData; } } diff --git a/app/Containers/AppSection/Authentication/Actions/RegisterUserAction.php b/app/Containers/AppSection/Authentication/Actions/RegisterUserAction.php index 9685d6037..d17300615 100644 --- a/app/Containers/AppSection/Authentication/Actions/RegisterUserAction.php +++ b/app/Containers/AppSection/Authentication/Actions/RegisterUserAction.php @@ -32,7 +32,7 @@ public function run(RegisterUserRequest $request): User $user = app(CreateUserByCredentialsTask::class)->run($sanitizedData); $user->notify(new Welcome()); - app(SendVerificationEmailTask::class)->run($user); + app(SendVerificationEmailTask::class)->run($user, $request->verification_url); return $user; } diff --git a/app/Containers/AppSection/Authentication/Actions/SendVerificationEmailAction.php b/app/Containers/AppSection/Authentication/Actions/SendVerificationEmailAction.php index ce3d9d7f3..e216a0bd0 100644 --- a/app/Containers/AppSection/Authentication/Actions/SendVerificationEmailAction.php +++ b/app/Containers/AppSection/Authentication/Actions/SendVerificationEmailAction.php @@ -10,6 +10,6 @@ class SendVerificationEmailAction extends Action { public function run(SendVerificationEmailRequest $request): void { - app(SendVerificationEmailTask::class)->run($request->user()); + app(SendVerificationEmailTask::class)->run($request->user(), $request->verification_url); } } diff --git a/app/Containers/AppSection/Authentication/Configs/appSection-authentication.php b/app/Containers/AppSection/Authentication/Configs/appSection-authentication.php index e074d81b9..161fe2ba3 100644 --- a/app/Containers/AppSection/Authentication/Configs/appSection-authentication.php +++ b/app/Containers/AppSection/Authentication/Configs/appSection-authentication.php @@ -12,6 +12,7 @@ */ 'require_email_verification' => true, + 'verification_link_expiration_time' => 30, // in minute /* |-------------------------------------------------------------------------- diff --git a/app/Containers/AppSection/Authentication/Notifications/VerifyEmail.php b/app/Containers/AppSection/Authentication/Notifications/VerifyEmail.php index af48f1266..b33967f57 100644 --- a/app/Containers/AppSection/Authentication/Notifications/VerifyEmail.php +++ b/app/Containers/AppSection/Authentication/Notifications/VerifyEmail.php @@ -7,11 +7,17 @@ use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Notifications\Messages\MailMessage; +use Illuminate\Support\Facades\URL; class VerifyEmail extends Notification implements ShouldQueue { use Queueable; + public function __construct( + private string $verification_url, + ) { + } + public function via($notifiable): array { return ['mail']; @@ -21,7 +27,7 @@ public function toMail(UserModel $notifiable): MailMessage { return (new MailMessage()) ->subject('Verify Email Address') - ->line('Please click the button below to verify your email address.') + ->line('Please click the below button to verify your email address.') ->action('Verify Email Address', $this->createUrl($notifiable)) ->line('If you did not create an account, no further action is required.'); } @@ -31,6 +37,13 @@ private function createUrl(UserModel $notifiable): string $id = config('apiato.hash-id') ? $notifiable->getHashedKey() : $notifiable->getKey(); $hash = sha1($notifiable->getEmailForVerification()); - return request('verification_url') . "/$id/$hash"; + return $this->verification_url . '?url=' . URL::temporarySignedRoute( + 'verification.verify', + now()->addMinutes(config('appSection-authentication.verification_link_expiration_time')), + [ + 'id' => $id, + 'hash' => $hash, + ] + ); } } diff --git a/app/Containers/AppSection/Authentication/Tasks/SendVerificationEmailTask.php b/app/Containers/AppSection/Authentication/Tasks/SendVerificationEmailTask.php index 487110075..14df203c8 100644 --- a/app/Containers/AppSection/Authentication/Tasks/SendVerificationEmailTask.php +++ b/app/Containers/AppSection/Authentication/Tasks/SendVerificationEmailTask.php @@ -2,15 +2,15 @@ namespace App\Containers\AppSection\Authentication\Tasks; -use App\Ship\Parents\Models\UserModel; +use App\Ship\Contracts\MustVerifyEmail; use App\Ship\Parents\Tasks\Task; class SendVerificationEmailTask extends Task { - public function run(UserModel $user): void + public function run(MustVerifyEmail $user, ?string $verificationUrl = null): void { - if (config('appSection-authentication.require_email_verification') && !$user->hasVerifiedEmail()) { - $user->sendEmailVerificationNotification(); + if (config('appSection-authentication.require_email_verification') && !$user->hasVerifiedEmail() && !is_null($verificationUrl)) { + $user->sendEmailVerificationNotificationWithVerificationUrl($verificationUrl); } } } diff --git a/app/Containers/AppSection/Authentication/Tests/Unit/RegisterUserActionTest.php b/app/Containers/AppSection/Authentication/Tests/Unit/RegisterUserActionTest.php index 69f976352..6ed6ced75 100644 --- a/app/Containers/AppSection/Authentication/Tests/Unit/RegisterUserActionTest.php +++ b/app/Containers/AppSection/Authentication/Tests/Unit/RegisterUserActionTest.php @@ -17,23 +17,48 @@ */ class RegisterUserActionTest extends TestCase { - public function testSendNotification_AfterUserRegistration(): void + public function testAfterUserRegistration_GivenEmailVerificationEnabled_SendNotification(): void { + if (!config('appSection-authentication.require_email_verification')) { + $this->markTestSkipped(); + } Notification::fake(); - + config(['appSection-authentication.require_email_verification', false]); $data = [ 'email' => 'Mahmoud@test.test', 'password' => 'so-secret', + 'verification_url' => config('appSection-authentication.allowed-verify-email-urls')[0], ]; $request = new RegisterUserRequest($data); + request()->merge($request->all()); $user = app(RegisterUserAction::class)->run($request); - $this->assertEquals($data['email'], $user->email); + $this->assertModelExists($user); + $this->assertEquals(strtolower($data['email']), $user->email); Notification::assertSentTo($user, Welcome::class); + Notification::assertSentTo($user, VerifyEmail::class); + } + public function testAfterUserRegistration_GivenEmailVerificationDisabled_ShouldNotSendVerifyEmailNotification(): void + { if (config('appSection-authentication.require_email_verification')) { - Notification::assertSentTo($user, VerifyEmail::class); + $this->markTestSkipped(); } + Notification::fake(); + $data = [ + 'email' => 'Mahmoud@test.test', + 'password' => 'so-secret', + 'verification_url' => config('appSection-authentication.allowed-verify-email-urls')[0], + ]; + + $request = new RegisterUserRequest($data); + request()->merge($request->all()); + $user = app(RegisterUserAction::class)->run($request); + + $this->assertModelExists($user); + $this->assertEquals(strtolower($data['email']), $user->email); + Notification::assertSentTo($user, Welcome::class); + Notification::assertNotSentTo($user, VerifyEmail::class); } } diff --git a/app/Containers/AppSection/Authentication/Tests/Unit/SendVerificationEmailActionTest.php b/app/Containers/AppSection/Authentication/Tests/Unit/SendVerificationEmailActionTest.php deleted file mode 100644 index 456c914ba..000000000 --- a/app/Containers/AppSection/Authentication/Tests/Unit/SendVerificationEmailActionTest.php +++ /dev/null @@ -1,33 +0,0 @@ -unverified()->create(); - // enable email verification - config(['appSection-authentication.require_email_verification' => true]); - $request = SendVerificationEmailRequest::injectData([], $unverifiedUser); - - - app(SendVerificationEmailAction::class)->run($request); - - Notification::assertSentTo($unverifiedUser, VerifyEmail::class); - } -} diff --git a/app/Containers/AppSection/Authentication/Tests/Unit/SendVerificationEmailTaskTest.php b/app/Containers/AppSection/Authentication/Tests/Unit/SendVerificationEmailTaskTest.php new file mode 100644 index 000000000..c4ad9d504 --- /dev/null +++ b/app/Containers/AppSection/Authentication/Tests/Unit/SendVerificationEmailTaskTest.php @@ -0,0 +1,40 @@ +unverified()->create(); + config(['appSection-authentication.require_email_verification' => true]); + + app(SendVerificationEmailTask::class)->run($unverifiedUser, 'this_doesnt_matter_for_the_test'); + + Notification::assertSentTo($unverifiedUser, VerifyEmail::class); + } + + public function testGivenEmailVerificationDisabled_ShouldNotSendVerificationEmail(): void + { + Notification::fake(); + $unverifiedUser = User::factory()->unverified()->create(); + config(['appSection-authentication.require_email_verification' => false]); + + app(SendVerificationEmailTask::class)->run($unverifiedUser); + + Notification::assertNotSentTo($unverifiedUser, VerifyEmail::class); + } +} diff --git a/app/Containers/AppSection/Authentication/UI/API/Controllers/RegisterUserController.php b/app/Containers/AppSection/Authentication/UI/API/Controllers/RegisterUserController.php index 88de4a905..781ee5010 100644 --- a/app/Containers/AppSection/Authentication/UI/API/Controllers/RegisterUserController.php +++ b/app/Containers/AppSection/Authentication/UI/API/Controllers/RegisterUserController.php @@ -2,12 +2,10 @@ namespace App\Containers\AppSection\Authentication\UI\API\Controllers; -use Apiato\Core\Exceptions\IncorrectIdException; use Apiato\Core\Exceptions\InvalidTransformerException; use App\Containers\AppSection\Authentication\Actions\RegisterUserAction; use App\Containers\AppSection\Authentication\UI\API\Requests\RegisterUserRequest; use App\Containers\AppSection\User\UI\API\Transformers\UserTransformer; -use App\Ship\Exceptions\CreateResourceFailedException; use App\Ship\Parents\Controllers\ApiController; class RegisterUserController extends ApiController @@ -15,13 +13,11 @@ class RegisterUserController extends ApiController /** * @param RegisterUserRequest $request * @return array - * @throws CreateResourceFailedException * @throws InvalidTransformerException - * @throws IncorrectIdException */ public function registerUser(RegisterUserRequest $request): array { - $user = app(RegisterUserAction::class)->run($request); + $user = app(RegisterUserAction::class)->transactionalRun($request); return $this->transform($user, UserTransformer::class); } diff --git a/app/Containers/AppSection/Authentication/UI/API/Routes/SendVerificationEmail.v1.public.php b/app/Containers/AppSection/Authentication/UI/API/Routes/SendVerificationEmail.v1.public.php index a714b270e..975512b94 100644 --- a/app/Containers/AppSection/Authentication/UI/API/Routes/SendVerificationEmail.v1.public.php +++ b/app/Containers/AppSection/Authentication/UI/API/Routes/SendVerificationEmail.v1.public.php @@ -10,6 +10,11 @@ * @apiVersion 1.0.0 * @apiPermission Authenticated ['permissions' => '', 'roles' => ''] * + * @apiHeader {String} accept=application/json + * @apiHeader {String} authorization=Bearer + * + * @apiPermission Authenticated ['permissions' => '', 'roles' => ''] + * * @apiParam {String} verification_url required|url * * @apiSuccessExample {json} Success-Response: @@ -20,7 +25,8 @@ use App\Containers\AppSection\Authentication\UI\API\Controllers\SendVerificationEmailController; use Illuminate\Support\Facades\Route; -Route::post('/email/verification-notification', [SendVerificationEmailController::class, 'sendVerificationEmail']) - ->name('verification.verify') - ->middleware(['auth:api']); +if (config('appSection-authentication.require_email_verification')) { + Route::post('/email/verification-notification', [SendVerificationEmailController::class, 'sendVerificationEmail']) + ->middleware(['auth:api']); +} diff --git a/app/Containers/AppSection/Authentication/UI/API/Routes/VerifyEmail.v1.public.php b/app/Containers/AppSection/Authentication/UI/API/Routes/VerifyEmail.v1.public.php index 980d20892..dd2fdb177 100644 --- a/app/Containers/AppSection/Authentication/UI/API/Routes/VerifyEmail.v1.public.php +++ b/app/Containers/AppSection/Authentication/UI/API/Routes/VerifyEmail.v1.public.php @@ -7,9 +7,15 @@ * @api {POST} /v1/email/verify/:id/:hash Verify Email * @apiDescription Verify user email * + * Value of `url` query string in the verification link (sent to the user by email) should be directly used to call the api and verify the user + * + * example of a verification email link sent to the user which is used to verify the use `http://apiato.test/email/verify?url=http://api.apiato.test/v1/email/verify/XbPW7awNkzl83LD6/eaabd911e2e07ede6456d3bd5725c6d4a5c2dc0b?expires=1646913047&signature=232702865b8353c445b39c50397e66db33c74df80e3db5a7c0d46ef94c8ab6a9` + * * @apiVersion 1.0.0 * @apiPermission none * + * @apiHeader {String} accept=application/json + * * @apiSuccessExample {json} Success-Response: * HTTP/1.1 200 OK * {} @@ -18,5 +24,6 @@ use App\Containers\AppSection\Authentication\UI\API\Controllers\VerifyEmailController; use Illuminate\Support\Facades\Route; -Route::post('email/verify/{id}/{hash}', [VerifyEmailController::class, 'verifyEmail']); - +Route::post('email/verify/{id}/{hash}', [VerifyEmailController::class, 'verifyEmail']) + ->name('verification.verify') + ->middleware('signed'); diff --git a/app/Containers/AppSection/Authentication/UI/API/Tests/Functional/RegisterUserTest.php b/app/Containers/AppSection/Authentication/UI/API/Tests/Functional/RegisterUserTest.php index 17033a492..acdf894ec 100644 --- a/app/Containers/AppSection/Authentication/UI/API/Tests/Functional/RegisterUserTest.php +++ b/app/Containers/AppSection/Authentication/UI/API/Tests/Functional/RegisterUserTest.php @@ -2,7 +2,11 @@ namespace App\Containers\AppSection\Authentication\UI\API\Tests\Functional; +use App\Containers\AppSection\Authentication\Notifications\VerifyEmail; +use App\Containers\AppSection\Authentication\Notifications\Welcome; use App\Containers\AppSection\Authentication\UI\API\Tests\ApiTestCase; +use App\Containers\AppSection\User\Models\User; +use Illuminate\Support\Facades\Notification; use Illuminate\Testing\Fluent\AssertableJson; /** @@ -37,7 +41,7 @@ public function testGivenEmailVerificationEnabled_RegisterNewUserWithCredentials $response->assertStatus(200); $response->assertJson( - fn(AssertableJson $json) => $json->has('data') + fn (AssertableJson $json) => $json->has('data') ->where('data.email', $data['email']) ->etc() ); @@ -55,7 +59,7 @@ public function testGivenEmailVerificationDisabled_RegisterNewUserWithCredential $response->assertStatus(200); $response->assertJson( - fn(AssertableJson $json) => $json->has('data') + fn (AssertableJson $json) => $json->has('data') ->where('data.email', $data['email']) ->etc() ); @@ -63,16 +67,11 @@ public function testGivenEmailVerificationDisabled_RegisterNewUserWithCredential public function testRegisterNewUserUsingGetVerb(): void { - $data = [ - 'email' => 'apiato@mail.test', - 'password' => 'secret', - ]; - - $response = $this->endpoint('get@v1/register')->makeCall($data); + $response = $this->endpoint('get@v1/register')->makeCall(); $response->assertStatus(405); $response->assertJson( - fn(AssertableJson $json) => $json->has('message') + fn (AssertableJson $json) => $json->has('message') ->where('message', 'The GET method is not supported for this route. Supported methods: POST.') ->etc() ); @@ -96,7 +95,7 @@ public function testRegisterExistingUser(): void $response->assertStatus(422); $response->assertJson( - fn(AssertableJson $json) => $json->has('errors') + fn (AssertableJson $json) => $json->has('errors') ->where('errors.email.0', 'The email has already been taken.') ->etc() ); @@ -112,20 +111,20 @@ public function testRegisterNewUserWithoutData(): void if (config('appSection-authentication.require_email_verification')) { $response->assertJson( - fn(AssertableJson $json) => $json->hasAll(['message', 'errors' => 3]) + fn (AssertableJson $json) => $json->hasAll(['message', 'errors' => 3]) ->has( 'errors', - fn(AssertableJson $json) => $json->where('email.0', 'The email field is required.') + fn (AssertableJson $json) => $json->where('email.0', 'The email field is required.') ->where('password.0', 'The password field is required.') ->where('verification_url.0', 'The verification url field is required.') ) ); } else { $response->assertJson( - fn(AssertableJson $json) => $json->hasAll(['message', 'errors' => 2]) + fn (AssertableJson $json) => $json->hasAll(['message', 'errors' => 2]) ->has( 'errors', - fn(AssertableJson $json) => $json->where('email.0', 'The email field is required.') + fn (AssertableJson $json) => $json->where('email.0', 'The email field is required.') ->where('password.0', 'The password field is required.') ) ); @@ -142,7 +141,7 @@ public function testRegisterNewUserWithInvalidEmail(): void $response->assertStatus(422); $response->assertJson( - fn(AssertableJson $json) => $json->has('errors') + fn (AssertableJson $json) => $json->has('errors') ->where('errors.email.0', 'The email must be a valid email address.') ->etc() ); @@ -158,10 +157,10 @@ public function testRegisterNewUserWithInvalidPassword(): void $response->assertStatus(422); $response->assertJson( - fn(AssertableJson $json) => $json->has('errors') + fn (AssertableJson $json) => $json->has('errors') ->has( 'errors.password', - fn(AssertableJson $json) => $json + fn (AssertableJson $json) => $json ->where('0', 'The password must contain at least one uppercase and one lowercase letter.') ->where('1', 'The password must contain at least one letter.') ->where('2', 'The password must contain at least one number.') @@ -172,24 +171,38 @@ public function testRegisterNewUserWithInvalidPassword(): void public function testRegisterNewUserWithNotAllowedVerificationUrl(): void { - if (!config('appSection-authentication.require_email_verification')) { - $this->markTestSkipped(); - } - - config(['appSection-authentication.require_email_verification' => []]); + config(['appSection-authentication.require_email_verification' => true]); $data = [ 'email' => 'test@test.test', 'password' => 's3cr3tPa$$', - 'verification_url' => 'http://notallowed.test/wrong', + 'verification_url' => 'http://notallowed.test/wrong/hopyfuly/noone/make/a/route/like/this', ]; $response = $this->makeCall($data); $response->assertStatus(422); $response->assertJson( - fn(AssertableJson $json) => $json->hasAll(['message', 'errors' => 1]) + fn (AssertableJson $json) => $json->hasAll(['message', 'errors' => 1]) ->where('errors.verification_url.0', 'The selected verification url is invalid.') ); } + + public function testGivenEmailVerificationDisabled_ShouldNotSendVerificationEmail(): void + { + config(['appSection-authentication.require_email_verification' => false]); + + Notification::fake(); + $data = [ + 'email' => 'test@test.test', + 'password' => 's3cr3tPa$$', + 'verification_url' => config('appSection-authentication.allowed-verify-email-urls')[0], + ]; + + $response = $this->makeCall($data); + $registeredUser = User::find($this->decode($response->json()['data']['id'])); + $response->assertStatus(200); + Notification::assertSentTo($registeredUser, Welcome::class); + Notification::assertNotSentTo($registeredUser, VerifyEmail::class); + } } diff --git a/app/Containers/AppSection/Authentication/UI/API/Tests/Functional/SendVerificationEmailTest.php b/app/Containers/AppSection/Authentication/UI/API/Tests/Functional/SendVerificationEmailTest.php new file mode 100644 index 000000000..b6fe37138 --- /dev/null +++ b/app/Containers/AppSection/Authentication/UI/API/Tests/Functional/SendVerificationEmailTest.php @@ -0,0 +1,95 @@ + '', + 'roles' => '', + ]; + + public function testGivenEmailVerificationEnabled_SendVerificationEmail(): void + { + if (!config('appSection-authentication.require_email_verification')) { + $this->markTestSkipped(); + } + Notification::fake(); + $this->testingUser = User::factory()->unverified()->create(); + + $data = [ + 'verification_url' => config('appSection-authentication.allowed-verify-email-urls')[0], + ]; + + $response = $this->makeCall($data); + + $response->assertStatus(202); + Notification::assertSentTo($this->testingUser, VerifyEmail::class); + } + + public function testSendingWithoutRequiredData_ShouldThrowError(): void + { + if (!config('appSection-authentication.require_email_verification')) { + $this->markTestSkipped(); + } + $data = []; + + $response = $this->makeCall($data); + + $response->assertStatus(422); + + $response->assertJson( + fn (AssertableJson $json) => $json->hasAll(['message', 'errors' => 1]) + ->has( + 'errors', + fn (AssertableJson $json) => $json->where('verification_url.0', 'The verification url field is required.') + ) + ); + } + + public function testRegisterNewUserWithNotAllowedVerificationUrl(): void + { + if (!config('appSection-authentication.require_email_verification')) { + $this->markTestSkipped(); + } + $data = [ + 'email' => 'test@test.test', + 'password' => 's3cr3tPa$$', + 'name' => 'Bruce Lee', + 'verification_url' => 'http://notallowed.test/wrong', + ]; + + $response = $this->makeCall($data); + + $response->assertStatus(422); + $response->assertJson( + fn (AssertableJson $json) => $json->hasAll(['message', 'errors' => 1]) + ->where('errors.verification_url.0', 'The selected verification url is invalid.') + ); + } + + public function testGivenEmailVerificationIsDisabled_ShouldThrow404(): void + { + if (config('appSection-authentication.require_email_verification')) { + $this->markTestSkipped(); + } + $response = $this->makeCall([]); + + $response->assertStatus(404); + + } +} diff --git a/app/Containers/AppSection/Authentication/UI/API/Tests/Functional/VerifyEmailTest.php b/app/Containers/AppSection/Authentication/UI/API/Tests/Functional/VerifyEmailTest.php index 6a01c6df1..ed7a6cb2e 100644 --- a/app/Containers/AppSection/Authentication/UI/API/Tests/Functional/VerifyEmailTest.php +++ b/app/Containers/AppSection/Authentication/UI/API/Tests/Functional/VerifyEmailTest.php @@ -6,6 +6,7 @@ use App\Containers\AppSection\Authentication\UI\API\Tests\ApiTestCase; use App\Containers\AppSection\User\Models\User; use Illuminate\Support\Facades\Notification; +use Illuminate\Support\Facades\URL; /** * Class VerifyEmailTest. @@ -18,19 +19,33 @@ class VerifyEmailTest extends ApiTestCase protected string $endpoint = 'post@v1/email/verify/{id}/{hash}'; protected array $access = [ - 'permissions' => '', 'roles' => '', + 'permissions' => '', ]; public function testVerifyEmail(): void { Notification::fake(); $unverifiedUser = User::factory()->unverified()->create(); + $hashedEmail = sha1($unverifiedUser->getEmailForVerification()); // enable email verification config(['appSection-authentication.require_email_verification' => true]); + $url = URL::temporarySignedRoute( + 'verification.verify', + now()->addMinutes(30), + [ + 'id' => $unverifiedUser->getHashedKey(), + 'hash' => $hashedEmail, + ] + ); + + $match = []; + $expires = $match[preg_match('/expires=(.*?)&/', $url, $match)]; + $signature = $match[preg_match('/signature=(.*)/', $url, $match)]; $response = $this->injectId($unverifiedUser->id) - ->injectId(sha1($unverifiedUser->getEmailForVerification()), skipEncoding: true, replace: '{hash}') + ->injectId($hashedEmail, skipEncoding: true, replace: '{hash}') + ->endpoint($this->endpoint . "?expires=$expires&signature=$signature") ->makeCall(); $response->assertStatus(200); @@ -38,4 +53,32 @@ public function testVerifyEmail(): void $this->assertTrue($unverifiedUser->hasVerifiedEmail()); Notification::assertSentTo($unverifiedUser, EmailVerified::class); } + + public function testVerifyEmailShouldNotBeAcceptedIfRoutesSignatureIsNotVerified(): void + { + Notification::fake(); + $unverifiedUser = User::factory()->unverified()->create(); + $hashedEmail = sha1($unverifiedUser->getEmailForVerification()); + // enable email verification + config(['appSection-authentication.require_email_verification' => true]); + $url = URL::temporarySignedRoute( + 'verification.verify', + now()->addMinutes(30), + [ + 'id' => $unverifiedUser->getHashedKey(), + 'hash' => $hashedEmail, + ] + ); + + $match = []; + $expires = $match[preg_match('/expires=(.*?)&/', $url, $match)]; + $signature = 'invalid_sig'; + + $response = $this->injectId($unverifiedUser->id) + ->injectId($hashedEmail, skipEncoding: true, replace: '{hash}') + ->endpoint($this->endpoint . "?expires=$expires&signature=$signature") + ->makeCall(); + + $response->assertStatus(403); + } } diff --git a/app/Containers/AppSection/User/Models/User.php b/app/Containers/AppSection/User/Models/User.php index b3961b421..8c8fc1af9 100644 --- a/app/Containers/AppSection/User/Models/User.php +++ b/app/Containers/AppSection/User/Models/User.php @@ -2,11 +2,12 @@ namespace App\Containers\AppSection\User\Models; +use App\Containers\AppSection\Authentication\Notifications\VerifyEmail; use App\Containers\AppSection\Authentication\Traits\AuthenticationTrait; use App\Containers\AppSection\Authorization\Traits\AuthorizationTrait; +use App\Ship\Contracts\MustVerifyEmail; use App\Ship\Parents\Models\UserModel; -use App\Containers\AppSection\Authentication\Notifications\VerifyEmail; -use Illuminate\Contracts\Auth\MustVerifyEmail; +use Illuminate\Database\Eloquent\Casts\Attribute; use Illuminate\Notifications\Notifiable; class User extends UserModel implements MustVerifyEmail @@ -33,8 +34,15 @@ class User extends UserModel implements MustVerifyEmail 'birth' => 'date', ]; - public function sendEmailVerificationNotification() + public function sendEmailVerificationNotificationWithVerificationUrl(string $verificationUrl) + { + $this->notify(new VerifyEmail($verificationUrl)); + } + + protected function email(): Attribute { - $this->notify(new VerifyEmail()); + return new Attribute( + get: fn ($value): string => strtolower($value), + ); } } diff --git a/app/Ship/Contracts/MustVerifyEmail.php b/app/Ship/Contracts/MustVerifyEmail.php new file mode 100644 index 000000000..5d0c1b7b5 --- /dev/null +++ b/app/Ship/Contracts/MustVerifyEmail.php @@ -0,0 +1,13 @@ +