Skip to content

Commit

Permalink
revert: @node-rs/argon2 -> node-argon2 (coder#4829)
Browse files Browse the repository at this point in the history
* revert: partial revert of 723469a

This reverts part of the changes introduced in refactor: migrate from argon2 ->
@node-rs/argon2 (coder#4733)

Switching to @node-rs/argon2 introduced bugs that we couldn't solve due to
limitations in npm.

see here
coder#4804 (comment)
  • Loading branch information
jsjoeio authored and TinLe committed Apr 23, 2022
1 parent 22a79cb commit 25e032b
Show file tree
Hide file tree
Showing 8 changed files with 369 additions and 102 deletions.
4 changes: 4 additions & 0 deletions ci/build/build-standalone-release.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
#!/usr/bin/env bash
set -euo pipefail

# This is due to an upstream issue with RHEL7/CentOS 7 comptability with node-argon2
# See: https://github.com/cdr/code-server/pull/3422#pullrequestreview-677765057
export npm_config_build_from_source=true

main() {
cd "$(dirname "${0}")/../.."

Expand Down
3 changes: 3 additions & 0 deletions ci/build/npm-postinstall.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ detect_arch() {
}

ARCH="${NPM_CONFIG_ARCH:-$(detect_arch)}"
# This is due to an upstream issue with RHEL7/CentOS 7 comptability with node-argon2
# See: https://github.com/cdr/code-server/pull/3422#pullrequestreview-677765057
export npm_config_build_from_source=true

main() {
# Grabs the major version of node from $npm_config_user_agent which looks like
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
},
"dependencies": {
"@coder/logger": "1.1.16",
"@node-rs/argon2": "^1.0.5",
"argon2": "^0.28.0",
"compression": "^1.7.4",
"cookie-parser": "^1.4.5",
"env-paths": "^2.2.0",
Expand Down
10 changes: 2 additions & 8 deletions src/node/util.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { logger } from "@coder/logger"
import * as argon2 from "@node-rs/argon2"
import * as argon2 from "argon2"
import * as cp from "child_process"
import * as crypto from "crypto"
import envPaths from "env-paths"
Expand Down Expand Up @@ -167,12 +166,7 @@ export const isHashMatch = async (password: string, hash: string) => {
if (password === "" || hash === "" || !hash.startsWith("$")) {
return false
}
try {
return await argon2.verify(hash, password)
} catch (error: any) {
logger.error(error)
return false
}
return await argon2.verify(hash, password)
}

/**
Expand Down
2 changes: 2 additions & 0 deletions test/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"@types/node-fetch": "^2.5.8",
"@types/supertest": "^2.0.11",
"@types/wtfnode": "^0.7.0",
"argon2": "^0.28.0",
"jest": "^27.3.1",
"jest-fetch-mock": "^3.0.3",
"jsdom": "^16.4.0",
Expand All @@ -19,6 +20,7 @@
},
"resolutions": {
"ansi-regex": "^5.0.1",
"argon2/@mapbox/node-pre-gyp/tar": "^6.1.9",
"set-value": "^4.0.1",
"tmpl": "^1.0.5",
"path-parse": "^1.0.7",
Expand Down
8 changes: 4 additions & 4 deletions test/unit/node/util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,16 +141,16 @@ describe("isHashMatch", () => {
const actual = await util.isHashMatch(password, _hash)
expect(actual).toBe(false)
})
it("should return false if the hash doesn't start with a $", async () => {
it("should return false and not throw an error if the hash doesn't start with a $", async () => {
const password = "hellowpasssword"
const _hash = "n2i$v=19$m=4096,t=3,p=1$EAoczTxVki21JDfIZpTUxg$rkXgyrW4RDGoDYrxBFD4H2DlSMEhP4h+Api1hXnGnFY"
expect(async () => await util.isHashMatch(password, _hash)).not.toThrow()
expect(await util.isHashMatch(password, _hash)).toBe(false)
})
it("should return false if the password and hash don't match", async () => {
it("should reject the promise and throw if error", async () => {
const password = "hellowpasssword"
const _hash = "$ar2i"
const actual = await util.isHashMatch(password, _hash)
expect(actual).toBe(false)
expect(async () => await util.isHashMatch(password, _hash)).rejects.toThrow()
})
})

Expand Down
Loading

0 comments on commit 25e032b

Please sign in to comment.