Skip to content

Commit 723469a

Browse files
jsjoeiojsjoeio
authored
refactor: migrate from argon2 -> @node-rs/argon2 (#4733)
* chore(deps): replace argon2 w/@node-rs/argon2 * refactor: clean up hashPassword functions * refactor(util): pass in process.platform * fix: use correct settings for test-extension Before, it was running into errors with an @types package. Now, we're correctly running `tsc` so it picks up our `tsconfig.json` and we're telling TypeScript to not typecheck our lib and exclude `node_modules`
1 parent 2752d95 commit 723469a

File tree

10 files changed

+1165
-2357
lines changed

10 files changed

+1165
-2357
lines changed

‎ci/build/build-standalone-release.sh

-4
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
#!/usr/bin/env bash
22
set -euo pipefail
33

4-
# This is due to an upstream issue with RHEL7/CentOS 7 comptability with node-argon2
5-
# See: https://github.com/cdr/code-server/pull/3422#pullrequestreview-677765057
6-
export npm_config_build_from_source=true
7-
84
main() {
95
cd "$(dirname "${0}")/../.."
106

‎ci/build/npm-postinstall.sh

-3
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,6 @@ detect_arch() {
1818
}
1919

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

2522
main() {
2623
# Grabs the major version of node from $npm_config_user_agent which looks like

‎package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@
8282
},
8383
"dependencies": {
8484
"@coder/logger": "1.1.16",
85-
"argon2": "^0.28.0",
85+
"@node-rs/argon2": "^1.0.5",
8686
"compression": "^1.7.4",
8787
"cookie-parser": "^1.4.5",
8888
"env-paths": "^2.2.0",

‎src/node/util.ts

+5-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { logger } from "@coder/logger"
2-
import * as argon2 from "argon2"
2+
import * as argon2 from "@node-rs/argon2"
33
import * as cp from "child_process"
44
import * as crypto from "crypto"
55
import envPaths from "env-paths"
@@ -58,10 +58,10 @@ export const paths = getEnvPaths()
5858
* On MacOS this function gets the standard XDG directories instead of using the native macOS
5959
* ones. Most CLIs do this as in practice only GUI apps use the standard macOS directories.
6060
*/
61-
export function getEnvPaths(): Paths {
61+
export function getEnvPaths(platform = process.platform): Paths {
6262
const paths = envPaths("code-server", { suffix: "" })
6363
const append = (p: string): string => path.join(p, "code-server")
64-
switch (process.platform) {
64+
switch (platform) {
6565
case "darwin":
6666
return {
6767
// envPaths uses native directories so force Darwin to use the XDG spec
@@ -175,7 +175,8 @@ export const isHashMatch = async (password: string, hash: string) => {
175175
try {
176176
return await argon2.verify(hash, password)
177177
} catch (error: any) {
178-
throw new Error(error)
178+
logger.error(error)
179+
return false
179180
}
180181
}
181182

‎test/e2e/extensions/test-extension/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,6 @@
2424
"typescript": "^4.0.5"
2525
},
2626
"scripts": {
27-
"build": "tsc extension.ts"
27+
"build": "tsc"
2828
}
2929
}

‎test/e2e/extensions/test-extension/tsconfig.json

+4-2
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
"module": "commonjs",
55
"outDir": ".",
66
"strict": true,
7-
"baseUrl": "./"
7+
"baseUrl": "./",
8+
"skipLibCheck": true
89
},
9-
"include": ["./extension.ts"]
10+
"include": ["./extension.ts"],
11+
"exclude": ["node_modules"]
1012
}

‎test/package.json

-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
"@types/node-fetch": "^2.5.8",
99
"@types/supertest": "^2.0.11",
1010
"@types/wtfnode": "^0.7.0",
11-
"argon2": "^0.28.0",
1211
"jest": "^27.3.1",
1312
"jest-fetch-mock": "^3.0.3",
1413
"jsdom": "^16.4.0",
@@ -20,7 +19,6 @@
2019
},
2120
"resolutions": {
2221
"ansi-regex": "^5.0.1",
23-
"argon2/@mapbox/node-pre-gyp/tar": "^6.1.9",
2422
"set-value": "^4.0.1",
2523
"tmpl": "^1.0.5",
2624
"path-parse": "^1.0.7",

‎test/unit/node/util.test.ts

+9-64
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,6 @@ import * as util from "../../../src/node/util"
77

88
describe("getEnvPaths", () => {
99
describe("on darwin", () => {
10-
let ORIGINAL_PLATFORM = ""
11-
12-
beforeAll(() => {
13-
ORIGINAL_PLATFORM = process.platform
14-
15-
Object.defineProperty(process, "platform", {
16-
value: "darwin",
17-
})
18-
})
19-
2010
beforeEach(() => {
2111
jest.resetModules()
2212
jest.mock("env-paths", () => {
@@ -27,23 +17,14 @@ describe("getEnvPaths", () => {
2717
})
2818
})
2919
})
30-
31-
afterAll(() => {
32-
// Restore old platform
33-
34-
Object.defineProperty(process, "platform", {
35-
value: ORIGINAL_PLATFORM,
36-
})
37-
})
38-
3920
it("should return the env paths using xdgBasedir", () => {
4021
jest.mock("xdg-basedir", () => ({
4122
data: "/home/usr/.local/share",
4223
config: "/home/usr/.config",
4324
runtime: "/tmp/runtime",
4425
}))
4526
const getEnvPaths = require("../../../src/node/util").getEnvPaths
46-
const envPaths = getEnvPaths()
27+
const envPaths = getEnvPaths("darwin")
4728

4829
expect(envPaths.data).toEqual("/home/usr/.local/share/code-server")
4930
expect(envPaths.config).toEqual("/home/usr/.config/code-server")
@@ -53,24 +34,14 @@ describe("getEnvPaths", () => {
5334
it("should return the env paths using envPaths when xdgBasedir is undefined", () => {
5435
jest.mock("xdg-basedir", () => ({}))
5536
const getEnvPaths = require("../../../src/node/util").getEnvPaths
56-
const envPaths = getEnvPaths()
37+
const envPaths = getEnvPaths("darwin")
5738

5839
expect(envPaths.data).toEqual("/home/envPath/.local/share")
5940
expect(envPaths.config).toEqual("/home/envPath/.config")
6041
expect(envPaths.runtime).toEqual("/tmp/envPath/runtime")
6142
})
6243
})
6344
describe("on win32", () => {
64-
let ORIGINAL_PLATFORM = ""
65-
66-
beforeAll(() => {
67-
ORIGINAL_PLATFORM = process.platform
68-
69-
Object.defineProperty(process, "platform", {
70-
value: "win32",
71-
})
72-
})
73-
7445
beforeEach(() => {
7546
jest.resetModules()
7647
jest.mock("env-paths", () => {
@@ -82,34 +53,16 @@ describe("getEnvPaths", () => {
8253
})
8354
})
8455

85-
afterAll(() => {
86-
// Restore old platform
87-
88-
Object.defineProperty(process, "platform", {
89-
value: ORIGINAL_PLATFORM,
90-
})
91-
})
92-
9356
it("should return the env paths using envPaths", () => {
9457
const getEnvPaths = require("../../../src/node/util").getEnvPaths
95-
const envPaths = getEnvPaths()
58+
const envPaths = getEnvPaths("win32")
9659

9760
expect(envPaths.data).toEqual("/windows/envPath/.local/share")
9861
expect(envPaths.config).toEqual("/windows/envPath/.config")
9962
expect(envPaths.runtime).toEqual("/tmp/envPath/runtime")
10063
})
10164
})
10265
describe("on other platforms", () => {
103-
let ORIGINAL_PLATFORM = ""
104-
105-
beforeAll(() => {
106-
ORIGINAL_PLATFORM = process.platform
107-
108-
Object.defineProperty(process, "platform", {
109-
value: "linux",
110-
})
111-
})
112-
11366
beforeEach(() => {
11467
jest.resetModules()
11568
jest.mock("env-paths", () => {
@@ -121,20 +74,12 @@ describe("getEnvPaths", () => {
12174
})
12275
})
12376

124-
afterAll(() => {
125-
// Restore old platform
126-
127-
Object.defineProperty(process, "platform", {
128-
value: ORIGINAL_PLATFORM,
129-
})
130-
})
131-
13277
it("should return the runtime using xdgBasedir if it exists", () => {
13378
jest.mock("xdg-basedir", () => ({
13479
runtime: "/tmp/runtime",
13580
}))
13681
const getEnvPaths = require("../../../src/node/util").getEnvPaths
137-
const envPaths = getEnvPaths()
82+
const envPaths = getEnvPaths("linux")
13883

13984
expect(envPaths.data).toEqual("/linux/envPath/.local/share")
14085
expect(envPaths.config).toEqual("/linux/envPath/.config")
@@ -144,7 +89,7 @@ describe("getEnvPaths", () => {
14489
it("should return the env paths using envPaths when xdgBasedir is undefined", () => {
14590
jest.mock("xdg-basedir", () => ({}))
14691
const getEnvPaths = require("../../../src/node/util").getEnvPaths
147-
const envPaths = getEnvPaths()
92+
const envPaths = getEnvPaths("linux")
14893

14994
expect(envPaths.data).toEqual("/linux/envPath/.local/share")
15095
expect(envPaths.config).toEqual("/linux/envPath/.config")
@@ -192,16 +137,16 @@ describe("isHashMatch", () => {
192137
const actual = await util.isHashMatch(password, _hash)
193138
expect(actual).toBe(false)
194139
})
195-
it("should return false and not throw an error if the hash doesn't start with a $", async () => {
140+
it("should return false if the hash doesn't start with a $", async () => {
196141
const password = "hellowpasssword"
197142
const _hash = "n2i$v=19$m=4096,t=3,p=1$EAoczTxVki21JDfIZpTUxg$rkXgyrW4RDGoDYrxBFD4H2DlSMEhP4h+Api1hXnGnFY"
198-
expect(async () => await util.isHashMatch(password, _hash)).not.toThrow()
199143
expect(await util.isHashMatch(password, _hash)).toBe(false)
200144
})
201-
it("should reject the promise and throw if error", async () => {
145+
it("should return false if the password and hash don't match", async () => {
202146
const password = "hellowpasssword"
203147
const _hash = "$ar2i"
204-
expect(async () => await util.isHashMatch(password, _hash)).rejects.toThrow()
148+
const actual = await util.isHashMatch(password, _hash)
149+
expect(actual).toBe(false)
205150
})
206151
})
207152

0 commit comments

Comments
 (0)