diff --git a/src/backend/controllers/events/EventsController.ts b/src/backend/controllers/events/EventsController.ts index 14a9cffb6..9ccacffb3 100644 --- a/src/backend/controllers/events/EventsController.ts +++ b/src/backend/controllers/events/EventsController.ts @@ -19,9 +19,10 @@ import type { Request, Response } from 'express'; import type { Actor } from '../../core/actor.js'; -import { Controller, Get, Post } from '../../core/http/decorators.js'; +import { Controller, Delete, Get, Post } from '../../core/http/decorators.js'; import { HttpError } from '../../core/http/HttpError.js'; import { DURABLE_LIST_LIMIT_CAP } from '../../stores/events/DurableSubscriptionStore.js'; +import { KV_HANDLE_LIST_LIMIT_CAP } from '../../stores/events/KvShareHandleStore.js'; import { normalizeLimit } from '../../util/pagination.js'; import { PuterController } from '../types.js'; import { @@ -156,6 +157,48 @@ export class EventsController extends PuterController { ); } + /** GET /events/kv-handles — what this account has shared out. */ + @Get('/kv-handles', { + subdomain: 'api', + requireAuth: true, + allowAccessToken: true, + rateLimit: EVENTS_LIST_LIMIT, + }) + async listKvHandles(req: Request, res: Response): Promise { + const actor = this.#requireActor(req); + const query = (req.query ?? {}) as Record; + + const page = await this.services.events.listKvHandles(actor, { + limit: normalizeLimit(query.limit, { + cap: KV_HANDLE_LIST_LIMIT_CAP, + }), + cursor: typeof query.cursor === 'string' ? query.cursor : undefined, + includeTotal: query.includeTotal === 'true', + }); + + res.json({ + items: page.items, + ...(page.cursor ? { cursor: page.cursor } : {}), + ...(page.total !== undefined ? { total: page.total } : {}), + }); + } + + /** + * DELETE /events/kv-handles/:handle — take a shared region back. A handle + * this account did not mint reads as absent. + */ + @Delete('/kv-handles/:handle', { + subdomain: 'api', + requireAuth: true, + allowAccessToken: true, + }) + async revokeKvHandle(req: Request, res: Response): Promise { + const actor = this.#requireActor(req); + res.json( + await this.services.events.revokeKvHandle(actor, req.params.handle), + ); + } + // -- Handlers ---------------------------------------------------- // // Deploying an app's code, so the gate is the same as the verbs above plus diff --git a/src/backend/services/events/EventsService.ts b/src/backend/services/events/EventsService.ts index fcf284d1d..a2fd779b7 100644 --- a/src/backend/services/events/EventsService.ts +++ b/src/backend/services/events/EventsService.ts @@ -51,6 +51,7 @@ import { HANDLER_SETTLE_BATCH, isSuspendedReason, } from '../../stores/events/DurableSubscriptionStore.js'; +import type { KvShareHandleListOptions } from '../../stores/events/KvShareHandleStore.js'; import { HANDLER_NAME_MAX_LENGTH, hashContent, @@ -170,6 +171,7 @@ import { import { SubscriptionCache } from './subscriptionCache.js'; import { assertShareableAppUid, + assertShareablePermission, assertShareablePrefix, kvShareGrantCovers, kvShareOwnerImplicator, @@ -669,7 +671,7 @@ const handlerAppRequired = (): HttpError => * separate consent. */ const handleOwnerOnly = (): HttpError => - new HttpError(403, 'Only an account session may mint a share handle', { + new HttpError(403, 'Only an account session may manage share handles', { legacyCode: 'events_kv_handle_owner_only', }); @@ -1713,7 +1715,9 @@ export class EventsService extends PuterService { ); const grantee = await this.#resolveGrantee(request); - const permission = kvSharePermission(owner.uuid, appUid, keyPrefix); + const permission = assertShareablePermission( + kvSharePermission(owner.uuid, appUid, keyPrefix), + ); await this.services.permission.grantUserUserPermission( actor, grantee.username, @@ -1732,6 +1736,94 @@ export class EventsService extends PuterService { return { handle: row.handle, prefix: row.keyPrefix }; } + /** + * Take a shared region back. + * + * The grant goes first — the subtree with it, since a deeper grant would + * leave access to part of a region that has just been withdrawn — and the + * handle is retired after. Removing the grant is what actually settles the + * subscriptions, and it is the only half that stops delivery, so it is the + * half that must not be able to fail after the other has succeeded. In that + * order both steps are idempotent and a failure anywhere is a retry: the + * reverse leaves a handle nothing can address standing on a grant + * everything still passes, and no way to try again. + * + * Not gated on the feature flag. Withdrawing access only ever narrows, and + * an install that turned handles off must still be able to retire the ones + * it has. + */ + async revokeKvHandle( + actor: Actor, + handle: unknown, + ): Promise<{ handle: string; revokedAt: number }> { + if (!this.enabled) throw disabled(); + const owner = actor.user; + if (owner?.id === undefined) throw disabled(); + if (actor.effectiveApp !== null) throw handleOwnerOnly(); + + await this.#spendHandleBudget(owner.id); + + const named = typeof handle === 'string' ? handle : ''; + const share = await this.stores.kvShareHandle.getByHandle(named); + // Unknown and somebody else's read the same way, so revoking cannot be + // used to find out that a handle exists. + if (!share || share.ownerUserId !== owner.id) + throw unknownKvShareHandle(named); + + const grantee = await this.stores.user.getById(share.granteeUserId); + if (!grantee?.username) + throw new HttpError(500, 'The grantee of this handle is gone', { + legacyCode: 'internal_error', + }); + await this.services.permission.revokeUserUserPermissionSubtree( + actor, + grantee.username, + share.permission, + { reason: 'kv share handle revoked' }, + ); + + const revoked = await this.stores.kvShareHandle.retire(named, owner.id); + if (!revoked?.revokedAt) throw unknownKvShareHandle(named); + return { handle: revoked.handle, revokedAt: revoked.revokedAt }; + } + + /** + * What this account has shared out of its key-value data, retired handles + * included: they are the only record of what was shared and when it + * stopped. The grantee has no listing of their own — what they hold is the + * handle they were given. + */ + async listKvHandles( + actor: Actor, + options: KvShareHandleListOptions = {}, + ): Promise> { + if (!this.enabled) throw disabled(); + const owner = actor.user; + if (owner?.id === undefined) throw disabled(); + if (actor.effectiveApp !== null) throw handleOwnerOnly(); + + const page = await this.stores.kvShareHandle.listForOwner( + owner.id, + options, + ); + const grantees = await this.stores.user.getByIds([ + ...new Set(page.items.map((row) => row.granteeUserId)), + ]); + + return { + ...page, + items: page.items.map((row) => ({ + handle: row.handle, + prefix: row.keyPrefix, + appUid: row.appUid, + granteeUsername: + grantees.get(row.granteeUserId)?.username ?? null, + createdAt: row.createdAt, + revokedAt: row.revokedAt, + })), + }; + } + /** Who a mint is for. Named by username or uuid; unknown reads as absent. */ async #resolveGrantee( request: MintKvHandleRequest, diff --git a/src/backend/services/events/kvHandleRevoke.integration.test.ts b/src/backend/services/events/kvHandleRevoke.integration.test.ts new file mode 100644 index 000000000..b90d3dc58 --- /dev/null +++ b/src/backend/services/events/kvHandleRevoke.integration.test.ts @@ -0,0 +1,483 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +/** + * Taking a shared key-value region back. + * + * Revoking a handle is not its own settle mechanism: it withdraws the grant, + * and the revocation pass that already handles an unshare does the rest. What + * these cases pin is that the handle really is 1:1 with what has to be settled + * — the holder index answers it, nothing scans for the handle — and that + * everything a revocation is supposed to take with it actually goes. + */ + +import { v4 as uuidv4 } from 'uuid'; +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; +import { EVENTS_COALESCE_WINDOW_MS } from '../../controllers/events/limits.js'; +import { makeActor, type Actor } from '../../core/actor.js'; +import { runWithContext } from '../../core/context.js'; +import { + createTestUser, + setupPuterTestEnv, + type PuterTestEnv, +} from '../../testUtil.js'; +import type { IConfig } from '../../types.js'; +import type { DeliveryEnvelope } from './EventsService.js'; +import { kvAnchorToken } from './subjects.js'; + +const BOOT_TIMEOUT_MS = 120_000; +const SOCKET_ID = 'kv-revoke-socket'; +const TABLE = 'event_subscriptions'; +const PREFIX = 'workspace:abc:'; + +interface TestUser { + actor: Actor; + username: string; + id: number; + uuid: string; +} + +let env: PuterTestEnv; +let owner: TestUser; +let guest: TestUser; +let stranger: TestUser; +let delivered: DeliveryEnvelope[]; + +const events = () => env.server.services.events; + +const settled = (count = 1) => + vi.waitFor(() => expect(delivered.length).toBeGreaterThanOrEqual(count), { + timeout: EVENTS_COALESCE_WINDOW_MS * 12, + interval: 25, + }); + +const quiet = () => + new Promise((resolve) => + setTimeout(resolve, EVENTS_COALESCE_WINDOW_MS * 3), + ); + +const userFor = async (username: string): Promise => { + const row = await env.server.stores.user.getByUsername(username); + return { + actor: makeActor({ user: row as never }), + username, + id: row!.id, + uuid: row!.uuid as string, + }; +}; + +const ownerWrites = (key: string, value: unknown): Promise => + runWithContext({ actor: owner.actor }, () => + env.server.drivers.kvStore.set({ key, value }), + ); + +const mint = (prefix = PREFIX) => + events().mintKvHandle(owner.actor, { + granteeUsername: guest.username, + prefix, + }); + +/** A durable `single` row, which is the only kind that holds a backlog. */ +const subscribeDurable = async (handle: string) => + ( + await events().subscribeDurable(guest.actor, { + subject: `kv:${handle}:*`, + delivery: 'single', + handlerName: 'onChange', + }) + ).sub; + +const rowOf = async (subId: string) => { + const [row] = await env.server.clients.db.pread( + `SELECT \`suspended_reason\` FROM \`${TABLE}\` WHERE \`sub_id\` = ?`, + [subId], + ); + return row as { suspended_reason: unknown } | undefined; +}; + +const suspendedRow = (subId: string) => + vi.waitFor( + async () => + expect((await rowOf(subId))?.suspended_reason).toBe( + 'permission_revoked', + ), + { timeout: 5_000, interval: 25 }, + ); + +/** Whether the shared region is still watched in its owner's keyspace. */ +const ownerWatches = async (): Promise => { + const watched = await env.server.stores.eventSubscription.watchedTokens( + owner.id, + [kvAnchorToken(owner.uuid, 'os-global', PREFIX)], + ); + return watched.length > 0; +}; + +const clearRows = async () => { + await env.server.clients.db.write(`DELETE FROM \`${TABLE}\``, []); + for (const id of [owner.id, guest.id, stranger.id]) { + await events().reapSocket(id, SOCKET_ID); + events().invalidateUser(id); + await env.server.stores.eventSubscription.markRegionCold(id); + await env.server.stores.durableSubscription.warmRegion(id); + } + delivered.length = 0; +}; + +beforeAll(async () => { + env = await setupPuterTestEnv({ + events: { enabled: true, kvHandles: true }, + // Seeded accounts carry no email, which the plan machinery reads as a + // temporary account — and a temporary account holds no durable rows. + unlimitedMetering: true, + } as IConfig); + + const strangerName = `kv-onlooker-${uuidv4().slice(0, 8)}`; + await createTestUser(env.server, { + username: strangerName, + password: 'pw-test-1234', + }); + + owner = await userFor(env.users.user.username); + guest = await userFor(env.users.other.username); + stranger = await userFor(strangerName); + + delivered = []; + events().onDelivered = (envelope) => delivered.push(envelope); +}, BOOT_TIMEOUT_MS); + +afterAll(async () => { + await env?.shutdown(); +}); + +describe('revoking a handle', () => { + it('settles the subscriptions it was holding up, and takes their backlog', async () => { + await clearRows(); + const { handle } = await mint(); + const sub = await subscribeDurable(handle); + + await ownerWrites(`${PREFIX}messages:1`, { body: 'before' }); + await vi.waitFor( + async () => + expect( + await env.server.stores.pendingDelivery.depth(sub.subId), + ).toBeGreaterThan(0), + { timeout: 5_000, interval: 25 }, + ); + expect(await ownerWatches()).toBe(true); + + await events().revokeKvHandle(owner.actor, handle); + await suspendedRow(sub.subId); + + // The anchor leaves the *owner's* watched set, which is the keyspace a + // write looks in. + expect(await ownerWatches()).toBe(false); + // Purged, not held: the backlog names keys its holder has just lost the + // right to see. + expect(await env.server.stores.pendingDelivery.depth(sub.subId)).toBe( + 0, + ); + + delivered.length = 0; + await ownerWrites(`${PREFIX}messages:2`, { body: 'after' }); + await quiet(); + expect(delivered).toEqual([]); + }); + + it('stops a session subscription through the delivery re-check', async () => { + await clearRows(); + const { handle } = await mint(); + const { sub } = await events().subscribe(guest.actor, SOCKET_ID, { + subject: `kv:${handle}:*`, + }); + + await ownerWrites(`${PREFIX}live:1`, 1); + await settled(); + expect(delivered[0].subId).toBe(sub.subId); + + await events().revokeKvHandle(owner.actor, handle); + delivered.length = 0; + + await ownerWrites(`${PREFIX}live:2`, 2); + await quiet(); + expect(delivered).toEqual([]); + }); + + it('finds its subscriptions on the holder index, not by scanning', async () => { + await clearRows(); + const { handle } = await mint(); + const sub = await subscribeDurable(handle); + + const store = env.server.stores.durableSubscription; + const byHolder = vi.spyOn(store, 'listActiveForHolder'); + const settle = vi.spyOn(events(), 'settleRevokedGrant'); + + try { + await events().revokeKvHandle(owner.actor, handle); + await suspendedRow(sub.subId); + + // One announcement, one settle pass, one indexed read of the + // holder's rows — no lookup keyed on the handle anywhere. + expect(settle).toHaveBeenCalledTimes(1); + expect(byHolder).toHaveBeenCalledTimes(1); + expect(byHolder).toHaveBeenCalledWith(guest.id, null); + } finally { + byHolder.mockRestore(); + settle.mockRestore(); + } + }); + + it('withdraws the grant, deeper grants included', async () => { + await clearRows(); + const { handle } = await mint(); + const deeper = await mint(`${PREFIX}messages:`); + + await events().revokeKvHandle(owner.actor, handle); + + // The subtree goes with the region: a grant left underneath would keep + // access to part of what was just taken back. + await expect( + events().subscribe(guest.actor, SOCKET_ID, { + subject: `kv:${deeper.handle}:*`, + }), + ).rejects.toMatchObject({ legacyCode: 'subject_does_not_exist' }); + }); + + it('lets a fresh handle over the same region work, and leaves the old one dead', async () => { + await clearRows(); + const first = await mint(); + await events().revokeKvHandle(owner.actor, first.handle); + + const second = await mint(); + expect(second.handle).not.toBe(first.handle); + + const { sub } = await events().subscribe(guest.actor, SOCKET_ID, { + subject: `kv:${second.handle}:*`, + }); + delivered.length = 0; + await ownerWrites(`${PREFIX}messages:3`, { body: 'again' }); + await settled(); + expect(delivered[0].subId).toBe(sub.subId); + + await expect( + events().subscribe(guest.actor, SOCKET_ID, { + subject: `kv:${first.handle}:*`, + }), + ).rejects.toMatchObject({ legacyCode: 'subject_does_not_exist' }); + }); + + it.each([ + ['a handle nobody minted', async () => `kvh-${uuidv4()}`], + ['one this account did not mint', async () => (await mint()).handle], + ])('answers %s as absent', async (label, handleFor) => { + const handle = await handleFor(); + const actor = + label === 'one this account did not mint' + ? stranger.actor + : owner.actor; + + await expect( + events().revokeKvHandle(actor, handle), + ).rejects.toMatchObject({ + legacyCode: 'subject_does_not_exist', + }); + }); + + it('is idempotent, and keeps the moment it first stopped', async () => { + // The grant comes down before the handle is stamped, so a failure + // between them leaves a retry to finish the job — which only works if + // reaching a retired handle again succeeds. + const { handle } = await mint(); + const first = await events().revokeKvHandle(owner.actor, handle); + const again = await events().revokeKvHandle(owner.actor, handle); + expect(again).toEqual(first); + }); + + it('leaves nothing standing when the handle stamp fails', async () => { + // The half that stops delivery is the grant, so a failure after it + // must not leave access behind — and the owner must be able to retry. + await clearRows(); + const { handle } = await mint(); + await events().subscribe(guest.actor, SOCKET_ID, { + subject: `kv:${handle}:*`, + }); + + const store = env.server.stores.kvShareHandle; + const retire = store.retire.bind(store); + const failing = vi + .spyOn(store, 'retire') + .mockRejectedValueOnce(new Error('write failed')); + await expect( + events().revokeKvHandle(owner.actor, handle), + ).rejects.toThrow('write failed'); + failing.mockRestore(); + + // The grant is already gone, so nothing is delivered even though the + // handle still reads as live. + delivered.length = 0; + await ownerWrites(`${PREFIX}messages:9`, { body: 'nope' }); + await quiet(); + expect(delivered).toEqual([]); + + // And the retry converges rather than answering "no such handle". + const revoked = await events().revokeKvHandle(owner.actor, handle); + expect(revoked.handle).toBe(handle); + expect(await retire(handle, owner.id)).toMatchObject({ + revokedAt: revoked.revokedAt, + }); + }); + + it('refuses an app acting for the owner', async () => { + const { handle } = await mint(); + const uid = `app-${uuidv4()}`; + await env.server.clients.db.write( + 'INSERT INTO `apps` (`uid`, `name`, `title`, `index_url`, `owner_user_id`) VALUES (?, ?, ?, ?, ?)', + [uid, uid, uid, `https://${uid}.example/`, owner.id], + ); + const app = await env.server.stores.app.getByUid(uid); + const appActor = makeActor({ + user: owner.actor.user as never, + app: { uid, id: app!.id }, + }); + + await expect( + events().revokeKvHandle(appActor, handle), + ).rejects.toMatchObject({ + legacyCode: 'events_kv_handle_owner_only', + }); + }); +}); + +describe('the owner`s handle listing', () => { + it('shows what was shared, retired handles included', async () => { + const { handle } = await mint(); + await events().revokeKvHandle(owner.actor, handle); + + const page = await events().listKvHandles(owner.actor, { + includeTotal: true, + }); + const row = page.items.find((one) => one.handle === handle); + + expect(row).toMatchObject({ + prefix: PREFIX, + appUid: 'os-global', + granteeUsername: guest.username, + }); + expect(row?.revokedAt).toBeTypeOf('number'); + expect(page.total).toBe(page.items.length + (page.cursor ? 1 : 0)); + }); + + it('shows the grantee nothing of the owner`s', async () => { + await mint(); + const page = await events().listKvHandles(guest.actor); + expect(page.items).toEqual([]); + }); + + it('pages, and the cursor picks up where it left off', async () => { + await mint(); + await mint(); + + const first = await events().listKvHandles(owner.actor, { limit: 1 }); + expect(first.items).toHaveLength(1); + expect(first.cursor).toBeTypeOf('string'); + + const second = await events().listKvHandles(owner.actor, { + limit: 1, + cursor: first.cursor, + }); + expect(second.items).toHaveLength(1); + expect(second.items[0].handle).not.toBe(first.items[0].handle); + }); +}); + +describe('the handle routes over HTTP', () => { + const call = ( + method: string, + path: string, + token: string, + body?: unknown, + ) => + fetch(new URL(path, env.apiOrigin), { + method, + headers: { + 'content-type': 'application/json', + authorization: `Bearer ${token}`, + }, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), + }); + + it('mints, lists and revokes over the wire', async () => { + const minted = await call( + 'POST', + '/events/kv-handles', + env.users.user.token, + { granteeUsername: guest.username, prefix: 'over-http:' }, + ); + expect(minted.status).toBe(200); + const { handle, prefix } = (await minted.json()) as { + handle: string; + prefix: string; + }; + expect(prefix).toBe('over-http:'); + + const listed = await call( + 'GET', + '/events/kv-handles?limit=100', + env.users.user.token, + ); + const page = (await listed.json()) as { + items: Array<{ handle: string }>; + }; + expect(page.items.some((row) => row.handle === handle)).toBe(true); + + const revoked = await call( + 'DELETE', + `/events/kv-handles/${handle}`, + env.users.user.token, + ); + expect(revoked.status).toBe(200); + await expect(revoked.json()).resolves.toMatchObject({ handle }); + }); + + it('shows one account nothing of another`s, and revokes nothing of theirs', async () => { + const { handle } = await mint(); + + const listed = await call( + 'GET', + '/events/kv-handles', + env.users.other.token, + ); + const page = (await listed.json()) as { items: unknown[] }; + expect(page.items).toEqual([]); + + const revoked = await call( + 'DELETE', + `/events/kv-handles/${handle}`, + env.users.other.token, + ); + expect(revoked.status).toBe(404); + }); + + it('turns an anonymous caller away', async () => { + const res = await fetch(new URL('/events/kv-handles', env.apiOrigin), { + method: 'GET', + }); + expect(res.status).toBeGreaterThanOrEqual(400); + }); +}); diff --git a/src/backend/services/events/kvShares.test.ts b/src/backend/services/events/kvShares.test.ts index e0ac44505..996471c52 100644 --- a/src/backend/services/events/kvShares.test.ts +++ b/src/backend/services/events/kvShares.test.ts @@ -23,6 +23,7 @@ import { HttpError } from '../../core/http/HttpError.js'; import { PermissionUtil } from '../permission/permissionUtil.js'; import { assertShareableAppUid, + assertShareablePermission, assertShareablePrefix, keyPrefixSegments, kvShareGrantCovers, @@ -136,10 +137,14 @@ describe('granted prefixes', () => { ); }); - it('refuses a prefix past the key size limit', () => { - expect(() => assertShareablePrefix('a'.repeat(1025))).toThrow( - HttpError, - ); + it('refuses a region too deep to fit the grant column', () => { + const deep = kvSharePermission(OWNER, APP, 'a'.repeat(300)); + expect(() => assertShareablePermission(deep)).toThrow(HttpError); + expect( + assertShareablePermission( + kvSharePermission(OWNER, APP, 'workspace:abc:'), + ), + ).toBeTypeOf('string'); }); it('keeps the trailing delimiter optional', () => { diff --git a/src/backend/services/events/kvShares.ts b/src/backend/services/events/kvShares.ts index 8fc6530a6..dfaf76e86 100644 --- a/src/backend/services/events/kvShares.ts +++ b/src/backend/services/events/kvShares.ts @@ -19,8 +19,10 @@ import { randomUUID } from 'node:crypto'; import { HttpError } from '../../core/http/HttpError.js'; -import { MAX_KEY_BYTES } from '../../stores/systemKv/SystemKVStore.js'; -import { MANAGE_PERM_PREFIX } from '../permission/consts.js'; +import { + MANAGE_PERM_PREFIX, + PERMISSION_MAX_LEN, +} from '../permission/consts.js'; import type { PermissionImplicator } from '../permission/permissionUtil.js'; import { PermissionUtil } from '../permission/permissionUtil.js'; import { @@ -53,9 +55,6 @@ import { /** Root of the cross-user key-value share namespace. */ export const KV_SHARE_PERMISSION_PREFIX = 'kv-share'; -/** Longest key prefix a handle may be granted on, matching the key limit. */ -export const KV_SHARE_PREFIX_MAX_BYTES = MAX_KEY_BYTES; - /** Width of the `app_uid` column the handle row stores its namespace in. */ export const KV_SHARE_APP_UID_MAX_LENGTH = 40; @@ -143,11 +142,6 @@ export const assertShareablePrefix = (keyPrefix: unknown): string => { throw invalidPrefix('`prefix` must be a string'); if (keyPrefix.includes('*') || keyPrefix.includes('?')) throw invalidPrefix('A share prefix is a key prefix, not a pattern'); - if (Buffer.byteLength(keyPrefix, 'utf8') > KV_SHARE_PREFIX_MAX_BYTES) - throw invalidPrefix( - `A share prefix may not exceed ${KV_SHARE_PREFIX_MAX_BYTES} bytes`, - ); - // Normalizing drops empty segments, so `a::b:` would silently become a // grant on `a:b:` — a region other than the one asked for. Refused rather // than rewritten; only the trailing delimiter is optional. @@ -177,6 +171,19 @@ export const assertShareableAppUid = (appUid: string): string => { return appUid; }; +/** + * The grant a mint would issue, refused when it would not fit the column every + * permission table declares. Key prefixes run to a kilobyte and permissions do + * not, so the real bound on how deep a region may be is this one. + */ +export const assertShareablePermission = (permission: string): string => { + if (permission.length > PERMISSION_MAX_LEN) + throw invalidPrefix( + `A share prefix must leave the grant under ${PERMISSION_MAX_LEN} characters`, + ); + return permission; +}; + // -- Permission rules ------------------------------------------------- /** diff --git a/src/backend/services/permission/PermissionService.ts b/src/backend/services/permission/PermissionService.ts index 910c8e620..10d2b7d44 100644 --- a/src/backend/services/permission/PermissionService.ts +++ b/src/backend/services/permission/PermissionService.ts @@ -26,6 +26,7 @@ import { PuterService } from '../types'; import { FLAT_PERM_WARM_TTL_SECONDS, MANAGE_PERM_PREFIX, + PERMISSION_MAX_LEN, PERMISSION_SCAN_CACHE_TTL_SECONDS, } from './consts'; import { @@ -46,12 +47,6 @@ import { } from '../../data/hardcoded-permissions.js'; import { UserRow } from '../../stores/user/UserStore'; -/** - * Width of the `permission` column in the permission tables, which every - * dialect declares as `varchar(255)`. - */ -const PERMISSION_MAX_LEN = 255; - // -- Types ------------------------------------------------------------ export interface ScanOptions { @@ -1004,6 +999,80 @@ export class PermissionService extends PuterService { return acting?.uid ? { appUid: acting.uid } : null; } + /** + * Withdraw a grant and everything the holder was given beneath it. + * + * Prefix implication is what makes the subtree part necessary: a grant on a + * region answers every check inside it, so leaving a deeper grant standing + * would leave access to part of a region that has just been taken back. + * Scoped to this issuer's grants to this holder — a region two people were + * given is two grants, and one being withdrawn is not the other's + * business. + * + * Returns the permissions removed. Each is announced separately, because + * each is a distinct thing something may have been standing on. + */ + async revokeUserUserPermissionSubtree( + actor: Actor, + username: string, + permission: string, + meta: GrantMeta = {}, + ): Promise { + permission = await this.rewritePermission(permission); + const user = await this.stores.user.getByUsername(username); + if (!user) + throw new HttpError(404, `user_does_not_exist: ${username}`, { + legacyCode: 'subject_does_not_exist', + }); + if (!actor.user?.id) + throw new HttpError(403, 'actor must be a user', { + legacyCode: 'forbidden', + }); + const issuerId = actor.user.id; + + const isSelfRevoke = user.id === issuerId; + if ( + !isSelfRevoke && + !(await this.canManagePermission(actor, permission)) + ) { + throw new HttpError(403, `permission_denied: ${permission}`, { + legacyCode: 'permission_denied', + }); + } + + const removed = + await this.stores.permission.deleteUserUserPermSubtreeForHolder( + user.id, + issuerId, + permission, + ); + + for (const removedPermission of removed) { + this.stores.permission + .auditUserUserPerm({ + holder_user_id: user.id, + issuer_user_id: issuerId, + permission: removedPermission, + action: 'revoke', + reason: meta.reason ?? 'revoked via PermissionService', + extra: this.#auditActorContext(actor), + }) + .catch((err) => { + console.warn( + '[PermissionService] failed to audit user-user revoke:', + err, + ); + }); + } + + // Before the announcement, so whatever settles on it re-derives from a + // counter that has already moved. + if (user.uuid) await this.#bumpUserCacheGeneration(user.uuid); + for (const removedPermission of removed) + this.#announceRevoked(user.id, null, removedPermission); + return removed; + } + /** * Rewrite a permission on its way into (or out of) a user-app row. * diff --git a/src/backend/services/permission/consts.ts b/src/backend/services/permission/consts.ts index a0493b279..f5fc9990c 100644 --- a/src/backend/services/permission/consts.ts +++ b/src/backend/services/permission/consts.ts @@ -20,6 +20,13 @@ export const MANAGE_PERM_PREFIX = 'manage'; export const PERM_KEY_PREFIX = 'perm'; +/** + * Width of the `permission` column in the permission tables, which every + * dialect declares as `varchar(255)`. Anything building a permission string + * from user-supplied parts has to fit inside it. + */ +export const PERMISSION_MAX_LEN = 255; + /** * De-facto placeholder permission for permission rewrites that do not grant any * access. diff --git a/src/backend/stores/events/KvShareHandleStore.ts b/src/backend/stores/events/KvShareHandleStore.ts index a673610cb..a8cdf5a77 100644 --- a/src/backend/stores/events/KvShareHandleStore.ts +++ b/src/backend/stores/events/KvShareHandleStore.ts @@ -18,6 +18,11 @@ */ import { mintKvHandleId } from '../../services/events/kvShares.js'; +import { + decodeCursor, + encodeCursor, + type PageResult, +} from '../../util/pagination.js'; import { PuterStore } from '../types.js'; /** @@ -31,6 +36,9 @@ import { PuterStore } from '../types.js'; const TABLE = 'kv_share_handles'; +export const KV_HANDLE_LIST_DEFAULT_LIMIT = 50; +export const KV_HANDLE_LIST_LIMIT_CAP = 200; + export interface KvShareHandle { handle: string; ownerUserId: number; @@ -44,6 +52,12 @@ export interface KvShareHandle { revokedAt: number | null; } +export interface KvShareHandleListOptions { + limit?: number; + cursor?: string; + includeTotal?: boolean; +} + export interface MintKvShareHandleInput { ownerUserId: number; granteeUserId: number; @@ -54,6 +68,11 @@ export interface MintKvShareHandleInput { const nowSeconds = (): number => Math.floor(Date.now() / 1000); +const asNumber = (value: unknown): number | null => { + const parsed = Number(value); + return Number.isFinite(parsed) ? parsed : null; +}; + const toRow = (row: Record): KvShareHandle => ({ handle: String(row.handle), ownerUserId: Number(row.owner_user_id), @@ -116,4 +135,86 @@ export class KvShareHandleStore extends PuterStore { ); return rows.length > 0 ? toRow(rows[0]) : null; } + + /** + * Retire one handle, scoped to its owner. Returns the row as it now stands, + * or `null` when this owner has no handle by that name — the one answer an + * unknown handle and somebody else's both get, so retiring cannot be used + * to find out that a handle exists. + * + * Idempotent: an already-retired handle keeps the timestamp it has. The + * caller withdraws the grant first and that step can fail, so this one has + * to be safe to reach twice. + * + * The `revoked_at IS NULL` predicate makes the read-then-write a + * compare-and-set: two callers racing means one stamps the row and the + * other reports the stamp it found, rather than both writing. + */ + async retire( + handle: string, + ownerUserId: number, + ): Promise { + const existing = await this.getByHandle(handle); + if (!existing || existing.ownerUserId !== ownerUserId) return null; + if (existing.revokedAt !== null) return existing; + + const at = nowSeconds(); + const result = await this.clients.db.write( + `UPDATE \`${TABLE}\` SET \`revoked_at\` = ? ` + + 'WHERE `handle` = ? AND `owner_user_id` = ? ' + + 'AND `revoked_at` IS NULL', + [at, handle, ownerUserId], + ); + if (result?.anyRowsAffected === false) + return await this.getByHandle(handle); + return { ...existing, revokedAt: at }; + } + + /** + * What one owner has minted, revoked handles included — they are the record + * of what was shared and when it stopped. Keyset-paginated on `id`. + */ + async listForOwner( + ownerUserId: number, + options: KvShareHandleListOptions = {}, + ): Promise> { + const limit = Math.min( + Math.max( + 1, + Math.floor(options.limit ?? KV_HANDLE_LIST_DEFAULT_LIMIT), + ), + KV_HANDLE_LIST_LIMIT_CAP, + ); + const after = asNumber(decodeCursor(options.cursor)?.id); + + const where = ['`owner_user_id` = ?']; + const params: unknown[] = [ownerUserId]; + if (after !== null) { + where.push('`id` > ?'); + params.push(after); + } + + const rows = await this.clients.db.read( + `SELECT \`id\`, ${SELECT_COLUMNS} FROM \`${TABLE}\` ` + + `WHERE ${where.join(' AND ')} ORDER BY \`id\` LIMIT ?`, + [...params, limit + 1], + ); + + const page = rows.slice(0, limit); + const result: PageResult = { items: page.map(toRow) }; + if (rows.length > limit) + result.cursor = encodeCursor({ + id: Number(page[page.length - 1].id), + }); + + if (options.includeTotal) { + const [count] = await this.clients.db.read( + `SELECT COUNT(*) AS \`total\` FROM \`${TABLE}\` ` + + 'WHERE `owner_user_id` = ?', + [ownerUserId], + ); + result.total = Number(count?.total ?? 0); + } + return result; + } } diff --git a/src/backend/stores/permission/PermissionStore.test.ts b/src/backend/stores/permission/PermissionStore.test.ts index b13f32ed0..01a6a8733 100644 --- a/src/backend/stores/permission/PermissionStore.test.ts +++ b/src/backend/stores/permission/PermissionStore.test.ts @@ -977,6 +977,108 @@ describe('PermissionStore', () => { expect(rows[0].issuer_user_id).toBe(issuerB.id); }); + it('takes a subtree from one holder without touching another`s', async () => { + const issuer = await makeUser(); + const holder = await makeUser(); + const other = await makeUser(); + const region = 'kv-share:owner:app:workspace:abc'; + + for (const user of [holder, other]) { + await store.upsertUserUserPerm(user.id, issuer.id, region, {}); + await store.upsertUserUserPerm( + user.id, + issuer.id, + `${region}:messages`, + {}, + ); + } + // A neighbour that merely shares the text prefix. + await store.upsertUserUserPerm( + holder.id, + issuer.id, + 'kv-share:owner:app:workspace:abcdef', + {}, + ); + + const removed = await store.deleteUserUserPermSubtreeForHolder( + holder.id, + issuer.id, + region, + ); + + expect(removed.sort()).toEqual([region, `${region}:messages`]); + expect( + ( + await store.readLinkedUserUserPerms(holder.id, [ + 'kv-share:owner:app:workspace:abcdef', + ]) + ).map((row) => row.permission), + ).toEqual(['kv-share:owner:app:workspace:abcdef']); + // The same region granted to somebody else is a different grant. + expect( + await store.readLinkedUserUserPerms(other.id, [ + region, + `${region}:messages`, + ]), + ).toHaveLength(2); + }); + + it('leaves another issuer`s subtree grant standing', async () => { + const issuerA = await makeUser(); + const issuerB = await makeUser(); + const holder = await makeUser(); + const region = 'kv-share:owner:app:shared'; + await store.upsertUserUserPerm(holder.id, issuerA.id, region, {}); + await store.upsertUserUserPerm(holder.id, issuerB.id, region, {}); + + expect( + await store.deleteUserUserPermSubtreeForHolder( + holder.id, + issuerA.id, + region, + ), + ).toEqual([region]); + + const rows = await store.readLinkedUserUserPerms(holder.id, [ + region, + ]); + expect(rows).toHaveLength(1); + expect(rows[0].issuer_user_id).toBe(issuerB.id); + }); + + it('treats LIKE wildcards in the holder-scoped subtree as literal text', async () => { + const issuer = await makeUser(); + const holder = await makeUser(); + + // Same hazard as deleteAppGrantsByPermissionPrefix, at the + // holder-scoped delete this family also uses. + await store.upsertUserUserPerm( + holder.id, + issuer.id, + 'kv-share:owner:app:a_c', + {}, + ); + await store.upsertUserUserPerm( + holder.id, + issuer.id, + 'kv-share:owner:app:abc', + {}, + ); + + const removed = await store.deleteUserUserPermSubtreeForHolder( + holder.id, + issuer.id, + 'kv-share:owner:app:a_c', + ); + + expect(removed).toEqual(['kv-share:owner:app:a_c']); + expect( + await store.readLinkedUserUserPerms(holder.id, [ + 'kv-share:owner:app:abc', + ]), + ).toHaveLength(1); + }); + it('reports whether the delete matched a row', async () => { const issuer = await makeUser(); const holder = await makeUser(); diff --git a/src/backend/stores/permission/PermissionStore.ts b/src/backend/stores/permission/PermissionStore.ts index dc6b5cb55..c9af8282c 100644 --- a/src/backend/stores/permission/PermissionStore.ts +++ b/src/backend/stores/permission/PermissionStore.ts @@ -116,6 +116,29 @@ export interface FlatPermRef { permission: string; } +/** + * Match a permission and everything beneath it. + * + * `_` and `%` are LIKE wildcards, so an unescaped one would widen the match + * beyond the intended subtree. `!` as the escape character, matching + * FSEntryStore: a backslash one would have to be written `ESCAPE '\\'` in the + * SQL text, and MySQL processes backslash escapes inside string literals, so + * the `'\'` a JS `'\\'` produces reads as an escaped quote and leaves the + * literal unterminated. SQLite and Postgres accept it, which is why only MySQL + * would have seen the parse error. + */ +const subtreeClause = ( + permissions: string[], +): { where: string; params: string[] } => ({ + where: permissions + .map(() => "(`permission` = ? OR `permission` LIKE ? ESCAPE '!')") + .join(' OR '), + params: permissions.flatMap((permission) => [ + permission, + `${permission.replace(/([!%_])/g, '!$1')}:%`, + ]), +}); + /** * PermissionStore owns the _persistence_ side of permissions: * @@ -474,14 +497,7 @@ export class PermissionStore extends PuterStore { }> > { if (permissions.length === 0) return []; - // See deleteAppGrantsByPermissionPrefix for why `!` is the escape. - const where = permissions - .map(() => "(`permission` = ? OR `permission` LIKE ? ESCAPE '!')") - .join(' OR '); - const params = permissions.flatMap((permission) => [ - permission, - `${permission.replace(/([!%_])/g, '!$1')}:%`, - ]); + const { where, params } = subtreeClause(permissions); const rows = (await this.clients.db.read( 'SELECT `holder_user_id`, `issuer_user_id`, `permission` FROM `user_to_user_permissions` ' + @@ -517,6 +533,46 @@ export class PermissionStore extends PuterStore { return rows; } + /** + * The same subtree delete, narrowed to one issuer's grants to one holder. + * + * Withdrawing a share names a person, not a region: two people granted on + * the same prefix hold two independent grants, and taking one back must not + * take the other's with it. Returns the permissions removed, so the caller + * can audit them and announce each one. + */ + async deleteUserUserPermSubtreeForHolder( + holderUserId: number, + issuerUserId: number, + permission: string, + ): Promise { + const { where, params } = subtreeClause([permission]); + const scope = '`holder_user_id` = ? AND `issuer_user_id` = ?'; + const scoped = [holderUserId, issuerUserId, ...params]; + + const rows = (await this.clients.db.read( + 'SELECT `permission` FROM `user_to_user_permissions` ' + + `WHERE ${scope} AND (${where})`, + scoped, + )) as Array<{ permission: string }>; + if (rows.length === 0) return []; + + await this.clients.db.write( + `DELETE FROM \`user_to_user_permissions\` WHERE ${scope} AND (${where})`, + scoped, + ); + + const removed = rows.map((row) => String(row.permission)); + await this.delFlatUserPerms( + removed.map((perm) => ({ holderUserId, permission: perm })), + ); + await this.publishCacheKeys({ + keys: [this.#u2uCacheKey(holderUserId)], + broadcast: true, + }); + return removed; + } + async auditUserUserPerm( entry: AuditEntry & { holder_user_id: number; @@ -738,18 +794,7 @@ export class PermissionStore extends PuterStore { permission: string; }> > { - // `_` and `%` are LIKE wildcards, so an unescaped one would widen the - // match beyond the intended subtree. - // - // `!` as the escape character, matching FSEntryStore: a backslash one - // would have to be written `ESCAPE '\\'` in the SQL text, and MySQL - // processes backslash escapes inside string literals, so the `'\'` a - // JS `'\\'` produces reads as an escaped quote and leaves the literal - // unterminated. SQLite and Postgres accept it, which is why only MySQL - // would have seen the parse error. - const escaped = permission.replace(/([!%_])/g, '!$1'); - const exact = permission; - const prefix = `${escaped}:%`; + const { where, params } = subtreeClause([permission]); const removed: Array<{ table: 'user_to_app_permissions' | 'dev_to_app_permissions'; @@ -764,8 +809,8 @@ export class PermissionStore extends PuterStore { ] as const) { const rows = (await this.clients.db.read( `SELECT \`user_id\`, \`app_id\`, \`permission\` FROM \`${table}\` ` + - "WHERE `permission` = ? OR `permission` LIKE ? ESCAPE '!'", - [exact, prefix], + `WHERE ${where}`, + params, )) as Array<{ user_id: number; app_id: number; @@ -774,9 +819,8 @@ export class PermissionStore extends PuterStore { if (rows.length === 0) continue; await this.clients.db.write( - `DELETE FROM \`${table}\` ` + - "WHERE `permission` = ? OR `permission` LIKE ? ESCAPE '!'", - [exact, prefix], + `DELETE FROM \`${table}\` WHERE ${where}`, + params, ); for (const row of rows) removed.push({ table, ...row }); } diff --git a/src/backend/stores/systemKv/SystemKVStore.ts b/src/backend/stores/systemKv/SystemKVStore.ts index 84c4d8a28..a19ed0f9d 100644 --- a/src/backend/stores/systemKv/SystemKVStore.ts +++ b/src/backend/stores/systemKv/SystemKVStore.ts @@ -137,7 +137,7 @@ export interface RecursiveRecord { /** Namespace app component for an actor acting without an app. */ export const KV_GLOBAL_APP_KEY = 'os-global'; const SYSTEM_NAMESPACE = `v1:${SYSTEM_ACTOR_UUID}:${KV_GLOBAL_APP_KEY}`; -export const MAX_KEY_BYTES = 1024; +const MAX_KEY_BYTES = 1024; /** Optimistic-concurrency counter every reserved-item write moves. */ const RESERVED_VERSION_ATTR = 'version';