mirror of
https://github.com/projectsend/projectsend.git
synced 2026-10-04 05:25:51 +00:00
7be81d3586
The daily refresh doubles as the health check for a connected OAuth mailbox, and its own docblock says why that matters: a grant can die silently, "which for a portal whose password-reset mails ride on this connection must surface as a warning, not as a support ticket weeks later". It decided whether to warn by reading last_error -- but the send path writes that column too. OAuthCodeFlowBroker::refresh() records the failure and notifies nobody, and freshAccessToken() reaches it from every send. So on an installation that actually sends mail, the send lands first, the command reads the column as "already told them", and the warning never goes out. last_error is cleared only by a successful refresh, which a dead grant never has, so it never goes out again either. Measured on main, one dead grant, two orders: nobody sends, command first 1 notification, then quiet correct a password-reset mail first 0 ... 0 ... 0 never The alarm worked on installations that were not using the mailbox and failed on the ones that were. The anti-nag rule is not the problem and does not change. The problem is that last_error answers "is this broken", which any writer may set, while the command needs "have the admins been told", which only the notifier can. The table's own comment shows the conflation -- one column described as "what the settings page's warning and the admin notification read". So the notification gets its own column. broken_notified_at is stamped when the command notifies, and cleared wherever last_error is cleared: a successful refresh, a disconnect, a changed client id. The three call sites go through MailOAuthConnection::clearFailure() rather than nulling two columns each, because a connection left marked "already told them" while healthy would go quiet the next time it died -- the same bug in a new place.
105 lines
4.4 KiB
PHP
105 lines
4.4 KiB
PHP
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
namespace App\Modules\Platform\Mail\Console;
|
|
|
|
use App\Models\User;
|
|
use App\Modules\Identity\Permissions\Permission;
|
|
use App\Modules\Identity\Permissions\PermissionChecker;
|
|
use App\Modules\Identity\UserType;
|
|
use App\Modules\Notifications\Notifier;
|
|
use App\Modules\Platform\Mail\MailOAuthBrokers;
|
|
use App\Modules\Platform\Mail\MailOAuthConnection;
|
|
use App\Modules\Platform\Mail\MailOAuthException;
|
|
use App\Modules\Platform\Settings\MailConfigApplier;
|
|
use Illuminate\Console\Command;
|
|
|
|
/**
|
|
* Keeps every connected OAuth mailbox able to send, and says so early
|
|
* when one no longer can.
|
|
*
|
|
* Transports already refresh on demand at send time; what they cannot do
|
|
* is refresh on an installation that sends rarely — and a delegated
|
|
* refresh token dies of pure disuse (Microsoft's sliding inactivity
|
|
* window). A daily refresh keeps the window sliding, and doubles as the
|
|
* health check: the delegated flow's one real weakness is that a grant
|
|
* can die silently (password reset, Conditional Access change), which
|
|
* for a portal whose password-reset mails ride on this connection must
|
|
* surface as a warning, not as a support ticket weeks later.
|
|
*/
|
|
class RefreshMailOAuthTokensCommand extends Command
|
|
{
|
|
protected $signature = 'projectsend:refresh-mail-oauth-tokens';
|
|
|
|
protected $description = 'Refresh connected OAuth mailbox tokens and flag connections that need to be reconnected (runs daily)';
|
|
|
|
public function handle(MailOAuthBrokers $brokers, Notifier $notifier, PermissionChecker $permissions, MailConfigApplier $mailConfig): int
|
|
{
|
|
$connections = MailOAuthConnection::query()->get()->filter(
|
|
fn (MailOAuthConnection $connection): bool => $connection->usable(),
|
|
);
|
|
|
|
if ($connections->isEmpty()) {
|
|
$this->info('No connected OAuth mailboxes; nothing to refresh.');
|
|
|
|
return self::SUCCESS;
|
|
}
|
|
|
|
foreach ($connections as $connection) {
|
|
$hadError = $connection->last_error !== null;
|
|
|
|
try {
|
|
$brokers->for($connection->provider)->refresh($connection);
|
|
|
|
$this->info("Refreshed {$connection->provider->value} ({$connection->account_email}).");
|
|
|
|
// Back from the dead (an admin fixed things upstream
|
|
// without reconnecting): the applier may have been
|
|
// resolving "not ready" and must see the recovery.
|
|
if ($hadError) {
|
|
$mailConfig->flush();
|
|
}
|
|
} catch (MailOAuthException $e) {
|
|
$this->error("Could not refresh {$connection->provider->value}: {$e->getMessage()}");
|
|
|
|
if (! $e->needsReconnect) {
|
|
continue;
|
|
}
|
|
|
|
// Only on the transition into the broken state — the
|
|
// notification would otherwise repeat daily for as long
|
|
// as nobody reconnects, and a nagging alert trains
|
|
// people to ignore the one that matters.
|
|
//
|
|
// Asked of broken_notified_at, not of last_error. The
|
|
// question is "have the admins been told", and last_error
|
|
// cannot answer it: the send path writes that column too
|
|
// (OAuthCodeFlowBroker::refresh, reached from
|
|
// freshAccessToken) and notifies nobody. On an
|
|
// installation that actually sends mail, that write lands
|
|
// first — so reading it as "already told them" left this
|
|
// silent for good, on exactly the installations whose
|
|
// password-reset mail rides on the connection.
|
|
if ($connection->broken_notified_at === null) {
|
|
$recipients = array_values(User::query()->where('type', UserType::Staff)->get()
|
|
->filter(fn (User $staff): bool => $permissions->allows($staff, Permission::EditSettings))
|
|
->all());
|
|
|
|
$notifier->send('mail_oauth_connection_broken', $recipients, data: [
|
|
'provider' => $connection->provider->label(),
|
|
'account' => (string) $connection->account_email,
|
|
]);
|
|
|
|
$connection->broken_notified_at = now();
|
|
$connection->save();
|
|
}
|
|
|
|
$mailConfig->flush();
|
|
}
|
|
}
|
|
|
|
return self::SUCCESS;
|
|
}
|
|
}
|