feat(intake): issue named upload links and accept photo submissions (#222) #250
@@ -10,6 +10,7 @@ import adminEmailTemplatesRouter from './routes/adminEmailTemplates';
|
||||
import adminCategoriesRouter from './routes/adminCategories';
|
||||
import adminTagsRouter from './routes/adminTags';
|
||||
import adminUploadLinksRouter from './routes/adminUploadLinks';
|
||||
import intakeRouter from './routes/intake';
|
||||
import adminVersionRouter from './routes/adminVersion';
|
||||
import filtersRouter from './routes/filters';
|
||||
import customersRouter from './routes/customers';
|
||||
@@ -63,6 +64,9 @@ app.get('/api/config', (_req, res) => {
|
||||
app.use('/api/items', itemsRouter);
|
||||
app.use('/api/filters', filtersRouter);
|
||||
app.use('/api/cart', cartRouter);
|
||||
// Public and unauthenticated by design (#222). No requireAdminGate: the token
|
||||
// in the path is the whole access control, and every refusal is a 404.
|
||||
app.use('/api/intake', intakeRouter);
|
||||
app.use('/api/checkout/cart', cartCheckoutRouter);
|
||||
// requireAdminGate is attached to each admin router rather than to a path
|
||||
// prefix. Attached to the router, an admin router added later at some other
|
||||
|
||||
@@ -124,3 +124,35 @@ export const verificationResendLimiter = rateLimit({
|
||||
error: 'we have already sent several verification emails recently. Check your spam folder, and try again later.'
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* Keyed on the caller alone, because an intake submission carries no email.
|
||||
*
|
||||
* The comment on `keyByCallerAndEmail` warns that a bare `ip:` bucket is a
|
||||
* shared allowance rather than a per-caller one, and that trade is accepted
|
||||
* here deliberately: the *link* is the per-caller identity, and its
|
||||
* `submission_count` against `max_submissions` is the per-caller cap. This
|
||||
* limiter exists for a different job — bounding what one address can throw at
|
||||
* an unauthenticated endpoint that writes files to disk.
|
||||
*
|
||||
* ipKeyGenerator rather than `req.ip` raw, for the reason #84 records: a
|
||||
* residential IPv6 customer is delegated a whole prefix and can source every
|
||||
* request from a different address inside it for free, so keying on the exact
|
||||
* address counts each one as a new caller and never bounds anything.
|
||||
*/
|
||||
export function keyByCaller(req: Request): string {
|
||||
return ipKeyGenerator(req.ip ?? '');
|
||||
}
|
||||
|
||||
// Deliberately looser than the password-reset allowance. Somebody photographing
|
||||
// a box of stock legitimately submits several items in a row, and the cost of
|
||||
// refusing them is a lost consignment — whereas the cost of allowing a few too
|
||||
// many is some disk the volume guard and the per-link cap already bound.
|
||||
export const intakeLimiter = rateLimit({
|
||||
windowMs: 15 * 60 * 1000,
|
||||
limit: 20,
|
||||
keyGenerator: keyByCaller,
|
||||
standardHeaders: 'draft-7',
|
||||
legacyHeaders: false,
|
||||
message: { error: 'too many submissions — please try again later' }
|
||||
});
|
||||
|
||||
@@ -0,0 +1,164 @@
|
||||
import { Router, Request, Response, NextFunction } from 'express';
|
||||
import { pool, requireRow } from '../db';
|
||||
import { asyncRoute } from '../asyncRoute';
|
||||
import { hashToken } from '../uploadLinks';
|
||||
import { uploadImages, verifyUploadedImages, insertItemImages } from '../imageUpload';
|
||||
import { intakeLimiter } from '../rateLimit';
|
||||
|
||||
const router = Router();
|
||||
|
||||
/**
|
||||
* The public way in: photos of one item, from someone with no account (#222).
|
||||
*
|
||||
* Everything here is reachable by a stranger holding a URL, so the shape of
|
||||
* every refusal matters. Unknown, revoked and exhausted links are all 404 and
|
||||
* indistinguishable from outside — whether a link exists is not something a
|
||||
* stranger needs to be able to learn, which is the same reasoning `uploads.ts`
|
||||
* applies to files.
|
||||
*
|
||||
* The AI is deliberately not called here. A slow or failing model request must
|
||||
* not turn into a failed upload for someone who did nothing wrong, and the
|
||||
* photos may be the only copy — the item is often no longer in the sender's
|
||||
* hands. The row is left at `state='queued'` for the worker in #223.
|
||||
*/
|
||||
|
||||
interface LinkRow {
|
||||
id: number;
|
||||
label: string;
|
||||
}
|
||||
|
||||
/** The link resolved by `requireUsableLink`, carried through to the handler. */
|
||||
interface IntakeRequest extends Request {
|
||||
uploadLink?: LinkRow;
|
||||
}
|
||||
|
||||
/**
|
||||
* The link a token opens, or null.
|
||||
*
|
||||
* The cap is applied in SQL rather than in a later branch, so that "usable" is
|
||||
* one concept with one definition used identically by the GET and the POST.
|
||||
*/
|
||||
async function usableLink(token: string): Promise<LinkRow | null> {
|
||||
const { rows } = await pool.query<LinkRow>(
|
||||
`SELECT id, label FROM upload_links
|
||||
WHERE token_hash = $1
|
||||
AND revoked_at IS NULL
|
||||
AND (max_submissions IS NULL OR submission_count < max_submissions)`,
|
||||
[hashToken(token)]
|
||||
);
|
||||
return rows[0] ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves the link *before* multer runs, so a stranger holding a bad token
|
||||
* cannot cause a single byte to be written to the uploads volume.
|
||||
*
|
||||
* `discardUnlessAccepted` would delete those files afterwards, but "written
|
||||
* then deleted" is a materially worse position than "never written" on an
|
||||
* endpoint the whole internet can reach: it is disk churn an unauthenticated
|
||||
* caller controls, and it leans on a cleanup that a crash between the write
|
||||
* and the unlink would skip. Ordering this ahead of `uploadImages` is the
|
||||
* whole mitigation, and a test asserts it.
|
||||
*/
|
||||
const requireUsableLink = asyncRoute(
|
||||
async (req: Request, res: Response, next: NextFunction) => {
|
||||
const link = await usableLink(req.params.token as string);
|
||||
if (!link) {
|
||||
res.status(404).json({ error: 'not found' });
|
||||
return;
|
||||
}
|
||||
(req as IntakeRequest).uploadLink = link;
|
||||
next();
|
||||
}
|
||||
);
|
||||
|
||||
router.get('/:token', intakeLimiter, asyncRoute(async (req: Request, res: Response) => {
|
||||
const link = await usableLink(req.params.token as string);
|
||||
if (!link) {
|
||||
return res.status(404).json({ error: 'not found' });
|
||||
}
|
||||
// The label only. Nothing about the catalogue, the admin, or other links.
|
||||
res.json({ label: link.label });
|
||||
}));
|
||||
|
||||
router.post(
|
||||
'/:token',
|
||||
intakeLimiter,
|
||||
requireUsableLink,
|
||||
uploadImages,
|
||||
asyncRoute(async (req: Request, res: Response) => {
|
||||
// Set by requireUsableLink above. Re-checked rather than asserted non-null,
|
||||
// so a future reordering of the middleware fails as a 404 rather than as a
|
||||
// crash on undefined.
|
||||
const link = (req as IntakeRequest).uploadLink;
|
||||
if (!link) {
|
||||
return res.status(404).json({ error: 'not found' });
|
||||
}
|
||||
|
||||
const files = (req.files as Express.Multer.File[]) || [];
|
||||
if (files.length === 0) {
|
||||
return res.status(400).json({ error: 'at least one photo is required' });
|
||||
}
|
||||
|
||||
const refusal = await verifyUploadedImages(req);
|
||||
if (refusal) {
|
||||
return res.status(400).json({ error: refusal });
|
||||
}
|
||||
|
||||
const note = typeof req.body?.note === 'string' ? req.body.note.trim() : '';
|
||||
|
||||
const client = await pool.connect();
|
||||
try {
|
||||
await client.query('BEGIN');
|
||||
|
||||
// A placeholder name. `items.name` is NOT NULL and nobody has named this
|
||||
// yet — the drafting worker or the admin replaces it. A timestamp rather
|
||||
// than "Untitled" so several waiting submissions stay tellable apart in
|
||||
// the inventory list.
|
||||
const { rows } = await client.query<{ id: number }>(
|
||||
`INSERT INTO items (name, description, status)
|
||||
VALUES ($1, $2, 'pending')
|
||||
RETURNING id`,
|
||||
[`Submission ${new Date().toISOString()}`, null]
|
||||
);
|
||||
const itemId = requireRow(rows, 'the intake item INSERT').id;
|
||||
|
||||
await insertItemImages(client, itemId, files, 0);
|
||||
|
||||
await client.query(
|
||||
`INSERT INTO item_drafts (item_id, upload_link_id, submitter_note)
|
||||
VALUES ($1, $2, $3)`,
|
||||
[itemId, link.id, note === '' ? null : note]
|
||||
);
|
||||
|
||||
// Counted inside the transaction and guarded on the same conditions as
|
||||
// the lookup, so two submissions racing for the last slot of a capped
|
||||
// link cannot both succeed.
|
||||
const counted = await client.query(
|
||||
`UPDATE upload_links
|
||||
SET submission_count = submission_count + 1, last_used_at = now()
|
||||
WHERE id = $1
|
||||
AND revoked_at IS NULL
|
||||
AND (max_submissions IS NULL OR submission_count < max_submissions)`,
|
||||
[link.id]
|
||||
);
|
||||
if (counted.rowCount === 0) {
|
||||
await client.query('ROLLBACK');
|
||||
return res.status(404).json({ error: 'not found' });
|
||||
}
|
||||
|
||||
await client.query('COMMIT');
|
||||
// No item id in the response: the sender has no business knowing about
|
||||
// the catalogue, and nothing they could do with it.
|
||||
res.status(201).json({ ok: true });
|
||||
} catch (err) {
|
||||
await client.query('ROLLBACK');
|
||||
console.error(err);
|
||||
res.status(500).json({ error: 'internal error' });
|
||||
} finally {
|
||||
client.release();
|
||||
}
|
||||
})
|
||||
);
|
||||
|
||||
export default router;
|
||||
@@ -0,0 +1,209 @@
|
||||
import request from 'supertest';
|
||||
import { promises as fs } from 'fs';
|
||||
import app from '../../src/app';
|
||||
import { pool } from '../../src/db';
|
||||
import { resetDb, closeDb } from './setup/testDb';
|
||||
|
||||
const UPLOADS_DIR = process.env.UPLOADS_DIR as string;
|
||||
|
||||
// The same 1x1 PNG the upload validation suite uses, so the accepted case
|
||||
// exercises the whole path rather than a buffer that merely starts right.
|
||||
const PNG = Buffer.from(
|
||||
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==',
|
||||
'base64'
|
||||
);
|
||||
|
||||
// multer.diskStorage does not create its destination.
|
||||
beforeAll(async () => {
|
||||
await fs.mkdir(UPLOADS_DIR, { recursive: true });
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
await resetDb();
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await pool.end();
|
||||
await closeDb();
|
||||
});
|
||||
|
||||
async function storedFiles(): Promise<string[]> {
|
||||
return fs.readdir(UPLOADS_DIR);
|
||||
}
|
||||
|
||||
async function issueLink(label = 'Sarah', maxSubmissions?: number | null): Promise<string> {
|
||||
const res = await request(app)
|
||||
.post('/api/admin/upload-links')
|
||||
.send({ label, ...(maxSubmissions === undefined ? {} : { maxSubmissions }) });
|
||||
expect(res.status).toBe(201);
|
||||
return res.body.token as string;
|
||||
}
|
||||
|
||||
describe('checking a link before showing the form', () => {
|
||||
it('names the link so the page can greet the sender', async () => {
|
||||
const token = await issueLink('Sarah');
|
||||
const res = await request(app).get(`/api/intake/${token}`);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.label).toBe('Sarah');
|
||||
});
|
||||
|
||||
// 404 rather than 403 throughout: whether a link exists is not something a
|
||||
// stranger needs to be able to distinguish. Same reasoning as uploads.ts.
|
||||
it('404s an unknown token', async () => {
|
||||
const res = await request(app).get('/api/intake/not-a-real-token');
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
|
||||
it('404s a revoked link', async () => {
|
||||
const token = await issueLink();
|
||||
const { rows } = await pool.query<{ id: number }>(`SELECT id FROM upload_links`);
|
||||
await request(app).post(`/api/admin/upload-links/${rows[0]?.id}/revoke`);
|
||||
|
||||
const res = await request(app).get(`/api/intake/${token}`);
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
});
|
||||
|
||||
describe('submitting an item', () => {
|
||||
it('creates a pending item with its images, note and provenance', async () => {
|
||||
const token = await issueLink('Sarah');
|
||||
|
||||
const res = await request(app)
|
||||
.post(`/api/intake/${token}`)
|
||||
.field('note', 'Hand-thrown stoneware, chip on the base')
|
||||
.attach('images', PNG, 'front.png')
|
||||
.attach('images', PNG, 'back.png');
|
||||
|
||||
expect(res.status).toBe(201);
|
||||
expect(res.body.ok).toBe(true);
|
||||
|
||||
const { rows: items } = await pool.query<{ id: number; status: string; price_cents: number }>(
|
||||
`SELECT id, status, price_cents FROM items`
|
||||
);
|
||||
expect(items).toHaveLength(1);
|
||||
expect(items[0]?.status).toBe('pending');
|
||||
// The migration's default, not a price anyone chose.
|
||||
expect(items[0]?.price_cents).toBe(8000);
|
||||
|
||||
const { rows: images } = await pool.query<{ image_path: string }>(
|
||||
`SELECT image_path FROM item_images WHERE item_id = $1 ORDER BY sort_order`,
|
||||
[items[0]?.id]
|
||||
);
|
||||
expect(images).toHaveLength(2);
|
||||
expect(images[0]?.image_path).toMatch(/^\/uploads\/[a-f0-9-]+\.png$/);
|
||||
|
||||
const { rows: drafts } = await pool.query(
|
||||
`SELECT submitter_note, state, price_source, upload_link_id
|
||||
FROM item_drafts WHERE item_id = $1`,
|
||||
[items[0]?.id]
|
||||
);
|
||||
expect(drafts[0]?.submitter_note).toBe('Hand-thrown stoneware, chip on the base');
|
||||
expect(drafts[0]?.state).toBe('queued');
|
||||
expect(drafts[0]?.price_source).toBe('default');
|
||||
expect(drafts[0]?.upload_link_id).not.toBeNull();
|
||||
});
|
||||
|
||||
it('counts the submission against the link', async () => {
|
||||
const token = await issueLink();
|
||||
await request(app).post(`/api/intake/${token}`).attach('images', PNG, 'a.png');
|
||||
|
||||
const { rows } = await pool.query<{ submission_count: number; last_used_at: string | null }>(
|
||||
`SELECT submission_count, last_used_at FROM upload_links`
|
||||
);
|
||||
expect(rows[0]?.submission_count).toBe(1);
|
||||
expect(rows[0]?.last_used_at).not.toBeNull();
|
||||
});
|
||||
|
||||
it('refuses a submission with no photos', async () => {
|
||||
const token = await issueLink();
|
||||
const res = await request(app).post(`/api/intake/${token}`).field('note', 'nothing attached');
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
const { rows } = await pool.query(`SELECT id FROM items`);
|
||||
expect(rows).toHaveLength(0);
|
||||
});
|
||||
|
||||
// The file is named .png and declared image/png, but the bytes are not.
|
||||
// This is the check that cannot happen before the write.
|
||||
it('refuses a file whose bytes disagree with its type', async () => {
|
||||
const token = await issueLink();
|
||||
const res = await request(app)
|
||||
.post(`/api/intake/${token}`)
|
||||
.attach('images', Buffer.from('<html>not an image</html>'), {
|
||||
filename: 'evil.png',
|
||||
contentType: 'image/png'
|
||||
});
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
const { rows } = await pool.query(`SELECT id FROM items`);
|
||||
expect(rows).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('404s a revoked link without creating anything', async () => {
|
||||
const token = await issueLink();
|
||||
const { rows: links } = await pool.query<{ id: number }>(`SELECT id FROM upload_links`);
|
||||
await request(app).post(`/api/admin/upload-links/${links[0]?.id}/revoke`);
|
||||
|
||||
const res = await request(app).post(`/api/intake/${token}`).attach('images', PNG, 'a.png');
|
||||
|
||||
expect(res.status).toBe(404);
|
||||
const { rows } = await pool.query(`SELECT id FROM items`);
|
||||
expect(rows).toHaveLength(0);
|
||||
});
|
||||
|
||||
// The reason requireUsableLink is ordered ahead of uploadImages. Without that
|
||||
// ordering this still returns 404 and still creates no item — the bytes just
|
||||
// reach the disk first and are deleted afterwards. This asserts they never
|
||||
// arrive, so a future reordering fails here rather than quietly handing an
|
||||
// unauthenticated caller control of disk churn.
|
||||
it('writes nothing to the uploads volume for a token that does not work', async () => {
|
||||
const before = await storedFiles();
|
||||
|
||||
const res = await request(app)
|
||||
.post('/api/intake/not-a-real-token')
|
||||
.attach('images', PNG, 'a.png');
|
||||
|
||||
expect(res.status).toBe(404);
|
||||
expect(await storedFiles()).toEqual(before);
|
||||
});
|
||||
|
||||
it('stops accepting once the link hits its cap', async () => {
|
||||
const token = await issueLink('One shot', 1);
|
||||
|
||||
const first = await request(app).post(`/api/intake/${token}`).attach('images', PNG, 'a.png');
|
||||
expect(first.status).toBe(201);
|
||||
|
||||
const second = await request(app).post(`/api/intake/${token}`).attach('images', PNG, 'b.png');
|
||||
expect(second.status).toBe(404);
|
||||
|
||||
const { rows } = await pool.query(`SELECT id FROM items`);
|
||||
expect(rows).toHaveLength(1);
|
||||
});
|
||||
|
||||
// #226 applies here too, and this route is exactly where it matters most:
|
||||
// the photo comes from a stranger's phone rather than the shop's own camera.
|
||||
it('strips metadata from a submitted photo', async () => {
|
||||
const token = await issueLink();
|
||||
await request(app).post(`/api/intake/${token}`).attach('images', PNG, 'a.png');
|
||||
|
||||
const { rows } = await pool.query<{ image_path: string }>(`SELECT image_path FROM item_images`);
|
||||
const sharp = (await import('sharp')).default;
|
||||
const stored = await sharp(
|
||||
`${UPLOADS_DIR}/${rows[0]?.image_path.replace('/uploads/', '')}`
|
||||
).metadata();
|
||||
|
||||
expect(stored.exif).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('a submitted item does not reach the storefront', () => {
|
||||
it('is absent from the public catalogue', async () => {
|
||||
const token = await issueLink();
|
||||
await request(app).post(`/api/intake/${token}`).attach('images', PNG, 'a.png');
|
||||
|
||||
const res = await request(app).get('/api/items');
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user