Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions backend/__tests__/unit/routes/grants.read.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
});
});
5 changes: 4 additions & 1 deletion backend/__tests__/unit/services/roomGrantService.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});
});
8 changes: 6 additions & 2 deletions backend/models/RoomGrant.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -44,7 +46,7 @@ export interface IRoomGrant extends Document {

export interface RoomGrantModel extends Model<IRoomGrant> {
/** Revoke a grant and all descendants with one updateMany write. */
revokeCascade(grantId: string): Promise<number>;
revokeCascade(grantId: string, revokedBy: string): Promise<number>;
}

const GrantBudgetSchema = new Schema<IRoomGrantBudget>(
Expand Down Expand Up @@ -83,6 +85,7 @@ const RoomGrantSchema = new Schema<IRoomGrant>(
// 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 },
Expand All @@ -102,6 +105,7 @@ RoomGrantSchema.index({ connectionId: 1, installationId: 1 });
*/
RoomGrantSchema.statics.revokeCascade = async function revokeCascade(
grantId: string,
revokedBy: string,
): Promise<number> {
const ids: string[] = [grantId];
const seen = new Set(ids);
Expand All @@ -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;
};
Expand Down
5 changes: 3 additions & 2 deletions backend/routes/grants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ interface AuthenticatedRequest extends express.Request {
* left, so only the effective audience goes out.
*/
type GrantRow = Pick<IRoomGrant, 'grantId' | 'installationId' | 'target' | 'tools' | 'writeMode' | 'budget'
| 'expiresAt' | 'revokedAt' | 'parentGrantId' | 'rootGrantId' | 'createdAt'> & { 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,
Expand All @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions backend/services/roomGrantService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -387,11 +387,12 @@ export const attenuateGrant = async (input: RoomGrantAttenuationInput): Promise<
});
};

export const revokeGrant = async (grantId: string): Promise<number> => {
export const revokeGrant = async (grantId: string, revokedBy: string): Promise<number> => {
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 (
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion docs/plans/tools-catalogue-room-grants.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.

Expand Down
6 changes: 3 additions & 3 deletions frontend/src/v2/__tests__/V2ConnectorTools.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
Expand Down Expand Up @@ -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();
Expand Down
6 changes: 5 additions & 1 deletion frontend/src/v2/components/V2ConnectorTools.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -368,9 +369,12 @@ const V2ConnectorTools: React.FC<Props> = ({ 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 (
Expand Down
Loading