Skip to content

Commit

Permalink
refactor: migrate from argon2 -> @node-rs/argon2 (coder#4733)
Browse files Browse the repository at this point in the history
* 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`
  • Loading branch information
jsjoeio committed Jan 18, 2022
1 parent 2752d95 commit 723469a
Show file tree
Hide file tree
Showing 10 changed files with 1,165 additions and 2,357 deletions.
4 changes: 0 additions & 4 deletions ci/build/build-standalone-release.sh
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
#!/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: 0 additions & 3 deletions ci/build/npm-postinstall.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ 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 @@ -82,7 +82,7 @@
},
"dependencies": {
"@coder/logger": "1.1.16",
"argon2": "^0.28.0",
"@node-rs/argon2": "^1.0.5",
"compression": "^1.7.4",
"cookie-parser": "^1.4.5",
"env-paths": "^2.2.0",
Expand Down
9 changes: 5 additions & 4 deletions src/node/util.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { logger } from "@coder/logger"
import * as argon2 from "argon2"
import * as argon2 from "@node-rs/argon2"
import * as cp from "child_process"
import * as crypto from "crypto"
import envPaths from "env-paths"
Expand Down Expand Up @@ -58,10 +58,10 @@ export const paths = getEnvPaths()
* On MacOS this function gets the standard XDG directories instead of using the native macOS
* ones. Most CLIs do this as in practice only GUI apps use the standard macOS directories.
*/
export function getEnvPaths(): Paths {
export function getEnvPaths(platform = process.platform): Paths {
const paths = envPaths("code-server", { suffix: "" })
const append = (p: string): string => path.join(p, "code-server")
switch (process.platform) {
switch (platform) {
case "darwin":
return {
// envPaths uses native directories so force Darwin to use the XDG spec
Expand Down Expand Up @@ -175,7 +175,8 @@ export const isHashMatch = async (password: string, hash: string) => {
try {
return await argon2.verify(hash, password)
} catch (error: any) {
throw new Error(error)
logger.error(error)
return false
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/e2e/extensions/test-extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@
"typescript": "^4.0.5"
},
"scripts": {
"build": "tsc extension.ts"
"build": "tsc"
}
}
6 changes: 4 additions & 2 deletions test/e2e/extensions/test-extension/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
"module": "commonjs",
"outDir": ".",
"strict": true,
"baseUrl": "./"
"baseUrl": "./",
"skipLibCheck": true
},
"include": ["./extension.ts"]
"include": ["./extension.ts"],
"exclude": ["node_modules"]
}
2 changes: 0 additions & 2 deletions test/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
"@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 @@ -20,7 +19,6 @@
},
"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
73 changes: 9 additions & 64 deletions test/unit/node/util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,6 @@ import * as util from "../../../src/node/util"

describe("getEnvPaths", () => {
describe("on darwin", () => {
let ORIGINAL_PLATFORM = ""

beforeAll(() => {
ORIGINAL_PLATFORM = process.platform

Object.defineProperty(process, "platform", {
value: "darwin",
})
})

beforeEach(() => {
jest.resetModules()
jest.mock("env-paths", () => {
Expand All @@ -27,23 +17,14 @@ describe("getEnvPaths", () => {
})
})
})

afterAll(() => {
// Restore old platform

Object.defineProperty(process, "platform", {
value: ORIGINAL_PLATFORM,
})
})

it("should return the env paths using xdgBasedir", () => {
jest.mock("xdg-basedir", () => ({
data: "/home/usr/.local/share",
config: "/home/usr/.config",
runtime: "/tmp/runtime",
}))
const getEnvPaths = require("../../../src/node/util").getEnvPaths
const envPaths = getEnvPaths()
const envPaths = getEnvPaths("darwin")

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

expect(envPaths.data).toEqual("/home/envPath/.local/share")
expect(envPaths.config).toEqual("/home/envPath/.config")
expect(envPaths.runtime).toEqual("/tmp/envPath/runtime")
})
})
describe("on win32", () => {
let ORIGINAL_PLATFORM = ""

beforeAll(() => {
ORIGINAL_PLATFORM = process.platform

Object.defineProperty(process, "platform", {
value: "win32",
})
})

beforeEach(() => {
jest.resetModules()
jest.mock("env-paths", () => {
Expand All @@ -82,34 +53,16 @@ describe("getEnvPaths", () => {
})
})

afterAll(() => {
// Restore old platform

Object.defineProperty(process, "platform", {
value: ORIGINAL_PLATFORM,
})
})

it("should return the env paths using envPaths", () => {
const getEnvPaths = require("../../../src/node/util").getEnvPaths
const envPaths = getEnvPaths()
const envPaths = getEnvPaths("win32")

expect(envPaths.data).toEqual("/windows/envPath/.local/share")
expect(envPaths.config).toEqual("/windows/envPath/.config")
expect(envPaths.runtime).toEqual("/tmp/envPath/runtime")
})
})
describe("on other platforms", () => {
let ORIGINAL_PLATFORM = ""

beforeAll(() => {
ORIGINAL_PLATFORM = process.platform

Object.defineProperty(process, "platform", {
value: "linux",
})
})

beforeEach(() => {
jest.resetModules()
jest.mock("env-paths", () => {
Expand All @@ -121,20 +74,12 @@ describe("getEnvPaths", () => {
})
})

afterAll(() => {
// Restore old platform

Object.defineProperty(process, "platform", {
value: ORIGINAL_PLATFORM,
})
})

it("should return the runtime using xdgBasedir if it exists", () => {
jest.mock("xdg-basedir", () => ({
runtime: "/tmp/runtime",
}))
const getEnvPaths = require("../../../src/node/util").getEnvPaths
const envPaths = getEnvPaths()
const envPaths = getEnvPaths("linux")

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

expect(envPaths.data).toEqual("/linux/envPath/.local/share")
expect(envPaths.config).toEqual("/linux/envPath/.config")
Expand Down Expand Up @@ -192,16 +137,16 @@ describe("isHashMatch", () => {
const actual = await util.isHashMatch(password, _hash)
expect(actual).toBe(false)
})
it("should return false and not throw an error if the hash doesn't start with a $", async () => {
it("should return false 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 reject the promise and throw if error", async () => {
it("should return false if the password and hash don't match", async () => {
const password = "hellowpasssword"
const _hash = "$ar2i"
expect(async () => await util.isHashMatch(password, _hash)).rejects.toThrow()
const actual = await util.isHashMatch(password, _hash)
expect(actual).toBe(false)
})
})

Expand Down
Loading

0 comments on commit 723469a

Please sign in to comment.