diff --git a/server/middlewares/methodOverride.ts b/server/middlewares/methodOverride.ts index 9e37a2f4a..e80639a27 100644 --- a/server/middlewares/methodOverride.ts +++ b/server/middlewares/methodOverride.ts @@ -3,11 +3,10 @@ import queryString from "query-string"; export default function methodOverride() { return async function methodOverrideMiddleware(ctx: Context, next: Next) { + // TODO: Need to remove this use of ctx.body to enable proper typing of requests if (ctx.method === "POST") { ctx.body = ctx.request.body; } else if (ctx.method === "GET") { - ctx.method = 'POST'; // eslint-disable-line - ctx.body = queryString.parse(ctx.querystring); } diff --git a/server/routes/api/attachments.ts b/server/routes/api/attachments.ts index 4ae074f19..8adfc411f 100644 --- a/server/routes/api/attachments.ts +++ b/server/routes/api/attachments.ts @@ -7,6 +7,7 @@ import { AuthorizationError, ValidationError } from "@server/errors"; import auth from "@server/middlewares/authentication"; import { Attachment, Document, Event } from "@server/models"; import { authorize } from "@server/policies"; +import { ContextWithState } from "@server/types"; import { getPresignedPost, publicS3Endpoint, @@ -145,9 +146,10 @@ router.post("attachments.delete", auth(), async (ctx) => { }; }); -router.post("attachments.redirect", auth(), async (ctx) => { - const { id } = ctx.body; +const handleAttachmentsRedirect = async (ctx: ContextWithState) => { + const { id } = ctx.body as { id?: string }; assertUuid(id, "id is required"); + const { user } = ctx.state; const attachment = await Attachment.findByPk(id, { rejectOnEmpty: true, @@ -163,6 +165,9 @@ router.post("attachments.redirect", auth(), async (ctx) => { } else { ctx.redirect(attachment.canonicalUrl); } -}); +}; + +router.get("attachments.redirect", auth(), handleAttachmentsRedirect); +router.post("attachments.redirect", auth(), handleAttachmentsRedirect); export default router; diff --git a/server/routes/api/cron.ts b/server/routes/api/cron.ts index 2b5fc67da..52df20e3f 100644 --- a/server/routes/api/cron.ts +++ b/server/routes/api/cron.ts @@ -40,9 +40,11 @@ const cronHandler = async (ctx: Context) => { }; }; +router.get("cron.:period", cronHandler); router.post("cron.:period", cronHandler); // For backwards compatibility +router.get("utils.gc", cronHandler); router.post("utils.gc", cronHandler); export default router; diff --git a/server/routes/api/documents.ts b/server/routes/api/documents.ts index 1973808c3..96a35222d 100644 --- a/server/routes/api/documents.ts +++ b/server/routes/api/documents.ts @@ -14,6 +14,7 @@ import { NotFoundError, InvalidRequestError, AuthenticationError, + ValidationError, } from "@server/errors"; import auth from "@server/middlewares/authentication"; import { @@ -478,8 +479,10 @@ router.post("documents.restore", auth({ member: true }), async (ctx) => { // be caught as a 403 on the authorize call below. Otherwise we're checking here // that the original collection still exists and advising to pass collectionId // if not. - if (!collectionId) { - assertPresent(collection, "collectionId is required"); + if (!collectionId && !collection) { + throw ValidationError( + "Unable to restore to original collection, it may have been deleted" + ); } authorize(user, "update", collection); diff --git a/server/routes/api/index.test.ts b/server/routes/api/index.test.ts index e19716916..857c3031e 100644 --- a/server/routes/api/index.test.ts +++ b/server/routes/api/index.test.ts @@ -17,3 +17,10 @@ describe("GET unknown endpoint", () => { expect(res.status).toEqual(404); }); }); + +describe("PATCH unknown endpoint", () => { + it("should be method not allowed", async () => { + const res = await server.patch("/api/blah"); + expect(res.status).toEqual(405); + }); +}); diff --git a/server/routes/api/index.ts b/server/routes/api/index.ts index d1c96ee42..2e7201a4f 100644 --- a/server/routes/api/index.ts +++ b/server/routes/api/index.ts @@ -1,4 +1,4 @@ -import Koa from "koa"; +import Koa, { DefaultContext, DefaultState } from "koa"; import bodyParser from "koa-body"; import Router from "koa-router"; import env from "@server/env"; @@ -6,6 +6,7 @@ import { NotFoundError } from "@server/errors"; import errorHandling from "@server/middlewares/errorHandling"; import methodOverride from "@server/middlewares/methodOverride"; import { defaultRateLimiter } from "@server/middlewares/rateLimiter"; +import { AuthenticatedState } from "@server/types"; import apiKeys from "./apiKeys"; import attachments from "./attachments"; import auth from "./auth"; @@ -33,7 +34,10 @@ import users from "./users"; import views from "./views"; import webhookSubscriptions from "./webhookSubscriptions"; -const api = new Koa(); +const api = new Koa< + DefaultState & AuthenticatedState, + DefaultContext & { body: Record } +>(); const router = new Router(); // middlewares @@ -83,10 +87,15 @@ router.post("*", (ctx) => { ctx.throw(NotFoundError("Endpoint not found")); }); +router.get("*", (ctx) => { + ctx.throw(NotFoundError("Endpoint not found")); +}); + api.use(defaultRateLimiter()); // Router is embedded in a Koa application wrapper, because koa-router does not // allow middleware to catch any routes which were not explicitly defined. api.use(router.routes()); +api.use(router.allowedMethods()); export default api; diff --git a/server/routes/api/notificationSettings.ts b/server/routes/api/notificationSettings.ts index ef0c929f1..75f481a44 100644 --- a/server/routes/api/notificationSettings.ts +++ b/server/routes/api/notificationSettings.ts @@ -4,6 +4,7 @@ import auth from "@server/middlewares/authentication"; import { Team, NotificationSetting } from "@server/models"; import { authorize } from "@server/policies"; import { presentNotificationSetting } from "@server/presenters"; +import { ContextWithState } from "@server/types"; import { assertPresent, assertUuid } from "@server/validation"; const router = new Router(); @@ -55,8 +56,8 @@ router.post("notificationSettings.delete", auth(), async (ctx) => { }; }); -router.post("notificationSettings.unsubscribe", async (ctx) => { - const { id, token } = ctx.body; +const handleUnsubscribe = async (ctx: ContextWithState) => { + const { id, token } = ctx.body as { id?: string; token?: string }; assertUuid(id, "id is required"); assertPresent(token, "token is required"); @@ -77,6 +78,9 @@ router.post("notificationSettings.unsubscribe", async (ctx) => { } ctx.redirect(`${env.URL}?notice=invalid-auth`); -}); +}; + +router.get("notificationSettings.unsubscribe", handleUnsubscribe); +router.post("notificationSettings.unsubscribe", handleUnsubscribe); export default router; diff --git a/server/test/env.ts b/server/test/env.ts index 3d5e3cac2..61bfa0be3 100644 --- a/server/test/env.ts +++ b/server/test/env.ts @@ -7,6 +7,7 @@ env.GOOGLE_CLIENT_ID = "123"; env.GOOGLE_CLIENT_SECRET = "123"; env.SLACK_CLIENT_ID = "123"; env.SLACK_CLIENT_SECRET = "123"; +env.RATE_LIMITER_ENABLED = false; env.DEPLOYMENT = undefined; if (process.env.DATABASE_URL_TEST) { diff --git a/server/types.ts b/server/types.ts index 08d7431ce..1100c329f 100644 --- a/server/types.ts +++ b/server/types.ts @@ -6,12 +6,14 @@ export enum AuthenticationType { APP = "app", } +export type AuthenticatedState = { + user: User; + token: string; + authType: AuthenticationType; +}; + export type ContextWithState = Context & { - state: { - user: User; - token: string; - authType: AuthenticationType; - }; + state: AuthenticatedState; }; type BaseEvent = { diff --git a/server/validation.ts b/server/validation.ts index 86cb71572..10d5fadc3 100644 --- a/server/validation.ts +++ b/server/validation.ts @@ -1,21 +1,27 @@ import { isArrayLike } from "lodash"; +import { Primitive } from "utility-types"; import validator from "validator"; import { CollectionPermission } from "../shared/types"; import { validateColorHex } from "../shared/utils/color"; import { validateIndexCharacters } from "../shared/utils/indexCharacters"; import { ParamRequiredError, ValidationError } from "./errors"; -export const assertPresent = (value: unknown, message: string) => { +type IncomingValue = Primitive | string[]; + +export const assertPresent = (value: IncomingValue, message: string) => { if (value === undefined || value === null || value === "") { throw ParamRequiredError(message); } }; -export const assertArray = (value: unknown, message?: string) => { +export function assertArray( + value: IncomingValue, + message?: string +): asserts value { if (!isArrayLike(value)) { throw ValidationError(message); } -}; +} export const assertIn = ( value: string, @@ -37,41 +43,57 @@ export const assertSort = ( } }; -export const assertNotEmpty = (value: unknown, message: string) => { +export function assertNotEmpty( + value: IncomingValue, + message: string +): asserts value { assertPresent(value, message); if (typeof value === "string" && value.trim() === "") { throw ValidationError(message); } -}; +} -export const assertEmail = (value = "", message?: string) => { - if (!validator.isEmail(value)) { +export function assertEmail( + value: IncomingValue = "", + message?: string +): asserts value { + if (typeof value !== "string" || !validator.isEmail(value)) { throw ValidationError(message); } -}; +} -export const assertUrl = (value = "", message?: string) => { +export function assertUrl( + value: IncomingValue = "", + message?: string +): asserts value { if ( + typeof value !== "string" || !validator.isURL(value, { protocols: ["http", "https"], require_valid_protocol: true, }) ) { - throw ValidationError(message ?? `${value} is an invalid url!`); + throw ValidationError(message ?? `${String(value)} is an invalid url!`); } -}; +} -export const assertUuid = (value: unknown, message?: string) => { +export function assertUuid( + value: IncomingValue, + message?: string +): asserts value { if (typeof value !== "string") { throw ValidationError(message); } if (!validator.isUUID(value)) { throw ValidationError(message); } -}; +} -export const assertPositiveInteger = (value: unknown, message?: string) => { +export const assertPositiveInteger = ( + value: IncomingValue, + message?: string +) => { if ( !validator.isInt(String(value), { min: 0,