diff --git a/graphile/graphile-settings/src/index.ts b/graphile/graphile-settings/src/index.ts index 8d41a1e9d..0c5740aa4 100644 --- a/graphile/graphile-settings/src/index.ts +++ b/graphile/graphile-settings/src/index.ts @@ -58,9 +58,6 @@ export * from './presets/index'; // Re-export makePgService for convenience export { makePgService }; -// Upload utilities -export { streamToStorage } from './upload-resolver'; - // Presigned URL utilities export { getPresignedUrlS3Config } from './presigned-url-resolver'; diff --git a/graphile/graphile-settings/src/upload-resolver.ts b/graphile/graphile-settings/src/upload-resolver.ts index 0fd3a17e2..423020b74 100644 --- a/graphile/graphile-settings/src/upload-resolver.ts +++ b/graphile/graphile-settings/src/upload-resolver.ts @@ -21,7 +21,6 @@ import uploadNames from '@constructive-io/upload-names'; import { getEnvOptions } from '@constructive-io/graphql-env'; import { Logger } from '@pgpmjs/logger'; import { randomBytes } from 'crypto'; -import type { Readable } from 'stream'; import type { FileUpload, UploadFieldDefinition, @@ -78,30 +77,6 @@ function generateKey(filename: string): string { return `${rand}-${uploadNames(filename)}`; } -/** - * Streams a file to S3/MinIO storage and returns the URL and metadata. - * - * Reusable by both the GraphQL upload resolver and REST /upload endpoint. - */ -export async function streamToStorage( - readStream: Readable, - filename: string, -): Promise<{ url: string; filename: string; mime: string }> { - const s3 = getStreamer(); - const key = generateKey(filename); - const result = await s3.upload({ - readStream, - filename, - key, - bucket: bucketName, - }); - return { - url: result.upload.Location, - filename, - mime: result.contentType, - }; -} - /** * Upload resolver that streams files to S3/MinIO. * diff --git a/graphile/graphile-upload-plugin/ARCHITECTURE.md b/graphile/graphile-upload-plugin/ARCHITECTURE.md index a41affac5..27755ee74 100644 --- a/graphile/graphile-upload-plugin/ARCHITECTURE.md +++ b/graphile/graphile-upload-plugin/ARCHITECTURE.md @@ -4,7 +4,6 @@ This document is the single source of truth for Constructive upload behavior acr - `graphile/graphile-upload-plugin` (GraphQL upload field wrapping) - `graphile/graphile-settings` (upload resolver + storage helpers) -- `graphql/server` (`POST /upload` endpoint + upload auth middleware) It replaces the previous upload handoff notes so upload behavior is documented in one place. @@ -20,11 +19,6 @@ phase (before resolver-time upload transformation), causing: - `"Attempted to update a record, but no new values were specified."` -The stable production pattern is now: - -1. Upload file to `POST /upload` (REST multipart endpoint) -2. Patch DB row with returned metadata via normal JSON GraphQL mutation - ## Current Design ### 1) GraphQL Upload Plugin (Resolver-Based Compatibility Layer) @@ -64,8 +58,7 @@ Files: - `uploads/s3-streamer/src/streamer.ts` - `uploads/s3-streamer/src/utils.ts` -`uploadResolver` and `streamToStorage` are shared by GraphQL resolver path and -REST endpoint path. +`uploadResolver` streams GraphQL upload values to configured object storage. Security behavior: @@ -76,73 +69,6 @@ Security behavior: This prevents writing disallowed files to object storage and avoids orphaned objects from post-upload MIME rejection. -### 3) REST `/upload` Endpoint + Upload-Specific Auth - -File: -- `graphql/server/src/middleware/upload.ts` - -This file now contains both: - -- `createUploadAuthenticateMiddleware(opts)` -- `uploadRoute` - -`upload-auth.ts` was intentionally consolidated into `upload.ts`. - -Server wiring: -- `graphql/server/src/server.ts` - - `app.post('/upload', uploadAuthenticate, ...uploadRoute)` - -#### Upload auth semantics - -`/upload` is strict: - -- Requires bearer token -- Returns HTTP `401` for missing/invalid auth -- Requires `req.token.user_id` - -It does not rely on shared GraphQL auth behavior for anonymous fallback. - -RLS module resolution for `/upload`: - -1. Use `req.api.rlsModule` when present (API-scoped) -2. Fallback to database-scoped lookup by `database_id` -3. Fallback to database-scoped lookup by `dbname` - -This preserves existing GraphQL route semantics while making `/upload` work for -both: - -- meta API hosts (`api.localhost:3000`) -- data API hosts (`app-public-...localhost:3000`) - -#### Upload endpoint behavior - -Request: - -- `POST /upload` -- `Content-Type: multipart/form-data` -- one file field named `file` -- `Authorization: Bearer ` - -Success response (`200`): - -```json -{ - "url": "https://bucket.s3.amazonaws.com/a8f3k2m9x1-photo.jpg", - "filename": "photo.jpg", - "mime": "image/jpeg", - "size": 123456 -} -``` - -Errors: - -- `401` auth missing/invalid -- `400` no `file` field -- `413` payload too large (multer limit) - -Current route file-size limit: -- `10 MB` (`graphql/server/src/middleware/upload.ts`) - ## Resolver-Based vs Plan-Based (Decision) The upload plugin remains resolver-based on purpose. @@ -163,18 +89,13 @@ Revisit if: ## Frontend Integration Contract -Use two-step flow: +Use GraphQL-backed upload flows. Do not derive a REST upload endpoint from the +GraphQL endpoint. -1. Upload file to `/upload` -2. Patch actual DB column field in JSON GraphQL mutation +For domain-backed row fields, use GraphQL multipart direct upload when the +generated schema exposes `*Upload` input fields. -Do not patch `*Upload` fields in CRUD inputs for this flow. - -Upload URL derivation from GraphQL endpoint: - -```ts -const uploadUrl = graphqlEndpoint.replace(/\/graphql\/?$/, '/upload'); -``` +For storage-module file records, use presigned URL upload. Column patch shape depends on DB domain: @@ -189,13 +110,8 @@ From `graphql/server/src/server.ts`: - `graphqlUploadExpress()` - `multipartBridge` - shared API/auth/graphile chain -- `/upload` - - shared API resolution (`api`) - - upload-specific auth (`createUploadAuthenticateMiddleware`) - - multipart parsing + storage (`uploadRoute`) -`graphqlUploadExpress` remains scoped to `/graphql` and does not interfere with -`/upload`. +`graphqlUploadExpress` remains scoped to `/graphql`. ## Test Coverage @@ -205,12 +121,26 @@ Current key tests: - max size enforcement on resolver-consumed streams - `graphile/graphile-settings/__tests__/upload-resolver.test.ts` - MIME validation before storage upload -- `graphql/server/src/middleware/__tests__/upload.test.ts` - - upload auth behavior, fallback RLS resolution, strict auth handling -## Operational Notes +## Removed REST `/upload` Endpoint + +The historical REST `POST /upload` endpoint and its upload-specific auth +middleware have been removed from `graphql/server`. + +Removed server pieces included: + +- `graphql/server/src/middleware/upload.ts` +- `createUploadAuthenticateMiddleware(opts)` +- `uploadRoute` +- `multer` +- `streamToStorage()` + +Clients must stop deriving upload URLs with: + +```ts +const uploadUrl = graphqlEndpoint.replace(/\/graphql\/?$/, '/upload'); +``` + +Known dashboard impact is documented in: -- Auth failures on `/upload` are intentionally HTTP `401` JSON errors. -- GraphQL shared auth behavior (including anon fallback patterns) is unchanged. -- This separation avoids cross-route behavior regressions while fixing data API - upload auth. +- `graphile/graphile-upload-plugin/DASHBOARD_UPLOAD_FOLLOWUP.md` diff --git a/graphile/graphile-upload-plugin/DASHBOARD_UPLOAD_FOLLOWUP.md b/graphile/graphile-upload-plugin/DASHBOARD_UPLOAD_FOLLOWUP.md new file mode 100644 index 000000000..c0a53130b --- /dev/null +++ b/graphile/graphile-upload-plugin/DASHBOARD_UPLOAD_FOLLOWUP.md @@ -0,0 +1,94 @@ +# Dashboard Upload Follow-Up + +This document records dashboard impact from removing the historical Constructive +REST upload endpoint: + +```text +POST /upload +``` + +Constructive no longer serves that endpoint. Dashboard code that derives +`/upload` from a GraphQL endpoint must migrate to a GraphQL-backed upload path. + +Snapshot checked: + +- `constructive`: `main` at `6d810d1b80` +- `dashboard`: `main` at `c92bd5c427` + +## Affected Dashboard Runtime Code + +The current dashboard `packages/sheets` package still has runtime dependencies +on REST `/upload`. + +### `packages/sheets/src/hooks/use-sheets-upload.ts` + +Current flow: + +```text +REST /upload +-> returns { url, filename, mime, size } +-> GraphQL row update mutation patches the upload/image/attachment field +``` + +The code derives the endpoint with: + +```ts +const uploadUrl = endpoint.replace(/\/graphql\/?$/, '/upload'); +``` + +This will fail after the REST endpoint is removed. + +### `packages/sheets/src/context/sheets-execute.ts` + +`createSheetsUpload()` also derives `/upload` from the configured GraphQL +endpoint and returns `{ url }` for the caller. + +This should be migrated with the hook above so sheets has one upload path. + +### `packages/sheets/src/hooks/use-sheets-upload.test.ts` + +The test currently expects a REST request to `/upload` followed by a GraphQL +mutation to `/graphql`. + +The test should be rewritten around the replacement GraphQL upload flow. + +## Not Blocking This Removal + +These dashboard areas are not direct REST `/upload` dependencies: + +- Admin app GraphQL upload fields such as `logoUpload`, `faviconUpload`, and + `ogImageUpload` +- Generated SDK hooks such as `requestUploadUrl` and `confirmUpload` +- Presigned upload helpers such as `uploadAppFile` and `putToPresignedUrl` +- `packages/sheets/src/grid/editors/upload-editor.tsx`, where + `https://example.com/uploads/...` is only a mock URL + +## Recommended Migration + +For sheets row fields backed by Constructive upload domains, prefer GraphQL +multipart direct upload. + +Expected direction: + +```text +executeFieldUpload() +-> build a GraphQL multipart request to /graphql +-> call the relevant update mutation +-> patch with Upload: File +-> read the updated field value from the returned row +``` + +This keeps the existing row-field model close to the old REST response shape: + +- `image` / `upload` domains return `{ url, filename, mime }` +- `attachment` returns a URL string + +Before implementing the dashboard migration, verify: + +- The target generated schema exposes `Upload` input fields. +- Existing sheets UI code can still consume the returned field values. +- Cookie-authenticated dashboard requests satisfy `/graphql` CSRF behavior. +- Bearer-token authenticated requests continue to skip CSRF as expected. + +If sheets should instead model files as storage-module records, use presigned +URL upload as a separate product/data-model migration. diff --git a/graphql/server/package.json b/graphql/server/package.json index d2902bad1..f8b9ee8e4 100644 --- a/graphql/server/package.json +++ b/graphql/server/package.json @@ -47,14 +47,12 @@ "@constructive-io/graphql-env": "workspace:^", "@constructive-io/graphql-types": "workspace:^", "@constructive-io/s3-utils": "workspace:^", - "@constructive-io/upload-names": "workspace:^", "@constructive-io/url-domains": "workspace:^", "@graphile-contrib/pg-many-to-many": "2.0.0-rc.2", "@pgpmjs/env": "workspace:^", "@pgpmjs/logger": "workspace:^", "@pgpmjs/server-utils": "workspace:^", "@pgpmjs/types": "workspace:^", - "@pgsql/quotes": "^17.1.0", "cors": "^2.8.6", "deepmerge": "^4.3.1", "express": "^5.2.1", @@ -71,7 +69,6 @@ "graphql": "16.13.0", "graphql-upload": "^13.0.0", "lru-cache": "^11.2.7", - "multer": "^2.1.1", "pg": "^8.21.0", "pg-cache": "workspace:^", "pg-env": "workspace:^", @@ -86,7 +83,6 @@ "@types/cors": "^2.8.17", "@types/express": "^5.0.6", "@types/graphql-upload": "^8.0.12", - "@types/multer": "^2.1.0", "@types/pg": "^8.20.0", "@types/request-ip": "^0.0.41", "cookie-parser": "^1.4.7", diff --git a/graphql/server/src/index.ts b/graphql/server/src/index.ts index 06d1561bd..53dd89cc1 100644 --- a/graphql/server/src/index.ts +++ b/graphql/server/src/index.ts @@ -3,7 +3,6 @@ export * from './server'; // Export middleware for use in testing packages export { createApiMiddleware, getSubdomain, getApiConfig } from './middleware/api'; export { createAuthenticateMiddleware } from './middleware/auth'; -export { createUploadAuthenticateMiddleware } from './middleware/upload'; export { cors } from './middleware/cors'; export { graphile } from './middleware/graphile'; export { flush, flushService } from './middleware/flush'; diff --git a/graphql/server/src/middleware/__tests__/upload.test.ts b/graphql/server/src/middleware/__tests__/upload.test.ts deleted file mode 100644 index 33abc00d0..000000000 --- a/graphql/server/src/middleware/__tests__/upload.test.ts +++ /dev/null @@ -1,503 +0,0 @@ -import type { NextFunction, Request, Response } from 'express'; -import { getPgPool } from 'pg-cache'; -import pgQueryContext from 'pg-query-context'; -import type { ApiStructure } from '../../types'; -import { createUploadAuthenticateMiddleware } from '../upload'; - -jest.mock('pg-cache', () => ({ - getPgPool: jest.fn(), -})); - -jest.mock('pg-query-context', () => jest.fn()); - -const mockGetPgPool = getPgPool as jest.MockedFunction; -const mockPgQueryContext = pgQueryContext as jest.MockedFunction; - -interface MockPool { - query: jest.Mock; -} - -const baseApi: ApiStructure = { - apiId: 'api-123', - dbname: 'tenant_db', - anonRole: 'anonymous', - roleName: 'authenticated', - schema: ['public'], - apiModules: [], - databaseId: 'db-123', - isPublic: true, -}; - -function makeReq(input: { - api?: ApiStructure; - headers?: Record; - clientIp?: string; - token?: Record; -} = {}): Request { - const headers = input.headers ?? {}; - const normalizedHeaders = Object.fromEntries( - Object.entries(headers).map(([key, value]) => [key.toLowerCase(), value]), - ); - - return { - api: input.api, - headers, - clientIp: input.clientIp ?? '127.0.0.1', - token: input.token, - get(name: string) { - return normalizedHeaders[name.toLowerCase()]; - }, - } as unknown as Request; -} - -function makeRes(): Response { - return { - status: jest.fn().mockReturnThis(), - json: jest.fn().mockReturnThis(), - send: jest.fn().mockReturnThis(), - } as unknown as Response; -} - -function makeNext(): NextFunction { - return jest.fn() as unknown as NextFunction; -} - -describe('createUploadAuthenticateMiddleware', () => { - let rootPool: MockPool; - let tenantPool: MockPool; - - beforeEach(() => { - jest.clearAllMocks(); - - rootPool = { - query: jest.fn(), - }; - - tenantPool = { - query: jest.fn(), - }; - - mockGetPgPool.mockImplementation((config: any) => { - if (config?.database === 'tenant_db') return tenantPool as any; - return rootPool as any; - }); - }); - - it('returns 500 when API info is missing', async () => { - const middleware = createUploadAuthenticateMiddleware({ - pg: { database: 'services' }, - } as any); - const req = makeReq(); - const res = makeRes(); - const next = makeNext(); - - await middleware(req, res, next); - - expect((res.status as jest.Mock)).toHaveBeenCalledWith(500); - expect((res.send as jest.Mock)).toHaveBeenCalledWith('Missing API info'); - expect(next).not.toHaveBeenCalled(); - }); - - it('returns 401 when bearer token is missing', async () => { - const middleware = createUploadAuthenticateMiddleware({ - pg: { database: 'services' }, - server: { strictAuth: false }, - } as any); - const req = makeReq({ api: baseApi }); - const res = makeRes(); - const next = makeNext(); - - await middleware(req, res, next); - - expect((res.status as jest.Mock)).toHaveBeenCalledWith(401); - expect((res.json as jest.Mock)).toHaveBeenCalledWith({ - error: 'Authentication required', - }); - expect(next).not.toHaveBeenCalled(); - }); - - it('authenticates using API-scoped RLS module', async () => { - const middleware = createUploadAuthenticateMiddleware({ - pg: { database: 'services' }, - server: { strictAuth: false }, - } as any); - const req = makeReq({ - api: { - ...baseApi, - rlsModule: { - authenticate: 'authenticate', - authenticateStrict: 'authenticate_strict', - privateSchema: { schemaName: 'private' }, - publicSchema: { schemaName: 'public' }, - currentRole: 'current_user', - currentRoleId: 'current_user_id', - currentIpAddress: 'current_ip_address', - currentUserAgent: 'current_user_agent', - }, - }, - headers: { - authorization: 'Bearer good-token', - origin: 'https://app.localhost', - 'user-agent': 'JestAgent/1.0', - }, - clientIp: '10.0.0.1', - }); - const res = makeRes(); - const next = makeNext(); - - mockPgQueryContext.mockResolvedValue({ - rowCount: 1, - rows: [{ id: 'token-id', user_id: 'user-id' }], - } as any); - - await middleware(req, res, next); - - expect(mockPgQueryContext).toHaveBeenCalledWith( - expect.objectContaining({ - client: tenantPool, - query: 'SELECT * FROM private.authenticate($1)', - variables: ['good-token'], - context: expect.objectContaining({ - 'jwt.claims.ip_address': '10.0.0.1', - 'jwt.claims.origin': 'https://app.localhost', - 'jwt.claims.user_agent': 'JestAgent/1.0', - }), - }), - ); - expect(req.token).toEqual({ id: 'token-id', user_id: 'user-id' }); - expect(next).toHaveBeenCalledTimes(1); - }); - - it('falls back to database-scoped RLS module lookup when api.rlsModule is missing', async () => { - const middleware = createUploadAuthenticateMiddleware({ - pg: { database: 'services' }, - server: { strictAuth: false }, - } as any); - const req = makeReq({ - api: { - ...baseApi, - apiId: undefined, - rlsModule: undefined, - }, - headers: { authorization: 'Bearer fallback-token' }, - }); - const res = makeRes(); - const next = makeNext(); - - // typed rls_settings query returns no rows (table may not exist yet) - rootPool.query.mockResolvedValueOnce({ rows: [] }); - // legacy api_modules fallback - rootPool.query.mockResolvedValueOnce({ - rows: [ - { - data: { - authenticate: 'authenticate', - authenticate_strict: 'authenticate_strict', - authenticate_schema: 'private', - role_schema: 'public', - current_role: 'current_user', - current_role_id: 'current_user_id', - current_ip_address: 'current_ip_address', - current_user_agent: 'current_user_agent', - }, - private_schema_name: 'private', - public_schema_name: 'public', - }, - ], - }); - - mockPgQueryContext.mockResolvedValue({ - rowCount: 1, - rows: [{ id: 'token-id', user_id: 'user-id' }], - } as any); - - await middleware(req, res, next); - - expect(rootPool.query).toHaveBeenCalledWith( - expect.stringContaining('WHERE'), - ['db-123'], - ); - expect(next).toHaveBeenCalledTimes(1); - }); - - it('falls back to api_id-scoped RLS module lookup when api.rlsModule is missing', async () => { - const middleware = createUploadAuthenticateMiddleware({ - pg: { database: 'services' }, - server: { strictAuth: false }, - } as any); - const req = makeReq({ - api: { - ...baseApi, - rlsModule: undefined, - }, - headers: { authorization: 'Bearer fallback-token' }, - }); - const res = makeRes(); - const next = makeNext(); - - rootPool.query.mockResolvedValueOnce({ - rows: [ - { - data: { - authenticate: 'authenticate', - authenticate_strict: 'authenticate_strict', - authenticate_schema: 'private', - role_schema: 'public', - current_role: 'current_user', - current_role_id: 'current_user_id', - current_ip_address: 'current_ip_address', - current_user_agent: 'current_user_agent', - }, - private_schema_name: 'private', - public_schema_name: 'public', - }, - ], - }); - - mockPgQueryContext.mockResolvedValue({ - rowCount: 1, - rows: [{ id: 'token-id', user_id: 'user-id' }], - } as any); - - await middleware(req, res, next); - - expect(rootPool.query).toHaveBeenCalledWith( - expect.stringContaining('WHERE'), - ['api-123'], - ); - expect(next).toHaveBeenCalledTimes(1); - }); - - it('falls back to dbname lookup when databaseId is missing', async () => { - const middleware = createUploadAuthenticateMiddleware({ - pg: { database: 'services' }, - server: { strictAuth: false }, - } as any); - const req = makeReq({ - api: { - ...baseApi, - apiId: undefined, - databaseId: undefined, - rlsModule: undefined, - }, - headers: { authorization: 'Bearer fallback-token' }, - }); - const res = makeRes(); - const next = makeNext(); - - // typed rls_settings query returns no rows (table may not exist yet) - rootPool.query.mockResolvedValueOnce({ rows: [] }); - // legacy api_modules fallback - rootPool.query.mockResolvedValueOnce({ - rows: [ - { - data: { - authenticate: 'authenticate', - authenticate_strict: 'authenticate_strict', - authenticate_schema: 'private', - role_schema: 'public', - current_role: 'current_user', - current_role_id: 'current_user_id', - current_ip_address: 'current_ip_address', - current_user_agent: 'current_user_agent', - }, - private_schema_name: 'private', - public_schema_name: 'public', - }, - ], - }); - mockPgQueryContext.mockResolvedValue({ - rowCount: 1, - rows: [{ id: 'token-id', user_id: 'user-id' }], - } as any); - - await middleware(req, res, next); - - expect(rootPool.query).toHaveBeenCalledWith( - expect.stringContaining('WHERE'), - ['tenant_db'], - ); - expect(next).toHaveBeenCalledTimes(1); - }); - - it('returns 401 when no RLS module can be resolved', async () => { - const middleware = createUploadAuthenticateMiddleware({ - pg: { database: 'services' }, - server: { strictAuth: false }, - } as any); - const req = makeReq({ - api: { - ...baseApi, - apiId: undefined, - rlsModule: undefined, - }, - headers: { authorization: 'Bearer missing-module-token' }, - }); - const res = makeRes(); - const next = makeNext(); - - // typed rls_settings query returns no rows - rootPool.query.mockResolvedValueOnce({ rows: [] }); - // legacy api_modules by database_id returns no rows - rootPool.query.mockResolvedValueOnce({ rows: [] }); - // typed rls_settings by dbname returns no rows - rootPool.query.mockResolvedValueOnce({ rows: [] }); - // legacy api_modules by dbname returns no rows - rootPool.query.mockResolvedValueOnce({ rows: [] }); - - await middleware(req, res, next); - - expect((res.status as jest.Mock)).toHaveBeenCalledWith(401); - expect((res.json as jest.Mock)).toHaveBeenCalledWith({ - error: 'Authentication required', - }); - expect(mockPgQueryContext).not.toHaveBeenCalled(); - expect(next).not.toHaveBeenCalled(); - }); - - it('returns 401 when token validation returns no rows', async () => { - const middleware = createUploadAuthenticateMiddleware({ - pg: { database: 'services' }, - server: { strictAuth: false }, - } as any); - const req = makeReq({ - api: { - ...baseApi, - rlsModule: { - authenticate: 'authenticate', - authenticateStrict: 'authenticate_strict', - privateSchema: { schemaName: 'private' }, - publicSchema: { schemaName: 'public' }, - currentRole: 'current_user', - currentRoleId: 'current_user_id', - currentIpAddress: 'current_ip_address', - currentUserAgent: 'current_user_agent', - }, - }, - headers: { authorization: 'Bearer invalid-token' }, - }); - const res = makeRes(); - const next = makeNext(); - - mockPgQueryContext.mockResolvedValue({ - rowCount: 0, - rows: [], - } as any); - - await middleware(req, res, next); - - expect((res.status as jest.Mock)).toHaveBeenCalledWith(401); - expect((res.json as jest.Mock)).toHaveBeenCalledWith({ - error: 'Authentication required', - }); - expect(next).not.toHaveBeenCalled(); - }); - - it('returns 401 when token validation throws', async () => { - const middleware = createUploadAuthenticateMiddleware({ - pg: { database: 'services' }, - server: { strictAuth: false }, - } as any); - const req = makeReq({ - api: { - ...baseApi, - rlsModule: { - authenticate: 'authenticate', - authenticateStrict: 'authenticate_strict', - privateSchema: { schemaName: 'private' }, - publicSchema: { schemaName: 'public' }, - currentRole: 'current_user', - currentRoleId: 'current_user_id', - currentIpAddress: 'current_ip_address', - currentUserAgent: 'current_user_agent', - }, - }, - headers: { authorization: 'Bearer bad-token' }, - }); - const res = makeRes(); - const next = makeNext(); - - mockPgQueryContext.mockRejectedValue(new Error('boom')); - - await middleware(req, res, next); - - expect((res.status as jest.Mock)).toHaveBeenCalledWith(401); - expect((res.json as jest.Mock)).toHaveBeenCalledWith({ - error: 'Authentication required', - }); - expect(next).not.toHaveBeenCalled(); - }); - - it('uses authenticateStrict when strictAuth is enabled', async () => { - const middleware = createUploadAuthenticateMiddleware({ - pg: { database: 'services' }, - server: { strictAuth: true }, - } as any); - const req = makeReq({ - api: { - ...baseApi, - rlsModule: { - authenticate: 'authenticate', - authenticateStrict: 'authenticate_strict', - privateSchema: { schemaName: 'private' }, - publicSchema: { schemaName: 'public' }, - currentRole: 'current_user', - currentRoleId: 'current_user_id', - currentIpAddress: 'current_ip_address', - currentUserAgent: 'current_user_agent', - }, - }, - headers: { authorization: 'Bearer strict-token' }, - }); - const res = makeRes(); - const next = makeNext(); - - mockPgQueryContext.mockResolvedValue({ - rowCount: 1, - rows: [{ id: 'strict-token-id', user_id: 'strict-user-id' }], - } as any); - - await middleware(req, res, next); - - expect(mockPgQueryContext).toHaveBeenCalledWith( - expect.objectContaining({ - query: 'SELECT * FROM private.authenticate_strict($1)', - }), - ); - expect(next).toHaveBeenCalledTimes(1); - }); - - it('returns 401 when strictAuth is enabled but authenticateStrict is missing', async () => { - const middleware = createUploadAuthenticateMiddleware({ - pg: { database: 'services' }, - server: { strictAuth: true }, - } as any); - const req = makeReq({ - api: { - ...baseApi, - rlsModule: { - authenticate: 'authenticate', - authenticateStrict: '', - privateSchema: { schemaName: 'private' }, - publicSchema: { schemaName: 'public' }, - currentRole: 'current_user', - currentRoleId: 'current_user_id', - currentIpAddress: 'current_ip_address', - currentUserAgent: 'current_user_agent', - }, - }, - headers: { authorization: 'Bearer strict-token' }, - }); - const res = makeRes(); - const next = makeNext(); - - await middleware(req, res, next); - - expect((res.status as jest.Mock)).toHaveBeenCalledWith(401); - expect((res.json as jest.Mock)).toHaveBeenCalledWith({ - error: 'Authentication required', - }); - expect(mockPgQueryContext).not.toHaveBeenCalled(); - expect(next).not.toHaveBeenCalled(); - }); -}); diff --git a/graphql/server/src/middleware/upload.ts b/graphql/server/src/middleware/upload.ts deleted file mode 100644 index 5a9ee7209..000000000 --- a/graphql/server/src/middleware/upload.ts +++ /dev/null @@ -1,396 +0,0 @@ -import { Logger } from '@pgpmjs/logger'; -import type { PgpmOptions } from '@pgpmjs/types'; -import type { NextFunction, Request, RequestHandler, Response } from 'express'; -import fs from 'fs'; -import multer from 'multer'; -import os from 'os'; -import path from 'path'; -import { QuoteUtils } from '@pgsql/quotes'; -import type { Pool } from 'pg'; -import { getPgPool } from 'pg-cache'; -import pgQueryContext from 'pg-query-context'; -import { streamToStorage } from 'graphile-settings'; -import type { RlsModule } from '../types'; -import './types'; - -const uploadLog = new Logger('upload'); -const authLog = new Logger('upload-auth'); - -const envFileSize = process.env.MAX_UPLOAD_FILE_SIZE - ? parseInt(process.env.MAX_UPLOAD_FILE_SIZE, 10) - : NaN; -const MAX_FILE_SIZE = envFileSize > 0 ? envFileSize : 10 * 1024 * 1024; - -const BLOCKED_MIME_TYPES = new Set([ - 'application/x-executable', - 'application/x-sharedlib', - 'application/x-mach-binary', - 'application/x-dosexec', - 'text/html', - 'application/xhtml+xml', - 'application/javascript', - 'text/javascript' -]); - -const parseFile = multer({ - storage: multer.diskStorage({ - destination: os.tmpdir(), - filename: (_req, _file, cb) => { - const uniqueSuffix = `${Date.now()}-${Math.round(Math.random() * 1e9)}`; - cb(null, `upload-${uniqueSuffix}.tmp`); - }, - }), - limits: { fileSize: MAX_FILE_SIZE }, -}).single('file'); - -const parseFileWithErrors: RequestHandler = (req, res, next) => { - parseFile(req, res, (err: any) => { - if (!err) return next(); - if (err.code === 'LIMIT_FILE_SIZE') { - return res.status(413).json({ error: `File exceeds maximum size of ${MAX_FILE_SIZE / (1024 * 1024)} MB` }); - } - if (err.code === 'LIMIT_UNEXPECTED_FILE') { - return res.status(400).json({ error: 'Unexpected file field. Send a single file as "file".' }); - } - return res.status(400).json({ error: 'File upload failed' }); - }); -}; - -const RLS_MODULE_BY_DATABASE_ID_SQL = ` - SELECT am.data - FROM services_public.api_modules am - JOIN services_public.apis a ON am.api_id = a.id - WHERE am.name = 'rls_module' AND a.database_id = $1 - ORDER BY a.id - LIMIT 1 -`; - -const RLS_MODULE_BY_API_ID_SQL = ` - SELECT data - FROM services_public.api_modules - WHERE api_id = $1 AND name = 'rls_module' - LIMIT 1 -`; - -const RLS_MODULE_BY_DBNAME_SQL = ` - SELECT am.data - FROM services_public.api_modules am - JOIN services_public.apis a ON am.api_id = a.id - WHERE am.name = 'rls_module' AND a.dbname = $1 - ORDER BY a.id - LIMIT 1 -`; - -const RLS_SETTINGS_BY_DATABASE_ID_SQL = ` - SELECT - auth_schema.schema_name AS authenticate_schema, - role_schema.schema_name AS role_schema, - auth_fn.name AS authenticate, - auth_strict_fn.name AS authenticate_strict, - role_fn.name AS current_role, - role_id_fn.name AS current_role_id, - ua_fn.name AS current_user_agent, - ip_fn.name AS current_ip_address - FROM services_public.rls_settings rs - LEFT JOIN metaschema_public.schema auth_schema ON rs.authenticate_schema_id = auth_schema.id - LEFT JOIN metaschema_public.schema role_schema ON rs.role_schema_id = role_schema.id - LEFT JOIN metaschema_public.function auth_fn ON rs.authenticate_function_id = auth_fn.id - LEFT JOIN metaschema_public.function auth_strict_fn ON rs.authenticate_strict_function_id = auth_strict_fn.id - LEFT JOIN metaschema_public.function role_fn ON rs.current_role_function_id = role_fn.id - LEFT JOIN metaschema_public.function role_id_fn ON rs.current_role_id_function_id = role_id_fn.id - LEFT JOIN metaschema_public.function ua_fn ON rs.current_user_agent_function_id = ua_fn.id - LEFT JOIN metaschema_public.function ip_fn ON rs.current_ip_address_function_id = ip_fn.id - WHERE rs.database_id = $1 - LIMIT 1 -`; - -const RLS_SETTINGS_BY_DBNAME_SQL = ` - SELECT - auth_schema.schema_name AS authenticate_schema, - role_schema.schema_name AS role_schema, - auth_fn.name AS authenticate, - auth_strict_fn.name AS authenticate_strict, - role_fn.name AS current_role, - role_id_fn.name AS current_role_id, - ua_fn.name AS current_user_agent, - ip_fn.name AS current_ip_address - FROM services_public.rls_settings rs - JOIN services_public.apis a ON rs.database_id = a.database_id - LEFT JOIN metaschema_public.schema auth_schema ON rs.authenticate_schema_id = auth_schema.id - LEFT JOIN metaschema_public.schema role_schema ON rs.role_schema_id = role_schema.id - LEFT JOIN metaschema_public.function auth_fn ON rs.authenticate_function_id = auth_fn.id - LEFT JOIN metaschema_public.function auth_strict_fn ON rs.authenticate_strict_function_id = auth_strict_fn.id - LEFT JOIN metaschema_public.function role_fn ON rs.current_role_function_id = role_fn.id - LEFT JOIN metaschema_public.function role_id_fn ON rs.current_role_id_function_id = role_id_fn.id - LEFT JOIN metaschema_public.function ua_fn ON rs.current_user_agent_function_id = ua_fn.id - LEFT JOIN metaschema_public.function ip_fn ON rs.current_ip_address_function_id = ip_fn.id - WHERE a.dbname = $1 - LIMIT 1 -`; - -interface RlsModuleData { - authenticate: string; - authenticate_strict: string; - authenticate_schema: string; - role_schema: string; - current_role: string; - current_role_id: string; - current_ip_address: string; - current_user_agent: string; -} - -interface RlsModuleRow { - data: RlsModuleData | null; -} - -const toRlsModule = (row: RlsModuleRow | null): RlsModule | undefined => { - if (!row?.data) return undefined; - const d = row.data; - return { - authenticate: d.authenticate, - authenticateStrict: d.authenticate_strict, - privateSchema: { schemaName: d.authenticate_schema }, - publicSchema: { schemaName: d.role_schema }, - currentRole: d.current_role, - currentRoleId: d.current_role_id, - currentIpAddress: d.current_ip_address, - currentUserAgent: d.current_user_agent, - }; -}; - -const toRlsModuleFromSettings = (row: RlsModuleData | null): RlsModule | undefined => { - if (!row) return undefined; - return { - authenticate: row.authenticate, - authenticateStrict: row.authenticate_strict, - privateSchema: { schemaName: row.authenticate_schema }, - publicSchema: { schemaName: row.role_schema }, - currentRole: row.current_role, - currentRoleId: row.current_role_id, - currentIpAddress: row.current_ip_address, - currentUserAgent: row.current_user_agent, - }; -}; - -const getBearerToken = (authorization?: string): string | null => { - if (!authorization) return null; - const [authType, authToken] = authorization.split(' '); - if (authType?.toLowerCase() !== 'bearer' || !authToken) { - return null; - } - return authToken; -}; - -const queryRlsSettingsByDatabaseId = async (pool: Pool, databaseId: string): Promise => { - try { - const result = await pool.query(RLS_SETTINGS_BY_DATABASE_ID_SQL, [databaseId]); - return toRlsModuleFromSettings(result.rows[0] ?? null); - } catch { - return undefined; - } -}; - -const queryRlsSettingsByDbname = async (pool: Pool, dbname: string): Promise => { - try { - const result = await pool.query(RLS_SETTINGS_BY_DBNAME_SQL, [dbname]); - return toRlsModuleFromSettings(result.rows[0] ?? null); - } catch { - return undefined; - } -}; - -const queryRlsModuleByDatabaseId = async (pool: Pool, databaseId: string): Promise => { - const fromSettings = await queryRlsSettingsByDatabaseId(pool, databaseId); - if (fromSettings) return fromSettings; - const result = await pool.query(RLS_MODULE_BY_DATABASE_ID_SQL, [databaseId]); - return toRlsModule(result.rows[0] ?? null); -}; - -const queryRlsModuleByApiId = async (pool: Pool, apiId: string): Promise => { - const result = await pool.query(RLS_MODULE_BY_API_ID_SQL, [apiId]); - return toRlsModule(result.rows[0] ?? null); -}; - -const queryRlsModuleByDbname = async (pool: Pool, dbname: string): Promise => { - const fromSettings = await queryRlsSettingsByDbname(pool, dbname); - if (fromSettings) return fromSettings; - const result = await pool.query(RLS_MODULE_BY_DBNAME_SQL, [dbname]); - return toRlsModule(result.rows[0] ?? null); -}; - -const resolveUploadRlsModule = async (opts: PgpmOptions, req: Request): Promise => { - const api = req.api; - if (!api) return undefined; - - // Use API-scoped RLS module when available (e.g., meta API). - if (api.rlsModule) { - return api.rlsModule; - } - - const pool = getPgPool(opts.pg); - if (api.apiId) { - const byApiId = await queryRlsModuleByApiId(pool, api.apiId); - if (byApiId) return byApiId; - } - - if (api.databaseId) { - const byDatabaseId = await queryRlsModuleByDatabaseId(pool, api.databaseId); - if (byDatabaseId) return byDatabaseId; - } - - if (api.dbname) { - return queryRlsModuleByDbname(pool, api.dbname); - } - - return undefined; -}; - -const authError = (res: Response): Response => - res.status(401).json({ error: 'Authentication required' }); - -/** - * Upload-specific authentication middleware. - * - * This middleware enforces strict auth semantics for `POST /upload` while - * preserving existing GraphQL auth behavior for other routes. - */ -export const createUploadAuthenticateMiddleware = ( - opts: PgpmOptions, -): RequestHandler => { - return async (req: Request, res: Response, next: NextFunction): Promise => { - const api = req.api; - if (!api) { - res.status(500).send('Missing API info'); - return; - } - - if (req.token?.user_id) { - next(); - return; - } - - const authToken = getBearerToken(req.headers.authorization as string | undefined); - if (!authToken) { - authError(res); - return; - } - - let rlsModule: RlsModule | undefined; - try { - rlsModule = await resolveUploadRlsModule(opts, req); - } catch (error) { - authLog.error('[upload-auth] Failed to resolve RLS module for upload route', error); - authError(res); - return; - } - - if (!rlsModule) { - authLog.info(`[upload-auth] No RLS module found for db=${api.dbname} databaseId=${api.databaseId ?? 'none'}`); - authError(res); - return; - } - - const authFn = opts.server?.strictAuth ? rlsModule.authenticateStrict : rlsModule.authenticate; - const privateSchema = rlsModule.privateSchema?.schemaName; - if (!authFn || !privateSchema) { - authLog.warn( - `[upload-auth] Missing auth function or private schema for db=${api.dbname}; strictAuth=${opts.server?.strictAuth ?? false}`, - ); - authError(res); - return; - } - - const pool = getPgPool({ - ...opts.pg, - database: api.dbname, - }); - - const context: Record = {}; - if (req.clientIp) { - context['jwt.claims.ip_address'] = req.clientIp; - } - if (req.get('origin')) { - context['jwt.claims.origin'] = req.get('origin') as string; - } - if (req.get('User-Agent')) { - context['jwt.claims.user_agent'] = req.get('User-Agent') as string; - } - - try { - const result = await pgQueryContext({ - client: pool, - context, - query: `SELECT * FROM ${QuoteUtils.quoteQualifiedIdentifier(privateSchema, authFn)}($1)`, - variables: [authToken], - }); - - if (!result?.rowCount) { - authError(res); - return; - } - - req.token = result.rows[0]; - if (!req.token?.user_id) { - authError(res); - return; - } - - next(); - } catch (error) { - authLog.warn('[upload-auth] Upload authentication failed', error); - authError(res); - } - }; -}; - -/** - * REST file upload endpoint. - * - * Accepts a single file via multipart/form-data, streams it to S3/MinIO, - * and returns file metadata. The frontend uses this in a two-step flow: - * - * 1. POST /upload -> { url, filename, mime, size } - * 2. GraphQL mutation -> patch row with the returned metadata - */ -export const uploadRoute: RequestHandler[] = [ - parseFileWithErrors, - (async (req, res, next) => { - if (!req.token?.user_id) { - return res.status(401).json({ error: 'Authentication required' }); - } - - if (!req.file) { - return res.status(400).json({ error: 'No file provided. Send a "file" field.' }); - } - - if (req.file.mimetype && BLOCKED_MIME_TYPES.has(req.file.mimetype)) { - fs.unlink(req.file.path, () => {}); - return res.status(415).json({ error: 'File type not allowed' }); - } - - try { - const readStream = fs.createReadStream(req.file.path); - const result = await streamToStorage(readStream, req.file.originalname); - - uploadLog.debug( - `[upload] Uploaded file for user=${req.token.user_id} filename=${req.file.originalname} mime=${result.mime} size=${req.file.size}`, - ); - - res.json({ - url: result.url, - filename: result.filename, - mime: result.mime, - size: req.file.size, - }); - } catch (error) { - uploadLog.error('[upload] Upload processing failed', error); - if (!res.headersSent) { - res.status(500).json({ error: 'Upload processing failed' }); - } - } finally { - if (req.file?.path) { - fs.unlink(req.file.path, () => {}); - } - } - }) as RequestHandler, -]; diff --git a/graphql/server/src/server.ts b/graphql/server/src/server.ts index e2be61a07..efb64c030 100644 --- a/graphql/server/src/server.ts +++ b/graphql/server/src/server.ts @@ -37,7 +37,6 @@ import { createRequestLogger } from './middleware/observability/request-logger'; // Auth cookie handling is done via AuthCookiePlugin in grafserv import { createCaptchaMiddleware } from './middleware/captcha'; import { parseCookieValue, SESSION_COOKIE_NAME } from './middleware/cookie'; -import { createUploadAuthenticateMiddleware, uploadRoute } from './middleware/upload'; import { createLlmApiRouter } from './middleware/llm-api'; import { createContextMiddleware, requestIdMiddleware } from '@constructive-io/express-context'; import { startDebugSampler } from './diagnostics/debug-sampler'; @@ -91,7 +90,6 @@ class Server { const app = express(); const api = createApiMiddleware(effectiveOpts); const authenticate = createAuthenticateMiddleware(effectiveOpts); - const uploadAuthenticate = createUploadAuthenticateMiddleware(effectiveOpts); const requestLogger = createRequestLogger({ observabilityEnabled }); // Log startup configuration (non-sensitive values only) @@ -165,7 +163,6 @@ class Server { app.use(requestIdMiddleware()); app.use(requestLogger); app.use(api); - app.post('/upload', uploadAuthenticate, ...uploadRoute); app.use(authenticate); app.use(createContextMiddleware({ pg: effectiveOpts.pg })); app.use(createCaptchaMiddleware()); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index cc0b4dd2f..64aff37b6 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1528,9 +1528,6 @@ importers: '@constructive-io/s3-utils': specifier: workspace:^ version: link:../../uploads/s3-utils/dist - '@constructive-io/upload-names': - specifier: workspace:^ - version: link:../../uploads/upload-names/dist '@constructive-io/url-domains': specifier: workspace:^ version: link:../../packages/url-domains/dist @@ -1549,9 +1546,6 @@ importers: '@pgpmjs/types': specifier: workspace:^ version: link:../../pgpm/types/dist - '@pgsql/quotes': - specifier: ^17.1.0 - version: 17.1.0 cors: specifier: ^2.8.6 version: 2.8.6 @@ -1600,9 +1594,6 @@ importers: lru-cache: specifier: ^11.2.7 version: 11.2.7 - multer: - specifier: ^2.1.1 - version: 2.1.1 pg: specifier: ^8.21.0 version: 8.21.0 @@ -1640,9 +1631,6 @@ importers: '@types/graphql-upload': specifier: ^8.0.12 version: 8.0.12 - '@types/multer': - specifier: ^2.1.0 - version: 2.1.0 '@types/pg': specifier: ^8.20.0 version: 8.20.0 @@ -7151,12 +7139,6 @@ packages: integrity: sha512-hov8bUuiLiyFPGyFPE1lwWhmzYbirOXQNNo40+y3zow8aFVTeyn3VWL0VFFfdNddA8S4Vf0Tc062rzyNr7Paag==, } - '@types/multer@2.1.0': - resolution: - { - integrity: sha512-zYZb0+nJhOHtPpGDb3vqPjwpdeGlGC157VpkqNQL+UU2qwoacoQ7MpsAmUptI/0Oa127X32JzWDqQVEXp2RcIA==, - } - '@types/node@22.19.11': resolution: { @@ -7759,12 +7741,6 @@ packages: } engines: { node: '>= 8' } - append-field@1.0.0: - resolution: - { - integrity: sha512-klpgFSWLW1ZEs8svjfb7g4qWY0YS5imI82dTg+QahUvJ8YqAY0P10Uk8tTyh9ZGuYEZEMaeJYCF5BFuX552hsw==, - } - appstash@0.7.0: resolution: { @@ -8116,13 +8092,6 @@ packages: } engines: { node: '>=4.5.0' } - busboy@1.6.0: - resolution: - { - integrity: sha512-8SFQbg/0hQ9xy3UNTB0YEnsNBbWfhf7RtnzpL7TkBiTBRfrQ9Fxcnz7VJsleJpyp6rVLvXiuORqjlHi5q+PYuA==, - } - engines: { node: '>=10.16.0' } - byte-size@8.1.1: resolution: { @@ -11668,13 +11637,6 @@ packages: integrity: sha512-Lf+9+2r+Tdp5wXDXC4PcIBjTDtq4UKjCPMQhKIuzpJNW0b96kVqSwW0bT7FhRSfmAiFYgP+SCRvdrDozfh0U5w==, } - media-typer@0.3.0: - resolution: - { - integrity: sha512-dq+qelQ9akHpcOl/gUVRTxVIOkAJ1wR3QAvb4RsVjS8oVoFjDGTc679wJYmUmknUF5HwMLOgb5O+a3KxfWapPQ==, - } - engines: { node: '>= 0.6' } - media-typer@1.1.0: resolution: { @@ -12208,13 +12170,6 @@ packages: integrity: sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==, } - multer@2.1.1: - resolution: - { - integrity: sha512-mo+QTzKlx8R7E5ylSXxWzGoXoZbOsRMpyitcht8By2KHvMbf3tjwosZ/Mu/XYU6UuJ3VZnODIrak5ZrPiPyB6A==, - } - engines: { node: '>= 10.16.0' } - multimatch@5.0.0: resolution: { @@ -14174,13 +14129,6 @@ packages: } engines: { node: '>=0.8.0' } - streamsearch@1.1.0: - resolution: - { - integrity: sha512-Mcc5wHehp9aXz1ax6bZUyY5afg9u2rv5cqQI3mRrYkGC8rW2hM02jWuwjtL++LS5qinSyhj2QfLyNsuc+VsExg==, - } - engines: { node: '>=10.0.0' } - strfy-js@3.2.2: resolution: { @@ -14638,13 +14586,6 @@ packages: } engines: { node: '>=16' } - type-is@1.6.18: - resolution: - { - integrity: sha512-TkRKr9sUTxEH8MdfuCSP7VizJyzRNMjj2J2do2Jr3Kym598JVdEksuzPQCnlFPW4ky9Q+iA+ma9BGm06XQBy8g==, - } - engines: { node: '>= 0.6' } - type-is@2.0.1: resolution: { @@ -18108,10 +18049,6 @@ snapshots: '@types/minimist@1.2.5': {} - '@types/multer@2.1.0': - dependencies: - '@types/express': 5.0.6 - '@types/node@22.19.11': dependencies: undici-types: 6.21.0 @@ -18472,8 +18409,6 @@ snapshots: normalize-path: 3.0.0 picomatch: 2.3.1 - append-field@1.0.0: {} - appstash@0.7.0: {} aproba@2.0.0: {} @@ -18711,10 +18646,6 @@ snapshots: dependencies: dicer: 0.3.0 - busboy@1.6.0: - dependencies: - streamsearch: 1.1.0 - byte-size@8.1.1: {} bytes@3.1.2: {} @@ -21347,8 +21278,6 @@ snapshots: mdurl@2.0.0: {} - media-typer@0.3.0: {} - media-typer@1.1.0: {} mensch@0.3.4: {} @@ -21822,13 +21751,6 @@ snapshots: ms@2.1.3: {} - multer@2.1.1: - dependencies: - append-field: 1.0.0 - busboy: 1.6.0 - concat-stream: 2.0.0 - type-is: 1.6.18 - multimatch@5.0.0: dependencies: '@types/minimatch': 3.0.5 @@ -23293,8 +23215,6 @@ snapshots: streamsearch@0.1.2: {} - streamsearch@1.1.0: {} - strfy-js@3.2.2: dependencies: minimatch: 10.2.5 @@ -23601,11 +23521,6 @@ snapshots: type-fest@4.41.0: {} - type-is@1.6.18: - dependencies: - media-typer: 0.3.0 - mime-types: 2.1.35 - type-is@2.0.1: dependencies: content-type: 1.0.5