diff --git a/backend/__tests__/unit/routes/grants.read.test.js b/backend/__tests__/unit/routes/grants.read.test.js index 1db20abc8..5aa333600 100644 --- a/backend/__tests__/unit/routes/grants.read.test.js +++ b/backend/__tests__/unit/routes/grants.read.test.js @@ -82,7 +82,7 @@ const trailRow = (over = {}) => ({ }); const FIELDS = ['grantId', 'installationId', 'target', 'tools', 'writeMode', 'budget', 'effectiveAudience', - 'expiresAt', 'revokedAt', 'parentGrantId', 'rootGrantId', 'createdAt', 'grantedBy']; + 'expiresAt', 'revokedAt', 'revokedBy', 'parentGrantId', 'rootGrantId', 'createdAt', 'grantedBy']; beforeAll(async () => { mongod = await MongoMemoryServer.create(); @@ -219,7 +219,7 @@ describe('GET /api/grants/:grantId', () => { expect(res.body).toMatchObject({ grantId: row.grantId, installationId: 'install-1', target: { kind: 'pod', id: POD }, tools: ['github.list_issues', 'github.comment'], writeMode: 'write-with-confirm', - budget: { calls: 10, windowMs: 60000 }, effectiveAudience: [SEAT], revokedAt: null, + budget: { calls: 10, windowMs: 60000 }, effectiveAudience: [SEAT], revokedAt: null, revokedBy: null, parentGrantId: null, rootGrantId: 'grant_root', grantedBy: OWNER, }); expect(res.body.connectionId).toBeUndefined(); @@ -248,3 +248,16 @@ describe('GET /api/grants/:grantId', () => { } }); }); + +describe('POST /api/grants/:grantId/revoke', () => { + test('records the authenticated caller as revokedBy', async () => { + const row = await RoomGrant.create(grant()); + const res = await request(app).post(`/api/grants/${row.grantId}/revoke`).set('x-test-user', OWNER); + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ grantId: row.grantId, revoked: 1 }); + + const saved = await RoomGrant.findOne({ grantId: row.grantId }).lean(); + expect(saved.revokedAt).toBeInstanceOf(Date); + expect(saved.revokedBy).toBe(OWNER); + }); +}); diff --git a/backend/__tests__/unit/services/roomGrantService.test.js b/backend/__tests__/unit/services/roomGrantService.test.js index 6914bf415..cbe27c993 100644 --- a/backend/__tests__/unit/services/roomGrantService.test.js +++ b/backend/__tests__/unit/services/roomGrantService.test.js @@ -126,10 +126,13 @@ describe('RoomGrant', () => { const root = await service.mintGrant(baseGrant({ grantId: 'root-revoke' })); const child = await service.attenuateGrant({ parentGrantId: root.grantId, grantId: 'ignored-child' }); const grandchild = await service.attenuateGrant({ parentGrantId: child.grantId }); - await service.revokeGrant(root.grantId); + const revokedBy = 'revoker-1'; + await service.revokeGrant(root.grantId, revokedBy); for (const grant of [root, child, grandchild]) { await expect(service.assertGrantUsable({ grantId: grant.grantId })) .rejects.toMatchObject({ code: 'grant_revoked' }); + const saved = await RoomGrant.findOne({ grantId: grant.grantId }).lean(); + expect(saved.revokedBy).toBe(revokedBy); } }); }); diff --git a/backend/models/RoomGrant.ts b/backend/models/RoomGrant.ts index e8136f8fc..a27fe3caa 100644 --- a/backend/models/RoomGrant.ts +++ b/backend/models/RoomGrant.ts @@ -33,6 +33,8 @@ export interface IRoomGrant extends Document { audience: string[]; expiresAt: Date; revokedAt?: Date | null; + /** User who revoked this grant (and its descendants), when known. */ + revokedBy?: string | null; parentGrantId?: string | null; /** Denormalized root used to close revoke/mint races and check lineage. */ rootGrantId?: string | null; @@ -44,7 +46,7 @@ export interface IRoomGrant extends Document { export interface RoomGrantModel extends Model { /** Revoke a grant and all descendants with one updateMany write. */ - revokeCascade(grantId: string): Promise; + revokeCascade(grantId: string, revokedBy: string): Promise; } const GrantBudgetSchema = new Schema( @@ -83,6 +85,7 @@ const RoomGrantSchema = new Schema( // reason to delete the grant or its call trail. expiresAt: { type: Date, required: true }, revokedAt: { type: Date, default: null }, + revokedBy: { type: String, default: null, trim: true }, parentGrantId: { type: String, default: null, trim: true }, rootGrantId: { type: String, default: null, trim: true }, brokerId: { type: String, required: true, trim: true }, @@ -102,6 +105,7 @@ RoomGrantSchema.index({ connectionId: 1, installationId: 1 }); */ RoomGrantSchema.statics.revokeCascade = async function revokeCascade( grantId: string, + revokedBy: string, ): Promise { const ids: string[] = [grantId]; const seen = new Set(ids); @@ -126,7 +130,7 @@ RoomGrantSchema.statics.revokeCascade = async function revokeCascade( grantId: { $in: ids }, $or: [{ revokedAt: { $exists: false } }, { revokedAt: null }], }, - { $set: { revokedAt } }, + { $set: { revokedAt, revokedBy } }, ); return result.modifiedCount || 0; }; diff --git a/backend/routes/grants.ts b/backend/routes/grants.ts index a604658ed..40a6a7edd 100644 --- a/backend/routes/grants.ts +++ b/backend/routes/grants.ts @@ -62,7 +62,7 @@ interface AuthenticatedRequest extends express.Request { * left, so only the effective audience goes out. */ type GrantRow = Pick & { audience?: string[]; connectionId?: string }; + | 'expiresAt' | 'revokedAt' | 'revokedBy' | 'parentGrantId' | 'rootGrantId' | 'createdAt'> & { audience?: string[]; connectionId?: string }; const projectGrant = (grant: GrantRow, currentMembers: string[], grantedBy: string | null) => ({ grantId: grant.grantId, @@ -74,6 +74,7 @@ const projectGrant = (grant: GrantRow, currentMembers: string[], grantedBy: stri effectiveAudience: effectiveAudience(grant, currentMembers), expiresAt: grant.expiresAt, revokedAt: grant.revokedAt ?? null, + revokedBy: grant.revokedBy ?? null, parentGrantId: grant.parentGrantId ?? null, rootGrantId: grant.rootGrantId ?? null, createdAt: grant.createdAt, @@ -270,7 +271,7 @@ const revokeHandler = async (req: AuthenticatedRequest, res: express.Response): if (!connection || connectionOwnerId(connection) !== userId) { return res.status(403).json({ error: 'access_denied' }); } - const revoked = await revokeGrant(grant.grantId); + const revoked = await revokeGrant(grant.grantId, userId); return res.json({ grantId: grant.grantId, revoked }); } catch (error) { return handleError(res, error); diff --git a/backend/services/roomGrantService.ts b/backend/services/roomGrantService.ts index d33b56a18..74f4b611b 100644 --- a/backend/services/roomGrantService.ts +++ b/backend/services/roomGrantService.ts @@ -387,11 +387,12 @@ export const attenuateGrant = async (input: RoomGrantAttenuationInput): Promise< }); }; -export const revokeGrant = async (grantId: string): Promise => { +export const revokeGrant = async (grantId: string, revokedBy: string): Promise => { const id = asId(grantId, 'grantId'); + const actor = asId(revokedBy, 'revokedBy'); const root = await RoomGrant.findOne({ grantId: id }).select('grantId').lean(); if (!root) throw new RoomGrantError('grant_not_found', 'grant not found', 404); - return RoomGrant.revokeCascade(id); + return RoomGrant.revokeCascade(id, actor); }; export const assertGrantUsable = async ( diff --git a/docs/design/evidence/task-031-revoked-by-after-mobile.png b/docs/design/evidence/task-031-revoked-by-after-mobile.png new file mode 100644 index 000000000..84f5c0e93 Binary files /dev/null and b/docs/design/evidence/task-031-revoked-by-after-mobile.png differ diff --git a/docs/design/evidence/task-031-revoked-by-after.png b/docs/design/evidence/task-031-revoked-by-after.png new file mode 100644 index 000000000..6bad1649a Binary files /dev/null and b/docs/design/evidence/task-031-revoked-by-after.png differ diff --git a/docs/plans/tools-catalogue-room-grants.md b/docs/plans/tools-catalogue-room-grants.md index cdb2b10d8..a5989769d 100644 --- a/docs/plans/tools-catalogue-room-grants.md +++ b/docs/plans/tools-catalogue-room-grants.md @@ -153,7 +153,7 @@ The Add form: a `writeMode` segment (`read` / `read and write, ask first` / `rea - `GET /api/pods/:podId/grants` — every grant whose target is the pod or a seat in it, gated by `canViewPod`, each row in the field list below. - `GET /api/grants/:grantId/calls` — the trail from `ToolCall.listForGrant` plus the three counts by outcome. Gated by `canViewPod` on the grant's target pod; a seat grant's trail goes only to the granter and that seat. Every line carries `argsDigest` and never args. -- `GET /api/grants/:grantId` returns the whole row today, `connectionId` and `brokerId` included. Once the page reads it, it returns an explicit field list and nothing else: `grantId`, `installationId`, `target`, `tools`, `writeMode`, `budget`, `effectiveAudience`, `expiresAt`, `revokedAt`, `parentGrantId`, `rootGrantId`, `createdAt`, and `grantedBy` resolved from the Connection's owner. `connectionId` and `brokerId` are the broker's business, not the page's. The raw `audience` snapshot stays out too: it still names agents who have left, and Change access pre-fills from `effectiveAudience` (Vera 67568). +- `GET /api/grants/:grantId` returns the whole row today, `connectionId` and `brokerId` included. Once the page reads it, it returns an explicit field list and nothing else: `grantId`, `installationId`, `target`, `tools`, `writeMode`, `budget`, `effectiveAudience`, `expiresAt`, `revokedAt`, `revokedBy`, `parentGrantId`, `rootGrantId`, `createdAt`, and `grantedBy` resolved from the Connection's owner. `revokedBy` is the member who performed the revoke; `connectionId` and `brokerId` are the broker's business, not the page's. The raw `audience` snapshot stays out too: it still names agents who have left, and Change access pre-fills from `effectiveAudience` (Vera 67568). Named tests: `the pod grants list refuses a non-member`; `the trail refuses a non-member and never returns args`; `a seat grant's trail is visible only to its granter and the seat`; `GET /api/grants/:id returns the field list and never connectionId or brokerId`. diff --git a/frontend/src/v2/__tests__/V2ConnectorTools.test.tsx b/frontend/src/v2/__tests__/V2ConnectorTools.test.tsx index b5aae4d99..a9842e358 100644 --- a/frontend/src/v2/__tests__/V2ConnectorTools.test.tsx +++ b/frontend/src/v2/__tests__/V2ConnectorTools.test.tsx @@ -32,9 +32,9 @@ const pods = [ const grantLive = { grantId: 'grant_live', installationId: 'inst-1', target: { kind: 'pod', id: 'p1' }, tools: ['github.list_issues', 'github.comment_on_issue'], writeMode: 'write-with-confirm', budget: { calls: 50, windowMs: 3600000 }, effectiveAudience: ['a1'], expiresAt: iso(6 * 86400000), - revokedAt: null, parentGrantId: null, rootGrantId: null, createdAt: iso(-3600000), grantedBy: 'u1', + revokedAt: null, revokedBy: null, parentGrantId: null, rootGrantId: null, createdAt: iso(-3600000), grantedBy: 'u1', }; -const grantRevoked = { ...grantLive, grantId: 'grant_gone', target: { kind: 'pod', id: 'p2' }, writeMode: 'read', effectiveAudience: [], revokedAt: iso(-600000), createdAt: iso(-86400000) }; +const grantRevoked = { ...grantLive, grantId: 'grant_gone', target: { kind: 'pod', id: 'p2' }, writeMode: 'read', effectiveAudience: [], revokedAt: iso(-600000), revokedBy: 'u1', createdAt: iso(-86400000) }; const trail = { grantId: 'grant_live', calls: [ @@ -91,7 +91,7 @@ test('rows carry the states table: a live grant pulses when used in the last 10 expect(liveDot).toHaveClass('v2-connector-row__dot--live'); await waitFor(() => expect(liveDot).toHaveClass('v2-connector-row__dot--pulse')); const gone = screen.getByRole('button', { name: 'View GitHub in Ops' }); - expect(within(gone).getByText('revoked 10m ago')).toBeInTheDocument(); + expect(within(gone).getByText('revoked by sam 10m ago')).toBeInTheDocument(); expect(gone.querySelector('.v2-connector-row__dot')).toHaveClass('v2-connector-row__dot--empty'); // No not-yet row and no Add without a catalogue: nothing the server does not enforce. expect(screen.queryByText('not granted')).not.toBeInTheDocument(); diff --git a/frontend/src/v2/components/V2ConnectorTools.tsx b/frontend/src/v2/components/V2ConnectorTools.tsx index 13274d89f..551f484a5 100644 --- a/frontend/src/v2/components/V2ConnectorTools.tsx +++ b/frontend/src/v2/components/V2ConnectorTools.tsx @@ -29,6 +29,7 @@ export interface ToolGrant { effectiveAudience: string[]; expiresAt: string; revokedAt: string | null; + revokedBy: string | null; parentGrantId: string | null; rootGrantId: string | null; createdAt: string; @@ -368,9 +369,12 @@ const V2ConnectorTools: React.FC = ({ pods }) => { const entry = entryFor(grant); const label = toolLabel(grant); const when = t('tools.grantedWhen', { defaultValue: 'granted {{rel}}', rel: relativeTime(grant.createdAt) }); + const revokedBy = grant.revokedBy ? memberName(grant.revokedBy) : null; const line2 = dead ? (grant.revokedAt - ? t('tools.revokedLine', { defaultValue: 'revoked {{rel}}', rel: relativeTime(grant.revokedAt) }) + ? (revokedBy + ? t('tools.revokedByLine', { defaultValue: 'revoked by {{member}} {{rel}}', member: revokedBy, rel: relativeTime(grant.revokedAt) }) + : t('tools.revokedLine', { defaultValue: 'revoked {{rel}}', rel: relativeTime(grant.revokedAt) })) : t('tools.expiredLine', { defaultValue: 'expired {{rel}}', rel: relativeTime(grant.expiresAt) })) : `${audienceLabels(grant)} ${t('tools.mayUse', { defaultValue: 'may use it' })} · ${asksFirst(grant)}`; return (