diff --git a/backend/__tests__/integration/categoryReorder.test.js b/backend/__tests__/integration/categoryReorder.test.js index 4e180558..48f6f06c 100644 --- a/backend/__tests__/integration/categoryReorder.test.js +++ b/backend/__tests__/integration/categoryReorder.test.js @@ -2,9 +2,9 @@ * Layered per-event category ordering (#782). * * Two ordering layers, resolved per event: - * - GLOBAL default — photo_categories.display_order (migration 158), + * - GLOBAL default — photo_categories.display_order (migration 159), * set via POST /reorder-global; applies everywhere. - * - PER-EVENT override — event_category_order (migration 159), set via + * - PER-EVENT override — event_category_order (migration 160), set via * POST /reorder; overrides the default for one gallery. * - DELETE /reorder/:eventId clears an event's override. * @@ -58,7 +58,7 @@ describe('category ordering (#782)', () => { const getEvent = (eventId) => auth(request(app).get(`/api/admin/categories/event/${eventId}`)).expect(200); - describe('migration 158 backfill', () => { + describe('migration 159 backfill', () => { it('seeds display_order from alphabetical order, scoped per event', async () => { const eventId = await insertEvent('backfill-ev'); await insertCat('Reception', { event_id: eventId }); @@ -67,7 +67,7 @@ describe('category ordering (#782)', () => { // Re-run the migration: addColumn is guarded (no-op); the backfill loop // re-runs and assigns per-scope alphabetical order — what an upgrade does. - await require('../../migrations/core/158_add_category_display_order').up(db); + await require('../../migrations/core/159_add_category_display_order').up(db); const evCats = await db('photo_categories').where({ event_id: eventId }).orderBy('display_order', 'asc'); expect(evCats.map((c) => c.name)).toEqual(['Ceremony', 'Pre-Ceremony', 'Reception']); @@ -154,6 +154,47 @@ describe('category ordering (#782)', () => { }); }); + describe('event ownership (PR #790 review)', () => { + let limitedToken; + let foreignEventId; + + beforeAll(async () => { + const bcrypt = require('bcrypt'); + // A non-super_admin role that DOES hold settings.view + settings.edit — + // the exact case the review flagged (settings.edit is grantable). + const roleRes = await db('roles').insert({ name: 'gallery-mgr', display_name: 'Gallery Mgr' }).returning('id'); + const roleId = roleRes[0]?.id ?? roleRes[0]; + const permIds = await db('permissions').whereIn('name', ['settings.view', 'settings.edit']).pluck('id'); + await db('role_permissions').insert(permIds.map((permission_id) => ({ role_id: roleId, permission_id }))); + + const a2 = await db('admin_users').insert({ + username: 'limited', email: 'limited@example.com', + password_hash: await bcrypt.hash('x', 4), role_id: roleId, + must_change_password: false, created_at: new Date(), + }).returning('id'); + limitedToken = mintAdminToken(a2[0]?.id ?? a2[0]); + + // An event owned by a DIFFERENT admin (the seeded super_admin). + const owner = (await db('admin_users').where({ username: 'tester' }).first()).id; + await db('events').insert({ + event_type: 'wedding', password_hash: 'x', + expires_at: new Date(Date.now() + 9e9).toISOString(), + is_active: true, is_archived: false, slug: 'owned-ev', share_link: 'owned-ev', + event_name: 'Owned', event_date: '2026-01-01', created_by: owner, + }); + foreignEventId = (await db('events').where({ slug: 'owned-ev' }).first()).id; + }); + + const limitedAuth = (r) => r.set('Authorization', `Bearer ${limitedToken}`); + + it('blocks a non-owner from reading, reordering or resetting another event', async () => { + await limitedAuth(request(app).get(`/api/admin/categories/event/${foreignEventId}`)).expect(403); + await limitedAuth(request(app).post('/api/admin/categories/reorder')) + .send({ event_id: foreignEventId, orderedIds: [1] }).expect(403); + await limitedAuth(request(app).delete(`/api/admin/categories/reorder/${foreignEventId}`)).expect(403); + }); + }); + describe('POST / (create) appends to the end of its scope', () => { it('assigns display_order = max + 1 within the event', async () => { const eventId = await insertEvent('append-ev'); diff --git a/backend/migrations/core/158_add_category_display_order.js b/backend/migrations/core/159_add_category_display_order.js similarity index 97% rename from backend/migrations/core/158_add_category_display_order.js rename to backend/migrations/core/159_add_category_display_order.js index f3a977ad..78c5cbf8 100644 --- a/backend/migrations/core/158_add_category_display_order.js +++ b/backend/migrations/core/159_add_category_display_order.js @@ -1,5 +1,5 @@ /** - * Migration 158: per-event category ordering (#782). + * Migration 159: per-event category ordering (#782). * * Adds a `display_order` integer to `photo_categories` so photographers can * arrange an event's categories in the flow of the day (Pre-Ceremony → diff --git a/backend/migrations/core/159_add_event_category_order.js b/backend/migrations/core/160_add_event_category_order.js similarity index 93% rename from backend/migrations/core/159_add_event_category_order.js rename to backend/migrations/core/160_add_event_category_order.js index a110a05f..f244d230 100644 --- a/backend/migrations/core/159_add_event_category_order.js +++ b/backend/migrations/core/160_add_event_category_order.js @@ -1,7 +1,7 @@ /** - * Migration 159: per-event category order override (#782). + * Migration 160: per-event category order override (#782). * - * Builds on migration 158 (photo_categories.display_order = the GLOBAL default + * Builds on migration 159 (photo_categories.display_order = the GLOBAL default * order) by adding a per-event OVERRIDE layer. Global categories are shared * across every event, so a single display_order can only express one order for * them. This table lets a single gallery arrange its categories — globals AND diff --git a/backend/src/routes/adminCategories.js b/backend/src/routes/adminCategories.js index 75e91a49..809cbd43 100644 --- a/backend/src/routes/adminCategories.js +++ b/backend/src/routes/adminCategories.js @@ -4,6 +4,7 @@ const { db, logActivity } = require('../database/db'); const { formatBoolean } = require('../utils/dbCompat'); const { adminAuth } = require('../middleware/auth'); const { requirePermission } = require('../middleware/permissions'); +const { requireEventOwnership } = require('../middleware/ownership'); const { getEventCategoriesOrdered } = require('../utils/categoryOrder'); const logger = require('../utils/logger'); const router = express.Router(); @@ -26,7 +27,7 @@ router.get('/global', adminAuth, requirePermission('settings.view'), async (req, // Get categories for a specific event (global + event-specific), resolved to // the event's effective order: per-event override, else global default, else // name (#782). Each row carries `override_position` (null when not customised). -router.get('/event/:eventId', adminAuth, requirePermission('settings.view'), async (req, res) => { +router.get('/event/:eventId', adminAuth, requirePermission('settings.view'), requireEventOwnership, async (req, res) => { try { const categories = await getEventCategoriesOrdered(req.params.eventId); res.json(categories); @@ -282,6 +283,20 @@ router.post('/reorder', adminAuth, requirePermission('settings.edit'), [ const eventId = parseInt(req.body.event_id, 10); const orderedIds = req.body.orderedIds.map((id) => parseInt(id, 10)); + // Event ownership (event_id comes from the body, so requireEventOwnership — + // which reads req.params — can't be used here). Mirror it: super_admins + // bypass; other admins may only reorder events they own (ownerless + // legacy/system events allowed). + if (req.admin.roleName !== 'super_admin') { + const event = await db('events').where('id', eventId).first(); + if (!event) { + return res.status(404).json({ error: 'Event not found' }); + } + if (event.created_by && event.created_by !== req.admin.id) { + return res.status(403).json({ error: 'Access denied' }); + } + } + // Every id must be a category available to this event: a shared global OR // one of the event's own categories. Anything else is out of scope. const available = await db('photo_categories') @@ -317,7 +332,7 @@ router.post('/reorder', adminAuth, requirePermission('settings.edit'), [ }); // Clear an event's override — revert this gallery to the global default order. -router.delete('/reorder/:eventId', adminAuth, requirePermission('settings.edit'), async (req, res) => { +router.delete('/reorder/:eventId', adminAuth, requirePermission('settings.edit'), requireEventOwnership, async (req, res) => { try { const eventId = parseInt(req.params.eventId, 10); await db('event_category_order').where('event_id', eventId).del(); diff --git a/backend/src/utils/categoryOrder.js b/backend/src/utils/categoryOrder.js index f5ffa3af..2cabddd7 100644 --- a/backend/src/utils/categoryOrder.js +++ b/backend/src/utils/categoryOrder.js @@ -4,7 +4,7 @@ * Resolves an event's categories into their effective display order, layering: * 1. per-event override — event_category_order.position, when the event has * been customised; - * 2. the global default — photo_categories.display_order (migration 158); + * 2. the global default — photo_categories.display_order (migration 159); * 3. name. * * Globals and event-specific categories are ordered together so a custom order diff --git a/frontend/src/components/admin/EventCategoryManager.tsx b/frontend/src/components/admin/EventCategoryManager.tsx index d29dba35..78d136af 100644 --- a/frontend/src/components/admin/EventCategoryManager.tsx +++ b/frontend/src/components/admin/EventCategoryManager.tsx @@ -177,7 +177,7 @@ export const EventCategoryManager: React.FC = ({ even

{isCustomised ? t('categories.orderCustomisedHint', 'This gallery uses a custom order. Reset to follow the global default (Settings → Photo Categories).') - : t('categories.orderDefaultHint', 'Drag the arrows to set the order for this gallery. Otherwise it follows the global default (Settings → Photo Categories).')} + : t('categories.orderDefaultHint', 'Use the arrows to set the order for this gallery. Otherwise it follows the global default (Settings → Photo Categories).')}

{/* Add new category form */} @@ -378,7 +378,7 @@ export const EventCategoryManager: React.FC = ({ even /> {isSelected && ( -
+
)}