From e5b9b65b55494b143ccf3dd6023469a7f6a16acb Mon Sep 17 00:00:00 2001 From: Max Leiter Date: Tue, 5 Apr 2022 16:17:08 -0700 Subject: [PATCH] server: add basic is-admin tests and bug fixes --- .../badges/expiration-badge/index.tsx | 2 +- server/src/__tests__/e2e.ts | 28 +++--- server/src/lib/__tests__/config.ts | 98 ++++++++++--------- .../lib/__tests__/get-html-from-drift-file.ts | 17 ++++ .../src/lib/__tests__/middleware/is-admin.ts | 50 ++++++++++ server/src/lib/config.ts | 26 +++-- server/src/lib/get-html-from-drift-file.ts | 56 +++++------ server/src/lib/middleware/is-admin.ts | 10 +- server/src/routes/posts.ts | 1 - 9 files changed, 182 insertions(+), 106 deletions(-) create mode 100644 server/src/lib/__tests__/get-html-from-drift-file.ts create mode 100644 server/src/lib/__tests__/middleware/is-admin.ts diff --git a/client/components/badges/expiration-badge/index.tsx b/client/components/badges/expiration-badge/index.tsx index 4d6a1acb..7901d31c 100644 --- a/client/components/badges/expiration-badge/index.tsx +++ b/client/components/badges/expiration-badge/index.tsx @@ -49,9 +49,9 @@ const ExpirationBadge = ({ return ( {isExpired ? "Expired" : `Expires ${timeUntilString}`} - hideArrow ) diff --git a/server/src/__tests__/e2e.ts b/server/src/__tests__/e2e.ts index 1228945d..664ef5ca 100644 --- a/server/src/__tests__/e2e.ts +++ b/server/src/__tests__/e2e.ts @@ -1,16 +1,16 @@ -import * as request from 'supertest' -import { app } from '../app' +import * as request from "supertest" +import { app } from "../app" -describe('GET /health', () => { - it('should return 200 and a status up', (done) => { - request(app) - .get(`/health`) - .expect('Content-Type', /json/) - .expect(200) - .end((err, res) => { - if (err) return done(err) - expect(res.body).toMatchObject({ 'status': 'UP' }) - done() - }) - }) +describe("GET /health", () => { + it("should return 200 and a status up", (done) => { + request(app) + .get(`/health`) + .expect("Content-Type", /json/) + .expect(200) + .end((err, res) => { + if (err) return done(err) + expect(res.body).toMatchObject({ status: "UP" }) + done() + }) + }) }) diff --git a/server/src/lib/__tests__/config.ts b/server/src/lib/__tests__/config.ts index 84612f31..8a9aacac 100644 --- a/server/src/lib/__tests__/config.ts +++ b/server/src/lib/__tests__/config.ts @@ -1,62 +1,64 @@ import { config } from "../config" describe("Config", () => { - it("should build a valid development config when no environment is set", () => { - const emptyEnv = {}; - const result = config(emptyEnv); + it("should build a valid development config when no environment is set", () => { + const emptyEnv = {} + const result = config(emptyEnv) - expect(result).toHaveProperty("is_production", false) - expect(result).toHaveProperty("port") - expect(result).toHaveProperty("jwt_secret") - expect(result).toHaveProperty("drift_home") - expect(result).toHaveProperty("memory_db") - expect(result).toHaveProperty("enable_admin") - expect(result).toHaveProperty("secret_key") - expect(result).toHaveProperty("registration_password") - expect(result).toHaveProperty("welcome_content") - expect(result).toHaveProperty("welcome_title") - }) + expect(result).toHaveProperty("is_production", false) + expect(result).toHaveProperty("port") + expect(result).toHaveProperty("jwt_secret") + expect(result).toHaveProperty("drift_home") + expect(result).toHaveProperty("memory_db") + expect(result).toHaveProperty("enable_admin") + expect(result).toHaveProperty("secret_key") + expect(result).toHaveProperty("registration_password") + expect(result).toHaveProperty("welcome_content") + expect(result).toHaveProperty("welcome_title") + }) - it("should fail when building a prod environment without SECRET_KEY", () => { - expect(() => config({ NODE_ENV: "production" })) - .toThrow(new Error("Missing environment variable: SECRET_KEY")) - }) + it("should fail when building a prod environment without SECRET_KEY", () => { + expect(() => config({ NODE_ENV: "production" })).toThrow( + new Error("Missing environment variable: SECRET_KEY") + ) + }) - it("should build a prod config with a SECRET_KEY", () => { - const result = config({ NODE_ENV: "production", SECRET_KEY: "secret" }) + it("should build a prod config with a SECRET_KEY", () => { + const result = config({ NODE_ENV: "production", SECRET_KEY: "secret" }) - expect(result).toHaveProperty("is_production", true) - expect(result).toHaveProperty("secret_key", "secret") - }) + expect(result).toHaveProperty("is_production", true) + expect(result).toHaveProperty("secret_key", "secret") + }) - describe("jwt_secret", () => { - it("should use default jwt_secret when environment is blank string", () => { - const result = config({ JWT_SECRET: "" }) + describe("jwt_secret", () => { + it("should use default jwt_secret when environment is blank string", () => { + const result = config({ JWT_SECRET: "" }) - expect(result).toHaveProperty("is_production", false) - expect(result).toHaveProperty("jwt_secret", "myjwtsecret") - }) - }) + expect(result).toHaveProperty("is_production", false) + expect(result).toHaveProperty("jwt_secret", "myjwtsecret") + }) + }) - describe("booleans", () => { - it("should parse 'true' as true", () => { - const result = config({ MEMORY_DB: "true" }) + describe("booleans", () => { + it("should parse 'true' as true", () => { + const result = config({ MEMORY_DB: "true" }) - expect(result).toHaveProperty("memory_db", true) - }) - it("should parse 'false' as false", () => { - const result = config({ MEMORY_DB: "false" }) + expect(result).toHaveProperty("memory_db", true) + }) + it("should parse 'false' as false", () => { + const result = config({ MEMORY_DB: "false" }) - expect(result).toHaveProperty("memory_db", false) - }) - it("should fail when it is not parseable", () => { - expect(() => config({ MEMORY_DB: "foo" })) - .toThrow(new Error("Invalid boolean value: foo")) - }) - it("should default to false when the string is empty", () => { - const result = config({ MEMORY_DB: "" }) + expect(result).toHaveProperty("memory_db", false) + }) + it("should fail when it is not parseable", () => { + expect(() => config({ MEMORY_DB: "foo" })).toThrow( + new Error("Invalid boolean value: foo") + ) + }) + it("should default to false when the string is empty", () => { + const result = config({ MEMORY_DB: "" }) - expect(result).toHaveProperty("memory_db", false) - }) - }) + expect(result).toHaveProperty("memory_db", false) + }) + }) }) diff --git a/server/src/lib/__tests__/get-html-from-drift-file.ts b/server/src/lib/__tests__/get-html-from-drift-file.ts new file mode 100644 index 00000000..461b6f9b --- /dev/null +++ b/server/src/lib/__tests__/get-html-from-drift-file.ts @@ -0,0 +1,17 @@ +import getHtmlFromFile from "@lib/get-html-from-drift-file" + +describe("get-html-from-drift-file", () => { + it("should not wrap markdown in code blocks", () => { + const markdown = `## My Markdown` + const html = getHtmlFromFile({ content: markdown, title: "my-markdown.md" }) + // the string is

My Markdown

, + // but we dont wan't to be too strict in case markup changes + expect(html).toMatch(/

<\/h2>/) + }) + + it("should wrap code in code blocks", () => { + const code = `const foo = "bar"` + const html = getHtmlFromFile({ content: code, title: "my-code.js" }) + expect(html).toMatch(/
/)
+	})
+})
diff --git a/server/src/lib/__tests__/middleware/is-admin.ts b/server/src/lib/__tests__/middleware/is-admin.ts
new file mode 100644
index 00000000..ae585132
--- /dev/null
+++ b/server/src/lib/__tests__/middleware/is-admin.ts
@@ -0,0 +1,50 @@
+// import * as request from 'supertest'
+// import { app } from '../../../app'
+import { NextFunction, Response } from "express"
+import isAdmin from "@lib/middleware/is-admin"
+import { UserJwtRequest } from "@lib/middleware/jwt"
+
+describe("is-admin middlware", () => {
+	let mockRequest: Partial
+	let mockResponse: Partial
+	let nextFunction: NextFunction = jest.fn()
+
+	beforeEach(() => {
+		mockRequest = {}
+		mockResponse = {
+			sendStatus: jest.fn()
+		}
+	})
+
+	it("should return 401 if no authorization header", async () => {
+		const res = mockResponse as Response
+		isAdmin(mockRequest as UserJwtRequest, res, nextFunction)
+		expect(res.sendStatus).toHaveBeenCalledWith(401)
+	})
+
+	it("should return 401 if no token is supplied", async () => {
+		const req = mockRequest as UserJwtRequest
+		req.headers = {
+			authorization: "Bearer"
+		}
+		isAdmin(req, mockResponse as Response, nextFunction)
+		expect(mockResponse.sendStatus).toBeCalledWith(401)
+	})
+
+	it("should return 404 if config.enable_admin is false", async () => {
+		jest.mock("../../config", () => ({
+			enable_admin: false
+		}))
+
+		const req = mockRequest as UserJwtRequest
+		req.headers = {
+			authorization: "Bearer 123"
+		}
+		isAdmin(req, mockResponse as Response, nextFunction)
+		expect(mockResponse.sendStatus).toBeCalledWith(404)
+	})
+
+	// TODO: 403 if !isAdmin
+	// Verify it calls next() if admin
+	// Requires mocking config.enable_admin
+})
diff --git a/server/src/lib/config.ts b/server/src/lib/config.ts
index 9776f543..e4015357 100644
--- a/server/src/lib/config.ts
+++ b/server/src/lib/config.ts
@@ -6,12 +6,12 @@ type Config = {
 	memory_db: boolean
 	enable_admin: boolean
 	secret_key: string
-	registration_password: string,
-	welcome_content: string | undefined,
-	welcome_title: string | undefined,
+	registration_password: string
+	welcome_content: string | undefined
+	welcome_title: string | undefined
 }
 
-type EnvironmentValue = string | undefined;
+type EnvironmentValue = string | undefined
 type Environment = { [key: string]: EnvironmentValue }
 
 export const config = (env: Environment): Config => {
@@ -34,7 +34,10 @@ export const config = (env: Environment): Config => {
 		return str
 	}
 
-	const defaultIfUndefined = (str: EnvironmentValue, defaultValue: string): string => {
+	const defaultIfUndefined = (
+		str: EnvironmentValue,
+		defaultValue: string
+	): string => {
 		if (str === undefined) {
 			return defaultValue
 		}
@@ -52,11 +55,15 @@ export const config = (env: Environment): Config => {
 		}
 	}
 
-	const is_production = env.NODE_ENV === "production";
+	const is_production = env.NODE_ENV === "production"
 
-	const developmentDefault = (str: EnvironmentValue, name: string, defaultValue: string): string => {
-		if (is_production) return throwIfUndefined(str, name);
-		return defaultIfUndefined(str, defaultValue);
+	const developmentDefault = (
+		str: EnvironmentValue,
+		name: string,
+		defaultValue: string
+	): string => {
+		if (is_production) return throwIfUndefined(str, name)
+		return defaultIfUndefined(str, defaultValue)
 	}
 
 	validNodeEnvs(env.NODE_ENV)
@@ -72,7 +79,6 @@ export const config = (env: Environment): Config => {
 		registration_password: env.REGISTRATION_PASSWORD ?? "",
 		welcome_content: env.WELCOME_CONTENT,
 		welcome_title: env.WELCOME_TITLE
-
 	}
 	return config
 }
diff --git a/server/src/lib/get-html-from-drift-file.ts b/server/src/lib/get-html-from-drift-file.ts
index 4e796021..bbee1ea0 100644
--- a/server/src/lib/get-html-from-drift-file.ts
+++ b/server/src/lib/get-html-from-drift-file.ts
@@ -5,37 +5,35 @@ import { File } from "@lib/models/File"
  * returns rendered HTML from a  Drift file
  */
 function getHtmlFromFile({ content, title }: Pick) {
-    const renderAsMarkdown = [
-        "markdown",
-        "md",
-        "mdown",
-        "mkdn",
-        "mkd",
-        "mdwn",
-        "mdtxt",
-        "mdtext",
-        "text",
-        ""
-    ]
-    const fileType = () => {
-        const pathParts = title.split(".")
-        const language = pathParts.length > 1 ? pathParts[pathParts.length - 1] : ""
-        return language
-    }
-    const type = fileType()
-    let contentToRender: string = content || ""
+	const renderAsMarkdown = [
+		"markdown",
+		"md",
+		"mdown",
+		"mkdn",
+		"mkd",
+		"mdwn",
+		"mdtxt",
+		"mdtext",
+		"text",
+		""
+	]
+	const fileType = () => {
+		const pathParts = title.split(".")
+		const language = pathParts.length > 1 ? pathParts[pathParts.length - 1] : ""
+		return language
+	}
+	const type = fileType()
+	let contentToRender: string = content || ""
 
-    if (!renderAsMarkdown.includes(type)) {
-        contentToRender = `~~~${type}
+	if (!renderAsMarkdown.includes(type)) {
+		contentToRender = `~~~${type}
 ${content}
 ~~~`
-    } else {
-        contentToRender = "\n" + content
-    }
-    console.log(contentToRender.slice(0, 50))
-    const html = markdown(contentToRender)
-    return html
+	} else {
+		contentToRender = "\n" + content
+	}
+	const html = markdown(contentToRender)
+	return html
 }
 
-
-export default getHtmlFromFile
\ No newline at end of file
+export default getHtmlFromFile
diff --git a/server/src/lib/middleware/is-admin.ts b/server/src/lib/middleware/is-admin.ts
index 81efc7ca..3d50ad70 100644
--- a/server/src/lib/middleware/is-admin.ts
+++ b/server/src/lib/middleware/is-admin.ts
@@ -11,16 +11,20 @@ export interface UserJwtRequest extends Request {
 	user?: User
 }
 
-export default function authenticateToken(
+export default function isAdmin(
 	req: UserJwtRequest,
 	res: Response,
 	next: NextFunction
 ) {
+	if (!req.headers?.authorization) {
+		return res.sendStatus(401)
+	}
+
 	const authHeader = req.headers["authorization"]
 	const token = authHeader && authHeader.split(" ")[1]
-	if (token == null) return res.sendStatus(401)
+	if (!token) return res.sendStatus(401)
+	console.log(config)
 	if (!config.enable_admin) return res.sendStatus(404)
-
 	jwt.verify(token, config.jwt_secret, async (err: any, user: any) => {
 		if (err) return res.sendStatus(403)
 		const userObj = await UserModel.findByPk(user.id, {
diff --git a/server/src/routes/posts.ts b/server/src/routes/posts.ts
index 7795a1e1..d5869017 100644
--- a/server/src/routes/posts.ts
+++ b/server/src/routes/posts.ts
@@ -357,4 +357,3 @@ posts.delete("/:id", jwt, async (req: UserJwtRequest, res, next) => {
 		next(e)
 	}
 })
-