diff --git a/src/app/Mail/Helper.php b/src/app/Mail/Helper.php index 66ccc1a0..73586a32 100644 --- a/src/app/Mail/Helper.php +++ b/src/app/Mail/Helper.php @@ -1,131 +1,133 @@ build(); // @phpstan-ignore-line $mailer = \Illuminate\Container\Container::getInstance()->make('mailer'); return $mailer->render(['text' => $mail->textView], $mail->buildViewData()); } if ($type != 'html') { throw new \Exception("Unsupported output format"); } // HTML output return $mail->build()->render(); // @phpstan-ignore-line } /** * Sends an email * * @param Mailable $mail Email content generator * @param int|null $tenantId Tenant identifier * @param array $params Email parameters: to, cc * * @throws \Exception */ public static function sendMail(Mailable $mail, $tenantId = null, array $params = []): void { $class = class_basename(get_class($mail)); $recipients = []; // For now we do not support addresses + names, only addresses foreach (['to', 'cc'] as $idx) { if (!empty($params[$idx])) { if (is_array($params[$idx])) { $recipients = array_merge($recipients, $params[$idx]); } else { $recipients[] = $params[$idx]; } } } try { if (!empty($params['to'])) { $mail->to($params['to']); } if (!empty($params['cc'])) { $mail->cc($params['cc']); } - $fromAddress = Tenant::getConfig($tenantId, 'mail.from.address'); - $fromName = Tenant::getConfig($tenantId, 'mail.from.name'); - $replytoAddress = Tenant::getConfig($tenantId, 'mail.reply_to.address'); - $replytoName = Tenant::getConfig($tenantId, 'mail.reply_to.name'); + // Note: We're not using Laravel's global 'from' and 'reply_to' settings + + $fromAddress = Tenant::getConfig($tenantId, 'mail.sender.address'); + $fromName = Tenant::getConfig($tenantId, 'mail.sender.name'); + $replytoAddress = Tenant::getConfig($tenantId, 'mail.replyto.address'); + $replytoName = Tenant::getConfig($tenantId, 'mail.replyto.name'); if ($fromAddress) { $mail->from($fromAddress, $fromName); } if ($replytoAddress) { $mail->replyTo($replytoAddress, $replytoName); } Mail::send($mail); if ($user = $mail->getUser()) { $comment = "[{$class}] " . $mail->getSubject(); EventLog::createFor($user, EventLog::TYPE_MAILSENT, $comment, ['recipients' => $recipients]); } } catch (\Exception $e) { $format = "[%s] Failed to send mail to %s%s: %s"; $msg = sprintf($format, $class, implode(', ', $recipients), $params['add'] ?? '', $e->getMessage()); \Log::error($msg); throw $e; } } /** * Return user's email addresses, separately for use in To and Cc. * * @param \App\User $user The user * @param bool $external Include users's external email * * @return array To address as the first element, Cc address(es) as the second. */ public static function userEmails(\App\User $user, bool $external = false): array { $active = $user->isLdapReady() && $user->isImapReady(); // Sending an email to non-(ldap|imap)-ready user will fail, skip it // (or send to the external email only, when appropriate) $to = $active ? $user->email : null; $cc = []; // If user has no mailbox entitlement we should not send // the email to his main address, but use external address, if defined if ($active && !$user->hasSku('mailbox')) { $to = $user->getSetting('external_email'); } elseif ($external) { $ext_email = $user->getSetting('external_email'); if ($ext_email && $ext_email != $to) { $cc[] = $ext_email; } } return [$to, $cc]; } } diff --git a/src/config/mail.php b/src/config/mail.php index 837a2b89..e18e82a3 100644 --- a/src/config/mail.php +++ b/src/config/mail.php @@ -1,135 +1,145 @@ env('MAIL_MAILER', 'smtp'), /* |-------------------------------------------------------------------------- | Mailer Configurations |-------------------------------------------------------------------------- | | Here you may configure all of the mailers used by your application plus | their respective settings. Several examples have been configured for | you and you are free to add your own as your application requires. | | Laravel supports a variety of mail "transport" drivers to be used while | sending an e-mail. You will specify which one you are using for your | mailers below. You are free to add additional mailers as required. | | Supported: "smtp", "sendmail", "mailgun", "ses", "ses-v2", | "postmark", "log", "array", "failover" | */ 'mailers' => [ 'smtp' => [ 'transport' => 'smtp', 'url' => env('MAIL_URL'), 'host' => env('MAIL_HOST', 'smtp.mailgun.org'), 'port' => env('MAIL_PORT', 587), 'encryption' => env('MAIL_ENCRYPTION', 'tls'), 'username' => env('MAIL_USERNAME'), 'password' => env('MAIL_PASSWORD'), 'timeout' => null, 'local_domain' => env('MAIL_EHLO_DOMAIN'), ], 'ses' => [ 'transport' => 'ses', ], 'mailgun' => [ 'transport' => 'mailgun', ], 'postmark' => [ 'transport' => 'postmark', ], 'sendmail' => [ 'transport' => 'sendmail', 'path' => env('MAIL_SENDMAIL_PATH', '/usr/sbin/sendmail -t -i'), ], 'log' => [ 'transport' => 'log', 'channel' => env('MAIL_LOG_CHANNEL'), ], 'array' => [ 'transport' => 'array', ], 'failover' => [ 'transport' => 'failover', 'mailers' => [ 'smtp', 'log', ], ], ], /* |-------------------------------------------------------------------------- | Global "From" Address |-------------------------------------------------------------------------- | | You may wish for all e-mails sent by your application to be sent from | the same address. Here, you may specify a name and address that is | used globally for all e-mails that are sent by your application. | */ - 'from' => [ - 'address' => env('MAIL_FROM_ADDRESS', 'hello@example.com'), - 'name' => env('MAIL_FROM_NAME', 'Example'), + // Note: Laravel uses 'from', we added 'sender'. This way we make sure + // Laravel does not overwrite our From header that we set in App\Mail\Helper::sendMail(). + + 'from' => null, // do not use! + 'sender' => [ + 'address' => env('MAIL_FROM_ADDRESS', ''), + 'name' => env('MAIL_FROM_NAME', ''), ], /* |-------------------------------------------------------------------------- | Global "Reply-To" Address |-------------------------------------------------------------------------- | | You may wish for all e-mails sent by your application to be sent from | the same address. Here, you may specify a name and address that is | used globally for all e-mails that are sent by your application. | */ - 'reply_to' => [ + // Note: Laravel uses 'reply_to', we added 'replyto'. This way we make sure + // Laravel does not add an extra Reply-To header on top of our per-tenant + // Reply-To header that we set in App\Mail\Helper::sendMail(). + // More https://github.com/laravel/framework/issues/43034. + + 'reply_to' => null, // do not use! + 'replyto' => [ 'address' => env('MAIL_REPLYTO_ADDRESS', ''), 'name' => env('MAIL_REPLYTO_NAME', ''), ], /* |-------------------------------------------------------------------------- | Markdown Mail Settings |-------------------------------------------------------------------------- | | If you are using Markdown based email rendering, you may configure your | theme and component paths here, allowing you to customize the design | of the emails. Or, you may simply stick with the Laravel defaults! | */ 'markdown' => [ 'theme' => 'default', 'paths' => [ resource_path('views/vendor/mail'), ], ], ]; diff --git a/src/tests/Feature/Jobs/Password/RetentionEmailJobTest.php b/src/tests/Feature/Jobs/Password/RetentionEmailJobTest.php index 171106b3..4c8c92bf 100644 --- a/src/tests/Feature/Jobs/Password/RetentionEmailJobTest.php +++ b/src/tests/Feature/Jobs/Password/RetentionEmailJobTest.php @@ -1,75 +1,75 @@ deleteTestUser('PasswordRetention@UserAccount.com'); } /** * {@inheritDoc} * * @return void */ public function tearDown(): void { $this->deleteTestUser('PasswordRetention@UserAccount.com'); parent::tearDown(); } /** * Test job handle * * @return void */ public function testHandle() { $status = User::STATUS_ACTIVE | User::STATUS_LDAP_READY | User::STATUS_IMAP_READY; $user = $this->getTestUser('PasswordRetention@UserAccount.com', ['status' => $status]); $expiresOn = now()->copy()->addDays(7)->toDateString(); Mail::fake(); // Assert that no jobs were pushed... Mail::assertNothingSent(); $job = new RetentionEmailJob($user, $expiresOn); $job->handle(); $this->assertMatchesRegularExpression( '/^' . now()->format('Y-m-d') . ' [0-9]{2}:[0-9]{2}:[0-9]{2}$/', $user->getSetting('password_expiration_warning') ); // Assert the email sending job was pushed once Mail::assertSent(PasswordExpirationReminder::class, 1); // Assert the mail was sent to the code's email Mail::assertSent(PasswordExpirationReminder::class, function ($mail) use ($user) { return $mail->hasTo($user->email); }); // Assert sender Mail::assertSent(PasswordExpirationReminder::class, function ($mail) { - return $mail->hasFrom(\config('mail.from.address'), \config('mail.from.name')) - && $mail->hasReplyTo(\config('mail.reply_to.address'), \config('mail.reply_to.name')); + return $mail->hasFrom(\config('mail.sender.address'), \config('mail.sender.name')) + && $mail->hasReplyTo(\config('mail.replyto.address'), \config('mail.replyto.name')); }); } } diff --git a/src/tests/Feature/Jobs/PasswordResetEmailTest.php b/src/tests/Feature/Jobs/PasswordResetEmailTest.php index 829d56cc..362b4a7f 100644 --- a/src/tests/Feature/Jobs/PasswordResetEmailTest.php +++ b/src/tests/Feature/Jobs/PasswordResetEmailTest.php @@ -1,75 +1,75 @@ deleteTestUser('PasswordReset@UserAccount.com'); } /** * {@inheritDoc} * * @return void */ public function tearDown(): void { $this->deleteTestUser('PasswordReset@UserAccount.com'); parent::tearDown(); } /** * Test job handle * * @return void */ public function testPasswordResetEmailHandle() { $code = new VerificationCode([ 'mode' => 'password-reset', ]); $user = $this->getTestUser('PasswordReset@UserAccount.com'); $user->verificationcodes()->save($code); $user->setSettings(['external_email' => 'etx@email.com']); Mail::fake(); // Assert that no jobs were pushed... Mail::assertNothingSent(); $job = new PasswordResetEmail($code); $job->handle(); // Assert the email sending job was pushed once Mail::assertSent(PasswordReset::class, 1); // Assert the mail was sent to the code's email Mail::assertSent(PasswordReset::class, function ($mail) use ($code) { return $mail->hasTo($code->user->getSetting('external_email')); }); // Assert sender Mail::assertSent(PasswordReset::class, function ($mail) { - return $mail->hasFrom(\config('mail.from.address'), \config('mail.from.name')) - && $mail->hasReplyTo(\config('mail.reply_to.address'), \config('mail.reply_to.name')); + return $mail->hasFrom(\config('mail.sender.address'), \config('mail.sender.name')) + && $mail->hasReplyTo(\config('mail.replyto.address'), \config('mail.replyto.name')); }); } } diff --git a/src/tests/Unit/Mail/HelperTest.php b/src/tests/Unit/Mail/HelperTest.php index 42ec8e16..efd26b06 100644 --- a/src/tests/Unit/Mail/HelperTest.php +++ b/src/tests/Unit/Mail/HelperTest.php @@ -1,179 +1,179 @@ deleteTestUser('mail-helper-test@kolabnow.com'); \App\TenantSetting::truncate(); } /** * {@inheritDoc} */ public function tearDown(): void { $this->deleteTestUser('mail-helper-test@kolabnow.com'); \App\TenantSetting::truncate(); parent::tearDown(); } /** * Test Helper::sendMail() */ public function testSendMail(): void { EventLog::truncate(); Mail::fake(); $tenant = \App\Tenant::whereNotIn('id', [1])->first(); $invitation = new \App\SignupInvitation(); $invitation->id = 'test'; $mail = new \App\Mail\SignupInvitation($invitation); Helper::sendMail($mail, null, ['to' => 'to@test.com', 'cc' => 'cc@test.com']); Mail::assertSent(\App\Mail\SignupInvitation::class, 1); Mail::assertSent(\App\Mail\SignupInvitation::class, function ($mail) { return $mail->hasTo('to@test.com') && $mail->hasCc('cc@test.com') - && $mail->hasFrom(\config('mail.from.address'), \config('mail.from.name')) - && $mail->hasReplyTo(\config('mail.reply_to.address'), \config('mail.reply_to.name')); + && $mail->hasFrom(\config('mail.sender.address'), \config('mail.sender.name')) + && $mail->hasReplyTo(\config('mail.replyto.address'), \config('mail.replyto.name')); }); // Test with a tenant (but no per-tenant settings) Mail::fake(); $invitation->tenant_id = $tenant->id; $mail = new \App\Mail\SignupInvitation($invitation); Helper::sendMail($mail, $tenant->id, ['to' => 'to@test.com', 'cc' => 'cc@test.com']); Mail::assertSent(\App\Mail\SignupInvitation::class, 1); Mail::assertSent(\App\Mail\SignupInvitation::class, function ($mail) { return $mail->hasTo('to@test.com') && $mail->hasCc('cc@test.com') - && $mail->hasFrom(\config('mail.from.address'), \config('mail.from.name')) - && $mail->hasReplyTo(\config('mail.reply_to.address'), \config('mail.reply_to.name')); + && $mail->hasFrom(\config('mail.sender.address'), \config('mail.sender.name')) + && $mail->hasReplyTo(\config('mail.replyto.address'), \config('mail.replyto.name')); }); // Test with a tenant (but with per-tenant settings) Mail::fake(); $tenant->setSettings([ - 'mail.from.address' => 'from@test.com', - 'mail.from.name' => 'from name', - 'mail.reply_to.address' => 'replyto@test.com', - 'mail.reply_to.name' => 'replyto name', + 'mail.sender.address' => 'from@test.com', + 'mail.sender.name' => 'from name', + 'mail.replyto.address' => 'replyto@test.com', + 'mail.replyto.name' => 'replyto name', ]); $mail = new \App\Mail\SignupInvitation($invitation); Helper::sendMail($mail, $tenant->id, ['to' => 'to@test.com']); Mail::assertSent(\App\Mail\SignupInvitation::class, 1); Mail::assertSent(\App\Mail\SignupInvitation::class, function ($mail) { return $mail->hasTo('to@test.com') && $mail->hasFrom('from@test.com', 'from name') && $mail->hasReplyTo('replyto@test.com', 'replyto name'); }); // No EventLog entries up to this point $this->assertSame(0, EventLog::count()); // Assert EventLog entry $user = $this->getTestUser('mail-helper-test@kolabnow.com'); $mail = new \App\Mail\TrialEnd($user); Helper::sendMail($mail, $tenant->id, ['to' => 'to@test.com', 'cc' => 'cc@test.com']); $event = EventLog::where('object_id', $user->id)->where('object_type', User::class)->first(); $this->assertSame(EventLog::TYPE_MAILSENT, $event->type); $this->assertSame(['recipients' => ['to@test.com', 'cc@test.com']], $event->data); $this->assertSame("[TrialEnd] Kolab Now: Your trial phase has ended", $event->comment); // TODO: Test somehow exception case } /** * Test Helper::userEmails() */ public function testUserEmails(): void { $status = User::STATUS_ACTIVE | User::STATUS_LDAP_READY | User::STATUS_IMAP_READY; $user = $this->getTestUser('mail-helper-test@kolabnow.com', ['status' => $status]); // User with no mailbox and no external email list($to, $cc) = Helper::userEmails($user); $this->assertSame(null, $to); $this->assertSame([], $cc); list($to, $cc) = Helper::userEmails($user, true); $this->assertSame(null, $to); $this->assertSame([], $cc); // User with no mailbox but with external email $user->setSetting('external_email', 'external@test.com'); list($to, $cc) = Helper::userEmails($user); $this->assertSame('external@test.com', $to); $this->assertSame([], $cc); list($to, $cc) = Helper::userEmails($user, true); $this->assertSame('external@test.com', $to); $this->assertSame([], $cc); // User with mailbox and external email $sku = \App\Sku::withEnvTenantContext()->where('title', 'mailbox')->first(); $user->assignSku($sku); list($to, $cc) = Helper::userEmails($user); $this->assertSame($user->email, $to); $this->assertSame([], $cc); list($to, $cc) = Helper::userEmails($user, true); $this->assertSame($user->email, $to); $this->assertSame(['external@test.com'], $cc); // User with mailbox, but no external email $user->setSetting('external_email', null); list($to, $cc) = Helper::userEmails($user); $this->assertSame($user->email, $to); $this->assertSame([], $cc); list($to, $cc) = Helper::userEmails($user, true); $this->assertSame($user->email, $to); $this->assertSame([], $cc); // Use with mailbox, but not ready $user->setSetting('external_email', 'external@test.com'); $user->status = User::STATUS_ACTIVE | User::STATUS_LDAP_READY; $user->save(); list($to, $cc) = Helper::userEmails($user, true); $this->assertSame(null, $to); $this->assertSame(['external@test.com'], $cc); } }