feat(health): surface broken accounts, and repair them without re-adding

One expired Google grant on the reporting install had stopped mail for two
days and produced ~5 000 log lines, and the app knew: OAuthTokenManager wrote
`invalid_grant` to Account::$oauthLastRefreshError, CalendarSyncService wrote
the calendar failures, IntegrationTokenManager already composed "X needs to be
reconnected". Nothing read any of it back — grep for oauthLastRefreshError over
templates/ and src/Controller/ returned nothing, and no template contained the
word "reconnect". The condition was detected, stored, and discarded.

WHAT THIS ADDS

A health model (AccountHealthInspector) that reads that stored state and
answers "what is wrong, and what would fix it". It reads; it never probes. Five
problem classes: dead OAuth grant, calendar sync failing, degraded push,
integration needing reconnection, and abandoned queue work.

Consequences are attributed to their cause. One dead grant taking three
calendars and 500 jobs down with it is one thing to do, not nine, so the
calendars are listed under the account that explains them and the indicator
counts root causes.

Severity is drawn by consequence, not by how alarming the error looks. Degraded
push is a Notice and deliberately does NOT light the indicator: mail still
arrives, and a page that paints "slightly delayed" the same red as "stopped" is
a page people learn to close.

RECONNECT IN PLACE

OAuthAccountLinker::relink() writes a fresh grant onto the existing Account row.
Mail, threads, labels, rules, aliases, calendars and settings all key on that id
and all survive — the lighter alternative to delete-and-re-add, which is what
people resort to today and what costs them their mail.

Guarded on identity: the authorised mailbox must be the same address and the
same provider, compared case-insensitively, and a mismatch is refused outright
rather than resolved. Signing in as the wrong Google account is a mistake anyone
makes, and writing those tokens here would file a stranger's mail into these
threads with no undo. The repair intent travels in the session, never through
the provider redirect, so no account id is user-editable.

STOPPING THE LOG FLOOD

A calendar that cannot sync never updates lastSyncedAt, so findDueForSync()
returned it on every sweep forever. Calendar now carries syncFailureCount and
syncBackoffUntil; the sweep skips a calendar inside its window, and the handler
logs only when the failure is news.

Not a mute button, and not a reclassification — GoogleCalendarApiClient::token()
is right to refuse to call a refused refresh permanent. The first failure always
reports, a CHANGED message always reports, and the delay is capped at a day so a
condition that heals is retried within one.

SURFACING

Its own settings section rather than a block on Accounts: the problems span
accounts, calendars, connections and the queue, and a variable-height alert
stack would push the real account controls below the fold exactly when something
is wrong. Plus a topbar badge, following LogAlertGlobal's shape including
ResetInterface (worker mode) — but not dismissible, because "seen" is the right
model for a log and the wrong one for a condition that is still true.

Every string is translated (en/de/en_PI) and written for a person: what broke,
what it means for their mail, and what the button will do before they press it.
Provider error codes stay behind a disclosure.

Verified: phpunit 3411 tests green, composer stan clean, 5 Playwright specs
green, both themes at 1440px and 393px.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Paul LütznerandClaude Opus 5 committed 2026-08-13 09:21:35 +02:00
1 parent 650b98939d
commit 7dbd6cc890
30 files changed
+3917 -16

No files matched your search

+5
View File
@@ -14,6 +14,11 @@ twig:
calendar: '@App\Twig\CalendarGlobal'
onboarding: '@App\Twig\OnboardingGlobal'
logAlert: '@App\Twig\LogAlertGlobal'
# "Something about YOUR accounts needs attention" — the user-facing
# counterpart to logAlert, which is the admin's. A global for the same
# reason: the topbar renders from a layout on every authenticated page,
# so there is no controller in the middle to thread it through.
accountHealth: '@App\Twig\AccountHealthGlobal'
# True when the poll is asking for the message list on its own, so the
# mailbox layout can answer with the frame instead of the whole page.
listFragment: '@App\Twig\ListFragmentGlobal'
+51
View File
@@ -0,0 +1,51 @@
<?php
declare(strict_types=1);
namespace DoctrineMigrations;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;
/**
* Backoff state for a calendar whose sync keeps failing.
*
* The sweep asks findDueForSync() for calendars whose last_synced_at is old,
* and a calendar that CANNOT sync never updates last_synced_at — so it comes
* back due on every sweep, permanently. On the install this was written for,
* one expired Google sign-in kept three calendars in that loop for two days and
* produced 2 193 identical error lines and 503 dead-lettered jobs. These two
* columns are what lets the query skip a calendar that is already known to be
* failing until its next scheduled attempt.
*
* Both are added with defaults that mean "healthy, try immediately", so every
* existing row — including the broken ones this was written for — starts from a
* clean slate and gets exactly one attempt and one log line before any backoff
* applies. Starting them backed-off would have been the tidier-looking choice
* and the wrong one: it would suppress the first occurrence on rows where the
* first occurrence has never actually been shown to anybody.
*
* sync_failure_count is NOT NULL with a default because it is a counter and a
* null would have to be read as zero at every call site. sync_backoff_until is
* nullable because null is a real state with its own meaning — no window is in
* force — and is what findDueForSync() tests for explicitly.
*/
final class Version20260813090000 extends AbstractMigration
{
public function getDescription(): string
{
return 'Adds calendar.sync_failure_count and calendar.sync_backoff_until so a permanently failing sync backs off instead of retrying every sweep forever';
}
public function up(Schema $schema): void
{
$this->addSql('ALTER TABLE calendar ADD sync_failure_count INT DEFAULT 0 NOT NULL');
$this->addSql('ALTER TABLE calendar ADD sync_backoff_until TIMESTAMP(0) WITHOUT TIME ZONE DEFAULT NULL');
}
public function down(Schema $schema): void
{
$this->addSql('ALTER TABLE calendar DROP sync_failure_count');
$this->addSql('ALTER TABLE calendar DROP sync_backoff_until');
}
}
+81 -2
View File
@@ -5,7 +5,9 @@ declare(strict_types=1);
namespace App\Controller\Auth;
use App\Domain\Enum\Account\MailProvider;
use App\Domain\Exception\AccountIdentityMismatch;
use App\Entity\User\User;
use App\Repository\Mail\AccountRepository;
use App\Service\Onboarding\OnboardingFlow;
use League\OAuth2\Client\Provider\Exception\IdentityProviderException;
use Psr\Log\LoggerInterface;
@@ -35,8 +37,20 @@ class OAuthController extends AbstractController
private readonly MicrosoftOAuthErrorTranslator $microsoftErrorTranslator,
private readonly TranslatorInterface $translator,
private readonly LoggerInterface $logger,
private readonly AccountRepository $accounts,
) {}
/**
* The session key holding "this handshake is a repair, not a new account".
*
* Kept beside the state rather than passed through the provider's redirect:
* anything sent to the provider comes back under the user's control, and an
* account id that could be edited in the address bar is an invitation to
* write somebody else's tokens onto a row of your choosing. The id never
* leaves the server, and ownership is re-checked when it is read back.
*/
private const string SESSION_RECONNECT = 'oauth2_reconnect_account';
#[Route('/{provider}/connect', name: 'connect', methods: ['GET'])]
public function connect(string $provider, Request $request): RedirectResponse
{
@@ -52,6 +66,29 @@ class OAuthController extends AbstractController
$this->stateStore->remember($request->getSession(), self::SESSION_NAMESPACE, $client->getState());
// Reconnect mode. The health page sends the account it wants repaired;
// everything else omits it and gets the original find-or-create
// behaviour unchanged.
//
// The id is stored only after ownership has been proven, so the value
// read back in the callback is known to be this user's — the callback
// still re-checks, because a session outlives a login in ways that are
// not worth reasoning about at a distance.
$session = $request->getSession();
$session->remove(self::SESSION_RECONNECT);
$reconnect = $request->query->getInt('reconnect');
if (0 !== $reconnect) {
$account = $this->accounts->find($reconnect);
if (null === $account || $account->usr !== $this->getUser()) {
throw $this->createAccessDeniedException();
}
$session->set(self::SESSION_RECONNECT, (int) $account->id);
}
return new RedirectResponse($authUrl);
}
@@ -139,9 +176,51 @@ class OAuthController extends AbstractController
/** @var User $user */
$user = $this->getUser();
$this->accountLinker->link($user, $mailProvider, $email, $token);
$session = $request->getSession();
$reconnect = $session->get(self::SESSION_RECONNECT);
return $this->redirectToRoute($this->landingRoute());
// Single-use, like the state itself: a repair intent left in the
// session would be picked up by the next ordinary "add an account"
// handshake and silently turn it into an overwrite.
$session->remove(self::SESSION_RECONNECT);
if (false === is_int($reconnect)) {
$this->accountLinker->link($user, $mailProvider, $email, $token);
return $this->redirectToRoute($this->landingRoute());
}
$account = $this->accounts->find($reconnect);
if (null === $account || $account->usr !== $user) {
throw $this->createAccessDeniedException();
}
try {
$this->accountLinker->relink($account, $mailProvider, $email, $token);
} catch (AccountIdentityMismatch $e) {
// Refused, not resolved. Falling back to link() here would create a
// second account for the address they actually signed in as, which
// is a surprising thing to do to somebody who pressed "Reconnect"
// — and it would leave the broken one broken.
$this->logger->warning('Reconnect refused: the authorised mailbox is not this account', [
'accountId' => $account->id,
'provider' => $provider,
]);
$this->addFlash('error', $this->translator->trans('settings.health.reconnect.wrong_account', [
'%expected%' => $e->expected,
'%actual%' => $e->actual,
]));
return $this->redirectToRoute('app_settings_index', ['section' => 'health']);
}
$this->addFlash('success', $this->translator->trans('settings.health.reconnect.done', [
'%account%' => $account->email,
]));
return $this->redirectToRoute('app_settings_index', ['section' => 'health']);
}
// ── Private ───────────────────────────────────────────────────────────────
@@ -0,0 +1,197 @@
<?php
declare(strict_types=1);
namespace App\Controller\Settings;
use App\Entity\Calendar\Calendar;
use App\Entity\Mail\Account;
use App\Infrastructure\Messaging\Message\SyncCalendarMessage;
use App\Service\Monitoring\QueueMonitor;
use Doctrine\ORM\EntityManagerInterface;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Messenger\MessageBusInterface;
use Symfony\Component\Routing\Attribute\Route;
use Symfony\Component\Security\Http\Attribute\IsGranted;
use Symfony\Contracts\Translation\TranslatorInterface;
/**
* The repairs offered on the health page.
*
* Only the ones that had no home already. Push re-subscription belongs to
* AccountPushController and the integration reconnect belongs to the
* integration OAuth flow; the health page links to both rather than growing
* duplicates, because a second endpoint doing the same thing is a second
* endpoint to get the ownership check right on.
*
* ── What every action here promises ──────────────────────────────────────────
* Ownership is checked before anything is read, CSRF before anything is
* written, and each one reports what actually happened rather than assuming it
* worked. The user is repairing credentialed access to their own mail; an
* action here that guessed would be guessing with their mailbox.
*
* The reconnect itself is NOT here — it is a redirect into the existing OAuth
* flow (see OAuthController::connect, reconnect mode), because the round trip
* through the provider cannot be a POST from a page.
*/
#[Route('/settings/health', name: 'app_health_')]
#[IsGranted('ROLE_USER')]
final class AccountHealthController extends AbstractController
{
public function __construct(
private readonly EntityManagerInterface $em,
private readonly MessageBusInterface $bus,
private readonly QueueMonitor $queueMonitor,
private readonly TranslatorInterface $translator,
) {
}
/**
* Start a reconnect for one account.
*
* A GET that redirects, because its destination is the provider's consent
* screen. It writes nothing itself — the account is not touched until the
* callback comes back with a token AND the address has been checked — so
* there is no state change here for a CSRF token to protect. The ownership
* check still runs, so this cannot be used to discover whose account an id
* belongs to.
*/
#[Route('/reconnect/{id}', name: 'reconnect', methods: ['GET'])]
public function reconnect(Account $account): RedirectResponse
{
$this->denyUnlessOwner($account);
$provider = $account->oauthProvider;
if (null === $provider) {
// A password account has nothing to re-consent to; its repair is
// the ordinary edit form. Reaching this by hand is not worth a
// friendly page.
throw $this->createNotFoundException();
}
return $this->redirectToRoute('app_oauth_connect', [
'provider' => $provider,
'reconnect' => $account->id,
]);
}
/**
* Ask one calendar to try again now.
*
* Dispatched rather than run inline, matching CalendarSyncCommand: a
* hand-triggered sync that took a different path from the scheduled one
* would have different failure behaviour, which is how "it works when I do
* it myself" becomes a support thread.
*
* The backoff is cleared first, and that is as much the point of the button
* as the dispatch is. The handler itself does not check the window — a push
* notification or a hand-run sync is a reason to try whatever the schedule
* says — so the dispatch alone would work. What it would not do is undo the
* accumulated schedule: without the reset, a calendar that had climbed to a
* day-long window would attempt once now and then go quiet for another day,
* even if the user had just repaired the thing that was breaking it.
*/
#[Route('/calendar/{id}/resync', name: 'calendar_resync', methods: ['POST'])]
public function resyncCalendar(Request $request, Calendar $calendar): RedirectResponse
{
if ($calendar->usr !== $this->getUser()) {
throw $this->createAccessDeniedException();
}
$this->assertToken($request, 'health_calendar_' . $calendar->id);
$calendar->clearSyncBackoff();
$this->em->flush();
$this->bus->dispatch(new SyncCalendarMessage((int) $calendar->id));
$this->addFlash('success', $this->translator->trans('settings.health.flash.calendar_resync', [
'%calendar%' => $calendar->name,
]));
return $this->back();
}
/**
* Put abandoned background work back on the queue.
*
* Admin-gated for the reason AccountHealthInspector::abandonedWork()
* documents: the failure transport is instance-wide, so this is not a
* per-user action and must not be offered as one.
*
* Safe by nature — retrying work that fails again simply lands back where
* it was — which is why it is the primary of the two queue buttons.
*/
#[Route('/queue/retry', name: 'queue_retry', methods: ['POST'])]
#[IsGranted('ROLE_ADMIN')]
public function retryQueue(Request $request): RedirectResponse
{
$this->assertToken($request, 'health_queue_retry');
$retried = $this->queueMonitor->retryAll();
// The count is reported rather than a bare "done": retrying nothing and
// retrying four hundred things look identical otherwise, and the first
// usually means somebody else got there first.
$this->addFlash('success', $this->translator->trans('settings.health.flash.queue_retried', [
'%count%' => $retried,
]));
return $this->back();
}
/**
* Throw abandoned work away without retrying it.
*
* Destructive, and rendered apart from the safe repairs with that said in
* words. There is no undo — the envelopes are gone — so the page states
* that before the button rather than after it.
*/
#[Route('/queue/discard', name: 'queue_discard', methods: ['POST'])]
#[IsGranted('ROLE_ADMIN')]
public function discardQueue(Request $request): RedirectResponse
{
$this->assertToken($request, 'health_queue_discard');
$purged = $this->queueMonitor->purgeAll();
$this->addFlash('success', $this->translator->trans('settings.health.flash.queue_discarded', [
'%count%' => $purged,
]));
return $this->back();
}
// ── Private ───────────────────────────────────────────────────────────────
/**
* Per-action tokens, the way AliasController::assertToken() does it.
*
* One id per action per subject rather than the shared `ajax` token: these
* actions touch account credentials and background work, and a single token
* good for all of them makes any one XSS worth every one of them.
*/
private function assertToken(Request $request, string $id): void
{
$token = (string) ($request->request->get('_token') ?? $request->headers->get('X-CSRF-Token') ?? '');
if (false === $this->isCsrfTokenValid($id, $token)) {
throw $this->createAccessDeniedException();
}
}
private function denyUnlessOwner(Account $account): void
{
if ($account->usr !== $this->getUser()) {
throw $this->createAccessDeniedException();
}
}
private function back(): RedirectResponse
{
return $this->redirectToRoute('app_settings_index', ['section' => 'health']);
}
}
+43 -1
View File
@@ -24,6 +24,7 @@ use App\Repository\User\PushSubscriptionRepository;
use App\Repository\Label\LabelRepository;
use App\Repository\Rule\MailRuleRepository;
use App\Service\Calendar\Subscription\CalendarSourceLister;
use App\Service\Health\AccountHealthInspector;
use App\Service\Push\PushSubscriptionRegistry;
use Symfony\Component\DependencyInjection\Attribute\Autowire;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
@@ -36,7 +37,19 @@ use Symfony\Component\Security\Http\Attribute\IsGranted;
#[IsGranted('ROLE_USER')]
final class SettingsController extends AbstractController
{
private const array SECTIONS = ['accounts', 'profile', 'security', 'labels', 'calendars', 'sharing', 'filters', 'integrations', 'appearance', 'aliases', 'signature', 'app-passwords', 'notifications', 'general'];
/**
* `health` leads deliberately.
*
* It is not a settings pane — nothing on it is a preference — but it is
* where the topbar indicator points and where the OAuth reconnect returns
* to, so it needs a stable section of its own. It gets one rather than a
* block on `accounts` for two reasons: the problems it lists are not all
* account problems (calendars, file-store connections and abandoned
* background work all appear there), and a variable-height alert stack at
* the top of the accounts pane would push the actual account controls below
* the fold on exactly the installs that have something wrong.
*/
private const array SECTIONS = ['health', 'accounts', 'profile', 'security', 'labels', 'calendars', 'sharing', 'filters', 'integrations', 'appearance', 'aliases', 'signature', 'app-passwords', 'notifications', 'general'];
public function __construct(
private readonly AccountRepository $accountRepository,
@@ -59,6 +72,7 @@ final class SettingsController extends AbstractController
private readonly SecuritySectionViewData $securitySection,
private readonly UserTimezoneResolver $timezones,
private readonly ClockFormatResolver $clocks,
private readonly AccountHealthInspector $healthInspector,
) {
}
@@ -102,9 +116,37 @@ final class SettingsController extends AbstractController
?? AppLocale::tryFromRequest($this->defaultLocale)
?? AppLocale::English,
...$this->timezoneSectionData($section),
...$this->healthSectionData($section),
]);
}
/**
* The health report, and only when that section is on screen.
*
* The same rule the section methods below follow, and it earns its keep
* more than most: this is the one call on the page that reads the failure
* transport, which means deserialising envelopes rather than counting rows.
* Paying that on the labels tab would be paying it on every settings visit.
*
* The infrastructure half is asked for only for an admin — see
* AccountHealthInspector::abandonedWork() on why the queue is not a
* per-user question — so a non-admin never triggers that read at all.
*
* @return array<string, mixed>
*/
private function healthSectionData(string $section): array
{
$user = $this->getUser();
if ('health' !== $section || false === $user instanceof User) {
return [];
}
return [
'healthReport' => $this->healthInspector->inspect($user, $this->isGranted('ROLE_ADMIN')),
];
}
/**
* Where calendars can be subscribed from, and only when that section is on
* screen — asking the driver registry about every account is cheap but not
+75
View File
@@ -0,0 +1,75 @@
<?php
declare(strict_types=1);
namespace App\Domain\DTO\Health;
use App\Domain\Enum\Health\HealthIssueKind;
use App\Domain\Enum\Health\HealthSeverity;
/**
* One thing that is wrong, phrased for the person who has to fix it.
*
* Holds translation KEYS and their parameters, never rendered text: the
* inspector runs in workers and commands as well as in a request, and a service
* that formats English is a service that cannot be reused by the German one.
*
* ── On $causedBy ─────────────────────────────────────────────────────────────
* The live install this was built from had ONE dead Google grant producing
* three failing calendars, five hundred dead-lettered jobs and five thousand
* log lines. Listing those as six equal emergencies would be an honest reading
* of the stored state and a useless thing to show somebody. An issue that is
* downstream of another names it here, so the surface can nest it under its
* cause and the indicator can count causes rather than symptoms.
*/
final readonly class HealthIssue
{
/**
* @param string $id stable per subject, so a repair
* can be addressed and a test can
* assert on one card
* @param string $subject the account address, calendar or
* connection name this is about —
* already user-facing, never a key
* @param array<string, mixed> $titleParams
* @param array<string, mixed> $bodyParams
* @param list<HealthRepair> $repairs safe first; destructive ones flagged
* @param string|null $causedBy id of the issue this follows from
* @param string|null $detail the stored provider message, shown
* only behind a disclosure — see
* MailProvider::calendarScopes(): a
* raw error code is not an answer,
* but hiding it entirely leaves a
* self-hoster with nothing to search
*/
public function __construct(
public string $id,
public HealthIssueKind $kind,
public HealthSeverity $severity,
public string $subject,
public array $titleParams = [],
public array $bodyParams = [],
public array $repairs = [],
public ?string $causedBy = null,
public ?string $detail = null,
) {
}
/** Whether this is a consequence of something else on the same page. */
public function isConsequence(): bool
{
return null !== $this->causedBy;
}
/** @return list<HealthRepair> */
public function safeRepairs(): array
{
return array_values(array_filter($this->repairs, static fn (HealthRepair $r): bool => false === $r->destructive));
}
/** @return list<HealthRepair> */
public function destructiveRepairs(): array
{
return array_values(array_filter($this->repairs, static fn (HealthRepair $r): bool => true === $r->destructive));
}
}
+61
View File
@@ -0,0 +1,61 @@
<?php
declare(strict_types=1);
namespace App\Domain\DTO\Health;
/**
* One button on a health card: what it will do, and where pressing it goes.
*
* Carries a promise as well as a label. The brief this was built to is explicit
* that a repair states what it will do BEFORE doing it, and a label alone
* cannot — "Reconnect" does not say that the mail already downloaded stays put,
* which is the exact fear that makes people delete the account and add it back
* instead. So every repair has a promiseKey, and the card renders it.
*
* GET repairs are links, POST repairs are forms with a token. That split is not
* cosmetic: the OAuth repairs must leave the app entirely and come back, so
* they cannot be fetch() calls, while everything else changes state here and
* must therefore be a POST that carries CSRF.
*/
final readonly class HealthRepair
{
/**
* @param string $route Symfony route name
* @param array<string, mixed> $routeParams
* @param string $labelKey the button's text
* @param string $promiseKey one sentence: what pressing it does
* @param array<string, mixed> $promiseParams placeholders for the promise.
* Its own array rather than reusing
* the issue's body params: the
* reconnect promise names the
* mailbox you must sign in as,
* which is a different sentence
* from the one describing what
* broke, and rendering it with no
* parameters printed a literal
* "%account%" on the page
* @param string|null $csrfTokenId non-null for POST repairs; the id
* the controller validates against
* @param bool $destructive throws work away rather than
* retrying it — rendered apart from
* the safe repairs, never as the
* primary button
*/
public function __construct(
public string $route,
public array $routeParams,
public string $labelKey,
public string $promiseKey,
public array $promiseParams = [],
public ?string $csrfTokenId = null,
public bool $destructive = false,
) {
}
/** POST when it carries a token, GET when it does not. */
public function isPost(): bool
{
return null !== $this->csrfTokenId;
}
}
+158
View File
@@ -0,0 +1,158 @@
<?php
declare(strict_types=1);
namespace App\Domain\DTO\Health;
use App\Domain\Enum\Health\HealthSeverity;
/**
* Everything wrong with one user's accounts, at one moment.
*
* Sorted worst-first with consequences pulled in under their cause, so the
* template can render the list in order and needs no opinion of its own about
* what matters more.
*/
final readonly class HealthReport
{
/** @param list<HealthIssue> $issues */
private function __construct(
public array $issues,
) {
}
/**
* @param list<HealthIssue> $issues in any order
*/
public static function of(array $issues): self
{
return new self(self::arrange($issues));
}
public static function healthy(): self
{
return new self([]);
}
public function isHealthy(): bool
{
return [] === $this->issues;
}
/** The worst severity present, or null when there is nothing wrong. */
public function worstSeverity(): ?HealthSeverity
{
$worst = null;
foreach ($this->issues as $issue) {
if (null === $worst || $issue->severity->rank() < $worst->rank()) {
$worst = $issue->severity;
}
}
return $worst;
}
/**
* What the topbar counts.
*
* Root causes only, and only those worth interrupting for. The install this
* was built from would otherwise have shown "9" for what is, to the person
* looking at it, one broken Google account — and a number that overstates
* the problem is the fastest way to teach somebody the badge is noise.
*/
public function indicatorCount(): int
{
$count = 0;
foreach ($this->issues as $issue) {
if (true === $issue->isConsequence()) {
continue;
}
if (false === $issue->severity->warrantsIndicator()) {
continue;
}
++$count;
}
return $count;
}
/** The tone the indicator takes, counting only what it counts above. */
public function indicatorTone(): ?string
{
$worst = null;
foreach ($this->issues as $issue) {
if (true === $issue->isConsequence() || false === $issue->severity->warrantsIndicator()) {
continue;
}
if (null === $worst || $issue->severity->rank() < $worst->rank()) {
$worst = $issue->severity;
}
}
return $worst?->tone();
}
/**
* Worst first, with each cause immediately followed by the issues it
* explains.
*
* Consequences keep their cause's position rather than sorting on their own
* severity: a failing calendar listed three cards below the dead grant that
* caused it reads as a separate problem, which is exactly the confusion the
* causedBy link exists to prevent.
*
* @param list<HealthIssue> $issues
* @return list<HealthIssue>
*/
private static function arrange(array $issues): array
{
$roots = [];
$consequences = [];
foreach ($issues as $issue) {
if (true === $issue->isConsequence()) {
$consequences[(string) $issue->causedBy][] = $issue;
continue;
}
$roots[] = $issue;
}
// usort is not stable across equal ranks in a way worth relying on, so
// the comparator falls back to the id — a fixed order beats an
// arbitrary one when a test has to assert on it.
usort($roots, static function (HealthIssue $a, HealthIssue $b): int {
return [$a->severity->rank(), $a->id] <=> [$b->severity->rank(), $b->id];
});
$arranged = [];
foreach ($roots as $root) {
$arranged[] = $root;
foreach ($consequences[$root->id] ?? [] as $child) {
$arranged[] = $child;
}
unset($consequences[$root->id]);
}
// A consequence whose cause is not in the report is not a consequence
// any more — the cause was fixed, or was never this user's to see. It
// still has to be shown, or it vanishes with nothing having fixed it.
foreach ($consequences as $orphans) {
foreach ($orphans as $orphan) {
$arranged[] = $orphan;
}
}
return $arranged;
}
}
@@ -0,0 +1,90 @@
<?php
declare(strict_types=1);
namespace App\Domain\Enum\Health;
/**
* The kinds of thing that can be wrong with a user's accounts.
*
* Deliberately a closed set. The app already detects every one of these — the
* gap this feature closes is that it wrote them down and then never read them
* back — so a kind exists here only where there is a stored signal to read and
* a repair to offer. "Something in the logs looks odd" is not a kind: it has no
* repair, and an issue with no button is a worry with no way to act on it.
*
* Each kind names its own translation keys rather than the inspector building
* them by hand. The strings are the entire feature — the user's problem is that
* nothing told them anything — so they get a stable, greppable home.
*/
enum HealthIssueKind: string
{
/**
* The stored refresh token no longer works. Mail, calendar and every
* background job for the account stop together, which is why this is the
* one kind that can be the sole cause of thousands of log lines.
*/
case AccountReconnect = 'account_reconnect';
/**
* A calendar whose sync answers the same way every time — see
* CalendarSyncPermanentException. Surfaced per calendar rather than rolled
* into the account, because "reconnect and allow calendar access" and
* "reconnect" are different repairs and the user has to know which
* calendars went dark.
*/
case CalendarSyncFailing = 'calendar_sync_failing';
/**
* Registered for push but nothing is arriving, so the account is really on
* polling. Mail still lands; it lands late.
*/
case PushDegraded = 'push_degraded';
/** A file-store connection whose token could not be renewed. */
case IntegrationReconnect = 'integration_reconnect';
/** Background work that exhausted its retries and was put aside. */
case QueueWorkAbandoned = 'queue_work_abandoned';
/**
* The severity a kind carries when nothing else is known about it.
*
* The inspector may lower it — a calendar that is failing only because its
* account's grant is dead is a consequence, not a second emergency — but it
* never raises it.
*/
public function defaultSeverity(): HealthSeverity
{
return match ($this) {
self::AccountReconnect => HealthSeverity::Critical,
self::CalendarSyncFailing => HealthSeverity::Critical,
self::IntegrationReconnect => HealthSeverity::Warning,
self::QueueWorkAbandoned => HealthSeverity::Warning,
self::PushDegraded => HealthSeverity::Notice,
};
}
/** Headline — what is wrong, in the user's terms. */
public function titleKey(): string
{
return 'settings.health.issue.' . $this->value . '.title';
}
/** What it means for their mail, and what they will get back by fixing it. */
public function bodyKey(): string
{
return 'settings.health.issue.' . $this->value . '.body';
}
public function icon(): string
{
return match ($this) {
self::AccountReconnect => 'fa-link-slash',
self::CalendarSyncFailing => 'fa-calendar-xmark',
self::PushDegraded => 'fa-bolt-slash',
self::IntegrationReconnect => 'fa-plug-circle-exclamation',
self::QueueWorkAbandoned => 'fa-inbox',
};
}
}
+94
View File
@@ -0,0 +1,94 @@
<?php
declare(strict_types=1);
namespace App\Domain\Enum\Health;
/**
* How badly a health issue is hurting the user right now.
*
* Three levels, and the boundaries are drawn by consequence rather than by how
* alarming the underlying error looks:
*
* - Critical: mail or calendar data is NOT arriving, and will not until the
* user does something. A dead OAuth grant is the archetype.
* - Warning: something is broken but the data still flows, or the loss is
* bounded and already over — abandoned background work, an integration that
* only matters when you reach for it.
* - Notice: working, more slowly or less directly than it should. Push having
* fallen back to polling is the whole reason this level exists.
*
* The distinction is load-bearing, not decorative. A health page that paints
* "your mail is slightly delayed" the same red as "your mail has stopped" is a
* page people learn to close, and then the red that mattered is unread too.
* Notice therefore never lights the topbar indicator — see AccountHealthGlobal.
*/
enum HealthSeverity: string
{
case Critical = 'critical';
case Warning = 'warning';
case Notice = 'notice';
/**
* Sort weight, worst first. Used to order the health list and to pick the
* report's overall tone.
*/
public function rank(): int
{
return match ($this) {
self::Critical => 0,
self::Warning => 1,
self::Notice => 2,
};
}
/**
* Whether this level is worth interrupting the user for.
*
* The topbar indicator is the only interruption this feature has, and it
* is spent on the levels that mean something is not working. See the class
* docblock on why Notice deliberately does not qualify.
*/
public function warrantsIndicator(): bool
{
return self::Notice !== $this;
}
/** The tone token the topbar and the cards share, so they cannot disagree. */
public function tone(): string
{
return match ($this) {
self::Critical => 'danger',
self::Warning => 'warn',
self::Notice => 'info',
};
}
/** Card accent — a left border and a tinted icon, in both themes. */
public function cardClasses(): string
{
return match ($this) {
self::Critical => 'border-l-4 border-l-red-500',
self::Warning => 'border-l-4 border-l-amber-500',
self::Notice => 'border-l-4 border-l-sky-500',
};
}
public function iconClasses(): string
{
return match ($this) {
self::Critical => 'text-red-500',
self::Warning => 'text-amber-500',
self::Notice => 'text-sky-500',
};
}
public function icon(): string
{
return match ($this) {
self::Critical => 'fa-circle-exclamation',
self::Warning => 'fa-triangle-exclamation',
self::Notice => 'fa-circle-info',
};
}
}
@@ -0,0 +1,35 @@
<?php
declare(strict_types=1);
namespace App\Domain\Exception;
use RuntimeException;
/**
* A reconnect came back authorised as somebody other than the account being
* repaired.
*
* Raised by OAuthAccountLinker::relink() and never recovered from — see that
* method's identity-guard note. The two addresses are carried so the callback
* can tell the user which one it got, because "that was the wrong account" is
* only actionable if you say which one you were signed in as.
*
* Its own exception type rather than a plain RuntimeException so the callback
* can distinguish it from the provider failing: one means "sign out of the
* other account and try again" and the other means "the provider is unhappy",
* and offering the wrong advice for either wastes the user's afternoon.
*/
final class AccountIdentityMismatch extends RuntimeException
{
public function __construct(
public readonly string $expected,
public readonly string $actual,
) {
parent::__construct(sprintf(
'Reconnect authorised %s, but this account is %s. Refusing to re-point it.',
$actual,
$expected,
));
}
}
+143 -3
View File
@@ -157,6 +157,58 @@ class Calendar
#[ORM\Column(type: Types::TEXT, nullable: true)]
public ?string $lastSyncError = null;
/**
* How many times in a row this calendar's sync has failed. Reset to zero by
* the first success.
*
* Only ever read to work out how long to wait before the next attempt —
* it is not a statistic and nothing renders it.
*/
#[ORM\Column(options: ['default' => 0])]
public int $syncFailureCount = 0;
/**
* Do not attempt this calendar again before this moment. Null when it is
* healthy, or when a repair has just asked for an immediate retry.
*
* ── Why this column exists ───────────────────────────────────────────────
* The sweep asks findDueForSync() for calendars whose lastSyncedAt is old,
* and a calendar that CANNOT sync never updates lastSyncedAt — so it is due
* again fifteen minutes later, and again, permanently. On the install this
* was built from, one expired Google sign-in produced 2 193 identical error
* lines and 503 dead-lettered jobs from three calendars over two days.
*
* That is a defect on its own terms and not only a tidiness problem: the
* one log line that says what is wrong is worth nothing when it is repeated
* every quarter of an hour, because the volume is what makes people stop
* reading the log. Retrying something at the same rate forever is also not
* a retry policy — it is the absence of one.
*
* Note what this is deliberately NOT: a reclassification of the error.
* GoogleCalendarApiClient::token() refuses to call a refused refresh
* permanent, because a slow token endpoint arrives as the same object, and
* that judgement is correct. Backing off is the answer that does not
* require telling the two apart — it slows down without ever giving up.
*/
#[ORM\Column(nullable: true)]
public ?DateTimeImmutable $syncBackoffUntil = null;
/**
* Whether the failure just recorded was news, as opposed to the same thing
* this calendar is already backing off through.
*
* NOT a column, and deliberately so — it is true of one attempt, not of the
* calendar, and persisting it would invite somebody to read it a request
* later when it means nothing. It exists because recordSyncFailure() is
* called from a catch block that must rethrow, so its answer cannot be
* returned up the stack to SyncCalendarHandler, which is what needs it.
*
* Lives on the entity rather than on the sync service because the service
* is a singleton the FrankenPHP worker keeps alive across requests: state
* parked there would be read back by whichever calendar came next.
*/
public bool $syncFailureWasNews = true;
/**
* Free-form per-calendar preferences, mirroring Account::$settings — the
* long tail that does not deserve a column. Readers assume their defaults
@@ -276,12 +328,100 @@ class Calendar
{
$this->lastSyncedAt = new DateTimeImmutable();
$this->lastSyncError = null;
// One success is enough to earn the full rate back. Decaying the count
// instead would leave a calendar that recovered still being polled
// hourly, with nothing on screen to explain why it looks stale.
$this->syncFailureCount = 0;
$this->syncBackoffUntil = null;
}
/** Truncated like Integration::recordFailure(): a provider stack trace is not a message. */
public function recordSyncFailure(string $reason): void
/**
* Record a failure and decide how long to wait, answering whether this one
* is worth a log line.
*
* The answer is true when the failure is NEWS — the first of a run, a
* different message from the one already stored, or the first attempt after
* a backoff window has expired. It is false only while the calendar is
* inside a window it is already backing off through, which is exactly the
* repetition that produced two thousand identical lines.
*
* The first occurrence is therefore never suppressed: on a healthy calendar
* syncBackoffUntil is null, so the first failure always reports. Nor can a
* changed condition hide behind an old one — a different message reports
* immediately, whatever the window says.
*
* Truncated like Integration::recordFailure(): a provider stack trace is
* not a message.
*/
public function recordSyncFailure(string $reason, ?DateTimeImmutable $now = null): bool
{
$this->lastSyncError = mb_substr($reason, 0, 500);
$now = $now ?? new DateTimeImmutable();
$truncated = mb_substr($reason, 0, 500);
$isNews = null === $this->syncBackoffUntil
|| $this->syncBackoffUntil <= $now
|| $this->lastSyncError !== $truncated;
$this->lastSyncError = $truncated;
$this->syncFailureWasNews = $isNews;
// Counted per reported failure rather than per attempt. Messenger
// retries the same envelope several times, and letting each attempt
// advance the schedule would take a calendar from "wait a quarter of an
// hour" to "wait a day" on a single bad afternoon.
if (true === $isNews) {
++$this->syncFailureCount;
$this->syncBackoffUntil = $now->modify(sprintf('+%d seconds', $this->backoffSeconds()));
}
return $isNews;
}
/**
* Try again now, whatever the schedule said.
*
* The repair button's half of the bargain. A calendar that has backed off
* is not due, so dispatching a sync without this would be answered by a
* handler that declines to run — a button that visibly does nothing.
*
* lastSyncError is deliberately left alone: it is still true until a sync
* succeeds, and clearing it here would blank the health page the instant
* the button was pressed, whether or not anything got better.
*/
public function clearSyncBackoff(): void
{
$this->syncFailureCount = 0;
$this->syncBackoffUntil = null;
}
/** Whether this calendar is inside a window it agreed to wait out. */
public function isBackingOff(?DateTimeImmutable $now = null): bool
{
if (null === $this->syncBackoffUntil) {
return false;
}
return $this->syncBackoffUntil > ($now ?? new DateTimeImmutable());
}
/**
* How long to wait after the nth consecutive failure.
*
* Doubling from a quarter of an hour, capped at a day. The cap is the part
* that matters: without it a calendar that failed for a fortnight would
* next be tried a fortnight later, so a grant the user repaired this
* morning would sit dark until some time next month. A day means the worst
* case for a condition that silently fixed itself is one day, while a
* permanently dead calendar costs one attempt and one log line per day
* instead of ninety-six.
*/
private function backoffSeconds(): int
{
$steps = [900, 1800, 3600, 7200, 14400, 28800, 86400];
$index = max(0, $this->syncFailureCount - 1);
return $steps[min($index, count($steps) - 1)];
}
/**
@@ -55,10 +55,28 @@ final readonly class SyncCalendarHandler
try {
$touched = $this->sync->sync($calendar);
} catch (CalendarSyncException $e) {
$this->logger->error('CalendarSync: sync failed', [
'calendarId' => $calendar->id,
'error' => $e->getMessage(),
]);
// Logged when the failure is NEWS, not on every attempt. The
// service has just written the failure down, and
// Calendar::recordSyncFailure() left its answer on the entity: true
// for the first failure of a run, for a message that differs from
// the stored one, and for the first attempt after a backoff window
// expires — false only while this calendar is repeating something
// it is already backing off through.
//
// The first occurrence is never suppressed, and neither is a
// CHANGE. A calendar that was failing on a dead sign-in and starts
// failing on a deleted remote says so immediately, because the
// stored message no longer matches. That is the case a plain "log
// once per hour" throttle would have hidden, and it is the one that
// means something different has gone wrong.
if (true === $calendar->syncFailureWasNews) {
$this->logger->error('CalendarSync: sync failed', [
'calendarId' => $calendar->id,
'error' => $e->getMessage(),
'failures' => $calendar->syncFailureCount,
'nextTry' => $calendar->syncBackoffUntil?->format(DATE_ATOM),
]);
}
throw $e;
}
+18 -1
View File
@@ -165,17 +165,34 @@ class CalendarRepository extends ServiceEntityRepository
* hundred jobs into a queue that also carries mail. The remainder is picked
* up by the next sweep, which is fifteen minutes away.
*
* ── The backoff arm ──────────────────────────────────────────────────────
* A calendar that cannot sync never updates lastSyncedAt, so on the two
* criteria above it is due again on every single sweep, for as long as it
* stays broken — which is how three calendars produced 2 193 identical
* error lines in two days on the install this was written for. Calendars
* inside a backoff window are excluded here, at the point the work is
* created, rather than being dispatched and then declined: a job that is
* queued only to do nothing still costs a worker, still carries a retry
* policy, and still ends up in the failure transport.
*
* `syncBackoffUntil IS NULL` is an explicit arm for the same reason the
* lastSyncedAt one is — a null compares as unknown, not as true, and
* without it this would skip every healthy calendar and sweep only the
* broken ones.
*
* @return list<Calendar>
*/
public function findDueForSync(DateTimeImmutable $before, int $limit = 200): array
public function findDueForSync(DateTimeImmutable $before, int $limit = 200, ?DateTimeImmutable $now = null): array
{
return $this->createQueryBuilder('calendar')
->addSelect('COALESCE(calendar.lastSyncedAt, :epoch) AS HIDDEN dueSince')
->where('calendar.role = :remote')
->andWhere('calendar.remoteId IS NOT NULL')
->andWhere('calendar.lastSyncedAt IS NULL OR calendar.lastSyncedAt < :before')
->andWhere('calendar.syncBackoffUntil IS NULL OR calendar.syncBackoffUntil <= :now')
->setParameter('remote', CalendarRole::Remote)
->setParameter('before', $before)
->setParameter('now', $now ?? new DateTimeImmutable())
->setParameter('epoch', new DateTimeImmutable('@0'))
->orderBy('dueSince', 'ASC')
->addOrderBy('calendar.id', 'ASC')
@@ -271,6 +271,10 @@ final readonly class CalendarSyncService
*/
private function recordFailure(Calendar $calendar, \Throwable $e): void
{
// recordSyncFailure() also answers "is this worth saying out loud",
// and leaves that answer on the calendar as $syncFailureWasNews for
// SyncCalendarHandler to read. It is not returned up the stack because
// this is called from a catch block whose job is to rethrow.
$calendar->recordSyncFailure($e->getMessage());
if (false === $this->em->isOpen()) {
@@ -0,0 +1,412 @@
<?php
declare(strict_types=1);
namespace App\Service\Health;
use App\Domain\DTO\Health\HealthIssue;
use App\Domain\DTO\Health\HealthReport;
use App\Domain\DTO\Health\HealthRepair;
use App\Domain\Enum\Account\AuthType;
use App\Domain\Enum\Calendar\CalendarRole;
use App\Domain\Enum\Health\HealthIssueKind;
use App\Domain\Enum\Health\HealthSeverity;
use App\Domain\Enum\PushHealth;
use App\Entity\Calendar\Calendar;
use App\Entity\Integration\Integration;
use App\Entity\Mail\Account;
use App\Entity\User\User;
use App\Repository\Calendar\CalendarRepository;
use App\Repository\Integration\IntegrationRepository;
use App\Repository\Mail\AccountRepository;
use App\Service\Monitoring\QueueMonitor;
use App\Service\Push\PushSubscriptionRegistry;
/**
* What is wrong with this user's accounts, and what would fix it.
*
* ── Why this exists ──────────────────────────────────────────────────────────
* Every condition below was ALREADY detected and ALREADY stored before this
* class was written. Account::$oauthLastRefreshError is written by
* OAuthTokenManager and was read by no template and no controller;
* Calendar::$lastSyncError is written by CalendarSyncService and only ever
* rendered as a line of grey text; IntegrationTokenManager's docblock says the
* settings list "can say reconnect this" and nothing did. The install this was
* built from had one dead Google grant that had been stopping mail for two
* days, and the only place it appeared was five thousand log lines an admin
* would have had to go looking for.
*
* So this reads, it does not detect. Nothing here calls a provider, opens a
* socket or refreshes a token — it is a handful of indexed queries over state
* the sync layer already wrote, which is what lets it back a Twig global that
* renders on every authenticated page.
*
* ── The false-positive rule ──────────────────────────────────────────────────
* A health page that cries wolf is a health page with a "don't show this again"
* checkbox six months later. Every check below fires on a STORED FAILURE, never
* on an inference: a calendar with no lastSyncError is healthy even if it has
* never synced (it may mirror nothing yet), and an account with no
* oauthLastRefreshError is healthy even if it has not been refreshed in a
* month (nothing needed refreshing). The one check that reads a derived state
* — push — asks PushSubscriptionRegistry, which owns that judgement already.
*/
final readonly class AccountHealthInspector
{
public function __construct(
private AccountRepository $accounts,
private CalendarRepository $calendars,
private IntegrationRepository $integrations,
private PushSubscriptionRegistry $pushRegistry,
private QueueMonitor $queueMonitor,
) {
}
/**
* @param bool $includeInfrastructure whether to look at the failure
* transport, which is instance-wide
* rather than per-user — see
* abandonedWork()
*/
public function inspect(User $user, bool $includeInfrastructure = false): HealthReport
{
$issues = [];
// Read once and iterated twice. This backs a Twig global that renders on
// every authenticated page, so asking the repository a second time for
// the same rows would be a second query per page for nothing.
$accounts = $this->accounts->findForUserOrdered($user);
// Accounts first, and their ids kept, because everything else on the
// page may turn out to be downstream of one of them.
$deadGrants = [];
foreach ($accounts as $account) {
$issue = $this->accountGrant($account);
if (null === $issue) {
continue;
}
$issues[] = $issue;
$deadGrants[(int) $account->id] = $issue->id;
}
// A separate pass rather than the same loop: push is only asked about
// once the dead grants are known, because an account with no working
// token has no push worth reporting separately.
foreach ($accounts as $account) {
$issue = $this->push($account);
if (null !== $issue) {
$issues[] = $issue;
}
}
foreach ($this->calendars->findForUser($user) as $calendar) {
$issue = $this->calendar($calendar, $deadGrants);
if (null !== $issue) {
$issues[] = $issue;
}
}
foreach ($this->integrations->findForUserOrdered($user) as $integration) {
$issue = $this->integration($integration);
if (null !== $issue) {
$issues[] = $issue;
}
}
if (true === $includeInfrastructure) {
$issue = $this->abandonedWork();
if (null !== $issue) {
$issues[] = $issue;
}
}
return HealthReport::of($issues);
}
// ── Private ───────────────────────────────────────────────────────────────
/**
* The dead grant: the provider will not renew the sign-in any more.
*
* Fires on the stored error alone. OAuthTokenManager writes it on every
* failed refresh and clears it on every successful one, so a non-null value
* means the most recent attempt failed — which is precisely the condition
* where nothing will work until the user re-consents.
*
* A missing refresh token is the same problem arriving by a different road
* (OAuthTokenManager::refresh() throws for it before it ever reaches the
* provider), so it is folded in here rather than given a kind of its own:
* the sentence shown and the button offered would have been identical.
*/
private function accountGrant(Account $account): ?HealthIssue
{
if (AuthType::OAuth2->value !== $account->authType) {
return null;
}
$stored = $account->oauthLastRefreshError;
$missing = null === $account->oauthRefreshToken;
if (null === $stored && false === $missing) {
return null;
}
return new HealthIssue(
id: 'account-' . $account->id,
kind: HealthIssueKind::AccountReconnect,
severity: HealthSeverity::Critical,
subject: $account->email,
titleParams: ['%account%' => $account->email],
bodyParams: ['%account%' => $account->email],
repairs: [
new HealthRepair(
route: 'app_health_reconnect',
routeParams: ['id' => $account->id],
labelKey: 'settings.health.repair.reconnect.label',
promiseKey: 'settings.health.repair.reconnect.promise',
// The promise names the mailbox the user must sign in as.
// That is the sentence the identity guard exists to make
// good on, so it has to say WHICH address rather than
// "the same one".
promiseParams: ['%account%' => $account->email],
),
],
// Shown behind a disclosure only. MailProvider::calendarScopes()
// makes the case: "invalid_grant" is not something to put in front
// of a person as an explanation, but a self-hoster comparing notes
// with a forum post needs to be able to find it.
detail: $stored,
);
}
/**
* A calendar that has stopped filling in.
*
* Attributed to the account's dead grant when there is one. Three calendars
* failing because one Google sign-in expired is one problem with three
* symptoms, and the repair for all three is the single reconnect button on
* the account above — offering three more of them would imply three
* separate round trips through Google.
*
* Still listed rather than swallowed: the user's question is "why is my
* Feiertage calendar empty", and an account-level card alone does not
* answer it.
*
* @param array<int, string> $deadGrants account id => the issue id naming it
*/
private function calendar(Calendar $calendar, array $deadGrants): ?HealthIssue
{
if (null === $calendar->lastSyncError) {
return null;
}
if (CalendarRole::Remote !== $calendar->role) {
return null;
}
$accountId = $calendar->account?->id;
$causedBy = null !== $accountId ? ($deadGrants[(int) $accountId] ?? null) : null;
// A consequence is not a second emergency. It keeps its place in the
// list under its cause, but it does not add to the count in the topbar
// and it does not paint another red card.
$severity = null !== $causedBy ? HealthSeverity::Warning : HealthSeverity::Critical;
$repairs = [];
if (null === $causedBy) {
// Only offer a retry where a retry could plausibly work. When the
// grant is dead, "try again" is a button whose entire behaviour is
// to fail, and the reconnect above is the real answer.
$repairs[] = new HealthRepair(
route: 'app_health_calendar_resync',
routeParams: ['id' => $calendar->id],
labelKey: 'settings.health.repair.calendar_resync.label',
promiseKey: 'settings.health.repair.calendar_resync.promise',
csrfTokenId: 'health_calendar_' . $calendar->id,
);
}
return new HealthIssue(
id: 'calendar-' . $calendar->id,
kind: HealthIssueKind::CalendarSyncFailing,
severity: $severity,
subject: $calendar->name,
titleParams: ['%calendar%' => $calendar->name],
bodyParams: ['%calendar%' => $calendar->name],
repairs: $repairs,
causedBy: $causedBy,
detail: $calendar->lastSyncError,
);
}
/**
* Push registered, nothing arriving — so the account is really on polling.
*
* The quietest of the kinds on purpose. Mail still lands; it lands on the
* sweep's schedule instead of instantly, and telling somebody their mail
* has stopped when it has not is how a health page loses its credibility.
*
* Never reported for an account whose grant is dead: with no working token
* there is no push to speak of, and the reconnect is the repair for both.
*/
private function push(Account $account): ?HealthIssue
{
if (null !== $account->oauthLastRefreshError) {
return null;
}
if (PushHealth::Degraded !== $this->pushRegistry->health($account)) {
return null;
}
return new HealthIssue(
id: 'push-' . $account->id,
kind: HealthIssueKind::PushDegraded,
severity: HealthSeverity::Notice,
subject: $account->email,
titleParams: ['%account%' => $account->email],
bodyParams: ['%account%' => $account->email],
repairs: [
// The existing control, not a second one. AccountPushController
// already has the repair the accounts pane uses, under its own
// CSRF id; a parallel route here would be a second thing to
// keep correct for no gain.
new HealthRepair(
route: 'settings_account_push_repair',
routeParams: ['id' => $account->id],
labelKey: 'settings.health.repair.push_resubscribe.label',
promiseKey: 'settings.health.repair.push_resubscribe.promise',
csrfTokenId: 'account_push_' . $account->id,
),
],
);
}
/**
* A file-store connection whose token could not be renewed.
*
* IntegrationTokenManager already records the failure and already composes
* "X needs to be reconnected"; this is the list its docblock says should
* exist. The reconnect is the integration OAuth flow that is already there,
* which upserts onto the same row — see IntegrationTokenManager::
* storeAuthorization(), whose docblock is explicit that reconnecting
* updates rather than duplicates.
*/
private function integration(Integration $integration): ?HealthIssue
{
if (null === $integration->lastError) {
return null;
}
return new HealthIssue(
id: 'integration-' . $integration->id,
kind: HealthIssueKind::IntegrationReconnect,
severity: HealthSeverity::Warning,
subject: $integration->provider->label(),
titleParams: ['%connection%' => $integration->provider->label()],
bodyParams: ['%connection%' => $integration->provider->label()],
repairs: [
new HealthRepair(
route: 'app_integration_oauth_connect',
routeParams: ['provider' => $integration->provider->value],
labelKey: 'settings.health.repair.integration_reconnect.label',
promiseKey: 'settings.health.repair.integration_reconnect.promise',
),
],
detail: $integration->lastError,
);
}
/**
* Work that exhausted its retries and was put aside.
*
* Aggregated to a count, never a list. The install this was built from had
* five hundred and three of these and they were four distinct problems;
* rendering five hundred rows would bury that.
*
* ── Why this is admin-gated ──────────────────────────────────────────────
* The failure transport is instance-wide. Attributing an envelope to a user
* means deserialising it and walking whatever id it happens to carry back
* to an account, for every row — expensive, and wrong for any message that
* belongs to no user. Rather than guess, this is offered only to the role
* that can already see the whole queue and act on it, which on a
* self-hosted install is the person reading the page anyway. A non-admin
* loses nothing they could have acted on: the account card above already
* tells them their mail has stopped, which is their half of it.
*/
private function abandonedWork(): ?HealthIssue
{
$failed = $this->queueMonitor->failedMessages();
$count = count($failed);
if (0 === $count) {
return null;
}
return new HealthIssue(
id: 'queue-abandoned',
kind: HealthIssueKind::QueueWorkAbandoned,
severity: HealthSeverity::Warning,
subject: '',
titleParams: ['%count%' => $count],
bodyParams: ['%count%' => $count],
repairs: [
new HealthRepair(
route: 'app_health_queue_retry',
routeParams: [],
labelKey: 'settings.health.repair.queue_retry.label',
promiseKey: 'settings.health.repair.queue_retry.promise',
csrfTokenId: 'health_queue_retry',
),
new HealthRepair(
route: 'app_health_queue_discard',
routeParams: [],
labelKey: 'settings.health.repair.queue_discard.label',
promiseKey: 'settings.health.repair.queue_discard.promise',
csrfTokenId: 'health_queue_discard',
destructive: true,
),
],
detail: $this->summarise($failed),
);
}
/**
* The distinct failures behind the count, worst-repeated first.
*
* @param list<array{id: string, class: string, error: string|null, failedAt: mixed}> $failed
*/
private function summarise(array $failed): ?string
{
$tally = [];
foreach ($failed as $row) {
$class = $row['class'];
$short = substr((string) strrchr($class, '\\'), 1);
$key = ('' !== $short ? $short : $class) . ': ' . ($row['error'] ?? '—');
$tally[$key] = ($tally[$key] ?? 0) + 1;
}
if ([] === $tally) {
return null;
}
arsort($tally);
$lines = [];
foreach (array_slice($tally, 0, 5, true) as $key => $n) {
$lines[] = $n . ' × ' . $key;
}
return implode("\n", $lines);
}
}
+86
View File
@@ -6,6 +6,7 @@ namespace App\Service\OAuth;
use App\Domain\Enum\Account\AuthType;
use App\Domain\Enum\Account\MailProvider;
use App\Domain\Exception\AccountIdentityMismatch;
use App\Entity\Mail\Account;
use App\Entity\User\User;
use App\Repository\Mail\AccountRepository;
@@ -54,6 +55,91 @@ final readonly class OAuthAccountLinker
return $account;
}
/**
* Write a freshly authorised token onto an account that already exists,
* keeping everything else about it.
*
* This is the repair for a dead grant, and the whole point of it is what it
* does NOT do: no row is created, none is deleted, and nothing that hangs
* off the account id is touched. Mail, threads, mailboxes, labels, rules,
* aliases, calendars and per-account settings all key on that id and all
* survive, which is what makes this lighter than removing the account and
* adding it back — the operation people otherwise resort to, and the one
* that costs them their mail.
*
* ── The identity guard ───────────────────────────────────────────────────
* The one thing that must never happen is this account quietly coming to
* point at a DIFFERENT mailbox. A user with two Google accounts who is
* signed into the wrong one in their browser will sail through the consent
* screen without noticing, and the tokens that come back would be perfectly
* valid — for somebody else's mail. Every later sync would then write that
* mailbox's messages into this account's threads, and there is no undo for
* that short of a restore.
*
* So the address is compared before anything is written, and a mismatch is
* refused outright rather than resolved cleverly. Case-insensitively,
* because providers do not agree with themselves about capitalisation
* across the id_token and the profile response, and a refusal over `A`
* versus `a` would be a wall with no way through it.
*
* The provider must match too. The same address can legitimately exist as
* both a Google and a Microsoft account (see upsert()), and re-pointing one
* at the other would be the same corruption by a slower route.
*
* @throws AccountIdentityMismatch when the authorised mailbox is not this one
*/
public function relink(
Account $account,
MailProvider $provider,
string $email,
AccessTokenInterface $token,
): Account {
if (0 !== strcasecmp(trim($email), trim($account->email))) {
throw new AccountIdentityMismatch($account->email, $email);
}
if ($provider->value !== $account->oauthProvider) {
throw new AccountIdentityMismatch($account->email, $email);
}
$account->oauthAccessToken = $token->getToken();
$refreshToken = $token->getRefreshToken();
if (null !== $refreshToken && '' !== $refreshToken) {
$account->oauthRefreshToken = $refreshToken;
}
$expires = $token->getExpires();
if (null !== $expires) {
$account->oauthTokenExpiry = new DateTimeImmutable()->setTimestamp($expires);
}
// The two lines this whole feature turned on. OAuthTokenManager writes
// the error and clears it only on a successful REFRESH — but a
// reconnect never goes through refresh(), so without this the account
// would work again while still reporting itself broken, and the health
// page would keep insisting on a repair that had already happened.
$account->oauthLastRefreshError = null;
$account->oauthLastRefreshAt = new DateTimeImmutable();
// A grant that died while the account was switched off leaves it off.
// Reconnecting is an unambiguous "I want this working", so it is turned
// back on — but only if it was us that stopped it, which is why this
// reads the error rather than setting isActive unconditionally.
$account->isActive = true;
$this->em->flush();
// Aliases are seeded rather than reseeded: AliasSeeder is idempotent,
// and a mailbox that gained a send-as address while the grant was dead
// would otherwise never pick it up.
$this->aliasSeeder->seed($account);
return $account;
}
/**
* Resolve the mailbox address from the provider's resource-owner payload.
*
+103
View File
@@ -0,0 +1,103 @@
<?php
declare(strict_types=1);
namespace App\Twig;
use App\Entity\User\User;
use App\Service\Health\AccountHealthInspector;
use Symfony\Bundle\SecurityBundle\Security;
use Symfony\Contracts\Service\ResetInterface;
/**
* Whether anything about this user's accounts needs their attention.
*
* Shaped after LogAlertGlobal, which solves the same problem one floor up: the
* topbar renders from a layout on every authenticated page, so there is no
* controller in the middle to thread a variable through.
*
* ── Not admin-only, unlike LogAlertGlobal ────────────────────────────────────
* That one is gated because the log browser is behind ROLE_ADMIN, and an alarm
* about something you cannot open is just an alarm. This is the opposite case:
* these are the user's OWN accounts and every repair offered is one they are
* allowed to run. The infrastructure check is the single exception and is asked
* for separately — see the includeInfrastructure flag.
*
* ── Not dismissible, unlike LogAlertGlobal ───────────────────────────────────
* There is no counterpart to User::$logsSeenAt here, deliberately. "Seen" is
* the right model for a log, which is a record of things that happened; it is
* the wrong model for a condition that is still true. A dot the user can clear
* while their mail is still not arriving would be worse than no dot at all,
* because the next time it appears they will already have learned that clearing
* it is what you do. This one goes away when the problem does.
*
* Per-request memoised, including the healthy answer, so a topbar that grew a
* second reference to it would not cost a second round of queries.
*/
final class AccountHealthGlobal implements ResetInterface
{
private bool $resolved = false;
/** @var array{tone: string, count: int}|null */
private ?array $attention = null;
public function __construct(
private readonly AccountHealthInspector $inspector,
private readonly Security $security,
) {
}
/**
* Worker-mode hygiene, for the reason LogAlertGlobal spells out: FrankenPHP
* keeps this service alive between requests, so without this the first
* answer a worker computes is the answer every later request — and every
* later USER — receives. Here that would mean showing one user's broken
* account to another, so it is a correctness requirement and not tidiness.
*/
public function reset(): void
{
$this->resolved = false;
$this->attention = null;
}
/**
* The tone and count for the indicator, or null when there is nothing to
* say.
*
* The count is root causes worth interrupting for, not issues — see
* HealthReport::indicatorCount(). One dead Google grant reads as "1" even
* when it has taken three calendars and five hundred jobs down with it,
* because that is how many things the user has to do.
*
* @return array{tone: string, count: int}|null
*/
public function getAttention(): ?array
{
if (true === $this->resolved) {
return $this->attention;
}
$this->resolved = true;
$user = $this->security->getUser();
if (false === $user instanceof User) {
return null;
}
// Infrastructure is left out here on purpose. The failure transport is
// instance-wide and reading it means deserialising envelopes; that is
// a fine cost on the settings page somebody navigated to, and not one
// to pay on every page render for a badge.
$report = $this->inspector->inspect($user);
$count = $report->indicatorCount();
$tone = $report->indicatorTone();
if (0 === $count || null === $tone) {
return null;
}
$this->attention = ['tone' => $tone, 'count' => $count];
return $this->attention;
}
}
+56 -4
View File
@@ -258,6 +258,7 @@
logAlert answers null for everyone else.
#}
{% set unseenLogs = logAlert.unseen %}
{% set attention = accountHealth.attention %}
<div class="relative ml-1"
data-controller="ui--user-menu admin--log-alert"
{# The log browser is a frame; this is not in it, so opening or
@@ -271,9 +272,14 @@
class="flex cursor-pointer items-center justify-center w-8 h-8 rounded-full bg-accent hover:ring-2 hover:ring-accent/60 hover:ring-offset-1 ring-offset-surface text-accent-ink text-xs font-semibold transition-all
{% if unseenLogs %}ring-2 ring-offset-1 {{ unseenLogs.tone == 'danger' ? 'ring-red-500' : 'ring-amber-500' }}{% endif %}"
aria-label="{{ 'header.user_menu.label'|trans({ '%email%': app.user.email }) }}"
title="{{ unseenLogs
? ('header.user_menu.unseen_logs.' ~ unseenLogs.tone)|trans({ '%count%': unseenLogs.count })
: app.user.email }}">
{# Account health outranks the log line in the tooltip: it is
the user's own mail, and it is the one a non-admin can
ever see. The log wording stays for the admin case. #}
title="{{ attention
? 'header.user_menu.account_attention'|trans({ '%count%': attention.count })
: (unseenLogs
? ('header.user_menu.unseen_logs.' ~ unseenLogs.tone)|trans({ '%count%': unseenLogs.count })
: app.user.email) }}">
{# The initial is the fallback, not the design: an avatar is
optional and most users never set one. #}
{% if app.user.avatar %}
@@ -285,6 +291,34 @@
{% endif %}
</button>
{#
"Something about your accounts needs attention."
A DOT, not a second ring, and that is the whole reason it is a
separate element. logAlert already outlines this button for
admins; a second ring on the same element would have meant one
of the two silently winning, and they mean different things —
that one is "there are unread errors in the log", this one is
"your mail is not arriving". They can be true at once and an
admin has to be able to tell them apart, so this sits in the
corner and leaves the outline alone.
Not dismissible, unlike the log outline: there is no seen-at
counterpart here on purpose. See AccountHealthGlobal — "seen" is
the right model for a record of what happened and the wrong one
for a condition that is still true. It goes when the problem
goes.
aria-hidden with the count in the button's title instead: a
screen reader gets one sentence rather than a floating number
with no antecedent.
#}
{% if attention %}
<span class="absolute -top-0.5 -right-0.5 flex items-center justify-center min-w-[0.875rem] h-3.5 px-1 rounded-full text-[9px] font-bold text-white ring-2 ring-surface pointer-events-none
{{ attention.tone == 'danger' ? 'bg-red-500' : 'bg-amber-500' }}"
aria-hidden="true">{{ attention.count }}</span>
{% endif %}
<div id="user-menu" data-ui--user-menu-target="menu" class="hidden absolute right-0 mt-2 w-56 bg-surface border border-line rounded-pane shadow-lg py-1 z-[200]">
<div class="px-4 py-2.5 border-b border-line">
{% set displayName = (app.user.nameFirst ~ ' ' ~ app.user.nameLast)|trim %}
@@ -302,9 +336,27 @@
<i class="fa-solid fa-user w-4 text-gray-400" aria-hidden="true"></i>
{{ 'header.user_menu.profile'|trans }}
</a>
<a href="{{ path('app_settings_index') }}" class="flex items-center gap-2 px-4 py-2 text-sm text-ink-soft hover:bg-hover">
{#
The dot on the avatar says something is wrong; this is where
it goes. Pointed straight at the health section rather than
at settings generally — the same choice the admin entry
below makes for the log browser, and for the same reason: an
indicator that lands you on a page where you still have to
hunt has only moved the hunting.
#}
<a href="{{ path('app_settings_index', attention ? { section: 'health' } : {}) }}"
id="user-menu-settings"
class="flex items-center gap-2 px-4 py-2 text-sm text-ink-soft hover:bg-hover">
<i class="fa-solid fa-gear w-4 text-gray-400" aria-hidden="true"></i>
{{ 'header.user_menu.settings'|trans }}
{% if attention %}
<span class="ml-auto inline-flex items-center gap-1.5 text-xs
{{ attention.tone == 'danger' ? 'text-danger' : 'text-warning' }}">
<span class="w-1.5 h-1.5 rounded-full {{ attention.tone == 'danger' ? 'bg-red-500' : 'bg-amber-500' }}"
aria-hidden="true"></span>
{{ attention.count }}
</span>
{% endif %}
</a>
{% if is_granted('ROLE_ADMIN') %}
{# The outline on the button says something is wrong; this
+152
View File
@@ -0,0 +1,152 @@
{#
Account health: what is wrong, and the lightest thing that fixes it.
Reads a HealthReport, already ordered worst-first with each consequence
following its cause (HealthReport::arrange), so this file has no opinion
about priority — it renders the list in the order it is given.
Two rules the markup exists to keep:
1. Every card says what the button will DO before you press it. The whole
reason people delete an account and add it back is that "Reconnect"
does not promise their mail will still be there, so the promise is on
the page, not in a tooltip.
2. Destructive repairs are visually separate and never the primary. They
sit under a rule, in muted text, with the word that says what is lost.
#}
<section id="settings-health" data-health-section>
<div class="mb-4 px-4 @2xl:px-0">
<h2 class="text-sm font-semibold text-ink">{{ 'settings.health.heading'|trans }}</h2>
<p class="text-xs text-ink-faint mt-0.5">{{ 'settings.health.subheading'|trans }}</p>
</div>
{% if healthReport.isHealthy %}
{#
The healthy state is a real state and gets real words. A blank pane
reads as "not loaded yet"; this reads as "checked, nothing wrong",
which is the answer somebody who followed the topbar dot here after
fixing something needs to see.
#}
<div class="rounded-pane border border-line px-5 py-8 text-center" data-health-empty>
<i class="fa-solid fa-circle-check text-2xl text-emerald-500 mb-3 block" aria-hidden="true"></i>
<p class="text-sm font-medium text-ink">{{ 'settings.health.healthy.title'|trans }}</p>
<p class="text-xs text-ink-faint mt-1 max-w-sm mx-auto">{{ 'settings.health.healthy.body'|trans }}</p>
</div>
{% else %}
<div class="space-y-3">
{% for issue in healthReport.issues %}
{#
A consequence is indented and quieter than its cause. It is
still a card — the user's question really is "why is this
one calendar empty" — but it must not read as another
emergency, or one dead sign-in looks like nine problems.
#}
<div class="rounded-pane border border-line bg-surface {{ issue.severity.cardClasses }} {{ issue.isConsequence ? 'ml-0 @2xl:ml-6' : '' }}"
data-health-issue="{{ issue.id }}"
data-health-kind="{{ issue.kind.value }}"
data-health-severity="{{ issue.severity.value }}"
{% if issue.isConsequence %}data-health-caused-by="{{ issue.causedBy }}"{% endif %}>
<div class="px-4 py-4 @2xl:px-5">
<div class="flex items-start gap-3">
<i class="fa-solid {{ issue.kind.icon }} mt-0.5 w-4 shrink-0 {{ issue.severity.iconClasses }}" aria-hidden="true"></i>
<div class="min-w-0 flex-1">
<h3 class="text-sm font-semibold text-ink">
{{ issue.kind.titleKey|trans(issue.titleParams) }}
</h3>
{# What it means for their mail, in plain words. #}
<p class="text-xs text-ink-muted mt-1 leading-relaxed">
{{ issue.kind.bodyKey|trans(issue.bodyParams) }}
</p>
{% set safeRepairs = issue.safeRepairs %}
{% set riskyRepairs = issue.destructiveRepairs %}
{% if safeRepairs is not empty %}
<div class="mt-3 space-y-2.5">
{% for repair in safeRepairs %}
<div>
{# The promise, above the button that keeps it. #}
<p class="text-xs text-ink-faint mb-1.5">{{ repair.promiseKey|trans(repair.promiseParams) }}</p>
{% if repair.isPost %}
<form method="post" action="{{ path(repair.route, repair.routeParams) }}">
<input type="hidden" name="_token" value="{{ csrf_token(repair.csrfTokenId) }}">
<button type="submit"
class="inline-flex items-center gap-2 rounded-lg bg-accent px-3 py-1.5 text-xs font-medium text-accent-ink hover:opacity-90 transition-opacity cursor-pointer">
{{ repair.labelKey|trans }}
</button>
</form>
{% else %}
{#
A link, not a form: this one leaves the
app for the provider's consent screen and
comes back. Nothing is written until it
does, so there is no state change here for
a token to protect — the write happens in
the callback, behind the state check and
the address guard.
#}
<a href="{{ path(repair.route, repair.routeParams) }}"
data-turbo="false"
class="inline-flex items-center gap-2 rounded-lg bg-accent px-3 py-1.5 text-xs font-medium text-accent-ink hover:opacity-90 transition-opacity">
{{ repair.labelKey|trans }}
<i class="fa-solid fa-arrow-up-right-from-square text-[10px]" aria-hidden="true"></i>
</a>
{% endif %}
</div>
{% endfor %}
</div>
{% endif %}
{% if issue.detail is not null %}
{#
The provider's own words, behind a
disclosure. MailProvider::calendarScopes()
makes the case that "invalid_grant" is not
an explanation to put in front of a person
— but a self-hoster matching this against a
forum thread needs to be able to find it, so
it is one click away rather than absent.
#}
<details class="mt-3 group">
<summary class="text-xs text-ink-faint cursor-pointer hover:text-ink-muted select-none list-none inline-flex items-center gap-1.5">
<i class="fa-solid fa-chevron-right text-[9px] transition-transform group-open:rotate-90" aria-hidden="true"></i>
{{ 'settings.health.technical_detail'|trans }}
</summary>
<pre class="mt-2 overflow-x-auto rounded-lg bg-raised px-3 py-2 text-[11px] leading-relaxed text-ink-muted whitespace-pre-wrap break-words">{{ issue.detail }}</pre>
</details>
{% endif %}
{% if riskyRepairs is not empty %}
{#
Under a rule and after the safe options,
never beside them. These throw work away.
#}
<div class="mt-4 pt-3 border-t border-line">
{% for repair in riskyRepairs %}
<p class="text-xs text-ink-faint mb-1.5">{{ repair.promiseKey|trans(repair.promiseParams) }}</p>
{# data-turbo-confirm, matching every other
destructive action in the app — see the
admin failed-queue panel. #}
<form method="post" action="{{ path(repair.route, repair.routeParams) }}"
data-turbo-confirm="{{ 'settings.health.confirm.destructive'|trans }}">
<input type="hidden" name="_token" value="{{ csrf_token(repair.csrfTokenId) }}">
<button type="submit"
class="inline-flex items-center gap-2 rounded-lg border border-line px-3 py-1.5 text-xs font-medium text-danger hover:bg-hover transition-colors cursor-pointer">
{{ repair.labelKey|trans }}
</button>
</form>
{% endfor %}
</div>
{% endif %}
</div>
</div>
</div>
</div>
{% endfor %}
</div>
{% endif %}
</section>
+53 -1
View File
@@ -67,6 +67,21 @@
the previous copy-paste had already drifted — two sections
shared one icon and one of them highlighted it a different
colour when active. #}
{#
Health leads, above the group headings rather than
inside one. It is not a category of setting — it is
the page that tells you something is broken — and
filing it under "Mail" or "Personal" would bury the
one entry that is time-sensitive.
It is always present, not only when something is
wrong. A nav item that appears and disappears is one
people cannot learn, and "where do I check?" has to
have an answer on a good day too. The badge is what
changes, not the link.
#}
{% set attention = accountHealth.attention %}
{% set navGroups = [
{ label: 'settings.nav.group.personal', items: [
{ section: 'profile', icon: 'fa-solid fa-user' },
@@ -102,6 +117,41 @@
{% set navActive = 'flex items-center gap-2.5 px-3 py-2 rounded-lg text-sm font-medium bg-raised text-ink' %}
{% set navInactive = 'flex items-center gap-2.5 px-3 py-2 rounded-lg text-sm text-ink-muted hover:bg-hover transition-colors' %}
{#
Health leads, and sits outside the groups rather
than inside one. It is not a category of setting
— it is the page that says something is broken —
and filing it under "Mail" or "Personal" would
bury the one entry that is time-sensitive.
Inside the <nav> and not above it, so it joins the
wrapping strip this becomes on mobile. Lifted out
to a sibling it would have needed `hidden md:block`
like the group headings do, and the phone would
have had no way to reach the page at all.
Always present, not only when something is wrong:
a nav item that comes and goes is one nobody can
learn, and "where do I check?" needs an answer on
a good day too. The badge changes, the link does
not.
#}
<a href="{{ path('app_settings_index', { section: 'health' }) }}"
class="{{ section == 'health' ? navActive : navInactive }} md:mb-1"
{{ section == 'health' ? 'aria-current="page"' : '' }}>
<i class="fa-solid fa-heart-pulse w-4 {{ attention ? (attention.tone == 'danger' ? 'text-red-500' : 'text-amber-500') : (section == 'health' ? 'text-zinc-400' : '') }}"
aria-hidden="true"></i>
{{ 'settings.nav.health'|trans }}
{% if attention %}
{# The number is root causes, not symptoms —
see HealthReport::indicatorCount(). #}
<span class="ml-auto inline-flex items-center justify-center min-w-5 h-5 px-1.5 rounded-full text-[11px] font-semibold text-white
{{ attention.tone == 'danger' ? 'bg-red-500' : 'bg-amber-500' }}">
{{ attention.count }}
</span>
{% endif %}
</a>
{% for group in navGroups %}
{# Wraps into a row on mobile, where the nav is a
horizontal strip and a heading per group would cost
@@ -144,7 +194,9 @@
it, so pane width and viewport width do not move
together. #}
<div class="min-w-0 -mx-6 @2xl:mx-0">
{% if section == 'appearance' %}
{% if section == 'health' %}
{% include 'settings/_health.html.twig' %}
{% elseif section == 'appearance' %}
{% include 'settings/_appearance.html.twig' %}
{% elseif section == 'profile' %}
{% include 'settings/_profile.html.twig' %}
+1
View File
@@ -41,6 +41,7 @@ final class PageRendersTest extends WebTestCase
yield 'settings' => ['/settings'];
yield 'settings appearance' => ['/settings?section=appearance'];
yield 'settings accounts' => ['/settings?section=accounts'];
yield 'settings health' => ['/settings?section=health'];
yield 'settings filters' => ['/settings?section=filters'];
yield 'settings integrations' => ['/settings?section=integrations'];
yield 'settings aliases' => ['/settings?section=aliases'];
@@ -0,0 +1,317 @@
<?php
declare(strict_types=1);
namespace App\Tests\Controller\Settings;
use App\Domain\Enum\Account\AuthType;
use App\Domain\Enum\Account\MailProvider;
use App\Domain\Enum\Integration\Provider;
use App\Entity\Calendar\Calendar;
use App\Entity\Integration\Integration;
use App\Entity\Mail\Account;
use App\Entity\User\User;
use App\Repository\User\UserRepository;
use DateTimeImmutable;
use Doctrine\DBAL\Connection;
use Doctrine\ORM\EntityManagerInterface;
use Symfony\Bundle\FrameworkBundle\KernelBrowser;
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
/**
* The two problem classes that are NOT emergencies, and the rule that keeps
* them from being dressed as one.
*
* WHY THESE HAVE THEIR OWN FILE
* ─────────────────────────────
* A health page that paints "your mail is a few minutes late" the same red as
* "your mail has stopped" is a page people learn to close, and then the red
* that mattered goes unread too. Degraded push is the archetype: mail is still
* arriving, nothing is lost, and there is genuinely no rush — so it is a
* Notice, it is styled differently, and above all it does NOT light the topbar
* indicator.
*
* That last part is the assertion worth having
* (testDegradedPushDoesNotLightTheIndicator). Everything else about severity is
* cosmetic and would survive being got wrong; the indicator is the thing that
* interrupts somebody, and spending it on "no rush" is how it stops meaning
* anything.
*/
final class AccountHealthQuieterProblemsTest extends WebTestCase
{
private const string ADMIN_EMAIL = 'e2e-admin@plmail.test';
private EntityManagerInterface $em;
private Connection $connection;
protected function tearDown(): void
{
if (true === isset($this->connection) && true === $this->connection->isTransactionActive()) {
$this->connection->rollBack();
}
parent::tearDown();
}
// ── push: real, but quiet ────────────────────────────────────────────────
public function testDegradedPushIsReportedAsANotice(): void
{
$client = static::createClient();
$user = $this->boot($client);
$account = $this->gmailAccount($user, 'slow@joder.dev');
// Registered, but the watch lapsed — so nothing is being delivered
// whatever the flag says.
$account->pushEnabled = true;
$account->gmailWatchExpiry = new DateTimeImmutable('-1 day');
$this->em->flush();
$crawler = $client->request('GET', '/settings?section=health');
$card = $crawler->filter('[data-health-issue="push-' . $account->id . '"]');
self::assertSame(1, $card->count(), 'degraded push is surfaced');
self::assertSame('push_degraded', $card->attr('data-health-kind'));
self::assertSame('notice', $card->attr('data-health-severity'), 'and it is not an emergency');
// It offers the repair that already exists, rather than a second one.
self::assertSame(1, $card->filter('form[action*="/push/repair"]')->count());
}
/**
* The rule this whole file exists for. The card is on the page; the badge
* is not lit. Somebody who came looking will find it; nobody is
* interrupted for it.
*/
public function testDegradedPushDoesNotLightTheIndicator(): void
{
$client = static::createClient();
$user = $this->boot($client);
$account = $this->gmailAccount($user, 'notloud@joder.dev');
$account->pushEnabled = true;
$account->gmailWatchExpiry = new DateTimeImmutable('-1 day');
$this->em->flush();
$crawler = $client->request('GET', '/settings?section=health');
self::assertSame(1, $crawler->filter('[data-health-issue="push-' . $account->id . '"]')->count());
self::assertSame(
0,
$crawler->filter('nav a[href*="section=health"] span.rounded-full')->count(),
'a notice never lights the indicator',
);
}
/** Push that is simply off is a choice, not a fault. */
public function testPushBeingOffIsNotAProblem(): void
{
$client = static::createClient();
$user = $this->boot($client);
$account = $this->gmailAccount($user, 'polling@joder.dev');
$account->pushEnabled = false;
$this->em->flush();
$crawler = $client->request('GET', '/settings?section=health');
self::assertSame(0, $crawler->filter('[data-health-issue="push-' . $account->id . '"]')->count());
}
/**
* With the grant dead there is no push to speak of, and the reconnect is
* the repair for both. Reporting them separately would put two cards on the
* page for one thing to do.
*/
public function testPushIsNotReportedSeparatelyWhenTheGrantIsDead(): void
{
$client = static::createClient();
$user = $this->boot($client);
$account = $this->gmailAccount($user, 'both@joder.dev');
$account->pushEnabled = true;
$account->gmailWatchExpiry = new DateTimeImmutable('-1 day');
$account->oauthLastRefreshError = 'invalid_grant';
$this->em->flush();
$crawler = $client->request('GET', '/settings?section=health');
self::assertSame(1, $crawler->filter('[data-health-issue="account-' . $account->id . '"]')->count());
self::assertSame(
0,
$crawler->filter('[data-health-issue="push-' . $account->id . '"]')->count(),
'one thing to do, one card',
);
}
// ── integrations ─────────────────────────────────────────────────────────
/**
* IntegrationTokenManager already records the failure and already composes
* "X needs to be reconnected". Its docblock says the settings list should
* say so; this is that list.
*/
public function testAnIntegrationThatCouldNotBeRenewedIsReported(): void
{
$client = static::createClient();
$user = $this->boot($client);
$integration = new Integration($user, Provider::Dropbox, 'Dropbox');
$integration->recordFailure('Could not renew access to Dropbox: invalid_grant');
$this->em->persist($integration);
$this->em->flush();
$crawler = $client->request('GET', '/settings?section=health');
$card = $crawler->filter('[data-health-issue="integration-' . $integration->id . '"]');
self::assertSame(1, $card->count());
self::assertSame('integration_reconnect', $card->attr('data-health-kind'));
self::assertSame('warning', $card->attr('data-health-severity'));
// Mail is explicitly said to be unaffected — the fear this card has to
// answer is "has something happened to my email?".
self::assertStringContainsString('mail is unaffected', $card->text());
// Reconnect goes through the integration OAuth flow that already
// exists, which upserts onto the same row rather than duplicating it.
self::assertSame(1, $card->filter('a[href*="/integrations/oauth/"]')->count());
}
public function testAWorkingIntegrationIsNotReported(): void
{
$client = static::createClient();
$user = $this->boot($client);
$integration = new Integration($user, Provider::Nextcloud, 'Nextcloud');
$integration->recordSuccess();
$this->em->persist($integration);
$this->em->flush();
$crawler = $client->request('GET', '/settings?section=health');
self::assertSame(0, $crawler->filter('[data-health-issue="integration-' . $integration->id . '"]')->count());
}
// ── the queue, and who may touch it ──────────────────────────────────────
/**
* The failure transport is instance-wide, so the queue card is an admin
* thing — see AccountHealthInspector::abandonedWork(). A non-admin must not
* be shown a button that would act on everybody's work.
*/
public function testANonAdminIsNotOfferedTheQueueRepairs(): void
{
$client = static::createClient();
$this->boot($client);
// The seeded admin is an admin; log in as somebody who is not.
$plain = $this->plainUser();
$client->loginUser($plain);
$crawler = $client->request('GET', '/settings?section=health');
self::assertResponseIsSuccessful();
self::assertSame(
0,
$crawler->filter('[data-health-kind="queue_work_abandoned"]')->count(),
'the queue is not a per-user question',
);
}
public function testTheQueueRepairsRefuseANonAdmin(): void
{
$client = static::createClient();
$this->boot($client);
$plain = $this->plainUser();
$client->loginUser($plain);
$client->request('POST', '/settings/health/queue/retry', ['_token' => 'anything']);
self::assertResponseStatusCodeSame(403);
$client->request('POST', '/settings/health/queue/discard', ['_token' => 'anything']);
self::assertResponseStatusCodeSame(403);
}
/** Even an admin needs a token — these are state-changing POSTs. */
public function testTheQueueRepairsNeedACsrfToken(): void
{
$client = static::createClient();
$this->boot($client);
$client->request('POST', '/settings/health/queue/retry');
self::assertResponseStatusCodeSame(403);
$client->request('POST', '/settings/health/queue/discard');
self::assertResponseStatusCodeSame(403);
}
// ── helpers ──────────────────────────────────────────────────────────────
private function boot(KernelBrowser $client): User
{
$container = static::getContainer();
$this->em = $container->get(EntityManagerInterface::class);
$this->connection = $container->get(Connection::class);
$user = $container->get(UserRepository::class)->findOneBy(['email' => self::ADMIN_EMAIL]);
if (null === $user) {
self::markTestSkipped('run `app:test:seed-user --admin` first');
}
$client->loginUser($user);
$client->disableReboot();
$this->connection->beginTransaction();
$this->em->createQuery('DELETE FROM ' . Calendar::class . ' c WHERE c.usr = :usr')
->setParameter('usr', $user)->execute();
$this->em->createQuery('DELETE FROM ' . Account::class . ' a WHERE a.usr = :usr')
->setParameter('usr', $user)->execute();
$this->em->createQuery('DELETE FROM ' . Integration::class . ' i WHERE i.usr = :usr')
->setParameter('usr', $user)->execute();
$this->em->clear();
return $this->em->find(User::class, $user->id);
}
/** A user with ROLE_USER and nothing more. */
private function plainUser(): User
{
$user = new User();
$user->email = 'plain-' . bin2hex(random_bytes(4)) . '@joder.dev';
$user->password = 'not-a-real-hash';
$user->nameFirst = 'Plain';
$user->nameLast = 'User';
$this->em->persist($user);
$this->em->flush();
return $user;
}
private function gmailAccount(User $user, string $email): Account
{
$account = new Account();
$account->usr = $user;
$account->name = $email;
$account->username = $email;
$account->email = $email;
$account->authType = AuthType::OAuth2->value;
$account->oauthProvider = MailProvider::Google->value;
$account->oauthAccessToken = 'access-token';
$account->oauthRefreshToken = 'refresh-token';
$account->isActive = true;
$this->em->persist($account);
$this->em->flush();
return $account;
}
}
@@ -0,0 +1,591 @@
<?php
declare(strict_types=1);
namespace App\Tests\Controller\Settings;
use App\Domain\Enum\Account\AuthType;
use App\Domain\Enum\Account\MailProvider;
use App\Domain\Enum\Calendar\CalendarRole;
use App\Entity\Calendar\Calendar;
use App\Entity\Mail\Account;
use App\Entity\User\User;
use App\Repository\User\UserRepository;
use Doctrine\DBAL\Connection;
use Doctrine\ORM\EntityManagerInterface;
use Symfony\Bundle\FrameworkBundle\KernelBrowser;
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
/**
* The account-health section: what it says, when it says nothing, and who is
* allowed to press the buttons.
*
* WHAT THIS IS FOR
* ────────────────
* The app already DETECTED every condition on this page and then discarded it.
* Account::$oauthLastRefreshError was written by OAuthTokenManager and read by
* no template and no controller; on the install this was built from it had held
* `invalid_grant` for two days while mail silently stopped arriving, and the
* only trace was five thousand log lines nobody had reason to open.
*
* THE ASSERTION THAT MATTERS MOST is not that a problem shows up — that is the
* easy half, and a page that showed a card for everything would pass it. It is
* testAHealthyAccountReportsNothing and its siblings: a health page that cries
* wolf gets ignored, and once it is ignored the red card that mattered is
* unread too. Every detection test below has a healthy counterpart.
*/
final class AccountHealthSectionTest extends WebTestCase
{
private const string ADMIN_EMAIL = 'e2e-admin@plmail.test';
private EntityManagerInterface $em;
private Connection $connection;
protected function tearDown(): void
{
if (true === isset($this->connection) && true === $this->connection->isTransactionActive()) {
$this->connection->rollBack();
}
parent::tearDown();
}
// ── the section itself ───────────────────────────────────────────────────
public function testTheSectionRendersAndIsReachableFromTheNav(): void
{
$client = static::createClient();
$this->boot($client);
$crawler = $client->request('GET', '/settings?section=accounts');
self::assertResponseIsSuccessful();
self::assertGreaterThan(
0,
$crawler->filter('nav a[href*="section=health"]')->count(),
'the settings nav links to the health section',
);
$crawler = $client->request('GET', '/settings?section=health');
self::assertResponseIsSuccessful();
self::assertGreaterThan(0, $crawler->filter('#settings-health')->count());
// Survives SettingsController::SECTIONS — an unknown section falls back
// to accounts, which would render no panel at all and still be a 200.
self::assertGreaterThan(
0,
$crawler->filter('nav a[href*="section=health"][aria-current="page"]')->count(),
'the nav marks health as the current page',
);
}
// ── nothing wrong means nothing said ─────────────────────────────────────
/**
* The false-positive guard. A working OAuth account, a calendar that has
* never synced because it mirrors nothing, and a password account all
* count as healthy — none of them is evidence of a problem.
*/
public function testAHealthyAccountReportsNothing(): void
{
$client = static::createClient();
$user = $this->boot($client);
$this->oauthAccount($user, 'working@joder.dev');
$this->account($user, 'imap@joder.dev');
// A calendar with no stored error. Never synced, which is normal for one
// that has only just been subscribed to.
$calendar = $this->calendar($user, 'Empty but fine');
$calendar->lastSyncedAt = null;
$this->em->flush();
$crawler = $client->request('GET', '/settings?section=health');
self::assertResponseIsSuccessful();
self::assertSame(
0,
$crawler->filter('[data-health-issue]')->count(),
'nothing is wrong, so nothing is reported',
);
self::assertSame(1, $crawler->filter('[data-health-empty]')->count(), 'and it says so in words');
}
/**
* The indicator is absent when there is nothing to indicate. Pinned
* separately from the page: the badge renders from a Twig global on every
* authenticated page, so it can be wrong while the page is right.
*/
public function testTheIndicatorIsAbsentWhenEverythingIsWorking(): void
{
$client = static::createClient();
$user = $this->boot($client);
$this->oauthAccount($user, 'quiet@joder.dev');
$this->em->flush();
$crawler = $client->request('GET', '/settings?section=health');
self::assertResponseIsSuccessful();
self::assertSame(
0,
$crawler->filter('nav a[href*="section=health"] span.rounded-full')->count(),
'no badge on the nav entry',
);
}
// ── the dead grant ───────────────────────────────────────────────────────
/**
* The condition the whole feature exists for: stored, and until now never
* read back.
*/
public function testADeadOauthGrantIsReportedWithAReconnect(): void
{
$client = static::createClient();
$user = $this->boot($client);
$account = $this->oauthAccount($user, 'broken@joder.dev');
$account->oauthLastRefreshError = 'invalid_grant';
$this->em->flush();
$crawler = $client->request('GET', '/settings?section=health');
self::assertResponseIsSuccessful();
$card = $crawler->filter('[data-health-issue="account-' . $account->id . '"]');
self::assertSame(1, $card->count(), 'the dead grant gets a card');
self::assertSame('account_reconnect', $card->attr('data-health-kind'));
self::assertSame('critical', $card->attr('data-health-severity'));
// Phrased for a person. The provider's code is NOT the headline — see
// MailProvider::calendarScopes() on why "403" is not an explanation.
self::assertStringContainsString('broken@joder.dev', $card->text());
self::assertStringNotContainsString('invalid_grant', $card->filter('h3')->text());
// ...but it is still findable, one disclosure away.
self::assertStringContainsString('invalid_grant', $card->filter('details pre')->text());
// The repair is a link, not a form: it leaves for the provider's
// consent screen and comes back, so it cannot be a POST from here.
$link = $card->filter('a[href*="/settings/health/reconnect/"]');
self::assertSame(1, $link->count(), 'there is a reconnect button');
self::assertStringEndsWith('/reconnect/' . $account->id, (string) $link->attr('href'));
// ...and following it hands off to the OAuth flow with the repair
// intent attached, rather than to the "add an account" path.
$client->request('GET', (string) $link->attr('href'));
self::assertResponseRedirects();
self::assertStringContainsString(
'reconnect=' . $account->id,
(string) $client->getResponse()->headers->get('Location'),
);
}
/**
* An OAuth account with no refresh token on file at all — the same problem
* arriving by a different road (OAuthTokenManager::refresh() throws for it
* before it ever reaches the provider).
*/
public function testAnAccountWithNoRefreshTokenIsAlsoReported(): void
{
$client = static::createClient();
$user = $this->boot($client);
$account = $this->oauthAccount($user, 'tokenless@joder.dev');
$account->oauthRefreshToken = null;
$this->em->flush();
$crawler = $client->request('GET', '/settings?section=health');
self::assertSame(1, $crawler->filter('[data-health-issue="account-' . $account->id . '"]')->count());
}
/**
* A password account has no grant to be dead, and must never be told to
* reconnect one.
*/
public function testAPasswordAccountIsNeverAskedToReconnect(): void
{
$client = static::createClient();
$user = $this->boot($client);
$account = $this->account($user, 'imaponly@joder.dev');
$this->em->flush();
$crawler = $client->request('GET', '/settings?section=health');
self::assertSame(0, $crawler->filter('[data-health-issue="account-' . $account->id . '"]')->count());
}
// ── calendars, and the difference between a cause and a symptom ──────────
/**
* A failing calendar on a healthy account stands on its own: it is the
* problem, so it is critical and it gets its own retry.
*/
public function testAFailingCalendarOnAHealthyAccountIsItsOwnProblem(): void
{
$client = static::createClient();
$user = $this->boot($client);
$account = $this->oauthAccount($user, 'fine@joder.dev');
$calendar = $this->remoteCalendar($user, $account, 'Holidays');
$calendar->lastSyncError = 'The calendar no longer exists at the remote.';
$this->em->flush();
$crawler = $client->request('GET', '/settings?section=health');
$card = $crawler->filter('[data-health-issue="calendar-' . $calendar->id . '"]');
self::assertSame(1, $card->count());
self::assertSame('calendar_sync_failing', $card->attr('data-health-kind'));
self::assertSame('critical', $card->attr('data-health-severity'));
self::assertNull($card->attr('data-health-caused-by'), 'nothing above it explains it');
// And it offers a retry, because a retry could plausibly work here.
self::assertSame(1, $card->filter('form[action*="/resync"]')->count());
}
/**
* The grouping that keeps one broken sign-in from reading as four
* emergencies. This is the exact shape of the live install: one dead grant,
* three calendars that stopped with it.
*/
public function testCalendarsFailingBecauseOfADeadGrantAreShownAsConsequences(): void
{
$client = static::createClient();
$user = $this->boot($client);
$account = $this->oauthAccount($user, 'cascade@joder.dev');
$account->oauthLastRefreshError = 'invalid_grant';
$calendars = [];
foreach (['Feiertage', 'Familie', 'cascade@joder.dev'] as $name) {
$calendar = $this->remoteCalendar($user, $account, $name);
$calendar->lastSyncError = 'Google would not renew the sign-in for this account.';
$calendars[] = $calendar;
}
$this->em->flush();
$crawler = $client->request('GET', '/settings?section=health');
foreach ($calendars as $calendar) {
$card = $crawler->filter('[data-health-issue="calendar-' . $calendar->id . '"]');
self::assertSame(1, $card->count(), 'the calendar is still listed by name');
self::assertSame(
'account-' . $account->id,
$card->attr('data-health-caused-by'),
'and it is attributed to the sign-in that caused it',
);
self::assertSame(
'warning',
$card->attr('data-health-severity'),
'a consequence is not a second emergency',
);
// No retry button: with the grant dead, "try again" is a button
// whose entire behaviour is to fail.
self::assertSame(0, $card->filter('form[action*="/resync"]')->count());
}
// The badge counts ONE thing to do, not four.
$badge = $crawler->filter('nav a[href*="section=health"] span.rounded-full');
self::assertSame(1, $badge->count());
self::assertSame('1', trim($badge->text()));
}
/** A calendar that syncs fine is not mentioned. */
public function testAWorkingCalendarIsNotReported(): void
{
$client = static::createClient();
$user = $this->boot($client);
$account = $this->oauthAccount($user, 'cal-ok@joder.dev');
$calendar = $this->remoteCalendar($user, $account, 'Working');
$calendar->recordSyncSuccess();
$this->em->flush();
$crawler = $client->request('GET', '/settings?section=health');
self::assertSame(0, $crawler->filter('[data-health-issue="calendar-' . $calendar->id . '"]')->count());
}
// ── the indicator appears, and goes when the problem does ────────────────
/**
* The other half of the indicator requirement, and the reason there is no
* "mark as seen" anywhere in this feature.
*
* LogAlertGlobal is dismissible because a log is a record of things that
* happened. A health issue is a condition that is still true, and a dot the
* user could clear while their mail was still not arriving would be worse
* than no dot at all — the next time it appeared, they would already have
* learned that clearing it is what you do. So the only thing that clears it
* is the repair, which is what this asserts.
*/
public function testTheIndicatorAppearsWithTheProblemAndGoesWhenItIsRepaired(): void
{
$client = static::createClient();
$user = $this->boot($client);
$account = $this->oauthAccount($user, 'indicator@joder.dev');
$account->oauthLastRefreshError = 'invalid_grant';
$this->em->flush();
$crawler = $client->request('GET', '/settings?section=health');
$badge = $crawler->filter('nav a[href*="section=health"] span.rounded-full');
self::assertSame(1, $badge->count(), 'the indicator appears');
self::assertSame('1', trim($badge->text()));
// Repaired the way a real reconnect repairs it — OAuthAccountLinker
// clears exactly this field, and nothing else marks the issue "read".
$account = $this->em->find(Account::class, $account->id);
$account->oauthLastRefreshError = null;
$this->em->flush();
$crawler = $client->request('GET', '/settings?section=health');
self::assertSame(
0,
$crawler->filter('nav a[href*="section=health"] span.rounded-full')->count(),
'and goes when the condition does — with nothing having been dismissed',
);
self::assertSame(1, $crawler->filter('[data-health-empty]')->count());
}
// ── ownership and CSRF ───────────────────────────────────────────────────
/**
* These actions touch account credentials and background work. A logged-in
* session is not enough.
*/
public function testResyncingACalendarWithoutATokenIsRefused(): void
{
$client = static::createClient();
$user = $this->boot($client);
$account = $this->oauthAccount($user, 'csrf@joder.dev');
$calendar = $this->remoteCalendar($user, $account, 'Guarded');
$calendar->lastSyncError = 'something';
$this->em->flush();
$client->request('POST', '/settings/health/calendar/' . $calendar->id . '/resync');
self::assertResponseStatusCodeSame(403);
}
public function testResyncingSomebodyElsesCalendarIsRefused(): void
{
$client = static::createClient();
$user = $this->boot($client);
$stranger = $this->stranger();
$calendar = $this->calendar($stranger, 'Not yours');
$this->em->flush();
$client->request('POST', '/settings/health/calendar/' . $calendar->id . '/resync', [
'_token' => 'anything',
]);
// Ownership is checked BEFORE the token, so this is a refusal on the
// grounds that matter rather than an accidental pass on a bad token.
self::assertResponseStatusCodeSame(403);
}
public function testReconnectingSomebodyElsesAccountIsRefused(): void
{
$client = static::createClient();
$this->boot($client);
$stranger = $this->stranger();
$account = $this->oauthAccount($stranger, 'theirs@joder.dev');
$this->em->flush();
$client->request('GET', '/settings/health/reconnect/' . $account->id);
self::assertResponseStatusCodeSame(403);
}
/**
* The reconnect entry point refuses an id that is not the caller's even at
* the OAuth controller, which is the one that stores the repair intent in
* the session.
*/
public function testStartingAnOauthReconnectForSomebodyElsesAccountIsRefused(): void
{
$client = static::createClient();
$this->boot($client);
$stranger = $this->stranger();
$account = $this->oauthAccount($stranger, 'session@joder.dev');
$this->em->flush();
$client->request('GET', '/oauth/google/connect?reconnect=' . $account->id);
self::assertResponseStatusCodeSame(403);
}
/** The calendar resync really does clear the backoff, not just dispatch. */
public function testResyncingClearsTheBackoffSoTheSweepPicksItUpAgain(): void
{
$client = static::createClient();
$user = $this->boot($client);
$account = $this->oauthAccount($user, 'backoff@joder.dev');
$calendar = $this->remoteCalendar($user, $account, 'Backed off');
$calendar->lastSyncError = 'The calendar no longer exists at the remote.';
$calendar->recordSyncFailure('The calendar no longer exists at the remote.');
$this->em->flush();
self::assertTrue($calendar->isBackingOff());
$crawler = $client->request('GET', '/settings?section=health');
$form = $crawler->filter('[data-health-issue="calendar-' . $calendar->id . '"] form[action*="/resync"]');
self::assertSame(1, $form->count());
$client->request('POST', (string) $form->attr('action'), [
'_token' => (string) $form->filter('input[name="_token"]')->attr('value'),
]);
self::assertResponseRedirects('/settings?section=health');
$this->em->clear();
$reloaded = $this->em->find(Calendar::class, $calendar->id);
self::assertFalse($reloaded->isBackingOff(), 'the schedule is reset so the sweep tries again');
self::assertNotNull($reloaded->lastSyncError, 'but it is still broken until a sync says otherwise');
}
// ── helpers ──────────────────────────────────────────────────────────────
private function boot(KernelBrowser $client): User
{
$container = static::getContainer();
$this->em = $container->get(EntityManagerInterface::class);
$this->connection = $container->get(Connection::class);
$user = $container->get(UserRepository::class)->findOneBy(['email' => self::ADMIN_EMAIL]);
if (null === $user) {
self::markTestSkipped('run `app:test:seed-user --admin` first');
}
$client->loginUser($user);
$client->disableReboot();
$this->connection->beginTransaction();
// Seeded fixtures would otherwise show up as health issues of their own
// and make every count assertion below depend on them.
$this->em->createQuery(
'DELETE FROM ' . Calendar::class . ' c WHERE c.usr = :usr',
)->setParameter('usr', $user)->execute();
$this->em->createQuery(
'DELETE FROM ' . Account::class . ' a WHERE a.usr = :usr',
)->setParameter('usr', $user)->execute();
$this->em->clear();
return $this->em->find(User::class, $user->id);
}
/** Another user, so ownership can be tested against a real second owner. */
private function stranger(): User
{
$user = new User();
$user->email = 'stranger-' . bin2hex(random_bytes(4)) . '@joder.dev';
$user->password = 'not-a-real-hash';
$user->nameFirst = 'Stran';
$user->nameLast = 'Ger';
$this->em->persist($user);
$this->em->flush();
return $user;
}
private function account(User $user, string $email): Account
{
$account = new Account();
$account->usr = $user;
$account->name = $email;
$account->username = $email;
$account->email = $email;
$account->authType = AuthType::Password->value;
$account->isActive = true;
$account->imapHost = 'imap.example.test';
$this->em->persist($account);
$this->em->flush();
return $account;
}
/** A connected, working OAuth account — the healthy baseline. */
private function oauthAccount(User $user, string $email): Account
{
$account = new Account();
$account->usr = $user;
$account->name = $email;
$account->username = $email;
$account->email = $email;
$account->authType = AuthType::OAuth2->value;
$account->oauthProvider = MailProvider::Google->value;
$account->oauthAccessToken = 'access-token';
$account->oauthRefreshToken = 'refresh-token';
$account->isActive = true;
$this->em->persist($account);
$this->em->flush();
return $account;
}
private function calendar(User $user, string $name): Calendar
{
$calendar = new Calendar();
$calendar->usr = $user;
$calendar->name = $name;
$this->em->persist($calendar);
$this->em->flush();
return $calendar;
}
/** A calendar that mirrors a mail account's provider — role Remote. */
private function remoteCalendar(User $user, Account $account, string $name): Calendar
{
$calendar = new Calendar();
$calendar->usr = $user;
$calendar->account = $account;
$calendar->name = $name;
$calendar->role = CalendarRole::Remote;
$calendar->remoteId = 'remote-' . bin2hex(random_bytes(4));
$this->em->persist($calendar);
$this->em->flush();
return $calendar;
}
}
@@ -0,0 +1,242 @@
<?php
declare(strict_types=1);
namespace App\Tests\Entity\Calendar;
use App\Entity\Calendar\Calendar;
use DateTimeImmutable;
use PHPUnit\Framework\TestCase;
/**
* The backoff that stops a permanently failing calendar from re-logging the
* same line every fifteen minutes forever.
*
* WHAT THIS IS FOR
* ────────────────
* On the install this was written from, one expired Google sign-in produced
* 2 193 identical `CalendarSync: sync failed` lines and 503 dead-lettered jobs
* across three calendars in two days. The mechanism was not subtle: the sweep
* asks for calendars whose lastSyncedAt is old, a calendar that cannot sync
* never updates lastSyncedAt, so it is due again on every single sweep.
*
* The tests below pin the three properties that make the fix a backoff rather
* than a mute button, because a mute button would have passed a naive test
* suite just as well:
*
* - the FIRST failure always reports (testTheFirstFailureIsAlwaysReported)
* - a CHANGED failure reports immediately, whatever the window says
* (testADifferentFailureIsReportedEvenInsideTheWindow)
* - the window is bounded, so a condition that silently heals is retried
* within a day rather than in a fortnight (testTheDelayIsCappedAtOneDay)
*
* Times are injected throughout. A test that slept would be a test nobody runs.
*/
final class CalendarSyncBackoffTest extends TestCase
{
private const string REASON = 'Google would not renew the sign-in for this account.';
public function testAHealthyCalendarIsNotBackingOff(): void
{
$calendar = new Calendar();
self::assertFalse($calendar->isBackingOff());
self::assertNull($calendar->syncBackoffUntil);
self::assertSame(0, $calendar->syncFailureCount);
}
/**
* The rule the whole feature would be worthless without: suppressing the
* first occurrence would mean a calendar could break and say nothing.
*/
public function testTheFirstFailureIsAlwaysReported(): void
{
$calendar = new Calendar();
$now = new DateTimeImmutable('2026-08-13 09:00:00');
self::assertTrue(
$calendar->recordSyncFailure(self::REASON, $now),
'the first failure a calendar ever has is news',
);
self::assertSame(self::REASON, $calendar->lastSyncError);
self::assertSame(1, $calendar->syncFailureCount);
self::assertTrue($calendar->isBackingOff($now));
}
/**
* The 2 193 lines, prevented. Same calendar, same error, still inside the
* window it already agreed to wait out — nothing further is logged.
*/
public function testRepeatingTheSameFailureInsideTheWindowIsNotReported(): void
{
$calendar = new Calendar();
$start = new DateTimeImmutable('2026-08-13 09:00:00');
self::assertTrue($calendar->recordSyncFailure(self::REASON, $start));
// The messenger retries of the very same envelope, moments later.
self::assertFalse($calendar->recordSyncFailure(self::REASON, $start->modify('+2 seconds')));
self::assertFalse($calendar->recordSyncFailure(self::REASON, $start->modify('+10 seconds')));
// And the sweeps that would have come round in the meantime.
self::assertFalse($calendar->recordSyncFailure(self::REASON, $start->modify('+5 minutes')));
self::assertFalse($calendar->recordSyncFailure(self::REASON, $start->modify('+14 minutes')));
// The schedule did NOT advance on the silent ones. Otherwise a single
// afternoon of retries would push the next attempt out by a day.
self::assertSame(1, $calendar->syncFailureCount, 'silent repeats do not advance the schedule');
}
/**
* The window expires and the calendar reports again — quieter, not muted.
* This is what keeps the condition visible in the log over a long outage.
*/
public function testTheFailureIsReportedAgainOnceTheWindowExpires(): void
{
$calendar = new Calendar();
$start = new DateTimeImmutable('2026-08-13 09:00:00');
$calendar->recordSyncFailure(self::REASON, $start);
// First window is a quarter of an hour.
self::assertFalse($calendar->recordSyncFailure(self::REASON, $start->modify('+14 minutes')));
self::assertTrue(
$calendar->recordSyncFailure(self::REASON, $start->modify('+16 minutes')),
'once the window is out, the condition says so again',
);
self::assertSame(2, $calendar->syncFailureCount);
}
/**
* The case a plain "log at most once an hour" throttle would have hidden,
* and the one that matters most: the calendar was failing on a dead
* sign-in, and is now failing on something else entirely.
*/
public function testADifferentFailureIsReportedEvenInsideTheWindow(): void
{
$calendar = new Calendar();
$start = new DateTimeImmutable('2026-08-13 09:00:00');
$calendar->recordSyncFailure(self::REASON, $start);
self::assertTrue(
$calendar->recordSyncFailure('The calendar no longer exists at the remote.', $start->modify('+1 minute')),
'a different failure is news whatever the window says',
);
self::assertSame('The calendar no longer exists at the remote.', $calendar->lastSyncError);
}
/**
* Doubling, so a calendar that is genuinely dead costs less and less.
*/
public function testTheDelayGrowsWithConsecutiveFailures(): void
{
$calendar = new Calendar();
$at = new DateTimeImmutable('2026-08-13 09:00:00');
$windows = [];
for ($i = 0; $i < 5; ++$i) {
self::assertTrue($calendar->recordSyncFailure(self::REASON, $at));
$until = $calendar->syncBackoffUntil;
$windows[] = $until->getTimestamp() - $at->getTimestamp();
// Jump to the moment the window opens again.
$at = $until->modify('+1 second');
}
self::assertSame([900, 1800, 3600, 7200, 14400], $windows);
}
/**
* The cap is the part that stops backoff becoming abandonment. Without it a
* calendar that failed for a fortnight would next be tried a fortnight
* later, so a sign-in repaired this morning would sit dark for weeks.
*/
public function testTheDelayIsCappedAtOneDay(): void
{
$calendar = new Calendar();
$at = new DateTimeImmutable('2026-08-13 09:00:00');
for ($i = 0; $i < 20; ++$i) {
$calendar->recordSyncFailure(self::REASON, $at);
$at = $calendar->syncBackoffUntil->modify('+1 second');
}
$last = $calendar->recordSyncFailure(self::REASON, $at);
self::assertTrue($last);
self::assertSame(
86400,
$calendar->syncBackoffUntil->getTimestamp() - $at->getTimestamp(),
'however long it has been broken, it is still tried once a day',
);
}
/**
* One success is enough to earn the full rate back. A calendar that
* recovered but stayed on an hourly schedule would look stale with nothing
* on screen to explain it.
*/
public function testASuccessClearsTheBackoffCompletely(): void
{
$calendar = new Calendar();
$at = new DateTimeImmutable('2026-08-13 09:00:00');
for ($i = 0; $i < 4; ++$i) {
$calendar->recordSyncFailure(self::REASON, $at);
$at = $calendar->syncBackoffUntil->modify('+1 second');
}
self::assertGreaterThan(1, $calendar->syncFailureCount);
$calendar->recordSyncSuccess();
self::assertNull($calendar->lastSyncError);
self::assertNull($calendar->syncBackoffUntil);
self::assertSame(0, $calendar->syncFailureCount);
self::assertFalse($calendar->isBackingOff());
// ...and the next failure after a recovery is news all over again.
self::assertTrue($calendar->recordSyncFailure(self::REASON, $at));
}
/**
* The repair button's half: clear the schedule, but do NOT pretend the
* calendar is healthy. lastSyncError is still true until a sync succeeds,
* and blanking it here would empty the health page the instant the button
* was pressed whether or not anything got better.
*/
public function testClearingTheBackoffKeepsTheStoredError(): void
{
$calendar = new Calendar();
$at = new DateTimeImmutable('2026-08-13 09:00:00');
$calendar->recordSyncFailure(self::REASON, $at);
$calendar->clearSyncBackoff();
self::assertFalse($calendar->isBackingOff($at));
self::assertSame(0, $calendar->syncFailureCount);
self::assertSame(self::REASON, $calendar->lastSyncError, 'still broken until a sync says otherwise');
}
/**
* The flag SyncCalendarHandler reads to decide whether to log, pinned
* directly — the handler is thin enough that this IS the behaviour.
*/
public function testTheNewsFlagTracksTheReturnValue(): void
{
$calendar = new Calendar();
$at = new DateTimeImmutable('2026-08-13 09:00:00');
$calendar->recordSyncFailure(self::REASON, $at);
self::assertTrue($calendar->syncFailureWasNews);
$calendar->recordSyncFailure(self::REASON, $at->modify('+1 minute'));
self::assertFalse($calendar->syncFailureWasNews);
}
}
@@ -0,0 +1,335 @@
<?php
declare(strict_types=1);
namespace App\Tests\Service\OAuth;
use App\Domain\Enum\Account\AuthType;
use App\Domain\Enum\Account\MailProvider;
use App\Domain\Enum\Mail\MessageCategory;
use App\Domain\Enum\Mail\ThreadingMethod;
use App\Domain\Exception\AccountIdentityMismatch;
use App\Entity\Label\Label;
use App\Entity\Mail\Account;
use App\Entity\Mail\Message;
use App\Entity\Mail\MessageThread;
use App\Entity\User\User;
use App\Repository\User\UserRepository;
use App\Service\OAuth\OAuthAccountLinker;
use DateTimeImmutable;
use Doctrine\DBAL\Connection;
use Doctrine\ORM\EntityManagerInterface;
use League\OAuth2\Client\Token\AccessToken;
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
/**
* Reconnecting an account in place — the repair this whole feature is for.
*
* WHAT THIS IS FOR
* ────────────────
* When an OAuth grant dies, the only remedy plMail offered was to delete the
* account and add it back, which throws away every message, thread, label
* assignment, rule and calendar hanging off that account id. People do it
* anyway, because nothing tells them there is another way. relink() is the
* other way, and these tests are the promise the settings page makes on its
* behalf: your mail is still there afterwards.
*
* THE TEST THAT MATTERS MOST is testReconnectingAsADifferentMailboxIsRefused.
* A user with two Google accounts, signed into the wrong one in their browser,
* sails through the consent screen without noticing — and the tokens that come
* back are perfectly valid, for somebody else's mail. Writing them onto this
* row would make every later sync file a stranger's messages into these
* threads, and there is no undo for that short of a restore. So the address is
* compared before anything is written, and a mismatch is refused rather than
* resolved cleverly.
*/
final class OAuthAccountRelinkTest extends KernelTestCase
{
private const string ADMIN_EMAIL = 'e2e-admin@plmail.test';
private EntityManagerInterface $em;
private Connection $connection;
private OAuthAccountLinker $linker;
private User $user;
protected function setUp(): void
{
self::bootKernel();
$container = static::getContainer();
$this->em = $container->get(EntityManagerInterface::class);
$this->connection = $container->get(Connection::class);
$this->linker = $container->get(OAuthAccountLinker::class);
$user = $container->get(UserRepository::class)->findOneBy(['email' => self::ADMIN_EMAIL]);
if (null === $user) {
self::markTestSkipped('run `app:test:seed-user --admin` first');
}
$this->connection->beginTransaction();
$this->user = $user;
}
protected function tearDown(): void
{
if (true === isset($this->connection) && true === $this->connection->isTransactionActive()) {
$this->connection->rollBack();
}
parent::tearDown();
}
/**
* The promise the button makes, asserted on the things that would actually
* be lost by a delete-and-re-add: the row identity itself, and everything
* keyed to it.
*/
public function testReconnectingKeepsTheAccountRowAndEverythingOnIt(): void
{
$account = $this->deadAccount('keeper@joder.dev');
$label = $this->label('Receipts');
$thread = $this->thread($account, 'An invoice', $label);
$accountId = $account->id;
$threadId = $thread->id;
$this->linker->relink($account, MailProvider::Google, 'keeper@joder.dev', $this->token('fresh-access'));
$this->em->clear();
$reloaded = $this->em->find(Account::class, $accountId);
// The SAME row. Not a new one beside it, which is what link() would
// have produced if the address had differed by so much as a case.
self::assertNotNull($reloaded);
self::assertSame($accountId, $reloaded->id);
self::assertSame('keeper@joder.dev', $reloaded->email);
// The mail is still hanging off it.
$keptThread = $this->em->find(MessageThread::class, $threadId);
self::assertNotNull($keptThread, 'the thread survived the reconnect');
self::assertSame($accountId, $keptThread->account->id);
self::assertCount(1, $keptThread->messages, 'and so did its message');
self::assertSame('An invoice', $keptThread->subject);
// Including the label assignment, which lives on the join table and is
// the first casualty of deleting the account.
self::assertCount(1, $keptThread->labels);
self::assertSame('Receipts', $keptThread->labels->first()->name);
}
/**
* The new tokens really are written, and the stored failure really is
* cleared — the two lines that turn the health page from a list of
* complaints into something that goes away when fixed.
*
* Without the clear, the account would work again while still reporting
* itself broken: OAuthTokenManager only clears the error on a successful
* REFRESH, and a reconnect never goes through refresh().
*/
public function testReconnectingWritesTheNewTokensAndClearsTheStoredFailure(): void
{
$account = $this->deadAccount('tokens@joder.dev');
self::assertSame('invalid_grant', $account->oauthLastRefreshError);
$this->linker->relink($account, MailProvider::Google, 'tokens@joder.dev', $this->token('fresh-access', 'fresh-refresh'));
$this->em->clear();
$reloaded = $this->em->find(Account::class, $account->id);
self::assertSame('fresh-access', $reloaded->oauthAccessToken);
self::assertSame('fresh-refresh', $reloaded->oauthRefreshToken);
self::assertNull($reloaded->oauthLastRefreshError, 'the health page has to be able to go quiet');
self::assertNotNull($reloaded->oauthLastRefreshAt);
}
/**
* A provider that does not rotate refresh tokens omits it from the
* response, and the stored one stays valid. Overwriting with null would
* break every later refresh — which is the bug that would turn one
* reconnect into a reconnect every hour.
*/
public function testAResponseWithNoRefreshTokenKeepsTheStoredOne(): void
{
$account = $this->deadAccount('norotate@joder.dev');
$this->linker->relink($account, MailProvider::Google, 'norotate@joder.dev', $this->token('fresh-access'));
$this->em->clear();
$reloaded = $this->em->find(Account::class, $account->id);
self::assertSame('stale-refresh', $reloaded->oauthRefreshToken);
}
/**
* A grant that died while the account was switched off. Reconnecting is an
* unambiguous "I want this working", so it comes back on.
*/
public function testReconnectingReactivatesADisabledAccount(): void
{
$account = $this->deadAccount('disabled@joder.dev');
$account->isActive = false;
$this->em->flush();
$this->linker->relink($account, MailProvider::Google, 'disabled@joder.dev', $this->token('fresh-access'));
$this->em->clear();
self::assertTrue($this->em->find(Account::class, $account->id)->isActive);
}
// ── the identity guard ───────────────────────────────────────────────────
/**
* THE one that must never regress. Signing in as the wrong Google account
* is a mistake anybody makes, and it must cost a refusal rather than a
* mailbox.
*/
public function testReconnectingAsADifferentMailboxIsRefused(): void
{
$account = $this->deadAccount('mine@joder.dev');
$thread = $this->thread($account, 'Still mine');
$this->expectException(AccountIdentityMismatch::class);
try {
$this->linker->relink($account, MailProvider::Google, 'someone-else@joder.dev', $this->token('their-access', 'their-refresh'));
} finally {
// Nothing was written on the way to the refusal. The check runs
// before the first assignment precisely so this holds.
$this->em->clear();
$reloaded = $this->em->find(Account::class, $account->id);
self::assertSame('mine@joder.dev', $reloaded->email, 'the account still points at its own mailbox');
self::assertSame('stale-access', $reloaded->oauthAccessToken, 'and carries none of the stranger\'s tokens');
self::assertSame('stale-refresh', $reloaded->oauthRefreshToken);
self::assertSame('invalid_grant', $reloaded->oauthLastRefreshError, 'still reported as broken');
self::assertNotNull(
$this->em->find(MessageThread::class, $thread->id),
'and the mail that was never at risk is still there',
);
}
}
/**
* Providers do not agree with themselves about capitalisation between the
* id_token and the profile response. A refusal over `A` versus `a` would be
* a wall with no way through it, so the comparison is case-insensitive.
*/
public function testADifferentlyCapitalisedAddressIsTheSameMailbox(): void
{
$account = $this->deadAccount('casing@joder.dev');
$this->linker->relink($account, MailProvider::Google, 'Casing@Joder.dev', $this->token('fresh-access'));
$this->em->clear();
self::assertNull($this->em->find(Account::class, $account->id)->oauthLastRefreshError);
}
/**
* The same address can legitimately exist as both a Google and a Microsoft
* account — see OAuthAccountLinker::upsert(). Re-pointing one at the other
* is the same corruption by a slower route.
*/
public function testReconnectingWithADifferentProviderIsRefused(): void
{
$account = $this->deadAccount('crossed@joder.dev');
$this->expectException(AccountIdentityMismatch::class);
$this->linker->relink($account, MailProvider::Microsoft, 'crossed@joder.dev', $this->token('fresh-access'));
}
// ── helpers ──────────────────────────────────────────────────────────────
/** An OAuth account in exactly the state the live install was found in. */
private function deadAccount(string $email): Account
{
$account = new Account();
$account->usr = $this->user;
$account->name = $email;
$account->username = $email;
$account->email = $email;
$account->authType = AuthType::OAuth2->value;
$account->oauthProvider = MailProvider::Google->value;
$account->oauthAccessToken = 'stale-access';
$account->oauthRefreshToken = 'stale-refresh';
$account->oauthLastRefreshError = 'invalid_grant';
$account->isActive = true;
$this->em->persist($account);
$this->em->flush();
return $account;
}
private function label(string $name): Label
{
$label = new Label();
$label->usr = $this->user;
$label->name = $name;
$label->isVisible = true;
$this->em->persist($label);
$this->em->flush();
return $label;
}
private function thread(Account $account, string $subject, ?Label $label = null): MessageThread
{
$thread = new MessageThread();
$thread->account = $account;
$thread->subject = $subject;
$thread->normalizedSubject = mb_strtolower($subject);
$thread->threadingMethod = ThreadingMethod::SubjectFallback;
$thread->lastMessageAt = new DateTimeImmutable();
$thread->category = MessageCategory::Primary;
$thread->messageCount = 1;
$thread->unreadCount = 0;
if (null !== $label) {
$thread->addLabel($label);
}
$this->em->persist($thread);
$message = new Message();
$message->account = $account;
$message->thread = $thread;
$message->subject = $subject;
$message->fromAddress = 'sender@example.test';
$message->receivedAt = new DateTimeImmutable();
$message->sentAt = $message->receivedAt;
$message->seenAt = new DateTimeImmutable();
$message->flags = [];
$message->hasAttachments = false;
$thread->addMessage($message);
$this->em->persist($message);
$this->em->flush();
return $thread;
}
private function token(string $access, ?string $refresh = null): AccessToken
{
$values = [
'access_token' => $access,
'expires' => time() + 3600,
];
if (null !== $refresh) {
$values['refresh_token'] = $refresh;
}
return new AccessToken($values);
}
}
+169
View File
@@ -0,0 +1,169 @@
import { expect } from "@playwright/test";
import { test } from "./support/test";
import { consoleCommand } from "./support/config";
/**
* The account-health section, in a browser.
*
* A browser spec rather than more controller tests because the things worth
* holding here are rendered ones and only meet the user as appearance:
*
* - the promise text is ABOVE the button, not in a title attribute. The whole
* reason people delete an account and re-add it is that "Reconnect" does not
* promise their mail survives; a promise nobody sees is not a promise.
* - the destructive repair is visually separated from the safe ones. A
* controller test can assert the markup order and would not notice the rule
* between them disappearing.
* - it has to be legible in both themes and at 393px. `emulateMedia` does
* nothing in this app — the theme is `data-theme` on <html> — so the dark
* captures set the attribute directly, which is also how a real user's
* choice arrives.
*
* Seeds its own broken state through the console and clears it afterwards, so
* it neither depends on nor leaves behind a damaged account for the specs that
* assert on account lists.
*/
/**
* The account `app:test:seed-mail` gives this worker. Named here because the
* assertions below read it off the page.
*/
const ACCOUNT = "E2E Mailbox";
/**
* Put this worker's existing account into the exact state the live install was
* found in: an OAuth account whose stored refresh error is `invalid_grant`
* while it is still marked active.
*
* The worker's OWN account is mutated rather than a fresh one inserted, and
* that is deliberate on two counts. It is closer to the real condition — a
* grant dies on an account that already has mail behind it, which is the whole
* reason reconnecting has to preserve things — and it avoids an INSERT that
* would have to name the `user` table, whose quoting has to survive a template
* literal, a shell, and Postgres reserving the word. `restoreTheAccount` puts
* it back for the specs that assert on the account list.
*/
function breakTheAccount(): void {
// NOT seeded here. The `workerAuth` fixture is worker-scoped and
// `auto: true`, so it has already run seedUser() and seed("seed-mail") and
// signed in before this hook is reached — the account exists.
//
// Calling seedUser() again here is actively harmful, and it cost an
// afternoon: it re-hashes the password, Symfony compares the stored hash
// against the one in the session token, decides the user has changed, and
// throws the session away. Every test then ran against the login page,
// which fails as "the health card is not visible" and looks exactly like a
// rendering bug.
consoleCommand(
`dbal:run-sql "UPDATE account SET auth_type = 'oauth2', oauth_provider = 'google', oauth_access_token = 'stale', oauth_refresh_token = 'stale', oauth_last_refresh_error = 'invalid_grant' WHERE email = '${ACCOUNT}'"`,
);
}
function restoreTheAccount(): void {
consoleCommand(
`dbal:run-sql "UPDATE account SET auth_type = 'password', oauth_provider = NULL, oauth_access_token = NULL, oauth_refresh_token = NULL, oauth_last_refresh_error = NULL WHERE email = '${ACCOUNT}'"`,
);
}
test.describe("account health", () => {
test.beforeAll(breakTheAccount);
test.afterAll(restoreTheAccount);
test.beforeEach(async ({ page }) => {
await page.goto("/settings?section=health");
});
test("surfaces the dead sign-in with a repair, in the user's words", async ({
page,
}) => {
const card = page.locator('[data-health-kind="account_reconnect"]');
await expect(card).toBeVisible();
// The headline names the mailbox and says what to do — no error code.
await expect(card.getByRole("heading")).toContainText(ACCOUNT);
await expect(card.getByRole("heading")).not.toContainText(
"invalid_grant",
);
// The promise is on the page, above the button that keeps it. This is
// the sentence that makes reconnecting a smaller act than deleting.
await expect(card).toContainText("stay exactly as they are");
await expect(card).toContainText("nothing is deleted");
// And the provider's own words are one disclosure away, not gone.
await expect(card.locator("details")).toBeVisible();
await expect(card.locator("details pre")).not.toBeVisible();
await card.locator("summary").click();
await expect(card.locator("details pre")).toContainText("invalid_grant");
});
test("the topbar carries an indicator that leads to this page", async ({
page,
}) => {
await page.goto("/");
const badge = page.locator("#user-menu-btn + span");
await expect(badge).toBeVisible();
await expect(badge).toHaveText("1");
// Following it lands on health, not on settings-in-general — an
// indicator that drops you somewhere you still have to hunt has only
// moved the hunting.
await page.locator("#user-menu-btn").click();
await page.locator("#user-menu-settings").click();
await expect(page).toHaveURL(/section=health/);
await expect(
page.locator('[data-health-kind="account_reconnect"]'),
).toBeVisible();
});
test("reads at 393px", async ({ page }) => {
await page.setViewportSize({ width: 393, height: 850 });
await page.goto("/settings?section=health");
const card = page.locator('[data-health-kind="account_reconnect"]');
await expect(card).toBeVisible();
// The card must not push the page sideways — the rule the whole
// settings pane follows.
const overflow = await page.evaluate(
() =>
document.documentElement.scrollWidth >
document.documentElement.clientWidth + 1,
);
expect(overflow).toBe(false);
});
for (const theme of ["light", "dark"] as const) {
test(`renders in the ${theme} theme`, async ({ page }, testInfo) => {
// emulateMedia does nothing here: the theme is an attribute on
// <html>, which is also how the user's own choice is applied.
await page.evaluate(
(t) => document.documentElement.setAttribute("data-theme", t),
theme,
);
const card = page.locator('[data-health-kind="account_reconnect"]');
await expect(card).toBeVisible();
// Proving it is styled rather than merely present: a card that lost
// its background would still be "visible".
const bg = await card.evaluate(
(el) => getComputedStyle(el).backgroundColor,
);
expect(bg).not.toBe("rgba(0, 0, 0, 0)");
await testInfo.attach(`health-${theme}.png`, {
body: await page.screenshot({ fullPage: true }),
contentType: "image/png",
});
});
}
});
+112
View File
@@ -1265,6 +1265,7 @@ header:
search:
placeholder: E-Mails durchsuchen…
user_menu:
account_attention: '%count% Dinge an deinen Konten brauchen Aufmerksamkeit'
admin: Administration
label: Benutzermenü für %email%
profile: Profil
@@ -1972,6 +1973,116 @@ settings:
then: Dann
when: Wenn
yes: Ja
health:
confirm:
destructive: >-
Damit werden die wartenden Aufgaben endgültig verworfen. Nichts
wird erneut versucht, und es lässt sich nicht rückgängig
machen. Fortfahren?
flash:
calendar_resync: '%calendar% wird jetzt neu synchronisiert. Lade die Seite gleich neu, um zu sehen, ob es geklappt hat.'
queue_discarded: '%count% liegen gebliebene Aufgaben verworfen.'
queue_retried: '%count% liegen gebliebene Aufgaben wieder eingereiht.'
heading: 'Zustand deiner Konten'
healthy:
body: >-
Deine Konten sind angemeldet, deine Kalender synchronisieren,
und es wartet nichts auf dich. Sollte einmal keine Post mehr
ankommen, steht hier, woran es liegt.
title: 'Alles funktioniert'
issue:
account_reconnect:
body: >-
Für dieses Konto kommt keine neue Post mehr an, und mit ihr
steht alles still, was daran hängt: die Kalender­synchroni­
sierung, Filter, die beim Empfang laufen, und das Senden
über diese Adresse. Die bereits geladene Post ist sicher
und weiterhin da. Das passiert, wenn ein Passwort geändert
oder die Zwei-Faktor-Anmeldung eingeschaltet wurde, oder
wenn die Anmeldung beim Anbieter widerrufen wurde.
title: '%account% muss sich neu anmelden'
calendar_sync_failing:
body: >-
Dieser Kalender übernimmt keine Änderungen mehr und zeigt
den Stand von seiner letzten erfolgreichen Synchroni­
sierung. Termine, die seither anderswo angelegt oder
verschoben wurden, fehlen hier — und deine Änderungen
werden nicht nach außen übertragen.
title: '%calendar% synchronisiert nicht mehr'
integration_reconnect:
body: >-
Anhänge in %connection% zu speichern und Dateien von dort
anzuhängen funktioniert erst wieder nach einer neuen
Verbindung. Deine Post ist davon nicht betroffen — es geht
nur um die Verbindung zu diesem Dienst. An den dort
abgelegten Dateien wurde nichts verändert.
title: '%connection% muss neu verbunden werden'
push_degraded:
body: >-
Deine Post kommt weiterhin an, nur nicht mehr im selben
Augenblick: Dieses Konto ruft wieder nach Zeitplan ab, neue
Nachrichten können also ein paar Minuten brauchen. Es geht
nichts verloren, und es eilt nicht.
title: 'Sofortzustellung ist für %account% aus'
queue_work_abandoned:
body: >-
%count% Hintergrund­aufgaben sind wiederholt gescheitert
und wurden beiseitegelegt, damit sie es nicht endlos weiter
versuchen. Meist ist das die Folge eines der Probleme
darüber und keine eigene Störung — behebe erst dieses und
reihe sie danach wieder ein.
title: '%count% Hintergrund­aufgaben wurden aufgegeben'
repair:
calendar_resync:
label: 'Jetzt synchronisieren'
promise: >-
Fordert diesen Kalender sofort zu einer neuen
Synchronisierung auf und setzt ihn zurück auf den normalen
Zeitplan. Es wird nichts gelöscht.
integration_reconnect:
label: 'Neu verbinden'
promise: >-
Führt dich zur erneuten Anmeldung und stellt danach die
bestehende Verbindung wieder her. Deine Filter und
gespeicherten Ablageorte bleiben erhalten.
push_resubscribe:
label: 'Sofortzustellung wieder einschalten'
promise: >-
Meldet dieses Konto erneut für die Sofortzustellung an.
Deine Post kommt in der Zwischenzeit ohnehin weiter an.
queue_discard:
label: 'Verwerfen'
promise: >-
Wirft die liegen gebliebenen Aufgaben weg, ohne sie
auszuführen. Das lässt sich nicht rückgängig machen, und
was sie nicht erledigt haben, bleibt unerledigt.
queue_retry:
label: 'Wieder einreihen'
promise: >-
Versucht die liegen gebliebenen Aufgaben noch einmal.
Unbedenklich — was erneut scheitert, landet einfach wieder
hier.
reconnect:
label: 'Konto neu verbinden'
promise: >-
Führt dich zur erneuten Anmeldung und hinterlegt die neue
Anmeldung an genau diesem Konto. Deine Post, Ordner,
Labels, Filter, Kalender und Einstellungen bleiben
unverändert — es wird nichts erneut geladen und nichts
gelöscht. Du musst dich als %account% anmelden; eine andere
Adresse wird abgelehnt und nicht stillschweigend
übernommen.
reconnect:
done: '%account% ist wieder verbunden. Post und Kalender holen in den nächsten Minuten auf.'
wrong_account: >-
Damit hast du dich als %actual% angemeldet, dieses Konto ist
aber %expected%. Es wurde nichts verändert. Melde dich beim
Anbieter von %actual% ab und versuche es erneut — die Adresse
zu tauschen hätte ein fremdes Postfach unter dieses gemischt.
subheading: >-
Was nicht mehr funktioniert, was das für deine Post bedeutet, und
der kleinste Schritt, der es behebt.
technical_detail: 'Technische Einzelheiten'
integrations:
ask_admin: 'Deine Administration hat diesen Dienst nicht freigeschaltet.'
auth:
@@ -2155,6 +2266,7 @@ settings:
calendar: Kalender
mail: E-Mail
personal: Persönlich
health: 'Zustand der Konten'
profile: Profil
filters: Filter
integrations: Integrationen
+106
View File
@@ -1255,6 +1255,7 @@ header:
search:
placeholder: Search mail…
user_menu:
account_attention: '%count% things about your accounts need attention'
admin: Admin
label: User menu for %email%
profile: Profile
@@ -1957,6 +1958,110 @@ settings:
then: Then
when: When
yes: Yes
health:
confirm:
destructive: >-
This throws the waiting work away for good. Nothing is retried
and it cannot be brought back. Continue?
flash:
calendar_resync: '%calendar% is syncing again now. Reload in a moment to see whether it worked.'
queue_discarded: 'Discarded %count% abandoned jobs.'
queue_retried: 'Put %count% abandoned jobs back on the queue.'
heading: 'Account health'
healthy:
body: >-
Your accounts are signed in, your calendars are syncing and
nothing is waiting on you. If mail ever stops arriving, this is
the page that will say why.
title: 'Everything is working'
issue:
account_reconnect:
body: >-
New mail has stopped arriving for this account, and so has
everything that depends on it — calendar syncing, filters
that run on delivery, and sending from this address. The
mail already downloaded is safe and still here. This
happens when a password is changed, two-factor
authentication is switched on, or the sign-in is revoked
from the provider's security page.
title: '%account% needs you to sign in again'
calendar_sync_failing:
body: >-
This calendar has stopped picking up changes, so it is
showing what it knew the last time it worked. Events added
or moved elsewhere since then are not here yet, and changes
you make here are not being sent out.
title: '%calendar% has stopped syncing'
integration_reconnect:
body: >-
Saving attachments to %connection% and attaching files from
it will not work until it is connected again. Your mail is
unaffected — this is only the link to that service. Nothing
stored there has been touched.
title: '%connection% needs reconnecting'
push_degraded:
body: >-
Your mail is still arriving, just not the instant it is
sent — this account has fallen back to checking on a
schedule, so new messages can take a few minutes to show
up. Nothing is lost, and there is no rush to fix it.
title: 'Instant delivery is off for %account%'
queue_work_abandoned:
body: >-
%count% background jobs tried, failed repeatedly, and were
set aside so they would stop retrying. This is usually the
aftermath of one of the problems above rather than a
separate fault — fix that first, then put these back on the
queue.
title: '%count% background jobs were given up on'
repair:
calendar_resync:
label: 'Try syncing now'
promise: >-
Asks this calendar to sync again right away, and puts it
back on the normal schedule. Nothing on it is deleted.
integration_reconnect:
label: 'Reconnect'
promise: >-
Sends you to sign in again, then restores the existing
connection. Your filters and saved destinations keep
working.
push_resubscribe:
label: 'Turn instant delivery back on'
promise: >-
Registers this account for instant delivery again. Your
mail keeps arriving either way while this is sorted out.
queue_discard:
label: 'Discard them'
promise: >-
Throws the abandoned jobs away without running them. There
is no undo, and anything they had not finished stays
unfinished.
queue_retry:
label: 'Put them back on the queue'
promise: >-
Tries the abandoned jobs once more. Safe to press — any
that fail again simply end up back here.
reconnect:
label: 'Reconnect this account'
promise: >-
Takes you to sign in again, then puts the new sign-in onto
this same account. Your mail, folders, labels, filters,
calendars and settings all stay exactly as they are —
nothing is downloaded again and nothing is deleted. You
must sign in as %account%; if you pick a different address
it will be refused rather than swapped in.
reconnect:
done: '%account% is connected again. Mail and calendar syncing will catch up over the next few minutes.'
wrong_account: >-
That signed you in as %actual%, but this account is %expected%.
Nothing has been changed. Sign out of %actual% with that
provider, then try again — swapping the address would have
mixed a different mailbox into this one.
subheading: >-
Anything that has stopped working, what it means for your mail, and
the smallest thing that fixes it.
technical_detail: 'Technical detail'
integrations:
ask_admin: 'Your administrator has not enabled this service.'
auth:
@@ -2138,6 +2243,7 @@ settings:
calendar: Calendar
mail: Mail
personal: Personal
health: 'Account health'
profile: Profile
filters: Filters
integrations: Integrations
+105
View File
@@ -983,6 +983,7 @@ header:
search:
placeholder: Search the hold…
user_menu:
account_attention: "%count% things about yer crew be needin' an eye"
admin: Cap'n's quarters
label: Crew menu for %email%
profile: Yer colours
@@ -1645,6 +1646,109 @@ settings:
then: Then
when: When
yes: Aye
health:
confirm:
destructive: >-
This heaves th'' waitin'' work overboard fer good. Naught be
tried again, an'' naught comes back up. Carry on?
flash:
calendar_resync: '%calendar% be sailin'' again now. Come back in a moment t'' see if she held.'
queue_discarded: 'Heaved %count% abandoned tasks overboard.'
queue_retried: 'Put %count% abandoned tasks back in th'' queue.'
heading: 'State o'' yer crew'
healthy:
body: >-
Yer crew be signed on, yer almanacs be sailin'' true, an''
naught waits on ye. Should th'' post ever stop comin'' in,
this be th'' page that says why.
title: 'All hands healthy'
issue:
account_reconnect:
body: >-
No fresh post be comin'' in fer this one, an'' neither be
aught that hangs off it — th'' almanac, th'' sieves that
run on arrival, nor sendin'' from this address. Th'' post
ye already hauled aboard be safe an'' still here. This
happens when a secret word be changed, when th'' two-key
lock be thrown, or when th'' harbourmaster tears up yer
papers.
title: '%account% must sign on again'
calendar_sync_failing:
body: >-
This almanac has stopped takin'' on changes, so she shows
what she knew th'' last time she sailed. Aught added or
shifted elsewhere since then be not aboard, an'' yer own
changes be not gettin'' out.
title: '%calendar% has stopped sailin'''
integration_reconnect:
body: >-
Stowin'' cargo at %connection% an'' haulin'' it back will
not work till she be joined again. Yer post be untouched —
this be only th'' rope t'' that port. Naught stowed there
has been disturbed.
title: '%connection% needs joinin'' again'
push_degraded:
body: >-
Yer post still arrives, only not th'' instant it be sent —
this one has fallen back t'' checkin'' on th'' bell, so
fresh letters can take some minutes t'' show. Naught be
lost, an'' there be no hurry.
title: 'Instant delivery be off fer %account%'
queue_work_abandoned:
body: >-
%count% below-decks tasks tried, failed again an'' again,
an'' were set aside so they''d stop hammerin'' away. Most
times this be th'' wake o'' one o'' th'' troubles above
rather than a fault o'' its own — mend that first, then put
these back in th'' queue.
title: '%count% below-decks tasks were given up on'
repair:
calendar_resync:
label: 'Try sailin'' now'
promise: >-
Bids this almanac sail again straight away, an'' puts her
back on th'' usual bell. Naught aboard her be thrown out.
integration_reconnect:
label: 'Join again'
promise: >-
Sends ye t'' sign on again, then restores th'' rope ye
already had. Yer sieves an'' stowin'' places keep workin''.
push_resubscribe:
label: 'Set instant delivery back on'
promise: >-
Signs this one up fer instant delivery again. Yer post
keeps arrivin'' either way while it be sorted.
queue_discard:
label: 'Heave ''em overboard'
promise: >-
Throws th'' abandoned tasks away without runnin'' them.
There be no haulin'' them back, an'' what they left undone
stays undone.
queue_retry:
label: 'Put ''em back in th'' queue'
promise: >-
Tries th'' abandoned tasks once more. Safe t'' press — any
that fail again just wash back up here.
reconnect:
label: 'Sign this one on again'
promise: >-
Takes ye t'' sign on again, then sets th'' fresh papers on
this very same crew member. Yer post, yer holds, yer flags,
yer sieves, yer almanacs an'' yer riggin'' all stay exactly
as they be — naught be hauled down twice an'' naught be
thrown out. Ye must sign on as %account%; pick a different
address an'' it be refused, not swapped in.
reconnect:
done: '%account% be signed on again. Post an'' almanacs will catch up over th'' next few minutes.'
wrong_account: >-
That signed ye on as %actual%, but this one be %expected%.
Naught has been changed. Go ashore from %actual% with that
harbourmaster, then try again — swappin'' th'' address would
have mixed a stranger''s post in with yer own.
subheading: >-
Aught that has stopped workin'', what it means fer yer post, an''
th'' smallest thing that mends it.
technical_detail: 'Th'' fine print'
integrations:
ask_admin: 'Th'' quartermaster has not opened this port.'
auth:
@@ -1826,6 +1930,7 @@ settings:
calendar: "Th' almanac"
mail: "Th' post"
personal: Yerself
health: "State o' yer crew"
profile: Yer colours
filters: Sieves
integrations: 'Friendly ports'