Only the author or an admin can edit a log entry
PATCH /log/:logid had no check, so any account could edit the tag, note, label and bucket of any entry -- including loads whose rows it cannot see. That reads as harmless annotation and is not: label and bucket name the pivot's columns for everyone in the version, so a rep could rename the company's segments. Same rule as undo now, author or admin, with the fields shown read-only rather than editable-then-403 -- in the change log's tag and note cells and on the Baseline page's label and bucket. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
61268f2a7a
commit
39b2a7e4a2
10
CLAUDE.md
10
CLAUDE.md
@ -380,9 +380,13 @@ Enforced at:
|
|||||||
- every operation, through `sliceUnits()`
|
- every operation, through `sliceUnits()`
|
||||||
- `/sources/:id/values/:col` — completion reads the *source* table, which no
|
- `/sources/:id/values/:col` — completion reads the *source* table, which no
|
||||||
scope has touched, so without it a dropdown enumerates the whole business
|
scope has touched, so without it a dropdown enumerates the whole business
|
||||||
- `DELETE /log/:logid` — by owner, not territory: undo removes an entry's rows
|
- `DELETE /log/:logid` and `PATCH /log/:logid` — by owner, not territory. Undo
|
||||||
wholesale, and half-undoing one would leave a state nothing describes. Your
|
removes an entry's rows wholesale, and half-undoing one would leave a state
|
||||||
own entries, or an admin's override.
|
nothing describes. The PATCH looks like a private annotation and is not:
|
||||||
|
`label` and `bucket` name the pivot's columns for everyone in the version, so
|
||||||
|
unguarded it let any account rename the company's segments. Your own entries,
|
||||||
|
or an admin's override, and the UI greys out the rest rather than offering a
|
||||||
|
click that answers 403.
|
||||||
- recode's `set` — a scoped account cannot set the territory column at all.
|
- recode's `set` — a scoped account cannot set the territory column at all.
|
||||||
Moving a row between territories is reassignment, not forecasting, and it
|
Moving a row between territories is reassignment, not forecasting, and it
|
||||||
would vanish from the view that would have shown what happened.
|
would vanish from the view that would have shown what happened.
|
||||||
|
|||||||
@ -190,6 +190,21 @@ module.exports = function(pool) {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
try {
|
try {
|
||||||
|
// Same rule as undo: your own entries, or an admin's. These are
|
||||||
|
// annotations, but label and bucket name the pivot's columns for
|
||||||
|
// everyone who opens the version, so an unguarded PATCH let any
|
||||||
|
// account rename the company's segments -- including on loads whose
|
||||||
|
// rows it cannot see.
|
||||||
|
const owner = await pool.query(
|
||||||
|
`SELECT pf_user FROM pf.log WHERE id = $1`, [logId]
|
||||||
|
);
|
||||||
|
if (!owner.rows.length) return res.status(404).json({ error: 'Log entry not found' });
|
||||||
|
if (!req.session?.user?.is_admin && owner.rows[0].pf_user !== sessionUser(req)) {
|
||||||
|
return res.status(403).json({
|
||||||
|
error: `That entry was made by ${owner.rows[0].pf_user || 'someone else'} — only they or an administrator can change it`
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
// COALESCE on the flag, not the value: an explicit null or '' must be
|
// COALESCE on the flag, not the value: an explicit null or '' must be
|
||||||
// able to clear a field, which COALESCE on the value alone would ignore
|
// able to clear a field, which COALESCE on the value alone would ignore
|
||||||
const result = await pool.query(
|
const result = await pool.query(
|
||||||
|
|||||||
@ -1,5 +1,6 @@
|
|||||||
import { useState, useEffect } from 'react'
|
import { useState, useEffect } from 'react'
|
||||||
import Timeline from '../components/Timeline.jsx'
|
import Timeline from '../components/Timeline.jsx'
|
||||||
|
import useAuth from '../auth.jsx'
|
||||||
|
|
||||||
const OPERATORS = ['BETWEEN', '=', '!=', 'IN', 'NOT IN', 'IS NULL', 'IS NOT NULL']
|
const OPERATORS = ['BETWEEN', '=', '!=', 'IN', 'NOT IN', 'IS NULL', 'IS NOT NULL']
|
||||||
|
|
||||||
@ -86,6 +87,7 @@ function normalizeFilters(stored) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export default function Baseline({ sources = [], sourceId, versions = [], versionId, setVersionId, refreshVersions }) {
|
export default function Baseline({ sources = [], sourceId, versions = [], versionId, setVersionId, refreshVersions }) {
|
||||||
|
const { user: me } = useAuth()
|
||||||
const [filterCols, setFilterCols] = useState([])
|
const [filterCols, setFilterCols] = useState([])
|
||||||
const [log, setLog] = useState([])
|
const [log, setLog] = useState([])
|
||||||
|
|
||||||
@ -149,6 +151,11 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio
|
|||||||
// Both are only read at load time, hence the reload in the confirmation: the
|
// Both are only read at load time, hence the reload in the confirmation: the
|
||||||
// label is part of the aggregated row the pivot holds, not something it can
|
// label is part of the aggregated row the pivot holds, not something it can
|
||||||
// re-derive in place.
|
// re-derive in place.
|
||||||
|
// Same rule the server enforces: your own entries, or an admin's. A segment's
|
||||||
|
// label and bucket name the pivot's columns for everyone in the version, so
|
||||||
|
// they are not the private annotation they look like.
|
||||||
|
const canEdit = (entry) => !!me && (me.is_admin || entry.pf_user === me.username)
|
||||||
|
|
||||||
async function saveLogField(entry, field, value) {
|
async function saveLogField(entry, field, value) {
|
||||||
const next = value.trim()
|
const next = value.trim()
|
||||||
if (next === (entry[field] || '')) return
|
if (next === (entry[field] || '')) return
|
||||||
@ -516,6 +523,8 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio
|
|||||||
defaultValue={entry.label || ''}
|
defaultValue={entry.label || ''}
|
||||||
key={`label-${entry.id}-${entry.label || ''}`}
|
key={`label-${entry.id}-${entry.label || ''}`}
|
||||||
onBlur={e => saveLogField(entry, 'label', e.target.value)}
|
onBlur={e => saveLogField(entry, 'label', e.target.value)}
|
||||||
|
readOnly={!canEdit(entry)}
|
||||||
|
title={canEdit(entry) ? '' : `${entry.pf_user || 'Another account'} made this segment`}
|
||||||
placeholder={entry.tag || entry.note || '—'}
|
placeholder={entry.tag || entry.note || '—'}
|
||||||
className="w-full border border-transparent hover:border-gray-200
|
className="w-full border border-transparent hover:border-gray-200
|
||||||
focus:border-blue-400 rounded px-1 py-0.5 text-xs
|
focus:border-blue-400 rounded px-1 py-0.5 text-xs
|
||||||
@ -539,6 +548,8 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio
|
|||||||
list="pf-bucket-options"
|
list="pf-bucket-options"
|
||||||
onChange={e => setBuckets(b => ({ ...b, [entry.id]: e.target.value }))}
|
onChange={e => setBuckets(b => ({ ...b, [entry.id]: e.target.value }))}
|
||||||
onBlur={e => saveLogField(entry, 'bucket', e.target.value)}
|
onBlur={e => saveLogField(entry, 'bucket', e.target.value)}
|
||||||
|
readOnly={!canEdit(entry)}
|
||||||
|
title={canEdit(entry) ? '' : `${entry.pf_user || 'Another account'} made this segment`}
|
||||||
placeholder="—"
|
placeholder="—"
|
||||||
className="w-full border border-transparent hover:border-gray-200 focus:border-blue-400
|
className="w-full border border-transparent hover:border-gray-200 focus:border-blue-400
|
||||||
rounded px-1 py-0.5 text-xs focus:outline-none bg-transparent" />
|
rounded px-1 py-0.5 text-xs focus:outline-none bg-transparent" />
|
||||||
|
|||||||
@ -1932,7 +1932,7 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio
|
|||||||
{fmtSliceSummary(entry.slice)}
|
{fmtSliceSummary(entry.slice)}
|
||||||
</button>
|
</button>
|
||||||
</td>
|
</td>
|
||||||
<LogCell entry={entry} field="tag" placeholder="add tag"
|
<LogCell entry={entry} field="tag" canEdit={canUndo(entry)} placeholder="add tag"
|
||||||
editing={editingCell} setEditing={setEditingCell} onSave={saveLogField}
|
editing={editingCell} setEditing={setEditingCell} onSave={saveLogField}
|
||||||
listId="pf-tag-options"
|
listId="pf-tag-options"
|
||||||
render={(v) => (
|
render={(v) => (
|
||||||
@ -1940,7 +1940,7 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio
|
|||||||
{v}
|
{v}
|
||||||
</span>
|
</span>
|
||||||
)} />
|
)} />
|
||||||
<LogCell entry={entry} field="note" placeholder="add note"
|
<LogCell entry={entry} field="note" canEdit={canUndo(entry)} placeholder="add note"
|
||||||
editing={editingCell} setEditing={setEditingCell} onSave={saveLogField} />
|
editing={editingCell} setEditing={setEditingCell} onSave={saveLogField} />
|
||||||
<td className="px-3 py-2 text-gray-500 truncate" title={entry.pf_user || ''}>
|
<td className="px-3 py-2 text-gray-500 truncate" title={entry.pf_user || ''}>
|
||||||
{entry.pf_user || '—'}
|
{entry.pf_user || '—'}
|
||||||
@ -2117,10 +2117,24 @@ function PanelChrome({ dock, setDock, floating, onMouseDown, onClose }) {
|
|||||||
|
|
||||||
// One inline-editable annotation cell in the change log. Click to edit, Enter to
|
// One inline-editable annotation cell in the change log. Click to edit, Enter to
|
||||||
// save, Escape to cancel — the same gesture for note and tag.
|
// save, Escape to cancel — the same gesture for note and tag.
|
||||||
function LogCell({ entry, field, placeholder, editing, setEditing, onSave, listId, render }) {
|
function LogCell({ entry, field, placeholder, editing, setEditing, onSave, listId, render, canEdit = true }) {
|
||||||
const active = editing?.id === entry.id && editing?.field === field
|
const active = canEdit && editing?.id === entry.id && editing?.field === field
|
||||||
const value = entry[field] || ''
|
const value = entry[field] || ''
|
||||||
|
|
||||||
|
// Someone else's entry: shown, not editable. label and bucket name the
|
||||||
|
// pivot's columns for everyone, so these are not the private annotations
|
||||||
|
// they look like.
|
||||||
|
if (!canEdit) {
|
||||||
|
return (
|
||||||
|
<td className="px-3 py-2 text-gray-400 overflow-hidden">
|
||||||
|
<span className="block truncate px-1 -mx-1"
|
||||||
|
title={value ? `${value} — ${entry.pf_user || 'another account'}'s entry` : ''}>
|
||||||
|
{value ? (render ? render(value) : value) : <span className="text-gray-300">—</span>}
|
||||||
|
</span>
|
||||||
|
</td>
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
if (active) {
|
if (active) {
|
||||||
return (
|
return (
|
||||||
<td className="px-3 py-2">
|
<td className="px-3 py-2">
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user