diff --git a/apps/server/src/api-mcp/mcp.auth.ts b/apps/server/src/api-mcp/mcp.auth.ts deleted file mode 100644 index 18affc588..000000000 --- a/apps/server/src/api-mcp/mcp.auth.ts +++ /dev/null @@ -1,21 +0,0 @@ -import type { NextFunction, Request, RequestHandler, Response } from 'express'; - -import { hasPassword, hashedPassword } from '../api-data/session/session.service.js'; - -/** - * Wraps the app authenticate middleware with support for the Authorization header. - * MCP clients conventionally authenticate with `Authorization: Bearer ` - * rather than cookies or query params; any other request falls through to the - * app middleware, keeping the behaviour of the shared middleware untouched. - */ -export function makeMcpAuthenticate(fallback: RequestHandler): RequestHandler { - return function mcpAuthenticate(req: Request, res: Response, next: NextFunction) { - if (hasPassword) { - const authHeader = req.headers.authorization; - if (authHeader?.startsWith('Bearer ') && authHeader.slice(7) === hashedPassword) { - return next(); - } - } - return fallback(req, res, next); - }; -} diff --git a/apps/server/src/app.ts b/apps/server/src/app.ts index 29d9d17a6..04bd00a33 100644 --- a/apps/server/src/app.ts +++ b/apps/server/src/app.ts @@ -13,7 +13,6 @@ import { socket } from './adapters/WebsocketAdapter.js'; // Import Routers import { appRouter } from './api-data/index.js'; import { integrationRouter } from './api-integration/integration.router.js'; -import { makeMcpAuthenticate } from './api-mcp/mcp.auth.js'; import { mcpRouter } from './api-mcp/mcp.router.js'; import { flushPendingWrites, getDataProvider } from './classes/data-provider/DataProvider.js'; // Services @@ -102,7 +101,7 @@ app.get(`${prefix}/ready`, (_req, res) => { app.use(`${prefix}/login`, loginRouter); // router for login flow app.use(`${prefix}/data`, authenticate, appRouter); // router for application data app.use(`${prefix}/api`, authenticate, integrationRouter); // router for integrations -app.use(`${prefix}/mcp`, makeMcpAuthenticate(authenticate), mcpRouter); // router for MCP agent integration +app.use(`${prefix}/mcp`, authenticate, mcpRouter); // router for MCP agent integration // serve static external files app.use( diff --git a/apps/server/src/middleware/__tests__/authenticate.test.ts b/apps/server/src/middleware/__tests__/authenticate.test.ts index aa1bd897f..f18e8efe3 100644 --- a/apps/server/src/middleware/__tests__/authenticate.test.ts +++ b/apps/server/src/middleware/__tests__/authenticate.test.ts @@ -1,6 +1,35 @@ -import { describe, expect, it } from 'vitest'; +import type { IncomingMessage } from 'node:http'; -import { isPublicAssetRequest } from '../authenticate.js'; +import type { NextFunction, Request, Response } from 'express'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('../../api-data/session/session.service.js', () => ({ + hasPassword: true, + hashedPassword: 'valid-token', +})); + +import { + authenticateSocket, + isPublicAssetRequest, + makeAuthenticateMiddleware, +} from '../authenticate.js'; + +function makeResponse() { + return { + redirect: vi.fn(), + send: vi.fn(), + status: vi.fn().mockReturnThis(), + } as unknown as Response; +} + +function makeHeadersWithFailingAuthorization(cookie?: string) { + return { + cookie, + get authorization(): never { + throw new Error('Authorization header should not be read'); + }, + }; +} describe('isPublicAssetRequest()', () => { it('allows root public assets without a prefix', () => { @@ -18,3 +47,102 @@ describe('isPublicAssetRequest()', () => { expect(isPublicAssetRequest('/backstage', '')).toBe(false); }); }); + +describe('bearer authentication', () => { + const next = vi.fn() as NextFunction; + + beforeEach(() => { + next.mockClear(); + }); + + it('prioritises cookie authentication for API requests', () => { + const { authenticate } = makeAuthenticateMiddleware(''); + const req = { + cookies: { token: JSON.stringify({ token: 'valid-token' }) }, + headers: makeHeadersWithFailingAuthorization(), + query: {}, + } as unknown as Request; + + expect(() => authenticate(req, makeResponse(), next)).not.toThrow(); + expect(next).toHaveBeenCalledOnce(); + }); + + it('prioritises cookie authentication for redirecting routes', () => { + const { authenticateAndRedirect } = makeAuthenticateMiddleware(''); + const req = { + cookies: { token: JSON.stringify({ token: 'valid-token' }) }, + headers: makeHeadersWithFailingAuthorization(), + originalUrl: '/external/image.png', + query: {}, + } as unknown as Request; + + expect(() => authenticateAndRedirect(req, makeResponse(), next)).not.toThrow(); + expect(next).toHaveBeenCalledOnce(); + }); + + it('prioritises cookie authentication for WebSocket handshakes', () => { + const cookie = `token=${encodeURIComponent(JSON.stringify({ token: 'valid-token' }))}`; + const req = { headers: makeHeadersWithFailingAuthorization(cookie) } as IncomingMessage; + + expect(() => authenticateSocket({} as never, req, next)).not.toThrow(); + expect(next).toHaveBeenCalledOnce(); + }); + + it('authenticates API requests with a bearer token', () => { + const { authenticate } = makeAuthenticateMiddleware(''); + const req = { + cookies: {}, + headers: { authorization: 'Bearer valid-token' }, + query: {}, + } as unknown as Request; + const res = makeResponse(); + + authenticate(req, res, next); + + expect(next).toHaveBeenCalledOnce(); + expect(res.status).not.toHaveBeenCalled(); + }); + + it('authenticates redirecting routes with a bearer token', () => { + const { authenticateAndRedirect } = makeAuthenticateMiddleware('/stage'); + const req = { + cookies: {}, + headers: { authorization: 'Bearer valid-token' }, + originalUrl: '/stage/external/image.png', + query: {}, + } as unknown as Request; + const res = makeResponse(); + + authenticateAndRedirect(req, res, next); + + expect(next).toHaveBeenCalledOnce(); + expect(res.redirect).not.toHaveBeenCalled(); + }); + + it('authenticates WebSocket handshakes with a bearer token', () => { + const req = { + headers: { authorization: 'Bearer valid-token' }, + } as IncomingMessage; + + authenticateSocket({} as never, req, next); + + expect(next).toHaveBeenCalledOnce(); + expect(next).toHaveBeenCalledWith(); + }); + + it('rejects an invalid bearer token', () => { + const { authenticate } = makeAuthenticateMiddleware(''); + const req = { + cookies: {}, + headers: { authorization: 'Bearer invalid-token' }, + query: {}, + } as unknown as Request; + const res = makeResponse(); + + authenticate(req, res, next); + + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(401); + expect(res.send).toHaveBeenCalledWith('Unauthorized'); + }); +}); diff --git a/apps/server/src/middleware/authenticate.ts b/apps/server/src/middleware/authenticate.ts index 3b28b23cd..434674cb5 100644 --- a/apps/server/src/middleware/authenticate.ts +++ b/apps/server/src/middleware/authenticate.ts @@ -77,17 +77,16 @@ export function makeAuthenticateMiddleware(prefix: string) { const loginRedirectBase = `${prefix}/login?redirect=`; function authenticate(req: Request, res: Response, next: NextFunction) { - if (req.query.token) { - if (req.query.token === hashedPassword) { - return next(); - } + if (getTokenFromCookies(req.cookies) === hashedPassword) { + return next(); } - if (req.cookies?.token) { - const tokenFromCookie = getTokenFromCookie(req.cookies.token); - if (tokenFromCookie === hashedPassword) { - return next(); - } + if (getTokenFromAuthHeader(req.headers.authorization) === hashedPassword) { + return next(); + } + + if (getTokenFromParams(req.query) === hashedPassword) { + return next(); } res.status(401).send('Unauthorized'); @@ -105,17 +104,17 @@ export function makeAuthenticateMiddleware(prefix: string) { return next(); } - // we expect the token to be in the cookies - if (req.cookies?.token) { - const tokenFromCookie = getTokenFromCookie(req.cookies.token); - if (tokenFromCookie === hashedPassword) { - return next(); - } + if (getTokenFromCookies(req.cookies) === hashedPassword) { + return next(); + } + + if (getTokenFromAuthHeader(req.headers.authorization) === hashedPassword) { + return next(); } // we use query params for generating authenticated URLs and for clients like the companion module // if the user gives is a token in the query params, we set the cookie to be used in further requests - if (req.query.token === hashedPassword) { + if (getTokenFromParams(req.query) === hashedPassword) { if (hashedPassword !== undefined) { setSessionCookie(res, hashedPassword, prefix); } @@ -136,33 +135,16 @@ export function authenticateSocket(_ws: WebSocket, req: IncomingMessage, next: ( return next(); } - // check if the token is in the cookie - const cookieString = req.headers.cookie; - if (typeof cookieString === 'string') { - const cookies = parseCookie(cookieString); - if (cookies.token) { - const token = getTokenFromCookie(cookies.token); - if (token === hashedPassword) { - return next(); - } - } - } - - // check if token is in the params - simple string check first - const urlString = req.url || ''; - if (urlString.includes(`token=${hashedPassword}`)) { + if (getTokenFromCookies(req.headers.cookie) === hashedPassword) { return next(); } - // fallback to full URL parsing for other formats - try { - const url = new URL(urlString, `http://${req.headers.host}`); - const token = url.searchParams.get('token'); - if (token === hashedPassword) { - return next(); - } - } catch (_) { - // ignore URL parsing errors + if (getTokenFromAuthHeader(req.headers.authorization) === hashedPassword) { + return next(); + } + + if (getTokenFromParams(req.url, req.headers.host) === hashedPassword) { + return next(); } return next(new Error('Unauthorized')); @@ -181,19 +163,18 @@ function setSessionCookie(res: Response, token: string, prefix: string) { }); } -/** - * When calling this function we already know a cookie called 'token' exists - * And want to extract its value - */ -function getTokenFromCookie(cookieContents: string): string | undefined { - // Fast path: check if the hashed password is directly in the cookie string - // This avoids JSON parsing for the common case +function getTokenFromCookies(cookies: string | Record | undefined): string | undefined { + const cookieContents = typeof cookies === 'string' ? parseCookie(cookies).token : cookies?.token; + if (typeof cookieContents !== 'string') { + return undefined; + } + + // Fast path: avoid JSON parsing when the expected token can be found directly const cookieTokenString = '"token":"' + hashedPassword + '}"'; if (cookieTokenString && cookieContents.includes(cookieTokenString)) { return hashedPassword; } - // Fallback to JSON parsing for other cases or validation try { const cookie = JSON.parse(cookieContents); if (cookie && typeof cookie.token === 'string') { @@ -203,3 +184,29 @@ function getTokenFromCookie(cookieContents: string): string | undefined { // no error handling to do here } } + +function getTokenFromAuthHeader(authorization: string | undefined): string | undefined { + if (authorization?.startsWith('Bearer ')) { + return authorization.slice(7); + } +} + +function getTokenFromParams( + params: string | Record | undefined, + host?: string, +): string | undefined { + if (typeof params !== 'string') { + return typeof params?.token === 'string' ? params.token : undefined; + } + + // Fast path for WebSocket URLs + if (params.includes(`token=${hashedPassword}`)) { + return hashedPassword; + } + + try { + return new URL(params, `http://${host}`).searchParams.get('token') ?? undefined; + } catch (_) { + return undefined; + } +}