Commit ea2c043
authored
control-plane: guarded, auto-expiring temporary support access to restricted tenants (#3115)
* supabase: guarded, auto-expiring temporary support access to restricted tenants
Restricted tenants are deliberately not attached to estuary_support/, so
support cannot reach them by default. This adds an internal.support_access
tracking table plus SECURITY DEFINER functions to grant, revoke, and expire
temporary support access to a single tenant.
Operators receive EXECUTE on the functions (granted out of band by the
departmental-roles tooling) and never write public.role_grants directly, so
they cannot escalate their own access. The grant function only attaches
estuary_support/ to a named, existing tenant and logs who granted it, why,
and when it expires.
Expiry is enforced by pg_cron on the Supabase database, registered out of
band since pg_cron is absent from the sqlx test cluster. The sweeper only
removes grants that have a support_access tracking row, so the permanent
estuary_support/ grants normal tenants receive are never affected.
Adds pgTAP coverage in supabase/tests/support_access.test.sql.
Implements ADR estuary/security#746.
* supabase: only delete support grants owned by a tracking row
The temporary-access functions matched role_grants rows solely on
(subject_role, object_role), so they could not tell a temporary grant
from the permanent estuary_support/ grant every unrestricted tenant
carries:
- grant_support_access() on an unrestricted tenant hit the unique
conflict, did nothing, but still inserted a tracking row -- arming
expire_support_access() to delete the permanent grant later.
- revoke_support_access() deleted unconditionally, wiping a permanent
grant with no audit trace.
- Overlapping windows (a grant extended before it lapsed) share one
role_grants row; expiring the earlier window detached the tenant
while the later window was still open.
A role_grants row is now deleted only while an unrevoked
support_access row owns it: grant refuses unrestricted tenants unless
extending an active window, revoke requires an active tracking row,
and the sweeper skips tenants that still have an open window (and
counts only rows actually detached).
Also switch the functions to the repo-wide `set search_path to ''`
SECURITY DEFINER convention, and extend the pgTAP tests to cover the
unrestricted-tenant and overlapping-window cases with exact error
message assertions.
* supabase: move grant expiry onto role_grants/user_grants columns
Per PR review, expiration timestamps now live on the grant tables
themselves rather than only in the internal.support_access tracking
table: both role_grants and user_grants gain a nullable expires_at,
where NULL (every pre-existing row) means permanent. Enforcement is
unchanged from the sweeper design -- expired rows are DELETED on a
schedule -- so no Rust, RLS, authorization-function, or view changes
are needed; deletion propagates to every consumer by construction.
The columns make the safety guards structural rather than
ownership-tracked:
- grant_support_access() is one race-free upsert: a fresh grant inserts
with an expiry, an extension takes the later expiry (greatest), and
the conflict guard's WHERE expires_at IS NOT NULL makes converting a
permanent grant impossible (greatest(NULL, x) = x, so the guard is
load-bearing). The audit row records the effective expiry.
- revoke_support_access() deletes only rows with a non-NULL expires_at,
so it cannot detach an unrestricted tenant's permanent grant. It
deletes immediately: with no read-side expiry filters, a
lapsed-but-unswept row is still live access.
- expire_support_access() sweeps both tables where expires_at has
passed. Overlapping support windows share one row carrying the latest
expiry, so nothing is removed while a window remains open, and
permanent grants can never match.
expires_at gets SELECT-only column grants for PostgREST-facing roles:
readable (required for RETURNING-representation dashboard mutations,
pinned by the existing lives_ok tests) but writable only through the
SECURITY DEFINER functions. internal.support_access remains as the
append-only audit log.
Tests: pgTAP covers grant/extend/re-grant-over-lapsed/revoke semantics,
permanent-grant refusal, and the sweep across both tables, driven by
direct expires_at updates. Full pgTAP suite passes; the
control-plane-api suite passes serially against the migrated schema
with zero Rust changes, confirming forward compatibility of the added
columns.
* supabase: validate grant duration; audit revocation is explicit-only
Review feedback on the support-access functions:
- grant_support_access() rejects a NULL or non-positive duration. A NULL
duration computed a NULL expires_at, a permanent-looking grant; the
audit insert's NOT NULL constraint would abort the transaction anyway,
but only with a confusing error.
- expire_support_access() no longer stamps audit rows revoked. Audit
rows carry per-request expiries that an extension deliberately does
not move, so the sweep could mark a subsumed window as revoked by
enforcement while the tenant's access was still live, and the
sentinel revoked_by string conflated enforcement with revocation.
revoked_at/revoked_by now strictly record explicit revocation by a
named person; a row with revoked_at NULL and a past expires_at lapsed
on schedule.
* supabase: harden support-access extension and revoke paths
Fixes from a final review pass:
- revoke_support_access() stamps only windows still open (expires_at >
now()). Without the filter, audit rows of long-lapsed windows (which
stay revoked_at NULL by design) were mis-stamped as explicitly
revoked by whoever next revoked that tenant, corrupting the
access-review record.
- The extension upsert re-asserts capability = 'admin'. PostgREST RLS
lets a tenant's own admins update role_grants.capability, so an
extension could otherwise preserve an out-of-band downgrade while
logging an admin-implying audit row.
- detail now stays with the request whose window is in force: a shorter
no-op extension no longer re-attributes the standing window to the
losing request.
- Partial indexes on role_grants/user_grants (expires_at) WHERE
expires_at IS NOT NULL: the per-minute sweep matches almost no rows
and otherwise sequential-scans both tables.
- The audit insert relies on the flowid domain default rather than
calling internal.id_generator() explicitly, matching the table
definition's comment.
Tests cover the lapsed-history revoke, the capability re-assert, and
the detail attribution.1 parent 7af0370 commit ea2c043
2 files changed
Lines changed: 556 additions & 0 deletions
Lines changed: 230 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
0 commit comments