diff --git a/.claude/skills/key-shares/SKILL.md b/.claude/skills/key-shares/SKILL.md new file mode 100644 index 00000000000..38713d18a76 --- /dev/null +++ b/.claude/skills/key-shares/SKILL.md @@ -0,0 +1,147 @@ +--- +name: key-shares +description: Operate and diagnose shares — keys their owners have put into a realm for other people there to consume — the `key_shares` and `key_share_notices` tables and the `/_key-shares` and `/_key-share-notices` routes. Covers share states (`active`, `suspended`) and every suspension reason (`owner-lost-write`, `suspended-by-writer`, `key-deleted`), which change makes each, how a share resumes, the triggers on `realm_user_permissions` and `user_keys` that suspend shares however those rows change, who may share, unshare, suspend, resume and swap, who sees what, and every refusal the API answers with. Use when a user reports a share that stopped working or that they can't share, swap, suspend or see; when reading or querying `key_shares` / `key_share_notices` as an operator; or when changing `lib/key-shares.ts`, `handlers/handle-key-shares.ts` or the triggers. +--- + +# Key shares (operators) + +A **share** puts a key into a realm so that other people there can consume it +(a weather subscription, say). It holds no secret, only a reference to the +key, so one key can be shared into several realms while its value is stored +once, in `user_keys` (see the `user-keys` skill). Using your own key in a +realm is a different thing and needs no share. + +The code is `packages/realm-server/lib/key-shares.ts` (the rules) and +`packages/realm-server/handlers/handle-key-shares.ts` (the routes). The +triggers are in the migration that adds `key_shares`. + +## States and suspension reasons + +A share is `active` or `suspended`. A suspended share serves nothing and +always carries one reason: + +| `suspension_reason` | Made by | `key_id` | Ends when | +| --------------------- | --------------------------------------------------------------------------- | -------- | ----------------------------------------------------------------------------------------- | +| `owner-lost-write` | The owner's own permission row stops granting write, or is deleted (leaves) | kept | The owner's row grants write again (resumes by itself), or a direct writer swaps a key in | +| `suspended-by-writer` | A direct writer of the realm (`suspended_by`), for abuse or any reason | kept | A direct writer resumes it, or swaps a key in | +| `key-deleted` | The owner deleted the key the share holds | null | A direct writer swaps a key in | + +`key_shares_state_check` enforces these combinations. Only an active share is +suspended: a share suspended for one reason keeps that reason when another +applies too, so a share a writer suspended stays suspended when its owner +loses write and regains it. The exception is deleting the key: a share then +holds no key, so whatever it was suspended for, it becomes `key-deleted` +(clearing `suspended_by`), and the realm's writers get a notice of it. Resuming a `suspended-by-writer` share whose +owner is no longer a direct writer leaves it suspended as `owner-lost-write`, +and it resumes when they write again. + +A suspension keeps the share's place, label and history: only its owner's +unshare, or deleting its realm, deletes a share, so cards that name it by +label keep working once it is brought back. Deleting a realm deletes its +shares and their notices, so a realm later made at the same URL gets none of +them back. + +## Triggers + +Suspensions that follow from another change are made by triggers, so they +hold however the change is made (the realm's `_permissions` endpoint, the +admin permission upsert, publishing, deleting a realm, a database writer): + +- `key_shares_follow_permissions` on `realm_user_permissions`, after any + insert, update or delete of a row: suspends that person's active shares in + that realm when the row no longer grants write, and resumes their + `owner-lost-write` shares there when it does again. A share's owner is + always a person (only a direct writer can share or swap in), so their own + row is the whole answer. +- `key_shares_follow_key_delete` on `user_keys`, before a delete: suspends + every share holding the key as `key-deleted` and clears `key_id`. +- `key_shares_notice_suspension` on `key_shares`: each share going from active + to suspended, and each suspended share whose key is deleted, records a row + in `key_share_notices`. + +Sharing and swapping lock the person's permission row (`isDirectWriterLocked`) +until they commit, so a concurrent removal of their write waits and then +suspends what they made. + +## Who may do what + +| Action | Who | Ticket | +| ------- | -------------------------------------------------------------------------------------------------------------------------- | ---------------------------- | +| Share | The key's owner, into a realm where they are a direct writer | `share` for that key + realm | +| Unshare | The share's owner, whether or not they still write the realm | none | +| Suspend | Any direct writer of the realm, on an active share | none | +| Resume | Any direct writer of the realm, on a `suspended-by-writer` share | `resume` for that share | +| Swap | Any direct writer of the realm, putting one of their own keys into a suspended share; they become its owner and it resumes | `swap` for that key + share | + +A direct writer is a person whose own permission row grants write +(`isDirectWriter`): a `*` or `users` row, a bot account or a policy grant +makes no one a direct writer. Nobody swaps in another user's key: a ticket to +do so is refused at minting with 404, as for a key that doesn't exist. + +Changing a share's `limits` is for its owner only; the column is reserved and +no route sets it. + +## Visibility + +- A share's owner sees every share they own, in every realm, with its key + (`GET /_key-shares`); `?key=` narrows that to the shares holding one + key — the ones deleting it would suspend. +- A realm's direct writers see its shares with owner and state, without the + key of a share they don't own (`GET /_key-shares?realm=`), and its + notices, newest first, at most 100 (`GET /_key-share-notices?realm=`). + An owner who loses write stops seeing the realm's shares and notices, and + still sees their own. +- Anyone else, readers included, sees nothing of a realm's shares: the + listings answer 403 and a share by id answers 404. + +## The tables + +```sql +-- A realm's shares. +SELECT id, label, owner, key_id, state, suspension_reason, suspended_by, suspended_at +FROM key_shares WHERE realm_url = 'https://app.boxel.ai/someone/realm/' ORDER BY label; + +-- Why a share is suspended, and since when. +SELECT created_at, reason, owner, suspended_by +FROM key_share_notices WHERE share_id = '' ORDER BY created_at DESC; +``` + +Both tables hold no secret, but they are withheld from `readonly_role`, like +`user_keys`, and from every role that reads with its privileges (granted it +with INHERIT or SET, directly or through other roles). The migration revokes +each SELECT grant those roles hold, acting as the role that made it and with +CASCADE, never touching a table's owner or a member that only administers the +role. It then stops, naming the role and the column, if any of them can still +read a column (through a PUBLIC grant, say). Where the migrating role can't act +as a grant's grantor, it stops and names the grant. They are also excluded from +the SQLite schema dump. +`realm_url` is the realm's canonical URL, with a trailing slash, as +`realm_user_permissions` stores it. + +## The API + +All routes take the user's realm-server session and refuse a delegated or a +realm-authority session, as the keys API does. Bodies are JSON:API with type +`key-share`; a ticket goes in `meta.ticket`, minted from +`POST /_key-write-tickets` by a secure session (see the `user-keys` skill) +with `attributes.{action: 'share', keyId, realm}`, +`attributes.{action: 'swap', keyId, shareId}` or +`attributes.{action: 'resume', shareId}`. The request log keeps only the +method and path of these routes. + +| Route | Does | +| ------------------------------------- | ------------------------------------------------------------------------------ | +| `GET /_key-shares` | The user's own shares; `?key=` for one key's; `?realm=` for a realm's | +| `POST /_key-shares` | Share: `attributes.{realm, keyId, label}`, `meta.{ticket}` | +| `DELETE /_key-shares/:id` | Unshare | +| `POST /_key-shares/:id/suspend` | Suspend an active share | +| `POST /_key-shares/:id/resume` | Lift a writer's suspension: `meta.{ticket}` | +| `POST /_key-shares/:id/swap` | Swap a key in: `attributes.{keyId}`, `meta.{ticket}` | +| `GET /_key-share-notices?realm=` | A realm's suspension notices | + +| Status | Means | +| ------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 400 | Malformed body, a missing realm or key, a missing, over-long or control-character label, an attribute that can't be set | +| 403 | No valid ticket; sharing into a realm the owner isn't a direct writer of; a writer action by someone who isn't one; unsharing someone else's share; listing a realm's shares or notices without being a direct writer | +| 404 | No such share, or one the user can't see; no such key of the user's | +| 409 | A label the realm already has; suspending a suspended share; resuming one a writer didn't suspend; swapping into an active one | diff --git a/.claude/skills/user-keys/SKILL.md b/.claude/skills/user-keys/SKILL.md index a96054c7a92..5e1748d85f0 100644 --- a/.claude/skills/user-keys/SKILL.md +++ b/.claude/skills/user-keys/SKILL.md @@ -34,9 +34,12 @@ has lost track of a value replaces it. | `created_at`, `updated_at` | `timestamptz` | `key_write_tickets` — one row per unused ticket: `ticket_sha256`, `owner`, -`action` (`add`, `replace` or `delete`), `key_id` (the key a replace or -delete changes; null for an add, enforced by -`key_write_tickets_action_key_id_check`) and `expires_at`. Only the SHA-256 +`action` (`add`, `replace`, `delete`, `share`, `swap` or `resume`), `key_id` +(the key a replace, delete, share or swap uses; null for an add or a resume), +`realm_url` (the realm a share puts the key into; null otherwise), `share_id` +(the share a swap puts the key into, or a resume resumes; null otherwise) and +`expires_at`. +`key_write_tickets_action_key_id_check` enforces which action names what. Only the SHA-256 is stored; the ticket itself exists only in the response that minted it. Both are excluded from the SQLite schema dump and withheld from @@ -63,14 +66,17 @@ A value moved to another row or owner by a database writer does not open: ## Key write tickets -A key is added, replaced or deleted only with a ticket: minted for the -signed-in user and for one change, valid for 10 minutes, single use. A ticket -names its change when it is minted: `add`, or `replace` or `delete` of one of -the user's keys. It spends only on that change, so a ticket that leaks can't +A key is added, replaced, deleted, shared into a realm or swapped into a +share only with a ticket: minted for the signed-in user and for one change, +valid for 10 minutes, single use. A ticket names its change when it is +minted: `add`; `replace` or `delete` of one of the user's keys; `share` of one +of them into one realm; `swap` of one of them into one share; or `resume` of +one share a writer suspended (see the `key-shares` skill). It spends only on that change, so a ticket that leaks can't be put to a different one: a ticket to add a key replaces or deletes nothing, and a ticket to replace or delete one key does nothing to another, nor the -other action to the same key. A ticket for a key the user doesn't have is -refused with 404 at minting, as for no key. +other action to the same key, and a ticket to share a key into one realm, +swap it into one share or resume one share does nothing elsewhere. A ticket for a key the user +doesn't have is refused with 404 at minting, as for no key. The change consumes the ticket in the same transaction, so a change refused for another reason (a duplicate name, an unknown key, the wrong ticket for @@ -98,20 +104,20 @@ the ticket goes in `meta`. A body is at most exceed that. A query string on these routes is ignored, and left out of the request log line and Sentry breadcrumbs. -| Route | Does | -| -------------------------- | ------------------------------------------------------------------------------------------------------------------------------ | -| `POST /_key-write-tickets` | Mint a ticket for one change: type `key-write-ticket`, `attributes.{action, keyId?}`; answers `attributes.{ticket, expiresAt}` | -| `GET /_user-keys` | List the user's keys' metadata, by name | -| `GET /_user-keys/:id` | One key's metadata | -| `POST /_user-keys` | Add: `attributes.{name, kind?, value}`, `meta.{ticket}` (a ticket to `add`) | -| `PATCH /_user-keys/:id` | Replace the value: `attributes.{value}`, `meta.{ticket}` (a ticket to `replace` this key) | -| `DELETE /_user-keys/:id` | Delete: `meta.{ticket}` (a ticket to `delete` this key) | +| Route | Does | +| -------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------ | +| `POST /_key-write-tickets` | Mint a ticket for one change: type `key-write-ticket`, `attributes.{action, keyId?, realm?, shareId?}`; answers `attributes.{ticket, expiresAt}` | +| `GET /_user-keys` | List the user's keys' metadata, by name | +| `GET /_user-keys/:id` | One key's metadata | +| `POST /_user-keys` | Add: `attributes.{name, kind?, value}`, `meta.{ticket}` (a ticket to `add`) | +| `PATCH /_user-keys/:id` | Replace the value: `attributes.{value}`, `meta.{ticket}` (a ticket to `replace` this key) | +| `DELETE /_user-keys/:id` | Delete: `meta.{ticket}` (a ticket to `delete` this key); suspends every share holding it (`GET /_key-shares?key=` lists them first) | An attribute other than these, such as a `destination`, is refused with 400. | Status | Means | | ------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| 400 | Malformed body, a missing or over-long name or value, an attribute that can't be set, a ticket request with no valid action or with the wrong `keyId` for it | +| 400 | Malformed body, a missing or over-long name or value, an attribute that can't be set, a ticket request with no valid action or with the wrong `keyId`, `realm` or `shareId` for it | | 401 | No session, an invalid one, or a delegated one | | 403 | No valid ticket (missing, used, expired, another user's, or minted for a different change); a ticket asked for without a secure session, or by the app page; a realm-authority session | | 404 | No such key — also what another user's key answers, including when a ticket is asked for to change it | diff --git a/packages/host/config/schema/1792107467874_schema.sql b/packages/host/config/schema/1792187770930_schema.sql similarity index 100% rename from packages/host/config/schema/1792107467874_schema.sql rename to packages/host/config/schema/1792187770930_schema.sql diff --git a/packages/postgres/migrations/1792187770930_add-key-shares.js b/packages/postgres/migrations/1792187770930_add-key-shares.js new file mode 100644 index 00000000000..6b2953e3049 --- /dev/null +++ b/packages/postgres/migrations/1792187770930_add-key-shares.js @@ -0,0 +1,453 @@ +// Shares: a key its owner has put into a realm for other people there to +// consume. A share holds no secret, only the key it refers to; one key may be +// shared into several realms and its value is stored once, in `user_keys`. +// +// `key_shares` holds one row per share. `owner` is the owner of the key the +// share holds: whoever shared it, or the writer who last swapped one of their +// own keys in. `label` names the share for people and is unique within its +// realm. `state` is `active` or `suspended`; a suspended share carries the +// reason: +// +// - `owner-lost-write`: the owner is no longer a direct writer of the realm. +// The share resumes by itself if they become one again. +// - `suspended-by-writer`: a direct writer of the realm suspended it +// (`suspended_by`). +// - `key-deleted`: the owner deleted the key; `key_id` is then null. +// +// A suspended share serves nothing. A direct writer swaps one of their own +// keys into it to bring it back. `limits` is reserved for the limits a share's +// owner sets on its use. +// +// Triggers keep the state right whatever writes the rows they watch: +// +// - On `realm_user_permissions`, any change to a person's row suspends that +// person's active shares in the realm when the row no longer grants write, +// and resumes the ones it suspended for that reason when it does again. +// A share's owner is always a person whose own row granted write when they +// shared or swapped a key in, since only a direct writer (see +// `isDirectWriter`, which refuses wildcard rows and bot accounts) can do +// either; so their own row is the whole answer. +// - On `user_keys`, deleting a key suspends every share holding it, whatever +// it was suspended for before, since the share then holds no key. +// - On `key_shares`, a share going from active to suspended, or a suspended +// share whose key is deleted, records a row in `key_share_notices` for the +// realm's direct writers. +// +// Both tables are withheld from the read-only role used by dashboards and +// read-only database access, as `user_keys` is, and from every role that reads +// with its privileges: one granted it with INHERIT or SET, directly or through +// other roles granted that way. A role that only administers the membership +// isn't one (on Postgres 16 the role that creates `readonly_role` holds that +// membership, and it is often the tables' owner), and a table's owner is never +// treated as one, so its own privileges are left alone. +// +// A REVOKE removes only the grants its grantor made, so `up` revokes each +// SELECT grant those roles hold on a table or its columns acting as the role +// that made it, with CASCADE, which also removes the grants passed on from it +// at the same level. CASCADE on a table grant leaves the column grants passed +// on from it, which nobody could then revoke, so column grants go first. +// Within each level, grants a table's owner made go first, so a grant one +// reader passed on to another goes with the grant it came from. Acting as +// another role needs the migrating role to be a superuser or able to SET ROLE +// to it; where it isn't, `up` stops and names the grant rather than leave the +// table readable. SELECT granted to PUBLIC, or to a role outside the read-only +// role that one of its readers also belongs to, still lets them read, and +// revoking it isn't a migration's call, so `up` then asks +// `has_column_privilege` about every column of both tables for each reader and +// stops, naming the role and the column, if any of them can still read one. +// +// `readonly_role` exists only where it was provisioned (staging and +// production), so both steps are skipped wherever it doesn't. Every statement +// is idempotent, so running `up` again changes nothing. + +exports.up = (pgm) => { + pgm.sql(` + CREATE TABLE IF NOT EXISTS key_shares ( + id uuid PRIMARY KEY, + realm_url varchar NOT NULL, + label varchar NOT NULL, + -- The Matrix user id of the owner of the key the share holds. + owner varchar NOT NULL, + key_id uuid REFERENCES user_keys (id) ON DELETE SET NULL, + state varchar NOT NULL, + suspension_reason varchar, + suspended_by varchar, + suspended_at timestamptz, + limits jsonb NOT NULL DEFAULT '{}'::jsonb, + created_at timestamptz NOT NULL DEFAULT NOW(), + updated_at timestamptz NOT NULL DEFAULT NOW(), + CONSTRAINT key_shares_realm_url_label_key UNIQUE (realm_url, label), + CONSTRAINT key_shares_state_check CHECK ( + ( + state = 'active' + AND key_id IS NOT NULL + AND suspension_reason IS NULL + AND suspended_by IS NULL + AND suspended_at IS NULL + ) + OR ( + state = 'suspended' + AND suspended_at IS NOT NULL + AND ( + (suspension_reason = 'owner-lost-write' AND key_id IS NOT NULL AND suspended_by IS NULL) + OR (suspension_reason = 'suspended-by-writer' AND key_id IS NOT NULL AND suspended_by IS NOT NULL) + OR (suspension_reason = 'key-deleted' AND key_id IS NULL AND suspended_by IS NULL) + ) + ) + ) + ) + `); + pgm.sql(` + CREATE INDEX IF NOT EXISTS key_shares_realm_url_owner_idx + ON key_shares (realm_url, owner) + `); + pgm.sql(` + CREATE INDEX IF NOT EXISTS key_shares_owner_idx ON key_shares (owner) + `); + pgm.sql(` + CREATE INDEX IF NOT EXISTS key_shares_key_id_idx ON key_shares (key_id) + `); + pgm.sql(` + CREATE TABLE IF NOT EXISTS key_share_notices ( + id bigserial PRIMARY KEY, + share_id uuid NOT NULL REFERENCES key_shares (id) ON DELETE CASCADE, + realm_url varchar NOT NULL, + label varchar NOT NULL, + owner varchar NOT NULL, + reason varchar NOT NULL, + suspended_by varchar, + created_at timestamptz NOT NULL DEFAULT NOW() + ) + `); + pgm.sql(` + CREATE INDEX IF NOT EXISTS key_share_notices_realm_url_idx + ON key_share_notices (realm_url, created_at) + `); + + pgm.sql(` + CREATE OR REPLACE FUNCTION key_shares_follow_owner_write( + share_realm_url varchar, + share_owner varchar + ) RETURNS void LANGUAGE plpgsql AS $$ + BEGIN + -- Lock the person's shares before deciding. A resume that read no + -- permission row (there was none to lock) may be about to set a share + -- to 'owner-lost-write'. Waiting on its lock means the UPDATE below, + -- a new statement with a new snapshot, sees that committed reason and + -- resumes the share, rather than skipping it as 'suspended-by-writer'. + PERFORM 1 FROM key_shares + WHERE realm_url = share_realm_url AND owner = share_owner + FOR UPDATE; + IF EXISTS ( + SELECT 1 FROM realm_user_permissions + WHERE realm_url = share_realm_url + AND username = share_owner + AND write + ) THEN + UPDATE key_shares + SET state = 'active', + suspension_reason = NULL, + suspended_at = NULL, + updated_at = NOW() + WHERE realm_url = share_realm_url + AND owner = share_owner + AND state = 'suspended' + AND suspension_reason = 'owner-lost-write'; + ELSE + UPDATE key_shares + SET state = 'suspended', + suspension_reason = 'owner-lost-write', + suspended_at = NOW(), + updated_at = NOW() + WHERE realm_url = share_realm_url + AND owner = share_owner + AND state = 'active'; + END IF; + END + $$ + `); + pgm.sql(` + CREATE OR REPLACE FUNCTION key_shares_on_permission_change() + RETURNS trigger LANGUAGE plpgsql AS $$ + BEGIN + IF TG_OP IN ('UPDATE', 'DELETE') THEN + PERFORM key_shares_follow_owner_write(OLD.realm_url, OLD.username); + END IF; + IF TG_OP IN ('INSERT', 'UPDATE') THEN + PERFORM key_shares_follow_owner_write(NEW.realm_url, NEW.username); + END IF; + RETURN NULL; + END + $$ + `); + pgm.sql(` + DROP TRIGGER IF EXISTS key_shares_follow_permissions ON realm_user_permissions + `); + pgm.sql(` + CREATE TRIGGER key_shares_follow_permissions + AFTER INSERT OR UPDATE OR DELETE ON realm_user_permissions + FOR EACH ROW EXECUTE FUNCTION key_shares_on_permission_change() + `); + + pgm.sql(` + CREATE OR REPLACE FUNCTION key_shares_on_key_delete() + RETURNS trigger LANGUAGE plpgsql AS $$ + BEGIN + UPDATE key_shares + SET state = 'suspended', + suspension_reason = 'key-deleted', + suspended_by = NULL, + suspended_at = COALESCE(suspended_at, NOW()), + key_id = NULL, + updated_at = NOW() + WHERE key_id = OLD.id; + RETURN OLD; + END + $$ + `); + pgm.sql(`DROP TRIGGER IF EXISTS key_shares_follow_key_delete ON user_keys`); + pgm.sql(` + CREATE TRIGGER key_shares_follow_key_delete + BEFORE DELETE ON user_keys + FOR EACH ROW EXECUTE FUNCTION key_shares_on_key_delete() + `); + + pgm.sql(` + CREATE OR REPLACE FUNCTION key_shares_record_suspension() + RETURNS trigger LANGUAGE plpgsql AS $$ + BEGIN + INSERT INTO key_share_notices + (share_id, realm_url, label, owner, reason, suspended_by) + VALUES + (NEW.id, NEW.realm_url, NEW.label, NEW.owner, NEW.suspension_reason, NEW.suspended_by); + RETURN NULL; + END + $$ + `); + pgm.sql(`DROP TRIGGER IF EXISTS key_shares_notice_suspension ON key_shares`); + pgm.sql(` + CREATE TRIGGER key_shares_notice_suspension + AFTER UPDATE OF state, suspension_reason ON key_shares + FOR EACH ROW + WHEN ( + NEW.state = 'suspended' + AND ( + OLD.state = 'active' + OR ( + NEW.suspension_reason = 'key-deleted' + AND OLD.suspension_reason IS DISTINCT FROM 'key-deleted' + ) + ) + ) + EXECUTE FUNCTION key_shares_record_suspension() + `); + + // A ticket to share a key names the key and the realm it goes into; a + // ticket to swap a key in names the key and the share it goes into; a + // ticket to resume a share names the share. + pgm.sql(` + ALTER TABLE key_write_tickets ADD COLUMN IF NOT EXISTS realm_url varchar + `); + pgm.sql(` + ALTER TABLE key_write_tickets ADD COLUMN IF NOT EXISTS share_id uuid + `); + pgm.sql(` + ALTER TABLE key_write_tickets + DROP CONSTRAINT IF EXISTS key_write_tickets_action_key_id_check + `); + pgm.sql(` + ALTER TABLE key_write_tickets + ADD CONSTRAINT key_write_tickets_action_key_id_check CHECK ( + (action = 'add' AND key_id IS NULL AND realm_url IS NULL AND share_id IS NULL) + OR (action IN ('replace', 'delete') AND key_id IS NOT NULL AND realm_url IS NULL AND share_id IS NULL) + OR (action = 'share' AND key_id IS NOT NULL AND realm_url IS NOT NULL AND share_id IS NULL) + OR (action = 'swap' AND key_id IS NOT NULL AND realm_url IS NULL AND share_id IS NOT NULL) + OR (action = 'resume' AND key_id IS NULL AND realm_url IS NULL AND share_id IS NOT NULL) + ) + `); + + pgm.sql(` + DO $$ + DECLARE + migrating_role name := current_user; + table_name text; + grantor_name name; + grantee_name name; + revoked_grantor oid; + revoked_grantee oid; + BEGIN + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'readonly_role') THEN + FOREACH table_name IN ARRAY ARRAY['key_shares', 'key_share_notices'] + LOOP + LOOP + -- One SELECT grant the read-only role or a member of it still + -- holds on the table or one of its columns. A grant the table's + -- owner made has no grantor name. + SELECT + CASE WHEN held.grantor <> held.relowner + THEN grantor_role.rolname END, + grantee_role.rolname, + held.grantor, + held.grantee + INTO grantor_name, grantee_name, revoked_grantor, revoked_grantee + FROM ( + SELECT tbl.relowner, true AS on_table, acl.* + FROM pg_class tbl + CROSS JOIN LATERAL aclexplode(tbl.relacl) acl + WHERE tbl.oid = table_name::regclass + UNION ALL + SELECT tbl.relowner, false AS on_table, acl.* + FROM pg_class tbl + JOIN pg_attribute col ON col.attrelid = tbl.oid + CROSS JOIN LATERAL aclexplode(col.attacl) acl + WHERE tbl.oid = table_name::regclass + AND col.attnum > 0 + AND NOT col.attisdropped + ) held + JOIN pg_roles grantor_role ON grantor_role.oid = held.grantor + JOIN pg_roles grantee_role ON grantee_role.oid = held.grantee + WHERE held.privilege_type = 'SELECT' + AND grantee_role.oid <> held.relowner + AND grantee_role.oid IN ( + WITH RECURSIVE reader(oid) AS ( + SELECT oid FROM pg_roles WHERE rolname = 'readonly_role' + UNION + SELECT membership.member + FROM pg_auth_members membership + JOIN reader ON membership.roleid = reader.oid + WHERE membership.inherit_option OR membership.set_option + ) + SELECT oid FROM reader + ) + -- Column grants come before table grants: CASCADE on a table + -- grant leaves the column grants passed on from it, and once + -- their grantor has lost the table grant, nobody can revoke + -- them. Within each, grants the owner made come first, so + -- CASCADE takes what a reader passed on from one without acting + -- as that reader. + ORDER BY held.on_table, held.grantor <> held.relowner + LIMIT 1; + EXIT WHEN NOT FOUND; + IF grantor_name IS NOT NULL THEN + BEGIN + EXECUTE format('SET ROLE %I', grantor_name); + EXCEPTION WHEN insufficient_privilege THEN + RAISE EXCEPTION + 'cannot revoke the SELECT on % that % granted to %', + table_name, grantor_name, grantee_name + USING HINT = format( + 'Run the migration as a superuser or as a role that can SET ROLE %I.', + grantor_name + ); + END; + END IF; + EXECUTE format( + 'REVOKE SELECT ON TABLE %I FROM %I CASCADE', + table_name, + grantee_name + ); + EXECUTE format('SET ROLE %I', migrating_role); + IF EXISTS ( + SELECT 1 + FROM pg_class tbl + CROSS JOIN LATERAL aclexplode(tbl.relacl) acl + WHERE tbl.oid = table_name::regclass + AND acl.privilege_type = 'SELECT' + AND acl.grantor = revoked_grantor + AND acl.grantee = revoked_grantee + UNION ALL + SELECT 1 + FROM pg_attribute col + CROSS JOIN LATERAL aclexplode(col.attacl) acl + WHERE col.attrelid = table_name::regclass + AND acl.privilege_type = 'SELECT' + AND acl.grantor = revoked_grantor + AND acl.grantee = revoked_grantee + ) THEN + RAISE EXCEPTION 'the SELECT on % that % granted to % survived its REVOKE', + table_name, COALESCE(grantor_name, 'the owner'), grantee_name; + END IF; + END LOOP; + END LOOP; + END IF; + END + $$ + `); + pgm.sql(` + DO $$ + DECLARE + readable_by name; + readable_table name; + readable_column name; + BEGIN + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'readonly_role') THEN + WITH RECURSIVE reader (oid) AS ( + SELECT oid FROM pg_roles WHERE rolname = 'readonly_role' + UNION + SELECT membership.member + FROM pg_auth_members membership + JOIN reader ON reader.oid = membership.roleid + WHERE membership.inherit_option OR membership.set_option + ) + SELECT reader_role.rolname, tbl.relname, col.attname + INTO readable_by, readable_table, readable_column + FROM reader + JOIN pg_roles reader_role ON reader_role.oid = reader.oid + CROSS JOIN pg_class tbl + JOIN pg_attribute col ON col.attrelid = tbl.oid + WHERE tbl.oid IN ( + to_regclass('key_shares'), + to_regclass('key_share_notices') + ) + AND reader.oid <> tbl.relowner + AND col.attnum > 0 + AND NOT col.attisdropped + AND has_column_privilege(reader.oid, tbl.oid, col.attnum, 'SELECT') + ORDER BY tbl.relname, reader_role.rolname, col.attnum + LIMIT 1; + IF FOUND THEN + RAISE EXCEPTION + '% can read %.%, which is withheld from readonly_role and the roles that read through it', + readable_by, readable_table, readable_column + USING HINT = format( + 'Revoke the grant or role membership that lets %I read %I.%I, then run the migration again.', + readable_by, readable_table, readable_column + ); + END IF; + END IF; + END + $$ + `); +}; + +exports.down = (pgm) => { + pgm.sql(` + DELETE FROM key_write_tickets WHERE action IN ('share', 'swap', 'resume') + `); + pgm.sql(` + ALTER TABLE key_write_tickets + DROP CONSTRAINT IF EXISTS key_write_tickets_action_key_id_check + `); + pgm.sql(` + ALTER TABLE key_write_tickets + ADD CONSTRAINT key_write_tickets_action_key_id_check CHECK ( + (action = 'add' AND key_id IS NULL) + OR (action IN ('replace', 'delete') AND key_id IS NOT NULL) + ) + `); + pgm.sql(`ALTER TABLE key_write_tickets DROP COLUMN IF EXISTS share_id`); + pgm.sql(`ALTER TABLE key_write_tickets DROP COLUMN IF EXISTS realm_url`); + pgm.sql(`DROP TRIGGER IF EXISTS key_shares_notice_suspension ON key_shares`); + pgm.sql(`DROP TRIGGER IF EXISTS key_shares_follow_key_delete ON user_keys`); + pgm.sql(` + DROP TRIGGER IF EXISTS key_shares_follow_permissions ON realm_user_permissions + `); + pgm.sql(`DROP FUNCTION IF EXISTS key_shares_record_suspension()`); + pgm.sql(`DROP FUNCTION IF EXISTS key_shares_on_key_delete()`); + pgm.sql(`DROP FUNCTION IF EXISTS key_shares_on_permission_change()`); + pgm.sql( + `DROP FUNCTION IF EXISTS key_shares_follow_owner_write(varchar, varchar)`, + ); + pgm.sql(`DROP TABLE IF EXISTS key_share_notices`); + pgm.sql(`DROP TABLE IF EXISTS key_shares`); +}; diff --git a/packages/postgres/scripts/schema-dump.sh b/packages/postgres/scripts/schema-dump.sh index 9b6e1c311ff..83e77530011 100755 --- a/packages/postgres/scripts/schema-dump.sh +++ b/packages/postgres/scripts/schema-dump.sh @@ -34,6 +34,8 @@ docker exec boxel-pg pg_dump \ --exclude-table-and-children=proxy_endpoints \ --exclude-table-and-children=user_keys \ --exclude-table-and-children=key_write_tickets \ + --exclude-table-and-children=key_shares \ + --exclude-table-and-children=key_share_notices \ --exclude-table-and-children=secure_sessions \ --exclude-table-and-children=proxy_endpoint_history \ --exclude-table-and-children=billable_call_reservations \ diff --git a/packages/realm-server/handlers/handle-key-shares.ts b/packages/realm-server/handlers/handle-key-shares.ts new file mode 100644 index 00000000000..f977da61e10 --- /dev/null +++ b/packages/realm-server/handlers/handle-key-shares.ts @@ -0,0 +1,168 @@ +import type Koa from 'koa'; +import { setContextResponse } from '../middleware/index.ts'; +import type { CreateRoutesArgs } from '../routes.ts'; +import { + listOwnShares, + listRealmShareNotices, + listRealmShares, + resumeShare, + shareKey, + suspendShare, + swapKeyIntoShare, + unshareKey, + type KeyShare, + type KeyShareNotice, +} from '../lib/key-shares.ts'; +import { UserKeyError } from '../lib/user-keys.ts'; +import { + keysHandler, + ownerOf, + readJSON, + sendJSON, + ticketOf, +} from './handle-user-keys.ts'; + +// The shares API: sharing the signed-in user's keys into realms, and what a +// realm's direct writers do with its shares. Who may do what is set out in +// `lib/key-shares.ts`. Sharing, swapping and resuming each take a key write +// ticket in `meta`, so, as for the keys API, nothing here logs a body and the request +// log keeps only the method and path of these routes. + +type Handler = (ctxt: Koa.Context, next: Koa.Next) => Promise; + +function serialize(share: KeyShare) { + let { id, ...attributes } = share; + return { type: 'key-share', id, attributes }; +} + +function serializeNotice(notice: KeyShareNotice) { + let { id, ...attributes } = notice; + return { type: 'key-share-notice', id, attributes }; +} + +async function readShareBody( + ctxt: Koa.Context, + allowed: Set, +): Promise<{ attributes: Record; ticket: unknown }> { + let body = await readJSON(ctxt); + let data = body.data as + | { type?: unknown; attributes?: Record } + | undefined; + if (!data || typeof data !== 'object' || data.type !== 'key-share') { + throw new UserKeyError(400, `the resource type is 'key-share'`); + } + let attributes = data.attributes ?? {}; + if (typeof attributes !== 'object' || Array.isArray(attributes)) { + throw new UserKeyError(400, 'attributes must be an object'); + } + for (let name of Object.keys(attributes)) { + if (!allowed.has(name)) { + throw new UserKeyError(400, `'${name}' can't be set here`); + } + } + return { attributes, ticket: ticketOf(body) }; +} + +const SHARE_ATTRIBUTES = new Set(['realm', 'keyId', 'label']); +const SWAP_ATTRIBUTES = new Set(['keyId']); + +// `GET /_key-shares` lists the user's own shares, in every realm; with +// `?key=`, only those holding that key, which deleting it would suspend. +// `GET /_key-shares?realm=` lists a realm's shares, for one of its +// direct writers. +export function handleListKeyShares({ dbAdapter }: CreateRoutesArgs): Handler { + return keysHandler(async (ctxt) => { + let user = ownerOf(ctxt); + let realm = ctxt.URL.searchParams.get('realm'); + let keyId = ctxt.URL.searchParams.get('key'); + if (realm !== null && keyId !== null) { + throw new UserKeyError(400, 'list by realm or by key, not both'); + } + let shares = + realm !== null + ? await listRealmShares(dbAdapter, user, realm) + : await listOwnShares(dbAdapter, user, { + keyId: keyId ?? undefined, + }); + await sendJSON(ctxt, 200, { data: shares.map(serialize) }); + }); +} + +export function handleListKeyShareNotices({ + dbAdapter, +}: CreateRoutesArgs): Handler { + return keysHandler(async (ctxt) => { + let user = ownerOf(ctxt); + let notices = await listRealmShareNotices( + dbAdapter, + user, + ctxt.URL.searchParams.get('realm'), + ); + await sendJSON(ctxt, 200, { data: notices.map(serializeNotice) }); + }); +} + +export function handleShareKey({ dbAdapter }: CreateRoutesArgs): Handler { + return keysHandler(async (ctxt) => { + let owner = ownerOf(ctxt); + let { attributes, ticket } = await readShareBody(ctxt, SHARE_ATTRIBUTES); + let share = await shareKey(dbAdapter, { + owner, + realm: attributes.realm, + keyId: attributes.keyId, + label: attributes.label, + ticket, + }); + await sendJSON(ctxt, 201, { data: serialize(share) }); + }); +} + +export function handleUnshareKey({ dbAdapter }: CreateRoutesArgs): Handler { + return keysHandler(async (ctxt) => { + let user = ownerOf(ctxt); + await unshareKey(dbAdapter, { user, shareId: ctxt.params.shareId }); + await setContextResponse(ctxt, new Response(null, { status: 204 })); + }); +} + +export function handleSuspendKeyShare({ + dbAdapter, +}: CreateRoutesArgs): Handler { + return keysHandler(async (ctxt) => { + let user = ownerOf(ctxt); + let share = await suspendShare(dbAdapter, { + user, + shareId: ctxt.params.shareId, + }); + await sendJSON(ctxt, 200, { data: serialize(share) }); + }); +} + +export function handleResumeKeyShare({ dbAdapter }: CreateRoutesArgs): Handler { + return keysHandler(async (ctxt) => { + let user = ownerOf(ctxt); + let body = await readJSON(ctxt); + let share = await resumeShare(dbAdapter, { + user, + shareId: ctxt.params.shareId, + ticket: ticketOf(body), + }); + await sendJSON(ctxt, 200, { data: serialize(share) }); + }); +} + +export function handleSwapKeyIntoShare({ + dbAdapter, +}: CreateRoutesArgs): Handler { + return keysHandler(async (ctxt) => { + let user = ownerOf(ctxt); + let { attributes, ticket } = await readShareBody(ctxt, SWAP_ATTRIBUTES); + let share = await swapKeyIntoShare(dbAdapter, { + user, + shareId: ctxt.params.shareId, + keyId: attributes.keyId, + ticket, + }); + await sendJSON(ctxt, 200, { data: serialize(share) }); + }); +} diff --git a/packages/realm-server/handlers/handle-user-keys.ts b/packages/realm-server/handlers/handle-user-keys.ts index e09023928d7..b428dadcb29 100644 --- a/packages/realm-server/handlers/handle-user-keys.ts +++ b/packages/realm-server/handlers/handle-user-keys.ts @@ -20,7 +20,8 @@ import { type UserKeyMetadata, } from '../lib/user-keys.ts'; -// The management API for the signed-in user's own keys. Every route answers +// The management API for the signed-in user's own keys. The shares API +// (`handle-key-shares.ts`) answers with the same helpers. Every route answers // for the token's user alone, and no route returns a key's value. Adding, // replacing and deleting a key each take a key write ticket minted for that // change. A request body @@ -51,7 +52,11 @@ function serialize(key: UserKeyMetadata) { return { type: 'user-key', id, attributes }; } -async function sendJSON(ctxt: Koa.Context, status: number, body: unknown) { +export async function sendJSON( + ctxt: Koa.Context, + status: number, + body: unknown, +) { await setContextResponse( ctxt, new Response(JSON.stringify(body), { @@ -65,7 +70,7 @@ async function sendJSON(ctxt: Koa.Context, status: number, body: unknown) { // session check ahead of these routes already refuses a delegated session; a // realm-authority session is a realm acting on its own authority, not the // user, so it is refused here. -function ownerOf(ctxt: Koa.Context): string { +export function ownerOf(ctxt: Koa.Context): string { let token = ctxt.state.token as RealmServerTokenClaim | undefined; if (!token?.user) { throw new UserKeyError(403, 'keys are managed by a signed-in user'); @@ -100,7 +105,9 @@ async function readRawBody(ctxt: Koa.Context): Promise { return Buffer.concat(chunks).toString('utf8'); } -async function readJSON(ctxt: Koa.Context): Promise> { +export async function readJSON( + ctxt: Koa.Context, +): Promise> { let raw = await readRawBody(ctxt); let json: unknown; try { @@ -114,7 +121,7 @@ async function readJSON(ctxt: Koa.Context): Promise> { return json as Record; } -function ticketOf(body: { meta?: unknown }): unknown { +export function ticketOf(body: { meta?: unknown }): unknown { let meta = body.meta; return meta && typeof meta === 'object' ? (meta as Record).ticket @@ -158,7 +165,7 @@ function attributesOf( // Runs a handler, answering a `UserKeyError` with its status and message and // anything else with a bare 500. Only an error's message is logged, never the // request. -function keysHandler(fn: (ctxt: Koa.Context) => Promise): Handler { +export function keysHandler(fn: (ctxt: Koa.Context) => Promise): Handler { return async function (ctxt: Koa.Context, _next: Koa.Next) { try { await fn(ctxt); @@ -246,10 +253,12 @@ export function handleMintKeyWriteTicket({ if (!data || typeof data !== 'object' || data.type !== 'key-write-ticket') { throw new UserKeyError(400, `the resource type is 'key-write-ticket'`); } - let target = validateKeyWriteTarget( - data.attributes?.action, - data.attributes?.keyId, - ); + let target = validateKeyWriteTarget({ + action: data.attributes?.action, + keyId: data.attributes?.keyId, + realm: data.attributes?.realm, + shareId: data.attributes?.shareId, + }); let { ticket, expiresAt } = await mintKeyWriteTicket( dbAdapter, owner, diff --git a/packages/realm-server/handlers/realm-destruction-utils.ts b/packages/realm-server/handlers/realm-destruction-utils.ts index 76ef956b22e..868ef171fae 100644 --- a/packages/realm-server/handlers/realm-destruction-utils.ts +++ b/packages/realm-server/handlers/realm-destruction-utils.ts @@ -136,4 +136,8 @@ export async function removeRealmDatabaseArtifacts(args: { ]); await q([`DELETE FROM realm_file_meta WHERE realm_url =`, param(realmURL)]); await q([`DELETE FROM realm_metadata WHERE url =`, param(realmURL)]); + // A deleted realm's shares serve no one, and kept, they would resume for a + // realm later made at the same URL that grants their owner write. Their + // notices go with them. + await q([`DELETE FROM key_shares WHERE realm_url =`, param(realmURL)]); } diff --git a/packages/realm-server/lib/key-shares.ts b/packages/realm-server/lib/key-shares.ts new file mode 100644 index 00000000000..3686c6c27ca --- /dev/null +++ b/packages/realm-server/lib/key-shares.ts @@ -0,0 +1,550 @@ +import { randomUUID } from 'node:crypto'; +import { + isDirectWriterLocked, + param, + query, + type DBAdapter, +} from '@cardstack/runtime-common'; +import type { Querier } from '@cardstack/runtime-common/expression'; +import { normalizeRealmURL } from '../utils/realm-url.ts'; +import { consumeKeyWriteTicket, UserKeyError } from './user-keys.ts'; + +// Shares: a key its owner has put into a realm for other people there to +// consume. A share holds no secret, only the key it refers to, so one key may +// be shared into several realms and its value is stored once. +// +// Who may do what: +// +// - Only a key's owner shares it, and only into a realm where they are a +// direct writer (`isDirectWriter`). Sharing takes a key write ticket for +// that key and that realm. +// - Only a share's owner unshares it, whether or not they still write the +// realm: withdrawing one's own key needs no write. +// - Any direct writer of the realm suspends an active share. Any direct writer +// lifts a suspension a direct writer made, with a key write ticket for that +// share: lifting one puts a key back into use. +// - Any direct writer of the realm swaps one of their own keys into a +// suspended share, which makes them its owner and resumes it. Swapping takes +// a key write ticket for that key and that share. Nobody swaps in another +// user's key. +// - A share's owner sees all of their shares, in every realm. A realm's direct +// writers see its shares, with owner and state, and the notices recorded +// when one is suspended. Nobody else sees anything of a realm's shares. +// +// Suspensions that follow from other changes are made by triggers on the +// tables those changes write (see the migration that adds `key_shares`), so +// they hold however the change is made: a share is suspended when its owner +// stops being a direct writer of the realm, and resumes when they are one +// again; and it is suspended, keeping its place, when its key is deleted. +// Every share going from active to suspended records a notice, as does a +// suspended share whose key is deleted. Deleting a realm deletes its shares. +// +// Every check that a person is a direct writer runs inside the change's +// transaction with their permission row locked (`isDirectWriterLocked`), so a +// concurrent removal of their write waits for the change to commit and then +// suspends what it made. + +export type KeyShareState = 'active' | 'suspended'; + +export type KeyShareSuspensionReason = + | 'owner-lost-write' + | 'suspended-by-writer' + | 'key-deleted'; + +export interface KeyShare { + id: string; + realm: string; + label: string; + owner: string; + // The key the share holds; given only to the share's owner, and null once + // the key is deleted. + keyId?: string | null; + state: KeyShareState; + suspensionReason: KeyShareSuspensionReason | null; + suspendedBy: string | null; + suspendedAt: string | null; + createdAt: string; + updatedAt: string; +} + +export interface KeyShareNotice { + id: string; + shareId: string; + realm: string; + label: string; + owner: string; + reason: KeyShareSuspensionReason; + suspendedBy: string | null; + createdAt: string; +} + +// The most notices a realm's listing returns, newest first. +export const MAX_KEY_SHARE_NOTICES = 100; + +const SHARE_COLUMNS = `id, realm_url, label, owner, key_id, state, suspension_reason, suspended_by, suspended_at, created_at, updated_at`; + +function iso(value: unknown): string | null { + return value == null ? null : new Date(value as string).toISOString(); +} + +function toShare( + row: Record, + { withKey }: { withKey: boolean }, +): KeyShare { + return { + id: row.id as string, + realm: row.realm_url as string, + label: row.label as string, + owner: row.owner as string, + ...(withKey ? { keyId: (row.key_id as string | null) ?? null } : {}), + state: row.state as KeyShareState, + suspensionReason: + (row.suspension_reason as KeyShareSuspensionReason | null) ?? null, + suspendedBy: (row.suspended_by as string | null) ?? null, + suspendedAt: iso(row.suspended_at), + createdAt: iso(row.created_at)!, + updatedAt: iso(row.updated_at)!, + }; +} + +const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +export function isUUID(value: unknown): value is string { + return typeof value === 'string' && UUID.test(value); +} + +function noSuchShare() { + return new UserKeyError(404, 'no such share'); +} + +function noSuchKey() { + return new UserKeyError(404, 'no such key'); +} + +function notADirectWriter() { + return new UserKeyError( + 403, + 'only a direct writer of the realm can do this to its shares', + ); +} + +export function validateRealm(realm: unknown): URL { + let realmURL = typeof realm === 'string' ? normalizeRealmURL(realm) : null; + if (!realmURL) { + throw new UserKeyError(400, 'a share names the realm it is in'); + } + return realmURL; +} + +export const MAX_KEY_SHARE_LABEL_LENGTH = 100; + +export function validateShareLabel(label: unknown): string { + let trimmed = typeof label === 'string' ? label.trim() : ''; + if (trimmed.length === 0) { + throw new UserKeyError(400, 'a share needs a label'); + } + if (trimmed.length > MAX_KEY_SHARE_LABEL_LENGTH) { + throw new UserKeyError( + 400, + `a share's label is at most ${MAX_KEY_SHARE_LABEL_LENGTH} characters`, + ); + } + // eslint-disable-next-line no-control-regex + if (/[\u0000-\u001f\u007f]/.test(trimmed)) { + throw new UserKeyError( + 400, + `a share's label can't contain control characters`, + ); + } + return trimmed; +} + +// Locks are taken in the order the triggers take them: permission rows, then +// the key, then the share. A permission change locks its row and then, +// through its trigger, every share the person holds in that realm, waiting +// out any change to one before deciding what to suspend or resume. A row +// that doesn't exist can't be locked, so that wait is what keeps a resume +// and the person's write being granted from missing each other. Deleting a +// key locks the key and then the shares holding it. Taking a share before a +// row either of them holds would deadlock against them. + +// Whether each of `users` is a direct writer of the realm, with their +// permission rows locked in username order, so two changes locking the same +// rows can't wait on each other. +async function lockDirectWriters( + txQuery: Querier, + realmURL: URL, + users: string[], +): Promise> { + let writers = new Map(); + for (let user of [...new Set(users)].sort()) { + writers.set(user, await isDirectWriterLocked(txQuery, realmURL, user)); + } + return writers; +} + +// The user's own key, locked against a concurrent delete until the caller's +// transaction ends. Another user's key answers as one that doesn't exist. +async function lockOwnKey( + txQuery: Querier, + owner: string, + keyId: string, +): Promise { + let rows = await txQuery([ + `SELECT id FROM user_keys WHERE id = `, + param(keyId), + ` AND owner = `, + param(owner), + ` FOR SHARE`, + ]); + if (rows.length === 0) { + throw noSuchKey(); + } +} + +// The share, locked for the caller's change, if `user` may see it: as its +// owner, or as a direct writer of its realm. Any other share answers as one +// that doesn't exist. The user's permission row is locked first, and with +// `withOwner` the share's owner's too; with `ownKeyId`, the user's key is +// locked next. A swap made between reading the share's owner and locking the +// share changes the owner, so the locks are taken again for the new one. +async function lockVisibleShare( + txQuery: Querier, + user: string, + shareId: string, + { + withOwner = false, + ownKeyId, + }: { withOwner?: boolean; ownKeyId?: string } = {}, +): Promise<{ + row: Record; + isWriter: boolean; + ownerIsWriter: boolean; +}> { + if (!isUUID(shareId)) { + throw noSuchShare(); + } + for (;;) { + let [current] = await txQuery([ + `SELECT realm_url, owner FROM key_shares WHERE id = `, + param(shareId), + ]); + if (!current) { + throw noSuchShare(); + } + let owner = current.owner as string; + let writers = await lockDirectWriters( + txQuery, + new URL(current.realm_url as string), + withOwner ? [user, owner] : [user], + ); + if (ownKeyId !== undefined) { + await lockOwnKey(txQuery, user, ownKeyId); + } + let [row] = await txQuery([ + `SELECT ${SHARE_COLUMNS} FROM key_shares WHERE id = `, + param(shareId), + ` FOR UPDATE`, + ]); + if (!row) { + throw noSuchShare(); + } + if (withOwner && row.owner !== owner) { + continue; + } + let isWriter = writers.get(user)!; + if (!isWriter && row.owner !== user) { + throw noSuchShare(); + } + return { row, isWriter, ownerIsWriter: writers.get(owner) ?? false }; + } +} + +export async function shareKey( + dbAdapter: DBAdapter, + { + owner, + realm, + keyId, + label, + ticket, + }: { + owner: string; + realm: unknown; + keyId: unknown; + label: unknown; + ticket: unknown; + }, +): Promise { + let realmURL = validateRealm(realm); + let validLabel = validateShareLabel(label); + if (keyId === undefined || keyId === null) { + throw new UserKeyError(400, 'a share names the key it holds'); + } + if (!isUUID(keyId)) { + throw noSuchKey(); + } + let id = randomUUID(); + return await dbAdapter.withTransaction(async (txQuery) => { + await consumeKeyWriteTicket(txQuery, owner, ticket, { + action: 'share', + keyId, + realmURL: realmURL.href, + shareId: null, + }); + if (!(await isDirectWriterLocked(txQuery, realmURL, owner))) { + throw new UserKeyError( + 403, + 'a key is shared only into a realm its owner is a direct writer of', + ); + } + await lockOwnKey(txQuery, owner, keyId); + let [row] = await txQuery([ + `INSERT INTO key_shares (id, realm_url, label, owner, key_id, state) VALUES (`, + param(id), + `, `, + param(realmURL.href), + `, `, + param(validLabel), + `, `, + param(owner), + `, `, + param(keyId), + `, 'active') ON CONFLICT (realm_url, label) DO NOTHING RETURNING ${SHARE_COLUMNS}`, + ]); + if (!row) { + throw new UserKeyError( + 409, + `the realm already has a share labelled ${validLabel}`, + ); + } + return toShare(row, { withKey: true }); + }); +} + +export async function unshareKey( + dbAdapter: DBAdapter, + { user, shareId }: { user: string; shareId: string }, +): Promise { + await dbAdapter.withTransaction(async (txQuery) => { + let { row } = await lockVisibleShare(txQuery, user, shareId); + if (row.owner !== user) { + throw new UserKeyError(403, 'only its owner unshares a share'); + } + await txQuery([`DELETE FROM key_shares WHERE id = `, param(shareId)]); + }); +} + +// A direct writer suspends an active share, for abuse or any other reason of +// theirs. The realm's direct writers get a notice of it. Suspending takes no +// ticket: it only stops a key being used. +export async function suspendShare( + dbAdapter: DBAdapter, + { user, shareId }: { user: string; shareId: string }, +): Promise { + return await dbAdapter.withTransaction(async (txQuery) => { + let { row, isWriter } = await lockVisibleShare(txQuery, user, shareId); + if (!isWriter) { + throw notADirectWriter(); + } + if (row.state !== 'active') { + throw new UserKeyError(409, 'the share is already suspended'); + } + let [updated] = await txQuery([ + `UPDATE key_shares SET state = 'suspended', suspension_reason = 'suspended-by-writer', suspended_by = `, + param(user), + `, suspended_at = NOW(), updated_at = NOW() WHERE id = `, + param(shareId), + ` RETURNING ${SHARE_COLUMNS}`, + ]); + return toShare(updated, { withKey: updated.owner === user }); + }); +} + +// A direct writer lifts a suspension a direct writer made, with a key write +// ticket for that share: lifting one puts a key back into use, which card +// code in the writer's app page must not be able to do. If the share's owner +// is no longer a direct writer, the share stays suspended for that reason +// instead, and resumes when they are one again. +export async function resumeShare( + dbAdapter: DBAdapter, + { user, shareId, ticket }: { user: string; shareId: string; ticket: unknown }, +): Promise { + if (!isUUID(shareId)) { + throw noSuchShare(); + } + return await dbAdapter.withTransaction(async (txQuery) => { + await consumeKeyWriteTicket(txQuery, user, ticket, { + action: 'resume', + keyId: null, + realmURL: null, + shareId, + }); + let { row, isWriter, ownerIsWriter } = await lockVisibleShare( + txQuery, + user, + shareId, + { withOwner: true }, + ); + if (!isWriter) { + throw notADirectWriter(); + } + if (row.suspension_reason !== 'suspended-by-writer') { + throw new UserKeyError( + 409, + row.state === 'active' + ? 'the share is not suspended' + : 'only a suspension a writer made is lifted; swap a key in instead', + ); + } + let [updated] = await txQuery([ + ownerIsWriter + ? `UPDATE key_shares SET state = 'active', suspension_reason = NULL, suspended_at = NULL` + : `UPDATE key_shares SET suspension_reason = 'owner-lost-write'`, + `, suspended_by = NULL, updated_at = NOW() WHERE id = `, + param(shareId), + ` RETURNING ${SHARE_COLUMNS}`, + ]); + return toShare(updated, { withKey: updated.owner === user }); + }); +} + +// A direct writer puts one of their own keys into a suspended share. They +// become its owner and it resumes; its label, and so every card that names +// it, is unchanged. +export async function swapKeyIntoShare( + dbAdapter: DBAdapter, + { + user, + shareId, + keyId, + ticket, + }: { user: string; shareId: string; keyId: unknown; ticket: unknown }, +): Promise { + if (keyId === undefined || keyId === null) { + throw new UserKeyError(400, 'a swap names the key it puts in'); + } + if (!isUUID(keyId)) { + throw noSuchKey(); + } + if (!isUUID(shareId)) { + throw noSuchShare(); + } + return await dbAdapter.withTransaction(async (txQuery) => { + await consumeKeyWriteTicket(txQuery, user, ticket, { + action: 'swap', + keyId, + realmURL: null, + shareId, + }); + let { row, isWriter } = await lockVisibleShare(txQuery, user, shareId, { + ownKeyId: keyId, + }); + if (!isWriter) { + throw notADirectWriter(); + } + if (row.state !== 'suspended') { + throw new UserKeyError(409, 'only a suspended share takes another key'); + } + let [updated] = await txQuery([ + `UPDATE key_shares SET owner = `, + param(user), + `, key_id = `, + param(keyId), + `, state = 'active', suspension_reason = NULL, suspended_by = NULL, suspended_at = NULL, updated_at = NOW() WHERE id = `, + param(shareId), + ` RETURNING ${SHARE_COLUMNS}`, + ]); + return toShare(updated, { withKey: true }); + }); +} + +// The user's own shares, in every realm, by realm and label. With `keyId`, +// only the shares holding that key: the ones deleting it would suspend. +export async function listOwnShares( + dbAdapter: DBAdapter, + owner: string, + { keyId }: { keyId?: string } = {}, +): Promise { + if (keyId !== undefined && !isUUID(keyId)) { + return []; + } + let rows = await query(dbAdapter, [ + `SELECT ${SHARE_COLUMNS} FROM key_shares WHERE owner = `, + param(owner), + ...(keyId !== undefined ? [` AND key_id = `, param(keyId)] : []), + ` ORDER BY realm_url, label`, + ]); + return rows.map((row) => toShare(row, { withKey: true })); +} + +// Reads `realm`'s rows for one of its direct writers, with their permission +// row locked for the read, so write removed meanwhile can't let them read on. +async function readAsDirectWriter( + dbAdapter: DBAdapter, + user: string, + realm: unknown, + read: (txQuery: Querier, realmURL: URL) => Promise[]>, +): Promise[]> { + let realmURL = validateRealm(realm); + return await dbAdapter.withTransaction(async (txQuery) => { + if (!(await isDirectWriterLocked(txQuery, realmURL, user))) { + throw new UserKeyError( + 403, + `only a realm's direct writers see its shares`, + ); + } + return await read(txQuery, realmURL); + }); +} + +// A realm's shares, by label, for one of its direct writers. +export async function listRealmShares( + dbAdapter: DBAdapter, + user: string, + realm: unknown, +): Promise { + let rows = await readAsDirectWriter( + dbAdapter, + user, + realm, + async (txQuery, realmURL) => + await txQuery([ + `SELECT ${SHARE_COLUMNS} FROM key_shares WHERE realm_url = `, + param(realmURL.href), + ` ORDER BY label`, + ]), + ); + return rows.map((row) => toShare(row, { withKey: row.owner === user })); +} + +// The notices recorded when one of a realm's shares was suspended, or when +// deleting its key changed why it is suspended, newest first, for one of its +// direct writers. +export async function listRealmShareNotices( + dbAdapter: DBAdapter, + user: string, + realm: unknown, +): Promise { + let rows = await readAsDirectWriter( + dbAdapter, + user, + realm, + async (txQuery, realmURL) => + await txQuery([ + `SELECT id, share_id, realm_url, label, owner, reason, suspended_by, created_at FROM key_share_notices WHERE realm_url = `, + param(realmURL.href), + ` ORDER BY created_at DESC, id DESC LIMIT ${MAX_KEY_SHARE_NOTICES}`, + ]), + ); + return rows.map((row) => ({ + id: String(row.id), + shareId: row.share_id as string, + realm: row.realm_url as string, + label: row.label as string, + owner: row.owner as string, + reason: row.reason as KeyShareSuspensionReason, + suspendedBy: (row.suspended_by as string | null) ?? null, + createdAt: iso(row.created_at)!, + })); +} diff --git a/packages/realm-server/lib/request-telemetry.ts b/packages/realm-server/lib/request-telemetry.ts index dd00649cae0..ec305650df6 100644 --- a/packages/realm-server/lib/request-telemetry.ts +++ b/packages/realm-server/lib/request-telemetry.ts @@ -46,15 +46,20 @@ const proxyRoutePaths = new Set([ ]); // The routes that manage a user's own keys: `/_user-keys`, each key beneath -// it, and the key write tickets. With them, the secure-origin sign-in routes, -// whose bodies carry a password or the session they sign in. +// it, the key write tickets, and the shares of keys into realms with their +// notices. With them, the secure-origin sign-in routes, whose bodies carry a +// password or the session they sign in. export const USER_KEYS_PATH = '/_user-keys'; export const KEY_WRITE_TICKETS_PATH = '/_key-write-tickets'; +export const KEY_SHARES_PATH = '/_key-shares'; +export const KEY_SHARE_NOTICES_PATH = '/_key-share-notices'; export const SECURE_SESSION_PATH = '/_secure-session'; const keyRoutePaths = [ USER_KEYS_PATH, KEY_WRITE_TICKETS_PATH, + KEY_SHARES_PATH, + KEY_SHARE_NOTICES_PATH, SECURE_SESSION_PATH, ]; diff --git a/packages/realm-server/lib/user-keys.ts b/packages/realm-server/lib/user-keys.ts index 376246887a4..e0476fbb606 100644 --- a/packages/realm-server/lib/user-keys.ts +++ b/packages/realm-server/lib/user-keys.ts @@ -1,6 +1,7 @@ import { createHash, randomBytes, randomUUID } from 'node:crypto'; import { param, query, type DBAdapter } from '@cardstack/runtime-common'; import type { Expression, Querier } from '@cardstack/runtime-common/expression'; +import { normalizeRealmURL } from '../utils/realm-url.ts'; import type { SealedSecret, SecretBinding, @@ -15,10 +16,12 @@ import type { // A key is only its secret. It records no destination: nothing here says // where a key may be sent, and the value is sealed to the key alone. // -// A key is added, replaced or deleted only with a key write ticket: -// short-lived, single-use, minted for the signed-in user and for one change -// (an add, or a replace or delete of one key), and consumed in the same -// transaction as that change. The routes mint tickets only for requests from +// A key is added, replaced, deleted, shared into a realm or swapped into a +// share, and a share suspended by a writer is resumed, only with a key write +// ticket: short-lived, single-use, minted for the signed-in user and for one +// change (an add; a replace or delete of one key; a share of one key into one +// realm; a swap of one key into one share; or a resume of one share), and +// consumed in the same transaction as that change. The routes mint tickets only for requests from // the secure origin and for clients that send no `Origin`, as the CLI does, so // card code in the app page can't change a key from inside the browser. @@ -29,7 +32,14 @@ export type UserKeyKind = (typeof USER_KEY_KINDS)[number]; export type UserKeyState = 'active'; -export const KEY_WRITE_ACTIONS = ['add', 'replace', 'delete'] as const; +export const KEY_WRITE_ACTIONS = [ + 'add', + 'replace', + 'delete', + 'share', + 'swap', + 'resume', +] as const; export type KeyWriteAction = (typeof KEY_WRITE_ACTIONS)[number]; export const KEY_WRITE_TICKET_TTL_MS = 10 * 60 * 1000; @@ -65,45 +75,94 @@ function sha256(value: string): string { return createHash('sha256').update(value, 'utf8').digest('hex'); } -// The change a key write ticket is minted for: an add, or a replace or delete -// of one key. +// The change a key write ticket is minted for: an add; a replace or delete of +// one key; a share of one key into one realm; a swap of one key into one +// share; or a resume of one share. export interface KeyWriteTarget { action: KeyWriteAction; - // The key a replace or delete changes; an add names none. + // The key a replace, delete, share or swap uses; an add or a resume names + // none. keyId: string | null; + // The realm a share puts the key into; only a share names one. + realmURL: string | null; + // The share a swap puts the key into, or a resume resumes; only those name + // one. + shareId: string | null; } -export function validateKeyWriteTarget( - action: unknown, - keyId: unknown, -): KeyWriteTarget { +export function validateKeyWriteTarget({ + action, + keyId, + realm, + shareId, +}: { + action: unknown; + keyId?: unknown; + realm?: unknown; + shareId?: unknown; +}): KeyWriteTarget { if (!KEY_WRITE_ACTIONS.includes(action as KeyWriteAction)) { throw new UserKeyError( 400, `a ticket's action is one of: ${KEY_WRITE_ACTIONS.join(', ')}`, ); } - if (action === 'add') { - if (keyId !== undefined && keyId !== null) { - throw new UserKeyError(400, 'a ticket to add a key names no key'); + let named = (value: unknown) => value !== undefined && value !== null; + if (named(realm) && action !== 'share') { + throw new UserKeyError(400, `a ticket to ${action} names no realm`); + } + if (named(shareId) && action !== 'swap' && action !== 'resume') { + throw new UserKeyError(400, `a ticket to ${action} names no share`); + } + if (action === 'add' || action === 'resume') { + if (named(keyId)) { + throw new UserKeyError(400, `a ticket to ${action} names no key`); + } + if (action === 'add') { + return { action, keyId: null, realmURL: null, shareId: null }; } - return { action, keyId: null }; + return { + action, + keyId: null, + realmURL: null, + shareId: validShareId(shareId, action), + }; } if (typeof keyId !== 'string') { throw new UserKeyError(400, `a ticket to ${action} a key names the key`); } - return { action: action as KeyWriteAction, keyId }; + if (action === 'share') { + let realmURL = typeof realm === 'string' ? normalizeRealmURL(realm) : null; + if (!realmURL) { + throw new UserKeyError(400, 'a ticket to share a key names the realm'); + } + return { action, keyId, realmURL: realmURL.href, shareId: null }; + } + if (action === 'swap') { + return { + action, + keyId, + realmURL: null, + shareId: validShareId(shareId, action), + }; + } + return { + action: action as KeyWriteAction, + keyId, + realmURL: null, + shareId: null, + }; } -// Mints a ticket for `owner` and one change. A ticket to replace or delete a -// key is minted only for a key the owner has; another user's key answers as -// one that doesn't exist. Only the ticket's SHA-256 is stored, so the ticket +// Mints a ticket for `owner` and one change. A ticket that names a key is +// minted only for a key the owner has; another user's key answers as one that +// doesn't exist. Only the ticket's SHA-256 is stored, so the ticket // itself exists only in the response that carries it to the caller. Expired // tickets are swept on the way. export async function mintKeyWriteTicket( dbAdapter: DBAdapter, owner: string, - { action, keyId }: KeyWriteTarget, + { action, keyId, realmURL, shareId }: KeyWriteTarget, ): Promise<{ ticket: string; expiresAt: string }> { if (keyId !== null) { await getUserKey(dbAdapter, owner, keyId); @@ -114,7 +173,7 @@ export async function mintKeyWriteTicket( `DELETE FROM key_write_tickets WHERE expires_at <= NOW()`, ]); await query(dbAdapter, [ - `INSERT INTO key_write_tickets (ticket_sha256, owner, action, key_id, expires_at) VALUES (`, + `INSERT INTO key_write_tickets (ticket_sha256, owner, action, key_id, realm_url, share_id, expires_at) VALUES (`, param(sha256(ticket)), `, `, param(owner), @@ -123,6 +182,10 @@ export async function mintKeyWriteTicket( `, `, param(keyId), `, `, + param(realmURL), + `, `, + param(shareId), + `, `, param(expiresAt.toISOString()), `)`, ]); @@ -131,14 +194,14 @@ export async function mintKeyWriteTicket( // Consumes `ticket` inside the caller's transaction, so a write that fails // afterwards leaves the ticket unused. A ticket spends only on the change it -// was minted for: its owner, its action and, for a replace or delete, its key. -// Deleting the row is what makes a ticket single-use: a concurrent second use -// waits on the row and then finds it gone. -async function consumeKeyWriteTicket( +// was minted for: its owner, its action and whatever that action names (its +// key, realm or share). Deleting the row is what makes a ticket single-use: a +// concurrent second use waits on the row and then finds it gone. +export async function consumeKeyWriteTicket( txQuery: Querier, owner: string, ticket: unknown, - { action, keyId }: KeyWriteTarget, + { action, keyId, realmURL, shareId }: KeyWriteTarget, ): Promise { if (typeof ticket !== 'string' || ticket.length === 0) { throw new UserKeyError(403, INVALID_TICKET_MESSAGE); @@ -150,9 +213,9 @@ async function consumeKeyWriteTicket( param(owner), ` AND action = `, param(action), - ...(keyId === null - ? [` AND key_id IS NULL`] - : [` AND key_id = `, param(keyId)]), + ...matches('key_id', keyId), + ...matches('realm_url', realmURL), + ...matches('share_id', shareId), ` AND expires_at > NOW() RETURNING ticket_sha256`, ]); if (rows.length === 0) { @@ -160,6 +223,23 @@ async function consumeKeyWriteTicket( } } +// A share's id is a UUID; anything else names no share. +function validShareId(shareId: unknown, action: KeyWriteAction): string { + if (typeof shareId !== 'string') { + throw new UserKeyError(400, `a ticket to ${action} names the share`); + } + if (!UUID.test(shareId)) { + throw new UserKeyError(404, 'no such share'); + } + return shareId; +} + +function matches(column: string, value: string | null): Expression { + return value === null + ? [` AND ${column} IS NULL`] + : [` AND ${column} = `, param(value)]; +} + export function validateUserKeyName(name: unknown): string { if (typeof name !== 'string') { throw new UserKeyError(400, 'a key needs a name'); @@ -309,6 +389,8 @@ export async function addUserKey( await consumeKeyWriteTicket(txQuery, owner, ticket, { action: 'add', keyId: null, + realmURL: null, + shareId: null, }); let sealed = keyring.seal(validValue, binding(owner, id)); let rows = await txQuery([ @@ -348,6 +430,8 @@ export async function replaceUserKeyValue( await consumeKeyWriteTicket(txQuery, owner, ticket, { action: 'replace', keyId: id, + realmURL: null, + shareId: null, }); let sealed = keyring.seal(validValue, binding(owner, id)); let [updated] = await txQuery([ @@ -366,8 +450,10 @@ export async function replaceUserKeyValue( }); } -// Deletes a key. Nothing else refers to a key: no table records a key shared -// into a realm or approved for an operation. +// Deletes a key. Every share holding it is suspended in the same statement, +// by a trigger on `user_keys`, and keeps its place in its realm for a direct +// writer to swap another key into; `listOwnShares` with the key's id lists +// the shares a delete would suspend. export async function deleteUserKey( dbAdapter: DBAdapter, { owner, id, ticket }: { owner: string; id: string; ticket: unknown }, @@ -377,6 +463,8 @@ export async function deleteUserKey( await consumeKeyWriteTicket(txQuery, owner, ticket, { action: 'delete', keyId: id, + realmURL: null, + shareId: null, }); let rows = await txQuery([ `DELETE FROM user_keys WHERE id = `, diff --git a/packages/realm-server/routes.ts b/packages/realm-server/routes.ts index ad0f7f1172c..65c6679a30a 100644 --- a/packages/realm-server/routes.ts +++ b/packages/realm-server/routes.ts @@ -103,6 +103,8 @@ import handleCapture from './handlers/handle-capture.ts'; import { buildCreatePrerenderAuth } from './prerender/auth.ts'; import type { RealmRegistryReconciler } from './lib/realm-registry-reconciler.ts'; import { + KEY_SHARE_NOTICES_PATH, + KEY_SHARES_PATH, KEY_WRITE_TICKETS_PATH, OPENROUTER_PASSTHROUGH_PATH, REQUEST_FORWARD_PATH, @@ -125,6 +127,15 @@ import { handleReplaceUserKey, handleShowUserKey, } from './handlers/handle-user-keys.ts'; +import { + handleListKeyShareNotices, + handleListKeyShares, + handleResumeKeyShare, + handleShareKey, + handleSuspendKeyShare, + handleSwapKeyIntoShare, + handleUnshareKey, +} from './handlers/handle-key-shares.ts'; export type CreateRoutesArgs = { serverURL: string; @@ -310,6 +321,41 @@ export function createRoutes(args: CreateRoutesArgs) { jwtMiddleware(args.realmSecretSeed, args.dbAdapter), handleDeleteUserKey(args), ); + router.get( + KEY_SHARES_PATH, + jwtMiddleware(args.realmSecretSeed, args.dbAdapter), + handleListKeyShares(args), + ); + router.post( + KEY_SHARES_PATH, + jwtMiddleware(args.realmSecretSeed, args.dbAdapter), + handleShareKey(args), + ); + router.delete( + `${KEY_SHARES_PATH}/:shareId`, + jwtMiddleware(args.realmSecretSeed, args.dbAdapter), + handleUnshareKey(args), + ); + router.post( + `${KEY_SHARES_PATH}/:shareId/suspend`, + jwtMiddleware(args.realmSecretSeed, args.dbAdapter), + handleSuspendKeyShare(args), + ); + router.post( + `${KEY_SHARES_PATH}/:shareId/resume`, + jwtMiddleware(args.realmSecretSeed, args.dbAdapter), + handleResumeKeyShare(args), + ); + router.post( + `${KEY_SHARES_PATH}/:shareId/swap`, + jwtMiddleware(args.realmSecretSeed, args.dbAdapter), + handleSwapKeyIntoShare(args), + ); + router.get( + KEY_SHARE_NOTICES_PATH, + jwtMiddleware(args.realmSecretSeed, args.dbAdapter), + handleListKeyShareNotices(args), + ); router.post( REQUEST_FORWARD_PATH, jwtMiddleware(args.realmSecretSeed, args.dbAdapter), diff --git a/packages/realm-server/tests/key-shares-test.ts b/packages/realm-server/tests/key-shares-test.ts new file mode 100644 index 00000000000..687e65da2eb --- /dev/null +++ b/packages/realm-server/tests/key-shares-test.ts @@ -0,0 +1,1043 @@ +import QUnit from 'qunit'; +const { module, test } = QUnit; +import { basename, dirname, join } from 'path'; +import { createRequire } from 'module'; +import type { SuperTest, Test } from 'supertest'; +import supertest from 'supertest'; +import type { PgAdapter } from '@cardstack/postgres'; +import { + insertPermissions, + removeRealmPermissions, +} from '@cardstack/runtime-common'; +import { removeRealmDatabaseArtifacts } from '../handlers/realm-destruction-utils.ts'; +import { + setupDB, + setupSimpleRealmServerTemplate, + startSimpleRealmServer, + closeServer, + realmSecretSeed, +} from './helpers/index.ts'; +import { secureSessionAuth } from './helpers/secure-session.ts'; +import { createJWT as createRealmServerJWT } from '../utils/jwt.ts'; +import type { RealmHttpServer as Server } from '../server.ts'; +import { + installSecretsKeyring, + LocalKeyProvider, + resetSecretsAtRest, + type SecretsKeyring, +} from '../lib/secrets-at-rest/index.ts'; +import { addUserKey, mintKeyWriteTicket } from '../lib/user-keys.ts'; + +const require = createRequire(import.meta.url); +const KEY_SHARES_MIGRATION = join( + dirname(require.resolve('@cardstack/postgres/package.json')), + 'migrations', + '1792187770930_add-key-shares.js', +); + +const OWNER = '@share-owner:localhost'; +const WRITER = '@other-writer:localhost'; +const READER = '@realm-reader:localhost'; +const STRANGER = '@stranger:localhost'; +const REALM = 'http://127.0.0.1:4444/shared-realm/'; +const OTHER_REALM = 'http://127.0.0.1:4444/other-realm/'; + +module(basename(import.meta.filename), function () { + module('key shares API', function (hooks) { + let testRealmServer: Server; + let request: SuperTest; + let dbAdapter: PgAdapter; + let keyring: SecretsKeyring; + let secureOriginURL = new URL('https://secure.keys.test/secure'); + + let templateDatabase = setupSimpleRealmServerTemplate(hooks); + + setupDB(hooks, { + templateDatabase, + beforeEach: async (_dbAdapter, publisher, runner) => { + dbAdapter = _dbAdapter; + keyring = installSecretsKeyring( + await new LocalKeyProvider().loadDataKeys(), + ); + testRealmServer = await startSimpleRealmServer({ + dbAdapter, + publisher, + runner, + secureOriginURL, + }); + request = supertest(testRealmServer); + await insertPermissions(dbAdapter, new URL(REALM), { + [OWNER]: ['read', 'write'], + [WRITER]: ['read', 'write'], + [READER]: ['read'], + }); + await insertPermissions(dbAdapter, new URL(OTHER_REALM), { + [OWNER]: ['read', 'write'], + }); + }, + afterEach: async () => { + await closeServer(testRealmServer); + resetSecretsAtRest(); + }, + }); + + function auth(user: string): string { + return `Bearer ${createRealmServerJWT( + { user, sessionRoom: 'test-session' }, + realmSecretSeed, + )}`; + } + + async function keyFor(user: string, name = 'weather'): Promise { + let { ticket } = await mintKeyWriteTicket(dbAdapter, user, { + action: 'add', + keyId: null, + realmURL: null, + shareId: null, + }); + let key = await addUserKey(dbAdapter, keyring, { + owner: user, + name, + value: `sk-${name}-value`, + ticket, + }); + return key.id; + } + + async function mint(user: string, attributes: Record) { + return await request + .post('/_key-write-tickets') + .set('Authorization', await secureSessionAuth(dbAdapter, user)) + .set('Content-Type', 'application/json') + .send({ data: { type: 'key-write-ticket', attributes } }); + } + + async function ticketFor( + user: string, + attributes: Record, + ): Promise { + let response = await mint(user, attributes); + if (response.status !== 201) { + throw new Error( + `could not mint a ticket: ${response.status} ${response.text}`, + ); + } + return response.body.data.attributes.ticket; + } + + async function share( + user: string, + attributes: { realm: string; keyId: string; label: string }, + ticket?: string, + ) { + ticket ??= await ticketFor(user, { + action: 'share', + keyId: attributes.keyId, + realm: attributes.realm, + }); + return await request + .post('/_key-shares') + .set('Authorization', auth(user)) + .set('Content-Type', 'application/json') + .send({ data: { type: 'key-share', attributes }, meta: { ticket } }); + } + + async function sharedId( + user = OWNER, + label = 'Weather', + realm = REALM, + ): Promise { + let keyId = await keyFor(user, `${label}-key`); + let response = await share(user, { realm, keyId, label }); + if (response.status !== 201) { + throw new Error( + `could not share a key: ${response.status} ${response.text}`, + ); + } + return response.body.data.id; + } + + async function swap( + user: string, + shareId: string, + keyId: string, + ticket?: string, + ) { + ticket ??= await ticketFor(user, { action: 'swap', keyId, shareId }); + return await request + .post(`/_key-shares/${shareId}/swap`) + .set('Authorization', auth(user)) + .set('Content-Type', 'application/json') + .send({ + data: { type: 'key-share', attributes: { keyId } }, + meta: { ticket }, + }); + } + + async function act(user: string, shareId: string, action: string) { + return await request + .post(`/_key-shares/${shareId}/${action}`) + .set('Authorization', auth(user)); + } + + async function resume(user: string, shareId: string, ticket?: string) { + ticket ??= await ticketFor(user, { action: 'resume', shareId }); + return await request + .post(`/_key-shares/${shareId}/resume`) + .set('Authorization', auth(user)) + .set('Content-Type', 'application/json') + .send({ meta: { ticket } }); + } + + async function realmShares(user: string, realm = REALM) { + return await request + .get(`/_key-shares?realm=${encodeURIComponent(realm)}`) + .set('Authorization', auth(user)); + } + + async function notices(user: string, realm = REALM) { + return await request + .get(`/_key-share-notices?realm=${encodeURIComponent(realm)}`) + .set('Authorization', auth(user)); + } + + async function shareState(shareId: string) { + let [row] = await dbAdapter.execute( + `SELECT owner, key_id, state, suspension_reason FROM key_shares WHERE id = $1`, + { bind: [shareId] }, + ); + return row + ? { + owner: row.owner, + keyId: row.key_id, + state: row.state, + reason: row.suspension_reason, + } + : undefined; + } + + test('a direct writer shares their own key into the realm', async function (assert) { + let keyId = await keyFor(OWNER); + let response = await share(OWNER, { + realm: REALM, + keyId, + label: 'Weather', + }); + assert.strictEqual(response.status, 201, response.text); + let { id, attributes } = response.body.data; + assert.strictEqual(response.body.data.type, 'key-share'); + assert.deepEqual( + { + realm: attributes.realm, + label: attributes.label, + owner: attributes.owner, + keyId: attributes.keyId, + state: attributes.state, + suspensionReason: attributes.suspensionReason, + }, + { + realm: REALM, + label: 'Weather', + owner: OWNER, + keyId, + state: 'active', + suspensionReason: null, + }, + ); + assert.notOk( + JSON.stringify(response.body).includes('sk-'), + 'no value is returned', + ); + + let own = await request + .get('/_key-shares') + .set('Authorization', auth(OWNER)); + assert.deepEqual( + own.body.data.map((s: { id: string }) => s.id), + [id], + 'the owner sees the share', + ); + + let listed = await realmShares(WRITER); + assert.strictEqual(listed.status, 200, listed.text); + assert.deepEqual( + listed.body.data.map( + (s: { attributes: Record }) => s.attributes, + ), + [ + { + realm: REALM, + label: 'Weather', + owner: OWNER, + state: 'active', + suspensionReason: null, + suspendedBy: null, + suspendedAt: null, + createdAt: attributes.createdAt, + updatedAt: attributes.updatedAt, + }, + ], + `another writer sees the share's owner and state, not its key`, + ); + }); + + test('one key is shared into several realms, its label unique within each', async function (assert) { + let keyId = await keyFor(OWNER); + let first = await share(OWNER, { realm: REALM, keyId, label: 'Weather' }); + assert.strictEqual(first.status, 201, first.text); + let other = await share(OWNER, { + realm: OTHER_REALM, + keyId, + label: 'Weather', + }); + assert.strictEqual(other.status, 201, 'the same label in another realm'); + + let anotherKey = await keyFor(OWNER, 'second'); + let clash = await share(OWNER, { + realm: REALM, + keyId: anotherKey, + label: 'Weather', + }); + assert.strictEqual(clash.status, 409, 'a label is unique in its realm'); + }); + + test('a user who is not a direct writer of the realm cannot share into it', async function (assert) { + let readerKey = await keyFor(READER); + let byReader = await share(READER, { + realm: REALM, + keyId: readerKey, + label: 'Weather', + }); + assert.strictEqual(byReader.status, 403, 'a reader is refused'); + + let strangerKey = await keyFor(STRANGER); + let byStranger = await share(STRANGER, { + realm: REALM, + keyId: strangerKey, + label: 'Weather', + }); + assert.strictEqual(byStranger.status, 403, 'a stranger is refused'); + + await insertPermissions(dbAdapter, new URL(REALM), { + '*': ['read', 'write'], + }); + let byWildcard = await share(STRANGER, { + realm: REALM, + keyId: strangerKey, + label: 'Weather', + }); + assert.strictEqual( + byWildcard.status, + 403, + 'write through a `*` row does not make a direct writer', + ); + let [{ count }] = await dbAdapter.execute( + `SELECT COUNT(*)::int AS count FROM key_shares`, + ); + assert.strictEqual(count, 0, 'nothing was shared'); + }); + + test('only the owner shares a key, with a ticket for that key and realm', async function (assert) { + let ownerKey = await keyFor(OWNER); + let minted = await mint(WRITER, { + action: 'share', + keyId: ownerKey, + realm: REALM, + }); + assert.strictEqual( + minted.status, + 404, + `no ticket is minted to share another user's key`, + ); + + let writerTicket = await ticketFor(WRITER, { + action: 'share', + keyId: await keyFor(WRITER), + realm: REALM, + }); + let withOwnTicket = await share( + WRITER, + { realm: REALM, keyId: ownerKey, label: 'Weather' }, + writerTicket, + ); + assert.strictEqual( + withOwnTicket.status, + 403, + `a ticket for one key doesn't share another`, + ); + + let otherRealmTicket = await ticketFor(OWNER, { + action: 'share', + keyId: ownerKey, + realm: OTHER_REALM, + }); + let wrongRealm = await share( + OWNER, + { realm: REALM, keyId: ownerKey, label: 'Weather' }, + otherRealmTicket, + ); + assert.strictEqual( + wrongRealm.status, + 403, + `a ticket for one realm doesn't share into another`, + ); + + let noTicket = await request + .post('/_key-shares') + .set('Authorization', auth(OWNER)) + .set('Content-Type', 'application/json') + .send({ + data: { + type: 'key-share', + attributes: { realm: REALM, keyId: ownerKey, label: 'Weather' }, + }, + }); + assert.strictEqual(noTicket.status, 403, 'a share needs a ticket'); + }); + + test(`removing the owner's write suspends their shares, and restoring it resumes them`, async function (assert) { + let shareId = await sharedId(); + + await insertPermissions(dbAdapter, new URL(REALM), { + [OWNER]: ['read'], + }); + let afterLoss = await shareState(shareId); + assert.deepEqual( + [afterLoss?.owner, afterLoss?.state, afterLoss?.reason], + [OWNER, 'suspended', 'owner-lost-write'], + 'losing write suspends the share, which is kept', + ); + + await insertPermissions(dbAdapter, new URL(REALM), { + [OWNER]: ['read', 'write'], + }); + assert.strictEqual( + (await shareState(shareId))!.state, + 'active', + 'regaining write resumes it', + ); + + await insertPermissions(dbAdapter, new URL(REALM), { [OWNER]: [] }); + let afterLeaving = await shareState(shareId); + assert.deepEqual( + [afterLeaving?.state, afterLeaving?.reason], + ['suspended', 'owner-lost-write'], + 'leaving the realm suspends it', + ); + + let listed = await notices(WRITER); + assert.strictEqual(listed.status, 200, listed.text); + assert.deepEqual( + listed.body.data.map((n: { attributes: Record }) => [ + n.attributes.shareId, + n.attributes.label, + n.attributes.owner, + n.attributes.reason, + ]), + [ + [shareId, 'Weather', OWNER, 'owner-lost-write'], + [shareId, 'Weather', OWNER, 'owner-lost-write'], + ], + `the realm's direct writers get a notice of each suspension`, + ); + + let formerWriter = await realmShares(OWNER); + assert.strictEqual( + formerWriter.status, + 403, + `the former writer no longer sees the realm's shares`, + ); + let own = await request + .get('/_key-shares') + .set('Authorization', auth(OWNER)); + assert.deepEqual( + own.body.data.map( + (s: { id: string; attributes: { state: string } }) => [ + s.id, + s.attributes.state, + ], + ), + [[shareId, 'suspended']], + 'the owner still sees where their key is shared', + ); + }); + + test('a writer suspended share stays suspended when the owner regains write', async function (assert) { + let shareId = await sharedId(); + let suspended = await act(WRITER, shareId, 'suspend'); + assert.strictEqual(suspended.status, 200, suspended.text); + await insertPermissions(dbAdapter, new URL(REALM), { + [OWNER]: ['read'], + }); + await insertPermissions(dbAdapter, new URL(REALM), { + [OWNER]: ['read', 'write'], + }); + let state = await shareState(shareId); + assert.deepEqual( + [state?.state, state?.reason], + ['suspended', 'suspended-by-writer'], + ); + }); + + test('any direct writer suspends a share, and lifts that suspension', async function (assert) { + let shareId = await sharedId(); + + let byReader = await act(READER, shareId, 'suspend'); + assert.strictEqual(byReader.status, 404, `a reader can't see the share`); + + let suspended = await act(WRITER, shareId, 'suspend'); + assert.strictEqual(suspended.status, 200, suspended.text); + assert.deepEqual( + [ + suspended.body.data.attributes.state, + suspended.body.data.attributes.suspensionReason, + suspended.body.data.attributes.suspendedBy, + ], + ['suspended', 'suspended-by-writer', WRITER], + ); + assert.strictEqual( + (await act(WRITER, shareId, 'suspend')).status, + 409, + 'a suspended share is not suspended again', + ); + let listed = await notices(OWNER); + assert.deepEqual( + listed.body.data.map((n: { attributes: Record }) => [ + n.attributes.reason, + n.attributes.suspendedBy, + ]), + [['suspended-by-writer', WRITER]], + 'the suspension is noticed', + ); + + let noTicket = await request + .post(`/_key-shares/${shareId}/resume`) + .set('Authorization', auth(OWNER)) + .set('Content-Type', 'application/json') + .send({}); + assert.strictEqual(noTicket.status, 403, 'lifting one needs a ticket'); + let otherShare = await sharedId(OWNER, 'Other'); + let otherTicket = await ticketFor(OWNER, { + action: 'resume', + shareId: otherShare, + }); + assert.strictEqual( + (await resume(OWNER, shareId, otherTicket)).status, + 403, + `a ticket to resume one share doesn't resume another`, + ); + + let resumed = await resume(OWNER, shareId); + assert.strictEqual(resumed.status, 200, resumed.text); + assert.strictEqual(resumed.body.data.attributes.state, 'active'); + }); + + test('a share suspended by a writer, whose owner has lost write, stays suspended for that when lifted', async function (assert) { + let shareId = await sharedId(); + await act(WRITER, shareId, 'suspend'); + await insertPermissions(dbAdapter, new URL(REALM), { + [OWNER]: ['read'], + }); + let resumed = await resume(WRITER, shareId); + assert.strictEqual(resumed.status, 200, resumed.text); + assert.deepEqual( + [ + resumed.body.data.attributes.state, + resumed.body.data.attributes.suspensionReason, + ], + ['suspended', 'owner-lost-write'], + ); + await insertPermissions(dbAdapter, new URL(REALM), { + [OWNER]: ['read', 'write'], + }); + assert.strictEqual((await shareState(shareId))!.state, 'active'); + }); + + test('a remaining direct writer swaps their own key into a suspended share', async function (assert) { + let shareId = await sharedId(); + let writerKey = await keyFor(WRITER); + + let whileActive = await swap(WRITER, shareId, writerKey); + assert.strictEqual( + whileActive.status, + 409, + 'an active share takes no other key', + ); + + await insertPermissions(dbAdapter, new URL(REALM), { [OWNER]: [] }); + + let swapped = await swap(WRITER, shareId, writerKey); + assert.strictEqual(swapped.status, 200, swapped.text); + let { attributes } = swapped.body.data; + assert.deepEqual( + [ + attributes.label, + attributes.owner, + attributes.keyId, + attributes.state, + attributes.suspensionReason, + ], + ['Weather', WRITER, writerKey, 'active', null], + 'the share keeps its label and takes the writer as owner', + ); + + await insertPermissions(dbAdapter, new URL(REALM), { + [WRITER]: ['read'], + }); + assert.strictEqual( + (await shareState(shareId))!.reason, + 'owner-lost-write', + 'it follows its new owner', + ); + }); + + test(`a non-writer can't swap, and nobody swaps in another user's key`, async function (assert) { + let shareId = await sharedId(); + await insertPermissions(dbAdapter, new URL(REALM), { [OWNER]: [] }); + + let readerKey = await keyFor(READER); + let byReader = await swap(READER, shareId, readerKey); + assert.strictEqual(byReader.status, 404, `a reader can't see the share`); + + await insertPermissions(dbAdapter, new URL(REALM), { + [STRANGER]: ['read'], + }); + let strangerKey = await keyFor(STRANGER); + let byStranger = await swap(STRANGER, shareId, strangerKey); + assert.strictEqual(byStranger.status, 404); + + let readerKeyTicket = await mint(WRITER, { + action: 'swap', + keyId: readerKey, + shareId, + }); + assert.strictEqual( + readerKeyTicket.status, + 404, + `no ticket is minted to swap in another user's key`, + ); + let writerTicket = await ticketFor(WRITER, { + action: 'swap', + keyId: await keyFor(WRITER), + shareId, + }); + let withOwnTicket = await swap(WRITER, shareId, readerKey, writerTicket); + assert.strictEqual( + withOwnTicket.status, + 403, + `a ticket for the writer's key doesn't swap in another`, + ); + + let otherShare = await sharedId(WRITER, 'Other'); + await act(WRITER, otherShare, 'suspend'); + let writerKey = await keyFor(WRITER, 'third'); + let otherShareTicket = await ticketFor(WRITER, { + action: 'swap', + keyId: writerKey, + shareId: otherShare, + }); + let wrongShare = await swap(WRITER, shareId, writerKey, otherShareTicket); + assert.strictEqual( + wrongShare.status, + 403, + `a ticket to swap into one share doesn't swap into another`, + ); + + let state = await shareState(shareId); + assert.deepEqual( + [state!.owner, state!.state], + [OWNER, 'suspended'], + 'the share is unchanged', + ); + }); + + test('deleting a key lists the shares it would affect, and suspends them', async function (assert) { + let keyId = await keyFor(OWNER); + let inRealm = await share(OWNER, { realm: REALM, keyId, label: 'A' }); + let inOther = await share(OWNER, { + realm: OTHER_REALM, + keyId, + label: 'B', + }); + await sharedId(OWNER, 'Unrelated'); + + let affected = await request + .get(`/_key-shares?key=${keyId}`) + .set('Authorization', auth(OWNER)); + assert.strictEqual(affected.status, 200, affected.text); + assert.deepEqual( + affected.body.data.map((s: { id: string }) => s.id).sort(), + [inRealm.body.data.id, inOther.body.data.id].sort(), + 'the shares holding the key are listed', + ); + + let ticket = await ticketFor(OWNER, { action: 'delete', keyId }); + let deleted = await request + .delete(`/_user-keys/${keyId}`) + .set('Authorization', auth(OWNER)) + .set('Content-Type', 'application/json') + .send({ meta: { ticket } }); + assert.strictEqual(deleted.status, 204, deleted.text); + + for (let id of [inRealm.body.data.id, inOther.body.data.id]) { + assert.deepEqual( + await shareState(id), + { + owner: OWNER, + keyId: null, + state: 'suspended', + reason: 'key-deleted', + }, + 'the share is suspended and keeps its place', + ); + } + let listed = await notices(WRITER); + assert.deepEqual( + listed.body.data.map( + (n: { attributes: Record }) => n.attributes.reason, + ), + ['key-deleted'], + ); + + let writerKey = await keyFor(WRITER); + let swapped = await swap(WRITER, inRealm.body.data.id, writerKey); + assert.strictEqual(swapped.status, 200, 'a writer swaps a key into it'); + }); + + test(`deleting the key of a share a writer suspended records a notice`, async function (assert) { + let keyId = await keyFor(OWNER); + let shared = await share(OWNER, { realm: REALM, keyId, label: 'A' }); + let shareId = shared.body.data.id; + await act(WRITER, shareId, 'suspend'); + + let ticket = await ticketFor(OWNER, { action: 'delete', keyId }); + await request + .delete(`/_user-keys/${keyId}`) + .set('Authorization', auth(OWNER)) + .set('Content-Type', 'application/json') + .send({ meta: { ticket } }); + + let state = await shareState(shareId); + assert.deepEqual( + [state?.state, state?.reason, state?.keyId], + ['suspended', 'key-deleted', null], + ); + let listed = await notices(WRITER); + assert.deepEqual( + listed.body.data.map( + (n: { attributes: Record }) => n.attributes.reason, + ), + ['key-deleted', 'suspended-by-writer'], + 'the writers learn the key is gone', + ); + }); + + test('deleting a realm deletes its shares', async function (assert) { + let shareId = await sharedId(); + let elsewhere = await sharedId(OWNER, 'Elsewhere', OTHER_REALM); + await removeRealmPermissions(dbAdapter, new URL(REALM)); + await removeRealmDatabaseArtifacts({ dbAdapter, realmURL: REALM }); + assert.strictEqual(await shareState(shareId), undefined); + let [{ count }] = await dbAdapter.execute( + `SELECT COUNT(*)::int AS count FROM key_share_notices WHERE share_id = $1`, + { bind: [shareId] }, + ); + assert.strictEqual(count, 0, 'its notices go with it'); + + await insertPermissions(dbAdapter, new URL(REALM), { + [OWNER]: ['read', 'write'], + }); + assert.deepEqual( + ( + await request.get('/_key-shares').set('Authorization', auth(OWNER)) + ).body.data.map((s: { id: string }) => s.id), + [elsewhere], + 'a realm later made at the same URL gets none of them back', + ); + }); + + test('ids that are not UUIDs name nothing', async function (assert) { + let shareId = await sharedId(); + let keyId = await keyFor(WRITER); + assert.strictEqual( + (await mint(WRITER, { action: 'swap', keyId, shareId: 'nope' })).status, + 404, + 'no swap ticket is minted for a malformed share id', + ); + assert.strictEqual( + (await mint(WRITER, { action: 'resume', shareId: 'nope' })).status, + 404, + 'no resume ticket is minted for a malformed share id', + ); + let ticket = await ticketFor(WRITER, { action: 'swap', keyId, shareId }); + assert.strictEqual( + (await swap(WRITER, 'nope', keyId, ticket)).status, + 404, + ); + assert.strictEqual( + (await swap(WRITER, shareId, 'nope', ticket)).status, + 404, + ); + assert.strictEqual((await act(WRITER, 'nope', 'suspend')).status, 404); + assert.strictEqual((await resume(WRITER, 'nope', ticket)).status, 404); + assert.strictEqual( + ( + await share( + WRITER, + { realm: REALM, keyId: 'nope', label: 'X' }, + ticket, + ) + ).status, + 404, + ); + assert.strictEqual( + (await swap(WRITER, shareId, keyId, ticket)).status, + 409, + 'the refusals left the ticket unspent', + ); + }); + + test(`a reader sees nothing of a realm's shares`, async function (assert) { + let shareId = await sharedId(); + for (let user of [READER, STRANGER]) { + assert.strictEqual((await realmShares(user)).status, 403); + assert.strictEqual((await notices(user)).status, 403); + let own = await request + .get('/_key-shares') + .set('Authorization', auth(user)); + assert.deepEqual(own.body.data, [], `${user} owns no shares`); + assert.strictEqual((await resume(user, shareId)).status, 404); + let unshare = await request + .delete(`/_key-shares/${shareId}`) + .set('Authorization', auth(user)); + assert.strictEqual(unshare.status, 404); + } + }); + + test('only its owner unshares a share, with or without write', async function (assert) { + let shareId = await sharedId(); + let byWriter = await request + .delete(`/_key-shares/${shareId}`) + .set('Authorization', auth(WRITER)); + assert.strictEqual(byWriter.status, 403, 'another writer is refused'); + + await insertPermissions(dbAdapter, new URL(REALM), { [OWNER]: [] }); + let byOwner = await request + .delete(`/_key-shares/${shareId}`) + .set('Authorization', auth(OWNER)); + assert.strictEqual(byOwner.status, 204, byOwner.text); + assert.strictEqual(await shareState(shareId), undefined); + }); + }); + + module('key shares tables', function (hooks) { + let dbAdapter: PgAdapter; + + // Roles and memberships are shared by every database on the server, so + // each test removes the memberships it makes, whatever happens. + const MEMBER = 'readonly_member_for_key_shares_test'; + const ADMIN_MEMBER = 'readonly_admin_member_for_key_shares_test'; + const OWNER = 'readonly_owner_for_key_shares_test'; + const OTHER_ROLE = 'other_role_for_key_shares_test'; + const TABLES = ['key_shares', 'key_share_notices']; + + setupDB(hooks, { + beforeEach: async (_dbAdapter) => { + dbAdapter = _dbAdapter; + // The role and its blanket grant as staging and production have them. + await createRole('readonly_role'); + await dbAdapter.execute( + `GRANT SELECT ON ALL TABLES IN SCHEMA public TO readonly_role`, + ); + }, + }); + + async function createRole(name: string) { + await dbAdapter.execute( + `DO $$ + BEGIN + CREATE ROLE ${name}; + EXCEPTION WHEN duplicate_object OR unique_violation THEN NULL; + END + $$`, + ); + } + + async function runUp() { + let statements: string[] = []; + require(KEY_SHARES_MIGRATION).up({ + sql: (statement: string) => statements.push(statement), + }); + for (let statement of statements) { + await dbAdapter.execute(statement); + } + } + + async function withMemberships( + memberships: [role: string, member: string, options?: string][], + fn: () => Promise, + ) { + for (let [role, member] of memberships) { + await createRole(role); + await createRole(member); + } + try { + for (let [role, member, options] of memberships) { + await dbAdapter.execute( + `GRANT ${role} TO ${member}${options ? ` WITH ${options}` : ''}`, + ); + } + await fn(); + } finally { + for (let [role, member] of [...memberships].reverse()) { + await dbAdapter.execute(`REVOKE ${role} FROM ${member}`); + } + } + } + + async function selectAs(role: string, sql: string) { + return await dbAdapter.withTransaction(async (query: any) => { + await query([`SET LOCAL ROLE ${role}`]); + return await query([sql]); + }); + } + + async function canSelect(role: string, table: string): Promise { + let [{ readable }] = await dbAdapter.execute( + `SELECT has_table_privilege('${role}', '${table}', 'SELECT') AS readable`, + ); + return readable as boolean; + } + + test("the read-only role and its members can't select either table", async function (assert) { + // A member that inherits the role and was also granted SELECT on the + // tables directly. + await withMemberships([['readonly_role', MEMBER]], async () => { + await dbAdapter.execute( + `GRANT SELECT ON TABLE ${TABLES.join(', ')} TO ${MEMBER}`, + ); + + await runUp(); + + for (let role of ['readonly_role', MEMBER]) { + for (let table of TABLES) { + await assert.rejects( + selectAs(role, `SELECT * FROM ${table}`), + new RegExp(`permission denied for table ${table}`), + `${role} can't select ${table}`, + ); + } + } + }); + }); + + test('revokes a grant another role made, and every grant passed on from it', async function (assert) { + // A role outside the read-only role holds SELECT with the grant option + // and passes it to a member, which passes it to the role itself. + await createRole(OTHER_ROLE); + await dbAdapter.execute( + `GRANT SELECT ON TABLE key_shares TO ${OTHER_ROLE} WITH GRANT OPTION`, + ); + await withMemberships([['readonly_role', MEMBER]], async () => { + await dbAdapter.withTransaction(async (query: any) => { + await query([`SET LOCAL ROLE ${OTHER_ROLE}`]); + await query([ + `GRANT SELECT ON TABLE key_shares TO ${MEMBER} WITH GRANT OPTION`, + ]); + }); + await dbAdapter.withTransaction(async (query: any) => { + await query([`SET LOCAL ROLE ${MEMBER}`]); + await query([ + `GRANT SELECT (owner) ON TABLE key_shares TO readonly_role`, + ]); + }); + + await runUp(); + + assert.false(await canSelect(MEMBER, 'key_shares')); + let [{ readable }] = await dbAdapter.execute( + `SELECT has_column_privilege('readonly_role', 'key_shares', 'owner', 'SELECT') AS readable`, + ); + assert.false(readable as boolean, 'the grant passed on is gone too'); + assert.true( + await canSelect(OTHER_ROLE, 'key_shares'), + 'the role outside the read-only role keeps its own', + ); + }); + }); + + test('stops when PUBLIC can read a column', async function (assert) { + await dbAdapter.execute( + `GRANT SELECT (owner) ON TABLE key_shares TO PUBLIC`, + ); + + await assert.rejects( + runUp(), + /can read key_shares\.owner, which is withheld from readonly_role/, + ); + }); + + test('stops when a member reads a table through another role', async function (assert) { + await withMemberships( + [ + ['readonly_role', MEMBER], + [OTHER_ROLE, MEMBER], + ], + async () => { + await dbAdapter.execute( + `GRANT SELECT ON TABLE key_share_notices TO ${OTHER_ROLE}`, + ); + + await assert.rejects( + runUp(), + new RegExp( + `${MEMBER} can read key_share_notices\\.\\w+, which is withheld`, + ), + ); + }, + ); + }); + + test('leaves SELECT to a member that holds only the ADMIN option on the role', async function (assert) { + // The role that creates the read-only role holds this membership on + // Postgres 16 and later, and reads nothing through it. + await withMemberships( + [ + [ + 'readonly_role', + ADMIN_MEMBER, + 'ADMIN TRUE, INHERIT FALSE, SET FALSE', + ], + ], + async () => { + await dbAdapter.execute( + `GRANT SELECT ON TABLE ${TABLES.join(', ')} TO ${ADMIN_MEMBER}`, + ); + + await runUp(); + + for (let table of TABLES) { + assert.true( + await canSelect(ADMIN_MEMBER, table), + `it still reads ${table}`, + ); + } + }, + ); + }); + + test('leaves SELECT to a member that owns a table', async function (assert) { + await createRole(OWNER); + await dbAdapter.execute(`ALTER TABLE key_shares OWNER TO ${OWNER}`); + await withMemberships([['readonly_role', OWNER]], async () => { + await runUp(); + + assert.true(await canSelect(OWNER, 'key_shares'), 'the owner reads it'); + assert.false( + await canSelect('readonly_role', 'key_shares'), + 'the role itself still does not', + ); + }); + }); + }); +}); diff --git a/packages/realm-server/tests/user-keys-test.ts b/packages/realm-server/tests/user-keys-test.ts index 185cd442ce5..8eced79f00f 100644 --- a/packages/realm-server/tests/user-keys-test.ts +++ b/packages/realm-server/tests/user-keys-test.ts @@ -719,6 +719,8 @@ module(basename(import.meta.filename), function () { let { ticket } = await mintKeyWriteTicket(dbAdapter, OWNER, { action: 'add', keyId: null, + realmURL: null, + shareId: null, }); return await addUserKey(dbAdapter, sealWith, { owner: OWNER, @@ -841,6 +843,8 @@ module(basename(import.meta.filename), function () { await mintKeyWriteTicket(dbAdapter, OWNER, { action: 'add', keyId: null, + realmURL: null, + shareId: null, }); async function selectAs(role: string, sql: string) { diff --git a/packages/runtime-common/db-queries/realm-permission-queries.ts b/packages/runtime-common/db-queries/realm-permission-queries.ts index 82e6d81c875..8f1f5c6758a 100644 --- a/packages/runtime-common/db-queries/realm-permission-queries.ts +++ b/packages/runtime-common/db-queries/realm-permission-queries.ts @@ -215,6 +215,28 @@ export async function isDirectWriter( return !!row && isDirectWriterRow(userId, row.write); } +/** + * `isDirectWriter`, asked inside the caller's transaction with the person's + * permission row locked until it ends. Anything that takes something on in a + * person's name on the strength of the answer asks it this way, so a change + * to that row made meanwhile waits for the caller's work to commit and then + * sees it. + */ +export async function isDirectWriterLocked( + txQuerier: Querier, + realmURL: URL, + userId: string, +): Promise { + let [row] = (await txQuerier([ + `SELECT write FROM realm_user_permissions WHERE realm_url =`, + param(realmURL.href), + `AND username =`, + param(userId), + `FOR SHARE`, + ])) as { write: boolean }[]; + return !!row && isDirectWriterRow(userId, row.write); +} + /** * The realm's direct writers, as `isDirectWriter` answers it, in username * order.