paul/picpeak
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(gallery): guest filters respect show_feedback_to_guests (#1044) (#1156)

Browse Source 
Stable twin of #1147, filter half only.

Every filter token on /photos is an OR of two halves: what THIS viewer marked, and what ANYONE marked. The fields built from the second half are gated on show_feedback_to_guests; the filter was not, so with the setting off ?filter=liked still returned exactly the photos other people liked — the membership instead of the count, one token at a time.

The half it left standing was also the wrong half: it read guest_identifier from the guest_id query parameter, which never matched anything, and accepting a caller-supplied identifier was a way back through the gate. Resolved from the request now, hidden rows excluded to match what the viewer can see.

Not carried: the color: token and the photo_admin_marks concurrent-write fix — colour labels and admin marks are not on this branch.

Merged with admin privileges: the author cannot self-approve.
This commit is contained in:
Paul Nothaft
2026-08-23 22:08:23 +02:00
committed by GitHub
parent d46397d92a
commit eaa8b41ba3
2 changed files with 281 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/__tests__/integration/galleryFilterFeedbackVisibility.test.js
+216
View File
@@ -0,0 +1,216 @@
/**
* Guest filters must respect show_feedback_to_guests (#1044 follow-up).
*
* Every filter token on /photos is an OR of two halves: what THIS viewer
* marked, and what ANYONE marked. The response fields built from the second
* half — like_count, comment_count — are all gated on
* show_feedback_to_guests. The FILTER was not.
*
* So with the setting off, the numbers were hidden but `?filter=liked` still
* returned exactly the photos other people had liked: the same information as
* a set instead of a count, one token at a time. These tests pin the gate on
* every token, and pin that the viewer's own half is never gated — filtering
* by what you yourself marked is yours to do regardless.
*/
const request = require('supertest');
const express = require('express');
const cookieParser = require('cookie-parser');
const jwt = require('jsonwebtoken');
const { bootCrmDb, seedMinimal } = require('./helpers/crmDb');
process.env.JWT_SECRET = process.env.JWT_SECRET || 'filter-visibility-secret';
const SLUG = 'filter-visibility-event';
const ME = 'guest-me-identifier';
const SOMEONE_ELSE = 'guest-other-identifier';
describe('guest filters and show_feedback_to_guests (#1044)', () => {
let db;
let cleanup;
let app;
let eventId;
let mine;
let theirs;
let myGuestRowId;
const galleryToken = () => jwt.sign(
{ eventId, eventSlug: SLUG, type: 'gallery' },
process.env.JWT_SECRET,
{ expiresIn: '1h', issuer: 'picpeak-auth' }
);
const setVisibility = (visible) => db('event_feedback_settings')
.where({ event_id: eventId })
.update({ show_feedback_to_guests: visible });
// A real verified guest, which is how the viewer's own feedback is actually
// identified — NOT the `guest_id` query parameter the frontend invents.
const guestToken = () => jwt.sign(
{ type: 'guest', guestId: myGuestRowId, eventId },
process.env.JWT_SECRET,
{ expiresIn: '1h', issuer: 'picpeak-auth' }
);
const filter = async (token, { as = 'me', claimGuestId } = {}) => {
const req = request(app)
.get(`/api/gallery/${SLUG}/photos`)
.query({ filter: token, ...(claimGuestId ? { guest_id: claimGuestId } : {}) })
.set('Authorization', `Bearer ${galleryToken()}`);
if (as === 'me') req.set('x-guest-token', guestToken());
const res = await req;
expect(res.status).toBe(200);
const photos = Array.isArray(res.body) ? res.body : res.body.photos;
return (photos || []).map((p) => p.id).sort((a, b) => a - b);
};
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
await seedMinimal(db);
const inserted = await db('events').insert({
slug: SLUG,
event_type: 'wedding',
event_name: 'Filter Visibility',
event_date: '2026-08-01',
host_email: '[email protected]',
admin_email: '[email protected]',
password_hash: 'x',
share_link: `/gallery/${SLUG}/share`,
share_token: 'filter-visibility-share',
expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(),
is_active: 1,
is_archived: 0,
is_draft: 0,
created_at: new Date().toISOString(),
}).returning('id');
eventId = inserted[0]?.id ?? inserted[0];
const addPhoto = async (name) => {
const p = await db('photos').insert({
event_id: eventId,
filename: name,
path: `events/filter/${name}`,
type: 'individual',
uploaded_at: new Date().toISOString(),
}).returning('id');
return p[0]?.id ?? p[0];
};
mine = await addPhoto('mine.jpg');
theirs = await addPhoto('theirs.jpg');
await db('event_feedback_settings').insert({
event_id: eventId,
feedback_enabled: true,
allow_likes: true,
allow_comments: true,
allow_ratings: true,
allow_favorites: true,
moderate_comments: false,
show_feedback_to_guests: true,
});
const guestRow = await db('gallery_guests').insert({
event_id: eventId,
name: 'Me',
identifier: ME,
created_at: new Date().toISOString(),
last_seen_at: new Date().toISOString(),
is_deleted: false,
}).returning('id');
myGuestRowId = guestRow[0]?.id ?? guestRow[0];
const feedback = (photoId, who, type, extra = {}) => db('photo_feedback').insert({
photo_id: photoId,
event_id: eventId,
guest_identifier: who,
// Submission links to the per-person guest row when one is present, and
// that is the column the viewer's own half resolves through.
guest_id: who === ME ? myGuestRowId : null,
feedback_type: type,
is_approved: true,
is_hidden: false,
created_at: new Date().toISOString(),
...extra,
});
// Everything on `theirs` belongs to somebody else; `mine` is this viewer's.
await feedback(mine, ME, 'like');
await feedback(theirs, SOMEONE_ELSE, 'like');
await feedback(theirs, SOMEONE_ELSE, 'favorite');
await feedback(theirs, SOMEONE_ELSE, 'comment', { comment_text: 'lovely' });
await feedback(theirs, SOMEONE_ELSE, 'rating', { rating: 5 });
// The denormalized counters the aggregate half of the filter reads.
await db('photos').where('id', theirs).update({
like_count: 1, favorite_count: 1, comment_count: 1, average_rating: 5,
});
await db('photos').where('id', mine).update({ like_count: 1 });
app = express();
app.use(express.json());
app.use(cookieParser());
app.use('/api/gallery', require('../../src/routes/gallery'));
}, 120000);
afterAll(async () => {
if (cleanup) await cleanup();
});
describe('with feedback visible to guests', () => {
beforeAll(() => setVisibility(true));
it('shows other people\'s marks through every token, as before', async () => {
expect(await filter('liked')).toEqual([mine, theirs].sort((a, b) => a - b));
expect(await filter('favorited')).toEqual([theirs]);
expect(await filter('rated')).toEqual([theirs]);
expect(await filter('commented')).toEqual([theirs]);
});
});
describe('with feedback hidden from guests', () => {
beforeAll(() => setVisibility(false));
it('stops every token from selecting on other people\'s marks', async () => {
// `theirs` is the photo only other guests marked. It must not come back
// through any token — a filter that selects on hidden feedback reports
// that feedback just as surely as a count would.
expect(await filter('favorited')).toEqual([]);
expect(await filter('rated')).toEqual([]);
expect(await filter('commented')).toEqual([]);
});
it('still filters by what the viewer marked themselves', async () => {
// The viewer's own half is never gated: this is their own action, and
// hiding it would break "show me the ones I liked" for no privacy gain.
expect(await filter('liked')).toEqual([mine]);
});
it('drops the viewer\'s own feedback once an admin hides it', async () => {
// Moderation has to reach the filter too. getPhotoFeedback excludes
// hidden rows for the guest's OWN feedback, so a photo matching here
// would come back with nothing visible on it to explain why.
await db('photo_feedback')
.where({ photo_id: mine, guest_id: myGuestRowId, feedback_type: 'like' })
.update({ is_hidden: true });
expect(await filter('liked')).toEqual([]);
await db('photo_feedback')
.where({ photo_id: mine, guest_id: myGuestRowId, feedback_type: 'like' })
.update({ is_hidden: false });
expect(await filter('liked')).toEqual([mine]);
});
it('ignores a guest_id supplied by the caller', async () => {
// The own-half is resolved from the request identity. If it honoured the
// query string instead, anyone holding another guest's identifier could
// read that guest's hidden memberships one token at a time — straight
// back through the gate this file exists to pin.
expect(await filter('favorited', { claimGuestId: SOMEONE_ELSE })).toEqual([]);
// And an anonymous caller claiming to be me gets nothing of mine.
expect(await filter('liked', { as: 'anon', claimGuestId: ME })).toEqual([]);
});
});
});
backend/src/routes/gallery.js
+65 -17
View File
@@ -413,7 +413,10 @@ router.get('/:slug/show/:token/state', handleAsync(async (req, res) => {
router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res) => {
try {
// Get filter and sort parameters from query
const { filter, guest_id, sort = 'upload_date', order = 'desc' } = req.query;
// `guest_id` is deliberately NOT read from the query string: the viewer's
// own feedback is resolved from the request identity instead (see the
// filter block). The frontend still sends it; it is ignored.
const { filter, sort = 'upload_date', order = 'desc' } = req.query;
// Get watermark settings to generate cache-busting version for URLs
const watermarkSettings = await watermarkService.getWatermarkSettings();
@@ -458,6 +461,13 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res)
// Execute the query
let photos = await photosQuery;
// Check if feedback should be visible to guests. Read BEFORE the filter
// block, not after: the filters below consult it, because a filter that
// selects on other people's feedback is a way of reading that feedback.
const feedbackService = require('../services/feedbackService');
const feedbackSettings = await feedbackService.getEventFeedbackSettings(req.event.id);
const showFeedbackToGuests = isClient || parseBooleanInput(feedbackSettings.show_feedback_to_guests, true);
// Apply filtering if requested (supports global stats + per-guest interactions)
if (filter) {
const filterTokens = new Set(
@@ -487,10 +497,37 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res)
});
};
// Whose feedback counts as "mine" for these filters.
//
// Resolved from the REQUEST, the same either/or the per-viewer is_liked
// query below uses — never from the `guest_id` query parameter. Two
// reasons, and both matter now that this is the only half left when
// feedback is hidden:
//
// - It never matched. The frontend's `gallery_guest_id` is a
// localStorage string it invents (`guest_<ts>_<rand>`) and never
// sends when submitting feedback; submissions store
// generateGuestIdentifier(req). So this lookup found nothing, and
// the filters only ever worked through the aggregate half — which
// is exactly the half now gated.
// - It is caller-controlled. Accepting an identifier from the query
// string would let anyone holding someone else's read their hidden
// memberships one token at a time, straight back through the gate.
//
// Hidden rows are excluded, matching what the viewer can actually SEE:
// getPhotoFeedback drops is_hidden for the guest's own feedback too.
// Unapproved rows are NOT excluded — a comment still in the moderation
// queue is still the viewer's own, and that same read keeps it.
let guestFeedbackByType = null;
if (guest_id) {
const guestFeedbackRows = await db('photo_feedback')
.where({ event_id: req.event.id, guest_identifier: guest_id })
{
const viewerFeedback = db('photo_feedback')
.where({ event_id: req.event.id, is_hidden: false });
if (req.guest?.id) {
viewerFeedback.where('guest_id', req.guest.id);
} else {
viewerFeedback.where('guest_identifier', generateGuestIdentifier(req));
}
const guestFeedbackRows = await viewerFeedback
.select('photo_id', 'feedback_type');
guestFeedbackByType = guestFeedbackRows.reduce((acc, row) => {
@@ -509,39 +546,50 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res)
}
};
// Every token below is an OR of two halves: what THIS viewer marked,
// and what ANYONE marked. The second half is other people's feedback,
// so it is gated on show_feedback_to_guests exactly like the counts
// this endpoint returns.
//
// Without the gate the setting only hides the numbers. A guest could
// still send `?filter=liked` and get back precisely the set of photos
// other people liked — the membership, one token at a time, which is
// most of what the counts would have told them. The viewer's own half
// is always theirs to filter by.
const includeAggregate = (predicate) => {
if (showFeedbackToGuests) includeBy(predicate);
};
if (filterTokens.has('liked')) {
includeGuestMatches('like');
includeBy(photo => (photo.like_count || 0) > 0);
includeAggregate(photo => (photo.like_count || 0) > 0);
}
if (filterTokens.has('favorited')) {
includeGuestMatches('favorite');
includeBy(photo => (photo.favorite_count || 0) > 0);
includeAggregate(photo => (photo.favorite_count || 0) > 0);
}
if (filterTokens.has('rated')) {
includeGuestMatches('rating');
includeBy(photo => (photo.average_rating || 0) > 0);
includeAggregate(photo => (photo.average_rating || 0) > 0);
}
if (filterTokens.has('commented')) {
includeGuestMatches('comment');
const commentedRows = await db('photo_feedback')
.where({ event_id: req.event.id, feedback_type: 'comment', is_approved: true, is_hidden: false })
.groupBy('photo_id')
.select('photo_id');
commentedRows.forEach(row => include.add(row.photo_id));
if (showFeedbackToGuests) {
const commentedRows = await db('photo_feedback')
.where({ event_id: req.event.id, feedback_type: 'comment', is_approved: true, is_hidden: false })
.groupBy('photo_id')
.select('photo_id');
commentedRows.forEach(row => include.add(row.photo_id));
}
}
photos = photos.filter(photo => include.has(photo.id));
}
}
// Check if feedback should be visible to guests
const feedbackService = require('../services/feedbackService');
const feedbackSettings = await feedbackService.getEventFeedbackSettings(req.event.id);
const showFeedbackToGuests = isClient || parseBooleanInput(feedbackSettings.show_feedback_to_guests, true);
// Then get comment counts separately
const commentCounts = await db('photo_feedback')
.whereIn('photo_id', photos.map(p => p.id))