Scope what an account can see and change to its territory

Territory filtering was deferred from v1, so every account saw and could
write every row. With the sales team about to adjust their own territories
that is the thing standing in the way, and it is also why a rep would wait
fifteen seconds to load 2.7M rows to work on a few thousand.

The list lives on pf.app_user.territory with is_admin beside it, and
col_meta.is_territory marks which column of a source the values belong to --
flagged rather than named in code, so a second source can be divided by
something other than a sales rep.

Fail closed: buildTerritoryClause returns FALSE for an empty list or an
unflagged source. An account nobody configured sees nothing, rather than
everything because a column was left null.

Built from the session, never the request. That is what separates it from
`scope`, which the browser sends and should: a filter the user chose belongs
in the payload, a permission cannot come from the thing it restrains. It is
ANDed on last, where nothing in the request can undo it.

Enforced on /data (the cursor and the count behind X-Row-Count), on /agg
before the GROUP BY since the territory column need not be in the grain, on
every operation through sliceUnits, and on the value completion endpoint --
which reads the source table, so without it a dropdown enumerates every
customer and rep in the business to someone shown none of their rows.

Undo is gated by owner rather than territory: it removes an entry's rows
wholesale, so half-undoing one would leave a state nothing describes. Recode
refuses to set the territory column unless you are an admin, since moving a
row between territories is reassignment, not forecasting.

./pf.sh gains set-territory, set-admin and orphan-territory. The last lists
territory values no account owns -- work under one is invisible to everybody
but an admin, which a typo causes easily and nothing in the app reveals.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Paul Trowbridge 2026-09-18 11:14:03 -04:00
parent b173a3ddaf
commit 712d114dc5
10 changed files with 313 additions and 15 deletions

View File

@ -353,6 +353,50 @@ effects call the API immediately.
`pf_user: 'admin'`, which any client could have set to anything. The audit log `pf_user: 'admin'`, which any client could have set to anything. The audit log
now names the account that made the change. now names the account that made the change.
## Territory scoping
An account sees and changes only its own territory. The list lives on
`pf.app_user.territory` (jsonb array) with `is_admin` for the accounts that see
everything, and `col_meta.is_territory` marks which column of a given source
the values belong to — one per source, flagged rather than named in code so a
second source can be divided by something other than a sales rep.
**Fail closed.** No territory and not an admin means no rows.
`buildTerritoryClause()` returns `FALSE`, not `TRUE`, for an empty list or an
unflagged source: an account somebody forgot to configure sees nothing instead
of the whole book.
**Built from the session, never the request.** This is the difference between
it and `scope`, which the browser sends and which is right to send, being a
filter the user chose. A permission cannot come from the thing it restrains, so
the territory predicate is ANDed on last, in `sliceUnits()` for writes and per
route for reads, where nothing in the payload can remove it.
Enforced at:
- `/data` — clause on the cursor *and* on the count behind `X-Row-Count`
- `/agg` — a `{{territory_clause}}` token applied **before** the GROUP BY, since
the territory column need not be part of the grain and may not survive it
- every operation, through `sliceUnits()`
- `/sources/:id/values/:col` — completion reads the *source* table, which no
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
wholesale, and half-undoing one would leave a state nothing describes. Your
own entries, or an admin's override.
- recode's `set` — a scoped account cannot set the territory column at all.
Moving a row between territories is reassignment, not forecasting, and it
would vanish from the view that would have shown what happened.
**The change log shows an entry's full impact**, not the reader's share. The
totals stamped on `pf.log` are company-wide, so an admin's version-wide scale
reads the same in every account — deliberate, and labelled, rather than
re-aggregating per territory.
Managed with `./pf.sh set-territory | set-admin | orphan-territory`.
`orphan-territory` lists values present in the data that no account owns; work
under one is invisible to everyone but an admin, which a typo causes easily and
nothing inside the app reveals.
## Light / dark mode ## Light / dark mode
Theme state lives in `ui/src/theme.jsx` — a React context (`ThemeContext`) with a `ThemeProvider` that wraps the app in `main.jsx`. Theme state lives in `ui/src/theme.jsx` — a React context (`ThemeContext`) with a `ThemeProvider` that wraps the app in `main.jsx`.

View File

@ -50,7 +50,29 @@ function sessionUser(req) {
return req.session?.user?.username || null; return req.session?.user?.username || null;
} }
module.exports = { hashPassword, verifyPassword, requireAuth, sessionUser, SCRYPT }; // What this account may see and change, read from the session for the same
// reason the username is: the browser must not be able to widen it.
//
// Returns { admin: true } for an account that sees everything, or
// { admin: false, values: [...] } for a scoped one. An empty list is a real
// answer meaning "nothing", not a missing one meaning "everything" -- an
// account created without a territory sees no rows until it is granted some.
function sessionTerritory(req) {
const u = req.session?.user;
if (!u) return { admin: false, values: [] };
if (u.is_admin) return { admin: true, values: null };
return { admin: false, values: Array.isArray(u.territory) ? u.territory : [] };
}
function requireAdmin(req, res, next) {
if (req.session?.user?.is_admin) return next();
res.status(403).json({ error: 'Administrator access required' });
}
module.exports = {
hashPassword, verifyPassword, requireAuth, requireAdmin,
sessionUser, sessionTerritory, SCRYPT,
};
// CLI: `node lib/auth.js hash` reads a password on stdin and prints its hash, // CLI: `node lib/auth.js hash` reads a password on stdin and prints its hash,
// so ./pf.sh can create users without the plaintext touching argv or psql. // so ./pf.sh can create users without the plaintext touching argv or psql.

View File

@ -8,7 +8,7 @@
// Tokens baked in at generation time: column names, source schema.table // Tokens baked in at generation time: column names, source schema.table
// Tokens substituted at request time: {{fc_table}}, {{where_clause}}, {{exclude_clause}}, // Tokens substituted at request time: {{fc_table}}, {{where_clause}}, {{exclude_clause}},
// {{version_id}}, {{logid}}, {{pf_user}}, {{note}}, // {{version_id}}, {{logid}}, {{pf_user}}, {{note}},
// {{label}}, {{bucket}}, {{tag}}, // {{label}}, {{bucket}}, {{tag}}, {{territory_clause}},
// {{params}}, {{slice}}, {{date_from}}, {{date_to}}, // {{params}}, {{slice}}, {{date_from}}, {{date_to}},
// {{value_incr}}, {{units_incr}}, {{set_clause}}, {{scale_factor}} // {{value_incr}}, {{units_incr}}, {{set_clause}}, {{scale_factor}}
@ -308,6 +308,7 @@ SELECT
FROM {{fc_table}} t FROM {{fc_table}} t
LEFT JOIN pf.log l LEFT JOIN pf.log l
ON l.id = t.pf_logid${VERSION_JOIN} ON l.id = t.pf_logid${VERSION_JOIN}
WHERE {{territory_clause}}
GROUP BY GROUP BY
${grain.groupCols('t.').join('\n ,')} ${grain.groupCols('t.').join('\n ,')}
,${LABEL_GROUP_COLS.join('\n ,')}`.trim(); ,${LABEL_GROUP_COLS.join('\n ,')}`.trim();
@ -658,6 +659,25 @@ function buildScopeClause(scope, dimCols, versionId) {
return parts.join('\nAND '); return parts.join('\nAND ');
} }
// The territory predicate: what this account may see and change.
//
// Server-side by construction. Today's `scope` is sent by the browser, which is
// right for a filter the user chose and would be fatal for a permission -- so
// this one is built from the session and ANDed on last, where nothing in the
// request can remove it.
//
// FALSE, not TRUE, for an account with no territory. The whole point is that a
// missing grant means no rows: an empty list that fell through to TRUE would
// hand the entire book to the first account somebody forgot to configure.
function buildTerritoryClause(territory, territoryCol, alias = '') {
if (!territory || territory.admin) return '';
if (!territoryCol) return 'FALSE';
const vals = (territory.values || []).filter(v => v != null && v !== '');
if (vals.length === 0) return 'FALSE';
const pfx = alias ? `${alias}.` : '';
return `${pfx}"${territoryCol}" IN (${vals.map(v => `'${esc(String(v))}'`).join(', ')})`;
}
// build a WHERE clause spanning several slices — an OR of AND-groups. // build a WHERE clause spanning several slices — an OR of AND-groups.
// A union of slices cannot be flattened into one IN list per column: slices // A union of slices cannot be flattened into one IN list per column: slices
// {Region:East, State:NY} and {Region:West, State:CA} would become // {Region:East, State:NY} and {Region:West, State:CA} would become
@ -754,6 +774,6 @@ function esc(val) {
return String(val).replace(/'/g, "''"); return String(val).replace(/'/g, "''");
} }
module.exports = { generateSQL, grainOf, COMPUTED_SLICE_COLS, buildScopeClause, module.exports = { generateSQL, grainOf, COMPUTED_SLICE_COLS, buildScopeClause, buildTerritoryClause,
SEGMENT_EXPR, BUCKET_EXPR, NOTE_EXPR, LABEL_GROUP_COLS, VERSION_JOIN, SEGMENT_EXPR, BUCKET_EXPR, NOTE_EXPR, LABEL_GROUP_COLS, VERSION_JOIN,
ADJUSTMENT_SEGMENT, ADJUSTMENT_BUCKET, UNLABELED_LOAD, dateGroupsOf, dimPeriodMapOf, dimPeriodJoins, applyTokens, buildWhere, buildWhereAny, buildExcludeClause, buildExcludePredicate, buildSetClause, buildFilterClause, esc }; ADJUSTMENT_SEGMENT, ADJUSTMENT_BUCKET, UNLABELED_LOAD, dateGroupsOf, dimPeriodMapOf, dimPeriodJoins, applyTokens, buildWhere, buildWhereAny, buildExcludeClause, buildExcludePredicate, buildSetClause, buildFilterClause, esc };

105
pf.sh
View File

@ -5,6 +5,7 @@ set -euo pipefail
# pf.sh — Pivot Forecast management script # pf.sh — Pivot Forecast management script
# Usage: ./pf.sh [deploy|start|stop|restart|status|logs|db-setup|config] # Usage: ./pf.sh [deploy|start|stop|restart|status|logs|db-setup|config]
# ./pf.sh [add-user|passwd|list-users|disable-user|enable-user] # ./pf.sh [add-user|passwd|list-users|disable-user|enable-user]
# ./pf.sh [set-territory|set-admin|orphan-territory]
# ./pf.sh (interactive menu) # ./pf.sh (interactive menu)
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@ -360,11 +361,102 @@ cmd_list_users() {
require_env; load_env require_env; load_env
echo; bold "Accounts" echo; bold "Accounts"
run_psql -c " run_psql -c "
SELECT username, display_name, is_active, SELECT username, display_name, is_active, is_admin,
coalesce(jsonb_array_length(territory), 0) AS territory_values,
to_char(last_login_at, 'YYYY-MM-DD HH24:MI') AS last_login to_char(last_login_at, 'YYYY-MM-DD HH24:MI') AS last_login
FROM pf.app_user ORDER BY username" FROM pf.app_user ORDER BY username"
} }
# Territory is what an account may see and change, as a list of values in the
# source's is_territory column. No territory and not an admin means no rows --
# so a new account is blind until this is run, which is the intended direction
# to fail in.
#
# Values are given comma-separated and have to match the column exactly, since
# that is what the SQL compares. set-territory with no values clears it.
cmd_set_territory() {
require_env; load_env
local username="${1:-}"; shift || true
[[ -z "$username" ]] && { read -rp " Username: " username; }
[[ -z "$username" ]] && die "Username is required."
local values="${*:-}"
[[ -z "$values" ]] && { read -rp " Territory values (comma separated, blank to clear): " values; }
local json="null"
if [[ -n "$values" ]]; then
json=$(python3 - "$values" <<'PYEOF'
import json, sys
vals = [v.strip() for v in sys.argv[1].split(',') if v.strip()]
print(json.dumps(vals))
PYEOF
)
fi
run_psql -v ON_ERROR_STOP=1 -tAc "
WITH upd AS (
UPDATE pf.app_user SET territory = $(if [[ "$json" == "null" ]]; then echo NULL; else echo "'$(sql_lit "$json")'::jsonb"; fi)
WHERE lower(username) = lower('$(sql_lit "$username")')
RETURNING username
)
SELECT count(*) FROM upd" | grep -q '^1$' \
|| die "No such account: $username"
ok "Territory updated for $username"
run_psql -c "SELECT username, is_admin, territory FROM pf.app_user WHERE lower(username) = lower('$(sql_lit "$username")')"
}
# An admin sees and changes everything, and is the only account that can recode
# the territory column or undo someone else's entry.
cmd_set_admin() {
require_env; load_env
local username="${1:-}" flag="${2:-true}"
[[ -z "$username" ]] && { read -rp " Username: " username; }
[[ -z "$username" ]] && die "Username is required."
[[ "$flag" != "true" && "$flag" != "false" ]] && die "Second argument must be true or false."
run_psql -v ON_ERROR_STOP=1 -tAc "
WITH upd AS (
UPDATE pf.app_user SET is_admin = $flag
WHERE lower(username) = lower('$(sql_lit "$username")')
RETURNING username
)
SELECT count(*) FROM upd" | grep -q '^1$' \
|| die "No such account: $username"
ok "$username is_admin = $flag"
}
# Territory values present in the data that belong to no account. Work under one
# is invisible to everybody but an admin, which is easy to cause by a typo and
# impossible to notice from inside the app.
cmd_orphan_territory() {
require_env; load_env
local source_id="${1:-}"
[[ -z "$source_id" ]] && { read -rp " Source id: " source_id; }
[[ -z "$source_id" ]] && die "Source id is required."
# The column and table are data, so the query is built in two steps rather
# than one clever one: read the names, then run the listing.
local meta col schema tname
meta=$(run_psql -tAF'|' -c "
SELECT m.cname, x.schema, x.tname
FROM pf.col_meta m JOIN pf.source x ON x.id = m.source_id
WHERE m.source_id = $source_id AND m.is_territory")
[[ -z "$meta" ]] && die "Source $source_id has no column marked is_territory."
IFS='|' read -r col schema tname <<< "$meta"
echo; bold "Territory values in $schema.$tname with no account"
run_psql -c "
SELECT DISTINCT s.\"$col\" AS unassigned
FROM \"$schema\".\"$tname\" s
WHERE TRUE
AND s.\"$col\" IS NOT NULL
AND s.\"$col\"::text NOT IN (
SELECT jsonb_array_elements_text(territory)
FROM pf.app_user
WHERE territory IS NOT NULL
)
ORDER BY 1"
}
# Deactivating leaves the row (and its history) in place, and drops any live # Deactivating leaves the row (and its history) in place, and drops any live
# session so the account loses access immediately rather than at cookie expiry. # session so the account loses access immediately rather than at cookie expiry.
cmd_disable_user() { cmd_disable_user() {
@ -489,6 +581,9 @@ interactive_menu() {
echo " 11) add-user create a login account" echo " 11) add-user create a login account"
echo " 12) passwd change an account password" echo " 12) passwd change an account password"
echo " 13) list-users show accounts" echo " 13) list-users show accounts"
echo " 16) set-territory grant an account its territory values"
echo " 17) set-admin make an account an administrator"
echo " 18) orphan-territory territory values no account owns"
echo " 14) disable-user deactivate an account and sign it out" echo " 14) disable-user deactivate an account and sign it out"
echo " 15) enable-user reactivate an account" echo " 15) enable-user reactivate an account"
echo " q) quit" echo " q) quit"
@ -508,6 +603,9 @@ interactive_menu() {
11|add-user) cmd_add_user ;; 11|add-user) cmd_add_user ;;
12|passwd) cmd_passwd ;; 12|passwd) cmd_passwd ;;
13|list-users) cmd_list_users ;; 13|list-users) cmd_list_users ;;
16|set-territory) cmd_set_territory ;;
17|set-admin) cmd_set_admin ;;
18|orphan-territory) cmd_orphan_territory ;;
14|disable-user) cmd_disable_user ;; 14|disable-user) cmd_disable_user ;;
15|enable-user) cmd_enable_user ;; 15|enable-user) cmd_enable_user ;;
q|Q|quit|exit) echo "Bye."; exit 0 ;; q|Q|quit|exit) echo "Bye."; exit 0 ;;
@ -532,8 +630,11 @@ case "${1:-}" in
add-user) cmd_add_user ;; add-user) cmd_add_user ;;
passwd) cmd_passwd ;; passwd) cmd_passwd ;;
list-users) cmd_list_users ;; list-users) cmd_list_users ;;
set-territory) shift; cmd_set_territory "$@" ;;
set-admin) shift; cmd_set_admin "$@" ;;
orphan-territory) shift; cmd_orphan_territory "$@" ;;
disable-user) cmd_disable_user "${2:-}" ;; disable-user) cmd_disable_user "${2:-}" ;;
enable-user) cmd_enable_user "${2:-}" ;; enable-user) cmd_enable_user "${2:-}" ;;
"") interactive_menu ;; "") interactive_menu ;;
*) die "Unknown command: $1. Valid: deploy start stop restart status logs db-setup config install-service uninstall-service add-user passwd list-users disable-user enable-user" ;; *) die "Unknown command: $1. Valid: deploy start stop restart status logs db-setup config install-service uninstall-service add-user passwd list-users disable-user enable-user set-territory set-admin orphan-territory" ;;
esac esac

View File

@ -70,6 +70,8 @@ module.exports = function(pool) {
id: user.id, id: user.id,
username: user.username, username: user.username,
display_name: user.display_name, display_name: user.display_name,
is_admin: !!user.is_admin,
territory: Array.isArray(user.territory) ? user.territory : [],
}; };
pool.query(`UPDATE pf.app_user SET last_login_at = now() WHERE id = $1`, [user.id]) pool.query(`UPDATE pf.app_user SET last_login_at = now() WHERE id = $1`, [user.id])
.catch(e => console.error('last_login_at update failed', e)); .catch(e => console.error('last_login_at update failed', e));

View File

@ -1,5 +1,6 @@
const express = require('express'); const express = require('express');
const { grainOf } = require('../lib/sql_generator'); const { grainOf } = require('../lib/sql_generator');
const { sessionUser } = require('../lib/auth');
const { fcTable } = require('../lib/utils'); const { fcTable } = require('../lib/utils');
module.exports = function(pool) { module.exports = function(pool) {
@ -112,6 +113,17 @@ module.exports = function(pool) {
if (!logResult.rows.length) return res.status(404).json({ error: 'Log entry not found' }); if (!logResult.rows.length) return res.status(404).json({ error: 'Log entry not found' });
const log = logResult.rows[0]; const log = logResult.rows[0];
if (log.status === 'closed') return res.status(403).json({ error: 'Version is closed' }); if (log.status === 'closed') return res.status(403).json({ error: 'Version is closed' });
// Undo deletes rows wholesale by logid, so it cannot be territory
// filtered the way a read or a write can -- half-undoing an entry
// would leave the version in a state nothing describes. Ownership
// instead: your own entries, or an admin's override. Territory alone
// would not do it anyway, since two accounts can share one.
if (!req.session?.user?.is_admin && log.pf_user !== sessionUser(req)) {
return res.status(403).json({
error: `That entry was made by ${log.pf_user || 'someone else'} — only they or an administrator can undo it`
});
}
const table = fcTable(log.tname, log.version_id); const table = fcTable(log.tname, log.version_id);
// In grain mode the client's table is indexed on pf_gkey, so undo has to // In grain mode the client's table is indexed on pf_gkey, so undo has to

View File

@ -1,9 +1,9 @@
const express = require('express'); const express = require('express');
const { tableFromArrays, tableToIPC } = require('apache-arrow'); const { tableFromArrays, tableToIPC } = require('apache-arrow');
const { applyTokens, buildWhere, buildWhereAny, COMPUTED_SLICE_COLS, buildScopeClause, buildExcludeClause, buildExcludePredicate, buildSetClause, dateGroupsOf, dimPeriodMapOf, esc, const { applyTokens, buildWhere, buildWhereAny, COMPUTED_SLICE_COLS, buildScopeClause, buildTerritoryClause, buildExcludeClause, buildExcludePredicate, buildSetClause, dateGroupsOf, dimPeriodMapOf, esc,
SEGMENT_EXPR, BUCKET_EXPR, NOTE_EXPR, VERSION_JOIN, SEGMENT_EXPR, BUCKET_EXPR, NOTE_EXPR, VERSION_JOIN,
ADJUSTMENT_SEGMENT, ADJUSTMENT_BUCKET } = require('../lib/sql_generator'); ADJUSTMENT_SEGMENT, ADJUSTMENT_BUCKET } = require('../lib/sql_generator');
const { sessionUser } = require('../lib/auth'); const { sessionUser, sessionTerritory } = require('../lib/auth');
const { fcTable } = require('../lib/utils'); const { fcTable } = require('../lib/utils');
module.exports = function(pool) { module.exports = function(pool) {
@ -47,10 +47,18 @@ module.exports = function(pool) {
// The scope is ANDed onto every unit rather than folded into the slices: it // The scope is ANDed onto every unit rather than folded into the slices: it
// applies to all of them equally, and under apply_mode 'each' folding it in // applies to all of them equally, and under apply_mode 'each' folding it in
// would repeat the same predicate in every statement for no gain. // would repeat the same predicate in every statement for no gain.
function sliceUnits(slices, ctx, applyMode, scope) { function sliceUnits(slices, ctx, applyMode, scope, req) {
const vid = ctx.version.id; const vid = ctx.version.id;
const scl = buildScopeClause(scope, ctx.filterCols, vid); const scl = buildScopeClause(scope, ctx.filterCols, vid);
const and = (w) => (scl ? (w === 'TRUE' ? scl : `${w}\nAND ${scl}`) : w); const terr = req ? territoryOf(req, ctx) : '';
const and = (w) => {
let out = w;
for (const extra of [scl, terr]) {
if (!extra) continue;
out = (out === 'TRUE' || !out) ? extra : `${out}\nAND ${extra}`;
}
return out;
};
return applyMode === 'each' return applyMode === 'each'
? slices.map(sl => ({ slices: [sl], where: and(buildWhere(sl, ctx.filterCols, vid)) })) ? slices.map(sl => ({ slices: [sl], where: and(buildWhere(sl, ctx.filterCols, vid)) }))
: [{ slices, where: and(buildWhereAny(slices, ctx.filterCols, vid)) }]; : [{ slices, where: and(buildWhereAny(slices, ctx.filterCols, vid)) }];
@ -127,6 +135,23 @@ module.exports = function(pool) {
} }
} }
// Moving a row between territories is reassignment, not forecasting, so a
// scoped account cannot set the territory column -- otherwise a rep could
// recode work into their own book, or quietly out of it, and the row would
// be gone from the view that would have shown what happened.
//
// Reads and writes are already scoped, so the *source* rows are safely the
// account's own; this is only about the destination.
function assertMayRecodeTerritory(req, ctx, set, res) {
if (!ctx.territoryCol) return true;
if (req.session?.user?.is_admin) return true;
if (!set || set[ctx.territoryCol] === undefined) return true;
res.status(403).json({
error: `Only an administrator can recode ${ctx.territoryCol} — that moves rows between territories`
});
return false;
}
// echo back what the caller asked for, for the audit log // echo back what the caller asked for, for the audit log
function pickIntent(body) { function pickIntent(body) {
const keys = ['mode', 'target_basis', 'value_incr', 'units_incr', 'value_pct', 'units_pct', 'pct', const keys = ['mode', 'target_basis', 'value_incr', 'units_incr', 'value_pct', 'units_pct', 'pct',
@ -337,10 +362,25 @@ module.exports = function(pool) {
filterCols: [...dimCols, ...dateCols], filterCols: [...dimCols, ...dateCols],
valueCol, valueCol,
unitsCol, unitsCol,
territoryCol: colMeta.find(c => c.is_territory)?.cname || null,
sql: sqlResult.rows[0].sql sql: sqlResult.rows[0].sql
}; };
} }
// Every read and every write goes through this, so a scoped account cannot
// reach a row outside its territory by any route. ANDed on last, after the
// slice and the client's own scope, where nothing in the request can undo
// it.
function territoryOf(req, ctx) {
return buildTerritoryClause(sessionTerritory(req), ctx.territoryCol);
}
function andTerritory(where, req, ctx) {
const t = territoryOf(req, ctx);
if (!t) return where;
return (!where || where === 'TRUE') ? t : `${where}\nAND ${t}`;
}
function guardOpen(version, res) { function guardOpen(version, res) {
if (version.status === 'closed') { if (version.status === 'closed') {
res.status(403).json({ error: 'Version is closed' }); res.status(403).json({ error: 'Version is closed' });
@ -363,7 +403,19 @@ module.exports = function(pool) {
} }
const tbl = fcTable(verResult.rows[0].tname, versionId); const tbl = fcTable(verResult.rows[0].tname, versionId);
const { rows: [{ count }] } = await pool.query(`SELECT COUNT(*) FROM ${tbl}`); // /data does not go through getContext, so it resolves the territory
// column itself. The count is scoped too, or the progress bar
// promises rows this account will never be sent.
const terrCol = (await pool.query(
`SELECT cname FROM pf.col_meta WHERE source_id = $1 AND is_territory LIMIT 1`,
[verResult.rows[0].source_id]
)).rows[0]?.cname || null;
const territory = sessionTerritory(req);
const terrBare = buildTerritoryClause(territory, terrCol);
const terrAlias = buildTerritoryClause(territory, terrCol, 't');
const terrWhere = terrBare ? `WHERE ${terrBare}` : '';
const { rows: [{ count }] } = await pool.query(`SELECT COUNT(*) FROM ${tbl} ${terrWhere}`);
const rowCount = parseInt(count); const rowCount = parseInt(count);
res.setHeader('Content-Type', 'application/vnd.apache.arrow.stream'); res.setHeader('Content-Type', 'application/vnd.apache.arrow.stream');
@ -382,6 +434,7 @@ module.exports = function(pool) {
FROM ${tbl} t FROM ${tbl} t
LEFT JOIN pf.log l LEFT JOIN pf.log l
ON l.id = t.pf_logid${VERSION_JOIN} ON l.id = t.pf_logid${VERSION_JOIN}
${terrAlias ? `WHERE ${terrAlias}` : ''}
`); `);
// Accumulate into column arrays (not row objects) to avoid allocating one JS // Accumulate into column arrays (not row objects) to avoid allocating one JS
@ -425,7 +478,13 @@ module.exports = function(pool) {
router.get('/versions/:id/agg', async (req, res) => { router.get('/versions/:id/agg', async (req, res) => {
try { try {
const ctx = await getContext(parseInt(req.params.id), 'get_agg'); const ctx = await getContext(parseInt(req.params.id), 'get_agg');
const sql = applyTokens(ctx.sql, { fc_table: ctx.table }); // Before the GROUP BY, not after: the territory column need not be
// part of the grain, so an aggregated row may not carry it at all.
const sql = applyTokens(ctx.sql, {
fc_table: ctx.table,
territory_clause:
buildTerritoryClause(sessionTerritory(req), ctx.territoryCol, 't') || 'TRUE',
});
const { rows } = await runSQL(sql); const { rows } = await runSQL(sql);
res.setHeader('Content-Type', 'application/vnd.apache.arrow.stream'); res.setHeader('Content-Type', 'application/vnd.apache.arrow.stream');
@ -666,7 +725,7 @@ module.exports = function(pool) {
// sum() OVER () distribute the increment across the whole pool. // sum() OVER () distribute the increment across the whole pool.
// 'each' runs the same statement once per slice, so every slice // 'each' runs the same statement once per slice, so every slice
// reaches the target on its own and gets its own log entry. // reaches the target on its own and gets its own log entry.
const units = sliceUnits(slices, ctx, applyMode, req.body.scope); const units = sliceUnits(slices, ctx, applyMode, req.body.scope, req);
const client = await pool.connect(); const client = await pool.connect();
let committed = false; let committed = false;
@ -749,10 +808,11 @@ module.exports = function(pool) {
const ctx = await getContext(parseInt(req.params.id), 'recode'); const ctx = await getContext(parseInt(req.params.id), 'recode');
if (!guardOpen(ctx.version, res)) return; if (!guardOpen(ctx.version, res)) return;
assertSelective(slices, ctx); assertSelective(slices, ctx);
if (!assertMayRecodeTerritory(req, ctx, set, res)) return;
const excludeClause = buildExcludeClause(ctx.version.exclude_iters); const excludeClause = buildExcludeClause(ctx.version.exclude_iters);
const setClause = buildSetClause(ctx.dimCols, set); const setClause = buildSetClause(ctx.dimCols, set);
const units = sliceUnits(slices, ctx, apply_mode === 'each' ? 'each' : 'prorate', req.body.scope); const units = sliceUnits(slices, ctx, apply_mode === 'each' ? 'each' : 'prorate', req.body.scope, req);
const client = await pool.connect(); const client = await pool.connect();
let committed = false; let committed = false;
@ -845,7 +905,7 @@ module.exports = function(pool) {
[cname, `${alias}."${periodCol}"`]) [cname, `${alias}."${periodCol}"`])
); );
const setClause = buildSetClause(ctx.dimCols, set, { derivedExprs, alias: 's' }); const setClause = buildSetClause(ctx.dimCols, set, { derivedExprs, alias: 's' });
const units = sliceUnits(slices, ctx, apply_mode === 'each' ? 'each' : 'prorate', req.body.scope); const units = sliceUnits(slices, ctx, apply_mode === 'each' ? 'each' : 'prorate', req.body.scope, req);
const client = await pool.connect(); const client = await pool.connect();
let committed = false; let committed = false;

View File

@ -1,5 +1,6 @@
const express = require('express'); const express = require('express');
const { generateSQL } = require('../lib/sql_generator'); const { generateSQL, buildTerritoryClause } = require('../lib/sql_generator');
const { sessionTerritory } = require('../lib/auth');
const { sessionUser } = require('../lib/auth'); const { sessionUser } = require('../lib/auth');
module.exports = function(pool) { module.exports = function(pool) {
@ -235,6 +236,18 @@ module.exports = function(pool) {
const params = []; const params = [];
let filter = `WHERE "${col}" IS NOT NULL`; let filter = `WHERE "${col}" IS NOT NULL`;
// Completion reads the *source* table, which no territory scope has
// touched -- so without this a scoped account could enumerate every
// customer, part and rep in the business from a dropdown, having
// been shown none of their rows.
const terrRow = (await pool.query(
`SELECT cname FROM pf.col_meta WHERE source_id = $1 AND is_territory LIMIT 1`,
[req.params.id]
)).rows[0];
const terrClause = buildTerritoryClause(sessionTerritory(req), terrRow?.cname || null);
if (terrClause) filter += ` AND ${terrClause}`;
if (q) { if (q) {
params.push(`%${q}%`); params.push(`%${q}%`);
filter += ` AND "${col}"::text ILIKE $${params.length}`; filter += ` AND "${col}"::text ILIKE $${params.length}`;

View File

@ -139,6 +139,18 @@ ALTER TABLE pf.log ADD COLUMN IF NOT EXISTS value_total double precision;
ALTER TABLE pf.log ADD COLUMN IF NOT EXISTS units_total double precision; ALTER TABLE pf.log ADD COLUMN IF NOT EXISTS units_total double precision;
ALTER TABLE pf.log ADD COLUMN IF NOT EXISTS measure_cols jsonb; ALTER TABLE pf.log ADD COLUMN IF NOT EXISTS measure_cols jsonb;
-- The column an account's territory is expressed in -- the rep, the region,
-- whatever this source divides ownership by. Exactly one per source.
--
-- Flagged here rather than named in code because it is source-specific, the
-- same way the grain is: a second source should not need a code change to be
-- scoped by something other than a sales rep.
--
-- It does two jobs. The server scopes every read and write to the values on the
-- account, and recode refuses to *set* this column unless the account is an
-- admin -- moving a row between territories is reassignment, not forecasting.
ALTER TABLE pf.col_meta ADD COLUMN IF NOT EXISTS is_territory boolean NOT NULL DEFAULT false;
-- Master data for a dim_group: one row per key value, with its sibling columns. -- Master data for a dim_group: one row per key value, with its sibling columns.
-- --
-- The source is transactional and often a view over all history, so deriving a -- The source is transactional and often a view over all history, so deriving a

View File

@ -24,3 +24,15 @@ CREATE TABLE IF NOT EXISTS pf.session (
); );
CREATE INDEX IF NOT EXISTS session_expire_idx ON pf.session (expire); CREATE INDEX IF NOT EXISTS session_expire_idx ON pf.session (expire);
-- What an account may see and change.
--
-- territory is the list of values allowed in the source's territory column
-- (col_meta.is_territory). It is a filter the server applies to every read and
-- every write; it is never sent by the client, which could remove it.
--
-- Fail closed: no territory and not an admin means no rows. An account created
-- without one sees nothing until someone grants it, rather than seeing the whole
-- book because a column was left null.
ALTER TABLE pf.app_user ADD COLUMN IF NOT EXISTS territory jsonb;
ALTER TABLE pf.app_user ADD COLUMN IF NOT EXISTS is_admin boolean NOT NULL DEFAULT false;