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

Make --secret override project config #308

Merged
merged 5 commits into from
Oct 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
59 changes: 35 additions & 24 deletions src/lib/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ export class ShellConfig {
rootConfig: RootConfig;
projectConfig: ProjectConfig | undefined;

// If `--secret` is passed, this will be set.
secretFlag?: Secret;
// The selected stack from the project config. If there is a project config, this will also be set.
stack: Stack | undefined;
// The fully configured endpoint, including command line flags that override things like the URL.
Expand Down Expand Up @@ -211,16 +213,16 @@ export class ShellConfig {
}
}

const stackFlag = this.flags.strOpt("stack");

if (this.projectConfig === undefined) {
const stackName = this.flags.strOpt("stack");
if (stackName !== undefined) {
if (stackFlag !== undefined) {
throw new Error(
`No ${PROJECT_FILE_NAME} was found, so stack '${stackName}' cannot be used`
`No ${PROJECT_FILE_NAME} was found, so stack '${stackFlag}' cannot be used`
);
}
} else {
const stackName =
this.flags.strOpt("stack") ?? this.projectConfig.defaultStack;
const stackName = stackFlag ?? this.projectConfig.defaultStack;

if (stackName !== undefined) {
this.stack = this.projectConfig.stacks[stackName];
Expand All @@ -239,27 +241,30 @@ export class ShellConfig {
this.rootConfig.defaultEndpoint;

const secretFlag = this.flags.strOpt("secret");
this.secretFlag =
secretFlag !== undefined ? Secret.parseFlag(secretFlag) : undefined;

if (secretFlag !== undefined && stackFlag !== undefined) {
throw new Error(
"Cannot specify both --secret and --stack, as --secret will override the settings from a stack."
);
}

if (endpointName === undefined) {
// 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: Secret.parse(secretFlag),
url: urlFlag,
graphqlHost: this.flags.strOpt("graphqlHost"),
graphqlPort: this.flags.numberOpt("graphqlPort"),
});
}
// This is a dummy secret. `--secret` must be set in this case, which
// `validate` enforces.
this.endpoint = new Endpoint({
secret: new Secret({ key: "", allowDatabase: true }),
url: urlFlag,
graphqlHost: this.flags.strOpt("graphqlHost"),
graphqlPort: this.flags.numberOpt("graphqlPort"),
});
} else {
this.endpoint = this.rootConfig.endpoints[endpointName];
if (this.endpoint === undefined) {
throw new Error(`No such endpoint '${endpointName}'`);
}

// override endpoint with values from flags.
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 @@ -269,7 +274,7 @@ export class ShellConfig {
}

validate = () => {
if (this.endpoint === undefined) {
if (this.endpoint === undefined && this.secretFlag === undefined) {
// No `~/.fauna-shell` was found, and no `--secret` was passed.
throw new Error(
`No endpoint or secret set. Set an endpoint in ~/.fauna-shell, ${PROJECT_FILE_NAME}, or pass --endpoint`
Expand All @@ -280,12 +285,18 @@ export class ShellConfig {
lookupEndpoint = (opts: { scope?: string }): EndpointConfig => {
this.validate();

let database = this.stack?.database.split("/") ?? [];
if (opts.scope !== undefined) {
database.push(...opts.scope.split("/"));
}
if (this.secretFlag !== undefined) {
const endpoint = this.endpoint!.makeScopedEndpoint();
endpoint.secret = this.secretFlag;
return endpoint;
} else {
let database = this.stack?.database.split("/") ?? [];
if (opts.scope !== undefined) {
database.push(...opts.scope.split("/"));
}

return this.endpoint!.makeScopedEndpoint(database);
return this.endpoint!.makeScopedEndpoint(database);
}
};

configErrors(): string[] {
Expand Down
6 changes: 6 additions & 0 deletions src/lib/fauna-command.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,12 @@ class FaunaCommand extends Command {
const { connectionOptions } = await this.getClient({ version: "4" });
const { hostname, port, protocol } = new URL(connectionOptions.url);

if (!connectionOptions.secret.allowDatabase) {
throw new Error(
"Cannot specify database with a secret that contains a database"
);
}

for (let i = 0; i < path.length; i++) {
const client = new Client({
domain: hostname,
Expand Down
29 changes: 25 additions & 4 deletions src/lib/secret.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,36 @@
export class Secret {
// A fauna key, like `fn1234`.
key: string;
// Do we allow database scope?
allowDatabase: boolean;
// A database scope, like `["foo", "bar"]`
databaseScope: string[];

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

static parseFlag(key: string) {
if (key.length === 0) {
throw new Error("Secret cannot be empty");
}
return new Secret({ key, allowDatabase: !key.includes(":") });
}

static parse(key: string) {
Comment on lines +19 to 26
Copy link
Contributor

Choose a reason for hiding this comment

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

wondering if we should just have one parse method, could take as a parameter whether or not the key can be scoped. Could use both of these methods under the hood. looking at 2 methods parse vs parseFlag may not be clear which to use but if we had an explicit parameter, allowScopedKey or something, then it may make it a bit more clear.

if (key.length === 0) {
throw new Error("Secret cannot be empty");
}
if (key.includes(":")) {
throw new Error("Secret cannot be scoped");
}
Comment on lines 30 to 32
Copy link
Contributor

Choose a reason for hiding this comment

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

my understanding here is that we wouldn't allow scoped secrets in the .fauna-shell file. I think that is fine, is backwards incompatible I believe though so could be a good thing for us to be aware of.

return new Secret({ key });
return new Secret({ key, allowDatabase: true });
}

buildSecret(opts?: { role?: string }): string {
Expand All @@ -38,13 +52,20 @@ export class Secret {
* secret. This mutates `this`.
*/
appendScope(scope: string) {
this.databaseScope.push(...scope.split("/"));
return this;
if (this.allowDatabase) {
this.databaseScope.push(...scope.split("/"));
return this;
} else {
throw new Error(
"Cannot specify database with a secret that contains a database"
);
}
}

clone(): Secret {
return new Secret({
key: this.key,
allowDatabase: this.allowDatabase,
databaseScope: [...this.databaseScope],
});
}
Expand Down
29 changes: 26 additions & 3 deletions test/commands/eval.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,14 +141,37 @@ describe("eval in v10", () => {
"--format",
"json-tagged",
"--secret",
`foo:MyDB`,
`${process.env.FAUNA_SECRET}:MyDB:admin`,
"--url",
getEndpoint()
])
.it("allows scoped secrets", (ctx) => {
expect(JSON.parse(ctx.stdout)).to.deep.equal({ two: { "@int": "3" } });
});


test
.do(async () => {
// This can fail if `MyDB` already exists, but thats fine.
await evalV10("Database.create({ name: 'MyDB' })");
})
.stdout()
// a scoped secret is never valid.
.command([
"eval",
"MyDB2",
"{ two: 3 }",
"--format",
"json-tagged",
"--secret",
`${process.env.FAUNA_SECRET}:MyDB:admin`,
"--url",
getEndpoint()
])
.catch((e) => {
expect(e.message).to.equal("Secret cannot be scoped");
expect(e.message).to.equal("Cannot specify database with a secret that contains a database");
})
.it("disallows scoped secrets");
.it("disallows scoped secrets and a scope argument");
});

function mockQuery(api) {
Expand Down
12 changes: 10 additions & 2 deletions test/lib/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,14 +162,15 @@ describe("root config with flags", () => {
).to.deep.contain({
secret: {
key: "fn555",
allowDatabase: true,
databaseScope: [],
},
url: "https://db.fauna.com",
});
});

it("disallows scoped secrets", () => {
expect(() =>
expect(
lookupEndpoint({
flags: {
secret: "fn555:db:@role/bar",
Expand All @@ -181,7 +182,14 @@ describe("root config with flags", () => {
},
},
})
).to.throw("Secret cannot be scoped");
).to.deep.contain({
secret: {
key: "fn555:db:@role/bar",
allowDatabase: false,
databaseScope: [],
},
url: "https://db.fauna.com",
});
});
});

Expand Down
42 changes: 31 additions & 11 deletions test/lib/secret.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,37 @@
import { expect } from "chai";
import { Secret } from "../../src/lib/secret";

describe("secret", () => {
describe("Secret.parse", () => {
it("disallows empty secrets", () => {
expect(() => Secret.parse("")).to.throw("Secret cannot be empty");
});

it("disallows scoped secrets", () => {
expect(() => Secret.parse("fn1234:foo")).to.throw(
"Secret cannot be scoped"
);
});
});

describe("Secret.parseFlag", () => {
it("disallows empty secrets", () => {
expect(() => Secret.parseFlag("")).to.throw("Secret cannot be empty");
});

it("allows scoped secrets", () => {
Secret.parseFlag("fn1234:foo");
});

it("disallows appending scopes to scoped secrets", () => {
const secret = Secret.parseFlag("fn1234:foo");

expect(() => secret.appendScope("bar")).to.throw(
"Cannot specify database with a secret that contains a database"
);
});
});

describe("Secret", () => {
it("appends paths scoped secrets", () => {
const secret = Secret.parse("fn1234");
expect(secret.databaseScope).to.eql([]);
Expand All @@ -21,16 +51,6 @@ describe("secret", () => {
expect(secret.databaseScope).to.eql(["foo", "bar"]);
});

it("disallows empty secrets", () => {
expect(() => Secret.parse("")).to.throw("Secret cannot be empty");
});

it("disallows scoped secrets", () => {
expect(() => Secret.parse("fn1234:foo")).to.throw(
"Secret cannot be scoped"
);
});

it("builds a secret with a role", () => {
const secret = Secret.parse("fn1234");
expect(secret.buildSecret()).to.equal("fn1234");
Expand Down