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

Improve credential handling #432

Merged
merged 25 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
fff1863
refresh credentials during fetch calls
mwilde345 Nov 22, 2024
15096a3
Merge branch 'v3' into no-preflight
mwilde345 Nov 22, 2024
7be933c
don't create dupe keys
mwilde345 Nov 22, 2024
b25d691
Update src/config/setup-test-container.mjs
mwilde345 Nov 22, 2024
f0cf18e
logger component
mwilde345 Nov 23, 2024
c8b5174
Merge branch 'v3' into no-preflight
mwilde345 Nov 23, 2024
2868551
Merge branch 'v3' into no-preflight
mwilde345 Nov 23, 2024
d1f0dc0
refactor credentials
mwilde345 Nov 26, 2024
4f1e31d
refactor credentials
mwilde345 Nov 26, 2024
1aa6db2
separate concerns of account and database creds into separate classes
mwilde345 Nov 26, 2024
9792a2b
revert create database changes
mwilde345 Nov 26, 2024
32e1a0f
get rid of authnz middleware
mwilde345 Nov 26, 2024
6c86634
further separate concerns
mwilde345 Nov 26, 2024
fa472a5
Merge branch 'v3' into no-preflight
mwilde345 Nov 26, 2024
7ac122f
fix directory for creds. add some env var options
mwilde345 Nov 26, 2024
8ab062a
fix lint
mwilde345 Nov 26, 2024
48ba039
skip busted test
mwilde345 Nov 26, 2024
b0c6dd9
unsafe accessor fix
mwilde345 Nov 26, 2024
dc96e44
try test fix
mwilde345 Nov 26, 2024
d6f9d3a
Merge branch 'v3' into no-preflight
mwilde345 Nov 26, 2024
4d9d837
Merge branch 'v3' into no-preflight
mwilde345 Nov 26, 2024
c35346e
fix tests
mwilde345 Nov 26, 2024
ff7cbf4
don't set a default role in the args. resolve it in credentials. mock…
mwilde345 Nov 27, 2024
ca34e38
readme and remove .fauna dir
mwilde345 Nov 27, 2024
28bfeff
don't use exit just throw an error
mwilde345 Nov 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ experiments
coverage
test-results.xml
.history
.fauna
mwilde345 marked this conversation as resolved.
Show resolved Hide resolved

# default fauna config file names
fauna.config.yaml,
Expand Down
21 changes: 0 additions & 21 deletions src/commands/database/database.mjs
Original file line number Diff line number Diff line change
@@ -1,34 +1,13 @@
//@ts-check

import { container } from "../../cli.mjs";
import { commonQueryOptions } from "../../lib/command-helpers.mjs";
import createCommand from "./create.mjs";
import deleteCommand from "./delete.mjs";
import listCommand from "./list.mjs";

function validateArgs(argv) {
const logger = container.resolve("logger");

if (argv.secret && argv.database) {
// Providing a database and secret are conflicting options. If both are provided,
// it is not clear which one to use.
throw new Error(
"Cannot use both the '--secret' and '--database' options together. Please specify only one.",
);
} else if (argv.role && argv.secret) {
// The '--role' option is not supported when using a secret. Secrets have an
// implicit role.
logger.warn(
"The '--role' option is not supported when using a secret. It will be ignored.",
);
}
return true;
}

function buildDatabase(yargs) {
return yargs
.options(commonQueryOptions)
.check(validateArgs)
.command(listCommand)
.command(createCommand)
.command(deleteCommand)
Expand Down
31 changes: 27 additions & 4 deletions src/lib/auth/credentials.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,50 @@ import { asValue, Lifetime } from "awilix";

import { container } from "../../cli.mjs";
import { FaunaAccountClient } from "../fauna-account-client.mjs";
import { cleanupSecretsFile } from "../file-util.mjs";
import { AccountKeys } from "./accountKeys.mjs";
import { DatabaseKeys } from "./databaseKeys.mjs";

const validateCredentialArgs = (argv) => {
const logger = container.resolve("logger");
if (argv.database && argv.secret) {
throw new Error(
"Cannot provide both a database and a secret. Please provide one or the other.",
"Cannot use both the '--secret' and '--database' options together. Please specify only one.",
);
} else if (argv.role && argv.secret) {
// The '--role' option is not supported when using a secret. Secrets have an
// implicit role.
logger.warn(
"The '--role' option is not supported when using a secret. It will be ignored.",
);
}
};
mwilde345 marked this conversation as resolved.
Show resolved Hide resolved

export class Credentials {
constructor(argv) {
// Get rid of orphaned database keys in the local storage
cleanupSecretsFile();
// Make sure auth-related arguments from users are legal
validateCredentialArgs(argv);

this.accountKeys = new AccountKeys(argv);
this.databaseKeys = new DatabaseKeys(argv, this.accountKeys.key);
this.cleanupSecretsFile();
}

/**
* Steps through account keys in local filesystem and if they are not found in the secrets file,
* delete the stale entries on the secrets file.
*/
cleanupSecretsFile() {
const accountKeyData = this.accountKeys.keyStore.getFile();
const accountKeys = Object.values(accountKeyData).map(
(value) => value.accountKey,
);
const secretKeyData = this.databaseKeys.keyStore.getFile();
Object.keys(secretKeyData).forEach((accountKey) => {
if (!accountKeys.includes(accountKey)) {
this.databaseKeys.keyStore.updateAccountKey(accountKey);
this.databaseKeys.keyStore.deleteAllDBKeysForAccount();
}
});
mwilde345 marked this conversation as resolved.
Show resolved Hide resolved
}

async login(accessToken) {
Expand Down
18 changes: 0 additions & 18 deletions src/lib/file-util.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -315,24 +315,6 @@ export class AccountKeyStorage extends CredentialsStorage {
}
}

/**
* Steps through account keys in local filesystem and if they are not found in the secrets file,
* delete the stale entries on the secrets file.
*/
export function cleanupSecretsFile() {
const accountKeyData = new CredentialsStorage("access_keys").getFile();
const accountKeys = Object.values(accountKeyData).map(
(value) => value.accountKey,
);
const secretKeyData = new CredentialsStorage("secret_keys").getFile();
Object.keys(secretKeyData).forEach((accountKey) => {
if (!accountKeys.includes(accountKey)) {
const secretKeyStorage = new SecretKeyStorage(accountKey);
secretKeyStorage.deleteAllDBKeysForAccount();
}
});
}

/**
* Checks if a value is a valid JSON string.
*
Expand Down
1 change: 0 additions & 1 deletion test/config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import path from "node:path";

import * as awilix from "awilix";
import { expect } from "chai";
import chalk from "chalk";
import notAllowed from "not-allowed";
import sinon from "sinon";
import stripAnsi from "strip-ansi";
Expand Down
3 changes: 2 additions & 1 deletion test/schema/pull.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,8 @@ describe("schema pull", function () {
expect(logger.stdout).to.have.been.calledWith("Change cancelled");
expect(fs.writeFile).to.have.not.been.called;
expect(fsp.unlink).to.have.not.been.called;
expect(fs.mkdirSync).to.have.not.been.called;
// Called twice during Credentials initialization, but not called during the pull command
expect(fs.mkdirSync).to.have.been.calledTwice;
mwilde345 marked this conversation as resolved.
Show resolved Hide resolved
});

it("can delete extraneous FSL files", async function () {
Expand Down
6 changes: 4 additions & 2 deletions test/shell.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ describe("shell", function () {

// start the shell
const runPromise = run(`shell --secret "secret"`, container);

// Wait for the shell to start (print ">")
await stdout.waitForWritten();
mwilde345 marked this conversation as resolved.
Show resolved Hide resolved
// send our first command
stdin.push(`${query}\n`);
await stdout.waitForWritten();
Expand Down Expand Up @@ -167,7 +168,8 @@ describe("shell", function () {

// start the shell
const runPromise = run(`shell --secret "secret" --version 4`, container);

// Wait for the shell to start (print ">")
await stdout.waitForWritten();
// send our first command
stdin.push(`${query}\n`);
await stdout.waitForWritten();
Expand Down