Verify email addresses before they reach the suite

app/Actions/Fortify/CreateNewUser.php

@@ -34,14 +34,13 @@ public function create(array $input): User
             'password' => $this->passwordRules(),
         ])->validate();
 
-        $user = User::create([
+        // Left unverified on purpose. Registration fires Registered, which
+        // sends the link that stamps email_verified_at, and every app in the
+        // suite trusts the address this returns.
+        return User::create([
             'name' => $input['name'],
             'email' => $input['email'],
             'password' => Hash::make($input['password']),
         ]);
-
-        $user->forceFill(['email_verified_at' => now()])->save();
-
-        return $user;
     }
 }

app/Http/Middleware/EnsureEmailIsVerifiedForOAuth.php

@@ -0,0 +1,42 @@
+<?php
+
+namespace App\Http\Middleware;
+
+use Closure;
+use Illuminate\Contracts\Auth\MustVerifyEmail;
+use Illuminate\Http\Request;
+use Symfony\Component\HttpFoundation\Response;
+
+/**
+ * Keep an unverified address from being handed to a suite app.
+ *
+ * Registration signs the new user in before they have opened the link, so
+ * without this they could walk straight into the authorization screen and
+ * leave with tokens for an address they have never proved they hold. The
+ * apps downstream match invitations and pre-existing accounts on that
+ * address, so the claim has to be true before it travels.
+ *
+ * This sits on the whole Passport route group, which also carries the back
+ * channel: /oauth/token and /oauth/userinfo are called by the client, not a
+ * browser, and have no session to read. They resolve no user here and pass
+ * straight through, which is why this tests for a user rather than demanding
+ * one the way the framework's `verified` middleware does.
+ */
+class EnsureEmailIsVerifiedForOAuth
+{
+    /**
+     * Handle an incoming request.
+     *
+     * @param  Closure(Request): Response  $next
+     */
+    public function handle(Request $request, Closure $next): Response
+    {
+        $user = $request->user();
+
+        if ($user instanceof MustVerifyEmail && ! $user->hasVerifiedEmail()) {
+            return redirect()->route('verification.notice');
+        }
+
+        return $next($request);
+    }
+}

app/Models/User.php

@@ -3,6 +3,7 @@
 namespace App\Models;
 
 use Database\Factories\UserFactory;
+use Illuminate\Contracts\Auth\MustVerifyEmail;
 use Illuminate\Database\Eloquent\Attributes\Fillable;
 use Illuminate\Database\Eloquent\Attributes\Hidden;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
@@ -14,9 +15,15 @@
 use Laravel\Passport\Contracts\OAuthenticatable;
 use Laravel\Passport\HasApiTokens;
 
+/**
+ * The MustVerifyEmail contract is what makes the verification real. Illuminate's
+ * base user has always carried the trait behind it, so hasVerifiedEmail() already
+ * answered from the column -- but nothing ever asked, because the `verified`
+ * middleware and Fortify's flow both test for this interface first.
+ */
 #[Fillable(['name', 'email', 'password'])]
 #[Hidden(['password', 'remember_token', 'two_factor_secret', 'two_factor_recovery_codes'])]
-class User extends Authenticatable implements OAuthenticatable, PasskeyUser
+class User extends Authenticatable implements MustVerifyEmail, OAuthenticatable, PasskeyUser
 {
     /** @use HasFactory<UserFactory> */
     use HasApiTokens, HasFactory, Notifiable, PasskeyAuthenticatable, TwoFactorAuthenticatable;

bootstrap/app.php

@@ -1,9 +1,11 @@
 <?php
 
+use App\Http\Middleware\EnsureEmailIsVerifiedForOAuth;
 use Illuminate\Foundation\Application;
 use Illuminate\Foundation\Configuration\Exceptions;
 use Illuminate\Foundation\Configuration\Middleware;
 use Illuminate\Http\Request;
+use Illuminate\Session\Middleware\StartSession;
 
 return Application::configure(basePath: dirname(__DIR__))
     ->withRouting(
@@ -13,7 +15,14 @@
         health: '/up',
     )
     ->withMiddleware(function (Middleware $middleware): void {
-        //
+        // Passport applies config('passport.middleware') as group middleware,
+        // which lands it ahead of the `web` group and so ahead of the session
+        // the check needs. Priority is what decides the order once the groups
+        // are expanded, so name it here and it runs once a user can be read.
+        $middleware->appendToPriorityList(
+            StartSession::class,
+            EnsureEmailIsVerifiedForOAuth::class,
+        );
     })
     ->withExceptions(function (Exceptions $exceptions): void {
         $exceptions->shouldRenderJsonWhen(

config/fortify.php

@@ -164,7 +164,7 @@
     'features' => [
         Features::registration(),
         Features::resetPasswords(),
-        // Features::emailVerification(),
+        Features::emailVerification(),
         Features::updateProfileInformation(),
         Features::updatePasswords(),
         Features::twoFactorAuthentication([

config/passport.php

@@ -1,5 +1,7 @@
 <?php
 
+use App\Http\Middleware\EnsureEmailIsVerifiedForOAuth;
+
 return [
 
     /*
@@ -15,7 +17,20 @@
 
     'guard' => 'web',
 
-    'middleware' => [],
+    /*
+    |--------------------------------------------------------------------------
+    | Passport Route Middleware
+    |--------------------------------------------------------------------------
+    |
+    | Applied to every route Passport registers. The one entry here stops an
+    | unverified address reaching the authorization screen; it is a no-op on
+    | the back-channel routes, which carry no session user.
+    |
+    */
+
+    'middleware' => [
+        EnsureEmailIsVerifiedForOAuth::class,
+    ],
 
     /*
     |--------------------------------------------------------------------------

routes/web.php

@@ -6,7 +6,9 @@
 
 Route::get('/', fn () => auth()->check() ? redirect()->route('home') : redirect()->route('login'));
 
-Route::middleware('auth')->group(function () {
+// `verified` as well as `auth`: an address nobody has proved they hold should
+// not be picking an app to sign in to, nor switching which account does.
+Route::middleware(['auth', 'verified'])->group(function () {
     Route::get('/home', HomeController::class)->name('home');
 
     Route::post('/oauth/switch-account', SwitchOAuthAccountController::class)

tests/Feature/Auth/EmailVerificationTest.php

@@ -0,0 +1,208 @@
+<?php
+
+use App\Http\Middleware\EnsureEmailIsVerifiedForOAuth;
+use App\Models\User;
+use Illuminate\Auth\Events\Registered;
+use Illuminate\Auth\Notifications\VerifyEmail;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Illuminate\Session\Middleware\StartSession;
+use Illuminate\Support\Facades\Event;
+use Illuminate\Support\Facades\Notification;
+use Illuminate\Support\Facades\URL;
+use Laravel\Fortify\Features;
+use Laravel\Passport\Client;
+use Laravel\Passport\ClientRepository;
+
+uses(RefreshDatabase::class);
+
+test('email verification is one of the enabled features', function () {
+    expect(Features::enabled(Features::emailVerification()))->toBeTrue();
+});
+
+test('registering leaves the address unverified and sends the link', function () {
+    Notification::fake();
+
+    $this->post('/register', [
+        'name' => 'Ada Lovelace',
+        'email' => 'ada@3ag.local',
+        'password' => 'a-long-enough-password',
+        'password_confirmation' => 'a-long-enough-password',
+    ]);
+
+    $user = User::query()->where('email', 'ada@3ag.local')->firstOrFail();
+
+    expect($user->email_verified_at)->toBeNull()
+        ->and($user->hasVerifiedEmail())->toBeFalse();
+
+    Notification::assertSentTo($user, VerifyEmail::class);
+});
+
+test('registering fires Registered, which is what sends the link', function () {
+    Event::fake();
+
+    $this->post('/register', [
+        'name' => 'Ada Lovelace',
+        'email' => 'ada@3ag.local',
+        'password' => 'a-long-enough-password',
+        'password_confirmation' => 'a-long-enough-password',
+    ]);
+
+    Event::assertDispatched(Registered::class);
+});
+
+test('an unverified user is held at the verification notice', function () {
+    $user = User::factory()->unverified()->create();
+
+    $this->actingAs($user)->get('/home')->assertRedirect(route('verification.notice'));
+});
+
+test('opening the link verifies the address', function () {
+    $user = User::factory()->unverified()->create();
+
+    $url = URL::temporarySignedRoute('verification.verify', now()->addMinutes(60), [
+        'id' => $user->getKey(),
+        'hash' => sha1($user->getEmailForVerification()),
+    ]);
+
+    $this->actingAs($user)->get($url)->assertRedirect(config('fortify.home').'?verified=1');
+
+    expect($user->fresh()->hasVerifiedEmail())->toBeTrue();
+});
+
+test('changing the email address requires verifying the new one', function () {
+    Notification::fake();
+
+    $user = User::factory()->create();
+
+    $this->actingAs($user)->put('/user/profile-information', [
+        'name' => $user->name,
+        'email' => 'moved@3ag.local',
+    ])->assertSessionHasNoErrors();
+
+    $user->refresh();
+
+    expect($user->email)->toBe('moved@3ag.local')
+        ->and($user->hasVerifiedEmail())->toBeFalse();
+
+    Notification::assertSentTo($user, VerifyEmail::class);
+});
+
+test('leaving the email address alone keeps it verified', function () {
+    Notification::fake();
+
+    $user = User::factory()->create();
+
+    $this->actingAs($user)->put('/user/profile-information', [
+        'name' => 'A New Name',
+        'email' => $user->email,
+    ])->assertSessionHasNoErrors();
+
+    expect($user->fresh()->hasVerifiedEmail())->toBeTrue();
+
+    Notification::assertNothingSent();
+});
+
+test('a tampered link does not verify the address', function () {
+    $user = User::factory()->unverified()->create();
+
+    $url = URL::temporarySignedRoute('verification.verify', now()->addMinutes(60), [
+        'id' => $user->getKey(),
+        'hash' => sha1('someone-elses@3ag.local'),
+    ]);
+
+    $this->actingAs($user)->get($url)->assertForbidden();
+
+    expect($user->fresh()->hasVerifiedEmail())->toBeFalse();
+});
+
+/*
+|--------------------------------------------------------------------------
+| The OAuth gate
+|--------------------------------------------------------------------------
+|
+| Registration signs the new user in before they have opened the link, so the
+| authorization screen is the door that actually has to be shut.
+|
+| Passport applies config('passport.middleware') as group middleware, which
+| puts the gate ahead of the `web` group -- and so ahead of the session it
+| reads. bootstrap/app.php moves it back with appendToPriorityList(), and
+| without that the gate resolves a null user and waves everyone through.
+|
+| The two request tests below do NOT catch that on their own: Laravel's test
+| client keeps one session store alive across the requests in a test, so by
+| the time the gate runs the store is already started and the user resolves
+| either way. Verified against a real server instead -- unverified user, cold
+| request: 302 to /email/verify with the priority entry, 200 without it. The
+| ordering test is the regression guard that works in this suite.
+|
+*/
+
+test('the gate is ordered after the session it reads', function () {
+    // The priority list is assembled when the HTTP kernel bootstraps, so it
+    // is empty until something has actually been routed.
+    $this->get('/login');
+
+    $priority = app('router')->middlewarePriority;
+
+    $session = array_search(StartSession::class, $priority, true);
+    $gate = array_search(EnsureEmailIsVerifiedForOAuth::class, $priority, true);
+
+    expect($session)->not->toBeFalse('StartSession is missing from the priority list')
+        ->and($gate)->not->toBeFalse('the OAuth gate is missing from the priority list -- Passport would run it before the session and it would pass everyone through')
+        ->and($gate)->toBeGreaterThan($session);
+});
+
+/**
+ * Sign in the way a browser does, so the user is read back out of the session.
+ *
+ * actingAs() would not do: it puts the user straight on the guard, so the
+ * gate finds one whether or not the session has started by the time it runs
+ * -- which is the exact mistake these tests exist to catch.
+ */
+function signInThroughTheLoginForm(User $user): void
+{
+    test()->post('/login', ['email' => $user->email, 'password' => 'password']);
+
+    test()->assertAuthenticatedAs($user);
+}
+
+function suiteClientRedirectUri(): string
+{
+    return 'https://productsyncmanager.test/auth/accounts/callback';
+}
+
+function suiteClient(): Client
+{
+    return app(ClientRepository::class)->createAuthorizationCodeGrantClient(
+        name: 'ProductSync',
+        redirectUris: [suiteClientRedirectUri()],
+    );
+}
+
+test('an unverified user cannot reach the authorization screen', function () {
+    $client = suiteClient();
+
+    signInThroughTheLoginForm(User::factory()->unverified()->create());
+
+    $this->get(authorizeUrl($client->getKey(), suiteClientRedirectUri()))
+        ->assertRedirect(route('verification.notice'));
+});
+
+test('a verified user still reaches the authorization screen', function () {
+    $client = suiteClient();
+
+    signInThroughTheLoginForm(User::factory()->create());
+
+    $this->get(authorizeUrl($client->getKey(), suiteClientRedirectUri()))
+        ->assertOk();
+});
+
+test('the back channel is untouched by the gate', function () {
+    // No session, so no user to read: the token endpoint has to answer on its
+    // own terms rather than being redirected to a login page.
+    $this->post('/oauth/token', [
+        'grant_type' => 'authorization_code',
+        'client_id' => 'nonsense',
+        'code' => 'nonsense',
+    ])->assertJson(fn ($json) => $json->etc());
+});