From 16b92d5b0f1c0d89836537cb989588e2b49ed3bf Mon Sep 17 00:00:00 2001 From: Jacob M-G Evans Date: Mon, 25 Apr 2022 17:20:45 -0500 Subject: [PATCH] non-TTY check for required variables: Added a check in non-TTY environments for `account_id`, `CLOUDFLARE_ACCOUNT_ID` and `CLOUDFLARE_API_TOKEN`. If `account_id` exists in `wrangler.toml` then `CLOUDFLARE_ACCOUNT_ID` is not needed in non-TTY scope. The `CLOUDFLARE_API_TOKEN` is necessary in non-TTY scope and will always error if missing. resolves #827 --- .../wrangler/src/__tests__/publish.test.ts | 31 +++++++++++++++++-- .../wrangler/src/__tests__/secret.test.ts | 15 ++++----- .../wrangler/src/__tests__/whoami.test.tsx | 6 ++++ packages/wrangler/src/pages.tsx | 16 +++++----- packages/wrangler/src/user.tsx | 11 ++++--- 5 files changed, 55 insertions(+), 24 deletions(-) diff --git a/packages/wrangler/src/__tests__/publish.test.ts b/packages/wrangler/src/__tests__/publish.test.ts index 9703e3311165..1eb66c351e78 100644 --- a/packages/wrangler/src/__tests__/publish.test.ts +++ b/packages/wrangler/src/__tests__/publish.test.ts @@ -178,7 +178,7 @@ describe("publish", () => { expect(std.err).toMatchInlineSnapshot(`""`); }); - it("should throw an error in non-TTY if 'account_id' & 'CLOUDFLARE_ACCOUNT_ID' is missing", async () => { + it("should throw an error in non-TTY & there is more than one account associated with API token", async () => { setIsTTY(false); process.env = { CLOUDFLARE_API_TOKEN: "hunter2", @@ -233,10 +233,35 @@ describe("publish", () => { await expect(runWrangler("publish index.js")).rejects.toThrowError(); expect(std.err).toMatchInlineSnapshot(` - "X [ERROR] Missing 'CLOUDFLARE_API_TOKEN' from non-TTY environment, please see docs for more info: TBD + "X [ERROR] In a non-interactive environment, it's necessary to set a CLOUDFLARE_API_TOKEN environment variable for wrangler to work. + Please go to https://developers.cloudflare.com/api/tokens/create/ for instructions on how to create an api token, and assign its value to CLOUDFLARE_API_TOKEN. + " + `); + }); + it("should throw error with no account ID provided and no members retrieved", async () => { + setIsTTY(false); + writeWranglerToml({ + account_id: undefined, + }); + process.env = { + CLOUDFLARE_API_TOKEN: "picard", + CLOUDFLARE_ACCOUNT_ID: undefined, + }; + writeWorkerSource(); + mockSubDomainRequest(); + mockUploadWorkerRequest(); + mockOAuthServerCallback(); + mockGetMemberships({ + success: false, + result: [], + }); - X [ERROR] Did not login, quitting... + await expect(runWrangler("publish index.js")).rejects.toThrowError(); + + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Failed to automatically retrieve account IDs for the logged in user. + In a non-interactive environment, it is mandatory to specify an account ID, either by assigning its value to CLOUDFLARE_ACCOUNT_ID, or as \`account_id\` in your \`wrangler.toml\` file. " `); diff --git a/packages/wrangler/src/__tests__/secret.test.ts b/packages/wrangler/src/__tests__/secret.test.ts index 7d9ecfb4dc60..d39b8d1bf447 100644 --- a/packages/wrangler/src/__tests__/secret.test.ts +++ b/packages/wrangler/src/__tests__/secret.test.ts @@ -4,7 +4,6 @@ import { mockAccountId, mockApiToken } from "./helpers/mock-account-id"; import { setMockResponse, unsetAllMocks } from "./helpers/mock-cfetch"; import { mockConsoleMethods } from "./helpers/mock-console"; import { mockConfirm, mockPrompt } from "./helpers/mock-dialogs"; -import { useMockIsTTY } from "./helpers/mock-istty"; import { mockOAuthFlow } from "./helpers/mock-oauth-flow"; import { useMockStdin } from "./helpers/mock-stdin"; import { runInTempDir } from "./helpers/run-in-tmp"; @@ -13,14 +12,13 @@ import { runWrangler } from "./helpers/run-wrangler"; describe("wrangler secret", () => { const std = mockConsoleMethods(); const { mockGetMemberships } = mockOAuthFlow(); - const { setIsTTY } = useMockIsTTY(); + runInTempDir(); mockAccountId(); mockApiToken(); afterEach(() => { unsetAllMocks(); - setIsTTY(true); }); describe("put", () => { @@ -177,11 +175,11 @@ describe("wrangler secret", () => { success: false, result: [], }); - await expect( - runWrangler("secret put the-key --name script-name") - ).rejects.toThrowErrorMatchingInlineSnapshot( - `"No account id found, quitting..."` - ); + await expect(runWrangler("secret put the-key --name script-name")) + .rejects.toThrowErrorMatchingInlineSnapshot(` + "Failed to automatically retrieve account IDs for the logged in user. + In a non-interactive environment, it is mandatory to specify an account ID, either by assigning its value to CLOUDFLARE_ACCOUNT_ID, or as \`account_id\` in your \`wrangler.toml\` file." + `); }); it("should use the account from wrangler.toml", async () => { @@ -204,7 +202,6 @@ describe("wrangler secret", () => { }); it("should error if a user has multiple accounts, and has not specified an account in wrangler.toml", async () => { - setIsTTY(false); mockGetMemberships({ success: true, result: [ diff --git a/packages/wrangler/src/__tests__/whoami.test.tsx b/packages/wrangler/src/__tests__/whoami.test.tsx index 719ba381c2e5..aa79a0e1227b 100644 --- a/packages/wrangler/src/__tests__/whoami.test.tsx +++ b/packages/wrangler/src/__tests__/whoami.test.tsx @@ -4,12 +4,18 @@ import { writeAuthConfigFile } from "../user"; import { getUserInfo, WhoAmI } from "../whoami"; import { setMockResponse } from "./helpers/mock-cfetch"; import { mockConsoleMethods } from "./helpers/mock-console"; +import { useMockIsTTY } from "./helpers/mock-istty"; import { runInTempDir } from "./helpers/run-in-tmp"; import type { UserInfo } from "../whoami"; describe("getUserInfo()", () => { runInTempDir({ homedir: "./home" }); const std = mockConsoleMethods(); + const { setIsTTY } = useMockIsTTY(); + + beforeEach(() => { + setIsTTY(true); + }); it("should return undefined if there is no config file", async () => { const userInfo = await getUserInfo(); diff --git a/packages/wrangler/src/pages.tsx b/packages/wrangler/src/pages.tsx index 3986dd77e6ba..ff44b609ce66 100644 --- a/packages/wrangler/src/pages.tsx +++ b/packages/wrangler/src/pages.tsx @@ -842,11 +842,11 @@ const createDeployment: CommandModule< const config = getConfigCache( PAGES_CONFIG_CACHE_FILENAME ); - const isInteractive = process.stdin.isTTY; - const accountId = await requireAuth(config, isInteractive); + const accountId = await requireAuth(config); projectName ??= config.project_name; + const isInteractive = process.stdin.isTTY; if (!projectName && isInteractive) { const existingOrNew = await new Promise<"new" | "existing">((resolve) => { const { unmount } = render( @@ -1605,8 +1605,8 @@ export const pages: BuilderCallback = (yargs) => { const config = getConfigCache( PAGES_CONFIG_CACHE_FILENAME ); - const isInteractive = process.stdin.isTTY; - const accountId = await requireAuth(config, isInteractive); + + const accountId = await requireAuth(config); const projects: Array = await listProjects({ accountId }); @@ -1650,9 +1650,9 @@ export const pages: BuilderCallback = (yargs) => { const config = getConfigCache( PAGES_CONFIG_CACHE_FILENAME ); - const isInteractive = process.stdin.isTTY; - const accountId = await requireAuth(config, isInteractive); + const accountId = await requireAuth(config); + const isInteractive = process.stdin.isTTY; if (!projectName && isInteractive) { projectName = await prompt("Enter the name of your new project:"); } @@ -1735,11 +1735,11 @@ export const pages: BuilderCallback = (yargs) => { const config = getConfigCache( PAGES_CONFIG_CACHE_FILENAME ); - const isInteractive = process.stdin.isTTY; - const accountId = await requireAuth(config, isInteractive); + const accountId = await requireAuth(config); projectName ??= config.project_name; + const isInteractive = process.stdin.isTTY; if (!projectName && isInteractive) { const projects = await listProjects({ accountId }); projectName = await new Promise((resolve) => { diff --git a/packages/wrangler/src/user.tsx b/packages/wrangler/src/user.tsx index fe5fb319bd46..7506fa6b3345 100644 --- a/packages/wrangler/src/user.tsx +++ b/packages/wrangler/src/user.tsx @@ -418,11 +418,9 @@ export function getAPIToken(): string | undefined { !localAPIToken && !LocalState.accessToken?.value ) { - logger.error( - "Missing 'CLOUDFLARE_API_TOKEN' from non-TTY environment, please see docs for more info: TBD" + throw new Error( + "In a non-interactive environment, it's necessary to set a CLOUDFLARE_API_TOKEN environment variable for wrangler to work. Please go to https://developers.cloudflare.com/api/tokens/create/ for instructions on how to create an api token, and assign its value to CLOUDFLARE_API_TOKEN." ); - - return; } return localAPIToken ?? LocalState.accessToken?.value; @@ -1142,6 +1140,11 @@ export async function getAccountId( .join("\n") ); } + } else { + if (!isInteractive) + throw new Error( + `Failed to automatically retrieve account IDs for the logged in user. In a non-interactive environment, it is mandatory to specify an account ID, either by assigning its value to CLOUDFLARE_ACCOUNT_ID, or as \`account_id\` in your \`wrangler.toml\` file.` + ); } return accountId; }