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.

Sign up for GitHub

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

Add improved secret management #301

Merged
merged 4 commits into from
Oct 12, 2023
Merged

Add improved secret management #301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b0974a2
Add secret class to manage scoped secrets
macmv Oct 12, 2023
4277b51
Update config to store a Secret instead of a string
macmv Oct 12, 2023
e40cc88
Update all the things to use Secret
macmv Oct 12, 2023
4c6d798
Add some eval tests for scoped secret handling
macmv Oct 12, 2023
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
3 changes: 2 additions & 1 deletion src/commands/cloud-login.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { hostname } from "os";
import { Command } from "@oclif/core";
import { underline, blue } from "chalk";
import fetch from "node-fetch";
import { Secret } from "../lib/secret";

const DEFAULT_NAME = "cloud";
const DB = process.env.FAUNA_URL ?? "https://db.fauna.com";
Expand Down Expand Up @@ -48,7 +49,7 @@ export default class CloudLoginCommand extends Command {
for (const region of Object.values(regions)) {
config.rootConfig.endpoints[region.endpointName(base)] = new Endpoint({
url: DB,
secret: region.secret,
secret: Secret.parse(region.secret),
});
}

Expand Down
25 changes: 22 additions & 3 deletions src/commands/endpoint/add.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Flags, Args, Command, ux } from "@oclif/core";
import { input, confirm } from "@inquirer/prompts";
import { Endpoint, ShellConfig, getRootConfigPath } from "../../lib/config";
import FaunaClient from "../../lib/fauna-client";
import { Secret } from "../../lib/secret";

export default class AddEndpointCommand extends Command {
static args = {
Expand Down Expand Up @@ -83,12 +84,30 @@ export default class AddEndpointCommand extends Command {
},
}));

const secret =
flags?.secret ?? (await input({ message: "Database Secret" }));
let secret: Secret;
if (flags?.secret === undefined) {
const v = await input({
message: "Database Secret",
validate: (secret) => {
try {
Secret.parse(secret);
return true;
} catch (e: any) {
return e.message;
}
},
});
secret = Secret.parse(v);
} else {
secret = Secret.parse(flags.secret);
}

ux.action.start("Checking secret");

const client = new FaunaClient({ secret, endpoint: url });
const client = new FaunaClient({
secret: secret.buildSecret(),
endpoint: url,
});
try {
const res = await client.query(`0`);
if (res.status !== 200) {
Expand Down
27 changes: 10 additions & 17 deletions src/lib/config/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const ini = require("ini");

import { ProjectConfig, Stack } from "./project-config";
import { RootConfig, Endpoint } from "./root-config";
import { Secret } from "../secret";

export { RootConfig, ProjectConfig, Endpoint, Stack };

Expand Down Expand Up @@ -118,12 +119,8 @@ export type ShellOpts = {
};

export type EndpointConfig = {
secret: string;
secret: Secret;
url: string;
/** this is currently just used for messaging purposes, the secret
* in this config will already contain the database path if needed.
*/
database?: string;
name?: string;
graphqlHost: string;
graphqlPort: number;
Expand Down Expand Up @@ -247,7 +244,7 @@ export class ShellConfig {
// No `~/.fauna-shell` was found, so `--secret` is required to make an endpoint. If `--secret` wasn't passed, `validate` should fail.
if (secretFlag !== undefined) {
this.endpoint = new Endpoint({
secret: secretFlag,
secret: Secret.parse(secretFlag),
url: urlFlag,
graphqlHost: this.flags.strOpt("graphqlHost"),
graphqlPort: this.flags.numberOpt("graphqlPort"),
Expand All @@ -260,7 +257,9 @@ export class ShellConfig {
}

// override endpoint with values from flags.
this.endpoint.secret = secretFlag ?? this.endpoint.secret;
if (secretFlag !== undefined) {
this.endpoint.secret = Secret.parse(secretFlag);
}
this.endpoint.url = urlFlag ?? this.endpoint.url;
this.endpoint.graphqlHost =
this.flags.strOpt("graphqlHost") ?? this.endpoint.graphqlHost;
Expand All @@ -278,21 +277,15 @@ export class ShellConfig {
}
};

lookupEndpoint = (opts: {
scope?: string;
role?: string;
}): EndpointConfig => {
lookupEndpoint = (opts: { scope?: string }): EndpointConfig => {
this.validate();

let database = this.stack?.database ?? "";
let database = this.stack?.database.split("/") ?? [];
if (opts.scope !== undefined) {
if (this.stack !== undefined) {
database += "/";
}
database += opts.scope;
database.push(...opts.scope.split("/"));
}

return this.endpoint!.makeScopedEndpoint(database, opts.role);
return this.endpoint!.makeScopedEndpoint(database);
};

configErrors(): string[] {
Expand Down
23 changes: 8 additions & 15 deletions src/lib/config/root-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Config, EndpointConfig, InvalidConfigError } from ".";
import fs from "fs";
import { Secret } from "../secret";
const ini = require("ini");

// Represents `~/.fauna-shell`
Expand Down Expand Up @@ -113,7 +114,7 @@ export class RootConfig {
*/
export class Endpoint {
name?: string;
secret: string;
secret: Secret;
url: string;

graphqlHost: string;
Expand All @@ -126,7 +127,7 @@ export class Endpoint {
} else {
return new Endpoint({
name: name,
secret: secOpt,
secret: Secret.parse(secOpt),
url: Endpoint.getURLFromConfig(config),

graphqlHost: config.strOpt("graphqlHost"),
Expand All @@ -137,7 +138,7 @@ export class Endpoint {

constructor(opts: {
name?: string;
secret: string;
secret: Secret;
url?: string;
graphqlHost?: string;
graphqlPort?: number;
Expand All @@ -150,26 +151,18 @@ export class Endpoint {
this.graphqlPort = opts.graphqlPort ?? 443;
}

makeScopedEndpoint(databaseScope?: string, role?: string): EndpointConfig {
let appendedRoleStr = "";
if (role) {
appendedRoleStr = `:${role}`;
} else if (databaseScope) {
appendedRoleStr = ":admin";
makeScopedEndpoint(databaseScope?: string[]): EndpointConfig {
const secret = this.secret.clone();
if (databaseScope !== undefined) {
secret.databaseScope.push(...databaseScope);
}

const secret =
this.secret +
(databaseScope ? `:${databaseScope}` : "") +
appendedRoleStr;

return {
secret,
name: this.name,
url: this.url,
graphqlHost: this.graphqlHost,
graphqlPort: this.graphqlPort,
database: databaseScope,
};
}

Expand Down
46 changes: 18 additions & 28 deletions src/lib/fauna-command.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ class FaunaCommand extends Command {
try {
connectionOptions = this.shellConfig.lookupEndpoint({
scope: dbScope,
role,
});

const { hostname, port, protocol } = new URL(connectionOptions.url);
Expand All @@ -71,7 +70,7 @@ class FaunaCommand extends Command {
domain: hostname,
port,
scheme: protocol?.replace(/:$/, ""),
secret: connectionOptions.secret,
secret: connectionOptions.secret.buildSecret({ role }),

// Force http1. See getClient.
fetch: fetch,
Expand Down Expand Up @@ -103,17 +102,15 @@ class FaunaCommand extends Command {
let connectedMessage;
if (connectionOptions.name !== undefined) {
connectedMessage = `Connected to endpoint: ${connectionOptions.name}`;
if (
connectionOptions.database !== undefined &&
connectionOptions.database !== ""
) {
connectedMessage += ` database: ${connectionOptions.database}`;
if (connectionOptions.secret.databaseScope.length > 0) {
connectedMessage += ` database: ${connectionOptions.secret.databaseScope.join(
"/"
)}`;
}
} else if (
connectionOptions.database !== undefined &&
connectionOptions.database !== ""
) {
connectedMessage = `Connected to database: ${connectionOptions.database}`;
} else if (connectionOptions.secret.databaseScope.length > 0) {
connectedMessage = `Connected to database: ${connectionOptions.secret.databaseScope.join(
"/"
)}`;
}
if (connectedMessage !== undefined) {
this.log(connectedMessage);
Expand All @@ -125,18 +122,15 @@ class FaunaCommand extends Command {
// construct v4 client
let connectionOptions;
try {
connectionOptions = this.shellConfig.lookupEndpoint({
scope: dbScope,
role,
});
connectionOptions = this.shellConfig.lookupEndpoint({ scope: dbScope });

const { hostname, port, protocol } = new URL(connectionOptions.url);

const client = new Client({
domain: hostname,
port,
scheme: protocol?.replace(/:$/, ""),
secret: connectionOptions.secret,
secret: connectionOptions.secret.buildSecret({ role }),
macmv marked this conversation as resolved.
Show resolved Hide resolved

// Force http1. Fixes tests I guess? I spent a solid 30 minutes
// debugging the whole `nock` thing in our tests, only to realize this
Expand Down Expand Up @@ -164,13 +158,10 @@ class FaunaCommand extends Command {
// construct v10 client
let connectionOptions;
try {
connectionOptions = this.shellConfig.lookupEndpoint({
scope: dbScope,
role,
});
connectionOptions = this.shellConfig.lookupEndpoint({ scope: dbScope });
const client = new FaunaClient({
endpoint: connectionOptions.url,
secret: connectionOptions.secret,
secret: connectionOptions.secret.buildSecret({ role }),
time: this.flags.timeout
? parseInt(this.flags.timeout, 10)
: undefined,
Expand Down Expand Up @@ -199,14 +190,11 @@ class FaunaCommand extends Command {
const { hostname, port, protocol } = new URL(connectionOptions.url);

for (let i = 0; i < path.length; i++) {
const secret =
connectionOptions.secret + ":" + path.slice(0, i).join("/") + ":admin";

const client = new Client({
domain: hostname,
port,
scheme: protocol?.replace(/:$/, ""),
secret,
secret: connectionOptions.secret.buildSecret(),

// See getClient.
fetch: fetch,
Expand All @@ -219,15 +207,17 @@ class FaunaCommand extends Command {
await client.close();

if (!exists) {
const fullPath = [...connectionOptions.secret.databaseScope, ...path.slice(0, i + 1)];
this.error(
`Database '${path.slice(0, i + 1).join("/")}' doesn't exist`
`Database '${fullPath.join("/")}' doesn't exist`
);
}

connectionOptions.secret.appendScope(path[i]);
}

return this.getClient({
dbScope: scope,
role: "admin",
version,
});
}
Expand Down
51 changes: 51 additions & 0 deletions src/lib/secret.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
export class Secret {
// A fauna key, like `fn1234`.
key: string;
// A database scope, like `["foo", "bar"]`
databaseScope: string[];

constructor(opts: { key: string; databaseScope?: string[] }) {
this.key = opts.key;
this.databaseScope = opts.databaseScope ?? [];
}

static parse(key: string) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does the 'current' shell allow you to have endpoints in your file that are scoped?

[endpoint.child]
secret=secret:child:admin
secret=secret:child:@role/myrole

maybe we don't care? just trying to think through if this could lead to backwards incompatible changes for users and where we draw the line on that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the current shell lets you do that, but it will break if you set a database path in fauna eval or fauna shell. It seems like something we ought to just disallow.

if (key.length === 0) {
throw new Error("Secret cannot be empty");
}
if (key.includes(":")) {
throw new Error("Secret cannot be scoped");
}
macmv marked this conversation as resolved.
Show resolved Hide resolved
return new Secret({ key });
}

buildSecret(opts?: { role?: string }): string {
let secret = this.key;
if (this.databaseScope.length > 0) {
secret += `:${this.databaseScope.join("/")}`;
}
if (opts?.role !== undefined || this.databaseScope.length > 0) {
const role = opts?.role ?? "admin";
secret += ["admin", "client", "server", "server-readonly"].includes(role)
? `:${role}`
: `:@role/${role}`;
macmv marked this conversation as resolved.
Show resolved Hide resolved
}
return secret;
}

/**
* Parses the given scope, appends it to the `Secret`, and returns the new
* secret. This mutates `this`.
*/
appendScope(scope: string) {
this.databaseScope.push(...scope.split("/"));
return this;
}

clone(): Secret {
return new Secret({
key: this.key,
databaseScope: [...this.databaseScope],
});
}
}
5 changes: 4 additions & 1 deletion src/lib/stack-factory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,10 @@ export class StackFactory {

promptDatabasePath = async (endpoint: Endpoint): Promise<string> => {
const { url, secret } = endpoint;
const client = new FaunaClient({ endpoint: url, secret });
const client = new FaunaClient({
endpoint: url,
secret: secret.buildSecret(),
});

const res = await client.query("0");
if (res.status !== 200) {
Expand Down
Loading