From aa3f8558f5daf034f001d4a52dbb0a40aea69326 Mon Sep 17 00:00:00 2001 From: "E. Cooper" Date: Wed, 18 Dec 2024 13:20:13 -0800 Subject: [PATCH 1/4] Simplify fauna local argv validation --- src/commands/local.mjs | 56 +++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/src/commands/local.mjs b/src/commands/local.mjs index 24cec7ad..e2acb988 100644 --- a/src/commands/local.mjs +++ b/src/commands/local.mjs @@ -124,6 +124,32 @@ ${chalk.red("Please use choose a different name using --name or align the --type } } +function validateContainerArgv(argv) { + if (argv.maxAttempts < 1) { + throw new ValidationError("--max-attempts must be greater than 0."); + } + if (argv.interval < 0) { + throw new ValidationError("--interval must be greater than or equal to 0."); + } +} + +function validateDatabaseArgv(argv) { + const dbOnlyArgs = { + typechecked: "--typechecked", + protected: "--protected", + priority: "--priority", + directory: "--fsl-directory", + }; + + for (const [arg, name] of Object.entries(dbOnlyArgs)) { + if (argv[arg] !== undefined && !argv.database) { + throw new ValidationError( + `${name} can only be set if --database is set.`, + ); + } + } +} + /** * Builds the yargs command for the local command * @param {import('yargs').Argv} yargs The yargs instance @@ -198,34 +224,8 @@ function buildLocalCommand(yargs) { }, }) .check((argv) => { - if (argv.maxAttempts < 1) { - throw new ValidationError("--max-attempts must be greater than 0."); - } - if (argv.interval < 0) { - throw new ValidationError( - "--interval must be greater than or equal to 0.", - ); - } - if (argv.typechecked !== undefined && !argv.database) { - throw new ValidationError( - "--typechecked can only be set if --database is set.", - ); - } - if (argv.protected && !argv.database) { - throw new ValidationError( - "--protected can only be set if --database is set.", - ); - } - if (argv.priority && !argv.database) { - throw new ValidationError( - "--priority can only be set if --database is set.", - ); - } - if (argv.directory && !argv.database) { - throw new ValidationError( - "--directory,--dir can only be set if --database is set.", - ); - } + validateContainerArgv(argv); + validateDatabaseArgv(argv); return true; }) .example([ From 60fde64bd3b1a620c14bc3066d1deedec13258ed Mon Sep 17 00:00:00 2001 From: "E. Cooper" Date: Wed, 18 Dec 2024 14:05:16 -0800 Subject: [PATCH 2/4] Simplify request handling in OAuthClient --- src/lib/auth/oauth-client.mjs | 108 ++++++++++++++++++++-------------- 1 file changed, 63 insertions(+), 45 deletions(-) diff --git a/src/lib/auth/oauth-client.mjs b/src/lib/auth/oauth-client.mjs index 5c986f5c..85cfe12e 100644 --- a/src/lib/auth/oauth-client.mjs +++ b/src/lib/auth/oauth-client.mjs @@ -1,10 +1,19 @@ import { createHash, randomBytes } from "crypto"; import http from "http"; import url from "url"; +import util from "util"; import { container } from "../../cli.mjs"; import SuccessPage from "./successPage.mjs"; +const ALLOWED_ORIGINS = [ + "http://localhost:3005", + "http://127.0.0.1:3005", + "http://dashboard.fauna.com", + "http://dashboard.fauna-dev.com", + "http://dashboard.fauna-preview.com", +]; + // Default to prod client id and secret const CLIENT_ID = process.env.FAUNA_CLIENT_ID ?? "Aq4_G0mOtm_F1fK3PuzE0k-i9F0"; // Native public clients are not confidential. The client secret is not used beyond @@ -52,64 +61,73 @@ class OAuthClient { return Buffer.from(randomBytes(20)).toString("base64url"); } + _handleRedirect({ pathname, res }) { + if (pathname === "/success") { + res.writeHead(200, { "Content-Type": "text/html" }); + res.write(SuccessPage); + res.end(); + this.closeServer(); + } else if (pathname !== "/") { + throw new Error("Invalid redirect uri"); + } + } + + _handleCode({ authCode, state, res }) { + if (!authCode || typeof authCode !== "string") { + throw new Error("Invalid authorization code received"); + } else { + this.authCode = authCode; + if (state !== this.state) { + throw new Error("Invalid state received"); + } + res.writeHead(302, { Location: "/success" }); + res.end(); + this.server.emit("auth_code_received"); + } + } + // req: IncomingMessage, res: ServerResponse _handleRequest(req, res) { const logger = container.resolve("logger"); - const allowedOrigins = [ - "http://localhost:3005", - "http://127.0.0.1:3005", - "http://dashboard.fauna.com", - "http://dashboard.fauna-dev.com", - "http://dashboard.fauna-preview.com", - ]; const origin = req.headers.origin || ""; - if (allowedOrigins.includes(origin)) { + if (ALLOWED_ORIGINS.includes(origin)) { res.setHeader("Access-Control-Allow-Origin", origin); res.setHeader("Access-Control-Allow-Methods", "GET"); res.setHeader("Access-Control-Allow-Headers", "Content-Type"); } - let errorMessage = ""; - - if (req.method === "GET") { - const parsedUrl = url.parse(req.url || "", true); - if (parsedUrl.pathname === "/success") { - res.writeHead(200, { "Content-Type": "text/html" }); - res.write(SuccessPage); - res.end(); - this.closeServer(); - } else if (parsedUrl.pathname !== "/") { - errorMessage = "Invalid redirect uri"; - this.closeServer(); - } - const query = parsedUrl.query; - if (query.error) { - errorMessage = `${query.error.toString()} - ${query.error_description}`; - this.closeServer(); - } - if (query.code) { - const authCode = query.code; - if (!authCode || typeof authCode !== "string") { - errorMessage = "Invalid authorization code received"; - this.server.close(); - } else { - this.authCode = authCode; - if (query.state !== this.state) { - errorMessage = "Invalid state received"; - this.closeServer(); - } - res.writeHead(302, { Location: "/success" }); - res.end(); - this.server.emit("auth_code_received"); + try { + // We only expect GET requests + if (req.method === "GET") { + const { pathname, query } = url.parse(req.url || "", true); + + // If the pathname is not "/", we're handling a redirect + if (pathname !== "/") { + this._handleRedirect({ pathname, res }); + } + + // If the query contains an error, we're handling an error + if (query.error) { + throw new Error( + `${query.error.toString()} - ${query.error_description}`, + ); } + + // If the query contains an auth code, we're handling a successful auth + if (query.code) { + this._handleCode({ authCode: query.code, state: query.state, res }); + } + } else { + throw new Error("Invalid request method"); } - } else { - errorMessage = "Invalid request method"; + } catch (e) { this.closeServer(); - } - if (errorMessage) { - logger.stderr(`Error during authentication: ${errorMessage}`); + logger.debug( + `Authentication error: ${util.inspect(e, true, 2, false)}`, + "creds", + ); + logger.stderr(`Error during authentication: ${e.message}`); } } From ec4f2673e46d16a0b1d91200bb693496e216b4fd Mon Sep 17 00:00:00 2001 From: "E. Cooper" Date: Wed, 18 Dec 2024 14:06:00 -0800 Subject: [PATCH 3/4] Lower complexity of query info formatting --- src/lib/fauna-client.mjs | 43 +++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/src/lib/fauna-client.mjs b/src/lib/fauna-client.mjs index 59a756cc..721283c9 100644 --- a/src/lib/fauna-client.mjs +++ b/src/lib/fauna-client.mjs @@ -181,26 +181,29 @@ export const formatQuerySummary = (summary) => { } }; -/** - * Selects a subset of query info fields from a v10 query response. - * @param {import("fauna").QueryInfo} response - The query response - * @param {string[]} include - The query info fields to include - * @returns {object} An object with the selected query info fields - */ -const pickAndCamelCaseQueryInfo = (response, include) => { - const queryInfo = {}; - - if (include.includes("txnTs") && response.txn_ts) - queryInfo.txnTs = response.txn_ts; - if (include.includes("schemaVersion") && response.schema_version) - queryInfo.schemaVersion = response.schema_version.toString(); - if (include.includes("summary") && response.summary) - queryInfo.summary = response.summary; - if (include.includes("queryTags") && response.query_tags) - queryInfo.queryTags = response.query_tags; - if (include.includes("stats") && response.stats) - queryInfo.stats = response.stats; +const getQueryInfoValue = (response, field) => { + switch (field) { + case "txnTs": + return response.txn_ts; + case "schemaVersion": + return response.schema_version?.toString(); + case "summary": + return response.summary; + case "queryTags": + return response.query_tags; + case "stats": + return response.stats; + default: + return undefined; + } +}; +const getIncludedQueryInfo = (response, include) => { + const queryInfo = {}; + include.forEach((field) => { + const value = getQueryInfoValue(response, field); + if (value) queryInfo[field] = value; + }); return queryInfo; }; @@ -224,7 +227,7 @@ export const formatQueryInfo = (response, { apiVersion, color, include }) => { return `${colorized}\n`; } else if (apiVersion === "10") { - const queryInfoToDisplay = pickAndCamelCaseQueryInfo(response, include); + const queryInfoToDisplay = getIncludedQueryInfo(response, include); if (Object.keys(queryInfoToDisplay).length === 0) return ""; From d03435889d574473794eae518303e7dc6c3ffc3d Mon Sep 17 00:00:00 2001 From: "E. Cooper" Date: Wed, 18 Dec 2024 14:06:25 -0800 Subject: [PATCH 4/4] Move all TODOs to issues we can track separately --- src/cli.mjs | 2 +- src/commands/shell.mjs | 1 - src/lib/auth/accountKeys.mjs | 2 +- src/lib/auth/credentials.mjs | 1 - src/lib/fauna-account-client.mjs | 3 --- src/lib/middleware.mjs | 1 - test/schema/commit.mjs | 1 - 7 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/cli.mjs b/src/cli.mjs index e95e84f8..fcb407c7 100644 --- a/src/cli.mjs +++ b/src/cli.mjs @@ -141,7 +141,7 @@ function buildYargs(argvInput) { .filter((key) => previousWord === key) .pop(); - // TODO: this doesn't handle aliasing, and it needs to + // This doesn't handle aliasing, and it needs to if ( currentWord === "--profile" || currentWordFlag === "profile" || diff --git a/src/commands/shell.mjs b/src/commands/shell.mjs index 2c234d23..ce25dd4b 100644 --- a/src/commands/shell.mjs +++ b/src/commands/shell.mjs @@ -35,7 +35,6 @@ async function shellCommand(argv) { prompt: `${argv.database || ""}> `, ignoreUndefined: true, preview: argv.apiVersion !== "10", - // TODO: integrate with fql-analyzer for completions completer: argv.apiVersion === "10" ? () => [] : undefined, output: container.resolve("stdoutStream"), input: container.resolve("stdinStream"), diff --git a/src/lib/auth/accountKeys.mjs b/src/lib/auth/accountKeys.mjs index 815147c7..794e192e 100644 --- a/src/lib/auth/accountKeys.mjs +++ b/src/lib/auth/accountKeys.mjs @@ -81,7 +81,7 @@ export class AccountKeys { async getOrRefreshKey() { if (this.keySource === "credentials-file") { const key = this.keyStore.get(); - // TODO: track ttl for account and refresh keys + if (!key) { this.logger.debug( "Found account key, but it is expired. Refreshing...", diff --git a/src/lib/auth/credentials.mjs b/src/lib/auth/credentials.mjs index 852d223e..e7533e6a 100644 --- a/src/lib/auth/credentials.mjs +++ b/src/lib/auth/credentials.mjs @@ -64,7 +64,6 @@ export class Credentials { this.accountKeys.keyStore.save({ accountKey, refreshToken, - // TODO: set expiration }); this.accountKeys.key = accountKey; } diff --git a/src/lib/fauna-account-client.mjs b/src/lib/fauna-account-client.mjs index efea7696..7d823e65 100644 --- a/src/lib/fauna-account-client.mjs +++ b/src/lib/fauna-account-client.mjs @@ -131,8 +131,6 @@ export class FaunaAccountClient { * @returns {Promise<{accountKey: string, refreshToken: string}>} - The session information. * @throws {Error} - Throws an error if there is an issue during session retrieval. */ - - // TODO: get/set expiration details static async getSession(accessToken) { const makeAccountRequest = container.resolve("makeAccountRequest"); try { @@ -149,7 +147,6 @@ export class FaunaAccountClient { } } - // TODO: get/set expiration details /** * Uses refreshToken to get a new accountKey and refreshToken. * @param {*} refreshToken diff --git a/src/lib/middleware.mjs b/src/lib/middleware.mjs index 3bdcc6dc..11402e68 100644 --- a/src/lib/middleware.mjs +++ b/src/lib/middleware.mjs @@ -43,7 +43,6 @@ export function fixPaths(argv) { } export function checkForUpdates(argv) { - // TODO: figure out upgrade path for SEA installations if (isSea()) return argv; const __filename = fileURLToPath(import.meta.url); diff --git a/test/schema/commit.mjs b/test/schema/commit.mjs index c0e89103..30f8e2c8 100644 --- a/test/schema/commit.mjs +++ b/test/schema/commit.mjs @@ -91,7 +91,6 @@ describe("schema commit", function () { }); it("errors if the schema is not in a ready state", async function () { - // TODO: what are the valid statuses? !none, !ready results in this case fetch.onCall(0).resolves(f({ status: "building", diff: diff })); const [error] = await tryToCatch(() =>