Fix CMS-2016 - Triggering activation email sends email verification link

riasvdv · riasvdv · commit f3e01fa68253 · 2026-05-04T16:58:42.000+02:00
This splits up the activation email and email verification email into two separate concepts. I mistook them for the same flow when porting this functionality earlier.
diff --git a/src/Http/Controllers/Users/ActivateController.php b/src/Http/Controllers/Users/ActivateController.php
@@ -112,7 +112,7 @@ public function sendActivationEmail(Request $request): Response
         $userVariable = $request->getSigned('userVariable') ?? 'user';
 
         try {
-            $user->sendEmailVerificationNotification();
+            $this->users->sendActivationEmail($user);
         } catch (Throwable $e) {
             return $this->asModelFailure(
                 $user,
diff --git a/src/Http/Controllers/Users/PermissionsController.php b/src/Http/Controllers/Users/PermissionsController.php
@@ -103,7 +103,7 @@ public function store(Request $request, Elements $elements): Response
             $request->boolean('sendActivationEmail')
         ) {
             try {
-                $user->sendEmailVerificationNotification();
+                Users::sendActivationEmail($user);
             } catch (Throwable $e) {
                 Flash::error(t('Couldn’t send the activation email: {error}', [
                     'error' => $e->getMessage(),
diff --git a/src/Http/Controllers/Users/SaveUserController.php b/src/Http/Controllers/Users/SaveUserController.php
@@ -317,7 +317,9 @@ public function __invoke(Request $request): Response
             $originalEmail = $user->email;
             $user->email = $user->unverifiedEmail;
 
-            $user->sendEmailVerificationNotification();
+            $isNewUser
+                ? $this->users->sendActivationEmail($user)
+                : $this->users->sendNewEmailVerifyEmail($user);
 
             // Put the original email back into place
             $user->email = $originalEmail;
diff --git a/src/Support/Facades/Users.php b/src/Support/Facades/Users.php
@@ -16,7 +16,9 @@
  * @method static void saveUserPreferences(\CraftCms\Cms\User\Elements\User $user, array $preferences)
  * @method static mixed getUserPreference(int $userId, string $key, mixed $default = null)
  * @method static bool sendPasswordResetEmail(\CraftCms\Cms\User\Elements\User $user)
- * @method static string getActivationUrl(\CraftCms\Cms\User\Elements\User $user)
+ * @method static bool sendActivationEmail(\CraftCms\Cms\User\Elements\User $user)
+ * @method static bool sendNewEmailVerifyEmail(\CraftCms\Cms\User\Elements\User $user)
+ * @method static string getActivationUrl(\CraftCms\Cms\User\Elements\User $user, string|null $token = null)
  * @method static string getEmailVerifyUrl(\CraftCms\Cms\User\Elements\User $user, string|null $token = null)
  * @method static string getPasswordResetUrl(\CraftCms\Cms\User\Elements\User $user, string|null $token = null)
  * @method static void removeCredentials(\CraftCms\Cms\User\Elements\User $user)
diff --git a/src/User/Commands/CreateCommand.php b/src/User/Commands/CreateCommand.php
@@ -152,8 +152,8 @@ public function handle(Elements $elements, GeneralConfig $generalConfig, Users $
             if ($this->option('send-activation-email') || confirm('Send an activation email now?')) {
                 $this->components->task(
                     'Sending activation email...',
-                    function () use ($user) {
-                        $user->sendEmailVerificationNotification();
+                    function () use ($users, $user) {
+                        $users->sendActivationEmail($user);
                     }
                 );
 
diff --git a/src/User/Elements/User.php b/src/User/Elements/User.php
@@ -57,6 +57,7 @@
 use CraftCms\Cms\User\Events\DefineFriendlyName;
 use CraftCms\Cms\User\Events\DefineName;
 use CraftCms\Cms\User\Models\User as UserModel;
+use CraftCms\Cms\User\Notifications\ActivationNotification;
 use CraftCms\Cms\User\Notifications\ResetPasswordNotification;
 use CraftCms\Cms\User\Notifications\VerifyEmailNotification;
 use CraftCms\Cms\User\Validation\UserRules;
@@ -734,6 +735,11 @@ public function sendPasswordResetNotification($token): void
         $this->notify(new ResetPasswordNotification($token));
     }
 
+    public function sendActivationNotification(string $token): void
+    {
+        $this->notify(new ActivationNotification($token));
+    }
+
     public function hasVerifiedEmail(): bool
     {
         return is_null($this->unverifiedEmail);
@@ -2038,7 +2044,9 @@ public function afterSave(bool $isNew): void
             $originalEmail = $this->email;
             $this->email = $this->unverifiedEmail;
 
-            $this->sendEmailVerificationNotification();
+            $isNew
+                ? Users::sendActivationEmail($this)
+                : Users::sendNewEmailVerifyEmail($this);
 
             // Put the original email back into place
             $this->email = $originalEmail;
diff --git a/src/User/Notifications/ActivationNotification.php b/src/User/Notifications/ActivationNotification.php
@@ -0,0 +1,45 @@
+<?php
+
+declare(strict_types=1);
+
+namespace CraftCms\Cms\User\Notifications;
+
+use CraftCms\Cms\Cms;
+use CraftCms\Cms\Support\Facades\Users;
+use CraftCms\Cms\Support\Template;
+use CraftCms\Cms\SystemMessage\Mailables\SystemMessageMailable;
+use CraftCms\Cms\SystemMessage\SystemMessages;
+use CraftCms\Cms\User\Elements\User;
+use Illuminate\Bus\Queueable;
+use Illuminate\Contracts\Queue\ShouldQueue;
+use Illuminate\Notifications\Channels\MailChannel;
+use Illuminate\Notifications\Notification;
+use SensitiveParameter;
+
+class ActivationNotification extends Notification implements ShouldQueue
+{
+    use Queueable;
+
+    public function __construct(
+        #[SensitiveParameter]
+        public string $token,
+    ) {
+        $this->queue = Cms::config()->queueName;
+    }
+
+    public function via(mixed $notifiable): array
+    {
+        return [MailChannel::class];
+    }
+
+    public function toMail(User $user): SystemMessageMailable
+    {
+        $url = Users::getActivationUrl($user, $this->token);
+
+        return app(SystemMessages::class)->mailable(
+            key: 'account_activation',
+            user: $user,
+            variables: ['link' => Template::raw($url)],
+        );
+    }
+}
diff --git a/src/User/Notifications/ResetPasswordNotification.php b/src/User/Notifications/ResetPasswordNotification.php
@@ -32,7 +32,7 @@ public function via(mixed $notifiable): array
 
     public function toMail(User $user): SystemMessageMailable
     {
-        $url = Users::getPasswordResetUrl($user);
+        $url = Users::getPasswordResetUrl($user, $this->token);
 
         return app(SystemMessages::class)->mailable(
             key: 'forgot_password',
diff --git a/src/User/Notifications/VerifyEmailNotification.php b/src/User/Notifications/VerifyEmailNotification.php
@@ -35,10 +35,9 @@ public function via(mixed $notifiable): array
     public function toMail(User $user): SystemMessageMailable
     {
         $url = Users::getEmailVerifyUrl($user, $this->token);
-        $key = $user->pending ? 'account_activation' : 'verify_new_email';
 
         return app(SystemMessages::class)->mailable(
-            key: $key,
+            key: 'verify_new_email',
             user: $user,
             variables: ['link' => Template::raw($url)],
         );
diff --git a/src/User/Users.php b/src/User/Users.php
@@ -252,20 +252,44 @@ public function sendPasswordResetEmail(User $user): bool
         return Password::broker('craft')->sendResetLink(['loginName' => $user->email]) === Password::RESET_LINK_SENT;
     }
 
+    /**
+     * Sends a new account activation email to a user.
+     *
+     * @throws InvalidElementException if the user doesn't validate
+     */
+    public function sendActivationEmail(User $user): bool
+    {
+        $user->sendActivationNotification($this->setVerificationCodeOnUser($user));
+
+        return true;
+    }
+
+    /**
+     * Sends a new email verification email to a user.
+     *
+     * @throws InvalidElementException if the user doesn't validate
+     */
+    public function sendNewEmailVerifyEmail(User $user): bool
+    {
+        $user->sendEmailVerificationNotification();
+
+        return true;
+    }
+
     /**
      * Sets a new verification code on a user, and returns their activation URL.
      *
      *
      * @throws InvalidElementException if the user doesn't validate
      */
-    public function getActivationUrl(User $user): string
+    public function getActivationUrl(User $user, ?string $token = null): string
     {
         // If the user doesn't have a password yet, use a Password Reset URL
-        if (! $user->password) {
-            return $this->getPasswordResetUrl($user);
+        if (! $user->getHasPassword()) {
+            return $this->getPasswordResetUrl($user, $token);
         }
 
-        return $this->getEmailVerifyUrl($user);
+        return $this->getEmailVerifyUrl($user, $token);
     }
 
     /**
diff --git a/tests/Feature/Http/Controllers/User/ActivateControllerTest.php b/tests/Feature/Http/Controllers/User/ActivateControllerTest.php
@@ -5,6 +5,7 @@
 use CraftCms\Cms\Support\Facades\UserPermissions;
 use CraftCms\Cms\User\Elements\User as UserElement;
 use CraftCms\Cms\User\Models\User;
+use CraftCms\Cms\User\Notifications\ActivationNotification;
 use Illuminate\Support\Facades\Notification;
 
 use function Pest\Laravel\actingAs;
@@ -99,6 +100,8 @@
     postJson(action([ActivateController::class, 'sendActivationEmail']), [
         'userId' => $pendingUser->id,
     ])->assertOk();
+
+    Notification::assertSentTimes(ActivationNotification::class, 1);
 });
 
 test('sendActivationEmail requires moderateUsers for inactive (non-pending) users', function () {
diff --git a/tests/Feature/User/EmailVerificationTest.php b/tests/Feature/User/EmailVerificationTest.php
@@ -9,6 +9,7 @@
 use CraftCms\Cms\Support\Facades\Users;
 use CraftCms\Cms\User\Elements\User;
 use CraftCms\Cms\User\Models\User as UserModel;
+use CraftCms\Cms\User\Notifications\ActivationNotification;
 use CraftCms\Cms\User\Notifications\VerifyEmailNotification;
 use Illuminate\Notifications\Channels\MailChannel;
 use Illuminate\Support\Facades\DB;
@@ -19,25 +20,25 @@
     Notification::fake();
 });
 
-test('sendActivationEmail dispatches VerifyEmailNotification', function () {
-    $user = UserModel::factory()->createElement();
-    $user->pending = true;
-    $user = User::find()->id($user->id)->one();
+test('sendActivationEmail dispatches ActivationNotification', function () {
+    $user = UserModel::factory()->createElement([
+        'active' => false,
+        'pending' => true,
+    ]);
 
-    $user->sendEmailVerificationNotification();
+    Users::sendActivationEmail($user);
 
     Notification::assertSentTo(
         $user,
-        VerifyEmailNotification::class,
+        ActivationNotification::class,
         fn ($notification, $channels) => in_array(MailChannel::class, $channels)
     );
 });
 
 test('sendNewEmailVerifyEmail dispatches VerifyEmailNotification', function () {
     $user = UserModel::factory()->createElement();
-    $user = User::find()->id($user->id)->one();
 
-    $user->sendEmailVerificationNotification();
+    Users::sendNewEmailVerifyEmail($user);
 
     Notification::assertSentTo(
         $user,
@@ -46,6 +47,42 @@
     );
 });
 
+test('activation notification uses set password link for passwordless users', function () {
+    $user = UserModel::factory()->createElement([
+        'active' => false,
+        'pending' => true,
+        'password' => null,
+    ]);
+    $token = Users::setVerificationCodeOnUser($user);
+    $mailable = new ActivationNotification($token)->toMail($user);
+
+    expect($mailable->key)->toBe('account_activation')
+        ->and((string) $mailable->variables['link'])->toContain('setpassword?code='.$token)
+        ->and((string) $mailable->variables['link'])->not->toContain('verifyemail?code=');
+});
+
+test('activation notification uses verify email link for users with passwords', function () {
+    $user = UserModel::factory()->createElement([
+        'active' => false,
+        'pending' => true,
+    ]);
+    $token = Users::setVerificationCodeOnUser($user);
+    $mailable = new ActivationNotification($token)->toMail($user);
+
+    expect($mailable->key)->toBe('account_activation')
+        ->and((string) $mailable->variables['link'])->toContain('verifyemail?code='.$token)
+        ->and((string) $mailable->variables['link'])->not->toContain('setpassword?code=');
+});
+
+test('email verification notification uses verify new email message and link', function () {
+    $user = UserModel::factory()->createElement();
+    $token = Users::setVerificationCodeOnUser($user);
+    $mailable = new VerifyEmailNotification($token)->toMail($user);
+
+    expect($mailable->key)->toBe('verify_new_email')
+        ->and((string) $mailable->variables['link'])->toContain('verifyemail?code='.$token);
+});
+
 test('purgeExpiredPendingUsers deletes users with expired tokens', function () {
     $user = UserModel::factory()->createElement(['pending' => true]);
     $user = User::find()->id($user->id)->one();
diff --git a/tests/Feature/User/PasswordResetTest.php b/tests/Feature/User/PasswordResetTest.php
@@ -37,3 +37,12 @@
 
     expect(Password::broker('craft')->tokenExists($user, $token))->toBeTrue();
 });
+
+test('reset password notification uses the broker token', function () {
+    $user = UserModel::factory()->createElement();
+    $token = Password::broker('craft')->createToken($user);
+    $mailable = new ResetPasswordNotification($token)->toMail($user);
+
+    expect((string) $mailable->variables['link'])->toContain('setpassword?code='.$token)
+        ->and(Password::broker('craft')->tokenExists($user, $token))->toBeTrue();
+});

Original file line number	Diff line number	Diff line change
`@@ -32,7 +32,7 @@ public function via(mixed $notifiable): array`
`32`	`32`
`33`	`33`	`public function toMail(User $user): SystemMessageMailable`
`34`	`34`	`{`
`35`		`- $url = Users::getPasswordResetUrl($user);`
	`35`	`+ $url = Users::getPasswordResetUrl($user, $this->token);`
`36`	`36`
`37`	`37`	`return app(SystemMessages::class)->mailable(`
`38`	`38`	`key: 'forgot_password',`