From c7b3e2c916ee11c163feb60096caf9df942427d7 Mon Sep 17 00:00:00 2001 From: Christian Fehmer Date: Tue, 10 Sep 2024 11:35:57 +0200 Subject: [PATCH] impr: move permission checks to contracts (@fehmer, @miodec) (#5848) !nuf --- .../__tests__/middlewares/permission.spec.ts | 317 ++++++++++++++++++ backend/package.json | 7 +- backend/scripts/openapi.ts | 87 +++-- backend/scripts/tsconfig.json | 11 + backend/src/api/routes/admin.ts | 2 - backend/src/api/routes/ape-keys.ts | 8 +- backend/src/api/routes/configuration.ts | 4 - backend/src/api/routes/index.ts | 7 +- backend/src/api/routes/quotes.ts | 18 - backend/src/api/routes/users.ts | 6 - backend/src/middlewares/permission.ts | 219 +++++++++--- frontend/package.json | 2 +- packages/contracts/package.json | 2 +- packages/contracts/src/admin.ts | 1 + packages/contracts/src/ape-keys.ts | 1 + packages/contracts/src/configuration.ts | 2 + packages/contracts/src/quotes.ts | 4 + packages/contracts/src/schemas/api.ts | 9 + packages/contracts/src/users.ts | 1 + pnpm-lock.yaml | 53 +-- 20 files changed, 611 insertions(+), 150 deletions(-) create mode 100644 backend/__tests__/middlewares/permission.spec.ts create mode 100644 backend/scripts/tsconfig.json diff --git a/backend/__tests__/middlewares/permission.spec.ts b/backend/__tests__/middlewares/permission.spec.ts new file mode 100644 index 000000000000..19146ba2d6d7 --- /dev/null +++ b/backend/__tests__/middlewares/permission.spec.ts @@ -0,0 +1,317 @@ +import { Response } from "express"; +import { verifyPermissions } from "../../src/middlewares/permission"; +import { EndpointMetadata } from "@monkeytype/contracts/schemas/api"; +import * as Misc from "../../src/utils/misc"; +import * as AdminUids from "../../src/dal/admin-uids"; +import * as UserDal from "../../src/dal/user"; +import MonkeyError from "../../src/utils/error"; + +const uid = "123456789"; + +describe("permission middleware", () => { + const handler = verifyPermissions(); + const res: Response = {} as any; + const next = vi.fn(); + const getPartialUserMock = vi.spyOn(UserDal, "getPartialUser"); + const isAdminMock = vi.spyOn(AdminUids, "isAdmin"); + const isDevMock = vi.spyOn(Misc, "isDevEnvironment"); + + beforeEach(() => { + next.mockReset(); + getPartialUserMock.mockReset().mockResolvedValue({} as any); + isDevMock.mockReset().mockReturnValue(false); + isAdminMock.mockReset().mockResolvedValue(false); + }); + afterEach(() => { + //next function must only be called once + expect(next).toHaveBeenCalledOnce(); + }); + + it("should bypass without requiredPermission", async () => { + //GIVEN + const req = givenRequest({}); + //WHEN + await handler(req, res, next); + + //THEN + expect(next).toHaveBeenCalledWith(); + }); + it("should bypass with empty requiredPermission", async () => { + //GIVEN + const req = givenRequest({ requirePermission: [] }); + //WHEN + await handler(req, res, next); + + //THE + expect(next).toHaveBeenCalledWith(); + }); + + describe("admin check", () => { + const requireAdminPermission: EndpointMetadata = { + requirePermission: "admin", + }; + + it("should fail without authentication", async () => { + //GIVEN + const req = givenRequest(requireAdminPermission); + //WHEN + await handler(req, res, next); + + //THEN + expect(next).toHaveBeenCalledWith( + new MonkeyError(403, "You don't have permission to do this.") + ); + }); + it("should pass without authentication if publicOnDev on dev", async () => { + //GIVEN + isDevMock.mockReturnValue(true); + const req = givenRequest( + { + ...requireAdminPermission, + authenticationOptions: { isPublicOnDev: true }, + }, + { uid } + ); + //WHEN + await handler(req, res, next); + + //THEN + expect(next).toHaveBeenCalledWith(); + }); + it("should fail without authentication if publicOnDev on prod ", async () => { + //GIVEN + const req = givenRequest( + { + ...requireAdminPermission, + authenticationOptions: { isPublicOnDev: true }, + }, + { uid } + ); + //WHEN + await handler(req, res, next); + + //THEN + expect(next).toHaveBeenCalledWith( + new MonkeyError(403, "You don't have permission to do this.") + ); + }); + it("should fail without admin permissions", async () => { + //GIVEN + const req = givenRequest(requireAdminPermission, { uid }); + + //WHEN + await handler(req, res, next); + + //THEN + expect(next).toHaveBeenCalledWith( + new MonkeyError(403, "You don't have permission to do this.") + ); + expect(isAdminMock).toHaveBeenCalledWith(uid); + }); + }); + describe("user checks", () => { + it("should fetch user only once", async () => { + //GIVEN + const req = givenRequest( + { + requirePermission: ["canReport", "canManageApeKeys"], + }, + { uid } + ); + + //WHEN + await handler(req, res, next); + + //THEN + expect(getPartialUserMock).toHaveBeenCalledOnce(); + expect(getPartialUserMock).toHaveBeenCalledWith( + uid, + "check user permissions", + ["canReport", "canManageApeKeys"] + ); + }); + it("should fail if authentication is missing", async () => { + //GIVEN + const req = givenRequest({ + requirePermission: ["canReport", "canManageApeKeys"], + }); + + //WHEN + await handler(req, res, next); + + //THEN + expect(next).toHaveBeenCalledWith( + new MonkeyError( + 403, + "Failed to check permissions, authentication required." + ) + ); + }); + }); + describe("quoteMod check", () => { + const requireQuoteMod: EndpointMetadata = { + requirePermission: "quoteMod", + }; + + it("should pass for quoteAdmin", async () => { + //GIVEN + getPartialUserMock.mockResolvedValue({ quoteMod: true } as any); + const req = givenRequest(requireQuoteMod, { uid }); + + //WHEN + await handler(req, res, next); + + //THEN + expect(next).toHaveBeenCalledWith(); + expect(getPartialUserMock).toHaveBeenCalledWith( + uid, + "check user permissions", + ["quoteMod"] + ); + }); + it("should pass for specific language", async () => { + //GIVEN + getPartialUserMock.mockResolvedValue({ quoteMod: "english" } as any); + const req = givenRequest(requireQuoteMod, { uid }); + + //WHEN + await handler(req, res, next); + + //THEN + expect(next).toHaveBeenCalledWith(); + expect(getPartialUserMock).toHaveBeenCalledWith( + uid, + "check user permissions", + ["quoteMod"] + ); + }); + it("should fail for empty string", async () => { + //GIVEN + getPartialUserMock.mockResolvedValue({ quoteMod: "" } as any); + const req = givenRequest(requireQuoteMod, { uid }); + + //WHEN + await handler(req, res, next); + + //THEN + expect(next).toHaveBeenCalledWith( + new MonkeyError(403, "You don't have permission to do this.") + ); + }); + it("should fail for missing quoteMod", async () => { + //GIVEN + getPartialUserMock.mockResolvedValue({} as any); + const req = givenRequest(requireQuoteMod, { uid }); + + //WHEN + await handler(req, res, next); + + //THEN + expect(next).toHaveBeenCalledWith( + new MonkeyError(403, "You don't have permission to do this.") + ); + }); + }); + describe("canReport check", () => { + const requireCanReport: EndpointMetadata = { + requirePermission: "canReport", + }; + + it("should fail if user cannot report", async () => { + //GIVEN + getPartialUserMock.mockResolvedValue({ canReport: false } as any); + const req = givenRequest(requireCanReport, { uid }); + + //WHEN + await handler(req, res, next); + + //THEN + expect(next).toHaveBeenCalledWith( + new MonkeyError(403, "You don't have permission to do this.") + ); + expect(getPartialUserMock).toHaveBeenCalledWith( + uid, + "check user permissions", + ["canReport"] + ); + }); + it("should pass if user can report", async () => { + //GIVEN + getPartialUserMock.mockResolvedValue({ canReport: true } as any); + const req = givenRequest(requireCanReport, { uid }); + + //WHEN + await handler(req, res, next); + + //THEN + expect(next).toHaveBeenCalledWith(); + }); + it("should pass if canReport is not set", async () => { + //GIVEN + getPartialUserMock.mockResolvedValue({} as any); + const req = givenRequest(requireCanReport, { uid }); + + //WHEN + await handler(req, res, next); + + //THEN + expect(next).toHaveBeenCalledWith(); + }); + }); + describe("canManageApeKeys check", () => { + const requireCanReport: EndpointMetadata = { + requirePermission: "canManageApeKeys", + }; + + it("should fail if user cannot report", async () => { + //GIVEN + getPartialUserMock.mockResolvedValue({ canManageApeKeys: false } as any); + const req = givenRequest(requireCanReport, { uid }); + + //WHEN + await handler(req, res, next); + + //THEN + expect(next).toHaveBeenCalledWith( + new MonkeyError( + 403, + "You have lost access to ape keys, please contact support" + ) + ); + expect(getPartialUserMock).toHaveBeenCalledWith( + uid, + "check user permissions", + ["canManageApeKeys"] + ); + }); + it("should pass if user can report", async () => { + //GIVEN + getPartialUserMock.mockResolvedValue({ canManageApeKeys: true } as any); + const req = givenRequest(requireCanReport, { uid }); + + //WHEN + await handler(req, res, next); + + //THEN + expect(next).toHaveBeenCalledWith(); + }); + it("should pass if canManageApeKeys is not set", async () => { + //GIVEN + getPartialUserMock.mockResolvedValue({} as any); + const req = givenRequest(requireCanReport, { uid }); + + //WHEN + await handler(req, res, next); + + //THEN + expect(next).toHaveBeenCalledWith(); + }); + }); +}); + +function givenRequest( + metadata: EndpointMetadata, + decodedToken?: Partial +): TsRestRequest { + return { tsRestRoute: { metadata }, ctx: { decodedToken } } as any; +} diff --git a/backend/package.json b/backend/package.json index 6e976b455d28..d571914132c1 100644 --- a/backend/package.json +++ b/backend/package.json @@ -24,9 +24,9 @@ "dependencies": { "@date-fns/utc": "1.2.0", "@monkeytype/contracts": "workspace:*", - "@ts-rest/core": "3.49.3", - "@ts-rest/express": "3.49.3", - "@ts-rest/open-api": "3.49.3", + "@ts-rest/core": "3.51.0", + "@ts-rest/express": "3.51.0", + "@ts-rest/open-api": "3.51.0", "bcrypt": "5.1.1", "bullmq": "1.91.1", "chalk": "4.1.2", @@ -87,6 +87,7 @@ "eslint": "8.57.0", "eslint-watch": "8.0.0", "ioredis-mock": "7.4.0", + "openapi3-ts": "2.0.2", "readline-sync": "1.4.10", "supertest": "6.2.3", "tsx": "4.16.2", diff --git a/backend/scripts/openapi.ts b/backend/scripts/openapi.ts index ffa5648df8f0..83c78277ad2f 100644 --- a/backend/scripts/openapi.ts +++ b/backend/scripts/openapi.ts @@ -2,14 +2,14 @@ import { generateOpenApi } from "@ts-rest/open-api"; import { contract } from "@monkeytype/contracts/index"; import { writeFileSync, mkdirSync } from "fs"; import { - ApeKeyRateLimit, EndpointMetadata, + Permission, } from "@monkeytype/contracts/schemas/api"; -import type { OpenAPIObject } from "openapi3-ts"; +import type { OpenAPIObject, OperationObject } from "openapi3-ts"; import { + RateLimitIds, getLimits, - limits, - RateLimit, + RateLimiterId, Window, } from "@monkeytype/contracts/rate-limit/index"; import { formatDuration } from "date-fns"; @@ -143,55 +143,76 @@ export function getOpenApi(): OpenAPIObject { operationMapper: (operation, route) => { const metadata = route.metadata as EndpointMetadata; - addRateLimit(operation, metadata); + if (!operation.description?.trim()?.endsWith(".")) + operation.description += "."; + operation.description += "\n\n"; - const result = { - ...operation, - ...addAuth(metadata), - ...addTags(metadata), - }; + addAuth(operation, metadata); + addRateLimit(operation, metadata); + addTags(operation, metadata); - return result; + return operation; }, } ); return openApiDocument; } -function addAuth(metadata: EndpointMetadata | undefined): object { - const auth = metadata?.["authenticationOptions"] ?? {}; +function addAuth( + operation: OperationObject, + metadata: EndpointMetadata | undefined +): void { + const auth = metadata?.authenticationOptions ?? {}; + const permissions = getRequiredPermissions(metadata) ?? []; const security: SecurityRequirementObject[] = []; - if (!auth.isPublic === true && !auth.isPublicOnDev === true) { - security.push({ BearerAuth: [] }); + if (!auth.isPublic && !auth.isPublicOnDev) { + security.push({ BearerAuth: permissions }); if (auth.acceptApeKeys === true) { - security.push({ ApeKey: [] }); + security.push({ ApeKey: permissions }); } } const includeInPublic = auth.isPublic === true || auth.acceptApeKeys === true; - return { - "x-public": includeInPublic ? "yes" : "no", - security, - }; + operation["x-public"] = includeInPublic ? "yes" : "no"; + operation.security = security; + + if (permissions.length !== 0) { + operation.description += `**Required permissions:** ${permissions.join( + ", " + )}\n\n`; + } } -function addTags(metadata: EndpointMetadata | undefined): object { - if (metadata === undefined || metadata.openApiTags === undefined) return {}; - return { - tags: Array.isArray(metadata.openApiTags) - ? metadata.openApiTags - : [metadata.openApiTags], - }; +function getRequiredPermissions( + metadata: EndpointMetadata | undefined +): Permission[] | undefined { + if (metadata === undefined || metadata.requirePermission === undefined) + return undefined; + + if (Array.isArray(metadata.requirePermission)) + return metadata.requirePermission; + return [metadata.requirePermission]; } -function addRateLimit(operation, metadata: EndpointMetadata | undefined): void { +function addTags( + operation: OperationObject, + metadata: EndpointMetadata | undefined +): void { + if (metadata === undefined || metadata.openApiTags === undefined) return; + operation.tags = Array.isArray(metadata.openApiTags) + ? metadata.openApiTags + : [metadata.openApiTags]; +} + +function addRateLimit( + operation: OperationObject, + metadata: EndpointMetadata | undefined +): void { if (metadata === undefined || metadata.rateLimit === undefined) return; const okResponse = operation.responses["200"]; if (okResponse === undefined) return; - if (!operation.description.trim().endsWith(".")) operation.description += "."; - operation.description += getRateLimitDescription(metadata.rateLimit); okResponse["headers"] = { @@ -211,10 +232,10 @@ function addRateLimit(operation, metadata: EndpointMetadata | undefined): void { }; } -function getRateLimitDescription(limit: RateLimit | ApeKeyRateLimit): string { +function getRateLimitDescription(limit: RateLimiterId | RateLimitIds): string { const limits = getLimits(limit); - let result = ` This operation can be called up to ${ + let result = `**Rate limit:** This operation can be called up to ${ limits.limiter.max } times ${formatWindow(limits.limiter.window)} for regular users`; @@ -224,7 +245,7 @@ function getRateLimitDescription(limit: RateLimit | ApeKeyRateLimit): string { )} with ApeKeys`; } - return result + "."; + return result + ".\n\n"; } function formatWindow(window: Window): string { diff --git a/backend/scripts/tsconfig.json b/backend/scripts/tsconfig.json new file mode 100644 index 000000000000..4de72146762b --- /dev/null +++ b/backend/scripts/tsconfig.json @@ -0,0 +1,11 @@ +{ + "extends": "@monkeytype/typescript-config/base.json", + "compilerOptions": { + "target": "ES6" + }, + "ts-node": { + "files": true + }, + "files": ["../src/types/types.d.ts"], + "include": ["./**/*"] +} diff --git a/backend/src/api/routes/admin.ts b/backend/src/api/routes/admin.ts index 4bed00ea31fc..805fd7d28331 100644 --- a/backend/src/api/routes/admin.ts +++ b/backend/src/api/routes/admin.ts @@ -5,7 +5,6 @@ import * as AdminController from "../controllers/admin"; import { adminContract } from "@monkeytype/contracts/admin"; import { initServer } from "@ts-rest/express"; import { validate } from "../../middlewares/configuration"; -import { checkIfUserIsAdmin } from "../../middlewares/permission"; import { callController } from "../ts-rest-adapter"; const commonMiddleware = [ @@ -15,7 +14,6 @@ const commonMiddleware = [ }, invalidMessage: "Admin endpoints are currently disabled.", }), - checkIfUserIsAdmin(), ]; const s = initServer(); diff --git a/backend/src/api/routes/ape-keys.ts b/backend/src/api/routes/ape-keys.ts index d59aca12e5a7..08ff067b6738 100644 --- a/backend/src/api/routes/ape-keys.ts +++ b/backend/src/api/routes/ape-keys.ts @@ -2,7 +2,7 @@ import { apeKeysContract } from "@monkeytype/contracts/ape-keys"; import { initServer } from "@ts-rest/express"; import * as ApeKeyController from "../controllers/ape-key"; import { callController } from "../ts-rest-adapter"; -import { checkUserPermissions } from "../../middlewares/permission"; + import { validate } from "../../middlewares/configuration"; const commonMiddleware = [ @@ -12,12 +12,6 @@ const commonMiddleware = [ }, invalidMessage: "ApeKeys are currently disabled.", }), - checkUserPermissions(["canManageApeKeys"], { - criteria: (user) => { - return user.canManageApeKeys ?? true; - }, - invalidMessage: "You have lost access to ape keys, please contact support", - }), ]; const s = initServer(); diff --git a/backend/src/api/routes/configuration.ts b/backend/src/api/routes/configuration.ts index e0b5df40e8ef..952cff290043 100644 --- a/backend/src/api/routes/configuration.ts +++ b/backend/src/api/routes/configuration.ts @@ -1,6 +1,5 @@ import { configurationContract } from "@monkeytype/contracts/configuration"; import { initServer } from "@ts-rest/express"; -import { checkIfUserIsAdmin } from "../../middlewares/permission"; import * as ConfigurationController from "../controllers/configuration"; import { callController } from "../ts-rest-adapter"; @@ -11,14 +10,11 @@ export default s.router(configurationContract, { handler: async (r) => callController(ConfigurationController.getConfiguration)(r), }, - update: { - middleware: [checkIfUserIsAdmin()], handler: async (r) => callController(ConfigurationController.updateConfiguration)(r), }, getSchema: { - middleware: [checkIfUserIsAdmin()], handler: async (r) => callController(ConfigurationController.getSchema)(r), }, }); diff --git a/backend/src/api/routes/index.ts b/backend/src/api/routes/index.ts index 786b488d81bb..c969eed5972b 100644 --- a/backend/src/api/routes/index.ts +++ b/backend/src/api/routes/index.ts @@ -35,6 +35,7 @@ import { ZodIssue } from "zod"; import { MonkeyValidationError } from "@monkeytype/contracts/schemas/api"; import { authenticateTsRestRequest } from "../../middlewares/auth"; import { rateLimitRequest } from "../../middlewares/rate-limit"; +import { verifyPermissions } from "../../middlewares/permission"; const pathOverride = process.env["API_PATH_OVERRIDE"]; const BASE_ROUTE = pathOverride !== undefined ? `/${pathOverride}` : ""; @@ -112,7 +113,11 @@ function applyTsRestApiRoutes(app: IRouter): void { .status(422) .json({ message, validationErrors } as MonkeyValidationError); }, - globalMiddleware: [authenticateTsRestRequest(), rateLimitRequest()], + globalMiddleware: [ + authenticateTsRestRequest(), + rateLimitRequest(), + verifyPermissions(), + ], }); } diff --git a/backend/src/api/routes/quotes.ts b/backend/src/api/routes/quotes.ts index 84711a730a94..bc48cd4d1bb3 100644 --- a/backend/src/api/routes/quotes.ts +++ b/backend/src/api/routes/quotes.ts @@ -1,23 +1,12 @@ import { quotesContract } from "@monkeytype/contracts/quotes"; import { initServer } from "@ts-rest/express"; import { validate } from "../../middlewares/configuration"; -import { checkUserPermissions } from "../../middlewares/permission"; import * as QuoteController from "../controllers/quote"; import { callController } from "../ts-rest-adapter"; -const checkIfUserIsQuoteMod = checkUserPermissions(["quoteMod"], { - criteria: (user) => { - return ( - user.quoteMod === true || - (typeof user.quoteMod === "string" && user.quoteMod !== "") - ); - }, -}); - const s = initServer(); export default s.router(quotesContract, { get: { - middleware: [checkIfUserIsQuoteMod], handler: async (r) => callController(QuoteController.getQuotes)(r), }, isSubmissionEnabled: { @@ -37,11 +26,9 @@ export default s.router(quotesContract, { handler: async (r) => callController(QuoteController.addQuote)(r), }, approveSubmission: { - middleware: [checkIfUserIsQuoteMod], handler: async (r) => callController(QuoteController.approveQuote)(r), }, rejectSubmission: { - middleware: [checkIfUserIsQuoteMod], handler: async (r) => callController(QuoteController.refuseQuote)(r), }, getRating: { @@ -58,11 +45,6 @@ export default s.router(quotesContract, { }, invalidMessage: "Quote reporting is unavailable.", }), - checkUserPermissions(["canReport"], { - criteria: (user) => { - return user.canReport !== false; - }, - }), ], handler: async (r) => callController(QuoteController.reportQuote)(r), }, diff --git a/backend/src/api/routes/users.ts b/backend/src/api/routes/users.ts index 1537149f32a6..af6ecf82c379 100644 --- a/backend/src/api/routes/users.ts +++ b/backend/src/api/routes/users.ts @@ -1,7 +1,6 @@ import { usersContract } from "@monkeytype/contracts/users"; import { initServer } from "@ts-rest/express"; import { validate } from "../../middlewares/configuration"; -import { checkUserPermissions } from "../../middlewares/permission"; import * as UserController from "../controllers/user"; import { callController } from "../ts-rest-adapter"; @@ -167,11 +166,6 @@ export default s.router(usersContract, { }, invalidMessage: "User reporting is unavailable.", }), - checkUserPermissions(["canReport"], { - criteria: (user) => { - return user.canReport !== false; - }, - }), ], handler: async (r) => callController(UserController.reportUser)(r), }, diff --git a/backend/src/middlewares/permission.ts b/backend/src/middlewares/permission.ts index 8288dee89843..2c22a91536ac 100644 --- a/backend/src/middlewares/permission.ts +++ b/backend/src/middlewares/permission.ts @@ -1,82 +1,199 @@ import _ from "lodash"; import MonkeyError from "../utils/error"; -import type { Response, NextFunction, RequestHandler } from "express"; +import type { Response, NextFunction } from "express"; import { getPartialUser } from "../dal/user"; import { isAdmin } from "../dal/admin-uids"; -import type { ValidationOptions } from "./configuration"; import { TsRestRequestHandler } from "@ts-rest/express"; import { TsRestRequestWithCtx } from "./auth"; -import { RequestAuthenticationOptions } from "@monkeytype/contracts/schemas/api"; +import { + EndpointMetadata, + RequestAuthenticationOptions, + PermissionId, +} from "@monkeytype/contracts/schemas/api"; import { isDevEnvironment } from "../utils/misc"; -/** - * Check if the user is an admin before handling request. - * Note that this middleware must be used after authentication in the middleware stack. - */ -export function checkIfUserIsAdmin< +type RequestPermissionCheck = { + type: "request"; + criteria: ( + req: TsRestRequestWithCtx, + metadata: EndpointMetadata | undefined + ) => Promise; + invalidMessage?: string; +}; + +type UserPermissionCheck = { + type: "user"; + fields: (keyof MonkeyTypes.DBUser)[]; + criteria: (user: MonkeyTypes.DBUser) => boolean; + invalidMessage?: string; +}; + +type PermissionCheck = UserPermissionCheck | RequestPermissionCheck; + +function buildUserPermission( + fields: K[], + criteria: (user: Pick) => boolean, + invalidMessage?: string +): UserPermissionCheck { + return { + type: "user", + fields, + criteria, + invalidMessage: invalidMessage, + }; +} + +const permissionChecks: Record = { + admin: { + type: "request", + criteria: async (req, metadata) => + await checkIfUserIsAdmin( + req.ctx.decodedToken, + metadata?.authenticationOptions + ), + }, + quoteMod: buildUserPermission( + ["quoteMod"], + (user) => + user.quoteMod === true || + (typeof user.quoteMod === "string" && user.quoteMod !== "") + ), + canReport: buildUserPermission( + ["canReport"], + (user) => user.canReport !== false + ), + canManageApeKeys: buildUserPermission( + ["canManageApeKeys"], + (user) => user.canManageApeKeys ?? true, + "You have lost access to ape keys, please contact support" + ), +}; + +export function verifyPermissions< T extends AppRouter | AppRoute >(): TsRestRequestHandler { return async ( req: TsRestRequestWithCtx, _res: Response, next: NextFunction - ) => { - try { - const options: RequestAuthenticationOptions = - req.tsRestRoute["metadata"]?.["authenticationOptions"] ?? {}; + ): Promise => { + const metadata = req.tsRestRoute["metadata"] as + | EndpointMetadata + | undefined; + const requiredPermissionIds = getRequiredPermissionIds(metadata); + if ( + requiredPermissionIds === undefined || + requiredPermissionIds.length === 0 + ) { + next(); + return; + } - if (options.isPublicOnDev && isDevEnvironment()) { - next(); + const checks = requiredPermissionIds.map((id) => permissionChecks[id]); + + if (checks.some((it) => it === undefined)) { + next(new MonkeyError(500, "Unknown permission id.")); + return; + } + + //handle request checks + const requestChecks = checks.filter((it) => it.type === "request"); + for (const check of requestChecks) { + if (!(await check.criteria(req, metadata))) { + next( + new MonkeyError( + 403, + check.invalidMessage ?? "You don't have permission to do this." + ) + ); return; } + } - const { uid } = req.ctx.decodedToken; - const admin = await isAdmin(uid); + //handle user checks + const userChecks = checks.filter((it) => it.type === "user"); + const checkResult = await checkUserPermissions( + req.ctx.decodedToken, + userChecks + ); - if (!admin) { - throw new MonkeyError(403, "You don't have permission to do this."); - } - } catch (error) { - next(error); + if (!checkResult.passed) { + next( + new MonkeyError( + 403, + checkResult.invalidMessage ?? "You don't have permission to do this." + ) + ); + return; } + //all checks passed next(); + return; }; } -/** - * Check user permissions before handling request. - * Note that this middleware must be used after authentication in the middleware stack. - */ -export function checkUserPermissions( - fields: K[], - options: ValidationOptions> -): RequestHandler { - const { criteria, invalidMessage = "You don't have permission to do this." } = - options; +function getRequiredPermissionIds( + metadata: EndpointMetadata | undefined +): PermissionId[] | undefined { + if (metadata === undefined || metadata.requirePermission === undefined) + return undefined; - return async ( - req: MonkeyTypes.Request, - _res: Response, - next: NextFunction - ) => { - try { - const { uid } = req.ctx.decodedToken; - - const userData = await getPartialUser( - uid, - "check user permissions", - fields - ); - const hasPermission = criteria(userData); + if (Array.isArray(metadata.requirePermission)) + return metadata.requirePermission; + return [metadata.requirePermission]; +} - if (!hasPermission) { - throw new MonkeyError(403, invalidMessage); - } - } catch (error) { - next(error); +async function checkIfUserIsAdmin( + decodedToken: MonkeyTypes.DecodedToken | undefined, + options: RequestAuthenticationOptions | undefined +): Promise { + if (decodedToken === undefined) return false; + if (options?.isPublicOnDev && isDevEnvironment()) return true; + + return await isAdmin(decodedToken.uid); +} + +type CheckResult = + | { + passed: true; } + | { + passed: false; + invalidMessage?: string; + }; - next(); +async function checkUserPermissions( + decodedToken: MonkeyTypes.DecodedToken | undefined, + checks: UserPermissionCheck[] +): Promise { + if (checks === undefined || checks.length === 0) { + return { + passed: true, + }; + } + if (decodedToken === undefined) { + return { + passed: false, + invalidMessage: "Failed to check permissions, authentication required.", + }; + } + + const user = (await getPartialUser( + decodedToken.uid, + "check user permissions", + checks.flatMap((it) => it.fields) + )) as MonkeyTypes.DBUser; + + for (const check of checks) { + if (!check.criteria(user)) + return { + passed: false, + invalidMessage: check.invalidMessage, + }; + } + + return { + passed: true, }; } diff --git a/frontend/package.json b/frontend/package.json index 40a562e6e700..e40ed64a2f1b 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -70,7 +70,7 @@ "dependencies": { "@date-fns/utc": "1.2.0", "@monkeytype/contracts": "workspace:*", - "@ts-rest/core": "3.49.3", + "@ts-rest/core": "3.51.0", "axios": "1.7.4", "canvas-confetti": "1.5.1", "chart.js": "3.7.1", diff --git a/packages/contracts/package.json b/packages/contracts/package.json index 8f97eb285888..d5b181ea6ecd 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -10,7 +10,7 @@ "lint": "eslint \"./**/*.ts\"" }, "dependencies": { - "@ts-rest/core": "3.49.3", + "@ts-rest/core": "3.51.0", "zod": "3.23.8" }, "devDependencies": { diff --git a/packages/contracts/src/admin.ts b/packages/contracts/src/admin.ts index 230ed3421295..093d6cb633c1 100644 --- a/packages/contracts/src/admin.ts +++ b/packages/contracts/src/admin.ts @@ -111,6 +111,7 @@ export const adminContract = c.router( openApiTags: "admin", authenticationOptions: { noCache: true }, rateLimit: "adminLimit", + requirePermission: "admin", }), commonResponses: CommonResponses, diff --git a/packages/contracts/src/ape-keys.ts b/packages/contracts/src/ape-keys.ts index 860b675a3a16..f083eea1e605 100644 --- a/packages/contracts/src/ape-keys.ts +++ b/packages/contracts/src/ape-keys.ts @@ -101,6 +101,7 @@ export const apeKeysContract = c.router( strictStatusCodes: true, metadata: meta({ openApiTags: "ape-keys", + requirePermission: "canManageApeKeys", }), commonResponses: CommonResponses, diff --git a/packages/contracts/src/configuration.ts b/packages/contracts/src/configuration.ts index 0f03d67f30d6..fa85788ec732 100644 --- a/packages/contracts/src/configuration.ts +++ b/packages/contracts/src/configuration.ts @@ -67,6 +67,7 @@ export const configurationContract = c.router( isPublicOnDev: true, }, rateLimit: "adminLimit", + requirePermission: "admin", }), }, getSchema: { @@ -83,6 +84,7 @@ export const configurationContract = c.router( noCache: true, }, rateLimit: "adminLimit", + requirePermission: "admin", }), }, }, diff --git a/packages/contracts/src/quotes.ts b/packages/contracts/src/quotes.ts index dd964dae6904..36a4efdf2624 100644 --- a/packages/contracts/src/quotes.ts +++ b/packages/contracts/src/quotes.ts @@ -98,6 +98,7 @@ export const quotesContract = c.router( }, metadata: meta({ rateLimit: "newQuotesGet", + requirePermission: "quoteMod", }), }, isSubmissionEnabled: { @@ -137,6 +138,7 @@ export const quotesContract = c.router( }, metadata: meta({ rateLimit: "newQuotesAction", + requirePermission: "quoteMod", }), }, rejectSubmission: { @@ -150,6 +152,7 @@ export const quotesContract = c.router( }, metadata: meta({ rateLimit: "newQuotesAction", + requirePermission: "quoteMod", }), }, getRating: { @@ -189,6 +192,7 @@ export const quotesContract = c.router( }, metadata: meta({ rateLimit: "quoteReportSubmit", + requirePermission: "canReport", }), }, }, diff --git a/packages/contracts/src/schemas/api.ts b/packages/contracts/src/schemas/api.ts index df18aa2cca03..d0ce83c124e4 100644 --- a/packages/contracts/src/schemas/api.ts +++ b/packages/contracts/src/schemas/api.ts @@ -15,6 +15,12 @@ export type OpenApiTag = | "users" | "quotes"; +export type PermissionId = + | "quoteMod" + | "canReport" + | "canManageApeKeys" + | "admin"; + export type EndpointMetadata = { /** Authentication options, by default a bearer token is required. */ authenticationOptions?: RequestAuthenticationOptions; @@ -25,6 +31,9 @@ export type EndpointMetadata = { * Only specifying RateLimiterId will use a default limiter with 30 requests/minute for ApeKey requests. */ rateLimit?: RateLimiterId | RateLimitIds; + + /** Role/Rples needed to access the endpoint*/ + requirePermission?: PermissionId | PermissionId[]; }; /** diff --git a/packages/contracts/src/users.ts b/packages/contracts/src/users.ts index f391beddd90b..fd1641fddb18 100644 --- a/packages/contracts/src/users.ts +++ b/packages/contracts/src/users.ts @@ -803,6 +803,7 @@ export const usersContract = c.router( }, metadata: meta({ rateLimit: "quoteReportSubmit", + requirePermission: "canReport", }), }, verificationEmail: { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 730ff76facb2..cdc2c1168fad 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -54,14 +54,14 @@ importers: specifier: workspace:* version: link:../packages/contracts '@ts-rest/core': - specifier: 3.49.3 - version: 3.49.3(zod@3.23.8) + specifier: 3.51.0 + version: 3.51.0(@types/node@20.14.11)(zod@3.23.8) '@ts-rest/express': - specifier: 3.49.3 - version: 3.49.3(@ts-rest/core@3.49.3(zod@3.23.8))(express@4.19.2)(zod@3.23.8) + specifier: 3.51.0 + version: 3.51.0(@ts-rest/core@3.51.0(@types/node@20.14.11)(zod@3.23.8))(express@4.19.2)(zod@3.23.8) '@ts-rest/open-api': - specifier: 3.49.3 - version: 3.49.3(@ts-rest/core@3.49.3(zod@3.23.8))(zod@3.23.8) + specifier: 3.51.0 + version: 3.51.0(@ts-rest/core@3.51.0(@types/node@20.14.11)(zod@3.23.8))(zod@3.23.8) bcrypt: specifier: 5.1.1 version: 5.1.1(encoding@0.1.13) @@ -237,6 +237,9 @@ importers: ioredis-mock: specifier: 7.4.0 version: 7.4.0(ioredis@4.28.5) + openapi3-ts: + specifier: 2.0.2 + version: 2.0.2 readline-sync: specifier: 1.4.10 version: 1.4.10 @@ -265,8 +268,8 @@ importers: specifier: workspace:* version: link:../packages/contracts '@ts-rest/core': - specifier: 3.49.3 - version: 3.49.3(zod@3.23.8) + specifier: 3.51.0 + version: 3.51.0(@types/node@20.14.11)(zod@3.23.8) axios: specifier: 1.7.4 version: 1.7.4(debug@4.3.6) @@ -452,8 +455,8 @@ importers: packages/contracts: dependencies: '@ts-rest/core': - specifier: 3.49.3 - version: 3.49.3(zod@3.23.8) + specifier: 3.51.0 + version: 3.51.0(@types/node@20.14.11)(zod@3.23.8) zod: specifier: 3.23.8 version: 3.23.8 @@ -2490,28 +2493,31 @@ packages: resolution: {integrity: sha512-EZ+XlSwjdLtscoBOnA/Ba6QBrmoxAR73tJFjnWxaJQsZxWBQv6bLUrDgZUdXkXRAOSkRHn0uXY6Wq/3SsV2WtQ==} engines: {node: '>=18'} - '@ts-rest/core@3.49.3': - resolution: {integrity: sha512-h/4aSH7SGsQfBZ5LcF2k8+TVtFSITYG4qI91tdf0YMddPsSZJho2OV9jhvycNEt+sosPsw/FDV2QFKBAUEr22w==} + '@ts-rest/core@3.51.0': + resolution: {integrity: sha512-v6lnWEcpZj1UgN9wb84XQ+EORP1QEtncFumoXMJjno5ZUV6vdjKze3MYcQN0C6vjBpIJPQEaI/gab2jr4/0KzQ==} peerDependencies: + '@types/node': ^18.18.7 || >=20.8.4 zod: ^3.22.3 peerDependenciesMeta: + '@types/node': + optional: true zod: optional: true - '@ts-rest/express@3.49.3': - resolution: {integrity: sha512-+ybB1yvzyPclxlSLMoZGuEL26DqxH/bLbWShWC2C28kRq0k768xYe8EvDD9VRrZMJqmnO+sndXdcGq40hvhcKA==} + '@ts-rest/express@3.51.0': + resolution: {integrity: sha512-osOo040EHoCMNMFBbEwZ6XheeEhG/zJGF4k3FOYUtaYSdxxYp5XkP8jQoV6I+Lu+O5jopnOQWoCpmNrrraIO8g==} peerDependencies: - '@ts-rest/core': ~3.49.0 + '@ts-rest/core': ~3.51.0 express: ^4.0.0 zod: ^3.22.3 peerDependenciesMeta: zod: optional: true - '@ts-rest/open-api@3.49.3': - resolution: {integrity: sha512-5N71UP/5KtjOyagc076arPwGkemDBlqGv7c42AbV2ca4dj1dlveis41VIpHsfWOdsFS548X+C9b0td7YVCdpqA==} + '@ts-rest/open-api@3.51.0': + resolution: {integrity: sha512-fvpvRr6HIbAMNZR//QQQi75z5qTxMEBMRtmbaBXVi5e1WVVwOK7P6YBaGWTQp6DXSvsZVULX5VZXmsDd1Z1dew==} peerDependencies: - '@ts-rest/core': ~3.49.0 + '@ts-rest/core': ~3.51.0 zod: ^3.22.3 '@tsconfig/node10@1.0.11': @@ -11696,21 +11702,22 @@ snapshots: '@ts-graphviz/ast': 2.0.3 '@ts-graphviz/common': 2.1.2 - '@ts-rest/core@3.49.3(zod@3.23.8)': + '@ts-rest/core@3.51.0(@types/node@20.14.11)(zod@3.23.8)': optionalDependencies: + '@types/node': 20.14.11 zod: 3.23.8 - '@ts-rest/express@3.49.3(@ts-rest/core@3.49.3(zod@3.23.8))(express@4.19.2)(zod@3.23.8)': + '@ts-rest/express@3.51.0(@ts-rest/core@3.51.0(@types/node@20.14.11)(zod@3.23.8))(express@4.19.2)(zod@3.23.8)': dependencies: - '@ts-rest/core': 3.49.3(zod@3.23.8) + '@ts-rest/core': 3.51.0(@types/node@20.14.11)(zod@3.23.8) express: 4.19.2 optionalDependencies: zod: 3.23.8 - '@ts-rest/open-api@3.49.3(@ts-rest/core@3.49.3(zod@3.23.8))(zod@3.23.8)': + '@ts-rest/open-api@3.51.0(@ts-rest/core@3.51.0(@types/node@20.14.11)(zod@3.23.8))(zod@3.23.8)': dependencies: '@anatine/zod-openapi': 1.14.2(openapi3-ts@2.0.2)(zod@3.23.8) - '@ts-rest/core': 3.49.3(zod@3.23.8) + '@ts-rest/core': 3.51.0(@types/node@20.14.11)(zod@3.23.8) openapi3-ts: 2.0.2 zod: 3.23.8