diff --git a/src/node/http.ts b/src/node/http.ts index eb8c91f9..e0309542 100644 --- a/src/node/http.ts +++ b/src/node/http.ts @@ -8,7 +8,7 @@ import { normalize, Options } from "../common/util" import { AuthType, DefaultedArgs } from "./cli" import { commit, rootPath } from "./constants" import { Heart } from "./heart" -import { hash } from "./util" +import { isHashMatch } from "./util" declare global { // eslint-disable-next-line @typescript-eslint/no-namespace @@ -67,7 +67,7 @@ export const authenticated = (req: express.Request): boolean => { req.cookies.key && (req.args["hashed-password"] ? safeCompare(req.cookies.key, req.args["hashed-password"]) - : req.args.password && safeCompare(req.cookies.key, hash(req.args.password))) + : req.args.password && isHashMatch(req.args.password, req.cookies.key)) ) default: throw new Error(`Unsupported auth type ${req.args.auth}`) diff --git a/src/node/routes/login.ts b/src/node/routes/login.ts index 017b8830..59444fd2 100644 --- a/src/node/routes/login.ts +++ b/src/node/routes/login.ts @@ -5,7 +5,7 @@ import * as path from "path" import safeCompare from "safe-compare" import { rootPath } from "../constants" import { authenticated, getCookieDomain, redirect, replaceTemplates } from "../http" -import { hash, humanPath } from "../util" +import { hash, hashLegacy, humanPath, isHashLegacyMatch } from "../util" export enum Cookie { Key = "key", @@ -74,12 +74,13 @@ router.post("/", async (req, res) => { if ( req.args["hashed-password"] - ? safeCompare(hash(req.body.password), req.args["hashed-password"]) + ? isHashLegacyMatch(req.body.password, req.args["hashed-password"]) : req.args.password && safeCompare(req.body.password, req.args.password) ) { + const hashedPassword = req.args["hashed-password"] ? hashLegacy(req.body.password) : hash(req.body.password) // The hash does not add any actual security but we do it for // obfuscation purposes (and as a side effect it handles escaping). - res.cookie(Cookie.Key, hash(req.body.password), { + res.cookie(Cookie.Key, hashedPassword, { domain: getCookieDomain(req.headers.host || "", req.args["proxy-domain"]), path: req.body.base || "/", sameSite: "lax", diff --git a/src/node/util.ts b/src/node/util.ts index e456f809..495c3389 100644 --- a/src/node/util.ts +++ b/src/node/util.ts @@ -8,6 +8,7 @@ import * as os from "os" import * as path from "path" import * as util from "util" import xdgBasedir from "xdg-basedir" +import safeCompare from "safe-compare" export interface Paths { data: string @@ -116,8 +117,38 @@ export const generatePassword = async (length = 24): Promise => { return buffer.toString("hex").substring(0, length) } -export const hash = (str: string): string => { - return bcrypt.hashSync(str, 10) +/** + * Used to hash the password. + */ +export const hash = (password: string): string => { + return bcrypt.hashSync(password, 10) +} + +/** + * Used to verify if the password matches the hash + */ +export const isHashMatch = (password: string, hash: string) => { + return bcrypt.compareSync(password, hash) +} + +/** + * Used to hash the password using the sha256 + * algorithm. We only use this to for checking + * the hashed-password set in the config. + * + * Kept for legacy reasons. + */ +export const hashLegacy = (str: string): string => { + return crypto.createHash("sha256").update(str).digest("hex") +} + +/** + * Used to check if the password matches the hash using + * the hashLegacy function + */ +export const isHashLegacyMatch = (password: string, hashPassword: string) => { + const hashedWithLegacy = hashLegacy(password) + return safeCompare(hashedWithLegacy, hashPassword) } const mimeTypes: { [key: string]: string } = { diff --git a/test/unit/node/util.test.ts b/test/unit/node/util.test.ts index 0b085f09..c95994c7 100644 --- a/test/unit/node/util.test.ts +++ b/test/unit/node/util.test.ts @@ -1,4 +1,4 @@ -import { hash } from "../../../src/node/util" +import { hash, isHashMatch, hashLegacy, isHashLegacyMatch } from "../../../src/node/util" describe("getEnvPaths", () => { describe("on darwin", () => { @@ -155,3 +155,37 @@ describe("hash", () => { expect(hashed).not.toBe(plainTextPassword) }) }) + +describe("isHashMatch", () => { + it("should return true if the password matches the hash", () => { + const password = "password123" + const _hash = hash(password) + expect(isHashMatch(password, _hash)).toBe(true) + }) + it("should return false if the password does not match the hash", () => { + const password = "password123" + const _hash = hash(password) + expect(isHashMatch("otherPassword123", _hash)).toBe(false) + }) +}) + +describe("hashLegacy", () => { + it("should return a hash of the string passed in", () => { + const plainTextPassword = "mySecretPassword123" + const hashed = hashLegacy(plainTextPassword) + expect(hashed).not.toBe(plainTextPassword) + }) +}) + +describe("isHashLegacyMatchh", () => { + it("should return true if is match", () => { + const password = "password123" + const _hash = hashLegacy(password) + expect(isHashLegacyMatch(password, _hash)).toBe(true) + }) + it("should return false if is match", () => { + const password = "password123" + const _hash = hashLegacy(password) + expect(isHashLegacyMatch("otherPassword123", _hash)).toBe(false) + }) +})