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: move configuration checks to contracts (@fehmer) #5851

Merged
merged 6 commits into from
Sep 11, 2024
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
185 changes: 185 additions & 0 deletions backend/__tests__/middlewares/configuration.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
import { RequireConfiguration } from "@monkeytype/contracts/require-configuration/index";
import { verifyRequiredConfiguration } from "../../src/middlewares/configuration";
import { Configuration } from "@monkeytype/contracts/schemas/configuration";
import { Response } from "express";
import MonkeyError from "../../src/utils/error";

describe("configuration middleware", () => {
const handler = verifyRequiredConfiguration();
const res: Response = {} as any;
const next = vi.fn();

beforeEach(() => {
next.mockReset();
});
afterEach(() => {
//next function must only be called once
expect(next).toHaveBeenCalledOnce();
});

it("should pass without requireConfiguration", async () => {
//GIVEN
const req = { tsRestRoute: { metadata: {} } } as any;

//WHEN
await handler(req, res, next);

//THEN
expect(next).toHaveBeenCalledWith();
});
it("should pass for enabled configuration", async () => {
//GIVEN
const req = givenRequest({ path: "maintenance" }, { maintenance: true });

//WHEN
await handler(req, res, next);

//THEN
expect(next).toHaveBeenCalledWith();
});
it("should pass for enabled configuration with complex path", async () => {
//GIVEN
const req = givenRequest(
{ path: "users.xp.streak.enabled" },
{ users: { xp: { streak: { enabled: true } as any } as any } as any }
);

//WHEN
await handler(req, res, next);

//THEN
expect(next).toHaveBeenCalledWith();
});
it("should fail for disabled configuration", async () => {
//GIVEN
const req = givenRequest({ path: "maintenance" }, { maintenance: false });

//WHEN
await handler(req, res, next);

//THEN
expect(next).toHaveBeenCalledWith(
new MonkeyError(503, "This endpoint is currently unavailable.")
);
});
it("should fail for disabled configuration and custom message", async () => {
//GIVEN
const req = givenRequest(
{ path: "maintenance", invalidMessage: "Feature not enabled." },
{ maintenance: false }
);

//WHEN
await handler(req, res, next);

//THEN
expect(next).toHaveBeenCalledWith(
new MonkeyError(503, "Feature not enabled.")
);
});
it("should fail for invalid path", async () => {
//GIVEN
const req = givenRequest({ path: "invalid.path" as any }, {});

//WHEN
await handler(req, res, next);

//THEN
expect(next).toHaveBeenCalledWith(
new MonkeyError(503, 'Invalid configuration path: "invalid.path"')
);
});
it("should fail for undefined value", async () => {
//GIVEN
const req = givenRequest(
{ path: "admin.endpointsEnabled" },
{ admin: {} as any }
);

//WHEN
await handler(req, res, next);

//THEN
expect(next).toHaveBeenCalledWith(
new MonkeyError(
500,
'Required configuration doesnt exist: "admin.endpointsEnabled"'
)
);
});
it("should fail for null value", async () => {
//GIVEN
const req = givenRequest(
{ path: "admin.endpointsEnabled" },
{ admin: { endpointsEnabled: null as any } }
);

//WHEN
await handler(req, res, next);

//THEN
expect(next).toHaveBeenCalledWith(
new MonkeyError(
500,
'Required configuration doesnt exist: "admin.endpointsEnabled"'
)
);
});
it("should fail for non booean value", async () => {
//GIVEN
const req = givenRequest(
{ path: "admin.endpointsEnabled" },
{ admin: { endpointsEnabled: "disabled" as any } }
);

//WHEN
await handler(req, res, next);

//THEN
expect(next).toHaveBeenCalledWith(
new MonkeyError(
500,
'Required configuration is not a boolean: "admin.endpointsEnabled"'
)
);
});
it("should pass for multiple configurations", async () => {
//GIVEN
const req = givenRequest(
[{ path: "maintenance" }, { path: "admin.endpointsEnabled" }],
{ maintenance: true, admin: { endpointsEnabled: true } }
);

//WHEN
await handler(req, res, next);

//THEN
expect(next).toHaveBeenCalledWith();
});
it("should fail for multiple configurations", async () => {
//GIVEN
const req = givenRequest(
[
{ path: "maintenance", invalidMessage: "maintenance mode" },
{ path: "admin.endpointsEnabled", invalidMessage: "admin disabled" },
],
{ maintenance: true, admin: { endpointsEnabled: false } }
);

//WHEN
await handler(req, res, next);

//THEN
expect(next).toHaveBeenCalledWith(new MonkeyError(503, "admin disabled"));
});
});

function givenRequest(
requireConfiguration: RequireConfiguration | RequireConfiguration[],
configuration: Partial<Configuration>
): TsRestRequest {
return {
tsRestRoute: { metadata: { requireConfiguration } },
ctx: { configuration },
} as any;
}
18 changes: 14 additions & 4 deletions backend/scripts/openapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { contract } from "@monkeytype/contracts/index";
import { writeFileSync, mkdirSync } from "fs";
import {
EndpointMetadata,
Permission,
PermissionId,
} from "@monkeytype/contracts/schemas/api";
import type { OpenAPIObject, OperationObject } from "openapi3-ts";
import {
Expand Down Expand Up @@ -142,15 +142,15 @@ export function getOpenApi(): OpenAPIObject {
setOperationId: "concatenated-path",
operationMapper: (operation, route) => {
const metadata = route.metadata as EndpointMetadata;

if (!operation.description?.trim()?.endsWith("."))
if (!operation.description?.trim()?.endsWith(".")) {
operation.description += ".";
}
operation.description += "\n\n";

addAuth(operation, metadata);
addRateLimit(operation, metadata);
addRequiredConfiguration(operation, metadata);
addTags(operation, metadata);

return operation;
},
}
Expand Down Expand Up @@ -262,6 +262,16 @@ function formatWindow(window: Window): string {
return "per " + window;
}

function addRequiredConfiguration(
operation: OperationObject,
metadata: EndpointMetadata | undefined
): void {
if (metadata === undefined || metadata.requireConfiguration === undefined)
return;

operation.description += `**Required configuration:** This operation can only be called if the [configuration](#tag/configuration/operation/configuration.get) for \`${metadata.requireConfiguration.path}\` is \`true\`.\n\n`;
}

//detect if we run this as a main
if (require.main === module) {
const args = process.argv.slice(2);
Expand Down
16 changes: 0 additions & 16 deletions backend/src/api/routes/admin.ts
Original file line number Diff line number Diff line change
@@ -1,41 +1,25 @@
// import joi from "joi";

import * as AdminController from "../controllers/admin";

import { adminContract } from "@monkeytype/contracts/admin";
import { initServer } from "@ts-rest/express";
import { validate } from "../../middlewares/configuration";
import { callController } from "../ts-rest-adapter";

const commonMiddleware = [
validate({
criteria: (configuration) => {
return configuration.admin.endpointsEnabled;
},
invalidMessage: "Admin endpoints are currently disabled.",
}),
];

const s = initServer();
export default s.router(adminContract, {
test: {
middleware: commonMiddleware,
handler: async (r) => callController(AdminController.test)(r),
},
toggleBan: {
middleware: commonMiddleware,
handler: async (r) => callController(AdminController.toggleBan)(r),
},
acceptReports: {
middleware: commonMiddleware,
handler: async (r) => callController(AdminController.acceptReports)(r),
},
rejectReports: {
middleware: commonMiddleware,
handler: async (r) => callController(AdminController.rejectReports)(r),
},
sendForgotPasswordEmail: {
middleware: commonMiddleware,
handler: async (r) =>
callController(AdminController.sendForgotPasswordEmail)(r),
},
Expand Down
15 changes: 0 additions & 15 deletions backend/src/api/routes/ape-keys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,18 @@ import { initServer } from "@ts-rest/express";
import * as ApeKeyController from "../controllers/ape-key";
import { callController } from "../ts-rest-adapter";

import { validate } from "../../middlewares/configuration";

const commonMiddleware = [
validate({
criteria: (configuration) => {
return configuration.apeKeys.endpointsEnabled;
},
invalidMessage: "ApeKeys are currently disabled.",
}),
];

const s = initServer();
export default s.router(apeKeysContract, {
get: {
middleware: commonMiddleware,
handler: async (r) => callController(ApeKeyController.getApeKeys)(r),
},
add: {
middleware: commonMiddleware,
handler: async (r) => callController(ApeKeyController.generateApeKey)(r),
},
save: {
middleware: commonMiddleware,
handler: async (r) => callController(ApeKeyController.editApeKey)(r),
},
delete: {
middleware: commonMiddleware,
handler: async (r) => callController(ApeKeyController.deleteApeKey)(r),
},
});
2 changes: 2 additions & 0 deletions backend/src/api/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import { MonkeyValidationError } from "@monkeytype/contracts/schemas/api";
import { authenticateTsRestRequest } from "../../middlewares/auth";
import { rateLimitRequest } from "../../middlewares/rate-limit";
import { verifyPermissions } from "../../middlewares/permission";
import { verifyRequiredConfiguration } from "../../middlewares/configuration";

const pathOverride = process.env["API_PATH_OVERRIDE"];
const BASE_ROUTE = pathOverride !== undefined ? `/${pathOverride}` : "";
Expand Down Expand Up @@ -116,6 +117,7 @@ function applyTsRestApiRoutes(app: IRouter): void {
globalMiddleware: [
authenticateTsRestRequest(),
rateLimitRequest(),
verifyRequiredConfiguration(),
verifyPermissions(),
],
});
Expand Down
20 changes: 0 additions & 20 deletions backend/src/api/routes/leaderboards.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,8 @@
import { initServer } from "@ts-rest/express";
import { validate } from "../../middlewares/configuration";
import * as LeaderboardController from "../controllers/leaderboard";

import { leaderboardsContract } from "@monkeytype/contracts/leaderboards";
import { callController } from "../ts-rest-adapter";

const requireDailyLeaderboardsEnabled = validate({
criteria: (configuration) => {
return configuration.dailyLeaderboards.enabled;
},
invalidMessage: "Daily leaderboards are not available at this time.",
});

const requireWeeklyXpLeaderboardEnabled = validate({
criteria: (configuration) => {
return configuration.leaderboards.weeklyXp.enabled;
},
invalidMessage: "Weekly XP leaderboards are not available at this time.",
});

const s = initServer();
export default s.router(leaderboardsContract, {
get: {
Expand All @@ -30,22 +14,18 @@ export default s.router(leaderboardsContract, {
callController(LeaderboardController.getRankFromLeaderboard)(r),
},
getDaily: {
middleware: [requireDailyLeaderboardsEnabled],
handler: async (r) =>
callController(LeaderboardController.getDailyLeaderboard)(r),
},
getDailyRank: {
middleware: [requireDailyLeaderboardsEnabled],
handler: async (r) =>
callController(LeaderboardController.getDailyLeaderboardRank)(r),
},
getWeeklyXp: {
middleware: [requireWeeklyXpLeaderboardEnabled],
handler: async (r) =>
callController(LeaderboardController.getWeeklyXpLeaderboardResults)(r),
},
getWeeklyXpRank: {
middleware: [requireWeeklyXpLeaderboardEnabled],
handler: async (r) =>
callController(LeaderboardController.getWeeklyXpLeaderboardRank)(r),
},
Expand Down
Loading
Loading