diff --git a/src/frontend/servers/y-provider/__tests__/convert.test.ts b/src/frontend/servers/y-provider/__tests__/convert.test.ts index 79dd8ffb2..f6accb5ea 100644 --- a/src/frontend/servers/y-provider/__tests__/convert.test.ts +++ b/src/frontend/servers/y-provider/__tests__/convert.test.ts @@ -6,7 +6,7 @@ import { YjsThreadStore } from '@blocknote/core/yjs'; import { ServerBlockNoteEditor } from '@blocknote/server-util'; import { Fragment, Node as PMNode } from 'prosemirror-model'; import request from 'supertest'; -import { afterEach, describe, expect, test, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; import { prosemirrorToYXmlFragment } from 'y-prosemirror'; import * as Y from 'yjs'; @@ -14,17 +14,17 @@ vi.mock('../src/env', async (importOriginal) => { return { ...(await importOriginal()), COLLABORATION_SERVER_ORIGIN: 'http://localhost:3000', - Y_PROVIDER_API_KEY: 'yprovider-api-key', }; }); import { docsBlockNoteSchema } from '@/blockSpecs'; import { initApp } from '@/servers'; -import { - Y_PROVIDER_API_KEY as apiKey, - COLLABORATION_SERVER_ORIGIN as origin, -} from '../src/env'; +import { JWKS_URL, COLLABORATION_SERVER_ORIGIN as origin } from '../src/env'; + +import { mockJwksEndpoint, signAdminToken } from './testUtils/adminJwt'; + +const apiKey = await signAdminToken(); const expectedMarkdown = '# Example document\n\nLorem ipsum dolor sit amet.'; const expectedHTML = @@ -147,8 +147,13 @@ const buildYjsUpdateWithComment = (): Buffer => { console.error = vi.fn(); describe('Conversion Testing', () => { + beforeEach(() => { + mockJwksEndpoint(JWKS_URL); + }); + afterEach(() => { vi.clearAllMocks(); + vi.unstubAllGlobals(); }); test('POST /api/convert with incorrect API key responds with 401', async () => { diff --git a/src/frontend/servers/y-provider/__tests__/middlewares.test.ts b/src/frontend/servers/y-provider/__tests__/middlewares.test.ts new file mode 100644 index 000000000..cc782f9fa --- /dev/null +++ b/src/frontend/servers/y-provider/__tests__/middlewares.test.ts @@ -0,0 +1,114 @@ +import express from 'express'; +import request from 'supertest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const { JWKS_URL } = vi.hoisted(() => ({ + JWKS_URL: 'http://app-dev:8000/api/v1.0/jwks', +})); + +vi.mock('../src/env', async (importOriginal) => { + return { + ...(await importOriginal()), + JWKS_URL, + }; +}); + +import { httpSecurity } from '@/middlewares'; + +import { + mockJwksEndpoint, + signAdminToken, + signAdminTokenWithWrongKey, + signExpiredAdminToken, + signToken, +} from './testUtils/adminJwt'; + +const buildApp = () => { + const app = express(); + app.get('/protected', httpSecurity, (req, res) => { + res.status(200).json({ ok: true }); + }); + return app; +}; + +describe('httpSecurity', () => { + beforeEach(() => { + mockJwksEndpoint(JWKS_URL); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('rejects requests without an authorization header', async () => { + const response = await request(buildApp()).get('/protected'); + + expect(response.status).toBe(401); + expect(response.body).toStrictEqual({ + error: 'Unauthorized: No credentials given', + }); + expect(fetch).not.toHaveBeenCalled(); + }); + + it('accepts a valid admin JWT signed by the Django backend', async () => { + const token = await signAdminToken(); + + const response = await request(buildApp()) + .get('/protected') + .set('authorization', `Bearer ${token}`); + + expect(response.status).toBe(200); + // Verified against the real JWKS document served over the (mocked) network. + expect(fetch).toHaveBeenCalledWith(JWKS_URL, expect.anything()); + }); + + it('rejects a token signed with a key that is not in the JWKS', async () => { + const token = await signAdminTokenWithWrongKey(); + + const response = await request(buildApp()) + .get('/protected') + .set('authorization', `Bearer ${token}`); + + expect(response.status).toBe(401); + expect(response.body).toStrictEqual({ + error: 'Unauthorized: Invalid API Key', + }); + }); + + it('rejects an expired admin JWT', async () => { + const token = await signExpiredAdminToken(); + + const response = await request(buildApp()) + .get('/protected') + .set('authorization', `Bearer ${token}`); + + expect(response.status).toBe(401); + expect(response.body).toStrictEqual({ + error: 'Unauthorized: Invalid API Key', + }); + }); + + it('rejects a validly signed JWT missing the admin claim', async () => { + const token = await signToken({ sub: 'someone' }); + + const response = await request(buildApp()) + .get('/protected') + .set('authorization', `Bearer ${token}`); + + expect(response.status).toBe(401); + expect(response.body).toStrictEqual({ + error: 'Unauthorized: Invalid API Key', + }); + }); + + it('rejects a bearer token that is not a valid JWT', async () => { + const response = await request(buildApp()) + .get('/protected') + .set('authorization', 'Bearer wrong-token'); + + expect(response.status).toBe(401); + expect(response.body).toStrictEqual({ + error: 'Unauthorized: Invalid API Key', + }); + }); +}); diff --git a/src/frontend/servers/y-provider/__tests__/server.test.ts b/src/frontend/servers/y-provider/__tests__/server.test.ts index 5454f8019..6d76bda62 100644 --- a/src/frontend/servers/y-provider/__tests__/server.test.ts +++ b/src/frontend/servers/y-provider/__tests__/server.test.ts @@ -1,5 +1,5 @@ import request from 'supertest'; -import { describe, expect, it, test, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, test, vi } from 'vitest'; import { routes } from '@/routes'; import { initApp } from '@/servers'; @@ -8,19 +8,25 @@ vi.mock('../src/env', async (importOriginal) => { return { ...(await importOriginal()), COLLABORATION_SERVER_ORIGIN: 'http://localhost:3000', - Y_PROVIDER_API_KEY: 'yprovider-api-key', CONVERSION_FILE_MAX_SIZE: 500 * 1024, // 500kb }; }); -import { - Y_PROVIDER_API_KEY as apiKey, - COLLABORATION_SERVER_ORIGIN as origin, -} from '../src/env'; +import { JWKS_URL, COLLABORATION_SERVER_ORIGIN as origin } from '../src/env'; + +import { mockJwksEndpoint, signAdminToken } from './testUtils/adminJwt'; console.error = vi.fn(); describe('Server Tests', () => { + beforeEach(() => { + mockJwksEndpoint(JWKS_URL); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + test('Ping Pong', async () => { const app = initApp(); @@ -43,12 +49,13 @@ describe('Server Tests', () => { it('allows payloads up to 500kb for the CONVERT route', async () => { const app = initApp(); + const apiKey = await signAdminToken(); const largePayload = 'a'.repeat(400 * 1024); // 400kb payload const response = await request(app) .post(routes.CONVERT) .set('origin', origin) - .set('authorization', apiKey) + .set('authorization', `Bearer ${apiKey}`) .set('content-type', 'text/markdown') .send(largePayload); @@ -57,12 +64,13 @@ describe('Server Tests', () => { it('rejects payloads larger than CONVERSION_FILE_MAX_SIZE for the CONVERT route', async () => { const app = initApp(); + const apiKey = await signAdminToken(); const oversizedPayload = 'a'.repeat(501 * 1024); // 501kb payload const response = await request(app) .post(routes.CONVERT) .set('origin', origin) - .set('authorization', apiKey) + .set('authorization', `Bearer ${apiKey}`) .set('content-type', 'text/markdown') .send(oversizedPayload); diff --git a/src/frontend/servers/y-provider/__tests__/testUtils/adminJwt.ts b/src/frontend/servers/y-provider/__tests__/testUtils/adminJwt.ts new file mode 100644 index 000000000..66d522aa6 --- /dev/null +++ b/src/frontend/servers/y-provider/__tests__/testUtils/adminJwt.ts @@ -0,0 +1,75 @@ +import { + SignJWT, + calculateJwkThumbprint, + exportJWK, + generateKeyPair, +} from 'jose'; +import { vi } from 'vitest'; + +import { JWT_ALGORITHM } from '@/middlewares'; + +const { privateKey, publicKey } = await generateKeyPair(JWT_ALGORITHM, { + extractable: true, +}); +const publicJwk = await exportJWK(publicKey); +const kid = await calculateJwkThumbprint(publicJwk); + +/** JWKS document shaped like the one Django's JWKSView publishes. */ +export const JWKS = { + keys: [{ ...publicJwk, kid, alg: JWT_ALGORITHM, use: 'sig' }], +}; + +/** Sign a token the way Django's JWTService would, for tests only. */ +export const signToken = (claims: Record) => + new SignJWT(claims) + .setProtectedHeader({ alg: JWT_ALGORITHM, kid }) + .setIssuedAt() + .setExpirationTime('1h') + .sign(privateKey); + +export const signAdminToken = () => signToken({ admin: true }); + +/** An admin token signed correctly but already past its expiry. */ +export const signExpiredAdminToken = () => + new SignJWT({ admin: true }) + .setProtectedHeader({ alg: JWT_ALGORITHM, kid }) + .setIssuedAt(Math.floor(Date.now() / 1000) - 3600) + .setExpirationTime(Math.floor(Date.now() / 1000) - 60) + .sign(privateKey); + +// A second, unrelated key pair: never published in the test JWKS, so a token +// signed with it must fail signature verification. +const { privateKey: roguePrivateKey } = await generateKeyPair(JWT_ALGORITHM, { + extractable: true, +}); + +/** + * An admin token carrying the real "kid" (so key lookup succeeds) but signed + * with a key that isn't the one published in the JWKS. + */ +export const signAdminTokenWithWrongKey = () => + new SignJWT({ admin: true }) + .setProtectedHeader({ alg: JWT_ALGORITHM, kid }) + .setIssuedAt() + .setExpirationTime('1h') + .sign(roguePrivateKey); + +/** + * Stub global fetch so jose's createRemoteJWKSet resolves our test JWKS + * instead of making a real network call to the Django backend. The real + * "jose" verification code still runs against a real signed token. + */ +export const mockJwksEndpoint = (jwksUrl: string) => { + vi.stubGlobal( + 'fetch', + vi.fn(async (input: string | URL) => { + if (input.toString() !== jwksUrl) { + throw new Error(`Unexpected fetch to ${input.toString()}`); + } + return new Response(JSON.stringify(JWKS), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + }), + ); +}; diff --git a/src/frontend/servers/y-provider/package.json b/src/frontend/servers/y-provider/package.json index a8ae2af9f..7b60741b7 100644 --- a/src/frontend/servers/y-provider/package.json +++ b/src/frontend/servers/y-provider/package.json @@ -23,6 +23,7 @@ "@tiptap/extensions": "*", "cors": "2.8.6", "express": "5.2.1", + "jose": "6.2.8", "y-prosemirror": "1.3.7", "yjs": "*" }, diff --git a/src/frontend/servers/y-provider/src/env.ts b/src/frontend/servers/y-provider/src/env.ts index 13dfcb577..564c72927 100644 --- a/src/frontend/servers/y-provider/src/env.ts +++ b/src/frontend/servers/y-provider/src/env.ts @@ -1,9 +1,14 @@ import { readFileSync } from 'fs'; +export const COLLABORATION_BACKEND_BASE_URL = + process.env.COLLABORATION_BACKEND_BASE_URL || 'http://app-dev:8000'; export const COLLABORATION_LOGGING = process.env.COLLABORATION_LOGGING || 'false'; export const COLLABORATION_SERVER_ORIGIN = process.env.COLLABORATION_SERVER_ORIGIN || 'http://localhost:3000'; +// TODO(yhub): unused since the yhub migration, mirrors the Django setting of +// the same name (see impress/settings.py). Kept until CollaborationService is +// reinstated. export const COLLABORATION_SERVER_SECRET = process.env .COLLABORATION_SERVER_SECRET_FILE ? readFileSync(process.env.COLLABORATION_SERVER_SECRET_FILE, 'utf-8') @@ -11,8 +16,8 @@ export const COLLABORATION_SERVER_SECRET = process.env export const CONVERSION_FILE_MAX_SIZE = process.env.CONVERSION_FILE_MAX_SIZE ? Number(process.env.CONVERSION_FILE_MAX_SIZE) : 20971520; // 20 MB default -export const Y_PROVIDER_API_KEY = process.env.Y_PROVIDER_API_KEY_FILE - ? readFileSync(process.env.Y_PROVIDER_API_KEY_FILE, 'utf-8') - : process.env.Y_PROVIDER_API_KEY || 'yprovider-api-key'; +// JWKS of the Django backend, used to verify the JWT it signs when calling us. +export const JWKS_URL = + process.env.JWKS_URL || `${COLLABORATION_BACKEND_BASE_URL}/api/v1.0/jwks`; export const PORT = Number(process.env.PORT || 4444); export const SENTRY_DSN = process.env.SENTRY_DSN || ''; diff --git a/src/frontend/servers/y-provider/src/middlewares.ts b/src/frontend/servers/y-provider/src/middlewares.ts index 11a9e1df5..c7364ddac 100644 --- a/src/frontend/servers/y-provider/src/middlewares.ts +++ b/src/frontend/servers/y-provider/src/middlewares.ts @@ -1,13 +1,9 @@ import cors from 'cors'; import { NextFunction, Request, Response } from 'express'; +import { createRemoteJWKSet, jwtVerify } from 'jose'; -import { - COLLABORATION_SERVER_ORIGIN, - COLLABORATION_SERVER_SECRET, - Y_PROVIDER_API_KEY, -} from '@/env'; +import { COLLABORATION_SERVER_ORIGIN, JWKS_URL } from '@/env'; -const VALID_API_KEYS = [COLLABORATION_SERVER_SECRET, Y_PROVIDER_API_KEY]; const allowedOrigins = COLLABORATION_SERVER_ORIGIN.split(','); export const corsMiddleware = cors({ @@ -16,11 +12,29 @@ export const corsMiddleware = cors({ credentials: true, }); -export const httpSecurity = ( +// Cached across requests: fetches the Django backend's public keys lazily and +// keeps them until their "kid" no longer matches a token, per jose's own policy. +const jwks = createRemoteJWKSet(new URL(JWKS_URL)); + +export const JWT_ALGORITHM = 'RS256'; + +/** + * Verify that the given token is an admin JWT signed by the Django backend. + */ +const isValidAdminToken = async (token: string): Promise => { + try { + const { payload } = await jwtVerify(token, jwks, { algorithms: [JWT_ALGORITHM] }); + return payload.admin === true; + } catch { + return false; + } +}; + +export const httpSecurity = async ( req: Request, res: Response, next: NextFunction, -): void => { +): Promise => { let apiKey = req.headers['authorization']; if (!apiKey) { @@ -32,7 +46,7 @@ export const httpSecurity = ( apiKey = apiKey.slice('Bearer '.length); } - if (!VALID_API_KEYS.includes(apiKey)) { + if (!(await isValidAdminToken(apiKey))) { res.status(401).json({ error: 'Unauthorized: Invalid API Key' }); return; } diff --git a/src/frontend/yarn.lock b/src/frontend/yarn.lock index 932eaba76..f8db34d9c 100644 --- a/src/frontend/yarn.lock +++ b/src/frontend/yarn.lock @@ -11453,6 +11453,11 @@ jest@30.4.2: import-local "^3.2.0" jest-cli "30.4.2" +jose@6.2.8: + version "6.2.8" + resolved "https://registry.yarnpkg.com/jose/-/jose-6.2.8.tgz#39c1459fe5eac84eb39b1623b8077dcf9ca6c506" + integrity sha512-Bsdjwm3Qsd/P0jR+BHDe3LytDfY7WBq2HmCCLIwuVRHMuEC9ae7/R474GIUdF1NgCyZjzVo/A9DOiOBtXq8ZoQ== + "js-tokens@^3.0.0 || ^4.0.0", js-tokens@^4.0.0: version "4.0.0" resolved "https://registry.yarnpkg.com/js-tokens/-/js-tokens-4.0.0.tgz#19203fb59991df98e3a287050d4647cdeaf32499" @@ -13417,7 +13422,7 @@ react-intersection-observer@11.0.0: resolved "https://registry.yarnpkg.com/react-intersection-observer/-/react-intersection-observer-11.0.0.tgz#c388c46dd9c36386bd3a4e4fe1af80baf46c6074" integrity sha512-tF2PjXa//GcUmkCIcZR2qGsj6HwnuunFQdgeJ89BwFE6epB7E9yIzdr018H1khxG7DiMpIDvAne1GKQMFwn+fw== -"react-is-18@npm:react-is@^18.3.1": +"react-is-18@npm:react-is@^18.3.1", react-is@^18.3.1: version "18.3.1" resolved "https://registry.yarnpkg.com/react-is/-/react-is-18.3.1.tgz#e83557dc12eae63a99e003a46388b1dcbb44db7e" integrity sha512-/LLMVyas0ljjAtoYiPqYiL8VWXzUUdThrmU5+n20DZv+a+ClRoevUzw5JxU+Ieh5/c87ytoTBV9G1FiKfNJdmg== @@ -13442,11 +13447,6 @@ react-is@^17.0.1: resolved "https://registry.yarnpkg.com/react-is/-/react-is-17.0.2.tgz#e691d4a8e9c789365655539ab372762b0efb54f0" integrity sha512-w2GsyukL62IJnlaff/nRegPQR94C/XXamvMWmSHRJ4y7Ts/4ocGRmTHvOs8PSE6pB3dWOrD/nueuU5sduBsQ4w== -react-is@^18.3.1: - version "18.3.1" - resolved "https://registry.yarnpkg.com/react-is/-/react-is-18.3.1.tgz#e83557dc12eae63a99e003a46388b1dcbb44db7e" - integrity sha512-/LLMVyas0ljjAtoYiPqYiL8VWXzUUdThrmU5+n20DZv+a+ClRoevUzw5JxU+Ieh5/c87ytoTBV9G1FiKfNJdmg== - react-lifecycles-compat@^3.0.0, react-lifecycles-compat@^3.0.4: version "3.0.4" resolved "https://registry.yarnpkg.com/react-lifecycles-compat/-/react-lifecycles-compat-3.0.4.tgz#4f1a273afdfc8f3488a8c516bfda78f872352362" @@ -14499,16 +14499,7 @@ string-length@^4.0.2: char-regex "^1.0.2" strip-ansi "^6.0.0" -"string-width-cjs@npm:string-width@^4.2.0": - version "4.2.3" - resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" - integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== - dependencies: - emoji-regex "^8.0.0" - is-fullwidth-code-point "^3.0.0" - strip-ansi "^6.0.1" - -string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: +"string-width-cjs@npm:string-width@^4.2.0", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -14642,14 +14633,7 @@ stringify-object@^3.3.0: is-obj "^1.0.1" is-regexp "^1.0.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1": - version "6.0.1" - resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" - integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== - dependencies: - ansi-regex "^5.0.1" - -strip-ansi@^6.0.0, strip-ansi@^6.0.1: +"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1: version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==