fix(security): harden four smaller gallery and contract paths, drop the unmounted photo auth middleware
- the customer contract PDF stream applies assertContractPdfPath like the
admin and public contract routes
- OG previews fall back to the site card for draft, archived and
deactivated galleries instead of leaking name, date and welcome message
- video Range requests are validated before the 206 is written; a NaN,
inverted or out-of-file range now answers 416
- share-token comparisons in gallery resolve/info use the constant-time
helper share-login already used
- middleware/photoAuth.js and the galleryAuth/photoAuth/verifyGalleryAccess
exports of middleware/auth.js were unreferenced since the static mounts
went; the auth.js copy had neither slug binding nor issuer pin, so it is
removed before anyone mounts it
(cherry picked from commit 835312e8e6)
This commit is contained in:
Paul Nothaft
@@ -4,7 +4,7 @@ const { formatBoolean } = require('../utils/dbCompat');
|
||||
const { isMissingRolesSchema } = require('../utils/dbErrors');
|
||||
const { isTokenRevoked } = require('../utils/tokenRevocation');
|
||||
const logger = require('../utils/logger');
|
||||
const { getAdminTokenFromRequest, getGalleryTokenFromRequest } = require('../utils/tokenUtils');
|
||||
const { getAdminTokenFromRequest } = require('../utils/tokenUtils');
|
||||
|
||||
/**
|
||||
* Enhanced admin authentication middleware with revocation checking
|
||||
@@ -136,170 +136,6 @@ async function adminAuth(req, res, next) {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Enhanced gallery authentication middleware with revocation checking
|
||||
*/
|
||||
async function galleryAuth(req, res, next) {
|
||||
try {
|
||||
const slug = req.params?.slug || req.requestedSlug;
|
||||
const token = getGalleryTokenFromRequest(req, slug);
|
||||
if (!token) {
|
||||
return res.status(401).json({ error: 'No token provided' });
|
||||
}
|
||||
|
||||
let decoded;
|
||||
try {
|
||||
decoded = jwt.verify(token, process.env.JWT_SECRET, {
|
||||
algorithms: ['HS256'],
|
||||
issuer: 'picpeak-auth',
|
||||
complete: true
|
||||
});
|
||||
decoded = decoded.payload;
|
||||
} catch (err) {
|
||||
if (err.name === 'TokenExpiredError') {
|
||||
return res.status(401).json({ error: 'Session expired', code: 'TOKEN_EXPIRED' });
|
||||
}
|
||||
return res.status(401).json({ error: 'Invalid session' });
|
||||
}
|
||||
|
||||
// Check if token is revoked
|
||||
if (await isTokenRevoked(decoded)) {
|
||||
return res.status(401).json({ error: 'Session has been invalidated', code: 'TOKEN_REVOKED' });
|
||||
}
|
||||
|
||||
// Verify token type
|
||||
if (decoded.type !== 'gallery') {
|
||||
return res.status(403).json({ error: 'Invalid access token' });
|
||||
}
|
||||
|
||||
// Check if event still exists and is active
|
||||
const event = await db('events')
|
||||
.where({
|
||||
id: decoded.eventId,
|
||||
is_active: true,
|
||||
is_archived: false
|
||||
})
|
||||
.first();
|
||||
|
||||
if (!event) {
|
||||
return res.status(404).json({ error: 'Gallery not found or expired' });
|
||||
}
|
||||
|
||||
// Check if gallery has expired (only if expires_at is set)
|
||||
// Galleries with null expires_at never expire
|
||||
if (event.expires_at && new Date(event.expires_at) < new Date()) {
|
||||
return res.status(410).json({
|
||||
error: 'Gallery has expired',
|
||||
code: 'GALLERY_EXPIRED'
|
||||
});
|
||||
}
|
||||
|
||||
// Add event info to request
|
||||
req.event = event;
|
||||
req.galleryToken = decoded;
|
||||
req.token = token;
|
||||
|
||||
next();
|
||||
} catch (error) {
|
||||
logger.error('Gallery auth middleware error:', error);
|
||||
res.status(401).json({ error: 'Authentication failed' });
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Photo access authentication
|
||||
* Validates both admin and gallery tokens for photo access
|
||||
*/
|
||||
async function photoAuth(req, res, next) {
|
||||
try {
|
||||
const slug = req.params?.slug || req.requestedSlug;
|
||||
const token = getAdminTokenFromRequest(req) || getGalleryTokenFromRequest(req, slug);
|
||||
if (!token) {
|
||||
return res.status(401).json({ error: 'Authentication required' });
|
||||
}
|
||||
|
||||
let decoded;
|
||||
try {
|
||||
decoded = jwt.verify(token, process.env.JWT_SECRET, { algorithms: ['HS256'] });
|
||||
} catch (err) {
|
||||
return res.status(401).json({ error: 'Invalid token' });
|
||||
}
|
||||
|
||||
// Check if token is revoked
|
||||
if (await isTokenRevoked(decoded)) {
|
||||
return res.status(401).json({ error: 'Token has been revoked', code: 'TOKEN_REVOKED' });
|
||||
}
|
||||
|
||||
// Allow both admin and gallery tokens
|
||||
if (decoded.type === 'admin') {
|
||||
const admin = await db('admin_users')
|
||||
.where({ id: decoded.id, is_active: formatBoolean(true) })
|
||||
.first();
|
||||
|
||||
if (!admin) {
|
||||
return res.status(401).json({ error: 'Invalid token' });
|
||||
}
|
||||
|
||||
req.auth = { type: 'admin', user: admin };
|
||||
} else if (decoded.type === 'gallery') {
|
||||
const event = await db('events')
|
||||
.where({
|
||||
id: decoded.eventId,
|
||||
is_active: true,
|
||||
is_archived: false
|
||||
})
|
||||
.first();
|
||||
|
||||
if (!event) {
|
||||
return res.status(404).json({ error: 'Gallery not found' });
|
||||
}
|
||||
|
||||
// For gallery tokens, ensure they can only access their event's photos
|
||||
req.auth = { type: 'gallery', event: event };
|
||||
} else {
|
||||
return res.status(403).json({ error: 'Invalid token type' });
|
||||
}
|
||||
|
||||
next();
|
||||
} catch (error) {
|
||||
logger.error('Photo auth middleware error:', error);
|
||||
res.status(401).json({ error: 'Authentication failed' });
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Verify gallery access for specific operations
|
||||
*/
|
||||
async function verifyGalleryAccess(req, res, next) {
|
||||
try {
|
||||
if (!req.auth) {
|
||||
return res.status(401).json({ error: 'Authentication required' });
|
||||
}
|
||||
|
||||
const { eventId } = req.params;
|
||||
|
||||
// Admins can access any gallery
|
||||
if (req.auth.type === 'admin') {
|
||||
return next();
|
||||
}
|
||||
|
||||
// Gallery tokens can only access their own event
|
||||
if (req.auth.type === 'gallery') {
|
||||
if (req.auth.event.id !== parseInt(eventId)) {
|
||||
return res.status(403).json({ error: 'Access denied' });
|
||||
}
|
||||
return next();
|
||||
}
|
||||
|
||||
res.status(403).json({ error: 'Access denied' });
|
||||
} catch (error) {
|
||||
res.status(500).json({ error: 'Access verification failed' });
|
||||
}
|
||||
}
|
||||
|
||||
module.exports = {
|
||||
adminAuth,
|
||||
galleryAuth,
|
||||
photoAuth,
|
||||
verifyGalleryAccess
|
||||
adminAuth
|
||||
};
|
||||
|
||||
@@ -1,176 +0,0 @@
|
||||
const bcrypt = require('bcrypt');
|
||||
const jwt = require('jsonwebtoken');
|
||||
const { db } = require('../database/db');
|
||||
const { formatBoolean } = require('../utils/dbCompat');
|
||||
const { getGalleryTokenFromRequest } = require('../utils/tokenUtils');
|
||||
const { isTokenRevoked } = require('../utils/tokenRevocation');
|
||||
const logger = require('../utils/logger');
|
||||
|
||||
async function photoAuth(req, res, next) {
|
||||
try {
|
||||
// Extract event slug from the path
|
||||
let eventSlug;
|
||||
|
||||
// For thumbnails, we need to parse the filename to get the event info
|
||||
if (req.path.startsWith('/thumb_')) {
|
||||
// For now, we'll rely on JWT token for thumbnail access
|
||||
eventSlug = null;
|
||||
} else {
|
||||
// For regular photos, the slug is the first part of the path
|
||||
eventSlug = req.path.split('/')[1];
|
||||
}
|
||||
|
||||
// First check for JWT token (from gallery access)
|
||||
const tokenFromRequest = getGalleryTokenFromRequest(req, eventSlug);
|
||||
if (tokenFromRequest) {
|
||||
const token = tokenFromRequest;
|
||||
try {
|
||||
// Try to verify with issuer first, fallback to no issuer for backward compatibility
|
||||
let decoded;
|
||||
try {
|
||||
decoded = jwt.verify(token, process.env.JWT_SECRET, {
|
||||
algorithms: ['HS256'],
|
||||
issuer: 'picpeak-auth'
|
||||
});
|
||||
} catch (issuerError) {
|
||||
// If verification fails with issuer, try without issuer (backward compatibility)
|
||||
if (issuerError.name === 'JsonWebTokenError' && issuerError.message.includes('jwt issuer invalid')) {
|
||||
decoded = jwt.verify(token, process.env.JWT_SECRET, { algorithms: ['HS256'] });
|
||||
} else {
|
||||
throw issuerError;
|
||||
}
|
||||
}
|
||||
|
||||
// Check if it's a gallery token
|
||||
if (decoded.type === 'gallery') {
|
||||
// For thumbnails, we need to verify the token is for a valid event
|
||||
if (!eventSlug) {
|
||||
// Resolve the token's event (by id, or legacy slug fallback)...
|
||||
let event = null;
|
||||
if (decoded.eventId) {
|
||||
event = await db('events')
|
||||
.where({ id: decoded.eventId, is_active: formatBoolean(true) })
|
||||
.first();
|
||||
}
|
||||
if (!event && decoded.eventSlug) {
|
||||
event = await db('events')
|
||||
.where({ slug: decoded.eventSlug, is_active: formatBoolean(true) })
|
||||
.first();
|
||||
}
|
||||
// ...then confirm the REQUESTED thumbnail actually belongs to
|
||||
// that event. Thumbnails are stored flat (thumbnails/thumb_<name>)
|
||||
// with deterministic, enumerable filenames derived from the
|
||||
// public event name + a sequential counter. Without this
|
||||
// ownership check any holder of a gallery token for any event
|
||||
// could enumerate and fetch another (password-protected) event's
|
||||
// entire thumbnail set, defeating the gallery password. A
|
||||
// traversal or foreign filename simply fails to match → denied.
|
||||
if (event) {
|
||||
const requestedKey = `thumbnails${req.path}`;
|
||||
const ownsThumbnail = await db('photos')
|
||||
.where({ event_id: event.id, thumbnail_path: requestedKey })
|
||||
.first();
|
||||
if (ownsThumbnail) {
|
||||
req.event = event;
|
||||
return next();
|
||||
}
|
||||
}
|
||||
}
|
||||
// For regular photos, check if token matches the event
|
||||
else if (decoded.eventSlug === eventSlug) {
|
||||
const event = await db('events')
|
||||
.where({ slug: eventSlug, is_active: formatBoolean(true) })
|
||||
.first();
|
||||
if (event) {
|
||||
req.event = event;
|
||||
return next();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Check if it's an admin token (admins can view all photos)
|
||||
if (decoded.type === 'admin') {
|
||||
// Enforce the same revocation / session-cutoff invalidation that
|
||||
// adminAuth does — otherwise a validly-signed admin JWT keeps
|
||||
// serving photos after logout, password change, or explicit
|
||||
// revocation (GHSA-x55x).
|
||||
if (await isTokenRevoked(decoded)) {
|
||||
return res.status(401).json({ error: 'Session expired' });
|
||||
}
|
||||
// adminAuth also (a) rejects tokens for a now-deactivated admin and
|
||||
// (b) rejects any token minted before the admin's last password
|
||||
// change. Token revocation alone doesn't cover those, so without
|
||||
// these two checks a stale or pre-password-change admin token still
|
||||
// fetches every photo.
|
||||
const admin = await db('admin_users')
|
||||
.where({ id: decoded.id, is_active: formatBoolean(true) })
|
||||
.select('id', 'password_changed_at')
|
||||
.first();
|
||||
if (!admin) {
|
||||
return res.status(401).json({ error: 'Session expired' });
|
||||
}
|
||||
if (admin.password_changed_at) {
|
||||
const passwordChangedSeconds = Math.floor(
|
||||
new Date(admin.password_changed_at).getTime() / 1000
|
||||
);
|
||||
if (decoded.iat < passwordChangedSeconds) {
|
||||
return res.status(401).json({ error: 'Session expired' });
|
||||
}
|
||||
}
|
||||
return next();
|
||||
}
|
||||
} catch (err) {
|
||||
// Token invalid, fall through to password check
|
||||
logger.warn('JWT verification failed in photoAuth', { error: err.message });
|
||||
}
|
||||
}
|
||||
|
||||
// Check for password header (legacy support)
|
||||
const password = req.headers['x-gallery-password'];
|
||||
|
||||
// If no eventSlug (thumbnails), and we don't have valid auth yet, deny access
|
||||
if (!eventSlug && !password && !tokenFromRequest) {
|
||||
return res.status(401).json({ error: 'Authentication required for thumbnails' });
|
||||
}
|
||||
|
||||
const event = await db('events').where({ slug: eventSlug, is_active: formatBoolean(true) }).first();
|
||||
if (!event) {
|
||||
return res.status(404).json({ error: 'Gallery not found' });
|
||||
}
|
||||
|
||||
const requiresPassword = !(event.require_password === false || event.require_password === 0 || event.require_password === '0');
|
||||
|
||||
if (!requiresPassword) {
|
||||
req.event = event;
|
||||
return next();
|
||||
}
|
||||
|
||||
if (!password && !tokenFromRequest) {
|
||||
return res.status(401).json({ error: 'Authentication required' });
|
||||
}
|
||||
|
||||
if (password) {
|
||||
const validPassword = await bcrypt.compare(password, event.password_hash);
|
||||
if (!validPassword) {
|
||||
await db('access_logs').insert({
|
||||
event_id: event.id,
|
||||
ip_address: req.ip,
|
||||
user_agent: req.headers['user-agent'],
|
||||
action: 'login_fail'
|
||||
});
|
||||
return res.status(401).json({ error: 'Invalid password' });
|
||||
}
|
||||
} else {
|
||||
// No valid authentication
|
||||
return res.status(401).json({ error: 'Invalid authentication' });
|
||||
}
|
||||
|
||||
req.event = event;
|
||||
next();
|
||||
} catch (error) {
|
||||
logger.error('Photo auth error', { error: error.message, stack: error.stack });
|
||||
res.status(500).json({ error: 'Authentication error' });
|
||||
}
|
||||
}
|
||||
|
||||
module.exports = photoAuth;
|
||||
@@ -18,6 +18,7 @@ const { body, param, validationResult } = require('express-validator');
|
||||
const { db, logActivity } = require('../database/db');
|
||||
const { formatBoolean } = require('../utils/dbCompat');
|
||||
const { getBcryptRounds, MAX_PASSWORD_LENGTH } = require('../utils/passwordValidation');
|
||||
const { assertContractPdfPath } = require('../utils/safePath');
|
||||
const logger = require('../utils/logger');
|
||||
const { errorResponse, safeValidationErrors } = require('../utils/routeHelpers');
|
||||
const { getClientIp } = require('../utils/requestIp');
|
||||
@@ -706,9 +707,13 @@ router.get('/contracts/:id/pdf', customerAuth, async (req, res) => {
|
||||
res.set('Content-Disposition', `inline; filename="${contract.contract_number}.pdf"`);
|
||||
return res.send(buf);
|
||||
}
|
||||
// Same containment the admin and public contract routes apply: the DB
|
||||
// path is written by the service layer today, but a bad row must not
|
||||
// turn this into an arbitrary-file read.
|
||||
const safePath = assertContractPdfPath(filePath);
|
||||
res.set('Content-Type', 'application/pdf');
|
||||
res.set('Content-Disposition', `inline; filename="${path.basename(filePath)}"`);
|
||||
fs.createReadStream(filePath).pipe(res);
|
||||
res.set('Content-Disposition', `inline; filename="${path.basename(safePath)}"`);
|
||||
fs.createReadStream(safePath).pipe(res);
|
||||
} catch (error) {
|
||||
errorResponse(res, error, 500, 'Failed to render contract PDF');
|
||||
}
|
||||
|
||||
@@ -11,6 +11,7 @@ const { getAppSetting } = require('../utils/appSettings');
|
||||
const archiver = require('archiver');
|
||||
const path = require('path');
|
||||
const { resolvePhotoContentType } = require('../utils/photoContentType');
|
||||
const { timingSafeEqualStr } = require('../utils/timingSafe');
|
||||
const router = express.Router();
|
||||
|
||||
// #756: a NULL per-event hero_logo_visible means "inherit the global
|
||||
@@ -168,7 +169,7 @@ router.get('/:slug/verify-token/:token', handleAsync(async (req, res) => {
|
||||
}
|
||||
|
||||
const expectedToken = getEventShareToken(event);
|
||||
if (token !== expectedToken) {
|
||||
if (!expectedToken || !timingSafeEqualStr(String(token), expectedToken)) {
|
||||
throw new NotFoundError('Gallery', 'Invalid gallery link');
|
||||
}
|
||||
|
||||
@@ -244,7 +245,7 @@ router.get('/:slug/info', async (req, res) => {
|
||||
// If token provided, verify it matches the share link
|
||||
if (token) {
|
||||
const expectedToken = getEventShareToken(event);
|
||||
if (!expectedToken || token !== expectedToken) {
|
||||
if (!expectedToken || !timingSafeEqualStr(String(token), expectedToken)) {
|
||||
return res.status(404).json({ error: 'Invalid gallery link' });
|
||||
}
|
||||
}
|
||||
@@ -1818,10 +1819,19 @@ router.get('/:slug/photo/:photoId',
|
||||
const parts = range.replace(/bytes=/, '').split('-');
|
||||
const start = parseInt(parts[0], 10);
|
||||
const end = parts[1] ? parseInt(parts[1], 10) : fileSize - 1;
|
||||
const chunksize = (end - start) + 1;
|
||||
// Validate before writing the 206: a NaN, inverted or out-of-file
|
||||
// range used to be committed to the headers and then throw while
|
||||
// streaming (or read past the end).
|
||||
if (!Number.isInteger(start) || !Number.isInteger(end)
|
||||
|| start < 0 || end < start || start >= fileSize) {
|
||||
res.set('Content-Range', `bytes */${fileSize}`);
|
||||
return res.status(416).end();
|
||||
}
|
||||
const boundedEnd = Math.min(end, fileSize - 1);
|
||||
const chunksize = (boundedEnd - start) + 1;
|
||||
|
||||
res.writeHead(206, {
|
||||
'Content-Range': `bytes ${start}-${end}/${fileSize}`,
|
||||
'Content-Range': `bytes ${start}-${boundedEnd}/${fileSize}`,
|
||||
'Accept-Ranges': 'bytes',
|
||||
'Content-Length': chunksize,
|
||||
'Content-Type': resolvePhotoContentType(photo),
|
||||
@@ -1830,8 +1840,8 @@ router.get('/:slug/photo/:photoId',
|
||||
});
|
||||
|
||||
const file = useStorageBackend
|
||||
? await storage.getRange(storageKey, start, end)
|
||||
: fs.createReadStream(filePath, { start, end });
|
||||
? await storage.getRange(storageKey, start, boundedEnd)
|
||||
: fs.createReadStream(filePath, { start, end: boundedEnd });
|
||||
pipeStreamToResponse(file, res, { context: `video range for photo ${photo.id}` });
|
||||
} else {
|
||||
res.writeHead(200, {
|
||||
|
||||
@@ -168,8 +168,19 @@ async function formatEventDate(value) {
|
||||
}
|
||||
}
|
||||
|
||||
// Draft, archived and deactivated galleries are refused by /info; the OG
|
||||
// preview must not leak their name, date and welcome message to crawlers.
|
||||
function isPubliclyVisible(event) {
|
||||
if (!event) return false;
|
||||
const truthy = (v) => v === true || v === 1 || v === '1' || v === 'true';
|
||||
if (truthy(event.is_draft) || truthy(event.is_archived)) return false;
|
||||
if (event.is_active === false || event.is_active === 0 || event.is_active === '0') return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
async function buildOgMetadata(slug, requestPath) {
|
||||
const event = await resolveSlug(slug);
|
||||
const resolved = await resolveSlug(slug);
|
||||
const event = isPubliclyVisible(resolved) ? resolved : null;
|
||||
const branding = await fetchBranding();
|
||||
const base = frontendBase();
|
||||
const siteName = branding.companyName || 'PicPeak';
|
||||
|
||||
Reference in New Issue
Block a user