Fix Email Verification (#1629)

* fix registration flow

* Add buttons to request new email verification / to verify email in admin

* Request new email verification when email changes

* Apply fixes from StyleCI

* Fix tests

* Use build-in laravel method for email verification

* Send a verification email to user when admin requests new verification

* Fix tests and dispatch Verified event on manual email verification

* Override verification logic

* Apply fixes from StyleCI

* Force user to active state

* Remove forced state change

* Remove unused service in VerificationController

* Apply fixes from StyleCI

app/Http/Controllers/Admin/UserController.php

@@ -18,6 +18,7 @@
 use App\Services\UserService;
 use App\Support\Timezonelist;
 use App\Support\Utils;
+use Illuminate\Auth\Events\Verified;
 use Illuminate\Http\RedirectResponse;
 use Illuminate\Http\Request;
 use Illuminate\Support\Facades\DB;
@@ -323,6 +324,52 @@ public function regen_apikey(int $id, Request $request): RedirectResponse
         return redirect(route('admin.users.edit', [$id]));
     }
 
+    public function verify_email(int $id, Request $request): RedirectResponse
+    {
+        $user = $this->userRepo->findWithoutFail($id);
+
+        if (empty($user)) {
+            Flash::error('User not found');
+            return back();
+        }
+
+        if ($user->hasVerifiedEmail()) {
+            Flash::error('User email already verified');
+            return back();
+        }
+
+        if ($user->markEmailAsVerified()) {
+            event(new Verified($user));
+        }
+
+        Flash::success('User email verified successfully');
+        return back();
+    }
+
+    public function request_email_verification(int $id, Request $request): RedirectResponse
+    {
+        $user = $this->userRepo->findWithoutFail($id);
+
+        if (empty($user)) {
+            Flash::error('User not found');
+            return back();
+        }
+
+        if (!$user->hasVerifiedEmail()) {
+            Flash::error('User email already not verified');
+            return back();
+        }
+
+        $user->update([
+            'email_verified_at' => null,
+        ]);
+
+        $user->sendEmailVerificationNotification();
+
+        Flash::success('User email verification requested successfully');
+        return back();
+    }
+
     /**
      * Get the type ratings that are available to the user
      *

app/Http/Controllers/Auth/VerificationController.php

@@ -2,8 +2,13 @@
 
 namespace App\Http\Controllers\Auth;
 
+use App\Models\User;
+use Illuminate\Auth\Access\AuthorizationException;
+use Illuminate\Auth\Events\Verified;
 use Illuminate\Foundation\Auth\VerifiesEmails;
+use Illuminate\Http\Request;
 use Illuminate\Routing\Controller;
+use Illuminate\Support\Facades\Log;
 
 class VerificationController extends Controller
 {
@@ -24,7 +29,7 @@ class VerificationController extends Controller
      *
      * @var string
      */
-    protected $redirectTo = '/dashboard';
+    protected string $redirectTo = '/dashboard';
 
     /**
      * Create a new controller instance.
@@ -33,8 +38,31 @@ class VerificationController extends Controller
      */
     public function __construct()
     {
-        $this->middleware('auth');
         $this->middleware('signed')->only('verify');
         $this->middleware('throttle:6,1')->only('verify', 'resend');
     }
+
+    public function verify(Request $request)
+    {
+        $user = User::find($request->route('id'));
+
+        if (!hash_equals((string) $request->route('id'), (string) $user->getKey())) {
+            throw new AuthorizationException();
+        }
+
+        if (!hash_equals((string) $request->route('hash'), sha1($user->getEmailForVerification()))) {
+            throw new AuthorizationException();
+        }
+
+        if ($user->hasVerifiedEmail()) {
+            return redirect($this->redirectPath());
+        }
+
+        if ($user->markEmailAsVerified()) {
+            Log::info('Marking user '.$user->id.' as verified');
+            event(new Verified($user));
+        }
+
+        return redirect($this->redirectPath())->with('verified', true);
+    }
 }

app/Http/Controllers/Frontend/ProfileController.php

@@ -213,8 +213,19 @@ public function update(Request $request): RedirectResponse
             $req_data['avatar'] = $path;
         }
 
+        // User needs to verify their new email address
+        if ($user->email != $request->input('email')) {
+            $req_data['email_verified_at'] = null;
+        }
+
         $this->userRepo->update($req_data, $id);
 
+        // We need to get a new instance of the user in order to send the verification email to the new email address
+        if ($user->email != $request->input('email')) {
+            $newUser = $this->userRepo->findWithoutFail($user->id);
+            $newUser->sendEmailVerificationNotification();
+        }
+
         // Save all of the user fields
         $userFields = UserField::all();
         foreach ($userFields as $field) {

app/Models/User.php

@@ -35,7 +35,7 @@
  * @property string           home_airport_id
  * @property string           avatar
  * @property Airline          airline
- * @property Flight[]         flights
+ * @property int              flights
  * @property int              flight_time
  * @property int              transfer_time
  * @property string           remember_token

app/Notifications/NotificationEventsHandler.php

@@ -10,14 +10,15 @@
 use App\Events\PirepPrefiled;
 use App\Events\PirepRejected;
 use App\Events\PirepStatusChange;
-use App\Events\UserRegistered;
 use App\Events\UserStateChanged;
+use App\Events\UserStatsChanged;
 use App\Models\Enums\PirepStatus;
 use App\Models\Enums\UserState;
 use App\Models\User;
 use App\Notifications\Messages\UserRejected;
 use App\Notifications\Notifiables\Broadcast;
 use Exception;
+use Illuminate\Auth\Events\Verified;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Facades\Notification;
@@ -37,9 +38,9 @@ class NotificationEventsHandler extends Listener
         PirepAccepted::class     => 'onPirepAccepted',
         PirepFiled::class        => 'onPirepFile',
         PirepRejected::class     => 'onPirepRejected',
-        UserRegistered::class    => 'onUserRegister',
         UserStateChanged::class  => 'onUserStateChange',
         UserStatsChanged::class  => 'onUserStatsChanged',
+        Verified::class          => 'onEmailVerified',
     ];
 
     public function __construct()
@@ -113,13 +114,13 @@ protected function notifyAllUsers(\App\Contracts\Notification $notification)
         }
     }
 
-    /**
-     * Send an email when the user registered
-     *
-     * @param UserRegistered $event
-     */
-    public function onUserRegister(UserRegistered $event): void
+    public function onEmailVerified(Verified $event): void
     {
+        // Return if the user has any flights (email change / admin requests new verification)
+        if ($event->user->flights > 0) {
+            return;
+        }
+
         Log::info('NotificationEvents::onUserRegister: '
             .$event->user->ident.' is '
             .UserState::label($event->user->state).', sending active email');

app/Providers/RouteServiceProvider.php

@@ -461,6 +461,12 @@ private function mapAdminRoutes()
             Route::get('users/{id}/regen_apikey', 'UserController@regen_apikey')
                 ->name('users.regen_apikey')->middleware('ability:admin,users');
 
+            Route::get('users/{id}/verify_email', 'UserController@verify_email')
+                ->name('users.verify_email')->middleware('ability:admin,users');
+
+            Route::get('users/{id}/request_email_verification', 'UserController@request_email_verification')
+                ->name('users.request_email_verification')->middleware('ability:admin,users');
+
             Route::resource('users', 'UserController')->middleware('ability:admin,users');
 
             Route::match([

app/Services/UserService.php

@@ -3,7 +3,6 @@
 namespace App\Services;
 
 use App\Contracts\Service;
-use App\Events\UserRegistered;
 use App\Events\UserStateChanged;
 use App\Events\UserStatsChanged;
 use App\Exceptions\PilotIdNotFound;
@@ -25,6 +24,7 @@
 use App\Support\Units\Time;
 use App\Support\Utils;
 use Carbon\Carbon;
+use Illuminate\Auth\Events\Registered;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\Hash;
 use Illuminate\Support\Facades\Log;
@@ -115,7 +115,7 @@ public function createUser(array $attrs, array $roles = []): User
         $this->calculatePilotRank($user);
         $user->refresh();
 
-        event(new UserRegistered($user));
+        event(new Registered($user));
 
         return $user;
     }

config/phpvms.php

@@ -124,6 +124,6 @@
      * Enable/disable email verification on registration
      */
     'registration' => [
-        'email_verification' => true,
+        'email_verification' => env('EMAIL_VERIFICATION_REQUIRED', true),
     ],
 ];

resources/views/admin/users/fields.blade.php

@@ -90,6 +90,12 @@
   <div class="form-group col-sm-12 text-right">
     {{-- <a href="{{ route('admin.users.regen_apikey', [$user->id]) }}" class="btn btn-warning" onclick="return confirm('Are you sure? This will reset this user\'s API key.')">New API Key</a> --}}
     &nbsp;
+    @if (!$user->email_verified_at)
+      <a href="{{ route('admin.users.verify_email', [$user->id]) }}" class="btn btn-warning">Verify email</a>
+    @else
+      <a href="{{ route('admin.users.request_email_verification', [$user->id]) }}" class="btn btn-warning">Request new email verification</a>
+    @endif
+
     {{ Form::button('Save', ['type' => 'submit', 'class' => 'btn btn-success']) }}
     <a href="{{ route('admin.users.index') }}" class="btn btn-default">Cancel</a>
   </div>

tests/RegistrationTest.php

@@ -6,6 +6,7 @@
 use App\Models\User;
 use App\Notifications\Messages\AdminUserRegistered;
 use App\Services\UserService;
+use Illuminate\Auth\Events\Verified;
 use Illuminate\Support\Facades\Hash;
 use Illuminate\Support\Facades\Notification;
 
@@ -29,10 +30,15 @@ public function testRegistration()
 
         $attrs = User::factory()->make()->makeVisible(['api_key', 'name', 'email'])->toArray();
         $attrs['password'] = Hash::make('secret');
+        $attrs['flights'] = 0;
         $user = $userSvc->createUser($attrs);
 
         $this->assertEquals(UserState::ACTIVE, $user->state);
 
+        if ($user->markEmailAsVerified()) {
+            event(new Verified($user));
+        }
+
         Notification::assertSentTo([$admin], AdminUserRegistered::class);
         Notification::assertNotSentTo([$user], AdminUserRegistered::class);
     }