From 4f75a00cb3a5325c99384bdaee7c3a03fc15371c Mon Sep 17 00:00:00 2001 From: Christian Fehmer Date: Wed, 11 Sep 2024 14:16:34 +0200 Subject: [PATCH] impr: use ts-rest for webhook endpoints (@fehmer, @miodec) (#5871) !nuf --- .../__tests__/api/controllers/public.spec.ts | 5 +- .../api/controllers/webhooks.spec.ts | 81 +++++++++++ backend/__tests__/middlewares/auth.spec.ts | 127 +++++++++++++++++- backend/scripts/openapi.ts | 6 + backend/src/api/controllers/webhooks.ts | 20 ++- backend/src/api/routes/index.ts | 3 +- backend/src/api/routes/webhooks.ts | 25 ++-- backend/src/middlewares/auth.ts | 87 ++++++------ backend/src/types/types.d.ts | 2 +- backend/src/utils/prometheus.ts | 4 +- packages/contracts/src/index.ts | 2 + packages/contracts/src/schemas/api.ts | 5 +- packages/contracts/src/webhooks.ts | 60 +++++++++ 13 files changed, 357 insertions(+), 70 deletions(-) create mode 100644 backend/__tests__/api/controllers/webhooks.spec.ts create mode 100644 packages/contracts/src/webhooks.ts diff --git a/backend/__tests__/api/controllers/public.spec.ts b/backend/__tests__/api/controllers/public.spec.ts index e5e348d72..3a77212a5 100644 --- a/backend/__tests__/api/controllers/public.spec.ts +++ b/backend/__tests__/api/controllers/public.spec.ts @@ -18,9 +18,8 @@ describe("PublicController", () => { //WHEN const { body } = await mockApp .get("/public/speedHistogram") - .query({ language: "english", mode: "time", mode2: "60" }); - //.expect(200); - console.log(body); + .query({ language: "english", mode: "time", mode2: "60" }) + .expect(200); //THEN expect(body).toEqual({ diff --git a/backend/__tests__/api/controllers/webhooks.spec.ts b/backend/__tests__/api/controllers/webhooks.spec.ts new file mode 100644 index 000000000..be54c47c1 --- /dev/null +++ b/backend/__tests__/api/controllers/webhooks.spec.ts @@ -0,0 +1,81 @@ +import GeorgeQueue from "../../../src/queues/george-queue"; +import crypto from "crypto"; +import request from "supertest"; +import app from "../../../src/app"; + +const mockApp = request(app); + +describe("WebhooksController", () => { + describe("githubRelease", () => { + const georgeSendReleaseAnnouncementMock = vi.spyOn( + GeorgeQueue, + "sendReleaseAnnouncement" + ); + const timingSafeEqualMock = vi.spyOn(crypto, "timingSafeEqual"); + + beforeEach(() => { + vi.stubEnv("GITHUB_WEBHOOK_SECRET", "GITHUB_WEBHOOK_SECRET"); + + georgeSendReleaseAnnouncementMock.mockReset(); + timingSafeEqualMock.mockReset().mockReturnValue(true); + }); + + it("should announce release", async () => { + //WHEN + const { body } = await mockApp + .post("/webhooks/githubRelease") + .set("x-hub-signature-256", "the-signature") + .send({ action: "published", release: { id: 1 } }) + .expect(200); + + //THEN + expect(body).toEqual({ + message: "Added release announcement task to queue", + data: null, + }); + + expect(georgeSendReleaseAnnouncementMock).toHaveBeenCalledWith("1"); + expect(timingSafeEqualMock).toHaveBeenCalledWith( + Buffer.from( + "sha256=ff0f3080539e9df19153f6b5b5780f66e558d61038e6cf5ecf4efdc7266a7751" + ), + Buffer.from("the-signature") + ); + }); + it("should ignore non-published actions", async () => { + //WHEN + const { body } = await mockApp + .post("/webhooks/githubRelease") + .set("x-hub-signature-256", "the-signature") + .send({ action: "created" }) + .expect(200); + + //THEN + expect(body.message).toEqual("No action taken"); + expect(georgeSendReleaseAnnouncementMock).not.toHaveBeenCalled(); + }); + it("should ignore additional properties", async () => { + //WHEN + await mockApp + .post("/webhooks/githubRelease") + .set("x-hub-signature-256", "the-signature") + .send({ + action: "published", + extra: "value", + release: { id: 1, extra2: "value" }, + }) + .expect(200); + }); + it("should fail with missing releaseId", async () => { + //WHEN + const { body } = await mockApp + .post("/webhooks/githubRelease") + .set("x-hub-signature-256", "the-signature") + .send({ action: "published" }) + .expect(422); + + //THEN + expect(body.message).toEqual('Missing property "release.id".'); + }); + }); +}); diff --git a/backend/__tests__/middlewares/auth.spec.ts b/backend/__tests__/middlewares/auth.spec.ts index 49af130f4..b59efef05 100644 --- a/backend/__tests__/middlewares/auth.spec.ts +++ b/backend/__tests__/middlewares/auth.spec.ts @@ -8,6 +8,7 @@ import { ObjectId } from "mongodb"; import { hashSync } from "bcrypt"; import MonkeyError from "../../src/utils/error"; import * as Misc from "../../src/utils/misc"; +import crypto from "crypto"; import { EndpointMetadata, RequestAuthenticationOptions, @@ -259,12 +260,14 @@ describe("middlewares/auth", () => { describe("authenticateTsRestRequest", () => { const prometheusRecordAuthTimeMock = vi.spyOn(Prometheus, "recordAuthTime"); const prometheusIncrementAuthMock = vi.spyOn(Prometheus, "incrementAuth"); + const timingSafeEqualMock = vi.spyOn(crypto, "timingSafeEqual"); - beforeEach(() => + beforeEach(() => { + timingSafeEqualMock.mockReset().mockReturnValue(true); [prometheusIncrementAuthMock, prometheusRecordAuthTimeMock].forEach( (it) => it.mockReset() - ) - ); + ); + }); it("should fail if token is not fresh", async () => { //GIVEN @@ -604,6 +607,124 @@ describe("middlewares/auth", () => { expect(prometheusIncrementAuthMock).toHaveBeenCalledWith("ApeKey"); expect(prometheusRecordAuthTimeMock).toHaveBeenCalledOnce(); }); + it("should allow githubwebhook with header", async () => { + vi.stubEnv("GITHUB_WEBHOOK_SECRET", "GITHUB_WEBHOOK_SECRET"); + //WHEN + const result = await authenticate( + { + headers: { "x-hub-signature-256": "the-signature" }, + body: { action: "published", release: { id: 1 } }, + }, + { isGithubWebhook: true } + ); + + //THEN + const decodedToken = result.decodedToken; + expect(decodedToken?.type).toBe("GithubWebhook"); + expect(decodedToken?.email).toBe(""); + expect(decodedToken?.uid).toBe(""); + expect(nextFunction).toHaveBeenCalledTimes(1); + + expect(prometheusIncrementAuthMock).toHaveBeenCalledWith("GithubWebhook"); + expect(prometheusRecordAuthTimeMock).toHaveBeenCalledOnce(); + expect(timingSafeEqualMock).toHaveBeenCalledWith( + Buffer.from( + "sha256=ff0f3080539e9df19153f6b5b5780f66e558d61038e6cf5ecf4efdc7266a7751" + ), + Buffer.from("the-signature") + ); + }); + it("should fail githubwebhook with mismatched signature", async () => { + vi.stubEnv("GITHUB_WEBHOOK_SECRET", "GITHUB_WEBHOOK_SECRET"); + timingSafeEqualMock.mockReturnValue(false); + + await expect(() => + authenticate( + { + headers: { "x-hub-signature-256": "the-signature" }, + body: { action: "published", release: { id: 1 } }, + }, + { isGithubWebhook: true } + ) + ).rejects.toThrowError("Github webhook signature invalid"); + + //THEH + expect(prometheusIncrementAuthMock).not.toHaveBeenCalled(); + expect(prometheusRecordAuthTimeMock).toHaveBeenCalledWith( + "None", + "failure", + expect.anything(), + expect.anything() + ); + }); + it("should fail without header when endpoint is using githubwebhook", async () => { + vi.stubEnv("GITHUB_WEBHOOK_SECRET", "GITHUB_WEBHOOK_SECRET"); + await expect(() => + authenticate( + { + headers: {}, + body: { action: "published", release: { id: 1 } }, + }, + { isGithubWebhook: true } + ) + ).rejects.toThrowError("Missing Github signature header"); + + //THEH + expect(prometheusIncrementAuthMock).not.toHaveBeenCalled(); + expect(prometheusRecordAuthTimeMock).toHaveBeenCalledWith( + "None", + "failure", + expect.anything(), + expect.anything() + ); + }); + it("should fail with missing GITHUB_WEBHOOK_SECRET when endpoint is using githubwebhook", async () => { + vi.stubEnv("GITHUB_WEBHOOK_SECRET", ""); + await expect(() => + authenticate( + { + headers: { "x-hub-signature-256": "the-signature" }, + body: { action: "published", release: { id: 1 } }, + }, + { isGithubWebhook: true } + ) + ).rejects.toThrowError("Missing Github Webhook Secret"); + + //THEH + expect(prometheusIncrementAuthMock).not.toHaveBeenCalled(); + expect(prometheusRecordAuthTimeMock).toHaveBeenCalledWith( + "None", + "failure", + expect.anything(), + expect.anything() + ); + }); + it("should throw 500 if something went wrong when validating the signature when endpoint is using githubwebhook", async () => { + vi.stubEnv("GITHUB_WEBHOOK_SECRET", "GITHUB_WEBHOOK_SECRET"); + timingSafeEqualMock.mockImplementation(() => { + throw new Error("could not validate"); + }); + await expect(() => + authenticate( + { + headers: { "x-hub-signature-256": "the-signature" }, + body: { action: "published", release: { id: 1 } }, + }, + { isGithubWebhook: true } + ) + ).rejects.toThrowError( + "Failed to authenticate Github webhook: could not validate" + ); + + //THEH + expect(prometheusIncrementAuthMock).not.toHaveBeenCalled(); + expect(prometheusRecordAuthTimeMock).toHaveBeenCalledWith( + "None", + "failure", + expect.anything(), + expect.anything() + ); + }); }); }); diff --git a/backend/scripts/openapi.ts b/backend/scripts/openapi.ts index 5798b28c9..744420b20 100644 --- a/backend/scripts/openapi.ts +++ b/backend/scripts/openapi.ts @@ -134,6 +134,12 @@ export function getOpenApi(): OpenAPIObject { "x-displayName": "Development", "x-public": "no", }, + { + name: "webhooks", + description: "Endpoints for incoming webhooks.", + "x-displayName": "Webhooks", + "x-public": "yes", + }, ], }, diff --git a/backend/src/api/controllers/webhooks.ts b/backend/src/api/controllers/webhooks.ts index 3bef5bbca..f82c58f82 100644 --- a/backend/src/api/controllers/webhooks.ts +++ b/backend/src/api/controllers/webhooks.ts @@ -1,15 +1,23 @@ -import { MonkeyResponse } from "../../utils/monkey-response"; +import { PostGithubReleaseRequest } from "@monkeytype/contracts/webhooks"; import GeorgeQueue from "../../queues/george-queue"; +import { MonkeyResponse2 } from "../../utils/monkey-response"; +import MonkeyError from "../../utils/error"; export async function githubRelease( - req: MonkeyTypes.Request -): Promise { + req: MonkeyTypes.Request2 +): Promise { const action = req.body.action; if (action === "published") { - const releaseId = req.body.release.id; + const releaseId = req.body.release?.id; + if (releaseId === undefined) + throw new MonkeyError(422, 'Missing property "release.id".'); + await GeorgeQueue.sendReleaseAnnouncement(releaseId); - return new MonkeyResponse("Added release announcement task to queue"); + return new MonkeyResponse2( + "Added release announcement task to queue", + null + ); } - return new MonkeyResponse("No action taken"); + return new MonkeyResponse2("No action taken", null); } diff --git a/backend/src/api/routes/index.ts b/backend/src/api/routes/index.ts index e415e6de7..3f73997d1 100644 --- a/backend/src/api/routes/index.ts +++ b/backend/src/api/routes/index.ts @@ -43,7 +43,6 @@ const BASE_ROUTE = pathOverride !== undefined ? `/${pathOverride}` : ""; const APP_START_TIME = Date.now(); const API_ROUTE_MAP = { - "/webhooks": webhooks, "/docs": docs, }; @@ -61,6 +60,7 @@ const router = s.router(contract, { dev, users, quotes, + webhooks, }); export function addApiRoutes(app: Application): void { @@ -154,7 +154,6 @@ function applyDevApiRoutes(app: Application): void { function applyApiRoutes(app: Application): void { addSwaggerMiddlewares(app); - //TODO move to globalMiddleware when all endpoints use tsrest app.use( (req: MonkeyTypes.Request, res: Response, next: NextFunction): void => { if (req.path.startsWith("/configuration")) { diff --git a/backend/src/api/routes/webhooks.ts b/backend/src/api/routes/webhooks.ts index d0e774d09..5946d75d0 100644 --- a/backend/src/api/routes/webhooks.ts +++ b/backend/src/api/routes/webhooks.ts @@ -1,17 +1,12 @@ // import joi from "joi"; -import { Router } from "express"; -import { authenticateGithubWebhook } from "../../middlewares/auth"; -import { asyncHandler } from "../../middlewares/utility"; -import { webhookLimit } from "../../middlewares/rate-limit"; -import { githubRelease } from "../controllers/webhooks"; +import { webhooksContract } from "@monkeytype/contracts/webhooks"; +import { initServer } from "@ts-rest/express"; +import * as WebhooksController from "../controllers/webhooks"; +import { callController } from "../ts-rest-adapter"; -const router = Router(); - -router.post( - "/githubRelease", - webhookLimit, - authenticateGithubWebhook(), - asyncHandler(githubRelease) -); - -export default router; +const s = initServer(); +export default s.router(webhooksContract, { + postGithubRelease: { + handler: async (r) => callController(WebhooksController.githubRelease)(r), + }, +}); diff --git a/backend/src/middlewares/auth.ts b/backend/src/middlewares/auth.ts index 74727f0aa..a6e100c90 100644 --- a/backend/src/middlewares/auth.ts +++ b/backend/src/middlewares/auth.ts @@ -18,6 +18,7 @@ import { RequestAuthenticationOptions } from "@monkeytype/contracts/schemas/api" import { Configuration } from "@monkeytype/contracts/schemas/configuration"; const DEFAULT_OPTIONS: RequestAuthenticationOptions = { + isGithubWebhook: false, isPublic: false, acceptApeKeys: false, requireFreshToken: false, @@ -78,10 +79,15 @@ async function _authenticateRequestInternal( const isPublic = options.isPublic || (options.isPublicOnDev && isDevEnvironment()); - const { authorization: authHeader } = req.headers; + const { + authorization: authHeader, + "x-hub-signature-256": githubWebhookHeader, + } = req.headers; try { - if (authHeader !== undefined && authHeader !== "") { + if (options.isGithubWebhook) { + token = authenticateGithubWebhook(req, githubWebhookHeader); + } else if (authHeader !== undefined && authHeader !== "") { token = await authenticateWithAuthHeader( authHeader, req.ctx.configuration, @@ -322,44 +328,49 @@ async function authenticateWithUid( }; } -export function authenticateGithubWebhook(): Handler { - return async ( - req: MonkeyTypes.Request, - _res: Response, - next: NextFunction - ): Promise => { - //authorize github webhook - const { "x-hub-signature-256": authHeader } = req.headers; - +export function authenticateGithubWebhook( + req: MonkeyTypes.Request, + authHeader: string | string[] | undefined +): MonkeyTypes.DecodedToken { + try { const webhookSecret = process.env["GITHUB_WEBHOOK_SECRET"]; - try { - if (webhookSecret === undefined || webhookSecret === "") { - throw new MonkeyError(500, "Missing Github Webhook Secret"); - } else if ( - authHeader === undefined || - authHeader === "" || - authHeader.length === 0 - ) { - throw new MonkeyError(401, "Missing Github signature header"); - } else { - const signature = crypto - .createHmac("sha256", webhookSecret) - .update(JSON.stringify(req.body)) - .digest("hex"); - const trusted = Buffer.from(`sha256=${signature}`, "ascii"); - const untrusted = Buffer.from(authHeader as string, "ascii"); - const isSignatureValid = crypto.timingSafeEqual(trusted, untrusted); - - if (!isSignatureValid) { - throw new MonkeyError(401, "Github webhook signature invalid"); - } - } - } catch (e) { - next(e); - return; + if (webhookSecret === undefined || webhookSecret === "") { + throw new MonkeyError(500, "Missing Github Webhook Secret"); } - next(); - }; + if ( + Array.isArray(authHeader) || + authHeader === undefined || + authHeader === "" + ) { + throw new MonkeyError(401, "Missing Github signature header"); + } + + const signature = crypto + .createHmac("sha256", webhookSecret) + .update(JSON.stringify(req.body)) + .digest("hex"); + const trusted = Buffer.from(`sha256=${signature}`, "ascii"); + const untrusted = Buffer.from(authHeader, "ascii"); + const isSignatureValid = crypto.timingSafeEqual(trusted, untrusted); + + if (!isSignatureValid) { + throw new MonkeyError(401, "Github webhook signature invalid"); + } + + return { + type: "GithubWebhook", + uid: "", + email: "", + }; + } catch (error) { + if (error instanceof MonkeyError) { + throw error; + } + throw new MonkeyError( + 500, + "Failed to authenticate Github webhook: " + (error as Error).message + ); + } } diff --git a/backend/src/types/types.d.ts b/backend/src/types/types.d.ts index dff561c7d..0cc14751c 100644 --- a/backend/src/types/types.d.ts +++ b/backend/src/types/types.d.ts @@ -8,7 +8,7 @@ type AppRoute = import("@ts-rest/core").AppRoute; type AppRouter = import("@ts-rest/core").AppRouter; declare namespace MonkeyTypes { export type DecodedToken = { - type: "Bearer" | "ApeKey" | "None"; + type: "Bearer" | "ApeKey" | "None" | "GithubWebhook"; uid: string; email: string; }; diff --git a/backend/src/utils/prometheus.ts b/backend/src/utils/prometheus.ts index 1aaa39f4b..5831acd8a 100644 --- a/backend/src/utils/prometheus.ts +++ b/backend/src/utils/prometheus.ts @@ -74,7 +74,9 @@ const leaderboardUpdate = new Gauge({ labelNames: ["language", "mode", "mode2", "step"], }); -export function incrementAuth(type: "Bearer" | "ApeKey" | "None"): void { +export function incrementAuth( + type: "Bearer" | "ApeKey" | "None" | "GithubWebhook" +): void { auth.inc({ type }); } diff --git a/packages/contracts/src/index.ts b/packages/contracts/src/index.ts index dc84d01f8..75aa48663 100644 --- a/packages/contracts/src/index.ts +++ b/packages/contracts/src/index.ts @@ -11,6 +11,7 @@ import { configurationContract } from "./configuration"; import { devContract } from "./dev"; import { usersContract } from "./users"; import { quotesContract } from "./quotes"; +import { webhooksContract } from "./webhooks"; const c = initContract(); @@ -27,4 +28,5 @@ export const contract = c.router({ dev: devContract, users: usersContract, quotes: quotesContract, + webhooks: webhooksContract, }); diff --git a/packages/contracts/src/schemas/api.ts b/packages/contracts/src/schemas/api.ts index d247783db..7697b3e54 100644 --- a/packages/contracts/src/schemas/api.ts +++ b/packages/contracts/src/schemas/api.ts @@ -14,7 +14,8 @@ export type OpenApiTag = | "configuration" | "development" | "users" - | "quotes"; + | "quotes" + | "webhooks"; export type PermissionId = | "quoteMod" @@ -60,6 +61,8 @@ export type RequestAuthenticationOptions = { noCache?: boolean; /** Allow unauthenticated requests on dev */ isPublicOnDev?: boolean; + /** Endpoint is a webhook only to be called by Github */ + isGithubWebhook?: boolean; }; export const MonkeyResponseSchema = z.object({ diff --git a/packages/contracts/src/webhooks.ts b/packages/contracts/src/webhooks.ts new file mode 100644 index 000000000..2fcd11793 --- /dev/null +++ b/packages/contracts/src/webhooks.ts @@ -0,0 +1,60 @@ +import { initContract } from "@ts-rest/core"; +import { z } from "zod"; +import { PSASchema } from "./schemas/psas"; + +import { + CommonResponses, + meta, + MonkeyResponseSchema, + responseWithData, +} from "./schemas/api"; + +/** + *Schema: https://docs.github.com/en/webhooks/webhook-events-and-payloads?actionType=published#release + We only specify the values we read and don't validate any other values. + */ +export const PostGithubReleaseRequestSchema = z.object({ + action: z.literal("published").or(z.string()), + release: z + .object({ + id: z.string().or(z.number().transform(String)), //we use string, github defines this as a number. + }) + .optional(), +}); +export type PostGithubReleaseRequest = z.infer< + typeof PostGithubReleaseRequestSchema +>; + +export const GetPsaResponseSchema = responseWithData(z.array(PSASchema)); +export type GetPsaResponse = z.infer; + +const c = initContract(); +export const webhooksContract = c.router( + { + postGithubRelease: { + summary: "Github release", + description: "Announce github release.", + method: "POST", + path: "/githubRelease", + body: PostGithubReleaseRequestSchema, //don't use strict + headers: z.object({ + "x-hub-signature-256": z.string(), + }), + responses: { + 200: MonkeyResponseSchema, + }, + }, + }, + { + pathPrefix: "/webhooks", + strictStatusCodes: true, + metadata: meta({ + openApiTags: "webhooks", + authenticationOptions: { + isGithubWebhook: true, + }, + rateLimit: "webhookLimit", + }), + commonResponses: CommonResponses, + } +);