Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

persist secrets #417

Merged
merged 18 commits into from
Nov 20, 2024
7 changes: 6 additions & 1 deletion src/cli.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,12 @@ function buildYargs(argvInput) {
alias: "d",
type: "string",
description: "a database path, including region",
// required: true,
},
role: {
alias: "r",
type: "string",
description: "a role",
default: "admin",
mwilde345 marked this conversation as resolved.
Show resolved Hide resolved
},
color: {
description:
Expand Down
5 changes: 3 additions & 2 deletions src/config/setup-test-container.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { f, InMemoryWritableStream } from "../../test/helpers.mjs";
import { parseYargs } from "../cli.mjs";
import { makeAccountRequest } from "../lib/account.mjs";
import { makeFaunaRequest } from "../lib/db.mjs";
import { AccountKey, SecretKey } from "../lib/file-util.mjs";
import buildLogger from "../lib/logger.mjs";
import { injectables, setupCommonContainer } from "./setup-container.mjs";

Expand Down Expand Up @@ -62,8 +63,8 @@ export function setupTestContainer() {
),
accountClient: awilix.asFunction(stub()),
oauthClient: awilix.asFunction(stub()),
accountCreds: awilix.asFunction(stub()),
secretCreds: awilix.asFunction(stub()),
accountCreds: awilix.asClass(AccountKey).scoped(),
secretCreds: awilix.asClass(SecretKey).scoped(),
// in tests, let's exit by throwing
errorHandler: awilix.asValue((error, exitCode) => {
error.code = exitCode;
Expand Down
11 changes: 6 additions & 5 deletions src/lib/auth/authNZ.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ export async function authNZMiddleware(argv) {
// Make sure required keys are there so handlers have no issue accesssing/using.
const { profile, database, role, url } = argv;
// TODO: for any args that aren't passed in, get them from configuration files

// TODO: will the account key and DB key be ping'd every time a command is run?
try {
const accountKey = await setAccountKey(profile);
if (database) {
Expand Down Expand Up @@ -62,7 +64,7 @@ export function cleanupSecretsFile() {
}

// TODO: account for env var for account key. if profile isn't defined.
async function setAccountKey(profile) {
export async function setAccountKey(profile) {
// Don't leave hanging db secrets that don't match up to stored account keys
cleanupSecretsFile();
const accountCreds = container.resolve("accountCreds");
Expand All @@ -86,7 +88,7 @@ export function getAccountKey(profile) {
const accountCreds = container.resolve("accountCreds");
try {
const creds = accountCreds.get({ key: profile });
return creds.account_key;
return creds.accountKey;
} catch (e) {
if (e instanceof CredsNotFoundError) {
// Throw InvalidCredsError back up to middleware entrypoint to prompt login
Expand Down Expand Up @@ -185,8 +187,7 @@ export async function checkDBKeyRemote(dbKey, url) {
if (result.status === 401) {
return null;
} else {
throw new Error(
`Error contacting fauna [${result.status}]: ${result.error.code}`,
);
const errorCode = result.body?.error?.code || "internal_error";
throw new Error(`Error contacting fauna [${result.status}]: ${errorCode}`);
}
}
168 changes: 168 additions & 0 deletions test/authNZ.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
import * as awilix from "awilix";
import { expect } from "chai";
import { beforeEach } from "mocha";
import sinon, { stub } from "sinon";

import { run } from "../src/cli.mjs";
import { setupTestContainer as setupContainer } from "../src/config/setup-test-container.mjs";
import { authNZMiddleware, setAccountKey } from "../src/lib/auth/authNZ.mjs";
import { InvalidCredsError } from "../src/lib/misc.mjs";
import { f } from "./helpers.mjs";

describe("authNZMiddleware", function () {
let container;
let fetch;
const validAccessKeyFile =
'{"test-profile": { "accountKey": "valid-account-key", "refreshToken": "valid-refresh-token"}}';
const validSecretKeyFile =
'{"valid-account-key": { "test-db": {"admin": "valid-db-key"}}}';
const mockAccountClient = () => {
return {
whoAmI: stub().resolves(true),
createKey: stub().resolves({ secret: "new-db-key" }),
refreshSession: stub().resolves({
account_key: "new-account-key",
refresh_token: "new-refresh-token",
}),
};
};

beforeEach(() => {
container = setupContainer();
container.register({
accountClient: awilix.asFunction(mockAccountClient).scoped(),
});
fetch = container.resolve("fetch");
});

it("should pass through if authRequired is false", async function () {
const argv = { authRequired: false };
const result = await authNZMiddleware(argv);
expect(fetch.called).to.be.false;
expect(result).to.deep.equal(argv);
});

it("should prompt login if InvalidCredsError is thrown", async function () {
const scope = container.createScope();
const argv = { authRequired: true, profile: "test-profile" };
await run("db list", scope);
const exit = scope.resolve("exit");
const accountCreds = scope.resolve("accountCreds");
const stdout = container.resolve("stdoutStream");
const stderr = container.resolve("stderrStream");

accountCreds.get = stub().throws(new InvalidCredsError());

await authNZMiddleware(argv);
expect(stdout.getWritten()).to.contain(
"To sign in, run:\n\nfauna login --profile test-profile\n",
);
expect(stderr.getWritten()).to.contain(
'The requested profile "test-profile" is not signed in or has expired.\nPlease re-authenticate',
);

expect(exit.calledOnce).to.be.true;
});

it("should refresh session if account key is invalid", async function () {
const argv = { authRequired: true, profile: "test-profile" };
const scope = container.createScope();

await run("db list", scope);
const accountCreds = scope.resolve("accountCreds");
accountCreds.save = stub();

const fs = scope.resolve("fs");
fs.readFileSync.withArgs(sinon.match(/secret_keys/)).returns("{}");
mwilde345 marked this conversation as resolved.
Show resolved Hide resolved
fs.readFileSync
.withArgs(sinon.match(/access_keys/))
.returns(validAccessKeyFile);

const accountClient = scope.resolve("accountClient");
accountClient.whoAmI.onFirstCall().throws(new InvalidCredsError());

await authNZMiddleware(argv);
expect(accountClient.refreshSession.calledOnce).to.be.true;
expect(accountCreds.save.calledOnce).to.be.true;
expect(accountCreds.save).to.have.been.calledWith({
creds: {
account_key: "new-account-key",
refresh_token: "new-refresh-token",
},
key: "test-profile",
});
});

describe("Short term DB Keys", () => {
let scope;
let fs;

const argv = {
authRequired: true,
profile: "test-profile",
database: "test-db",
url: "http://localhost",
role: "admin",
};
beforeEach(() => {
scope = container.createScope();
fs = scope.resolve("fs");
fs.readFileSync.callsFake((path) => {
if (path.includes("access_keys")) {
return validAccessKeyFile;
} else {
return validSecretKeyFile;
}
});
});
it("returns existing db key if it's valid", async function () {
await run("db list", scope);

const fetch = scope.resolve("fetch");
const secretCreds = scope.resolve("secretCreds");
fetch.resolves(f(true));
secretCreds.save = stub();

await authNZMiddleware(argv);
// Check that setDBKey was called and secrets were saved
expect(secretCreds.save.called).to.be.false;
});

it("creates a new db key if one doesn't exist", async function () {
await run("db list", scope);

const secretCreds = scope.resolve("secretCreds");
fs.readFileSync.withArgs(sinon.match(/secret_keys/)).returns("{}");

secretCreds.save = stub();

await authNZMiddleware(argv);
// Check that setDBKey was called and secrets were saved
expect(secretCreds.save.called).to.be.true;
expect(secretCreds.save).to.have.been.calledWith({
creds: {
path: "test-db",
role: "admin",
secret: "new-db-key",
},
key: "valid-account-key",
});
});

it("should clean up secrets file during setAccountKey", async function () {
await run("db list", scope);

const secretCreds = scope.resolve("secretCreds");
secretCreds.delete = stub();
fs.readFileSync
.withArgs(sinon.match(/secret_keys/))
.returns('{"old-account-key": {"admin": "old-db-key"}}');

await setAccountKey("test-profile");

// Verify the cleanup secrets logic
expect(secretCreds.delete.calledOnce).to.be.true;
expect(secretCreds.delete).to.have.been.calledWith("old-account-key");
});
});
});
4 changes: 2 additions & 2 deletions test/helpers.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import { Writable } from "node:stream";

// small helper for sinon to wrap your return value
// in the shape fetch would return it from the network
export function f(returnValue) {
return { json: async () => returnValue };
export function f(returnValue, status) {
return { json: async () => returnValue, status: status || 200 };
mwilde345 marked this conversation as resolved.
Show resolved Hide resolved
}

export const commonFetchParams = {
Expand Down