fix(projects): stop the cockpit offering email controls the API rejects (#976)
Closes #969. The cockpit's email feed rendered preview/resend/cancel/retry/send-now for every mail, consulting neither the caller's role nor their permissions, producing controls that always failed: 404 - requireOwnedQueuedEmail scopes queued mail through email_queue.event_id AND ownership of that event. CRM document mail carries no event_id; and project ownership does not imply event ownership, so a project the caller owns can hold another admin's event. 403 - preview needs events.view but the four write actions need email.send. getProjectOverview now stamps each email with an authoritative canAct, mirroring filterOwnedEventIds; created_by is selected only for that check and stripped before the response. The cockpit reads canAct and combines it with email.send. A missing canAct reads as false. Regression from the GHSA-93x4 fix in #960/#966, which added the ownership middleware.
This commit is contained in:
@@ -0,0 +1,149 @@
|
|||||||
|
/**
|
||||||
|
* getProjectOverview stamps each email with `canAct` — whether the queued-mail
|
||||||
|
* routes (requireOwnedQueuedEmail) would actually accept an action on it.
|
||||||
|
*
|
||||||
|
* The cockpit used to derive this client-side from `event_id != null`, which is
|
||||||
|
* weaker than the backend rule in a way that still produced dead controls:
|
||||||
|
* requireOwnedQueuedEmail ALSO requires ownership of that event, while
|
||||||
|
* getProjectOverview lists the project's events by project_id alone. Project
|
||||||
|
* ownership does not imply event ownership — ownedProjectsSubquery's
|
||||||
|
* `projects.created_by = admin.id` branch places no constraint on the linked
|
||||||
|
* events' owners, so a super_admin can attach admin B's event to admin A's
|
||||||
|
* project. See #969 / codex review round 1.
|
||||||
|
*/
|
||||||
|
|
||||||
|
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-canact-')), 'db.sqlite',
|
||||||
|
);
|
||||||
|
process.env.JWT_SECRET = process.env.JWT_SECRET || 'canact-test-secret';
|
||||||
|
|
||||||
|
const bcrypt = require('bcrypt');
|
||||||
|
const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
|
||||||
|
|
||||||
|
describe('getProjectOverview email canAct (#969)', () => {
|
||||||
|
let db; let cleanup; let projectService;
|
||||||
|
let adminA; let adminB; let superAdmin;
|
||||||
|
let projectId; let ownEventId; let foreignEventId; let ownerlessEventId;
|
||||||
|
|
||||||
|
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().toISOString(), updated_at: new Date().toISOString(),
|
||||||
|
}).returning('id');
|
||||||
|
return r[0]?.id ?? r[0];
|
||||||
|
};
|
||||||
|
|
||||||
|
const mkEvent = async (slug, createdBy, project) => {
|
||||||
|
const r = await db('events').insert({
|
||||||
|
slug, event_type: 'wedding', event_name: slug, event_date: '2026-08-01',
|
||||||
|
host_email: '[email protected]', admin_email: '[email protected]', password_hash: 'x',
|
||||||
|
share_token: `t-${slug}`, share_link: `/g/${slug}/t-${slug}`,
|
||||||
|
created_by: createdBy, project_id: project,
|
||||||
|
expires_at: new Date(Date.now() + 864e5).toISOString(),
|
||||||
|
is_active: 1, is_archived: 0, is_draft: 0,
|
||||||
|
created_at: new Date().toISOString(),
|
||||||
|
}).returning('id');
|
||||||
|
return r[0]?.id ?? r[0];
|
||||||
|
};
|
||||||
|
|
||||||
|
const mkMail = async (eventId, type) => {
|
||||||
|
const r = await db('email_queue').insert({
|
||||||
|
recipient_email: '[email protected]', email_type: type, status: 'sent',
|
||||||
|
event_id: eventId,
|
||||||
|
created_at: new Date().toISOString(), sent_at: new Date().toISOString(),
|
||||||
|
}).returning('id');
|
||||||
|
return r[0]?.id ?? r[0];
|
||||||
|
};
|
||||||
|
|
||||||
|
beforeAll(async () => {
|
||||||
|
({ db, cleanup } = await bootCrmDb());
|
||||||
|
await seedMinimal(db);
|
||||||
|
projectService = require('../../src/services/projectService');
|
||||||
|
|
||||||
|
adminA = await mkAdmin('canact-a', 'editor');
|
||||||
|
adminB = await mkAdmin('canact-b', 'editor');
|
||||||
|
superAdmin = await mkAdmin('canact-root', 'super_admin');
|
||||||
|
|
||||||
|
const p = await db('projects').insert({
|
||||||
|
name: 'Cockpit canAct', status: 'active', created_by: adminA,
|
||||||
|
created_at: new Date().toISOString(), updated_at: new Date().toISOString(),
|
||||||
|
}).returning('id');
|
||||||
|
projectId = p[0]?.id ?? p[0];
|
||||||
|
|
||||||
|
// All three hang off adminA's project. Only the first is adminA's; the
|
||||||
|
// third is an ownerless legacy row, which filterOwnedEventIds treats as
|
||||||
|
// owned by whoever asks — but only once we know who is asking.
|
||||||
|
ownEventId = await mkEvent('canact-own', adminA, projectId);
|
||||||
|
foreignEventId = await mkEvent('canact-foreign', adminB, projectId);
|
||||||
|
ownerlessEventId = await mkEvent('canact-legacy', null, projectId);
|
||||||
|
|
||||||
|
await mkMail(ownEventId, 'gallery_ready');
|
||||||
|
await mkMail(foreignEventId, 'gallery_ready');
|
||||||
|
await mkMail(ownerlessEventId, 'gallery_ready');
|
||||||
|
}, 120000);
|
||||||
|
|
||||||
|
afterAll(async () => { if (cleanup) await cleanup(); });
|
||||||
|
|
||||||
|
const byEvent = (overview) => {
|
||||||
|
const m = new Map();
|
||||||
|
for (const e of overview.emails) m.set(e.eventId, e);
|
||||||
|
return m;
|
||||||
|
};
|
||||||
|
|
||||||
|
it('clears mail on an event the caller owns', async () => {
|
||||||
|
const overview = await projectService.getProjectOverview(
|
||||||
|
projectId, {}, { id: adminA, roleName: 'editor' },
|
||||||
|
);
|
||||||
|
expect(byEvent(overview).get(ownEventId).canAct).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('denies mail on a foreign admin\'s event inside the caller\'s own project', async () => {
|
||||||
|
const overview = await projectService.getProjectOverview(
|
||||||
|
projectId, {}, { id: adminA, roleName: 'editor' },
|
||||||
|
);
|
||||||
|
// event_id is non-null here — the old client-side rule would have offered
|
||||||
|
// controls, and requireOwnedQueuedEmail would have 404'd them.
|
||||||
|
const row = byEvent(overview).get(foreignEventId);
|
||||||
|
expect(row.eventId).not.toBeNull();
|
||||||
|
expect(row.canAct).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('clears everything for a super_admin', async () => {
|
||||||
|
const overview = await projectService.getProjectOverview(
|
||||||
|
projectId, {}, { id: superAdmin, roleName: 'super_admin' },
|
||||||
|
);
|
||||||
|
expect(overview.emails.every((e) => e.canAct === true)).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('clears mail on an ownerless legacy event for an identified caller', async () => {
|
||||||
|
// Parity with filterOwnedEventIds, which allows created_by IS NULL.
|
||||||
|
const overview = await projectService.getProjectOverview(
|
||||||
|
projectId, {}, { id: adminA, roleName: 'editor' },
|
||||||
|
);
|
||||||
|
expect(byEvent(overview).get(ownerlessEventId).canAct).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('denies everything when no admin context is supplied', async () => {
|
||||||
|
// Including the ownerless event: `created_by == null` must not read as
|
||||||
|
// "owned" when we do not know who is asking (codex review round 2).
|
||||||
|
const overview = await projectService.getProjectOverview(projectId, {});
|
||||||
|
expect(overview.emails.length).toBe(3);
|
||||||
|
expect(overview.emails.every((e) => e.canAct === false)).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not leak event ownership to the client', async () => {
|
||||||
|
const overview = await projectService.getProjectOverview(
|
||||||
|
projectId, {}, { id: adminA, roleName: 'editor' },
|
||||||
|
);
|
||||||
|
expect(overview.events.length).toBe(3);
|
||||||
|
for (const e of overview.events) expect(e).not.toHaveProperty('created_by');
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -183,7 +183,7 @@ router.get('/:id/overview', requirePermission('events.view'), requireProjectOwne
|
|||||||
quotes: await userHasAnyPermission(req.admin.id, ['quotes.view']),
|
quotes: await userHasAnyPermission(req.admin.id, ['quotes.view']),
|
||||||
contracts: await userHasAnyPermission(req.admin.id, ['contracts.view']),
|
contracts: await userHasAnyPermission(req.admin.id, ['contracts.view']),
|
||||||
};
|
};
|
||||||
const overview = await projectService.getProjectOverview(parseInt(req.params.id, 10), perms);
|
const overview = await projectService.getProjectOverview(parseInt(req.params.id, 10), perms, req.admin);
|
||||||
return successResponse(res, overview);
|
return successResponse(res, overview);
|
||||||
}));
|
}));
|
||||||
|
|
||||||
|
|||||||
@@ -454,16 +454,48 @@ function computeValuation(invoices = [], quotes = []) {
|
|||||||
* Full overview aggregation for the cockpit. Returns the project, its events,
|
* Full overview aggregation for the cockpit. Returns the project, its events,
|
||||||
* and the rolled-up emails / quotes / contracts / invoices / hours + a
|
* and the rolled-up emails / quotes / contracts / invoices / hours + a
|
||||||
* timeline of milestones. `perms` gates which doc types are included.
|
* timeline of milestones. `perms` gates which doc types are included.
|
||||||
|
*
|
||||||
|
* `admin` (optional) is used only to stamp each email with `canAct` — whether
|
||||||
|
* the queued-mail routes would actually accept an action on it. See below.
|
||||||
*/
|
*/
|
||||||
async function getProjectOverview(id, perms = {}) {
|
async function getProjectOverview(id, perms = {}, admin = null) {
|
||||||
const project = await getProjectById(id);
|
const project = await getProjectById(id);
|
||||||
if (!project) throw new AppError('Project not found', 404);
|
if (!project) throw new AppError('Project not found', 404);
|
||||||
|
|
||||||
const events = await db('events')
|
// `created_by` is selected for the ownership check below and stripped again
|
||||||
|
// before the response — the cockpit has no business learning who owns a
|
||||||
|
// sibling event.
|
||||||
|
const eventRows = await db('events')
|
||||||
.where({ project_id: id })
|
.where({ project_id: id })
|
||||||
.select('id', 'event_name', 'event_date', 'slug', 'is_active', 'is_draft', 'expires_at', 'is_archived');
|
.select('id', 'event_name', 'event_date', 'slug', 'is_active', 'is_draft', 'expires_at', 'is_archived', 'created_by');
|
||||||
|
const events = eventRows.map(({ created_by: _ignored, ...e }) => e);
|
||||||
const eventIds = events.map((e) => e.id);
|
const eventIds = events.map((e) => e.id);
|
||||||
|
|
||||||
|
// Which of this project's events would filterOwnedEventIds() let `admin`
|
||||||
|
// act on. Mirrors that predicate exactly (ownership.js): super_admin gets
|
||||||
|
// everything, otherwise created_by IS NULL OR created_by = admin.id.
|
||||||
|
//
|
||||||
|
// Project ownership does NOT imply event ownership — ownedProjectsSubquery's
|
||||||
|
// `projects.created_by = admin.id` branch places no constraint on who owns
|
||||||
|
// the linked events, so a super_admin can attach admin B's event to admin
|
||||||
|
// A's project. Deriving actionability from `event_id != null` alone (as the
|
||||||
|
// UI first did) would then still render controls that requireOwnedQueuedEmail
|
||||||
|
// rejects with a 404.
|
||||||
|
// No admin context → nothing is actionable. Without this, an ownerless
|
||||||
|
// (legacy/system) event would satisfy `created_by == null` and be marked
|
||||||
|
// actionable for a caller we know nothing about.
|
||||||
|
const isSuperAdmin = admin?.roleName === 'super_admin';
|
||||||
|
let actionableEventIds = new Set();
|
||||||
|
if (isSuperAdmin) {
|
||||||
|
actionableEventIds = new Set(eventIds);
|
||||||
|
} else if (admin?.id != null) {
|
||||||
|
actionableEventIds = new Set(
|
||||||
|
eventRows
|
||||||
|
.filter((e) => e.created_by == null || Number(e.created_by) === Number(admin.id))
|
||||||
|
.map((e) => e.id),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
const out = { project, events, emails: [], quotes: [], contracts: [], invoices: [], hours: { entries: [], totalMinutes: 0 } };
|
const out = { project, events, emails: [], quotes: [], contracts: [], invoices: [], hours: { entries: [], totalMinutes: 0 } };
|
||||||
|
|
||||||
// Invoices (by event) incl. storno.
|
// Invoices (by event) incl. storno.
|
||||||
@@ -518,6 +550,11 @@ async function getProjectOverview(id, perms = {}) {
|
|||||||
queuedAt: e.created_at, sentAt: e.sent_at, error: e.error_message, eventId: e.event_id,
|
queuedAt: e.created_at, sentAt: e.sent_at, error: e.error_message, eventId: e.event_id,
|
||||||
// false → the cockpit preview will re-render from the current template.
|
// false → the cockpit preview will re-render from the current template.
|
||||||
stored: !!Number(e.has_rendered),
|
stored: !!Number(e.has_rendered),
|
||||||
|
// Would requireOwnedQueuedEmail accept preview/resend/cancel/retry/send-now
|
||||||
|
// on this row? Authoritative here because the client cannot derive it: CRM
|
||||||
|
// document mail has no event to own, and event mail additionally requires
|
||||||
|
// ownership of THAT event, which the response deliberately does not expose.
|
||||||
|
canAct: isSuperAdmin || (e.event_id != null && actionableEventIds.has(e.event_id)),
|
||||||
});
|
});
|
||||||
|
|
||||||
const emailRows = [];
|
const emailRows = [];
|
||||||
|
|||||||
@@ -27,6 +27,7 @@ import { useLocalizedDate } from '../../../hooks/useLocalizedDate';
|
|||||||
import { useMutationWithToast } from '../../../hooks';
|
import { useMutationWithToast } from '../../../hooks';
|
||||||
import { formatMoneyMinor } from '../../../utils/money';
|
import { formatMoneyMinor } from '../../../utils/money';
|
||||||
import { useFeatureFlags, type FeatureKey } from '../../../contexts/FeatureFlagsContext';
|
import { useFeatureFlags, type FeatureKey } from '../../../contexts/FeatureFlagsContext';
|
||||||
|
import { usePermissions } from '../../../contexts/PermissionsContext';
|
||||||
|
|
||||||
type FeedKind = 'email' | 'quote' | 'contract' | 'invoice' | 'gallery' | 'hours';
|
type FeedKind = 'email' | 'quote' | 'contract' | 'invoice' | 'gallery' | 'hours';
|
||||||
|
|
||||||
@@ -42,6 +43,9 @@ interface FeedItem {
|
|||||||
emailId?: number;
|
emailId?: number;
|
||||||
emailStatus?: string;
|
emailStatus?: string;
|
||||||
reRendered?: boolean;
|
reRendered?: boolean;
|
||||||
|
/** Server's verdict on whether the queued-mail routes would accept an action
|
||||||
|
* on this row. Not derivable client-side — see canActOnEmail. */
|
||||||
|
emailCanAct?: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** The feature flag that gates each document's detail ROUTE (RequireFeature
|
/** The feature flag that gates each document's detail ROUTE (RequireFeature
|
||||||
@@ -109,8 +113,23 @@ export const ProjectCockpitPage: React.FC = () => {
|
|||||||
const navigate = useNavigate();
|
const navigate = useNavigate();
|
||||||
const qc = useQueryClient();
|
const qc = useQueryClient();
|
||||||
const { flags } = useFeatureFlags();
|
const { flags } = useFeatureFlags();
|
||||||
|
const { hasPermission } = usePermissions();
|
||||||
const { format, formatTime } = useLocalizedDate();
|
const { format, formatTime } = useLocalizedDate();
|
||||||
|
|
||||||
|
// Which email controls are actually reachable for THIS admin, so the feed
|
||||||
|
// stops offering buttons the API will reject (#969):
|
||||||
|
// 404 — requireOwnedQueuedEmail (adminProjects.js) scopes queued mail
|
||||||
|
// through email_queue.event_id AND ownership of that event. CRM
|
||||||
|
// document mail carries no event_id at all. The server decides this
|
||||||
|
// for us (`canAct`) because neither half is derivable here: the
|
||||||
|
// overview deliberately does not expose who owns a sibling event.
|
||||||
|
// Absent (older backend) → treat as not actionable, so we fail to
|
||||||
|
// hiding a live control rather than offering a dead one.
|
||||||
|
// 403 — preview needs `events.view`, but resend/cancel/retry/send-now
|
||||||
|
// need `email.send`; the feed used to render all four regardless.
|
||||||
|
const canActOnEmail = (item: FeedItem) => item.emailCanAct === true;
|
||||||
|
const canSendEmail = hasPermission('email.send');
|
||||||
|
|
||||||
const [editName, setEditName] = useState<string | null>(null);
|
const [editName, setEditName] = useState<string | null>(null);
|
||||||
const [preview, setPreview] = useState<EmailPreview | null>(null);
|
const [preview, setPreview] = useState<EmailPreview | null>(null);
|
||||||
const [previewLoading, setPreviewLoading] = useState(false);
|
const [previewLoading, setPreviewLoading] = useState(false);
|
||||||
@@ -189,6 +208,7 @@ export const ProjectCockpitPage: React.FC = () => {
|
|||||||
title: t(`projects.feed.email`, 'Email') + ` · ${e.type}`,
|
title: t(`projects.feed.email`, 'Email') + ` · ${e.type}`,
|
||||||
subtitle: e.recipient + (e.error ? ` — ${e.error}` : ''),
|
subtitle: e.recipient + (e.error ? ` — ${e.error}` : ''),
|
||||||
status: e.status, emailId: e.id, emailStatus: e.status, reRendered: !e.stored,
|
status: e.status, emailId: e.id, emailStatus: e.status, reRendered: !e.stored,
|
||||||
|
emailCanAct: e.canAct,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
for (const q of data.quotes) {
|
for (const q of data.quotes) {
|
||||||
@@ -376,7 +396,9 @@ export const ProjectCockpitPage: React.FC = () => {
|
|||||||
// not a dead strip next to them). Hours have neither → static.
|
// not a dead strip next to them). Hours have neither → static.
|
||||||
const onRowClick = item.href
|
const onRowClick = item.href
|
||||||
? () => navigate(item.href as string)
|
? () => navigate(item.href as string)
|
||||||
: (item.kind === 'email' && item.emailId != null ? () => openPreview(item.emailId as number) : undefined);
|
: (item.kind === 'email' && item.emailId != null && canActOnEmail(item)
|
||||||
|
? () => openPreview(item.emailId as number)
|
||||||
|
: undefined);
|
||||||
return (
|
return (
|
||||||
<li
|
<li
|
||||||
key={item.key}
|
key={item.key}
|
||||||
@@ -397,7 +419,7 @@ export const ProjectCockpitPage: React.FC = () => {
|
|||||||
<span className="inline-block rounded-full px-2 py-0.5 text-xs bg-neutral-100 dark:bg-neutral-700 text-neutral-600 dark:text-neutral-300">{item.status}</span>
|
<span className="inline-block rounded-full px-2 py-0.5 text-xs bg-neutral-100 dark:bg-neutral-700 text-neutral-600 dark:text-neutral-300">{item.status}</span>
|
||||||
)}
|
)}
|
||||||
{item.amount && <span className="text-xs font-medium text-neutral-700 dark:text-neutral-300">{item.amount}</span>}
|
{item.amount && <span className="text-xs font-medium text-neutral-700 dark:text-neutral-300">{item.amount}</span>}
|
||||||
{item.kind === 'email' && item.emailId != null && (
|
{item.kind === 'email' && item.emailId != null && canActOnEmail(item) && (
|
||||||
<div className="flex items-center gap-1" onClick={(e) => e.stopPropagation()}>
|
<div className="flex items-center gap-1" onClick={(e) => e.stopPropagation()}>
|
||||||
<button onClick={() => openPreview(item.emailId as number)} className="inline-flex items-center gap-1 text-xs text-primary-600 hover:underline">
|
<button onClick={() => openPreview(item.emailId as number)} className="inline-flex items-center gap-1 text-xs text-primary-600 hover:underline">
|
||||||
<Eye className="w-3 h-3" />{t('projects.email.preview', 'Preview')}
|
<Eye className="w-3 h-3" />{t('projects.email.preview', 'Preview')}
|
||||||
@@ -410,12 +432,12 @@ export const ProjectCockpitPage: React.FC = () => {
|
|||||||
{t('projects.email.reRenderedTag', '≈ re-rendered')}
|
{t('projects.email.reRenderedTag', '≈ re-rendered')}
|
||||||
</span>
|
</span>
|
||||||
)}
|
)}
|
||||||
{item.emailStatus === 'sent' && (
|
{canSendEmail && item.emailStatus === 'sent' && (
|
||||||
<button onClick={() => emailActionMutation.mutate({ action: 'resend', emailId: item.emailId as number })} className="inline-flex items-center gap-1 text-xs text-neutral-600 dark:text-neutral-300 hover:underline">
|
<button onClick={() => emailActionMutation.mutate({ action: 'resend', emailId: item.emailId as number })} className="inline-flex items-center gap-1 text-xs text-neutral-600 dark:text-neutral-300 hover:underline">
|
||||||
<Send className="w-3 h-3" />{t('projects.email.resend', 'Resend')}
|
<Send className="w-3 h-3" />{t('projects.email.resend', 'Resend')}
|
||||||
</button>
|
</button>
|
||||||
)}
|
)}
|
||||||
{item.emailStatus === 'pending' && (
|
{canSendEmail && item.emailStatus === 'pending' && (
|
||||||
<>
|
<>
|
||||||
<button onClick={() => emailActionMutation.mutate({ action: 'sendNow', emailId: item.emailId as number })} className="inline-flex items-center gap-1 text-xs text-neutral-600 dark:text-neutral-300 hover:underline">
|
<button onClick={() => emailActionMutation.mutate({ action: 'sendNow', emailId: item.emailId as number })} className="inline-flex items-center gap-1 text-xs text-neutral-600 dark:text-neutral-300 hover:underline">
|
||||||
<Send className="w-3 h-3" />{t('projects.email.sendNow', 'Send now')}
|
<Send className="w-3 h-3" />{t('projects.email.sendNow', 'Send now')}
|
||||||
@@ -425,7 +447,7 @@ export const ProjectCockpitPage: React.FC = () => {
|
|||||||
</button>
|
</button>
|
||||||
</>
|
</>
|
||||||
)}
|
)}
|
||||||
{item.emailStatus === 'failed' && (
|
{canSendEmail && item.emailStatus === 'failed' && (
|
||||||
<button onClick={() => emailActionMutation.mutate({ action: 'retry', emailId: item.emailId as number })} className="inline-flex items-center gap-1 text-xs text-amber-600 hover:underline">
|
<button onClick={() => emailActionMutation.mutate({ action: 'retry', emailId: item.emailId as number })} className="inline-flex items-center gap-1 text-xs text-amber-600 hover:underline">
|
||||||
<RotateCw className="w-3 h-3" />{t('projects.email.retry', 'Retry')}
|
<RotateCw className="w-3 h-3" />{t('projects.email.retry', 'Retry')}
|
||||||
</button>
|
</button>
|
||||||
|
|||||||
@@ -0,0 +1,158 @@
|
|||||||
|
/**
|
||||||
|
* The cockpit's email feed used to render preview/resend/cancel/retry/send-now
|
||||||
|
* for every mail, consulting neither the caller's role nor their permissions.
|
||||||
|
* Two ways that produced dead controls (#969):
|
||||||
|
*
|
||||||
|
* 404 — requireOwnedQueuedEmail scopes queued mail through
|
||||||
|
* email_queue.event_id. CRM document mail (quote/contract/invoice/
|
||||||
|
* storno) is queued with event_id = null, so for a non-super_admin it
|
||||||
|
* has no ownable parent and always 404s. Introduced by the GHSA-93x4
|
||||||
|
* fix in #960/#966, which added the ownership middleware.
|
||||||
|
* 403 — preview requires `events.view` but the four write actions require
|
||||||
|
* `email.send`; the feed rendered all of them regardless.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { render, screen, waitFor } from '@testing-library/react';
|
||||||
|
import { vi, describe, it, expect, beforeEach } from 'vitest';
|
||||||
|
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
|
||||||
|
import { MemoryRouter, Routes, Route } from 'react-router-dom';
|
||||||
|
|
||||||
|
vi.mock('react-i18next', async () => {
|
||||||
|
const actual = await vi.importActual<typeof import('react-i18next')>('react-i18next');
|
||||||
|
return {
|
||||||
|
...actual,
|
||||||
|
useTranslation: () => ({
|
||||||
|
t: (k: string, fb?: unknown) => (typeof fb === 'string' ? fb : k),
|
||||||
|
i18n: { language: 'en' },
|
||||||
|
}),
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
|
vi.mock('react-toastify', () => ({ toast: { success: vi.fn(), error: vi.fn() } }));
|
||||||
|
|
||||||
|
// An event mail (actionable by anyone who owns the event) and a CRM document
|
||||||
|
// mail (event_id null → only a super_admin can act on it). Both 'sent', so the
|
||||||
|
// Resend button is the one under test in each row.
|
||||||
|
const OVERVIEW = {
|
||||||
|
project: {
|
||||||
|
id: 1, name: 'Hochzeit Müller', customerAccountId: 1,
|
||||||
|
customerEmail: '[email protected]', status: 'active',
|
||||||
|
createdAt: '2026-07-01T00:00:00Z', updatedAt: '2026-08-01T00:00:00Z',
|
||||||
|
},
|
||||||
|
events: [], quotes: [], contracts: [], invoices: [],
|
||||||
|
hours: { entries: [], totalMinutes: 0 },
|
||||||
|
milestones: [],
|
||||||
|
valuation: { byCurrency: [] },
|
||||||
|
emails: [
|
||||||
|
// Event mail the caller owns → the server says actionable.
|
||||||
|
{ id: 11, recipient: '[email protected]', type: 'gallery_ready', status: 'sent',
|
||||||
|
queuedAt: '2026-08-01T10:00:00Z', sentAt: '2026-08-01T10:00:05Z', error: null,
|
||||||
|
eventId: 42, stored: true, canAct: true },
|
||||||
|
// CRM document mail — no event to own, so 404 for a non-super_admin.
|
||||||
|
{ id: 12, recipient: '[email protected]', type: 'invoice_sent', status: 'sent',
|
||||||
|
queuedAt: '2026-08-01T11:00:00Z', sentAt: '2026-08-01T11:00:05Z', error: null,
|
||||||
|
eventId: null, stored: true, canAct: false },
|
||||||
|
// Event mail for a FOREIGN event: a super_admin can attach admin B's event
|
||||||
|
// to admin A's project, and project ownership does not imply event
|
||||||
|
// ownership — so eventId is non-null yet the action still 404s.
|
||||||
|
{ id: 13, recipient: '[email protected]', type: 'gallery_ready', status: 'sent',
|
||||||
|
queuedAt: '2026-08-01T12:00:00Z', sentAt: '2026-08-01T12:00:05Z', error: null,
|
||||||
|
eventId: 99, stored: true, canAct: false },
|
||||||
|
],
|
||||||
|
};
|
||||||
|
|
||||||
|
/** The same payload as a super_admin sees it — the server stamps every row. */
|
||||||
|
const OVERVIEW_SUPER = {
|
||||||
|
...OVERVIEW,
|
||||||
|
emails: OVERVIEW.emails.map((e) => ({ ...e, canAct: true })),
|
||||||
|
};
|
||||||
|
|
||||||
|
let currentOverview: typeof OVERVIEW = OVERVIEW;
|
||||||
|
|
||||||
|
vi.mock('../../../../services/projects.service', () => ({
|
||||||
|
projectsService: {
|
||||||
|
overview: vi.fn(async () => currentOverview),
|
||||||
|
emailPreview: vi.fn(async () => ({ html: '<p>x</p>', subject: 's' })),
|
||||||
|
resendEmail: vi.fn(), cancelEmail: vi.fn(), retryEmail: vi.fn(), sendEmailNow: vi.fn(),
|
||||||
|
update: vi.fn(), assignEvent: vi.fn(),
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('../../../../services/events.service', () => ({
|
||||||
|
eventsService: { getEvents: vi.fn(async () => []) },
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('../../../../contexts/FeatureFlagsContext', () => ({
|
||||||
|
useFeatureFlags: () => ({ flags: { projects: true, quotes: true, contracts: true, bills: true } }),
|
||||||
|
}));
|
||||||
|
|
||||||
|
let mockPerms = { isSuperAdmin: false, permissions: ['events.view'] };
|
||||||
|
vi.mock('../../../../contexts/PermissionsContext', () => ({
|
||||||
|
usePermissions: () => ({
|
||||||
|
isSuperAdmin: mockPerms.isSuperAdmin,
|
||||||
|
hasPermission: (p: string) => mockPerms.isSuperAdmin || mockPerms.permissions.includes(p),
|
||||||
|
}),
|
||||||
|
}));
|
||||||
|
|
||||||
|
import { ProjectCockpitPage } from '../ProjectCockpitPage';
|
||||||
|
|
||||||
|
const renderPage = async () => {
|
||||||
|
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
||||||
|
render(
|
||||||
|
<QueryClientProvider client={qc}>
|
||||||
|
<MemoryRouter initialEntries={['/admin/projects/1']}>
|
||||||
|
<Routes>
|
||||||
|
<Route path="/admin/projects/:id" element={<ProjectCockpitPage />} />
|
||||||
|
</Routes>
|
||||||
|
</MemoryRouter>
|
||||||
|
</QueryClientProvider>,
|
||||||
|
);
|
||||||
|
await waitFor(() => expect(screen.getAllByText(/Email/).length).toBeGreaterThan(0));
|
||||||
|
};
|
||||||
|
|
||||||
|
describe('ProjectCockpitPage email controls (#969)', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
mockPerms = { isSuperAdmin: false, permissions: ['events.view'] };
|
||||||
|
currentOverview = OVERVIEW;
|
||||||
|
});
|
||||||
|
|
||||||
|
it('super_admin sees controls on every row', async () => {
|
||||||
|
mockPerms = { isSuperAdmin: true, permissions: [] };
|
||||||
|
currentOverview = OVERVIEW_SUPER;
|
||||||
|
await renderPage();
|
||||||
|
// All three rows, Resend on all (super_admin implies email.send).
|
||||||
|
expect(screen.getAllByText('Preview')).toHaveLength(3);
|
||||||
|
expect(screen.getAllByText('Resend')).toHaveLength(3);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('a scoped admin gets controls only on mail the server says it can act on', async () => {
|
||||||
|
mockPerms = { isSuperAdmin: false, permissions: ['events.view', 'email.send'] };
|
||||||
|
await renderPage();
|
||||||
|
// Only the owned event mail. The CRM row (no event) and the foreign-event
|
||||||
|
// row both 404, so neither offers anything.
|
||||||
|
expect(screen.getAllByText('Preview')).toHaveLength(1);
|
||||||
|
expect(screen.getAllByText('Resend')).toHaveLength(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('an admin without email.send sees preview but no write actions (would 403)', async () => {
|
||||||
|
mockPerms = { isSuperAdmin: false, permissions: ['events.view'] };
|
||||||
|
await renderPage();
|
||||||
|
expect(screen.getAllByText('Preview')).toHaveLength(1);
|
||||||
|
expect(screen.queryByText('Resend')).toBeNull();
|
||||||
|
expect(screen.queryByText('Cancel')).toBeNull();
|
||||||
|
expect(screen.queryByText('Retry')).toBeNull();
|
||||||
|
expect(screen.queryByText('Send now')).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('offers nothing when the server omits canAct (older backend)', async () => {
|
||||||
|
mockPerms = { isSuperAdmin: false, permissions: ['events.view', 'email.send'] };
|
||||||
|
currentOverview = {
|
||||||
|
...OVERVIEW,
|
||||||
|
emails: OVERVIEW.emails.map(({ canAct: _drop, ...e }) => e as typeof OVERVIEW.emails[number]),
|
||||||
|
};
|
||||||
|
await renderPage();
|
||||||
|
// Fail to hiding a live control rather than offering a dead one.
|
||||||
|
expect(screen.queryByText('Preview')).toBeNull();
|
||||||
|
expect(screen.queryByText('Resend')).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -49,6 +49,10 @@ export interface ProjectEmail {
|
|||||||
eventId: number | null;
|
eventId: number | null;
|
||||||
/** true = exact HTML stored at send time; false = preview re-rendered. */
|
/** true = exact HTML stored at send time; false = preview re-rendered. */
|
||||||
stored: boolean;
|
stored: boolean;
|
||||||
|
/** Authoritative: would the queued-mail routes accept an action on this row?
|
||||||
|
* False for CRM document mail (no event to own) and for event mail the
|
||||||
|
* caller does not own. Absent on older backends → treated as false. */
|
||||||
|
canAct?: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface ProjectInvoice {
|
export interface ProjectInvoice {
|
||||||
|
|||||||
Reference in New Issue
Block a user