fix(spa): suppress 401 error toasts during SSO stale-session reload#21
Closed
awais786 wants to merge 18 commits into
Closed
fix(spa): suppress 401 error toasts during SSO stale-session reload#21awais786 wants to merge 18 commits into
awais786 wants to merge 18 commits into
Conversation
* feat(sso): add proxy auth middleware for mPass ForwardAuth Koa middleware reads X-Auth-Request-Email header injected by Traefik ForwardAuth (oauth2-proxy), finds or creates user, and sets JWT accessToken cookie. Gated on PROXY_AUTH_ENABLED env var. * refactor(sso): remove unused PROXY_AUTH_ENABLED gate The fork image is purpose-built for mPass SSO via Traefik ForwardAuth — there is no deployment where the middleware should be disabled at runtime. Outline's other auth plugins (OIDC, Google, Slack) are gated on credentials being set, not on a separate on/off flag, so this inconsistent kill switch was dead weight. Drop the env var read and the short-circuit branch. The middleware always runs when the fork image is in use. * ci: gate CI + CodeQL on foss-main instead of main Upstream Outline triggers the lint/types/test/test-server matrix and CodeQL analysis on push/PR to main. The Pressingly fork doesn't carry main, so fork PRs ran nothing. Retarget both workflows to foss-main so fork PRs exercise the full server + app test shards, lint, types, and the weekly CodeQL scan. * fix(proxyAuth): pass LogCategory label to Logger.info calls Logger.info signature is (label: LogCategory, message, extra?). Three call sites passed message as the first arg, tripping TS2345 in yarn tsc. Prepend 'authentication' on all three so the label slot is populated correctly and extras land in their own field. No runtime behaviour change — the fix only corrects the structured log shape and lets the build compile. * added forwardauth middleware * refactor: migrate ForwardAuth to AUTH_TYPE=SSO env var with SMB_NAME support Replaces the boolean FORWARD_AUTH_ENABLED flag with AUTH_TYPE="SSO" for better extensibility, and adds SMB_NAME to construct email addresses for users whose identity provider only returns a username (no domain). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: issue JWT cookie on first ForwardAuth request to auth.info Ensures cookie-dependent services (WebSocket, collaboration) can authenticate when the session was established via ForwardAuth headers. Also extracts FORWARDAUTH_SERVICE constant to remove stringly-typed "forwardauth" literals across middleware and routes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(forwardauth): merge sso-auth improvements into integrated auth pipeline Take the best parts of the sso-auth `proxyAuth` middleware and apply them to our integrated `authentication.ts` approach. Key changes: - Fix credential priority in `parseAuthentication`: the SSO header check now runs last — after Authorization header, body, query, and cookie. Previously it ran first, so even after a JWT cookie was issued every subsequent API request still hit the DB via the header path. Now, once the cookie exists the fast JWT path resumes and the DB lookup is skipped entirely. - Move JWT cookie issuance into `auth()` middleware: previously the cookie was only set inside the `auth.info` route handler. Any authenticated endpoint reached before `auth.info` (or a WebSocket upgrade) would have no cookie. Moving issuance into `auth()` means the cookie is issued on the very first successfully authenticated request, regardless of which endpoint it is. - Add `lastSignedIn` cookie alongside `accessToken`: the frontend reads this cookie to display the "last signed in with…" indicator. We were silently missing it. - Add `FORWARDAUTH_SERVICE` to `NON_SSO_SERVICES` in `auth.info`: ForwardAuth users have no `UserAuthentication` record so scheduling `ValidateSSOAccessTask` for them is incorrect. The proxy handles session re-validation. - Fix stale test env var: `env.FORWARD_AUTH_ENABLED` was removed when we migrated to `AUTH_TYPE=SSO`; tests still referenced the old name. Updated to `env.AUTH_TYPE = "SSO"` and added `cookies` mock to ForwardAuth test contexts since `auth()` now calls `ctx.cookies.set()` in that path. - Add `SMB_NAME=test` to `.env.test` for test coverage of non-email username handling. What we deliberately kept from our branch (not adopted from sso-auth): - `teamCreator` command for team provisioning (vs raw `Team.create`) - Role assignment: first user → Admin, subsequent → `team.defaultUserRole` - `SMB_NAME` fallback for non-email username claims - `AUTH_TYPE=SSO` env gate (sso-auth has no gate — always active) - Integrated auth pipeline (vs separate Koa middleware that only works for browser flows where HTML is loaded before any API call) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(forwardauth): remove proxyAuth middleware, superseded by auth pipeline `proxyAuth.ts` and its mount in `web.ts` are replaced by the ForwardAuth handling integrated into `authentication.ts`. Keeping both would cause double user-provisioning on every first request and leave the Koa middleware as dead code. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(forwardauth): lowercase email in provision assertion Middleware lowercases the email claim before storing, so the DB lookup and state.auth.user.email assertion must match on the lowercased value rather than the mixed-case input. --------- Co-authored-by: Azan Ali <azan.ali@arbisoft.com>
…T_URI (#4) AuthStore.logout() was setting logoutRedirectUri from env.OIDC_LOGOUT_URI, which pointed at a Cognito hosted /logout endpoint. In this deployment the app client has no hosted /logout, so the Cognito session always survives logout and the env-wired redirect was dead weight when unset and pointed at an unregistered URL when set. Simplified to derive the portal host from the current URL: - Rewrite "foss-<app>.<domain>" -> "foss.<domain>" and assign that as logoutRedirectUri whenever the user initiates a logout. - The portal host is outside ForwardAuth, so the user lands on the landing page instead of being silently re-authed into the dashboard. Re-auth still happens the next time the user clicks into a gated app, which is the expected behavior while Cognito hosted /logout is absent. Logout.tsx no longer branches on env.OIDC_LOGOUT_URI; AuthStore always sets logoutRedirectUri and the unauthenticated branch in Authenticated.tsx performs the navigation.
fixed bug where invalid cookie would lead to log in page
made askii.ai default email domain
use SMB_NAME instead of foss.
* fix: strip first subdomain for portal redirect on signout Replace hardcoded "moneta." prefix with an empty string so the regex strips just the leading subdomain (e.g. app.moneta.askii.ai → moneta.askii.ai) regardless of the deployment domain. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: use lookahead regex to safely strip leading subdomain Switch to /^[^.]+\.(?=[^.]*\.[^.]*\.)/ so the subdomain is only stripped when at least two dot-separated parts remain, preventing over-stripping on bare domains. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Outline's accessToken cookie was hardcoded to 3 months
(addMonths(new Date(), 3)) at three call sites — outliving the rest of
the foss-server-bundle's 7-day session window. After a stack-wide
session expiry users would stay signed in to Outline alone.
Changes:
- Export JWT_COOKIE_TTL_DAYS = 7 from server/utils/authentication.ts as
a single source of truth (with rationale in a docstring).
- Use addDays(new Date(), JWT_COOKIE_TTL_DAYS) at all three mint sites:
- server/middlewares/authentication.ts (FORWARDAUTH login)
- server/routes/auth/index.ts (/auth/redirect)
- server/utils/authentication.ts (signIn callback)
- Fix a pre-existing upstream issue at /auth/redirect: getJwtToken was
called with no expiresAt arg, producing a JWT the validator at
utils/jwt.ts:47 never rejects (claim missing → check skipped). Now
passes `expires` into both getJwtToken and the cookie set, matching
the pattern the other two paths already use.
- Tests cover both halves:
- middlewares/authentication.test.ts asserts the FORWARDAUTH cookie's
expires is ~now + JWT_COOKIE_TTL_DAYS (±60s).
- routes/auth/index.test.ts asserts /auth/redirect mints a JWT whose
expiresAt claim is set and ~now + JWT_COOKIE_TTL_DAYS (±60s).
In future, lift JWT_COOKIE_TTL_DAYS to an env var if deployment-specific
control is needed.
Outline's accessToken cookie was hardcoded to 3 months
(addMonths(new Date(), 3)) at three call sites — outliving the rest of
the foss-server-bundle's 7-day session window. After a stack-wide
session expiry users would stay signed in to Outline alone.
Changes:
- Export JWT_COOKIE_TTL_DAYS = 7 from server/utils/authentication.ts as
a single source of truth (with rationale in a docstring).
- Use addDays(new Date(), JWT_COOKIE_TTL_DAYS) at all three mint sites:
- server/middlewares/authentication.ts (FORWARDAUTH login)
- server/routes/auth/index.ts (/auth/redirect)
- server/utils/authentication.ts (signIn callback)
- Fix a pre-existing upstream issue at /auth/redirect: getJwtToken was
called with no expiresAt arg, producing a JWT the validator at
utils/jwt.ts:47 never rejects (claim missing → check skipped). Now
passes `expires` into both getJwtToken and the cookie set, matching
the pattern the other two paths already use.
- Tests cover both halves:
- middlewares/authentication.test.ts asserts the FORWARDAUTH cookie's
expires is ~now + JWT_COOKIE_TTL_DAYS (±60s).
- routes/auth/index.test.ts asserts /auth/redirect mints a JWT whose
expiresAt claim is set and ~now + JWT_COOKIE_TTL_DAYS (±60s).
In future, lift JWT_COOKIE_TTL_DAYS to an env var if deployment-specific
control is needed.
The ForwardAuth (AUTH_TYPE=SSO) branch of validateAuthentication looked
users up with User.findOne({ where: { email: { [Op.iLike]: email } } }).
ILIKE treats % and _ in the supplied value as wildcards, so a request
with "x-auth-request-email: %@%.%" produces ILIKE '%@%.%' — which
matches every row. The first matched user (typically the bootstrap
admin) was then issued a 3-month JWT cookie.
Switching to exact match (where: { email }) strips all special meaning
from those characters. Email is already .toLowerCase().trim()'d before
the lookup, matching the existing User.findByEmail pattern; emails are
stored canonically lowercased so case-insensitive matching is not
required.
Not exploitable in production: oauth2-proxy intercepts unauthenticated
requests before they reach Outline and overwrites x-auth-request-*
headers from the verified OIDC identity. Defense-in-depth fix at the
code level in case backend reachability ever changes.
Adds a regression test asserting wildcard input does not impersonate
an existing user.
When the user switches identity at the portal level (logs out, logs in as a different user), Outline's accessToken JWT cookie is stale until the server's mismatch check fires on the next request and clears it via err.headers. The SPA already handled this in SSO mode by navigating to the current URL to trigger a fresh-session flow, but multiple parallel API requests would 401 together (docs, access-tokens, team, etc) and each would throw an AuthorizationError that surfaced to UI as a "failed to get …" toast before the page finished navigating. Visible symptom: brief flash of "failed to get docs or access tokens" on identity switch, gone after the implicit reload. Fix: add an `isReauthenticating` flag on ApiClient. First 401 in SSO mode triggers the reload AND sets the flag; subsequent in-flight 401s see the flag and stall on a never-resolving promise instead of throwing. The page is navigating away, so blocking is correct — there's nothing useful for those callers to do. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When the user switches identity at the portal level (Logout-all → log in as a different user), Outline's `accessToken` JWT cookie is stale until the server's mismatch check (`server/middlewares/authentication.ts:416-424`) fires on the next request and clears it via `err.headers`. The server-side flush is correct, but the SPA shows a flash of "failed to get docs / access tokens" before the implicit reload completes.
Repro
Root cause
The SPA's `ApiClient.ts:181-202` handles 401 in SSO mode by:
```ts
window.location.replace(window.location.href);
throw new AuthorizationError();
```
The first 401 triggers the reload. But the page doesn't navigate instantly — and on the first post-switch render multiple API requests are in flight (docs/, access-tokens/, team/, etc). Each one 401s, each calls `window.location.replace`, and each throws `AuthorizationError`. Those throws propagate to UI components that catch them and show "failed to get …" toasts before the navigation completes.
Fix
Add an `isReauthenticating` flag on `ApiClient`. First 401 in SSO mode triggers the reload AND sets the flag; subsequent in-flight 401s stall on a never-resolving promise instead of throwing.
```ts
if (!this.isReauthenticating) {
this.isReauthenticating = true;
window.location.replace(window.location.href);
}
return new Promise(() => {
// Intentionally never resolves — the page is navigating away.
});
```
The page IS navigating away, so blocking the caller forever is correct — there's nothing useful for the SPA to do with the result of a request that's about to be discarded. Subsequent throws would only generate noise.
Risk
Tiny. The change is scoped to the SSO + 401 branch of `ApiClient.fetch`:
A never-resolving promise from `fetch` is the same shape callers see during a real network stall. Any caller that doesn't gracefully handle indefinite pending promises was already broken under normal network conditions.
Companion to
#19 (server-side stale-session flush). That PR makes the backend correctly clear the cookie on identity mismatch; this PR makes the user-visible reload smooth instead of flashing an error.
Test plan
🤖 Generated with Claude Code