From 3adbdf2cdb3a764846667b952b293c79f56d841b Mon Sep 17 00:00:00 2001 From: Christian Fehmer Date: Thu, 30 Nov 2023 13:58:28 +0100 Subject: [PATCH] impr: replace body based authorization in dev mode (fehmer) (#4821) * impr: add authorization header Uid in favour of authorization on with body on dev (fehmer) * refactor dev mode detection --- .../__tests__/api/controllers/user.spec.ts | 6 +- backend/__tests__/middlewares/auth.spec.ts | 66 +++++++++++++++++++ backend/src/api/controllers/result.ts | 11 ++-- backend/src/api/controllers/user.ts | 15 ++--- backend/src/api/routes/index.ts | 3 +- backend/src/api/routes/swagger.ts | 3 +- backend/src/dal/leaderboards.ts | 3 +- backend/src/init/email-client.ts | 5 +- backend/src/init/firebase-admin.ts | 3 +- backend/src/init/redis.ts | 7 +- backend/src/middlewares/ape-rate-limit.ts | 3 +- backend/src/middlewares/api-utils.ts | 15 +---- backend/src/middlewares/auth.ts | 39 +++++------ backend/src/middlewares/error.ts | 3 +- backend/src/middlewares/rate-limit.ts | 3 +- backend/src/utils/captcha.ts | 3 +- backend/src/utils/discord.ts | 3 +- backend/src/utils/error.ts | 3 +- backend/src/utils/misc.ts | 4 ++ backend/src/version.ts | 4 +- 20 files changed, 134 insertions(+), 68 deletions(-) diff --git a/backend/__tests__/api/controllers/user.spec.ts b/backend/__tests__/api/controllers/user.spec.ts index 86596634f..18e05ab38 100644 --- a/backend/__tests__/api/controllers/user.spec.ts +++ b/backend/__tests__/api/controllers/user.spec.ts @@ -56,6 +56,7 @@ describe("user controller test", () => { await mockApp .post("/users/signup") + .set("authorization", "Uid 123456789|newuser@mail.com") .send(newUser) .set({ Accept: "application/json", @@ -64,9 +65,8 @@ describe("user controller test", () => { const response = await mockApp .get("/users") - .send({ - uid: "123456789", - }) + .set("authorization", "Uid 123456789") + .send() .set({ Accept: "application/json", }) diff --git a/backend/__tests__/middlewares/auth.spec.ts b/backend/__tests__/middlewares/auth.spec.ts index 3caeb80e4..f8c0a832d 100644 --- a/backend/__tests__/middlewares/auth.spec.ts +++ b/backend/__tests__/middlewares/auth.spec.ts @@ -6,6 +6,8 @@ import { getCachedConfiguration } from "../../src/init/configuration"; import * as ApeKeys from "../../src/dal/ape-keys"; import { ObjectId } from "mongodb"; import { hashSync } from "bcrypt"; +import MonkeyError from "../../src/utils/error"; +import * as Misc from "../../src/utils/misc"; const mockDecodedToken: DecodedIdToken = { uid: "123456789", @@ -28,6 +30,7 @@ const mockApeKey = { }; jest.spyOn(ApeKeys, "getApeKey").mockResolvedValue(mockApeKey); jest.spyOn(ApeKeys, "updateLastUsedOn").mockResolvedValue(); +const isDevModeMock = jest.spyOn(Misc, "isDevEnvironment"); describe("middlewares/auth", () => { let mockRequest: Partial; @@ -35,6 +38,7 @@ describe("middlewares/auth", () => { let nextFunction: NextFunction; beforeEach(async () => { + isDevModeMock.mockReturnValue(true); let config = await getCachedConfiguration(true); config.apeKeys.acceptKeys = true; @@ -66,6 +70,10 @@ describe("middlewares/auth", () => { }) as unknown as NextFunction; }); + afterEach(() => { + isDevModeMock.mockReset(); + }); + describe("authenticateRequest", () => { it("should fail if token is not fresh", async () => { Date.now = jest.fn(() => 60001); @@ -191,5 +199,63 @@ describe("middlewares/auth", () => { expect(decodedToken?.uid).toBe("123"); expect(nextFunction).toHaveBeenCalledTimes(1); }); + it("should allow request with Uid on dev", async () => { + mockRequest.headers = { + authorization: "Uid 123", + }; + + const authenticateRequest = Auth.authenticateRequest({}); + + await authenticateRequest( + mockRequest as Request, + mockResponse as Response, + nextFunction + ); + + const decodedToken = mockRequest?.ctx?.decodedToken; + + expect(decodedToken?.type).toBe("Bearer"); + expect(decodedToken?.email).toBe(""); + expect(decodedToken?.uid).toBe("123"); + expect(nextFunction).toHaveBeenCalledTimes(1); + }); + it("should allow request with Uid and email on dev", async () => { + mockRequest.headers = { + authorization: "Uid 123|test@example.com", + }; + + const authenticateRequest = Auth.authenticateRequest({}); + + await authenticateRequest( + mockRequest as Request, + mockResponse as Response, + nextFunction + ); + + const decodedToken = mockRequest?.ctx?.decodedToken; + + expect(decodedToken?.type).toBe("Bearer"); + expect(decodedToken?.email).toBe("test@example.com"); + expect(decodedToken?.uid).toBe("123"); + expect(nextFunction).toHaveBeenCalledTimes(1); + }); + it("should fail request with Uid on non-dev", async () => { + isDevModeMock.mockReturnValue(false); + mockRequest.headers = { + authorization: "Uid 123", + }; + + const authenticateRequest = Auth.authenticateRequest({}); + + await expect(() => + authenticateRequest( + mockRequest as Request, + mockResponse as Response, + nextFunction + ) + ).rejects.toThrow( + new MonkeyError(401, "Baerer type uid is not supported") + ); + }); }); }); diff --git a/backend/src/api/controllers/result.ts b/backend/src/api/controllers/result.ts index a2a47e1b0..70196220a 100644 --- a/backend/src/api/controllers/result.ts +++ b/backend/src/api/controllers/result.ts @@ -11,6 +11,7 @@ import * as PublicDAL from "../../dal/public"; import { getCurrentDayTimestamp, getStartOfDayTimestamp, + isDevEnvironment, mapRange, roundTo2, stdDev, @@ -47,7 +48,7 @@ try { if (anticheatImplemented() === false) throw new Error("undefined"); Logger.success("Anticheat module loaded"); } catch (e) { - if (process.env.MODE === "dev") { + if (isDevEnvironment()) { Logger.warning( "No anticheat module found. Continuing in dev mode, results will not be validated." ); @@ -274,7 +275,7 @@ export async function addResult( throw new MonkeyError(status.code, "Result data doesn't make sense"); } } else { - if (process.env.MODE !== "dev") { + if (!isDevEnvironment()) { throw new Error("No anticheat module found"); } Logger.warning( @@ -373,7 +374,7 @@ export async function addResult( throw new MonkeyError(status.code, "Possible bot detected"); } } else { - if (process.env.MODE !== "dev") { + if (!isDevEnvironment()) { throw new Error("No anticheat module found"); } Logger.warning( @@ -478,7 +479,7 @@ export async function addResult( !result.bailedOut && user.banned !== true && user.lbOptOut !== true && - (process.env.MODE === "dev" || (user.timeTyping ?? 0) > 7200); + (isDevEnvironment() || (user.timeTyping ?? 0) > 7200); const selectedBadgeId = user.inventory?.badges?.find((b) => b.selected)?.id; @@ -555,7 +556,7 @@ export async function addResult( const eligibleForWeeklyXpLeaderboard = user.banned !== true && user.lbOptOut !== true && - (process.env.MODE === "dev" || (user.timeTyping ?? 0) > 7200); + (isDevEnvironment() || (user.timeTyping ?? 0) > 7200); const weeklyXpLeaderboard = WeeklyXpLeaderboard.get( weeklyXpLeaderboardConfig diff --git a/backend/src/api/controllers/user.ts b/backend/src/api/controllers/user.ts index c072fb631..d289de272 100644 --- a/backend/src/api/controllers/user.ts +++ b/backend/src/api/controllers/user.ts @@ -7,6 +7,7 @@ import * as DiscordUtils from "../../utils/discord"; import { MILLISECONDS_IN_DAY, buildAgentLog, + isDevEnvironment, sanitizeString, } from "../../utils/misc"; import GeorgeQueue from "../../queues/george-queue"; @@ -100,10 +101,9 @@ export async function sendVerificationEmail( link = await FirebaseAdmin() .auth() .generateEmailVerificationLink(email, { - url: - process.env.MODE === "dev" - ? "http://localhost:3000" - : "https://monkeytype.com", + url: isDevEnvironment() + ? "http://localhost:3000" + : "https://monkeytype.com", }); } catch (e) { if ( @@ -162,10 +162,9 @@ export async function sendForgotPasswordEmail( const link = await FirebaseAdmin() .auth() .generatePasswordResetLink(email, { - url: - process.env.MODE === "dev" - ? "http://localhost:3000" - : "https://monkeytype.com", + url: isDevEnvironment() + ? "http://localhost:3000" + : "https://monkeytype.com", }); await emailQueue.sendForgotPasswordEmail(email, userInfo.name, link); diff --git a/backend/src/api/routes/index.ts b/backend/src/api/routes/index.ts index e5a344c66..444e1b5c7 100644 --- a/backend/src/api/routes/index.ts +++ b/backend/src/api/routes/index.ts @@ -24,6 +24,7 @@ import { Router, static as expressStatic, } from "express"; +import { isDevEnvironment } from "../../utils/misc"; const pathOverride = process.env.API_PATH_OVERRIDE; const BASE_ROUTE = pathOverride ? `/${pathOverride}` : ""; @@ -51,7 +52,7 @@ function addApiRoutes(app: Application): void { // Cannot be added to the route map because it needs to be added before the maintenance handler app.use("/configuration", configuration); - if (process.env.MODE === "dev") { + if (isDevEnvironment()) { //disable csp to allow assets to load from unsecured http app.use((req, res, next) => { res.setHeader("Content-Security-Policy", ""); diff --git a/backend/src/api/routes/swagger.ts b/backend/src/api/routes/swagger.ts index ed3db5053..09abfb597 100644 --- a/backend/src/api/routes/swagger.ts +++ b/backend/src/api/routes/swagger.ts @@ -7,6 +7,7 @@ import { } from "swagger-ui-express"; import publicSwaggerSpec from "../../documentation/public-swagger.json"; import internalSwaggerSpec from "../../documentation/internal-swagger.json"; +import { isDevEnvironment } from "../../utils/misc"; const SWAGGER_UI_OPTIONS = { customCss: ".swagger-ui .topbar { display: none } .try-out { display: none }", @@ -18,7 +19,7 @@ function addSwaggerMiddlewares(app: Application): void { getSwaggerMiddleware({ name: "Monkeytype API", uriPath: "/stats", - authentication: process.env.MODE !== "dev", + authentication: !isDevEnvironment(), apdexThreshold: 100, swaggerSpec: internalSwaggerSpec, onAuthenticate: (_req, username, password) => { diff --git a/backend/src/dal/leaderboards.ts b/backend/src/dal/leaderboards.ts index a7e52e983..1e92ab48e 100644 --- a/backend/src/dal/leaderboards.ts +++ b/backend/src/dal/leaderboards.ts @@ -2,6 +2,7 @@ import * as db from "../init/db"; import Logger from "../utils/logger"; import { performance } from "perf_hooks"; import { setLeaderboard } from "../utils/prometheus"; +import { isDevEnvironment } from "../utils/misc"; const leaderboardUpdating: { [key: string]: boolean } = {}; @@ -111,7 +112,7 @@ export async function update( $ne: true, }, timeTyping: { - $gt: process.env.MODE === "dev" ? 0 : 7200, + $gt: isDevEnvironment() ? 0 : 7200, }, }, }, diff --git a/backend/src/init/email-client.ts b/backend/src/init/email-client.ts index 36c57fffe..6a61d9e0c 100644 --- a/backend/src/init/email-client.ts +++ b/backend/src/init/email-client.ts @@ -6,6 +6,7 @@ import mjml2html from "mjml"; import mustache from "mustache"; import { recordEmail } from "../utils/prometheus"; import { EmailTaskContexts, EmailType } from "../queues/email-queue"; +import { isDevEnvironment } from "../utils/misc"; interface EmailMetadata { subject: string; @@ -35,10 +36,10 @@ export async function init(): Promise { return; } - const { EMAIL_HOST, EMAIL_USER, EMAIL_PASS, EMAIL_PORT, MODE } = process.env; + const { EMAIL_HOST, EMAIL_USER, EMAIL_PASS, EMAIL_PORT } = process.env; if (!EMAIL_HOST || !EMAIL_USER || !EMAIL_PASS) { - if (MODE === "dev") { + if (isDevEnvironment()) { Logger.warning( "No email client configuration provided. Running without email." ); diff --git a/backend/src/init/firebase-admin.ts b/backend/src/init/firebase-admin.ts index 8f731b98f..9ab11c2be 100644 --- a/backend/src/init/firebase-admin.ts +++ b/backend/src/init/firebase-admin.ts @@ -3,6 +3,7 @@ import Logger from "../utils/logger"; import { readFileSync, existsSync } from "fs"; import MonkeyError from "../utils/error"; import path from "path"; +import { isDevEnvironment } from "../utils/misc"; const SERVICE_ACCOUNT_PATH = path.join( __dirname, @@ -11,7 +12,7 @@ const SERVICE_ACCOUNT_PATH = path.join( export function init(): void { if (!existsSync(SERVICE_ACCOUNT_PATH)) { - if (process.env.MODE === "dev") { + if (isDevEnvironment()) { Logger.warning( "Firebase service account key not found! Continuing in dev mode, but authentication will throw errors." ); diff --git a/backend/src/init/redis.ts b/backend/src/init/redis.ts index 6a4543b06..223adc6d0 100644 --- a/backend/src/init/redis.ts +++ b/backend/src/init/redis.ts @@ -3,6 +3,7 @@ import _ from "lodash"; import { join } from "path"; import IORedis from "ioredis"; import Logger from "../utils/logger"; +import { isDevEnvironment } from "../utils/misc"; let connection: IORedis.Redis; let connected = false; @@ -28,10 +29,10 @@ export async function connect(): Promise { return; } - const { REDIS_URI, MODE } = process.env; + const { REDIS_URI } = process.env; if (!REDIS_URI) { - if (MODE === "dev") { + if (isDevEnvironment()) { Logger.warning("No redis configuration provided. Running without redis."); return; } @@ -53,7 +54,7 @@ export async function connect(): Promise { connected = true; } catch (error) { Logger.error(error.message); - if (MODE === "dev") { + if (isDevEnvironment()) { await connection.quit(); Logger.warning( `Failed to connect to redis. Continuing in dev mode, running without redis.` diff --git a/backend/src/middlewares/ape-rate-limit.ts b/backend/src/middlewares/ape-rate-limit.ts index b93a2756c..54272558d 100644 --- a/backend/src/middlewares/ape-rate-limit.ts +++ b/backend/src/middlewares/ape-rate-limit.ts @@ -5,8 +5,9 @@ import rateLimit, { RateLimitRequestHandler, Options, } from "express-rate-limit"; +import { isDevEnvironment } from "../utils/misc"; -const REQUEST_MULTIPLIER = process.env.MODE === "dev" ? 1 : 1; +const REQUEST_MULTIPLIER = isDevEnvironment() ? 1 : 1; const getKey = (req: MonkeyTypes.Request, _res: Response): string => { return req?.ctx?.decodedToken?.uid; diff --git a/backend/src/middlewares/api-utils.ts b/backend/src/middlewares/api-utils.ts index 52d280df6..83e0a76c7 100644 --- a/backend/src/middlewares/api-utils.ts +++ b/backend/src/middlewares/api-utils.ts @@ -5,6 +5,7 @@ import { Response, NextFunction, RequestHandler } from "express"; import { handleMonkeyResponse, MonkeyResponse } from "../utils/monkey-response"; import { getUser } from "../dal/user"; import { isAdmin } from "../dal/admin-uids"; +import { isDevEnvironment } from "../utils/misc"; interface ValidationOptions { criteria: (data: T) => boolean; @@ -132,18 +133,6 @@ interface ValidationSchema { } function validateRequest(validationSchema: ValidationSchema): RequestHandler { - /** - * In dev environments, as an alternative to token authentication, - * you can pass the authentication middleware by having a user id in the body. - * Inject the user id into the schema so that validation will not fail. - */ - if (process.env.MODE === "dev") { - validationSchema.body = { - uid: joi.any(), - ...(validationSchema.body ?? {}), - }; - } - const { validationErrorMessage } = validationSchema; const normalizedValidationSchema: ValidationSchema = _.omit( validationSchema, @@ -177,7 +166,7 @@ function validateRequest(validationSchema: ValidationSchema): RequestHandler { */ function useInProduction(middlewares: RequestHandler[]): RequestHandler[] { return middlewares.map((middleware) => - process.env.MODE === "dev" ? emptyMiddleware : middleware + isDevEnvironment() ? emptyMiddleware : middleware ); } diff --git a/backend/src/middlewares/auth.ts b/backend/src/middlewares/auth.ts index 584762299..055eabb03 100644 --- a/backend/src/middlewares/auth.ts +++ b/backend/src/middlewares/auth.ts @@ -2,7 +2,7 @@ import { compare } from "bcrypt"; import { getApeKey, updateLastUsedOn } from "../dal/ape-keys"; import MonkeyError from "../utils/error"; import { verifyIdToken } from "../utils/auth"; -import { base64UrlDecode } from "../utils/misc"; +import { base64UrlDecode, isDevEnvironment } from "../utils/misc"; import { NextFunction, Response, Handler } from "express"; import statuses from "../constants/monkey-status-codes"; import { @@ -57,8 +57,6 @@ function authenticateRequest(authOptions = DEFAULT_OPTIONS): Handler { uid: "", email: "", }; - } else if (process.env.MODE === "dev") { - token = authenticateWithBody(req.body); } else { throw new MonkeyError( 401, @@ -105,25 +103,6 @@ function authenticateRequest(authOptions = DEFAULT_OPTIONS): Handler { }; } -function authenticateWithBody( - body: MonkeyTypes.Request["body"] -): MonkeyTypes.DecodedToken { - const { uid, email } = body; - - if (!uid) { - throw new MonkeyError( - 401, - "Running authorization in dev mode but still no uid was provided" - ); - } - - return { - type: "Bearer", - uid, - email: email ?? "", - }; -} - async function authenticateWithAuthHeader( authHeader: string, configuration: MonkeyTypes.Configuration, @@ -137,6 +116,8 @@ async function authenticateWithAuthHeader( return await authenticateWithBearerToken(token, options); case "ApeKey": return await authenticateWithApeKey(token, configuration, options); + case "Uid": + return await authenticateWithUid(token); } throw new MonkeyError( @@ -257,6 +238,20 @@ async function authenticateWithApeKey( } } +async function authenticateWithUid( + token: string +): Promise { + if (!isDevEnvironment()) { + throw new MonkeyError(401, "Baerer type uid is not supported"); + } + const uidAndEmail = token.split("|"); + return { + type: "Bearer", + uid: uidAndEmail[0], + email: uidAndEmail.length > 1 ? uidAndEmail[1] : "", + }; +} + function authenticateGithubWebhook(): Handler { return async ( req: MonkeyTypes.Request, diff --git a/backend/src/middlewares/error.ts b/backend/src/middlewares/error.ts index 5b93d7c97..727d8fe1b 100644 --- a/backend/src/middlewares/error.ts +++ b/backend/src/middlewares/error.ts @@ -6,6 +6,7 @@ import { incrementBadAuth } from "./rate-limit"; import { NextFunction, Response } from "express"; import { MonkeyResponse, handleMonkeyResponse } from "../utils/monkey-response"; import { recordClientErrorByVersion } from "../utils/prometheus"; +import { isDevEnvironment } from "../utils/misc"; async function errorHandlingMiddleware( error: Error, @@ -42,7 +43,7 @@ async function errorHandlingMiddleware( recordClientErrorByVersion(req.headers["x-client-version"] as string); } - if (process.env.MODE !== "dev" && monkeyResponse.status >= 500) { + if (!isDevEnvironment() && monkeyResponse.status >= 500) { const { uid, errorId } = monkeyResponse.data; try { diff --git a/backend/src/middlewares/rate-limit.ts b/backend/src/middlewares/rate-limit.ts index 27706827a..f8dede206 100644 --- a/backend/src/middlewares/rate-limit.ts +++ b/backend/src/middlewares/rate-limit.ts @@ -3,8 +3,9 @@ import MonkeyError from "../utils/error"; import { Response, NextFunction } from "express"; import { RateLimiterMemory } from "rate-limiter-flexible"; import rateLimit, { Options } from "express-rate-limit"; +import { isDevEnvironment } from "../utils/misc"; -const REQUEST_MULTIPLIER = process.env.MODE === "dev" ? 100 : 1; +const REQUEST_MULTIPLIER = isDevEnvironment() ? 100 : 1; const getKey = (req: MonkeyTypes.Request, _res: Response): string => { return (req.headers["cf-connecting-ip"] || diff --git a/backend/src/utils/captcha.ts b/backend/src/utils/captcha.ts index 357667e94..6b6748cc3 100644 --- a/backend/src/utils/captcha.ts +++ b/backend/src/utils/captcha.ts @@ -1,4 +1,5 @@ import fetch from "node-fetch"; +import { isDevEnvironment } from "./misc"; interface CaptchaData { success: boolean; @@ -8,7 +9,7 @@ interface CaptchaData { } export async function verify(captcha: string): Promise { - if (process.env.MODE === "dev") { + if (isDevEnvironment()) { return true; } const response = await fetch( diff --git a/backend/src/utils/discord.ts b/backend/src/utils/discord.ts index 187e8a440..ab2f1c8e6 100644 --- a/backend/src/utils/discord.ts +++ b/backend/src/utils/discord.ts @@ -1,4 +1,5 @@ import fetch from "node-fetch"; +import { isDevEnvironment } from "./misc"; const BASE_URL = "https://discord.com/api"; @@ -35,7 +36,7 @@ export async function getDiscordUser( export function getOauthLink(): string { return `${BASE_URL}/oauth2/authorize?client_id=798272335035498557&redirect_uri=${ - process.env.MODE === "dev" + isDevEnvironment() ? `http%3A%2F%2Flocalhost%3A3000%2Fverify` : `https%3A%2F%2Fmonkeytype.com%2Fverify` }&response_type=token&scope=identify`; diff --git a/backend/src/utils/error.ts b/backend/src/utils/error.ts index 4a0525330..7d306f0c9 100644 --- a/backend/src/utils/error.ts +++ b/backend/src/utils/error.ts @@ -1,4 +1,5 @@ import { v4 as uuidv4 } from "uuid"; +import { isDevEnvironment } from "./misc"; class MonkeyError extends Error { status: number; @@ -12,7 +13,7 @@ class MonkeyError extends Error { this.stack = stack; this.uid = uid; - if (process.env.MODE === "dev") { + if (isDevEnvironment()) { this.message = stack ? String(message) + "\nStack: " + String(stack) : String(message); diff --git a/backend/src/utils/misc.ts b/backend/src/utils/misc.ts index b2f136b20..12ac2a016 100644 --- a/backend/src/utils/misc.ts +++ b/backend/src/utils/misc.ts @@ -298,3 +298,7 @@ export function stringToNumberOrDefault( if (!Number.isFinite(value)) return defaultValue; return value; } + +export function isDevEnvironment(): boolean { + return process.env.MODE === "dev"; +} diff --git a/backend/src/version.ts b/backend/src/version.ts index f501a248e..1dd8a0ecb 100644 --- a/backend/src/version.ts +++ b/backend/src/version.ts @@ -1,5 +1,5 @@ import { join } from "path"; -import { padNumbers } from "./utils/misc"; +import { isDevEnvironment, padNumbers } from "./utils/misc"; import { readFileSync, writeFileSync, existsSync } from "fs"; const SERVER_VERSION_FILE_PATH = join(__dirname, "./server.version"); @@ -21,7 +21,7 @@ function getDateVersion(): string { } function getVersion(): string { - if (process.env.MODE === "dev") { + if (isDevEnvironment()) { return "DEVELOPMENT-VERSION"; }