Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

impr: use ts-rest for webhook endpoints (@fehmer) #5871

Merged
merged 12 commits into from
Sep 11, 2024
5 changes: 2 additions & 3 deletions backend/__tests__/api/controllers/public.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
81 changes: 81 additions & 0 deletions backend/__tests__/api/controllers/webhooks.spec.ts
Original file line number Diff line number Diff line change
@@ -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".');
});
});
});
127 changes: 124 additions & 3 deletions backend/__tests__/middlewares/auth.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
);
});
});
});

Expand Down
6 changes: 6 additions & 0 deletions backend/scripts/openapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
],
},

Expand Down
20 changes: 14 additions & 6 deletions backend/src/api/controllers/webhooks.ts
Original file line number Diff line number Diff line change
@@ -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<MonkeyResponse> {
req: MonkeyTypes.Request2<undefined, PostGithubReleaseRequest>
): Promise<MonkeyResponse2> {
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);
}
3 changes: 1 addition & 2 deletions backend/src/api/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ const BASE_ROUTE = pathOverride !== undefined ? `/${pathOverride}` : "";
const APP_START_TIME = Date.now();

const API_ROUTE_MAP = {
"/webhooks": webhooks,
"/docs": docs,
};

Expand All @@ -60,6 +59,7 @@ const router = s.router(contract, {
dev,
users,
quotes,
webhooks,
});

export function addApiRoutes(app: Application): void {
Expand Down Expand Up @@ -152,7 +152,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")) {
Expand Down
25 changes: 10 additions & 15 deletions backend/src/api/routes/webhooks.ts
Original file line number Diff line number Diff line change
@@ -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),
},
});
Loading
Loading