4e99897313
* fix(security): enforce event ownership on the v1 API surface (GHSA-9697) Migration 081 documents the intent — 'the token's effective permissions are the intersection of the user's role permissions and the token's own scope flags' — but it was never implemented. - apiTokenAuth selected only id/username/email/role_id, so req.admin.roleName was undefined. Every ownership helper keys on roleName, so the v1 surface could not tell a super_admin from a demoted viewer. Now joins roles and emits the same req.admin shape adminAuth does, including the roles-table-missing upgrade fallback. - No v1 route applied any ownership predicate: GET /events listed every event on the instance, and GET /events/:id/share-link returned ANY event's share_token — the gallery access credential, same class as GHSA-rh8r. List is now scoped via a new scopeEventsQuery helper; the three :id routes (detail, photo upload, share-link) use the existing requireEventOwnership. Not a breaking change: tokens are minted by super_admins, who bypass ownership. It closes the case where a token's owner is later demoted — userManagementService never touches api_tokens, so the token outlived the demotion with full read of every gallery's share token. events.category.test.js stubbed apiTokenAuth without roleName; giving the stub super_admin keeps requireEventOwnership from issuing a DB query and desyncing that suite's sequenced dbMock. * fix(security): codex round 2 — intersect v1 token scopes with role permissions (GHSA-9697) Ownership scoping alone left half the documented control missing. Migration 081 defines a token's effective permissions as the INTERSECTION of the owner's role permissions and the token's scope flags; requireApiScope only ever checked the scope half. A token minted while its owner was super_admin therefore kept write access after the owner was demoted to viewer — userManagementService never touches api_tokens, so the token outlives the demotion, and ownership scoping does not help because the demoted owner still owns their events. Adds requirePermission to all six v1 routes (events.create on create, events.view on the reads, photos.upload on upload). It keys on req.admin.id, which apiTokenAuth already populates. The two existing v1 suites mock the database, so a real permission lookup 500s — they now mock the permissions middleware as pass-through, matching how they already mock apiTokenAuth. Those suites cover route logic; the intersection is pinned by the new v1TokenPermissions suite. * fix(security): codex round 3 — fail closed on the roles-join fallback (GHSA-9697) The round-2 fix loaded the token owner's role so the v1 ownership checks could tell a super_admin from a demoted viewer, and mirrored adminAuth's roles-table-missing fallback. That fallback assigns role_name = 'super_admin', and the catch around it was unconditional — so ANY failure of the joined query (connection reset, deadlock, statement timeout) elevated the token owner to super_admin as long as the simpler fallback query then succeeded. A restricted owner could ride that into listing, reading and share-tokening every event on the instance, which is the exact hole GHSA-9697 closes. The fallback is now reached only for an error that genuinely names a missing roles table/column (PG 42P01/42703 or the SQLite/MySQL wording); anything else propagates to the 500 handler. Claude-Session: https://claude.ai/code/session_01F211U4dDbEj4zXiyKbi9me (cherry picked from commit 53d1e5d1b3148a7f4067308b08fcdf8ddab0a39f) --------- Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
109 lines
3.8 KiB
JavaScript
109 lines
3.8 KiB
JavaScript
/**
|
|
* v1 token scopes must intersect the owner's CURRENT role permissions
|
|
* (GHSA-9697, codex round 2).
|
|
*
|
|
* Migration 081 documents effective permissions as the intersection of the
|
|
* owner's role permissions and the token's scope flags. requireApiScope only
|
|
* ever checked the scope half, so a token minted while its owner was
|
|
* super_admin kept full write access after the owner was demoted to viewer —
|
|
* userManagementService never touches api_tokens, so the token outlives the
|
|
* demotion. Ownership scoping alone does not close this: the demoted owner
|
|
* still *owns* their events.
|
|
*/
|
|
|
|
const path = require('path');
|
|
const fs = require('fs');
|
|
const os = require('os');
|
|
|
|
process.env.NODE_ENV = 'test';
|
|
process.env.TEST_DATABASE_PATH = path.join(
|
|
fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-v1perm-')), 'db.sqlite',
|
|
);
|
|
process.env.JWT_SECRET = process.env.JWT_SECRET || 'v1perm-test-secret';
|
|
process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-v1perm-storage-'));
|
|
|
|
const request = require('supertest');
|
|
const express = require('express');
|
|
const bcrypt = require('bcrypt');
|
|
|
|
const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
|
|
const { generateApiToken } = require('../../src/middleware/apiTokenAuth');
|
|
|
|
describe('v1 token scopes intersect role permissions (GHSA-9697)', () => {
|
|
let db; let cleanup; let app; let viewerToken; let viewerEventId;
|
|
|
|
beforeAll(async () => {
|
|
({ db, cleanup } = await bootCrmDb());
|
|
await seedMinimal(db);
|
|
|
|
const role = await db('roles').where({ name: 'viewer' }).first();
|
|
const r = await db('admin_users').insert({
|
|
username: 'demoted-owner',
|
|
email: 'demoted@example.com',
|
|
password_hash: await bcrypt.hash('Passw0rd!', 4),
|
|
role_id: role.id,
|
|
is_active: 1,
|
|
created_at: new Date(),
|
|
updated_at: new Date(),
|
|
}).returning('id');
|
|
const ownerId = r[0]?.id ?? r[0];
|
|
|
|
// A token still carrying the broad 'admin' scope from before demotion.
|
|
const { plaintext, hashed } = generateApiToken();
|
|
await db('api_tokens').insert({
|
|
name: 'stale-token',
|
|
hashed_token: hashed,
|
|
scopes: 'admin',
|
|
created_by: ownerId,
|
|
created_at: new Date().toISOString(),
|
|
});
|
|
viewerToken = plaintext;
|
|
|
|
const ev = await db('events').insert({
|
|
slug: 'viewer-ev',
|
|
event_type: 'wedding',
|
|
event_name: 'Viewer Event',
|
|
event_date: '2026-08-01',
|
|
host_email: 'h@example.com',
|
|
admin_email: 'a@example.com',
|
|
password_hash: 'x',
|
|
share_token: 'vtok',
|
|
share_link: '/gallery/viewer-ev/vtok',
|
|
created_by: ownerId,
|
|
expires_at: new Date(Date.now() + 7 * 864e5).toISOString(),
|
|
is_active: 1, is_archived: 0, is_draft: 0,
|
|
created_at: new Date().toISOString(),
|
|
}).returning('id');
|
|
viewerEventId = ev[0]?.id ?? ev[0];
|
|
|
|
app = express();
|
|
app.use(express.json());
|
|
app.use('/api/v1', require('../../src/routes/v1/events'));
|
|
}, 120000);
|
|
|
|
afterAll(async () => { if (cleanup) await cleanup(); });
|
|
|
|
it('denies event creation to a demoted viewer despite an admin-scope token', async () => {
|
|
const res = await request(app)
|
|
.post('/api/v1/events')
|
|
.set('Authorization', `Bearer ${viewerToken}`)
|
|
.send({ event_name: 'Nope', event_type: 'wedding' });
|
|
expect(res.status).toBe(403);
|
|
});
|
|
|
|
it('denies photo upload to a demoted viewer on their OWN event', async () => {
|
|
const res = await request(app)
|
|
.post(`/api/v1/events/${viewerEventId}/photos`)
|
|
.set('Authorization', `Bearer ${viewerToken}`)
|
|
.attach('photo', Buffer.from('x'), 'a.jpg');
|
|
expect(res.status).toBe(403);
|
|
});
|
|
|
|
it('still allows the viewer to READ their own event', async () => {
|
|
const res = await request(app)
|
|
.get(`/api/v1/events/${viewerEventId}`)
|
|
.set('Authorization', `Bearer ${viewerToken}`);
|
|
expect(res.status).toBe(200);
|
|
});
|
|
});
|