From d4fcb7e311e51bba6d7579e9facd39b4a9917f48 Mon Sep 17 00:00:00 2001 From: Sylvain Duchesne Date: Mon, 12 Oct 2026 16:27:02 +0200 Subject: [PATCH] Log out when the app was removed from the pod instead of re-asking consent A registration read as installed:false means the user removed Festipod from their pod, often on purpose: re-running registerApp sent them to consent on every load with no way to sign out. The probe now logs out through the provider and lands on the picker with a notice that the authorization was removed and they can sign in again; Back and reload stay there. upgradeNeeded:true keeps the automatic consent redirect. The revocation e2e asserts the new path and fails on the previous one. --- scripts/e2e-revocation.ts | 62 +++++++++++++++++++++-------- src/app/AuthGate.tsx | 37 +++++++++-------- src/app/auth/appStatusProbe.test.ts | 44 ++++++++++++++++++++ src/app/auth/appStatusProbe.ts | 41 ++++++++++++++++++- 4 files changed, 147 insertions(+), 37 deletions(-) diff --git a/scripts/e2e-revocation.ts b/scripts/e2e-revocation.ts index ea4cb84..fa51009 100644 --- a/scripts/e2e-revocation.ts +++ b/scripts/e2e-revocation.ts @@ -14,14 +14,16 @@ * lands at the end of the minute and reads current. * 2. REVOKED — the registration is removed on the pod * (`POST {pod}/.auth-agent/app-registrations/remove`, the user's own token). - * (a) Without any reload, a window focus inside the minute — the user coming + * Without any reload, a window focus inside the minute — the user coming * back from the pod's tab — must, by the end of that minute, read the - * revocation and take the app off the connected view: the consent screen - * (the provider's `registerApp` redirect) or the picker. (b) Reloading the - * app on the event form must do the same through the load probe, and the - * event-creation drive that worked in phase 1 must not reach success: no - * "Événement relayé", no event page, and nothing with that title on the pod. - * 3. RECOVERED — accepting the consent screen brings the user back to a + * revocation (`installed:false`) and LOG THE USER OUT: the picker with the + * notice that the app's authorization was removed, NO redirect to the + * consent screen (a deliberate revocation is not sent back to consent), no + * session left (Back and a fresh load stay on the picker; the event-creation + * drive that worked in phase 1 lands on the picker, never on success, and + * nothing with that title reaches the pod). + * 3. RECOVERED — signing in again from the picker goes through + * `registerApp` → the consent screen; accepting it brings the user back to a * connected app whose probe reads current again, and a write lands. * * Nothing is stubbed: the probe result is whatever the pod answers. The probe @@ -314,22 +316,40 @@ function benign(e: HttpError): boolean { const leftByFocusTo = await whereIs(page); await snap(page, 'revocation-revoked-left-by-focus'); - // (b) RELOAD on the write path — the same event-creation drive that worked in - // phase 1: the load probe must take the app off again, and nothing may - // pretend the write succeeded. + const notice = page.locator('[data-testid="auth-picker-notice"]'); + const noticeShown = onApp(page) && (await notice.count().catch(() => 0)) > 0; + const noticeText = noticeShown ? ((await notice.first().innerText().catch(() => '')) as string).replace(/\s+/g, ' ').trim() : ''; + + // (b) No session left: Back must not return to the connected view, a fresh + // load must stay on the picker, and the same event-creation drive that worked + // in phase 1 must not reach success (it lands on the picker). + await page.goBack({ waitUntil: 'domcontentloaded' }).catch(() => {}); + await page.waitForTimeout(2000); + const stillPickerAfterBack = (await whereIs(page)) === 'picker'; + await page.goto(`${APP}/`, { waitUntil: 'domcontentloaded' }).catch(() => {}); + await page.waitForTimeout(2500); + const stillPickerAfterReload = (await whereIs(page)) === 'picker'; const titleB = `Concert revoque ${USER}`; - const loadLogsBefore = probeLogs.length; const writeB = await createEvent(page, titleB); await snap(page, 'revocation-revoked-after-write-attempt'); const leftAfterReload = await waitFor(async () => !onApp(page) || (await pickerShown(page)), 15000); const leftAfterReloadTo = await whereIs(page); - const loadProbeSawRevoked = probeLogs.slice(loadLogsBefore).some(l => /probe \(load\) → stale \(not-installed\)/.test(l.text)); const writeBOnPod = await podHoldsEventTitled(user, titleB); + const consentRedirectInRevoked = redirects.some(r => r.startsWith('[revoked]') && r.includes(`${clean(POD_FRONT)}/authorize`)); + const pageWritesInRevoked = writes.filter(w => w.phase === 'revoked'); await snap(page, 'revocation-revoked-left'); // ── Phase 3: RECOVERED ────────────────────────────────────────────────── phase = 'recovered'; - const reconnected = leftAfterReloadTo === 'consent' ? await driveToApp(page) : false; + // Sign in again from the picker: login → registerApp → consent → app. + const signInAgain = page.getByRole('button', { name: /Se connecter avec localhost:3000/i }); + let reconnected = false; + if (leftAfterReloadTo === 'picker' && (await signInAgain.count().catch(() => 0))) { + await signInAgain.first().click().catch(() => {}); + await page.waitForTimeout(1500); + reconnected = await driveToApp(page); + } + const consentReachedOnSignIn = hops.some(h => h.startsWith(' [recovered] [authorize')); const probeReadsCurrentAgain = reconnected && await waitFor( () => statusSeen.some(s => s.phase === 'recovered' && s.status === 200 && isCurrent(s.body)), 15000, @@ -364,20 +384,28 @@ function benign(e: HttpError): boolean { focusProbe, leftByFocus, leftByFocusTo, - loadProbeSawRevoked, + noticeShown, + noticeText, + consentRedirectInRevoked, + pageWritesInRevoked, + stillPickerAfterBack, + stillPickerAfterReload, leftAfterReload, leftAfterReloadTo, writeAttempt: { ...writeB, onPod: writeBOnPod }, ok: (revoke.status === 200 || revoke.status === 204) - && deferredOnTime(focusProbe) && leftByFocus && (leftByFocusTo === 'consent' || leftByFocusTo === 'picker') - && loadProbeSawRevoked && leftAfterReload && (leftAfterReloadTo === 'consent' || leftAfterReloadTo === 'picker') + && deferredOnTime(focusProbe) && leftByFocus && leftByFocusTo === 'picker' + && noticeShown && noticeText.includes('retirée') + && !consentRedirectInRevoked && !pageWritesInRevoked.length + && stillPickerAfterBack && stillPickerAfterReload && leftAfterReloadTo === 'picker' && !writeB.eventUri && !writeB.successToast && !writeBOnPod, }; const recovered = { reconnected, + consentReachedOnSignIn, probeReadsCurrentAgain, write: { ...writeC, onPod: writeCOnPod }, - ok: reconnected && probeReadsCurrentAgain && !!writeC.eventUri && writeC.successToast && writeCOnPod, + ok: reconnected && consentReachedOnSignIn && probeReadsCurrentAgain && !!writeC.eventUri && writeC.successToast && writeCOnPod, }; // Phase 2's ≥400s are the revocation's own consequences; the other phases must be clean. const fatal = errors4xx.filter(e => e.phase !== 'revoked' && !benign(e)); diff --git a/src/app/AuthGate.tsx b/src/app/AuthGate.tsx index c5328ac..e51e93c 100644 --- a/src/app/AuthGate.tsx +++ b/src/app/AuthGate.tsx @@ -43,7 +43,7 @@ import { safeRedirectPath, } from './auth/activityPodsAuth'; import { PodProviderPicker } from './auth/PodProviderPicker'; -import { recoverRegistration, resolveStaleRegistration, type StaleReason } from './auth/appStatusProbe'; +import { handleStaleReading, type StaleReason } from './auth/appStatusProbe'; import { useAppStatusProbe } from './auth/useAppStatusProbe'; /** @@ -53,9 +53,9 @@ import { useAppStatusProbe } from './auth/useAppStatusProbe'; */ type ActivityPodsAuthState = 'checking' | 'authenticated' | 'unauthenticated' | 'reauthorizing' | 'error'; -/** Shown on the picker when a revoked/stale registration could not be re-run. */ +/** Shown on the picker when the app was removed from the pod and the user was logged out. */ const REGISTRATION_LOST_NOTICE = - 'Festipod n’est plus autorisé sur votre Pod. Reconnectez-vous pour l’autoriser à nouveau.'; + 'L’autorisation de Festipod a été retirée de votre Pod : vous avez été déconnecté. Vous pouvez vous reconnecter à tout moment.'; type CallbackResult = Awaited>; @@ -104,11 +104,11 @@ export function AuthGate({ children }: { children: ReactNode }) { // Why the picker is back after a session existed (registration lost). const [apNotice, setApNotice] = useState(null); // Set when the app-status probe took the connected view down: the consent - // redirect is under way ('reauthorizing'), or the registration is gone and - // could not be re-run ('registration-lost'). The provider's token is still - // stored (logout is not ours to trigger here), so without this any later - // route change — a Back, a link — would re-derive "authenticated" from that - // token and bring back a view that can neither read nor write. Lives for the + // redirect is under way ('reauthorizing'), or the app was removed from the + // pod and the user logged out ('registration-lost'). Should the logout itself + // fail the token stays stored, and without this any later route change — a + // Back, a link — would re-derive "authenticated" from it and bring back a + // view that can neither read nor write. Lives for the // page: the consent and the picker's sign-in both leave it for the pod, and a // reload probes afresh. const blockedRef = useRef<{ kind: 'reauthorizing'; reason: StaleReason } | { kind: 'registration-lost' } | null>(null); @@ -243,28 +243,27 @@ export function AuthGate({ children }: { children: ReactNode }) { return () => { cancelled = true; }; }, [activityPodsEnabled, apAuthProvider, demoMode, navigate, route.page]); - // A revoked or stale registration: re-run the provider's registration flow. - // Its consent redirect IS the recovery; a resolved URI means the registration - // is current after all and the app stays as it is. When it fails, what - // happens depends on what was found: a REVOKED registration (nothing can be - // read or written) lands on the picker with a notice; a registration that - // only needs an upgrade stays connected and is retried by the next probe. + // A revoked or stale registration. Revoked (the app was removed from the + // pod): log out through the provider and land on the picker with a notice — + // no consent redirect. Out of date (grants to update): re-run the provider's + // registration flow, whose consent redirect IS the recovery; if that cannot + // run, stay connected and let the next probe retry. const handleStaleRegistration = useCallback(async (reason: StaleReason) => { const provider = apAuthProvider; if (!provider || recoveringRef.current) return; recoveringRef.current = true; - let outcome: Awaited>; + let handled: Awaited>; try { - outcome = await recoverRegistration({ + handled = await handleStaleReading(reason, { getWebId: () => provider.getSession()?.webId, registerApp: (webId) => provider.registerApp(webId), + logout: async () => { await provider.logout({}); }, }); } finally { recoveringRef.current = false; } - const resolution = resolveStaleRegistration(reason, outcome); - console.info(`[AppStatus] recovery (${reason}) → ${outcome.kind} → ${resolution}`); - if (outcome.kind === 'failed') console.error('[AppStatus] registerApp failed:', outcome.detail); + const { resolution, detail } = handled; + console.info(`[AppStatus] handled (${reason}) → ${detail} → ${resolution}`); switch (resolution) { case 'stay-connected': { // From the probe this changes nothing; from the reauthorizing screen's diff --git a/src/app/auth/appStatusProbe.test.ts b/src/app/auth/appStatusProbe.test.ts index 4c84b66..885c203 100644 --- a/src/app/auth/appStatusProbe.test.ts +++ b/src/app/auth/appStatusProbe.test.ts @@ -3,6 +3,7 @@ import { MIN_PROBE_INTERVAL_MS, classifyAppStatus, createAppStatusProbe, + handleStaleReading, recoverRegistration, resolveStaleRegistration, type AppStatusProbeDeps, @@ -317,3 +318,46 @@ describe('resolveStaleRegistration', () => { expect(resolveStaleRegistration('upgrade-needed', failed)).toBe('stay-connected'); }); }); + +describe('handleStaleReading', () => { + function spies(opts: { registerApp?: () => Promise; logout?: () => Promise } = {}) { + const calls = { registerApp: 0, logout: 0 }; + return { + calls, + deps: { + getWebId: () => 'http://pod/alice' as string | undefined, + registerApp: async () => { calls.registerApp++; return opts.registerApp ? opts.registerApp() : undefined; }, + logout: async () => { calls.logout++; if (opts.logout) await opts.logout(); }, + }, + }; + } + + test('not-installed: logs out and never calls registerApp (no consent redirect)', async () => { + const { calls, deps } = spies(); + const out = await handleStaleReading('not-installed', deps); + expect(out.resolution).toBe('registration-lost'); + expect(calls).toEqual({ registerApp: 0, logout: 1 }); + }); + + test('not-installed: a logout that throws still leaves the connected view, without registerApp', async () => { + const { calls, deps } = spies({ logout: async () => { throw new Error('storage unavailable'); } }); + const out = await handleStaleReading('not-installed', deps); + expect(out.resolution).toBe('registration-lost'); + expect(out.detail).toContain('storage unavailable'); + expect(calls).toEqual({ registerApp: 0, logout: 1 }); + }); + + test('upgrade-needed: re-runs registerApp (consent redirect) and does not log out', async () => { + const { calls, deps } = spies(); + const out = await handleStaleReading('upgrade-needed', deps); + expect(out.resolution).toBe('reauthorizing'); + expect(calls).toEqual({ registerApp: 1, logout: 0 }); + }); + + test('upgrade-needed: a registerApp that throws fails open — stays connected, no logout', async () => { + const { calls, deps } = spies({ registerApp: async () => { throw new Error('boom'); } }); + const out = await handleStaleReading('upgrade-needed', deps); + expect(out.resolution).toBe('stay-connected'); + expect(calls).toEqual({ registerApp: 1, logout: 0 }); + }); +}); diff --git a/src/app/auth/appStatusProbe.ts b/src/app/auth/appStatusProbe.ts index 3f2f163..5c1df15 100644 --- a/src/app/auth/appStatusProbe.ts +++ b/src/app/auth/appStatusProbe.ts @@ -26,6 +26,14 @@ * check, which acts on `upgradeNeeded` only, would miss it. The probe treats * `installed === false` as stale too. * + * WHAT STALE LEADS TO (user decision, 2026-10-12). The two readings are not the + * same thing. `upgrade-needed` (registered, grants out of date) re-runs + * `registerApp`: its consent redirect is the recovery. `not-installed` (the app + * was removed from the pod — often on purpose) does NOT go back to consent: the + * user is logged out through the provider's `logout` and lands on the picker + * with a notice, free to sign in again. Sending a deliberate revocation to the + * consent screen on every load left that user unable to log out. + * * SCHEDULE. Event-driven, never a polling loop (the app has none, by * doctrine): on app load once signed in, and when the window regains focus or * becomes visible. Throttled — no two runs start less than a minute apart, and @@ -209,7 +217,7 @@ export async function recoverRegistration(deps: RecoveryDeps): Promise Promise; +} + +/** + * Act on one stale reading. A revoked registration (`not-installed`) is never + * sent back to consent: it logs out (a logout that throws is logged by the + * caller through the detail; the app still leaves the connected view) and + * resolves `registration-lost`. An out-of-date one (`upgrade-needed`) re-runs + * `registerApp` and resolves through `resolveStaleRegistration`. + */ +export async function handleStaleReading( + reason: StaleReason, + deps: StaleReadingDeps, +): Promise<{ resolution: StaleResolution; detail: string }> { + if (reason === 'not-installed') { + try { + await deps.logout(); + return { resolution: 'registration-lost', detail: 'logged out' }; + } catch (err) { + return { resolution: 'registration-lost', detail: `logout failed: ${describeFailure(err)}` }; + } + } + const outcome = await recoverRegistration(deps); + return { + resolution: resolveStaleRegistration(reason, outcome), + detail: outcome.kind === 'failed' ? `registerApp failed: ${outcome.detail}` : outcome.kind, + }; +}