Skip to content

Commit 409b473

Browse files
committed
refactor: rewrite password logic at /login
1 parent a14ea39 commit 409b473

File tree

1 file changed

+22
-22
lines changed

1 file changed

+22
-22
lines changed

src/node/routes/login.ts

+22-22
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,15 @@ import * as path from "path"
55
import safeCompare from "safe-compare"
66
import { rootPath } from "../constants"
77
import { authenticated, getCookieDomain, redirect, replaceTemplates } from "../http"
8-
import { hash, hashLegacy, humanPath, isHashLegacyMatch, isHashMatch } from "../util"
8+
import {
9+
getPasswordMethod,
10+
handlePasswordValidation,
11+
hash,
12+
hashLegacy,
13+
humanPath,
14+
isHashLegacyMatch,
15+
isHashMatch,
16+
} from "../util"
917

1018
export enum Cookie {
1119
Key = "key",
@@ -62,36 +70,28 @@ router.get("/", async (req, res) => {
6270
})
6371

6472
router.post("/", async (req, res) => {
73+
const password = req.body.password
74+
const hashedPasswordFromArgs = req.args["hashed-password"]
75+
6576
try {
6677
// Check to see if they exceeded their login attempts
6778
if (!limiter.canTry()) {
6879
throw new Error("Login rate limited!")
6980
}
7081

71-
if (!req.body.password) {
82+
if (!password) {
7283
throw new Error("Missing password")
7384
}
7485

75-
// this logic below is flawed
76-
const theHash = await hash(req.body.password)
77-
const hashedPassword = req.args["hashed-password"] || ""
78-
const match = await isHashMatch(req.body.password, hashedPassword)
79-
// console.log(`The actual hash: ${theHash}`)
80-
// console.log(`hashed-password from config: ${hashedPassword}`)
81-
// console.log(theHash, hashedPassword)
82-
console.log(`is it a match??? ${match}`)
83-
if (
84-
req.args["hashed-password"]
85-
? isHashLegacyMatch(req.body.password, req.args["hashed-password"])
86-
: req.args.password && safeCompare(req.body.password, req.args.password)
87-
) {
88-
// NOTE@jsjoeio:
89-
// We store the hashed password as a cookie. In order to be backwards-comptabile for the folks
90-
// using sha256 (the original hashing algorithm), we need to check the hashed-password in the req.args
91-
// TODO all of this logic should be cleaned up honestly. The current implementation only checks for a hashed-password
92-
// but doesn't check which algorithm they are using.
93-
console.log(`What is this? ${req.args["hashed-password"]}`, Boolean(req.args["hashed-password"]))
94-
const hashedPassword = req.args["hashed-password"] ? hashLegacy(req.body.password) : await hash(req.body.password)
86+
const passwordMethod = getPasswordMethod(hashedPasswordFromArgs)
87+
const { isPasswordValid, hashedPassword } = await handlePasswordValidation({
88+
passwordMethod,
89+
hashedPasswordFromArgs,
90+
passwordFromRequestBody: password,
91+
passwordFromArgs: req.args.password,
92+
})
93+
94+
if (isPasswordValid) {
9595
// The hash does not add any actual security but we do it for
9696
// obfuscation purposes (and as a side effect it handles escaping).
9797
res.cookie(Cookie.Key, hashedPassword, {

0 commit comments

Comments
 (0)