fix(security): require bearer auth on GET /api/v1/reauth (#264)

[alexbj75]

Closes #264.

Problem

GET /api/v1/reauth was registered in src/api_re_auth.ts with a schema only — no preHandler, no onRequest, no auth of any kind — and src/api.ts registered the plugin without an auth hook. Any unauthenticated caller could therefore mint a valid OSC service access token for the instance (the global @fastify/rate-limit throttles, but does not authenticate).

Change

1. Auth on the route — mirroring the existing house pattern.
requireReAuth is a direct mirror of requireWhipAuth in src/api_whip.ts (lines 58–80): Authorization: Bearer … header, constant-time comparison via crypto.timingSafeEqual with an explicit length check, 401 with WWW-Authenticate: Bearer realm="reauth", charset="UTF-8", and auth disabled when no key is configured. No new dependency (timingSafeEqual is in Node core).

2. Configuration — same route as whipAuthKey.
New ApiReAuthOptions { reAuthKey?: string }, folded into ApiOptions and passed at registration, exactly as whipAuthKey is. src/server.ts feeds it from process.env.REAUTH_AUTH_KEY ?? process.env.WHIP_AUTH_KEY.

The WHIP_AUTH_KEY fallback is deliberate: WHIP_AUTH_KEY is currently the only bearer key this backend checks, and intercom-frontend already sends Authorization: Bearer ${VITE_BACKEND_API_KEY} on every call including reauth (src/api/api.ts:302–311). Existing deployments that set WHIP_AUTH_KEY therefore get the endpoint protected with no config change and no client change; REAUTH_AUTH_KEY allows a separate key. Deployments with neither key set are unchanged (endpoint stays open) — documented in readme.md.

Point 2 — token in the response body

Removed. The handler now sends { success: true }; the token continues to be delivered as the eyevinn-intercom-manager.sat httpOnly cookie. ReAuthResponse in src/models.ts updated accordingly.

Evidence this breaks no consumer:

• In this repo: nothing reads the field. grep -ri reauth src/ returns only the route itself, its registration in api.ts:157, and the model.
• Known external consumer: Eyevinn/intercom-frontend — API.reauth() is typed Promise<void> (src/api/api.ts:302) and use-reauth.tsx discards the resolved value entirely; it relies on the cookie. The cookie is httpOnly, so browser JS could never have read it from there anyway.

This is nonetheless a breaking response-schema change for any out-of-tree client that reads token from the body — call it out in release notes. Auth (point 1) is the actual fix; this is defense in depth (token out of logs, proxies, browser history).

Not silent when disabled (QA review). src/server.ts logs a warning at startup when OSC_ACCESS_TOKEN is set and the effective key is missing or whitespace-only:

SECURITY: GET /api/v1/reauth is UNAUTHENTICATED - anyone who can reach this server can obtain a valid OSC service access token. Reason: {no REAUTH_AUTH_KEY or WHIP_AUTH_KEY is set | REAUTH_AUTH_KEY/WHIP_AUTH_KEY is set but empty or whitespace only, which disables auth - this is most likely a configuration error}. Set REAUTH_AUTH_KEY to a non-empty secret to require a Bearer token.

The two reasons are separated on purpose: reAuthKey?.trim() makes REAUTH_AUTH_KEY=" " falsy, so a key that looks configured in an env file silently disables auth. That is a configuration error, not a choice.

Deployment note

intercom-frontend sends Authorization: Bearer ${VITE_BACKEND_API_KEY}. If a key is configured on the backend, the frontend must be built with a matching VITE_BACKEND_API_KEY — otherwise reauth starts returning 401 for a frontend built without one.

Tests

src/api_re_auth.test.ts — the token service is now mocked, so the suite no longer makes real network calls to token.svc.*.osaas.io:

• unauthenticated request with a key configured → 401 + WWW-Authenticate, no token minted, no cookie set
• wrong bearer token → 401, no token minted
• empty bearer (Authorization: Bearer) → 401
• malformed header without the Bearer prefix → 401
• token that is a proper prefix of the key → 401 (length check short-circuits timingSafeEqual)
• correct bearer token → 200, cookie set, body is { success: true } with no token
• no key configured → 200 (existing installations are not broken)
• token service unavailable → 500 (regression cover for the retry path)

Verification

• npm test — 14 suites, 250 tests, all pass
• npm run typecheck — clean
• npm run lint — 0 errors (280 pre-existing warnings, unchanged)
• prettier applied

Not touched: infra/terraform/aws/, dependencies, any other route.

[alexbj75]

QA-granskning — PR #283 (säkerhet: auth på GET /api/v1/reauth)

Verktygsdeklaration (§4c, effort-budgets): gh var körbart (auth OK). Granskningen är statisk kodläsning + korsverifiering mot Eyevinn/intercom-frontend via GitHub API. Kod kördes inte lokalt (ingen klon/npm test i denna session), och /reauth anropades aldrig mot någon driftsatt miljö. CI-status togs som given från CEO (lint/pretty/ts/unittests = pass). Fynd nedan är resonemang över kod jag faktiskt läst, ej över en körning.

Verdict: REQUEST CHANGES — en (1) blockerande punkt, resten är nits

Auth-logiken i sig är korrekt. Fixen stänger hålet för alla installationer som har en nyckel. Det som blockerar är att installationer utan nyckel fortfarande läcker en giltig SAT — tyst.

---

1. Är auth-kontrollen korrekt? — JA

requireReAuth är en teckenidentisk spegling av requireWhipAuth (src/api_whip.ts:58–80), bara realm-strängen skiljer ("reauth" vs "whip"). Inget tappat.

Genomgång av bypass-vägar (alla läser till 401):

• timingSafeEqual kastar på olika längd — skyddat av kortslutningen tokenBuf.length === keyBuf.length && timingSafeEqual(...). && utvärderar vänsterledet först, så anropet nås aldrig med olika längder. Korrekt.
• Saknat huvud → authHeader === undefined → ?.startsWith?. → undefined → token = '' → längd 0 ≠ nyckellängd → isValid=false → 401.
• Tom Bearer (Authorization: Bearer ) → token = ''.trim() = '' → samma väg → 401. Ingen bypass.
• Felformat huvud (Authorization: secret-123, Basic …) → ingen Bearer -prefix → token='' → 401.
• Token som är prefix av nyckeln (Bearer secret-12) → längdkontrollen fäller den före jämförelsen → 401.
• Icke-sträng-huvud → typeof authHeader !== 'string' fångas explicit.

Konstant tid: jämförelsen är konstant-tid för lika längd; längden på nyckeln läcker via timing/kortslutningen. Det är standard och accepterat för bearer-nycklar (samma som WHIP) — inte ett fynd.

Dubbelsändning av svar: requireReAuth skickar 401 och handlern gör return direkt på false. Inget dubbel-send.

MINOR (ej blockerande): 'Bearer ' jämförs skiftlägeskänsligt. RFC 7235 säger att auth-schemat är skiftlägesokänsligt, så bearer <token> avvisas felaktigt. Ärvt från requireWhipAuth och intercom-frontend skickar Bearer, så ingen praktisk regression — men det är ett interop-nit som borde fixas på båda ställena samtidigt, inte här.

MINOR: request: any, reply: any — otypat, ärvt från WHIP-mönstret. Speglingstrogenhet väger tyngre än typning här; ta det i en separat städ-PR om alls.

2. Default-beteendet — MAJOR, blockerande

Auth är av när ingen nyckel är satt, och REAUTH_AUTH_KEY ?? WHIP_AUTH_KEY (src/server.ts:52). Avvägningen är i sig rimlig: den bevarar befintliga installationer och skyddar automatiskt alla som redan satt WHIP_AUTH_KEY, utan klientändring. Den är dessutom dokumenterad i readme.md.

Men: efter att den här "säkerhetsfixen" gått i drift kan en installation med OSC_ACCESS_TOKEN satt och ingen nyckel fortfarande dela ut en giltig OSC service access token till vem som helst — och ingenstans i systemet finns en signal om det. Ingen logg, ingen startvarning, inget hälsofält. Den som ser att #264 är stängd kommer rimligen tro att instansen är skyddad. Det är precis den tysta exponeringen som gör konfigurationsberoende säkerhet farlig.

Extra footgun i samma kod: opts.reAuthKey?.trim() gör att REAUTH_AUTH_KEY=" " (blanksteg, t.ex. från en illa klistrad secret) blir tomt → falsy → auth tyst avstängd. Ett stavfel i en secret degraderar alltså till öppen endpoint utan ett ljud.

Krav före merge: en startvarning i src/server.ts (eller vid pluginregistrering) när OSC_ACCESS_TOKEN är satt och den effektiva reAuthKey är tom — inklusive fallet whitespace-only. Ungefär:

if (process.env.OSC_ACCESS_TOKEN && !reAuthKey?.trim()) {
  logger.warn(
    'SECURITY: OSC_ACCESS_TOKEN is set but neither REAUTH_AUTH_KEY nor WHIP_AUTH_KEY is configured — GET /api/v1/reauth is UNAUTHENTICATED and will mint service access tokens for any caller.'
  );
}

Tre rader, ingen brytande ändring, och den täcker båda hålen. Jag talar inte bort det här: det är auth på en credential-endpoint, och en tyst öppen dörr är inte "dokumenterad i readme"-nivå.

3. Testernas kvalitet — bra täckning, tre hål (MINOR)

Täcker rätt saker: 401 oautentiserad (+ WWW-Authenticate, ingen cookie, fetchMock ej anropad), 401 fel token, 200 rätt token (kropp {success:true}, token undefined, cookie satt), 200 utan nyckel, 500 vid nere token-tjänst.

Mockningen är verifierad: jest.setup.js sätter process.env.OSC_ACCESS_TOKEN='foo' globalt, och mockTokenService() ersätter global.fetch med afterEach-återställning till originalFetch. Inga live-anrop mot token.svc.*.osaas.io. Det är en reell förbättring — det tidigare testet lät fetch gå ut på riktigt.

Hål (lägg gärna till i samma PR, de är trerads-tester):

• Tom Bearer (authorization: 'Bearer ') — den klassiska bypassen. Logiken är bevisligen säker, men den är säker via en subtil trim()+längdkontroll som en framtida refaktor kan råka bryta. Regressionsskydd saknas.
• Felformat huvud (authorization: 'secret-123' utan schema, samt Basic …).
• Prefix-token (authorization: 'Bearer secret-12') — låser fast att längdkontrollen finns kvar.
• Timing testas inte — korrekt beslut, timingtester är flakiga i CI. Inget krav.
• server.ts-kopplingen (REAUTH_AUTH_KEY ?? WHIP_AUTH_KEY) är otestad. Acceptabelt — den raden är trivial och server.ts saknar testinfrastruktur.

4. Den brytande ändringen — påståendet är VERIFIERAT

Jag läste Eyevinn/intercom-frontend direkt (GitHub API, branch main):

• src/api/api.ts:302 — reauth: async (): Promise<void> => { return handleFetchRequest<void>(fetch(\${API_URL}reauth`, { method: "GET", headers: { ...(API_KEY ? { Authorization: `Bearer ${API_KEY}` } : {}) } })); }. Typad Promise, **och den skickar redan Authorization: Bearer`** — så auth-fixen kräver ingen klientändring.
• src/hooks/use-reauth.tsx — await API.reauth() utan att binda returvärdet; hooken bryr sig bara om kastat fel. Läser aldrig token.

Att ta bort token ur kroppen bryter alltså inget hos den kända konsumenten. FD:s motivering håller. Cookien är dessutom httpOnly, så webbläsar-JS kunde ändå aldrig läsa den därifrån. Släpp det som en brytande schemaändring i release notes — den formuleringen finns redan i PR-beskrivningen.

MINOR, deploy-koppling värd en rad i release notes: en installation som har WHIP_AUTH_KEY satt på backend men vars frontend byggts utan VITE_BACKEND_API_KEY kommer nu få 401 på reauth (tidigare 200). Kopplingen är förvisso redan sann för WHIP/WHEP, men reauth träffar alla vanliga användare, inte bara ingest. Nämn det.

5. Läckage i övrigt — INGET FUNNET

• 401-kroppen är { error: 'Unauthorized' } — säger inget om nyckeln, dess längd eller om en nyckel alls är konfigurerad. (Notera: att auth är avstängd kan förstås fortfarande härledas av att ett nyckellöst anrop ger 200 — det är punkt 2, inte ett läckage här.)
• WWW-Authenticate: Bearer realm="reauth", charset="UTF-8" — bara realm, ingen nyckel.
• 500-meddelandet innehåller endast uppströms status/statusText och antal försök. Ingen token, ingen PAT.
• Ingen console.log/logger rör json.token; token skrivs bara till cookien (httpOnly, secure, sameSite:'strict', 2h). Korrekt.
• PAT:en (OSC_ACCESS_TOKEN) skickas bara som x-pat-jwt uppströms, aldrig i något svar.

---

Måste ändras före merge

1. [MAJOR] Startvarning när OSC_ACCESS_TOKEN är satt men effektiv reauth-nyckel saknas eller är whitespace-only — se kodförslaget under punkt 2.

Bör ändras (rekommenderat, blockerar inte)

2. [MINOR] Tre negativa tester: tom Bearer, felformat huvud, prefix-token.
3. [MINOR] Release notes: nämn frontend/backend-nyckelkopplingen (VITE_BACKEND_API_KEY vs WHIP_AUTH_KEY) utöver den brytande schemaändringen.

Punkt 1 och 4 är rena godkännanden: auth-kontrollen är korrekt och en trogen spegling, och den brytande ändringen är verifierad mot rätt repo — inte accepterad på ord.

[birme]

All APIs when running in an OSC context is behind the OSC managed auth wall so we should not add any bearer auth on the app level that will conflict with that. The reauth endpoint is only relevant when running in an OSC context and where this is needed