fix(security): scope dashboard stats/analytics/activity to the caller's events (stable) (#964)

* fix(security): scope dashboard endpoints to the caller's events (GHSA-c2jj, gqx7, jhcf)

/dashboard/stats, /analytics and /activity are gated only by analytics.view,
which the editor role holds — but the events LIST restricts editors to their
own rows (adminEvents/crud.js: roleName === 'editor' -> created_by =
admin.id). So an editor saw instance-wide totals, and via /analytics
topGalleries other admins' gallery NAMES and SLUGS (the public gallery URL
component), for events invisible to them everywhere else.

- stats: all 10 aggregates scoped (events by id, photos/access_logs by
  event_id).
- analytics: all 8 series/aggregates scoped, including topGalleries. The
  external tracker device breakdown reports instance-wide data with no event
  filter, so a scoped caller falls through to the access_logs heuristic
  instead, which IS scoped.
- activity: feed scoped. activity_logs.event_id is nullable and the join is a
  leftJoin, so system-level rows (logins, settings changes) are deliberately
  excluded for a scoped caller — those are precisely the cross-admin actions
  the advisory is about.

Scoping keys on 'editor' to mirror the events list exactly, so the admin
role's dashboard is unchanged. filterOwnedEventIds uses the broader
'!== super_admin' rule; the two conventions disagree in this codebase and
matching the list is the no-regression choice.

* fix(security): codex round 2 — fix activity misattribution, scope via subquery (GHSA-jhcf, c2jj, gqx7)

- expenseService passed adminId as logActivity's THIRD positional parameter,
  which is eventId — so admin ids were being written into
  activity_logs.event_id. The /activity scoping filter trusts that column, and
  admin/event id sequences overlap, so a foreign admin's expense metadata could
  surface under an editor's event. All 11 calls now pass null for eventId and
  the admin as the actor, which is what they meant.
- Dashboard scoping now uses a SUBQUERY instead of pluck()+whereIn. An editor
  owning more events than the driver's bind-parameter limit (~999 SQLite,
  65535 Postgres) would have turned all three endpoints into 500s once each id
  became a placeholder; below the limit it still re-sent the full list for each
  of the ~10 aggregates per request.

Note: two billInboundNow() calls also end in ', adminId)' but have an unrelated
signature — verified untouched.

* fix(security): codex round 3 — correct legacy accounting activity rows (GHSA-jhcf)

expenseService called logActivity(type, metadata, adminId), but logActivity's
third positional parameter is eventId. Every expense / incoming-invoice entry
therefore stored the ACTING ADMIN'S ID in activity_logs.event_id.

Round 2 scoped the activity feed with
`WHERE activity_logs.event_id IN (SELECT id FROM events WHERE created_by = me)`,
which does nothing about the rows already on disk. Admin ids and event ids are
small integers from the same range, so on any upgraded instance an editor who
owns the event whose id happens to equal another admin's id is served that
admin's accounting activity, verbatim metadata included — GHSA-jhcf, still
live. Migration 168 re-attributes those rows (event_id holds exactly the actor
id that was lost) and then clears event_id so the scope predicate can no longer
match them. All ten activity types are emitted by expenseService and nothing
else, so no row with a genuine event_id is touched.

Also: the round-2 rewrite passed `{ type: 'admin', id: adminId }`
unconditionally, which stored actor_type='admin' with a null id for the
automated mailbox intake (emailIntakeService calls recordInboundDocument with
no adminId). adminActor() restores 'system' attribution for those.

Claude-Session: https://claude.ai/code/session_01F211U4dDbEj4zXiyKbi9me
(cherry picked from commit 459e9e42434defd0dc7b87246e4d894dd47dcc56)

---------

Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
This commit is contained in:
Paul Nothaft
2026-08-02 21:18:46 +02:00
committed by GitHub
parent 3b88036fda
commit 11f9f584de
5 changed files with 435 additions and 34 deletions
@@ -0,0 +1,83 @@
/**
* GHSA-jhcf round 3: scoping the activity feed does nothing about the rows
* already on disk. expenseService used to pass adminId into logActivity's
* `eventId` slot, so upgraded instances carry accounting rows whose event_id
* is an ADMIN id — and the scope predicate happily matches those against a
* same-numbered event the caller owns.
*/
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-mig168-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'mig168-test-secret';
const { bootCrmDb } = require('../integration/helpers/crmDb');
const migration = require('../../migrations/core/168_fix_expense_activity_event_id');
describe('migration 168 — legacy accounting activity rows (GHSA-jhcf)', () => {
let db; let cleanup;
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
}, 120000);
afterAll(async () => { if (cleanup) await cleanup(); });
it('re-attributes the admin id and clears event_id, leaving real rows alone', async () => {
await db('activity_logs').insert([
// Legacy shape: event_id is really admin #7, no actor recorded.
{
activity_type: 'expense_created',
actor_type: 'system',
actor_id: null,
event_id: 7,
metadata: JSON.stringify({ expenseId: 1 }),
created_at: new Date().toISOString(),
},
{
activity_type: 'incoming_invoice_captured',
actor_type: 'system',
actor_id: null,
event_id: 9,
metadata: JSON.stringify({ inboundDocumentId: 2 }),
created_at: new Date().toISOString(),
},
// A genuine event-scoped row from another subsystem must survive intact.
{
activity_type: 'photo_uploaded',
actor_type: 'admin',
actor_id: 3,
event_id: 7,
metadata: JSON.stringify({}),
created_at: new Date().toISOString(),
},
]);
await migration.up(db);
const expense = await db('activity_logs').where({ activity_type: 'expense_created' }).first();
expect(expense.event_id == null).toBe(true);
expect(Number(expense.actor_id)).toBe(7);
expect(expense.actor_type).toBe('admin');
const captured = await db('activity_logs').where({ activity_type: 'incoming_invoice_captured' }).first();
expect(captured.event_id == null).toBe(true);
expect(Number(captured.actor_id)).toBe(9);
const photo = await db('activity_logs').where({ activity_type: 'photo_uploaded' }).first();
expect(Number(photo.event_id)).toBe(7);
expect(Number(photo.actor_id)).toBe(3);
});
it('is idempotent on re-run', async () => {
await expect(migration.up(db)).resolves.toBeUndefined();
const expense = await db('activity_logs').where({ activity_type: 'expense_created' }).first();
expect(Number(expense.actor_id)).toBe(7);
expect(expense.event_id == null).toBe(true);
});
});
@@ -0,0 +1,196 @@
/**
* Dashboard endpoints must not leak other admins' data to event-scoped
* editors — GHSA-c2jj (/stats), GHSA-gqx7 (/analytics), GHSA-jhcf (/activity).
*
* All three are gated only by `analytics.view`, which the `editor` role holds.
* But the events LIST restricts editors to their own rows
* (adminEvents/crud.js: roleName === 'editor' → created_by = admin.id), so an
* editor saw instance-wide totals — and, via /analytics topGalleries, other
* admins' gallery names and SLUGS (the public gallery URL component) — for
* events invisible to them everywhere else.
*
* Scoping deliberately keys on `editor` to mirror the events list exactly, so
* the `admin` role's dashboard is unchanged.
*/
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-dashscope-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'dashscope-test-secret';
const request = require('supertest');
const express = require('express');
const bcrypt = require('bcrypt');
const jwt = require('jsonwebtoken');
const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
describe('dashboard scoping (GHSA-c2jj / gqx7 / jhcf)', () => {
let db; let cleanup; let app;
let editorToken; let superToken;
let ownEventId; let foreignEventId;
const mkAdmin = async (username, roleName) => {
const role = await db('roles').where({ name: roleName }).first();
const r = await db('admin_users').insert({
username,
email: `${username}@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 id = r[0]?.id ?? r[0];
const token = jwt.sign(
{ id, username, type: 'admin', role: roleName, loginTime: Date.now() },
process.env.JWT_SECRET,
{ expiresIn: '1h', issuer: 'picpeak-auth' },
);
return { id, token };
};
const mkEvent = async (slug, createdBy) => {
const r = await db('events').insert({
slug,
event_type: 'wedding',
event_name: `${slug}-name`,
event_date: '2026-08-01',
host_email: 'h@example.com',
admin_email: 'a@example.com',
password_hash: 'x',
share_token: `tok-${slug}`,
share_link: `/gallery/${slug}/tok-${slug}`,
created_by: createdBy,
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');
return r[0]?.id ?? r[0];
};
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
await seedMinimal(db);
const editor = await mkAdmin('scoped-editor', 'editor');
const sup = await mkAdmin('root-admin', 'super_admin');
editorToken = editor.token;
superToken = sup.token;
ownEventId = await mkEvent('own-gallery', editor.id);
foreignEventId = await mkEvent('foreign-gallery', sup.id);
// One photo + one view per event so the aggregates are non-zero.
for (const [eventId, name] of [[ownEventId, 'own'], [foreignEventId, 'foreign']]) {
await db('photos').insert({
event_id: eventId,
filename: `${name}.jpg`,
path: `events/active/${name}.jpg`,
type: 'individual',
size_bytes: 1000,
uploaded_at: new Date().toISOString(),
});
await db('access_logs').insert({
event_id: eventId,
action: 'view',
ip_address: `10.0.0.${eventId}`,
user_agent: 'Mozilla/5.0',
timestamp: new Date().toISOString(),
});
await db('activity_logs').insert({
activity_type: 'photo_viewed',
actor_type: 'admin',
actor_name: `${name}-actor`,
event_id: eventId,
created_at: new Date().toISOString(),
});
}
app = express();
app.use(express.json());
app.use('/api/admin/dashboard', require('../../src/routes/adminDashboard'));
}, 120000);
afterAll(async () => { if (cleanup) await cleanup(); });
it('/stats counts only the editor\'s own events and photos', async () => {
const res = await request(app)
.get('/api/admin/dashboard/stats')
.set('Authorization', `Bearer ${editorToken}`);
expect(res.status).toBe(200);
expect(Number(res.body.totalEvents)).toBe(1);
expect(Number(res.body.totalPhotos)).toBe(1);
expect(Number(res.body.storageUsed)).toBe(1000);
});
it('/analytics does not expose a foreign gallery name or slug', async () => {
const res = await request(app)
.get('/api/admin/dashboard/analytics?days=7')
.set('Authorization', `Bearer ${editorToken}`);
expect(res.status).toBe(200);
const body = JSON.stringify(res.body);
expect(body).not.toContain('foreign-gallery');
expect(body).not.toContain('foreign-gallery-name');
expect(res.body.topGalleries.map((g) => g.slug)).toEqual(['own-gallery']);
});
it('/activity does not surface a foreign event\'s entries', async () => {
const res = await request(app)
.get('/api/admin/dashboard/activity')
.set('Authorization', `Bearer ${editorToken}`);
expect(res.status).toBe(200);
const actors = res.body.map((a) => a.actorName);
expect(actors).toContain('own-actor');
expect(actors).not.toContain('foreign-actor');
});
it('leaves super_admin unscoped across all three', async () => {
const stats = await request(app)
.get('/api/admin/dashboard/stats')
.set('Authorization', `Bearer ${superToken}`);
expect(Number(stats.body.totalEvents)).toBe(2);
const analytics = await request(app)
.get('/api/admin/dashboard/analytics?days=7')
.set('Authorization', `Bearer ${superToken}`);
expect(analytics.body.topGalleries.map((g) => g.slug).sort())
.toEqual(['foreign-gallery', 'own-gallery']);
const activity = await request(app)
.get('/api/admin/dashboard/activity')
.set('Authorization', `Bearer ${superToken}`);
expect(activity.body.map((a) => a.actorName)).toContain('foreign-actor');
});
});
/**
* Codex round 2: the /activity filter trusts `activity_logs.event_id`, but
* expenseService was passing `adminId` into logActivity's third positional
* parameter — which is `eventId`. Admin and event id sequences overlap, so a
* foreign admin's expense metadata could surface under an editor's event.
* Those writers now pass the actor instead, leaving event_id NULL.
*/
describe('activity writers do not put admin ids in event_id (GHSA-jhcf)', () => {
it('expenseService passes the actor, not adminId, as the event id', () => {
const fs2 = require('fs');
const src = fs2.readFileSync(
require('path').join(__dirname, '../../src/services/expenseService.js'), 'utf8',
);
// No logActivity call may end with a bare `, adminId)` — that slot is eventId.
const offenders = src.split('\n').filter(
(l) => l.includes('logActivity(') && /,\s*adminId\s*\)/.test(l),
);
expect(offenders).toEqual([]);
// And the actor form must actually be in use.
expect(src).toContain("{ type: 'admin', id: adminId }");
});
});
@@ -0,0 +1,64 @@
/**
* GHSA-jhcf — data correction for legacy accounting activity rows.
*
* expenseService called `logActivity(type, metadata, adminId)`, but the third
* positional parameter of logActivity is `eventId`, not the actor. Every
* expense / incoming-invoice entry therefore stored the ACTING ADMIN'S ID in
* `activity_logs.event_id` (and no actor at all).
*
* That is not merely cosmetic. The dashboard activity feed now scopes rows via
* `WHERE activity_logs.event_id IN (SELECT id FROM events WHERE created_by = me)`.
* Admin ids and event ids are both small integers drawn from the same range, so
* on any upgraded instance an editor who happens to own the event whose id
* equals another admin's id is served that admin's accounting activity —
* verbatim metadata included. Scoping new writes correctly does nothing for the
* rows already on disk, so they are corrected here.
*
* The stored value is exactly the actor id we lost, so this re-attributes
* rather than discards: event_id → actor_id (when no actor was recorded), then
* event_id is cleared so the scope predicate can no longer match it.
*
* All ten activity types below are emitted by expenseService and nothing else,
* so no row with a genuine event_id is touched.
*/
const AFFECTED_TYPES = [
'incoming_invoice_captured',
'incoming_invoice_updated',
'incoming_invoice_categorized',
'incoming_invoice_rebilled',
'incoming_invoices_rebilled_bundle',
'incoming_invoice_supplier_payment',
'expense_created',
'expense_updated',
'expense_invoiced',
'expense_paid',
];
exports.up = async function up(knex) {
if (!(await knex.schema.hasTable('activity_logs'))) return;
if (!(await knex.schema.hasColumn('activity_logs', 'event_id'))) return;
const hasActorId = await knex.schema.hasColumn('activity_logs', 'actor_id');
const hasActorType = await knex.schema.hasColumn('activity_logs', 'actor_type');
if (hasActorId) {
const patch = { actor_id: knex.ref('event_id') };
if (hasActorType) patch.actor_type = 'admin';
await knex('activity_logs')
.whereIn('activity_type', AFFECTED_TYPES)
.whereNotNull('event_id')
.whereNull('actor_id')
.update(patch);
}
await knex('activity_logs')
.whereIn('activity_type', AFFECTED_TYPES)
.whereNotNull('event_id')
.update({ event_id: null });
};
// Irreversible by design: this is a data correction, and the pre-migration
// state is a cross-admin disclosure. Re-planting admin ids in event_id would
// reopen GHSA-jhcf.
exports.down = async function down() {};
+72 -23
View File
@@ -24,11 +24,47 @@ function normaliseDateKey(value) {
return String(value).slice(0, 10);
}
/**
* Event ids the caller's dashboard may aggregate over, or `null` when the
* caller is unrestricted (GHSA-c2jj / gqx7 / jhcf).
*
* These endpoints are gated only by `analytics.view`, which the `editor` role
* holds — yet the events *list* restricts editors to their own rows
* (adminEvents/crud.js: `roleName === 'editor'` → `created_by = admin.id`).
* The dashboard therefore reported instance-wide totals, and the analytics
* endpoint returned other admins' gallery names and slugs, to a role that
* cannot see those events anywhere else.
*
* Scoped on `editor` specifically to mirror the events list exactly, so the
* `admin` role's dashboard is unchanged. (`filterOwnedEventIds` uses the
* broader `!== super_admin` rule; the two conventions disagree in this
* codebase and matching the list is the no-regression choice.)
*
* @returns {Promise<number[]|null>} ids to restrict to, or null for no limit
*/
function isScopedAdmin(admin) {
return admin?.roleName === 'editor';
}
/**
* Restrict `query` to the caller's own events.
*
* Uses a SUBQUERY rather than materialising the id list. An editor owning more
* events than the driver's bind-parameter limit (~999 on SQLite, 65535 on
* Postgres) would otherwise blow past it once every id became a placeholder,
* turning all three dashboard endpoints into 500s — and even well below that
* limit the whole list was re-sent for each of the ~10 aggregates per request.
*/
function applyEventScope(query, admin, column) {
if (!isScopedAdmin(admin)) return query;
return query.whereIn(column, db('events').select('id').where('created_by', admin.id));
}
// Get dashboard statistics
router.get('/stats', adminAuth, requirePermission('analytics.view'), async (req, res) => {
try {
// Get active events count
const activeEvents = await db('events')
const activeEvents = await applyEventScope(db('events'), req.admin, 'id')
.where('is_active', formatBoolean(true))
.where('is_archived', formatBoolean(false))
.count('id as count')
@@ -39,7 +75,7 @@ router.get('/stats', adminAuth, requirePermission('analytics.view'), async (req,
sevenDaysFromNow.setDate(sevenDaysFromNow.getDate() + 7);
const now = new Date();
const expiringEvents = await db('events')
const expiringEvents = await applyEventScope(db('events'), req.admin, 'id')
.where('is_active', formatBoolean(true))
.where('is_archived', formatBoolean(false))
.where('expires_at', '<=', sevenDaysFromNow.toISOString())
@@ -48,12 +84,12 @@ router.get('/stats', adminAuth, requirePermission('analytics.view'), async (req,
.first();
// Get total photos count
const totalPhotos = await db('photos')
const totalPhotos = await applyEventScope(db('photos'), req.admin, 'event_id')
.count('id as count')
.first();
// Get storage usage (sum of all photo sizes)
const storageUsed = await db('photos')
const storageUsed = await applyEventScope(db('photos'), req.admin, 'event_id')
.sum('size_bytes as total')
.first();
@@ -61,21 +97,21 @@ router.get('/stats', adminAuth, requirePermission('analytics.view'), async (req,
const thirtyDaysAgo = new Date();
thirtyDaysAgo.setDate(thirtyDaysAgo.getDate() - 30);
const totalViews = await db('access_logs')
const totalViews = await applyEventScope(db('access_logs'), req.admin, 'event_id')
.where('action', 'view')
.where('timestamp', '>=', thirtyDaysAgo.toISOString())
.count('id as count')
.first();
// Get total downloads (last 30 days) - include both single and bulk downloads
const totalDownloads = await db('access_logs')
const totalDownloads = await applyEventScope(db('access_logs'), req.admin, 'event_id')
.whereIn('action', ['download', 'download_all', 'download_all_presigned', 'download_selected'])
.where('timestamp', '>=', thirtyDaysAgo.toISOString())
.count('id as count')
.first();
// Get archived events count
const archivedEvents = await db('events')
const archivedEvents = await applyEventScope(db('events'), req.admin, 'id')
.where('is_archived', formatBoolean(true))
.count('id as count')
.first();
@@ -83,7 +119,7 @@ router.get('/stats', adminAuth, requirePermission('analytics.view'), async (req,
// Get total events count (all events regardless of status) — used by the
// events list page to render accurate "All (N)" / Total Events counters
// when the table is server-paginated (#346).
const totalEvents = await db('events')
const totalEvents = await applyEventScope(db('events'), req.admin, 'id')
.count('id as count')
.first();
@@ -91,14 +127,14 @@ router.get('/stats', adminAuth, requirePermission('analytics.view'), async (req,
const sixtyDaysAgo = new Date();
sixtyDaysAgo.setDate(sixtyDaysAgo.getDate() - 60);
const previousViews = await db('access_logs')
const previousViews = await applyEventScope(db('access_logs'), req.admin, 'event_id')
.where('action', 'view')
.where('timestamp', '>=', sixtyDaysAgo.toISOString())
.where('timestamp', '<', thirtyDaysAgo.toISOString())
.count('id as count')
.first();
const previousDownloads = await db('access_logs')
const previousDownloads = await applyEventScope(db('access_logs'), req.admin, 'event_id')
.whereIn('action', ['download', 'download_all', 'download_all_presigned', 'download_selected'])
.where('timestamp', '>=', sixtyDaysAgo.toISOString())
.where('timestamp', '<', thirtyDaysAgo.toISOString())
@@ -136,9 +172,19 @@ router.get('/activity', adminAuth, requirePermission('analytics.view'), async (r
try {
const { limit } = getPagination(req, { limit: 10 });
const activities = await db('activity_logs')
.select('activity_logs.*', 'events.event_name')
.leftJoin('events', 'activity_logs.event_id', 'events.id')
// Scope the feed to the caller's own events (GHSA-jhcf) — it otherwise
// returned every admin's actions, including actor names and verbatim
// metadata. `activity_logs.event_id` is NULLABLE: system-level entries
// (logins, settings changes) carry no event, and those are deliberately
// EXCLUDED for a scoped caller rather than shown, since they are exactly
// the cross-admin actions this advisory is about.
const activities = await applyEventScope(
db('activity_logs')
.select('activity_logs.*', 'events.event_name')
.leftJoin('events', 'activity_logs.event_id', 'events.id'),
req.admin,
'activity_logs.event_id'
)
.orderBy('activity_logs.created_at', 'desc')
.limit(limit);
@@ -244,7 +290,7 @@ router.get('/health', adminAuth, requirePermission('settings.view'), async (req,
router.get('/analytics', adminAuth, requirePermission('analytics.view'), async (req, res) => {
try {
const days = sanitizeDays(req.query.days || 7);
// Generate date range
const dates = [];
for (let i = days - 1; i >= 0; i--) {
@@ -262,21 +308,21 @@ router.get('/analytics', adminAuth, requirePermission('analytics.view'), async (
const startDateStr = startDate.toISOString();
// Get views per day
const viewsData = await db('access_logs')
const viewsData = await applyEventScope(db('access_logs'), req.admin, 'event_id')
.select(db.raw('DATE(timestamp) as date'), db.raw('COUNT(*) as count'))
.where('action', 'view')
.where('timestamp', '>=', startDateStr)
.groupByRaw('DATE(timestamp)');
// Get downloads per day - include both single and bulk downloads
const downloadsData = await db('access_logs')
const downloadsData = await applyEventScope(db('access_logs'), req.admin, 'event_id')
.select(db.raw('DATE(timestamp) as date'), db.raw('COUNT(*) as count'))
.whereIn('action', ['download', 'download_all', 'download_all_presigned', 'download_selected'])
.where('timestamp', '>=', startDateStr)
.groupByRaw('DATE(timestamp)');
// Get unique visitors per day
const visitorsData = await db('access_logs')
const visitorsData = await applyEventScope(db('access_logs'), req.admin, 'event_id')
.select(db.raw('DATE(timestamp) as date'), db.raw('COUNT(DISTINCT ip_address) as count'))
.where('timestamp', '>=', startDateStr)
.groupByRaw('DATE(timestamp)');
@@ -303,7 +349,7 @@ router.get('/analytics', adminAuth, requirePermission('analytics.view'), async (
});
// Get top galleries by views with additional metrics
const topGalleries = await db('access_logs')
const topGalleries = await applyEventScope(db('access_logs'), req.admin, 'access_logs.event_id')
.select('events.id', 'events.event_name', 'events.slug')
.select(db.raw('COUNT(CASE WHEN action = \'view\' THEN 1 END) as views'))
.select(db.raw('COUNT(DISTINCT CASE WHEN action = \'view\' THEN ip_address END) as uniqueVisitors'))
@@ -324,7 +370,10 @@ router.get('/analytics', adminAuth, requirePermission('analytics.view'), async (
let devices = { desktop: 0, mobile: 0, tablet: 0 };
let devicesSource = 'access_logs';
const adapter = await resolveAdapter();
// The external tracker reports instance-wide device data with no way to
// filter it by event, so a scoped caller must not receive it (GHSA-gqx7).
// They fall through to the access_logs heuristic, which IS scoped.
const adapter = isScopedAdmin(req.admin) ? null : await resolveAdapter();
if (adapter) {
try {
const trackerDevices = await adapter.fetchDeviceBreakdown({
@@ -346,7 +395,7 @@ router.get('/analytics', adminAuth, requirePermission('analytics.view'), async (
// Local heuristic on access_logs user_agent. Coarse — `LIKE` doesn't
// cover every UA shape (some Android browsers, embedded webviews, etc.)
// — and counts come back as strings on Postgres, hence Number() below.
const deviceData = await db('access_logs')
const deviceData = await applyEventScope(db('access_logs'), req.admin, 'event_id')
.select(
db.raw(`
CASE
@@ -370,19 +419,19 @@ router.get('/analytics', adminAuth, requirePermission('analytics.view'), async (
}
// Calculate totals for the period (matching /stats logic)
const totalViews = await db('access_logs')
const totalViews = await applyEventScope(db('access_logs'), req.admin, 'event_id')
.where('action', 'view')
.where('timestamp', '>=', startDateStr)
.count('id as count')
.first();
const totalDownloadsCount = await db('access_logs')
const totalDownloadsCount = await applyEventScope(db('access_logs'), req.admin, 'event_id')
.whereIn('action', ['download', 'download_all', 'download_all_presigned', 'download_selected'])
.where('timestamp', '>=', startDateStr)
.count('id as count')
.first();
const totalUniqueVisitors = await db('access_logs')
const totalUniqueVisitors = await applyEventScope(db('access_logs'), req.admin, 'event_id')
.where('timestamp', '>=', startDateStr)
.countDistinct('ip_address as count')
.first();
+20 -11
View File
@@ -22,6 +22,15 @@ const { AppError } = require('../utils/errors');
const logger = require('../utils/logger');
const invoiceService = require('./invoiceService');
/**
* Actor for logActivity. `adminId` is legitimately absent on automated paths —
* emailIntakeService calls recordInboundDocument() with none — and an
* unconditional `{ type: 'admin' }` would store actor_type='admin' with a null
* id, mislabelling mailbox captures as somebody's deliberate action. Returning
* null restores logActivity's 'system' attribution for those.
*/
const adminActor = (adminId) => (adminId ? { type: 'admin', id: adminId } : null);
const DISPOSITIONS = ['rebill', 'durchlaufend', 'eigener_aufwand', 'duplikat', 'abgelehnt'];
const TAX_TREATMENTS = ['domestic', 'reverse_charge_service', 'foreign_vat_non_reclaimable', 'import_goods'];
const MARKUP_TYPES = ['none', 'percent', 'flat'];
@@ -193,7 +202,7 @@ async function recordInboundDocument({ source, filePath, originalFilename, mimeT
};
const inserted = await db('inbound_documents').insert(row).returning('id');
const id = typeof inserted[0] === 'object' ? inserted[0].id : inserted[0];
await logActivity('incoming_invoice_captured', { inboundDocumentId: id, source: row.source, duplicate: !!duplicateOfId }, adminId);
await logActivity('incoming_invoice_captured', { inboundDocumentId: id, source: row.source, duplicate: !!duplicateOfId }, null, adminActor(adminId));
return getInbound(id);
}
@@ -242,7 +251,7 @@ async function updateInbound(id, payload, adminId) {
if (payload[camel] !== undefined) patch[snake] = payload[camel] === '' ? null : payload[camel];
}
await db('inbound_documents').where({ id }).update(patch);
await logActivity('incoming_invoice_updated', { inboundDocumentId: id }, adminId);
await logActivity('incoming_invoice_updated', { inboundDocumentId: id }, null, adminActor(adminId));
return getInbound(id);
}
@@ -458,8 +467,8 @@ async function categorizeInbound(id, payload, adminId) {
});
// Audit logging AFTER commit — logActivity writes via the global db and would
// deadlock if run inside the transaction above on a SQLite-backed install.
await logActivity('incoming_invoice_categorized', { inboundDocumentId: id, disposition }, adminId);
if (billedInvoiceId) await logActivity('incoming_invoice_rebilled', { inboundDocumentId: id, invoiceId: billedInvoiceId }, adminId);
await logActivity('incoming_invoice_categorized', { inboundDocumentId: id, disposition }, null, adminActor(adminId));
if (billedInvoiceId) await logActivity('incoming_invoice_rebilled', { inboundDocumentId: id, invoiceId: billedInvoiceId }, null, adminActor(adminId));
return getInbound(id);
}
@@ -494,7 +503,7 @@ async function rebillInbound(id, payload, adminId, trx0) {
const invoiceId = trx0 ? await run(trx0) : await db.transaction(run);
// Log after commit (global-db write — see billInboundNow). When a caller
// supplied trx0, that outer transaction owns the audit log instead.
if (!trx0) await logActivity('incoming_invoice_rebilled', { inboundDocumentId: id, invoiceId }, adminId);
if (!trx0) await logActivity('incoming_invoice_rebilled', { inboundDocumentId: id, invoiceId }, null, adminActor(adminId));
return { document: await getInbound(id), invoiceId };
}
@@ -607,7 +616,7 @@ async function billPendingRebills(customerId, adminId) {
return { invoiceId, count: pending.length };
});
// Audit log after commit (global-db write — see billInboundNow).
await logActivity('incoming_invoices_rebilled_bundle', { customerId: customer.id, invoiceId: result.invoiceId, count: result.count }, adminId);
await logActivity('incoming_invoices_rebilled_bundle', { customerId: customer.id, invoiceId: result.invoiceId, count: result.count }, null, adminActor(adminId));
return result;
}
@@ -624,7 +633,7 @@ async function markInboundSupplierPayment(id, { paid, paidAt, paymentMethod, pay
supplier_payment_ref: paid ? (paymentReference || null) : null,
updated_at: new Date(),
});
await logActivity('incoming_invoice_supplier_payment', { inboundDocumentId: id, paid: !!paid }, adminId);
await logActivity('incoming_invoice_supplier_payment', { inboundDocumentId: id, paid: !!paid }, null, adminActor(adminId));
return getInbound(id);
}
@@ -714,7 +723,7 @@ async function createExpense(payload, adminId, { receiptPath } = {}) {
});
const inserted = await db('expenses').insert(row).returning('id');
const id = typeof inserted[0] === 'object' ? inserted[0].id : inserted[0];
await logActivity('expense_created', { expenseId: id, kind: row.kind }, adminId);
await logActivity('expense_created', { expenseId: id, kind: row.kind }, null, adminActor(adminId));
return getExpense(id);
}
@@ -747,7 +756,7 @@ async function updateExpense(id, payload, adminId, { receiptPath } = {}) {
}
if (receiptPath) patch.receipt_path = receiptPath;
await db('expenses').where({ id }).update(patch);
await logActivity('expense_updated', { expenseId: id }, adminId);
await logActivity('expense_updated', { expenseId: id }, null, adminActor(adminId));
return getExpense(id);
}
@@ -787,7 +796,7 @@ async function rebillExpense(id, payload, adminId, trx0) {
status: 'invoiced',
updated_at: new Date(),
});
await logActivity('expense_invoiced', { expenseId: id, invoiceId }, adminId);
await logActivity('expense_invoiced', { expenseId: id, invoiceId }, null, adminActor(adminId));
return invoiceId;
};
const invoiceId = trx0 ? await run(trx0) : await db.transaction(run);
@@ -807,7 +816,7 @@ async function markExpensePaid(id, { paid, paidAt, paymentMethod, paymentReferen
payment_reference: paid ? (paymentReference || null) : null,
updated_at: new Date(),
});
await logActivity('expense_paid', { expenseId: id, paid: !!paid }, adminId);
await logActivity('expense_paid', { expenseId: id, paid: !!paid }, null, adminActor(adminId));
return getExpense(id);
}