mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-17 09:05:08 +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.
108 lines
3.6 KiB
PHP
108 lines
3.6 KiB
PHP
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
namespace App\Modules\Platform\Mail;
|
|
|
|
use App\Modules\Platform\Settings\MailProvider;
|
|
use Illuminate\Database\Eloquent\Model;
|
|
use Illuminate\Support\Carbon;
|
|
|
|
/**
|
|
* One OAuth mail provider's app registration and its connected mailbox.
|
|
*
|
|
* Shaped after SocialSettings, including the part that matters most:
|
|
* `client_secret` and both tokens carry an `'encrypted'` cast, so a
|
|
* database dump does not hand over a credential that can send mail as
|
|
* the organization.
|
|
*
|
|
* The row splits into two halves with different lifetimes: the app
|
|
* registration (client_id/client_secret/tenant_id) survives a
|
|
* disconnect, while the connection itself (tokens, account, error state)
|
|
* is what connecting and disconnecting write. Transports read this row
|
|
* fresh at send time — tokens must never travel through the boot-config
|
|
* cache (see MailConfigApplier, which caches only readiness and the
|
|
* account address).
|
|
*
|
|
* @property int $id
|
|
* @property MailProvider $provider
|
|
* @property string|null $client_id
|
|
* @property string|null $client_secret
|
|
* @property string|null $tenant_id
|
|
* @property string|null $account_email
|
|
* @property string|null $access_token
|
|
* @property string|null $refresh_token
|
|
* @property Carbon|null $token_expires_at
|
|
* @property Carbon|null $last_refreshed_at
|
|
* @property string|null $last_error
|
|
* @property Carbon|null $broken_notified_at
|
|
*/
|
|
class MailOAuthConnection extends Model
|
|
{
|
|
protected $table = 'mail_oauth_connections';
|
|
|
|
protected $guarded = [];
|
|
|
|
protected function casts(): array
|
|
{
|
|
return [
|
|
'provider' => MailProvider::class,
|
|
'client_secret' => 'encrypted',
|
|
'access_token' => 'encrypted',
|
|
'refresh_token' => 'encrypted',
|
|
'token_expires_at' => 'datetime',
|
|
'last_refreshed_at' => 'datetime',
|
|
'broken_notified_at' => 'datetime',
|
|
];
|
|
}
|
|
|
|
/**
|
|
* The failure is over: the error and the record of having alarmed
|
|
* about it go together, because they describe one state.
|
|
*
|
|
* One method rather than two nulls at each call site. The three
|
|
* places that end a failure — a successful refresh, a disconnect, a
|
|
* changed client id — must never clear one and keep the other: a
|
|
* connection that is healthy but still marked "already told them"
|
|
* would go quiet the next time it dies, which is the shape of the
|
|
* bug this column was added to close.
|
|
*/
|
|
public function clearFailure(): void
|
|
{
|
|
$this->last_error = null;
|
|
$this->broken_notified_at = null;
|
|
}
|
|
|
|
public static function for(MailProvider $provider): self
|
|
{
|
|
return static::query()->firstOrNew(['provider' => $provider->value]);
|
|
}
|
|
|
|
/**
|
|
* Whether the connect flow can be started: the admin has entered the
|
|
* app registration, even if no mailbox is connected yet.
|
|
*/
|
|
public function configured(): bool
|
|
{
|
|
return $this->filled('client_id') && $this->filled('client_secret');
|
|
}
|
|
|
|
/**
|
|
* Whether transports can send through this connection. A half-torn
|
|
* state (configured but never connected, or tokens cleared by a
|
|
* disconnect) behaves as "not usable" rather than failing inside a
|
|
* queued job — the same rule SocialSettings::usable() follows.
|
|
*/
|
|
public function usable(): bool
|
|
{
|
|
return $this->configured() && $this->filled('refresh_token');
|
|
}
|
|
|
|
private function filled(string $attribute): bool
|
|
{
|
|
$value = $this->getAttribute($attribute);
|
|
|
|
return is_string($value) && trim($value) !== '';
|
|
}
|
|
}
|