fix: stop wrangler spamming console after login (#695)

If a user hasn't logged in and then they run a command that needs a login they'll get bounced to the login flow.
The login flow (if completed) would write their shiny new OAuth2 credentials to disk, but wouldn't reload the
in-memory state. This led to issues like #693, where even though the user was logged in on-disk, wrangler
wouldn't be aware of it.

We now update the in-memory login state each time new credentials are written to disk.

.changeset/hungry-ways-boil.md

@@ -0,0 +1,12 @@
+---
+"wrangler": patch
+---
+
+fix: stop wrangler spamming console after login
+
+If a user hasn't logged in and then they run a command that needs a login they'll get bounced to the login flow.
+The login flow (if completed) would write their shiny new OAuth2 credentials to disk, but wouldn't reload the
+in-memory state. This led to issues like #693, where even though the user was logged in on-disk, wrangler
+wouldn't be aware of it.
+
+We now update the in-memory login state each time new credentials are written to disk.

packages/wrangler/src/__tests__/helpers/mock-oauth-flow.ts

@@ -0,0 +1,178 @@
+import { ChildProcess } from "child_process";
+import fetchMock from "jest-fetch-mock";
+import { Request } from "undici";
+const { fetch } = jest.requireActual("undici") as {
+  fetch: (input: string) => Promise<unknown>;
+};
+
+// the response to send when wrangler wants an oauth grant
+let oauthGrantResponse: GrantResponseOptions | "timeout" = {};
+
+/**
+ * A mock implementation for `openInBrowser` that sends an oauth grant response
+ * to wrangler. Use it like this:
+ *
+ * ```js
+ * jest.mock("../open-in-browser");
+ * (openInBrowser as jest.Mock).mockImplementation(mockOpenInBrowser);
+ * ```
+ */
+export const mockOpenInBrowser = async (url: string, ..._args: unknown[]) => {
+  const { searchParams } = new URL(url);
+  if (oauthGrantResponse === "timeout") {
+    throw "unimplemented";
+  } else {
+    const queryParams = toQueryParams(oauthGrantResponse, searchParams);
+    // don't await this -- it will block the rest of the login flow
+    fetch(`${searchParams.get("redirect_uri")}?${queryParams}`).catch((e) => {
+      throw new Error(
+        "Failed to send OAuth Grant to wrangler, maybe the server was closed?",
+        e as Error
+      );
+    });
+    return new ChildProcess();
+  }
+};
+
+/**
+ * Functions to help with mocking various parts of the OAuth Flow
+ */
+export const mockOAuthFlow = () => {
+  afterEach(() => {
+    fetchMock.resetMocks();
+  });
+
+  const mockGrantAuthorization = ({
+    respondWith,
+  }: {
+    respondWith: "timeout" | "success" | "failure" | GrantResponseOptions;
+  }) => {
+    if (respondWith === "failure") {
+      oauthGrantResponse = {
+        error: "access_denied",
+      };
+    } else if (respondWith === "success") {
+      oauthGrantResponse = {
+        code: "test-oauth-code",
+      };
+    } else if (respondWith === "timeout") {
+      oauthGrantResponse = "timeout";
+    } else {
+      oauthGrantResponse = respondWith;
+    }
+  };
+
+  const mockRevokeAuthorization = () => {
+    const outcome = {
+      actual: new Request("https://example.org"),
+      expected: new Request("https://dash.cloudflare.com/oauth2/revoke", {
+        method: "POST",
+      }),
+    };
+
+    fetchMock.mockIf(outcome.expected.url, async (req) => {
+      outcome.actual = req;
+      return "";
+    });
+
+    return outcome;
+  };
+
+  const mockGrantAccessToken = ({
+    respondWith,
+  }: {
+    respondWith: MockTokenResponse;
+  }) => {
+    const outcome = {
+      actual: new Request("https://example.org"),
+      expected: new Request("https://dash.cloudflare.com/oauth2/token", {
+        method: "POST",
+      }),
+    };
+
+    fetchMock.mockOnceIf(outcome.expected.url, async (req) => {
+      outcome.actual = req;
+      return makeTokenResponse(respondWith);
+    });
+
+    return outcome;
+  };
+
+  return {
+    mockGrantAuthorization,
+    mockRevokeAuthorization,
+    mockGrantAccessToken,
+  };
+};
+
+type GrantResponseOptions = {
+  code?: string;
+  error?: ErrorType | ErrorType[];
+};
+
+const toQueryParams = (
+  { code, error }: GrantResponseOptions,
+  wranglerRequestParams: URLSearchParams
+): string => {
+  const queryParams = [];
+  if (code) {
+    queryParams.push(`code=${code}`);
+  }
+  if (error) {
+    const stringifiedErr = Array.isArray(error) ? error.join(",") : error;
+    queryParams.push(`error=${stringifiedErr}`);
+  }
+
+  queryParams.push(`state=${wranglerRequestParams.get("state")}`);
+
+  return queryParams.join("&");
+};
+
+type ErrorType =
+  | "invalid_request"
+  | "invalid_grant"
+  | "unauthorized_client"
+  | "access_denied"
+  | "unsupported_response_type"
+  | "invalid_scope"
+  | "server_error"
+  | "temporarily_unavailable"
+  | "invalid_client"
+  | "unsupported_grant_type"
+  | "invalid_json"
+  | "invalid_token"
+  | "test-token-grant-error";
+
+type MockTokenResponse =
+  | "ok"
+  | "error"
+  | {
+      access_token: string;
+      expires_in: number;
+      refresh_token: string;
+      scope: string;
+    }
+  | {
+      error: ErrorType;
+    };
+
+const makeTokenResponse = (partialResponse: MockTokenResponse): string => {
+  let fullResponse: MockTokenResponse;
+
+  if (partialResponse === "ok") {
+    fullResponse = {
+      access_token: "test-access-token",
+      expires_in: 100000,
+      refresh_token: "test-refresh-token",
+      scope: "account:read",
+    };
+  } else if (partialResponse === "error") {
+    fullResponse = {
+      error: "test-token-grant-error",
+    };
+  } else {
+    fullResponse = partialResponse;
+  }
+
+  return JSON.stringify(fullResponse);
+};

packages/wrangler/src/__tests__/jest.setup.ts

@@ -5,7 +5,9 @@ import {
   getCloudflareAPIBaseURL,
 } from "../cfetch/internal";
 import { confirm, prompt } from "../dialogs";
+import openInBrowser from "../open-in-browser";
 import { mockFetchInternal, mockFetchKVGetValue } from "./helpers/mock-cfetch";
+import { mockOpenInBrowser as mockOpenInBrowserForOAuthFlow } from "./helpers/mock-oauth-flow";
 import { MockWebSocket } from "./helpers/mock-web-socket";
 
 jest.mock("ws", () => {
@@ -67,3 +69,6 @@ jest.mock("../dev/dev", () => {
     return null;
   });
 });
+
+jest.mock("../open-in-browser");
+(openInBrowser as jest.Mock).mockImplementation(mockOpenInBrowserForOAuthFlow);

packages/wrangler/src/__tests__/publish.test.ts

@@ -1,6 +1,7 @@
 import * as fs from "node:fs";
 import * as path from "node:path";
 import * as TOML from "@iarna/toml";
+import { writeAuthConfigFile } from "../user";
 import { mockAccountId, mockApiToken } from "./helpers/mock-account-id";
 import {
   createFetchResult,
@@ -11,6 +12,7 @@ import {
 } from "./helpers/mock-cfetch";
 import { mockConsoleMethods } from "./helpers/mock-console";
 import { mockKeyListRequest } from "./helpers/mock-kv";
+import { mockOAuthFlow } from "./helpers/mock-oauth-flow";
 import { runInTempDir } from "./helpers/run-in-tmp";
 import { runWrangler } from "./helpers/run-wrangler";
 import writeWranglerToml from "./helpers/write-wrangler-toml";
@@ -19,22 +21,93 @@ import type { KVNamespaceInfo } from "../kv";
 import type { FormData, File } from "undici";
 
 describe("publish", () => {
+  mockAccountId();
+  mockApiToken();
+  runInTempDir({ homedir: "./home" });
+  const std = mockConsoleMethods();
+  const { mockGrantAccessToken, mockGrantAuthorization } = mockOAuthFlow();
+
   beforeEach(() => {
     // @ts-expect-error we're using a very simple setTimeout mock here
     jest.spyOn(global, "setTimeout").mockImplementation((fn, _period) => {
-      fn();
+      setImmediate(fn);
     });
   });
-  mockAccountId();
-  mockApiToken();
-  runInTempDir();
-  const std = mockConsoleMethods();
 
   afterEach(() => {
     unsetAllMocks();
     unsetMockFetchKVGetValues();
   });
 
+  describe("authentication", () => {
+    mockApiToken({ apiToken: null });
+    beforeEach(() => {
+      // @ts-expect-error disable the mock we'd setup earlier
+      // or else our server won't bother listening for oauth requests
+      // and will timeout and fail
+      global.setTimeout.mockRestore();
+    });
+
+    it("drops a user into the login flow if they're unauthenticated", async () => {
+      writeWranglerToml();
+      writeWorkerSource();
+      mockSubDomainRequest();
+      mockUploadWorkerRequest();
+
+      const accessTokenRequest = mockGrantAccessToken({ respondWith: "ok" });
+      mockGrantAuthorization({ respondWith: "success" });
+
+      await expect(runWrangler("publish index.js")).resolves.toBeUndefined();
+
+      expect(accessTokenRequest.actual.url).toEqual(
+        accessTokenRequest.expected.url
+      );
+
+      expect(std.out).toMatchInlineSnapshot(`
+        "Attempting to login via OAuth...
+        Successfully logged in.
+        Uploaded test-name (TIMINGS)
+        Published test-name (TIMINGS)
+          test-name.test-sub-domain.workers.dev"
+      `);
+      expect(std.warn).toMatchInlineSnapshot(`""`);
+      expect(std.err).toMatchInlineSnapshot(`""`);
+    });
+
+    it("warns a user when they're authenticated with an API token in wrangler config file", async () => {
+      writeWranglerToml();
+      writeWorkerSource();
+      mockSubDomainRequest();
+      mockUploadWorkerRequest();
+      writeAuthConfigFile({
+        api_token: "some-api-token",
+      });
+
+      const accessTokenRequest = mockGrantAccessToken({ respondWith: "ok" });
+      mockGrantAuthorization({ respondWith: "success" });
+
+      await expect(runWrangler("publish index.js")).resolves.toBeUndefined();
+
+      expect(accessTokenRequest.actual.url).toEqual(
+        accessTokenRequest.expected.url
+      );
+
+      expect(std.out).toMatchInlineSnapshot(`
+        "Attempting to login via OAuth...
+        Successfully logged in.
+        Uploaded test-name (TIMINGS)
+        Published test-name (TIMINGS)
+          test-name.test-sub-domain.workers.dev"
+      `);
+      expect(std.warn).toMatchInlineSnapshot(`
+        "It looks like you have used Wrangler 1's \`config\` command to login with an API token.
+        This is no longer supported in the current version of Wrangler.
+        If you wish to authenticate via an API token then please set the \`CLOUDFLARE_API_TOKEN\` environment variable."
+      `);
+      expect(std.err).toMatchInlineSnapshot(`""`);
+    });
+  });
+
   describe("environments", () => {
     it("should use legacy environments by default", async () => {
       writeWranglerToml({ env: { "some-env": {} } });