diff --git a/graphile/graphile-presigned-url-plugin/src/download-url-field.ts b/graphile/graphile-presigned-url-plugin/src/download-url-field.ts index 41d0fd9c70..4210c8d3ba 100644 --- a/graphile/graphile-presigned-url-plugin/src/download-url-field.ts +++ b/graphile/graphile-presigned-url-plugin/src/download-url-field.ts @@ -27,6 +27,7 @@ import type { GraphileConfig } from 'graphile-config'; import { withRequestPgClient } from 'graphile-plugin-utils'; import { DOWNLOAD_URL_FIELD } from 'graphile-storage-registry'; +import { isLiveFileRow } from './file-lifecycle'; import { resolveS3ForDatabase } from './physical-bucket'; import { generatePresignedGetUrl } from './s3-signer'; import { loadAllStorageModules, readBucketPhysicalName, resolveStorageConfigFromCodec } from './storage-module-cache'; @@ -86,13 +87,15 @@ export function createDownloadUrlPlugin( { description: 'URL to download this file. For public files, returns the public URL. ' + - 'For private files, returns a time-limited presigned URL.', + 'For private files, returns a time-limited presigned URL. ' + + 'Null until the upload is confirmed (status uploaded or processed), and for a rejected file.', type: GraphQLString, plan($parent: any) { const $key = $parent.get('key'); const $isPublic = $parent.get('is_public'); const $filename = $parent.get('filename'); const $bucketId = $parent.get('bucket_id'); + const $status = $parent.get('status'); const $withPgClient = (grafastContext() as any).get('withPgClient'); const $pgSettings = (grafastContext() as any).get('pgSettings'); @@ -102,12 +105,14 @@ export function createDownloadUrlPlugin( isPublic: $isPublic, filename: $filename, bucketId: $bucketId, + status: $status, withPgClient: $withPgClient, pgSettings: $pgSettings, }); - return lambda($combined, async ({ key, isPublic, filename, bucketId, withPgClient, pgSettings }: any) => { + return lambda($combined, async ({ key, isPublic, filename, bucketId, status, withPgClient, pgSettings }: any) => { if (!key) return null; + if (!isLiveFileRow({ status })) return null; if (!withPgClient || !pgSettings) return null; diff --git a/graphile/graphile-presigned-url-plugin/src/file-lifecycle.ts b/graphile/graphile-presigned-url-plugin/src/file-lifecycle.ts index 33f11dd392..5bd6174b88 100644 --- a/graphile/graphile-presigned-url-plugin/src/file-lifecycle.ts +++ b/graphile/graphile-presigned-url-plugin/src/file-lifecycle.ts @@ -6,8 +6,9 @@ export const LIVE_FILE_STATUSES = ['uploaded', 'processed']; /** - * Whether an existing row may be handed back as a dedup hit: only once the bytes - * it names are in S3. + * Whether a row is live (constructive-db file lifecycle invariant S2): a dedup + * hit, and a row `downloadUrl` signs a URL for, only once the bytes it names + * are in S3. * * The row is written before the PUT; the confirm-upload job moves it out of * `requested` only after inspecting the object, so its status is the answer. diff --git a/graphql/server-test/__fixtures__/seed/simple-seed-storage/schema.sql b/graphql/server-test/__fixtures__/seed/simple-seed-storage/schema.sql index 7fd0c4b92e..2de058ddd6 100644 --- a/graphql/server-test/__fixtures__/seed/simple-seed-storage/schema.sql +++ b/graphql/server-test/__fixtures__/seed/simple-seed-storage/schema.sql @@ -118,6 +118,21 @@ BEGIN EXECUTE format( 'GRANT EXECUTE ON FUNCTION %I.app_files_record_file(uuid, text, text, text, bigint, text, jsonb, text, uuid) TO administrator, authenticated, anonymous', schema_name || '-private'); + + -- The generator's bucket physical-name reader, which downloadUrl calls. + EXECUTE format($sql$ + CREATE FUNCTION %I.app_buckets_physical_name(bucket_id uuid) RETURNS text + LANGUAGE sql + SECURITY DEFINER + STABLE + AS $function$ + SELECT b.physical_name FROM %I.app_buckets b WHERE b.id = app_buckets_physical_name.bucket_id + $function$; + $sql$, schema_name || '-private', schema_name); + + EXECUTE format( + 'GRANT EXECUTE ON FUNCTION %I.app_buckets_physical_name(uuid) TO administrator, authenticated, anonymous', + schema_name || '-private'); END; $$; diff --git a/graphql/server-test/__tests__/download-url-status.integration.test.ts b/graphql/server-test/__tests__/download-url-status.integration.test.ts new file mode 100644 index 0000000000..9f75077cd3 --- /dev/null +++ b/graphql/server-test/__tests__/download-url-status.integration.test.ts @@ -0,0 +1,118 @@ +/** + * Integration tests: `downloadUrl` is handed out only for a file whose bytes a + * reader can GET. + * + * A files row is written before its presigned PUT runs, and the confirm-upload + * job settles its `status` after inspecting the object. A `requested` row may + * name an object that never arrives, and a `rejected` row is a settled + * failure, so neither may carry a download URL. + * + * pnpm test -- --testPathPattern=download-url-status + */ + +import path from 'path'; +import type { PgTestClient } from 'pgsql-test'; +import type supertest from 'supertest'; + +import { getConnections, seed } from '../src'; + +jest.setTimeout(120000); + +const localSeedRoot = path.join(__dirname, '..', '__fixtures__', 'seed'); +const sharedSeedRoot = path.join(__dirname, '..', '..', '..', '__fixtures__', 'seed'); +const sql = (seedDir: string, file: string) => path.join(localSeedRoot, seedDir, file); +const pgpmWorkspace = path.join(sharedSeedRoot, '..', '..'); + +const aliceDatabaseId = '80a2eaaf-f77e-4bfe-8506-df929ef1b8d9'; +const aliceSchema = 'simple-storage-public'; +const alicePrivateBucketId = 'd0000001-0000-0000-0000-000000000002'; + +const metaSchemas = [ + 'catalog_private', + 'routing_public', + 'apps_public', + 'metaschema_public', + 'metaschema_modules_public' +]; + +const STATUSES = ['requested', 'uploaded', 'processed', 'rejected']; + +const APP_FILES = ` + query AppFiles { + appFiles { + nodes { + key + downloadUrl + } + } + } +`; + +describe('downloadUrl by file status', () => { + let request: supertest.Agent; + let pg: PgTestClient; + let teardown: () => Promise; + let urlByKey: Record; + + beforeAll(async () => { + ({ request, pg, teardown } = await getConnections( + { + schemas: [aliceSchema], + authRole: 'anonymous', + server: { + useRouting: true, + api: { + isPublic: false, + metaSchemas + } + } + }, + [ + seed.pgpm(pgpmWorkspace), + seed.sqlfile([ + sql('simple-seed-storage', 'schema.sql'), + sql('simple-seed-storage', 'test-data.sql'), + sql('simple-seed-storage', 'auth-provision.sql') + ]) + ] + )); + + for (const status of STATUSES) { + await pg.query( + `INSERT INTO "${aliceSchema}".app_files + (bucket_id, key, content_hash, mime_type, filename, status) + VALUES ($1, $2, $2, 'text/plain', $3, $4)`, + [alicePrivateBucketId, `status-${status}`, `${status}.txt`, status] + ); + } + + const res = await request + .post('/graphql') + .set('X-Database-Id', aliceDatabaseId) + .set('X-Schemata', aliceSchema) + .send({ query: APP_FILES }); + expect(res.status).toBe(200); + expect(res.body.errors).toBeUndefined(); + urlByKey = Object.fromEntries( + res.body.data.appFiles.nodes.map((n: { key: string; downloadUrl: string | null }) => [ + n.key, + n.downloadUrl + ]) + ); + }); + + afterAll(async () => { + if (teardown) await teardown(); + }); + + it.each(['uploaded', 'processed'])('signs a download URL for a %s file', (status) => { + const url = urlByKey[`status-${status}`]; + expect(url).toEqual(expect.any(String)); + expect(new URL(url).pathname).toContain(`/status-${status}`); + }); + + it.each(['requested', 'rejected'])('returns no download URL for a %s file', (status) => { + expect(urlByKey).toHaveProperty(`status-${status}`); + expect(urlByKey[`status-${status}`]).toBeNull(); + }); +});