From 7ec8e19bf9bbae84430f64912e5a37781ac4fda4 Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Thu, 20 Aug 2026 16:33:52 -0400 Subject: [PATCH] fix(perms): keep /auth/revoke-user-user as a deprecated route Dropping this route with the rest of the unused user-to-user plumbing went too far. The grant side is retired and stays retired - puter.fs.share() is the only way in - but access those grants left behind has to remain withdrawable, and a caller reaching the endpoint over HTTP directly had no replacement. Revoking can only ever narrow what someone can reach, so keeping it carries no risk. revokeUserUserPermission never left the permission service; it is load-bearing for puter.fs.share(). This only re-wires the handler to it, with the gates it always had. Nothing in this repo calls the route, which makes it exactly what a later cleanup reads as dead, so a test pins the registration and its gate alongside the restored 400 and grant/revoke round-trip cases. The 501 stub at grant-user-user and the never-called /group/* routes stay deleted, as does puter.perms.revokeUser - puter.fs.unshare() replaces it and falls back to live grants when no share row exists. --- .../controllers/auth/AuthController.test.ts | 70 ++++++++++++++++++- .../controllers/auth/AuthController.ts | 28 ++++++++ 2 files changed, 97 insertions(+), 1 deletion(-) diff --git a/src/backend/controllers/auth/AuthController.test.ts b/src/backend/controllers/auth/AuthController.test.ts index 16877ab9f..0b6ca493d 100644 --- a/src/backend/controllers/auth/AuthController.test.ts +++ b/src/backend/controllers/auth/AuthController.test.ts @@ -38,7 +38,11 @@ import type { Actor } from '../../core/actor.js'; import { Context, runWithContext } from '../../core/context.js'; import { HttpError } from '../../core/http/HttpError.js'; import { requireUserActorGate } from '../../core/http/middleware/gates.js'; -import type { TokenSource } from '../../core/http/types.js'; +import { + ROUTES_METADATA_KEY, + type CollectedRoute, + type TokenSource, +} from '../../core/http/types.js'; import { PuterServer } from '../../server.js'; import { FULL_API_ACCESS } from '../../services/permission/consts.js'; import { setupTestServer } from '../../testUtil.js'; @@ -4724,6 +4728,16 @@ describe('AuthController.handleCaptchaGenerate + handleGetAntiCsrfToken', () => // ── Permission revoke flows ──────────────────────────────────────── describe('AuthController permission revokes', () => { + it('revoke-user-user: 400 on missing target_username/permission', async () => { + const { actor } = await makeUserAndActor(); + await expect( + controller.handleRevokeUserUser( + makeReq({ permission: 'fs:read' }, { actor }), + makeRes(), + ), + ).rejects.toMatchObject({ statusCode: 400 }); + }); + it('revoke-user-app: 400 on missing app_uid/permission', async () => { const { actor } = await makeUserAndActor(); await expect( @@ -4734,6 +4748,60 @@ describe('AuthController permission revokes', () => { ).rejects.toMatchObject({ statusCode: 400 }); }); + // No in-tree caller, so a later cleanup would read it as dead: pin the gate. + it('revoke-user-user: stays registered as a user-actor route', () => { + const proto = Object.getPrototypeOf(controller) as Record< + string, + CollectedRoute[] | undefined + >; + const route = (proto[ROUTES_METADATA_KEY] ?? []).find( + (r) => r.path === '/auth/revoke-user-user', + ); + expect(route, '/auth/revoke-user-user is not registered').toBeDefined(); + expect(route!.method).toBe('post'); + expect(route!.options).toMatchObject({ + subdomain: 'api', + requireUserActor: true, + }); + }); + + // Why the deprecated route exists: older grants stay withdrawable. + it('revoke-user-user: round-trips a grant + revoke without throwing', async () => { + const { actor: issuerActor, user: issuer } = await makeUserAndActor(); + const { user: target } = await makeUserAndActor(); + const permission = `service:test-revoke-${uuidv4()}:ii:read`; + await server.stores.permission.setFlatUserPerm( + issuer.id, + `manage:${permission}`, + { + permission: `manage:${permission}`, + deleted: false, + issuer_user_id: issuer.id, + } as never, + ); + // Through the service: the grant route is retired, the revoke is not. + await inCtx(issuerActor, () => + server.services.permission.grantUserUserPermission( + issuerActor, + target.username, + permission, + ), + ); + + // Asserts the controller path only: the process-wide Redis-mock scan + // cache makes a post-revoke `check()` unreliable across tests. + const res = makeRes(); + await inCtx(issuerActor, () => + controller.handleRevokeUserUser( + makeReq( + { target_username: target.username, permission }, + { actor: issuerActor }, + ), + res, + ), + ); + expect(res.body).toEqual({}); + }); }); // ── Permission checks + listing ──────────────────────────────────── diff --git a/src/backend/controllers/auth/AuthController.ts b/src/backend/controllers/auth/AuthController.ts index ae412cd3d..a358a45b2 100644 --- a/src/backend/controllers/auth/AuthController.ts +++ b/src/backend/controllers/auth/AuthController.ts @@ -3187,6 +3187,34 @@ export class AuthController extends PuterController { // -- Permission revokes ------------------------------------------ + /** + * @deprecated Use `puter.fs.unshare()`, which withdraws the share row and + * the grant together. Kept for direct HTTP callers: the grant side is + * retired, but access it left behind has to stay withdrawable. + */ + @Post('/auth/revoke-user-user', { + subdomain: 'api', + requireUserActor: true, + rateLimit: GRANT_LIMIT, + }) + async handleRevokeUserUser(req: Request, res: Response): Promise { + const { target_username, permission, meta } = req.body; + if (!target_username || !permission) { + throw new HttpError( + 400, + 'Missing `target_username` or `permission`', + { legacyCode: 'bad_request' }, + ); + } + await this.services.permission.revokeUserUserPermission( + req.actor!, + target_username, + permission, + meta, + ); + res.json({}); + } + @Post('/auth/revoke-user-app', { subdomain: 'api', requireUserActor: true,