From 409b473c82e12cebc59bf6b1e34b1f432c46d5e6 Mon Sep 17 00:00:00 2001
From: Joe Previte <jjprevite@gmail.com>
Date: Wed, 2 Jun 2021 16:10:07 -0700
Subject: [PATCH] refactor: rewrite password logic at /login

---
 src/node/routes/login.ts | 44 ++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/src/node/routes/login.ts b/src/node/routes/login.ts
index eb9a775a..b09585ca 100644
--- a/src/node/routes/login.ts
+++ b/src/node/routes/login.ts
@@ -5,7 +5,15 @@ import * as path from "path"
 import safeCompare from "safe-compare"
 import { rootPath } from "../constants"
 import { authenticated, getCookieDomain, redirect, replaceTemplates } from "../http"
-import { hash, hashLegacy, humanPath, isHashLegacyMatch, isHashMatch } from "../util"
+import {
+  getPasswordMethod,
+  handlePasswordValidation,
+  hash,
+  hashLegacy,
+  humanPath,
+  isHashLegacyMatch,
+  isHashMatch,
+} from "../util"
 
 export enum Cookie {
   Key = "key",
@@ -62,36 +70,28 @@ router.get("/", async (req, res) => {
 })
 
 router.post("/", async (req, res) => {
+  const password = req.body.password
+  const hashedPasswordFromArgs = req.args["hashed-password"]
+
   try {
     // Check to see if they exceeded their login attempts
     if (!limiter.canTry()) {
       throw new Error("Login rate limited!")
     }
 
-    if (!req.body.password) {
+    if (!password) {
       throw new Error("Missing password")
     }
 
-    // this logic below is flawed
-    const theHash = await hash(req.body.password)
-    const hashedPassword = req.args["hashed-password"] || ""
-    const match = await isHashMatch(req.body.password, hashedPassword)
-    // console.log(`The actual hash: ${theHash}`)
-    // console.log(`hashed-password from config: ${hashedPassword}`)
-    // console.log(theHash, hashedPassword)
-    console.log(`is it a match??? ${match}`)
-    if (
-      req.args["hashed-password"]
-        ? isHashLegacyMatch(req.body.password, req.args["hashed-password"])
-        : req.args.password && safeCompare(req.body.password, req.args.password)
-    ) {
-      // NOTE@jsjoeio:
-      // We store the hashed password as a cookie. In order to be backwards-comptabile for the folks
-      // using sha256 (the original hashing algorithm), we need to check the hashed-password in the req.args
-      // TODO all of this logic should be cleaned up honestly. The current implementation only checks for a hashed-password
-      // but doesn't check which algorithm they are using.
-      console.log(`What is this? ${req.args["hashed-password"]}`, Boolean(req.args["hashed-password"]))
-      const hashedPassword = req.args["hashed-password"] ? hashLegacy(req.body.password) : await hash(req.body.password)
+    const passwordMethod = getPasswordMethod(hashedPasswordFromArgs)
+    const { isPasswordValid, hashedPassword } = await handlePasswordValidation({
+      passwordMethod,
+      hashedPasswordFromArgs,
+      passwordFromRequestBody: password,
+      passwordFromArgs: req.args.password,
+    })
+
+    if (isPasswordValid) {
       // 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, hashedPassword, {
-- 
GitLab