Skip to content

Commit

Permalink
Revert "refactor: migrate from argon2 -> @node-rs/argon2 (#4733)"
Browse files Browse the repository at this point in the history
This reverts commit 723469a.
  • Loading branch information
jsjoeio committed Feb 4, 2022
1 parent 00224fa commit 3c636bd
Show file tree
Hide file tree
Showing 8 changed files with 242 additions and 108 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.4",
"compression": "^1.7.4",
"cookie-parser": "^1.4.5",
"env-paths": "^2.2.0",
Expand Down
10 changes: 4 additions & 6 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 @@ -58,10 +57,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(platform = process.platform): Paths {
export function getEnvPaths(): Paths {
const paths = envPaths("code-server", { suffix: "" })
const append = (p: string): string => path.join(p, "code-server")
switch (platform) {
switch (process.platform) {
case "darwin":
return {
// envPaths uses native directories so force Darwin to use the XDG spec
Expand Down Expand Up @@ -170,8 +169,7 @@ export const isHashMatch = async (password: string, hash: string) => {
try {
return await argon2.verify(hash, password)
} catch (error: any) {
logger.error(error)
return false
throw new Error(error)
}
}

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"
"build": "tsc extension.ts"
}
}
6 changes: 2 additions & 4 deletions test/e2e/extensions/test-extension/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@
"module": "commonjs",
"outDir": ".",
"strict": true,
"baseUrl": "./",
"skipLibCheck": true
"baseUrl": "./"
},
"include": ["./extension.ts"],
"exclude": ["node_modules"]
"include": ["./extension.ts"]
}
73 changes: 64 additions & 9 deletions test/unit/node/util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ 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 @@ -17,14 +27,23 @@ 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("darwin")
const envPaths = getEnvPaths()

expect(envPaths.data).toEqual("/home/usr/.local/share/code-server")
expect(envPaths.config).toEqual("/home/usr/.config/code-server")
Expand All @@ -34,14 +53,24 @@ 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("darwin")
const envPaths = getEnvPaths()

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 @@ -53,16 +82,34 @@ 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("win32")
const envPaths = getEnvPaths()

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 @@ -74,12 +121,20 @@ 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("linux")
const envPaths = getEnvPaths()

expect(envPaths.data).toEqual("/linux/envPath/.local/share")
expect(envPaths.config).toEqual("/linux/envPath/.config")
Expand All @@ -89,7 +144,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("linux")
const envPaths = getEnvPaths()

expect(envPaths.data).toEqual("/linux/envPath/.local/share")
expect(envPaths.config).toEqual("/linux/envPath/.config")
Expand Down Expand Up @@ -141,16 +196,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 3c636bd

Please sign in to comment.