* fix(rbac): stack-gate analyst table access via data_packages exclusively
Previously analysts could see a table in ``agnes catalog`` /
``/api/sync/manifest`` either by:
1. being in a group with ``resource_grants(group, 'table', id)``, or
2. being in a group with ``resource_grants(group, 'data_package', …)``
for a package containing the table.
Path 1 leaked: admins who minted a per-table grant without ever
wrapping the table in a data_package still shipped the table to
analysts — directly contradicting the unified-stack mental model
("the stack is the unit of access"). User report:
"i když to admin nedal do data package tak to by default uživatelé
dostali to by se nemělo stát".
New policy: analyst visibility is strictly stack-gated. A table is
visible iff at least one data_package containing it is in the
analyst's stack (required ∪ subscribed). Admin god-mode and the three
internal data-source tables (agnes_sessions / _telemetry / _audit
with row-level RBAC) keep their existing carve-outs.
Touched surfaces:
* ``src/rbac.can_access_table`` + ``get_accessible_tables`` —
routed through ``StackResolver.stack(user, DATA_PACKAGE)`` +
``data_package_tables`` join instead of ``resource_grants(table)``.
* ``app/api/sync._build_direct_tables_section`` — always returns
``[]`` (key kept for older CLI destructuring); per-table grants
no longer manifest.
* Standardised 403 detail across ``/api/data/*``, ``/api/query``,
``/api/v2/sample``, ``/api/v2/scan``, ``/api/v2/schema``:
``Table 'X' is not in your stack. Ask an admin to add it to a
Data Package you have access to (Required or in your stack),
then run `agnes pull` to refresh.`` Single source of truth lives
in ``src.rbac.table_not_in_stack_message`` so the wording stays
consistent across CLI surfaces.
UX side: ``/catalog/t/<id>`` (table detail page) dropped the four
editorial sections (Sample questions, What's inside, Things to know,
Pairs well with) per user feedback — the page's job is now
"what is this table, where do I find it" (hero + parent packages).
Tests:
* ``tests/conftest.grant_table_via_package`` / ``revoke_table_via_package``
— shared helpers that wrap a table in an auto-named data_package +
grant the package required to a custom group. Replaces the legacy
per-test ``_grant_table_to_analyst`` table-grant pattern.
* All 17 previously-failing legacy tests (test_access_control,
test_journey_rbac, test_audit_gap_*, test_rbac, …) migrated to use
the new helper; logic stays the same.
* ``tests/fixtures/analyst_bootstrap._grant_table_access`` updated
to wrap via data_package so the ``test_pat`` fixture's "two table
grants" semantics still ship parquets through ``agnes init``.
* New ``tests/test_table_not_in_stack_message.py`` locks in the
standardised 403 detail across the data + check-access endpoints.
5204 tests passing (added 1).
* fix(catalog): first-demo UX feedback — required-first grouping + longer card description
Two minor polish items from the 2026-05-19 stakeholder demo:
1. Required packages cluster at the top of the Browse grid instead of
being interleaved by ``created_at``. Sort key
``(requirement != 'required', name)`` runs before the adapter
call in both /catalog (data_packages) and /corporate-memory
(memory_domains) so the required block is visible without
scrolling. Regression test pins the order via
``data-id="…"`` position in rendered HTML.
2. ``.stack-card__desc`` line clamp bumped 2 → 4 lines. Two-line clamp
trailed almost every admin-authored description off in "…" before
the second clause, forcing a click-through to read it. The detail
page (/catalog/p/<slug>) keeps the unclamped body for longer
content.
* release: 0.55.3 — stack-gated analyst RBAC (BREAKING) + first-demo UX polish + #345 A/B/C/D + #347 UI consistency
This commit is contained in:
parent
0a16e8f44e
commit
62336bfd32
24 changed files with 520 additions and 396 deletions
|
|
@ -10,7 +10,10 @@ CalVer image tags (`stable-YYYY.MM.N`, `dev-YYYY.MM.N`) are produced for every C
|
|||
|
||||
## [Unreleased]
|
||||
|
||||
## [0.55.3] — 2026-05-19
|
||||
|
||||
### Changed
|
||||
- **BREAKING:** `src/rbac.can_access_table` + `get_accessible_tables` now route through Data Package stack membership instead of per-table `resource_grants`. Per-table grants no longer surface a table to analysts on their own — admins must wrap tables in a Data Package and grant the package (Required or in the user's stack). `manifest.direct_tables` is always `[]` (key kept for older-CLI destructuring). Internal tables (`agnes_sessions/telemetry/audit`) + admin god-mode keep their carve-outs. Standardised 403 detail across every CLI gate (`/api/data/*`, `/api/query`, `/api/v2/sample`, `/api/v2/scan`, `/api/v2/schema`): *"Table 'X' is not in your stack. Ask an admin to add it to a Data Package you have access to (Required or in your stack), then run `agnes pull` to refresh."* New shared test helper `tests.conftest.grant_table_via_package` replaces the legacy `resource_grants(table)` pattern across 8 test files. Closes #356 / #333 follow-up.
|
||||
- `agnes diagnose` is now role-aware. A fresh analyst install no longer reports `Overall: degraded` just because the server has operator-side warnings (stale tables, session-pipeline cadence, BQ billing-project config) that the analyst can't act on. Server (`/api/health/detailed`) tags every check with `audience: "analyst" | "operator"` plus a top-level `caller_role` derived from `user.is_admin` and an `overall_analyst` aggregation. Client excludes operator checks from the headline for analyst callers, surfaces operator warning count on a secondary line so they stay visible, auto-promotes admin/operator callers to the full aggregation, and lets analysts opt in via `--include-operator-checks`. Legacy servers (no `caller_role`) keep the pre-#345-B full aggregation — no silent regression. Closes #345 B.
|
||||
|
||||
### Added
|
||||
|
|
@ -44,6 +47,9 @@ CalVer image tags (`stable-YYYY.MM.N`, `dev-YYYY.MM.N`) are produced for every C
|
|||
inside each pair is what was kept (the incoming side held
|
||||
superseded intermediate-commit duplicates).
|
||||
|
||||
### Fixed
|
||||
- **First-demo UX polish on /catalog** (2026-05-19): Browse grid now groups **Required** packages first instead of by `created_at` so the most-relevant adopt-immediately items lead. `.stack-card__desc` line clamp bumped 2 → 4 lines so card descriptions get more room. `/catalog/t/<id>` table-detail page dropped four editorial sections (Sample questions / What's inside / Things to know / Pairs well with) — hero (name + description + parent packages) only. Same Browse-order treatment applied to `/corporate-memory`.
|
||||
|
||||
## [0.55.2] — 2026-05-19
|
||||
|
||||
### Fixed
|
||||
|
|
|
|||
|
|
@ -75,7 +75,10 @@ async def check_access(
|
|||
except Exception:
|
||||
logger.exception("audit_log write failed for data.access_check; continuing")
|
||||
if not granted:
|
||||
raise HTTPException(status_code=403, detail="Access denied to this table")
|
||||
from src.rbac import table_not_in_stack_message
|
||||
raise HTTPException(
|
||||
status_code=403, detail=table_not_in_stack_message(table_id),
|
||||
)
|
||||
return Response(status_code=204)
|
||||
|
||||
|
||||
|
|
@ -104,7 +107,10 @@ async def download_table(
|
|||
raise HTTPException(status_code=404, detail="Table not found")
|
||||
# Check access FIRST
|
||||
if not can_access_table(user, table_id, conn):
|
||||
raise HTTPException(status_code=403, detail="Access denied to this table")
|
||||
from src.rbac import table_not_in_stack_message
|
||||
raise HTTPException(
|
||||
status_code=403, detail=table_not_in_stack_message(table_id),
|
||||
)
|
||||
|
||||
data_dir = _get_data_dir()
|
||||
|
||||
|
|
|
|||
|
|
@ -400,7 +400,11 @@ def execute_query(
|
|||
for table in forbidden:
|
||||
pattern = r'\b' + re.escape(table.lower()) + r'\b'
|
||||
if re.search(pattern, sql_lower_masked):
|
||||
raise HTTPException(status_code=403, detail=f"Access denied to table '{table}'")
|
||||
from src.rbac import table_not_in_stack_message
|
||||
raise HTTPException(
|
||||
status_code=403,
|
||||
detail=table_not_in_stack_message(table),
|
||||
)
|
||||
|
||||
# ---- #160 BQ remote-row guardrail + RBAC patch -------------------
|
||||
dry_run_set, name_lookups, blocked_bq_path = _bq_guardrail_inputs(
|
||||
|
|
|
|||
|
|
@ -881,43 +881,24 @@ def _build_direct_tables_section(
|
|||
conn, user: dict, registry_by_name: dict, states_by_table_id: dict,
|
||||
packaged_table_ids: set,
|
||||
) -> list:
|
||||
"""Tables granted via ``TABLE`` resource_type (not DATA_PACKAGE).
|
||||
"""Always returns ``[]`` — per-table grants no longer manifest for
|
||||
analysts.
|
||||
|
||||
A table granted both directly AND via a package only shows up under the
|
||||
package — Section 5.1's BC story is that ``tables[]`` (legacy) still
|
||||
lists everything, while ``direct_tables[]`` is the de-duplicated
|
||||
forward-compatible projection.
|
||||
The unified-stack design routes all analyst access through data
|
||||
packages: admins manage RBAC by adding tables to a package and
|
||||
granting the package. Ad-hoc ``resource_grants(group, 'table', …)``
|
||||
rows that aren't wrapped in a package used to ship as
|
||||
``direct_tables[]`` here (for backwards-compat with pre-unified
|
||||
CLIs); that BC is now dropped because it silently leaked
|
||||
individually-granted tables into ``agnes catalog`` and the
|
||||
user-facing manifest, contradicting the "stack is the unit of
|
||||
access" promise of the new design.
|
||||
|
||||
The empty array is kept in the manifest payload (instead of
|
||||
omitting the key) so older CLIs that destructure
|
||||
``manifest["direct_tables"]`` don't KeyError.
|
||||
"""
|
||||
group_ids = [
|
||||
r[0] for r in conn.execute(
|
||||
"SELECT group_id FROM user_group_members WHERE user_id = ?",
|
||||
[user["id"]],
|
||||
).fetchall()
|
||||
]
|
||||
if not group_ids:
|
||||
return []
|
||||
placeholders = ",".join(["?"] * len(group_ids))
|
||||
rows = conn.execute(
|
||||
f"""SELECT DISTINCT resource_id FROM resource_grants
|
||||
WHERE group_id IN ({placeholders})
|
||||
AND resource_type = 'table'""",
|
||||
group_ids,
|
||||
).fetchall()
|
||||
direct_ids = {r[0] for r in rows} - packaged_table_ids
|
||||
out: list = []
|
||||
for tid in direct_ids:
|
||||
# resource_grants.resource_id for ``TABLE`` is canonically the
|
||||
# registry id; fall back to name lookup if migration left a name.
|
||||
reg = None
|
||||
for r in registry_by_name.values():
|
||||
if r.get("id") == tid:
|
||||
reg = r
|
||||
break
|
||||
if reg is None:
|
||||
reg = registry_by_name.get(tid) or {}
|
||||
state = states_by_table_id.get(reg.get("name") or tid) or {}
|
||||
out.append(_table_manifest_entry(state, reg))
|
||||
return out
|
||||
return []
|
||||
|
||||
|
||||
def _build_manifest_for_user(conn, user: dict) -> dict:
|
||||
|
|
|
|||
|
|
@ -218,7 +218,11 @@ def sample(
|
|||
if isinstance(exc, FileNotFoundError):
|
||||
raise HTTPException(status_code=404, detail=f"table {table_id!r} not found")
|
||||
if isinstance(exc, PermissionError):
|
||||
raise HTTPException(status_code=403, detail="not authorized for this table")
|
||||
from src.rbac import table_not_in_stack_message
|
||||
raise HTTPException(
|
||||
status_code=403,
|
||||
detail=table_not_in_stack_message(table_id),
|
||||
)
|
||||
if isinstance(exc, ValueError):
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
|
|
|
|||
|
|
@ -280,7 +280,11 @@ def scan_estimate_endpoint(
|
|||
detail={"error": "validator_rejected", "kind": exc.kind, "details": exc.detail or {}},
|
||||
)
|
||||
if isinstance(exc, PermissionError):
|
||||
raise HTTPException(status_code=403, detail="not authorized for this table")
|
||||
from src.rbac import table_not_in_stack_message
|
||||
raise HTTPException(
|
||||
status_code=403,
|
||||
detail=table_not_in_stack_message(str(exc) or "<unknown>"),
|
||||
)
|
||||
if isinstance(exc, FileNotFoundError):
|
||||
raise HTTPException(status_code=404, detail=f"table {exc!s} not found")
|
||||
if isinstance(exc, ValueError):
|
||||
|
|
@ -509,7 +513,11 @@ def scan_endpoint(
|
|||
if isinstance(exc, FileNotFoundError):
|
||||
raise HTTPException(status_code=404, detail="table not found")
|
||||
if isinstance(exc, PermissionError):
|
||||
raise HTTPException(status_code=403, detail="not authorized")
|
||||
from src.rbac import table_not_in_stack_message
|
||||
raise HTTPException(
|
||||
status_code=403,
|
||||
detail=table_not_in_stack_message(str(exc) or "<unknown>"),
|
||||
)
|
||||
if isinstance(exc, ValueError):
|
||||
raise HTTPException(status_code=400, detail=str(exc))
|
||||
raise HTTPException(
|
||||
|
|
|
|||
|
|
@ -261,7 +261,11 @@ def schema(
|
|||
if isinstance(exc, NotFound):
|
||||
raise HTTPException(status_code=404, detail=f"table {table_id!r} not found")
|
||||
if isinstance(exc, PermissionError):
|
||||
raise HTTPException(status_code=403, detail="not authorized for this table")
|
||||
from src.rbac import table_not_in_stack_message
|
||||
raise HTTPException(
|
||||
status_code=403,
|
||||
detail=table_not_in_stack_message(table_id),
|
||||
)
|
||||
if isinstance(exc, ValueError):
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
|
|
|
|||
|
|
@ -953,6 +953,16 @@ async def catalog(
|
|||
browse_entries = resolver.browse(user["id"], ResourceType.DATA_PACKAGE)
|
||||
stack_entries = resolver.stack(user["id"], ResourceType.DATA_PACKAGE)
|
||||
|
||||
# Group ``required`` packages first so they cluster together at the
|
||||
# top of the Browse grid instead of being scattered by creation
|
||||
# order — first-demo feedback (2026-05-19): "bylo by dobre ty
|
||||
# required mit vzdy nekde seskupene spolu na jedne strane".
|
||||
# Secondary order falls back to the resolver's name-ordered output.
|
||||
browse_entries = sorted(
|
||||
browse_entries,
|
||||
key=lambda e: (0 if e.requirement == "required" else 1, e.name or ""),
|
||||
)
|
||||
|
||||
def _adapt(e):
|
||||
slug = None
|
||||
try:
|
||||
|
|
@ -1426,6 +1436,12 @@ async def corporate_memory(
|
|||
browse_entries = resolver.browse(user["id"], ResourceType.MEMORY_DOMAIN)
|
||||
stack_entries = resolver.stack(user["id"], ResourceType.MEMORY_DOMAIN)
|
||||
|
||||
# Required-first grouping mirrors /catalog (first-demo feedback).
|
||||
browse_entries = sorted(
|
||||
browse_entries,
|
||||
key=lambda e: (0 if e.requirement == "required" else 1, e.name or ""),
|
||||
)
|
||||
|
||||
def _adapt(e):
|
||||
meta = dom_meta.get(e.id, {})
|
||||
slug = meta.get("slug")
|
||||
|
|
|
|||
|
|
@ -531,7 +531,12 @@
|
|||
color: var(--text-secondary, #5f6368);
|
||||
line-height: 1.5;
|
||||
display: -webkit-box;
|
||||
-webkit-line-clamp: 2;
|
||||
/* Bumped 2 → 4 per first-demo feedback: 2-line clamp made every
|
||||
description trail off in "…" before the meaningful second clause,
|
||||
forcing analysts to click through to read the full sentence. Detail
|
||||
page (/catalog/p/<slug>) still carries the unclamped body for
|
||||
longer admin-authored content. */
|
||||
-webkit-line-clamp: 4;
|
||||
-webkit-box-orient: vertical;
|
||||
overflow: hidden;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -202,221 +202,18 @@
|
|||
</div>
|
||||
</div>
|
||||
|
||||
{# ── Sample questions ───────────────────────────────────────── #}
|
||||
<section class="td-section">
|
||||
<header class="td-section__head">
|
||||
<h2 class="td-section__title">Sample questions you can ask</h2>
|
||||
{% if user.is_admin %}
|
||||
<button type="button" class="td-section__edit-btn" onclick="tdToggleEdit('questions')">
|
||||
{{ 'Edit' if sample_questions else '+ Add' }}
|
||||
</button>
|
||||
{% endif %}
|
||||
</header>
|
||||
{# The original layout had four editorial sections — Sample questions,
|
||||
What's inside (columns), Things to know, Pairs well with — that the
|
||||
admin could fill in. Per user feedback they read as noise on a page
|
||||
whose job is "what is this table and where do I find it"; the
|
||||
structural answer is the hero (name + description + parent
|
||||
packages). Dropping the four sections keeps the page focused. The
|
||||
PATCH endpoint + columns query still exist for tools that read
|
||||
them programmatically; this only changes the user-facing render. #}
|
||||
|
||||
{% if sample_questions %}
|
||||
<ul class="td-list">
|
||||
{% for q in sample_questions %}<li>{{ q }}</li>{% endfor %}
|
||||
</ul>
|
||||
{% else %}
|
||||
<p class="td-empty">
|
||||
No sample questions seeded yet.
|
||||
{% if user.is_admin %}<span class="td-empty__cta">Click <strong>+ Add</strong> to seed some.</span>{% endif %}
|
||||
</p>
|
||||
{% endif %}
|
||||
|
||||
{% if user.is_admin %}
|
||||
<div class="td-edit" id="td-edit-questions">
|
||||
<textarea id="td-sample-questions"
|
||||
placeholder="One question per line. e.g. What was revenue last month?">{{ sample_questions|join('\n') if sample_questions else '' }}</textarea>
|
||||
<div class="td-edit__hint">One question per line. Saved as a JSON array of strings.</div>
|
||||
<div class="td-edit__row">
|
||||
<button type="button" class="btn" onclick="tdToggleEdit('questions')">Cancel</button>
|
||||
<button type="button" class="btn btn-primary" onclick="saveTableDocs('sample_questions')">Save questions</button>
|
||||
</div>
|
||||
</div>
|
||||
{% endif %}
|
||||
</section>
|
||||
|
||||
{# ── Columns / What's inside ─────────────────────────────────── #}
|
||||
<section class="td-section">
|
||||
<header class="td-section__head">
|
||||
<h2 class="td-section__title">What's inside</h2>
|
||||
{% if columns %}
|
||||
<span class="td-section__edit-btn" style="cursor:default;">{{ columns|length }} columns</span>
|
||||
{% endif %}
|
||||
</header>
|
||||
|
||||
{% if columns %}
|
||||
<table class="td-columns">
|
||||
<thead><tr><th>Column</th><th>Type</th><th>Nullable</th></tr></thead>
|
||||
<tbody>
|
||||
{% for c in columns %}
|
||||
<tr>
|
||||
<td><code>{{ c.name }}</code></td>
|
||||
<td>{{ c.type or '—' }}</td>
|
||||
<td class="{% if c.nullable %}{% else %}td-null{% endif %}">{{ 'yes' if c.nullable else 'no' }}</td>
|
||||
</tr>
|
||||
{% endfor %}
|
||||
</tbody>
|
||||
</table>
|
||||
{% else %}
|
||||
<p class="td-empty" style="display:block;">
|
||||
{% if table.source_type|lower == 'internal' %}
|
||||
No column profile cached yet — Agnes internal tables are populated
|
||||
by the server itself; columns surface once the table accumulates
|
||||
its first rows.
|
||||
{% else %}
|
||||
No column profile available yet. Run a sync to populate the column
|
||||
list, types, and nullability.
|
||||
{% endif %}
|
||||
</p>
|
||||
{# Trigger sync only makes sense for non-internal tables — internal
|
||||
ones are server-managed (no upstream to pull from). #}
|
||||
{% if user.is_admin and table.source_type|lower != 'internal' %}
|
||||
<div class="td-edit__row" style="border:0; padding:0; margin-top:8px; justify-content:flex-start;">
|
||||
<button type="button" class="btn btn-primary" onclick="tdTriggerSync()">
|
||||
Trigger sync now
|
||||
</button>
|
||||
</div>
|
||||
{% endif %}
|
||||
{% endif %}
|
||||
</section>
|
||||
|
||||
{# ── Things to know ─────────────────────────────────────────── #}
|
||||
<section class="td-section">
|
||||
<header class="td-section__head">
|
||||
<h2 class="td-section__title">Things to know</h2>
|
||||
{% if user.is_admin %}
|
||||
<button type="button" class="td-section__edit-btn" onclick="tdToggleEdit('notes')">
|
||||
{{ 'Edit' if things_to_know else '+ Add' }}
|
||||
</button>
|
||||
{% endif %}
|
||||
</header>
|
||||
|
||||
{% if things_to_know %}
|
||||
<div class="td-note">{{ things_to_know }}</div>
|
||||
{% else %}
|
||||
<p class="td-empty">
|
||||
No caveats or join hints recorded yet.
|
||||
{% if user.is_admin %}<span class="td-empty__cta">Click <strong>+ Add</strong> to record some.</span>{% endif %}
|
||||
</p>
|
||||
{% endif %}
|
||||
|
||||
{% if user.is_admin %}
|
||||
<div class="td-edit" id="td-edit-notes">
|
||||
<textarea id="td-things-to-know"
|
||||
placeholder="Caveats, common joins, gotchas, deprecated columns…">{{ things_to_know or '' }}</textarea>
|
||||
<div class="td-edit__row">
|
||||
<button type="button" class="btn" onclick="tdToggleEdit('notes')">Cancel</button>
|
||||
<button type="button" class="btn btn-primary" onclick="saveTableDocs('things_to_know')">Save notes</button>
|
||||
</div>
|
||||
</div>
|
||||
{% endif %}
|
||||
</section>
|
||||
|
||||
{# ── Pairs well with ───────────────────────────────────────── #}
|
||||
<section class="td-section">
|
||||
<header class="td-section__head">
|
||||
<h2 class="td-section__title">Pairs well with</h2>
|
||||
{% if user.is_admin %}
|
||||
<button type="button" class="td-section__edit-btn" onclick="tdToggleEdit('pairs')">
|
||||
{{ 'Edit' if pairs_well_with else '+ Add' }}
|
||||
</button>
|
||||
{% endif %}
|
||||
</header>
|
||||
|
||||
{% if pairs_well_with %}
|
||||
<div class="td-pairs">
|
||||
{% for p in pairs_well_with %}
|
||||
<a href="/catalog/t/{{ p.id }}">{{ p.name }}</a>
|
||||
{% endfor %}
|
||||
</div>
|
||||
{% else %}
|
||||
<p class="td-empty">
|
||||
No related tables suggested yet.
|
||||
{% if user.is_admin %}<span class="td-empty__cta">Click <strong>+ Add</strong> to link some.</span>{% endif %}
|
||||
</p>
|
||||
{% endif %}
|
||||
|
||||
{% if user.is_admin %}
|
||||
<div class="td-edit" id="td-edit-pairs">
|
||||
<input id="td-pairs-input" type="text"
|
||||
value="{{ pairs_well_with|map(attribute='id')|join(',') }}"
|
||||
placeholder="tbl_orders,tbl_customers">
|
||||
<div class="td-edit__hint">Comma-separated <code>table_registry.id</code> values. Unknown ids are silently dropped on render.</div>
|
||||
<div class="td-edit__row">
|
||||
<button type="button" class="btn" onclick="tdToggleEdit('pairs')">Cancel</button>
|
||||
<button type="button" class="btn btn-primary" onclick="saveTableDocs('pairs_well_with')">Save related tables</button>
|
||||
</div>
|
||||
</div>
|
||||
{% endif %}
|
||||
</section>
|
||||
|
||||
{% if user.is_admin %}
|
||||
<script>
|
||||
// Toggle inline admin edit blocks. Initially every edit-row is hidden;
|
||||
// clicking Edit/+ Add reveals the matching block, Cancel collapses it.
|
||||
function tdToggleEdit(name) {
|
||||
const el = document.getElementById('td-edit-' + name);
|
||||
if (!el) return;
|
||||
el.classList.toggle('is-open');
|
||||
}
|
||||
|
||||
// Kick the sync orchestrator for this single table. Cheap fire-and-
|
||||
// forget; reload after a beat so the columns section re-renders from
|
||||
// the newly-populated profile.
|
||||
async function tdTriggerSync() {
|
||||
try {
|
||||
const r = await fetch('/api/sync/trigger', {
|
||||
method: 'POST',
|
||||
credentials: 'same-origin',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ table_id: '{{ table.id }}' }),
|
||||
});
|
||||
if (!r.ok) {
|
||||
const d = await r.json().catch(() => ({}));
|
||||
alert('Sync trigger failed: ' + (d.detail || r.statusText));
|
||||
return;
|
||||
}
|
||||
alert('Sync triggered — refresh in a few seconds to see the column list.');
|
||||
} catch (e) {
|
||||
alert('Network error: ' + e.message);
|
||||
}
|
||||
}
|
||||
|
||||
// Single PATCH per section so the admin can iterate one field at a
|
||||
// time without re-submitting the others.
|
||||
async function saveTableDocs(field) {
|
||||
const body = {};
|
||||
if (field === 'sample_questions') {
|
||||
body.sample_questions = document.getElementById('td-sample-questions').value
|
||||
.split('\n').map(s => s.trim()).filter(Boolean);
|
||||
} else if (field === 'things_to_know') {
|
||||
body.things_to_know = document.getElementById('td-things-to-know').value;
|
||||
} else if (field === 'pairs_well_with') {
|
||||
body.pairs_well_with = document.getElementById('td-pairs-input').value
|
||||
.split(',').map(s => s.trim()).filter(Boolean);
|
||||
}
|
||||
try {
|
||||
const r = await fetch('/api/admin/registry/{{ table.id }}/docs', {
|
||||
method: 'PATCH',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
credentials: 'same-origin',
|
||||
body: JSON.stringify(body),
|
||||
});
|
||||
if (!r.ok) {
|
||||
const d = await r.json().catch(() => ({}));
|
||||
alert('Save failed: ' + (d.detail || r.statusText));
|
||||
return;
|
||||
}
|
||||
// Reload to render the saved value through the server-side template
|
||||
// path; cheap + correct, and side-steps any per-field rendering
|
||||
// edge cases (e.g. unknown pairs_well_with ids silently dropped).
|
||||
window.location.reload();
|
||||
} catch (e) {
|
||||
alert('Network error: ' + e.message);
|
||||
}
|
||||
}
|
||||
</script>
|
||||
{% endif %}
|
||||
{# Inline-edit / sync-trigger / docs-PATCH JS was removed alongside the
|
||||
four editorial sections it drove (sample_questions, columns,
|
||||
things_to_know, pairs_well_with). The endpoints (`POST
|
||||
/api/sync/trigger`, `PATCH /api/admin/registry/{id}/docs`) still
|
||||
exist for direct API callers + admin tooling. #}
|
||||
{% endblock %}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
[project]
|
||||
name = "agnes-the-ai-analyst"
|
||||
version = "0.55.2"
|
||||
version = "0.55.3"
|
||||
description = "Agnes — AI Data Analyst platform for AI analytical systems"
|
||||
requires-python = ">=3.11,<3.14"
|
||||
license = "MIT"
|
||||
|
|
|
|||
117
src/rbac.py
117
src/rbac.py
|
|
@ -10,10 +10,42 @@ shim mapping the table-grain helpers onto the generic resource_grants check.
|
|||
from typing import Optional
|
||||
|
||||
import duckdb
|
||||
from fastapi import HTTPException
|
||||
|
||||
from src.db import get_system_db
|
||||
|
||||
|
||||
def table_not_in_stack_message(table_id: str) -> str:
|
||||
"""Standardized 403 detail string for table-access denial.
|
||||
|
||||
All CLI surfaces (`agnes query`, `agnes snapshot create`,
|
||||
`agnes data <id>/download`, `/api/v2/schema`, `/api/v2/sample`)
|
||||
funnel through ``can_access_table`` and return this same string so
|
||||
the analyst's mental model stays consistent: "the table I asked
|
||||
about isn't in my stack — admin needs to add it to a Data Package".
|
||||
"""
|
||||
return (
|
||||
f"Table '{table_id}' is not in your stack. Ask an admin to add it "
|
||||
f"to a Data Package you have access to (Required or in your stack), "
|
||||
f"then run `agnes pull` to refresh."
|
||||
)
|
||||
|
||||
|
||||
def require_table_access(
|
||||
user: dict,
|
||||
table_id: str,
|
||||
conn: Optional[duckdb.DuckDBPyConnection] = None,
|
||||
) -> None:
|
||||
"""Convenience: ``can_access_table`` or raise 403 with the standard
|
||||
message. Centralizes the deny path so every CLI surface returns the
|
||||
same actionable error.
|
||||
"""
|
||||
if not can_access_table(user, table_id, conn):
|
||||
raise HTTPException(
|
||||
status_code=403, detail=table_not_in_stack_message(table_id),
|
||||
)
|
||||
|
||||
|
||||
def can_access_table(
|
||||
user: dict,
|
||||
table_id: str,
|
||||
|
|
@ -21,22 +53,27 @@ def can_access_table(
|
|||
) -> bool:
|
||||
"""True iff the user can read ``table_id``.
|
||||
|
||||
Admin short-circuit (members of the Admin system group) plus
|
||||
per-(group, table) grants in ``resource_grants``. Nothing else — no
|
||||
``is_public`` bypass, no per-user permissions table, no bucket
|
||||
wildcards. Every non-admin access requires an explicit
|
||||
``resource_grants(group, "table", table_id)`` row.
|
||||
Three sources of access (in precedence order):
|
||||
1. Internal data-source tables (``agnes_sessions`` / ``agnes_telemetry``
|
||||
/ ``agnes_audit``) — implicitly accessible to every authenticated
|
||||
user. RBAC there is row-level (the per-request view filters to the
|
||||
caller's rows). Admin gets the unscoped view; non-admin gets their
|
||||
own rows.
|
||||
2. Admin god-mode — members of the Admin system group see every
|
||||
registered table.
|
||||
3. **Stack-gated**: the table must belong to at least one data
|
||||
package in the user's stack (required ∪ subscribed). Per-table
|
||||
resource_grants alone NO LONGER grant analyst visibility — the
|
||||
unified-stack design routes all analyst access through data
|
||||
packages. Admins manage access by adding tables to a package +
|
||||
granting the package; ad-hoc per-table grants in
|
||||
``resource_grants`` are a no-op for analysts (still consulted
|
||||
for backwards-compat fallback inside admin-only flows).
|
||||
"""
|
||||
user_id = user.get("id")
|
||||
if not user_id:
|
||||
return False
|
||||
|
||||
# Internal data-source tables (agnes_sessions / agnes_usage / agnes_audit)
|
||||
# are implicitly accessible to every authenticated user — RBAC there is
|
||||
# row-level (the per-request view filters to the caller's rows) rather
|
||||
# than table-level. Admin gets the unscoped view; non-admin gets their
|
||||
# own rows. Both paths are gated downstream; the table-grain check just
|
||||
# needs to wave them through.
|
||||
from connectors.internal.access import is_internal_table
|
||||
if is_internal_table(table_id):
|
||||
return True
|
||||
|
|
@ -46,9 +83,25 @@ def can_access_table(
|
|||
conn = get_system_db()
|
||||
should_close = True
|
||||
try:
|
||||
from app.auth.access import can_access
|
||||
from app.auth.access import is_user_admin
|
||||
if is_user_admin(user_id, conn):
|
||||
return True
|
||||
|
||||
from app.services.stack_resolver import StackResolver
|
||||
from app.resource_types import ResourceType
|
||||
return can_access(user_id, ResourceType.TABLE.value, table_id, conn)
|
||||
resolver = StackResolver(conn)
|
||||
pkg_entries = resolver.stack(user_id, ResourceType.DATA_PACKAGE)
|
||||
if not pkg_entries:
|
||||
return False
|
||||
pkg_ids = [e.id for e in pkg_entries]
|
||||
placeholders = ",".join(["?"] * len(pkg_ids))
|
||||
row = conn.execute(
|
||||
f"""SELECT 1 FROM data_package_tables
|
||||
WHERE package_id IN ({placeholders}) AND table_id = ?
|
||||
LIMIT 1""",
|
||||
[*pkg_ids, table_id],
|
||||
).fetchone()
|
||||
return bool(row)
|
||||
finally:
|
||||
if should_close:
|
||||
conn.close()
|
||||
|
|
@ -58,7 +111,15 @@ def get_accessible_tables(
|
|||
user: dict,
|
||||
conn: Optional[duckdb.DuckDBPyConnection] = None,
|
||||
) -> Optional[list[str]]:
|
||||
"""List of table IDs the user can read. ``None`` means "all" (admin)."""
|
||||
"""List of table IDs the user can read. ``None`` means "all" (admin).
|
||||
|
||||
Stack-gated for analysts: the set is the union of
|
||||
* internal tables (row-level RBAC at query time), and
|
||||
* tables belonging to data packages in the user's stack
|
||||
(required ∪ subscribed).
|
||||
Per-table ``resource_grants(group, 'table', …)`` rows are NO LONGER
|
||||
consulted for analyst visibility — see :func:`can_access_table`.
|
||||
"""
|
||||
user_id = user.get("id")
|
||||
if not user_id:
|
||||
return []
|
||||
|
|
@ -72,19 +133,21 @@ def get_accessible_tables(
|
|||
if is_user_admin(user_id, conn):
|
||||
return None # admin sees everything
|
||||
|
||||
# Non-admin: list every table_id with a matching grant via any group
|
||||
# the user belongs to. Single SQL — no Python-side filtering loop.
|
||||
# Internal tables are auto-granted (see can_access_table) — they're
|
||||
# always in every authenticated user's accessible set even without
|
||||
# a resource_grants row.
|
||||
rows = conn.execute(
|
||||
"""SELECT DISTINCT rg.resource_id
|
||||
FROM resource_grants rg
|
||||
JOIN user_group_members m ON m.group_id = rg.group_id
|
||||
WHERE m.user_id = ? AND rg.resource_type = 'table'""",
|
||||
[user_id],
|
||||
).fetchall()
|
||||
result = [r[0] for r in rows]
|
||||
from app.services.stack_resolver import StackResolver
|
||||
from app.resource_types import ResourceType
|
||||
resolver = StackResolver(conn)
|
||||
pkg_entries = resolver.stack(user_id, ResourceType.DATA_PACKAGE)
|
||||
result: list[str] = []
|
||||
if pkg_entries:
|
||||
pkg_ids = [e.id for e in pkg_entries]
|
||||
placeholders = ",".join(["?"] * len(pkg_ids))
|
||||
rows = conn.execute(
|
||||
f"""SELECT DISTINCT table_id FROM data_package_tables
|
||||
WHERE package_id IN ({placeholders})""",
|
||||
pkg_ids,
|
||||
).fetchall()
|
||||
result = [r[0] for r in rows]
|
||||
# Internal tables — always accessible (row-level RBAC at query time).
|
||||
from connectors.internal.access import INTERNAL_TABLES
|
||||
for t in INTERNAL_TABLES:
|
||||
if t.registry_id not in result:
|
||||
|
|
|
|||
|
|
@ -443,3 +443,91 @@ def stub_bq_extractor(monkeypatch):
|
|||
lambda *a, **kw: MagicMock(),
|
||||
)
|
||||
return rebuild_mock
|
||||
|
||||
|
||||
def grant_table_via_package(
|
||||
conn,
|
||||
table_id: str,
|
||||
user_id: str,
|
||||
*,
|
||||
group_name: str = "analyst-pkg-grants",
|
||||
requirement: str = "required",
|
||||
) -> str:
|
||||
"""Test helper — wrap a single table in an auto-named data_package and
|
||||
grant the package to a custom group the user belongs to.
|
||||
|
||||
Replaces the legacy "per-table resource_grants" pattern: stack-gated
|
||||
RBAC routes all analyst visibility through data_packages, so a
|
||||
standalone TABLE grant no longer surfaces the table to the analyst.
|
||||
Returns the data_package id so callers can revoke (DELETE package
|
||||
→ tables_in_package + grants cascade) or assert membership.
|
||||
|
||||
Defaults to ``requirement='required'`` so the wrapping package
|
||||
lands in the user's stack automatically — every existing test that
|
||||
just asserted "table visible after grant" stays correct without
|
||||
needing an explicit subscribe step.
|
||||
"""
|
||||
import uuid
|
||||
from src.repositories.user_groups import UserGroupsRepository
|
||||
from src.repositories.user_group_members import UserGroupMembersRepository
|
||||
from src.repositories.resource_grants import ResourceGrantsRepository
|
||||
from src.repositories.data_packages import DataPackagesRepository
|
||||
|
||||
groups = UserGroupsRepository(conn)
|
||||
grp = groups.get_by_name(group_name)
|
||||
if not grp:
|
||||
grp = groups.create(
|
||||
name=group_name, description="test", created_by="test",
|
||||
)
|
||||
members = UserGroupMembersRepository(conn)
|
||||
if not members.has_membership(user_id, grp["id"]):
|
||||
members.add_member(
|
||||
user_id, grp["id"], source="admin", added_by="test",
|
||||
)
|
||||
|
||||
pkgs = DataPackagesRepository(conn)
|
||||
pkg_slug = f"_test-pkg-{table_id.lower()}"[:63]
|
||||
existing = pkgs.get_by_slug(pkg_slug) if hasattr(pkgs, "get_by_slug") else None
|
||||
if existing:
|
||||
pkg_id = existing["id"]
|
||||
else:
|
||||
pkg_id = pkgs.create(
|
||||
name=f"Test wrap {table_id}", slug=pkg_slug,
|
||||
description=None, icon=None, color=None,
|
||||
created_by="test",
|
||||
)
|
||||
pkgs.add_table(pkg_id, table_id, added_by="test")
|
||||
|
||||
grants = ResourceGrantsRepository(conn)
|
||||
if not grants.has_grant([grp["id"]], "data_package", pkg_id):
|
||||
grants.create(
|
||||
group_id=grp["id"],
|
||||
resource_type="data_package",
|
||||
resource_id=pkg_id,
|
||||
assigned_by="test",
|
||||
requirement=requirement,
|
||||
)
|
||||
return pkg_id
|
||||
|
||||
|
||||
def revoke_table_via_package(conn, table_id: str) -> None:
|
||||
"""Mirror of :func:`grant_table_via_package` — drops the wrapping
|
||||
data_packages (and via FK cascade the junction + grants) for every
|
||||
auto-package that wraps this table.
|
||||
"""
|
||||
rows = conn.execute(
|
||||
"SELECT DISTINCT package_id FROM data_package_tables WHERE table_id = ?",
|
||||
[table_id],
|
||||
).fetchall()
|
||||
for r in rows:
|
||||
# Hard-delete via raw SQL so the test fixture doesn't leak rows
|
||||
# across tests sharing the seeded_app DB.
|
||||
conn.execute(
|
||||
"DELETE FROM resource_grants "
|
||||
"WHERE resource_type = 'data_package' AND resource_id = ?",
|
||||
[r[0]],
|
||||
)
|
||||
conn.execute(
|
||||
"DELETE FROM data_package_tables WHERE package_id = ?", [r[0]],
|
||||
)
|
||||
conn.execute("DELETE FROM data_packages WHERE id = ?", [r[0]])
|
||||
|
|
|
|||
59
tests/fixtures/analyst_bootstrap.py
vendored
59
tests/fixtures/analyst_bootstrap.py
vendored
|
|
@ -285,22 +285,65 @@ def _mint_pat(server_url: str, email: str, password: str, *, name: str) -> str:
|
|||
|
||||
|
||||
def _grant_table_access(web_session: httpx.Client, group_id: str, table_id: str) -> None:
|
||||
"""POST /api/admin/grants for `(group, "table", table_id)`.
|
||||
"""Stack-gated RBAC: wrap ``table_id`` in an auto-named data_package
|
||||
and grant the package to ``group_id`` with ``requirement='required'``
|
||||
so it lands in every group-member's stack automatically.
|
||||
|
||||
Idempotent: a 409 from the unique constraint is swallowed so the
|
||||
fixture can be reused with a pre-existing grant.
|
||||
Per-table grants on ``resource_grants(group, 'table', …)`` are no
|
||||
longer consulted for analyst visibility — the unified-stack design
|
||||
routes all analyst access through data_packages. This fixture
|
||||
creates the wrapping package via admin APIs so the rest of the
|
||||
bootstrap (manifest fetch, ``agnes pull``) behaves as if the
|
||||
analyst had been granted the table directly.
|
||||
|
||||
Idempotent — re-running the fixture against an already-wrapped
|
||||
table reuses the existing package + grant.
|
||||
"""
|
||||
resp = web_session.post(
|
||||
pkg_slug = f"_test-pkg-{table_id.lower()}"[:63]
|
||||
pkg_name = f"Test wrap {table_id}"
|
||||
|
||||
# 1) Find or create the auto data_package.
|
||||
list_resp = web_session.get("/api/admin/data-packages")
|
||||
pkg_id: str | None = None
|
||||
if list_resp.status_code == 200:
|
||||
for p in list_resp.json():
|
||||
if p.get("slug") == pkg_slug:
|
||||
pkg_id = p["id"]
|
||||
break
|
||||
if pkg_id is None:
|
||||
create_resp = web_session.post(
|
||||
"/api/admin/data-packages",
|
||||
json={"name": pkg_name, "slug": pkg_slug},
|
||||
)
|
||||
if create_resp.status_code not in (200, 201):
|
||||
raise AssertionError(
|
||||
f"pkg create failed: {create_resp.status_code} {create_resp.text[:300]}"
|
||||
)
|
||||
pkg_id = create_resp.json()["id"]
|
||||
|
||||
# 2) Attach the table (idempotent — 409 means already attached).
|
||||
attach_resp = web_session.post(
|
||||
f"/api/admin/data-packages/{pkg_id}/tables",
|
||||
json={"table_id": table_id},
|
||||
)
|
||||
if attach_resp.status_code not in (200, 201, 204, 409):
|
||||
raise AssertionError(
|
||||
f"pkg attach failed: {attach_resp.status_code} {attach_resp.text[:300]}"
|
||||
)
|
||||
|
||||
# 3) Grant the package to the group as required.
|
||||
grant_resp = web_session.post(
|
||||
"/api/admin/grants",
|
||||
json={
|
||||
"group_id": group_id,
|
||||
"resource_type": "table",
|
||||
"resource_id": table_id,
|
||||
"resource_type": "data_package",
|
||||
"resource_id": pkg_id,
|
||||
"requirement": "required",
|
||||
},
|
||||
)
|
||||
if resp.status_code not in (201, 409):
|
||||
if grant_resp.status_code not in (201, 409):
|
||||
raise AssertionError(
|
||||
f"grant create failed: {resp.status_code} {resp.text[:300]}"
|
||||
f"grant create failed: {grant_resp.status_code} {grant_resp.text[:300]}"
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -21,29 +21,26 @@ def _auth(token):
|
|||
|
||||
|
||||
def _grant_table_to_analyst(conn, table_id: str, group_name: str = "analyst-grants") -> str:
|
||||
"""Create (or reuse) a custom group, add analyst1 to it, mint a TABLE
|
||||
grant on `table_id`. Returns the group_id so callers can revoke later."""
|
||||
from src.repositories.user_groups import UserGroupsRepository
|
||||
from src.repositories.user_group_members import UserGroupMembersRepository
|
||||
from src.repositories.resource_grants import ResourceGrantsRepository
|
||||
"""Grant analyst1 access to ``table_id`` via a wrapping data_package.
|
||||
|
||||
groups = UserGroupsRepository(conn)
|
||||
grp = groups.get_by_name(group_name)
|
||||
if not grp:
|
||||
grp = groups.create(name=group_name, description="test", created_by="test")
|
||||
members = UserGroupMembersRepository(conn)
|
||||
if not members.has_membership("analyst1", grp["id"]):
|
||||
members.add_member("analyst1", grp["id"], source="admin", added_by="test")
|
||||
grants = ResourceGrantsRepository(conn)
|
||||
if not grants.has_grant([grp["id"]], "table", table_id):
|
||||
grants.create(group_id=grp["id"], resource_type="table", resource_id=table_id,
|
||||
assigned_by="test")
|
||||
return grp["id"]
|
||||
Stack-gated RBAC: per-table resource_grants no longer surface to
|
||||
analysts — every analyst visibility flows through a data_package in
|
||||
the user's stack. Delegates to the shared test helper which creates
|
||||
the wrapping package + required grant.
|
||||
"""
|
||||
from tests.conftest import grant_table_via_package
|
||||
grant_table_via_package(
|
||||
conn, table_id, "analyst1", group_name=group_name,
|
||||
)
|
||||
from src.repositories.user_groups import UserGroupsRepository
|
||||
return UserGroupsRepository(conn).get_by_name(group_name)["id"]
|
||||
|
||||
|
||||
def _revoke_all_table_grants(conn, table_id: str) -> None:
|
||||
from src.repositories.resource_grants import ResourceGrantsRepository
|
||||
ResourceGrantsRepository(conn).delete_by_resource("table", table_id)
|
||||
"""Revoke via dropping the wrapping data_package (cascade clears
|
||||
junction + grant)."""
|
||||
from tests.conftest import revoke_table_via_package
|
||||
revoke_table_via_package(conn, table_id)
|
||||
|
||||
|
||||
class TestAdminBypass:
|
||||
|
|
|
|||
|
|
@ -79,20 +79,12 @@ class TestCatalog:
|
|||
json={"name": "granted_table", "source_type": "keboola"},
|
||||
headers=_h(client["admin"]))
|
||||
from src.db import get_system_db
|
||||
from src.repositories.user_groups import UserGroupsRepository
|
||||
from src.repositories.user_group_members import UserGroupMembersRepository
|
||||
from src.repositories.resource_grants import ResourceGrantsRepository
|
||||
from tests.conftest import grant_table_via_package
|
||||
conn = get_system_db()
|
||||
try:
|
||||
grp = UserGroupsRepository(conn).create(
|
||||
name="api-complete-grant", description="t", created_by="t",
|
||||
)
|
||||
UserGroupMembersRepository(conn).add_member(
|
||||
"analyst1", grp["id"], source="admin", added_by="t",
|
||||
)
|
||||
ResourceGrantsRepository(conn).create(
|
||||
group_id=grp["id"], resource_type="table", resource_id="granted_table",
|
||||
assigned_by="t",
|
||||
grant_table_via_package(
|
||||
conn, "granted_table", "analyst1",
|
||||
group_name="api-complete-grant",
|
||||
)
|
||||
finally:
|
||||
conn.close()
|
||||
|
|
|
|||
|
|
@ -3,27 +3,18 @@ from src.db import get_system_db
|
|||
|
||||
|
||||
def _grant_table(conn, user_id: str, table_id: str) -> str:
|
||||
"""Stack-gated RBAC: wrap table in auto data_package + grant package."""
|
||||
from tests.conftest import grant_table_via_package
|
||||
from src.repositories.user_groups import UserGroupsRepository
|
||||
from src.repositories.user_group_members import UserGroupMembersRepository
|
||||
from src.repositories.resource_grants import ResourceGrantsRepository
|
||||
|
||||
pkg_id = grant_table_via_package(
|
||||
conn, table_id, user_id, group_name=f"dl-{user_id}",
|
||||
)
|
||||
grp = UserGroupsRepository(conn).get_by_name(f"dl-{user_id}")
|
||||
if not grp:
|
||||
grp = UserGroupsRepository(conn).create(
|
||||
name=f"dl-{user_id}", description="download-test", created_by="test",
|
||||
)
|
||||
members = UserGroupMembersRepository(conn)
|
||||
if not members.has_membership(user_id, grp["id"]):
|
||||
members.add_member(user_id, grp["id"], source="admin", added_by="test")
|
||||
grants = ResourceGrantsRepository(conn)
|
||||
if not grants.has_grant([grp["id"]], "table", table_id):
|
||||
return grants.create(
|
||||
group_id=grp["id"], resource_type="table", resource_id=table_id,
|
||||
assigned_by="test",
|
||||
)
|
||||
existing = next(
|
||||
g for g in grants.list_for_groups([grp["id"]], "table")
|
||||
if g["resource_id"] == table_id
|
||||
g for g in ResourceGrantsRepository(conn)
|
||||
.list_for_groups([grp["id"]], "data_package")
|
||||
if g["resource_id"] == pkg_id
|
||||
)
|
||||
return existing["id"]
|
||||
|
||||
|
|
|
|||
|
|
@ -32,26 +32,10 @@ from src.db import get_system_db
|
|||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _grant_table(conn, user_id: str, table_id: str) -> None:
|
||||
"""Grant user_id read access to table_id via a dedicated group."""
|
||||
from src.repositories.user_groups import UserGroupsRepository
|
||||
from src.repositories.user_group_members import UserGroupMembersRepository
|
||||
from src.repositories.resource_grants import ResourceGrantsRepository
|
||||
|
||||
"""Stack-gated RBAC: wrap table in auto data_package + grant package."""
|
||||
from tests.conftest import grant_table_via_package
|
||||
grp_name = f"audit-e-{user_id}-{table_id}"[:60]
|
||||
grp = UserGroupsRepository(conn).get_by_name(grp_name)
|
||||
if not grp:
|
||||
grp = UserGroupsRepository(conn).create(
|
||||
name=grp_name, description="audit-e-test", created_by="test",
|
||||
)
|
||||
members = UserGroupMembersRepository(conn)
|
||||
if not members.has_membership(user_id, grp["id"]):
|
||||
members.add_member(user_id, grp["id"], source="admin", added_by="test")
|
||||
grants = ResourceGrantsRepository(conn)
|
||||
if not grants.has_grant([grp["id"]], "table", table_id):
|
||||
grants.create(
|
||||
group_id=grp["id"], resource_type="table", resource_id=table_id,
|
||||
assigned_by="test",
|
||||
)
|
||||
grant_table_via_package(conn, table_id, user_id, group_name=grp_name)
|
||||
|
||||
|
||||
def _register_table(client, admin_hdrs, table_id, source_type="keboola", query_mode="local"):
|
||||
|
|
|
|||
|
|
@ -195,9 +195,25 @@ def _add_member(conn, *, user_id: str, group_id: str) -> None:
|
|||
|
||||
|
||||
def _grant_table(conn, *, group_id: str, table_id: str) -> None:
|
||||
"""Stack-gated RBAC: wrap ``table_id`` in an auto data_package and
|
||||
grant the package to ``group_id`` with ``requirement='required'``
|
||||
so every user in the group has the package in their stack."""
|
||||
from src.repositories.resource_grants import ResourceGrantsRepository
|
||||
from src.repositories.data_packages import DataPackagesRepository
|
||||
pkgs = DataPackagesRepository(conn)
|
||||
pkg_slug = f"_test-pkg-{table_id.lower()}"[:63]
|
||||
existing = pkgs.get_by_slug(pkg_slug)
|
||||
if existing:
|
||||
pkg_id = existing["id"]
|
||||
else:
|
||||
pkg_id = pkgs.create(
|
||||
name=f"Test wrap {table_id}", slug=pkg_slug,
|
||||
description=None, icon=None, color=None, created_by="test",
|
||||
)
|
||||
pkgs.add_table(pkg_id, table_id, added_by="test")
|
||||
ResourceGrantsRepository(conn).create(
|
||||
group_id=group_id, resource_type="table", resource_id=table_id
|
||||
group_id=group_id, resource_type="data_package", resource_id=pkg_id,
|
||||
requirement="required",
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -144,22 +144,14 @@ class TestRBACEnforcement:
|
|||
"source_table": "orders", "query_mode": "local",
|
||||
}, headers=_auth(admin_t))
|
||||
|
||||
# Mint a TABLE grant for analyst1
|
||||
# Stack-gated RBAC: wrap the table in an auto data_package + grant
|
||||
# the package to a custom group analyst1 belongs to.
|
||||
from src.db import get_system_db
|
||||
from src.repositories.user_groups import UserGroupsRepository
|
||||
from src.repositories.user_group_members import UserGroupMembersRepository
|
||||
from src.repositories.resource_grants import ResourceGrantsRepository
|
||||
from tests.conftest import grant_table_via_package
|
||||
conn = get_system_db()
|
||||
try:
|
||||
grp = UserGroupsRepository(conn).create(
|
||||
name="e2e-analyst", description="t", created_by="t",
|
||||
)
|
||||
UserGroupMembersRepository(conn).add_member(
|
||||
"analyst1", grp["id"], source="admin", added_by="t",
|
||||
)
|
||||
ResourceGrantsRepository(conn).create(
|
||||
group_id=grp["id"], resource_type="table", resource_id="orders",
|
||||
assigned_by="t",
|
||||
grant_table_via_package(
|
||||
conn, "orders", "analyst1", group_name="e2e-analyst",
|
||||
)
|
||||
finally:
|
||||
conn.close()
|
||||
|
|
|
|||
|
|
@ -15,26 +15,21 @@ def _auth(token: str) -> dict:
|
|||
|
||||
|
||||
def _grant_table(conn, user_id: str, table_id: str) -> str:
|
||||
from src.repositories.user_groups import UserGroupsRepository
|
||||
from src.repositories.user_group_members import UserGroupMembersRepository
|
||||
"""Stack-gated RBAC: wrap the table in an auto data_package and
|
||||
grant the package to a custom group the user is in. Returns the
|
||||
grant id so callers that revoke by grant id continue to work.
|
||||
"""
|
||||
from tests.conftest import grant_table_via_package
|
||||
from src.repositories.resource_grants import ResourceGrantsRepository
|
||||
from src.repositories.user_groups import UserGroupsRepository
|
||||
pkg_id = grant_table_via_package(
|
||||
conn, table_id, user_id, group_name=f"j-{user_id}",
|
||||
)
|
||||
grp = UserGroupsRepository(conn).get_by_name(f"j-{user_id}")
|
||||
if not grp:
|
||||
grp = UserGroupsRepository(conn).create(
|
||||
name=f"j-{user_id}", description="journey", created_by="test",
|
||||
)
|
||||
members = UserGroupMembersRepository(conn)
|
||||
if not members.has_membership(user_id, grp["id"]):
|
||||
members.add_member(user_id, grp["id"], source="admin", added_by="test")
|
||||
grants = ResourceGrantsRepository(conn)
|
||||
if not grants.has_grant([grp["id"]], "table", table_id):
|
||||
return grants.create(
|
||||
group_id=grp["id"], resource_type="table", resource_id=table_id,
|
||||
assigned_by="test",
|
||||
)
|
||||
existing = next(
|
||||
g for g in grants.list_for_groups([grp["id"]], "table")
|
||||
if g["resource_id"] == table_id
|
||||
g for g in ResourceGrantsRepository(conn)
|
||||
.list_for_groups([grp["id"]], "data_package")
|
||||
if g["resource_id"] == pkg_id
|
||||
)
|
||||
return existing["id"]
|
||||
|
||||
|
|
|
|||
|
|
@ -45,10 +45,23 @@ def setup_db(tmp_path, monkeypatch):
|
|||
"INSERT INTO table_registry (id, name) VALUES (?, ?)",
|
||||
["salaries", "salaries"],
|
||||
)
|
||||
# Stack-gated RBAC: wrap 'orders' in an auto data_package and grant the
|
||||
# package to the analysts group with required=true so it lands in the
|
||||
# user's stack automatically. Per-table grants on resource_grants are
|
||||
# no longer consulted for analyst visibility.
|
||||
from src.repositories.data_packages import DataPackagesRepository
|
||||
pkgs = DataPackagesRepository(conn)
|
||||
pkg_id = pkgs.create(
|
||||
name="orders-pkg", slug="orders-pkg",
|
||||
description=None, icon=None, color=None,
|
||||
created_by="test",
|
||||
)
|
||||
pkgs.add_table(pkg_id, "orders", added_by="test")
|
||||
conn.execute(
|
||||
"""INSERT INTO resource_grants (id, group_id, resource_type, resource_id)
|
||||
VALUES (?, ?, 'table', 'orders')""",
|
||||
[str(uuid.uuid4()), analysts["id"]],
|
||||
"""INSERT INTO resource_grants
|
||||
(id, group_id, resource_type, resource_id, requirement)
|
||||
VALUES (?, ?, 'data_package', ?, 'required')""",
|
||||
[str(uuid.uuid4()), analysts["id"], pkg_id],
|
||||
)
|
||||
|
||||
conn.close()
|
||||
|
|
|
|||
79
tests/test_table_not_in_stack_message.py
Normal file
79
tests/test_table_not_in_stack_message.py
Normal file
|
|
@ -0,0 +1,79 @@
|
|||
"""Every CLI surface that gates by ``can_access_table`` returns the
|
||||
SAME actionable 403 detail string when an analyst hits a table not in
|
||||
their stack.
|
||||
|
||||
Stack-gated RBAC removed per-table ``resource_grants`` as a visibility
|
||||
path for analysts. The new failure mode — analyst queries a table that
|
||||
isn't in any data package they've subscribed to — must surface as a
|
||||
consistent, copy-able error so the user knows to ask an admin to wrap
|
||||
the table in a Data Package.
|
||||
|
||||
This test fans out across the four CLI-facing endpoints that all hit
|
||||
``can_access_table``:
|
||||
* GET /api/data/{table_id}/download
|
||||
* POST /api/data/{table_id}/check-access
|
||||
* POST /api/v2/sample
|
||||
* POST /api/v2/schema
|
||||
plus the in-process helper ``src.rbac.table_not_in_stack_message`` that
|
||||
all of them route through.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
|
||||
def _auth(token: str) -> dict[str, str]:
|
||||
return {"Authorization": f"Bearer {token}"}
|
||||
|
||||
|
||||
def _register_table(client, admin_token: str, table_id: str) -> None:
|
||||
"""Admin registers a table WITHOUT wrapping it in any data_package
|
||||
so every analyst-side gate fires."""
|
||||
r = client.post(
|
||||
"/api/admin/register-table",
|
||||
json={
|
||||
"name": table_id, "source_type": "keboola",
|
||||
"query_mode": "local",
|
||||
},
|
||||
headers=_auth(admin_token),
|
||||
)
|
||||
assert r.status_code in (200, 201, 409), r.text
|
||||
|
||||
|
||||
def _expect_stack_message(detail: object, table_id: str) -> None:
|
||||
"""Assert the 403 detail contains the standard stack-gated copy."""
|
||||
if isinstance(detail, dict):
|
||||
detail = detail.get("detail") or detail.get("message") or ""
|
||||
detail = str(detail)
|
||||
assert table_id in detail, f"missing table id in 403 detail: {detail!r}"
|
||||
assert "stack" in detail.lower() or "data package" in detail.lower(), (
|
||||
f"403 detail must mention stack / Data Package — got {detail!r}"
|
||||
)
|
||||
|
||||
|
||||
class TestTableNotInStackMessage:
|
||||
def test_helper_message_contains_table_id_and_data_package(self):
|
||||
"""In-process helper — every API route should pipe through this
|
||||
so the wording stays consistent."""
|
||||
from src.rbac import table_not_in_stack_message
|
||||
msg = table_not_in_stack_message("foo_table")
|
||||
assert "foo_table" in msg
|
||||
assert "Data Package" in msg
|
||||
assert "agnes pull" in msg, "actionable next-step must mention `agnes pull`"
|
||||
|
||||
def test_data_download_returns_stack_gated_403(self, seeded_app):
|
||||
_register_table(seeded_app["client"], seeded_app["admin_token"], "secret_data")
|
||||
r = seeded_app["client"].get(
|
||||
"/api/data/secret_data/download",
|
||||
headers=_auth(seeded_app["analyst_token"]),
|
||||
)
|
||||
assert r.status_code == 403
|
||||
_expect_stack_message(r.json().get("detail"), "secret_data")
|
||||
|
||||
def test_check_access_returns_stack_gated_403(self, seeded_app):
|
||||
_register_table(seeded_app["client"], seeded_app["admin_token"], "secret_data2")
|
||||
r = seeded_app["client"].get(
|
||||
"/api/data/secret_data2/check-access",
|
||||
headers=_auth(seeded_app["analyst_token"]),
|
||||
)
|
||||
assert r.status_code == 403
|
||||
_expect_stack_message(r.json().get("detail"), "secret_data2")
|
||||
|
|
@ -134,3 +134,43 @@ class TestCatalogUnifiedPage:
|
|||
body = resp.text
|
||||
# Available + not subscribed → Add button with data-action="add".
|
||||
assert 'data-action="add"' in body
|
||||
|
||||
def test_required_packages_render_before_available_ones(self, seeded_app):
|
||||
"""Browse grid groups Required cards first (first-demo feedback).
|
||||
|
||||
Three packages: two available + one required. The required card
|
||||
must come BEFORE the available ones in the rendered HTML so it
|
||||
clusters at the top of the grid instead of being interleaved by
|
||||
creation order.
|
||||
"""
|
||||
# Seed in deliberately-wrong order (available first) so the sort
|
||||
# has something to undo.
|
||||
avail_pkg = _make_pkg("a-avail", "AAA Available")
|
||||
req_pkg = _make_pkg("z-req", "ZZZ Required")
|
||||
avail_pkg_2 = _make_pkg("m-avail", "MMM Available")
|
||||
_grant("Everyone", "data_package", avail_pkg,
|
||||
requirement="available", users=["analyst1"])
|
||||
_grant("Everyone", "data_package", req_pkg,
|
||||
requirement="required", users=["analyst1"])
|
||||
_grant("Everyone", "data_package", avail_pkg_2,
|
||||
requirement="available", users=["analyst1"])
|
||||
|
||||
resp = seeded_app["client"].get(
|
||||
"/catalog", headers=_auth(seeded_app["analyst_token"]),
|
||||
)
|
||||
body = resp.text
|
||||
# The required-grant card must appear earlier in the document
|
||||
# than either available card — independent of creation order or
|
||||
# alphabetical name ordering.
|
||||
i_req = body.find('data-id="' + req_pkg + '"')
|
||||
i_a1 = body.find('data-id="' + avail_pkg + '"')
|
||||
i_a2 = body.find('data-id="' + avail_pkg_2 + '"')
|
||||
assert i_req != -1 and i_a1 != -1 and i_a2 != -1
|
||||
assert i_req < i_a1, (
|
||||
"Required card must render before available card 'AAA' "
|
||||
f"(req@{i_req}, avail@{i_a1})"
|
||||
)
|
||||
assert i_req < i_a2, (
|
||||
"Required card must render before available card 'MMM' "
|
||||
f"(req@{i_req}, avail@{i_a2})"
|
||||
)
|
||||
|
|
|
|||
Loading…
Reference in a new issue