From 0281ca62d247fd383b33a262deadb7aa784e2aee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Tue, 11 Aug 2026 17:38:02 +0000 Subject: [PATCH] a deleted or blocked account loses its session on the next request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported from two browser windows: an account deleted from the dashboard survived a page refresh in the other one. Two independent halves. Server: userMiddleware looked the account up, then read the result as `dbUser?.passwordChangedAt` — so a DELETED account fell through the optional chain and the request proceeded on a token that is still cryptographically valid, for up to the full 30 days. `status` was the same hole from the other direction: signin refuses anything that is not Active, but nothing rechecked it afterwards, so marking someone Blocked did not end the session they already had, which is exactly when you would be doing it. Now the account must exist and be Active on every request. Client: nothing reacted to a 401 at all. onError fed the bug-report form and stopped there, so the window kept rendering off cached React Query data. A 401 now clears every storage key createClient reads and returns to the sign-in screen. /auth/ is exempt because a wrong password is also a 401 and reloading the form would look like a crash. window.officerBearerToken was declared non-optional, which made "there is no token" unspeakable. It has always been one of five sources, any of which may be absent. Co-Authored-By: Claude Opus 5 --- src/servers/_middlewares/user-middleware.ts | 28 +++++++++++++------ src/workspaces/hooks/src/useClient.ts | 30 +++++++++++++++++++++ src/workspaces/types/globals.d.ts | 6 ++++- 3 files changed, 55 insertions(+), 9 deletions(-) diff --git a/src/servers/_middlewares/user-middleware.ts b/src/servers/_middlewares/user-middleware.ts index 7107d523..287c7208 100644 --- a/src/servers/_middlewares/user-middleware.ts +++ b/src/servers/_middlewares/user-middleware.ts @@ -49,15 +49,27 @@ export const userMiddleware: MiddlewareHandler = async function (ctx, next) { if (await isTokenBlacklisted(user.jti)) throw errors.UNAUTHORIZED(); } - // Check if token was issued before password change - if (user.iat && user.id) { + // The account still has to exist, and still has to be allowed in. + // + // This lookup used to happen only for the password-change comparison below, and its result was read + // as `dbUser?.passwordChangedAt` — so a DELETED account fell straight through the optional chain and + // kept working on a token that is still cryptographically valid, for up to the full 30 days. Observed + // 2026-08-11: an account deleted from the dashboard survived a page refresh in another window. + // + // `status` is the same shape of hole. signin.ts refuses anything that is not 'Active', but nothing + // re-checked it afterwards, so marking someone Blocked or Banned did not end the session they already + // had — which is precisely when you would be doing it. + // + // Re-read per request rather than trusted as a claim, for the reason the role is not a claim either: + // a revocation has to take effect on the next request, not at next sign-in. + if (user.id) { const dbUser = await getUserById(user.id); - if (dbUser?.passwordChangedAt) { - // iat is in seconds, passwordChangedAt is a Date - const tokenIssuedAt = user.iat * 1000; - if (tokenIssuedAt < dbUser.passwordChangedAt.getTime()) { - throw errors.UNAUTHORIZED(); - } + if (!dbUser) throw errors.UNAUTHORIZED(); + if (dbUser.status !== 'Active') throw errors.UNAUTHORIZED(); + + // Token issued before the password changed → refuse. iat is seconds, passwordChangedAt is a Date. + if (user.iat && dbUser.passwordChangedAt && user.iat * 1000 < dbUser.passwordChangedAt.getTime()) { + throw errors.UNAUTHORIZED(); } } } diff --git a/src/workspaces/hooks/src/useClient.ts b/src/workspaces/hooks/src/useClient.ts index 2792cb7f..630840fc 100644 --- a/src/workspaces/hooks/src/useClient.ts +++ b/src/workspaces/hooks/src/useClient.ts @@ -165,6 +165,35 @@ export class ApiError extends Error { } } +/** + * A 401 on an authenticated call means the token is no longer good — deleted account, blocked account, + * changed password, explicit signout elsewhere. Clear it and get back to the sign-in screen. + * + * Until this existed, nothing in the client reacted to a 401 at all: `onError` fed the bug-report form and + * that was the end of it. So an account deleted from the dashboard kept its window rendering happily off + * cached React Query data, and a refresh looked like a live session. Observed 2026-08-11. + * + * `/auth/…` is exempt, and that exemption is load-bearing rather than tidy: a wrong password IS a 401, and + * reloading the sign-in page in response would wipe the form and look like a crash. + * + * `location.replace` rather than a router navigation: this module has no router, and after revocation the + * cleanest state is a fresh document with no stale query cache in it. + */ +const handleRevokedSession = (url: string) => { + if (url.includes('/auth/')) return; + + theToken = null; + // Every key `createClient` reads, or the next load signs straight back in with the dead token. + for (const key of ['PERTENTO_EDITOR_AUTH_TOKEN', 'BEARER_TOKEN']) { + localStorage.removeItem(key); + sessionStorage.removeItem(key); + } + window.officerBearerToken = undefined; + + // Guard against a reload loop if the sign-in screen itself ever 401s on a non-/auth call. + if (!window.location.pathname.startsWith('/signin')) window.location.replace('/'); +}; + const validateResponse = async (res: Response) => { if (res.status >= 400) { const { onError } = useClient.config; @@ -172,6 +201,7 @@ const validateResponse = async (res: Response) => { if (onError) { onError({ status: res.status, message }); } + if (res.status === 401) handleRevokedSession(res.url); throw new ApiError(res.status, message); } }; diff --git a/src/workspaces/types/globals.d.ts b/src/workspaces/types/globals.d.ts index cc958c32..4027fe40 100644 --- a/src/workspaces/types/globals.d.ts +++ b/src/workspaces/types/globals.d.ts @@ -16,7 +16,11 @@ declare global { }; interface Window { - officerBearerToken: string; + /** + * Optional, and honestly so: `createClient` reads this first but falls through to four other sources, + * and `handleRevokedSession` clears it. Declaring it non-optional made "there is no token" unspeakable. + */ + officerBearerToken?: string; __officerRuntimeBridge?: boolean; } }