diff --git a/.changeset/lemon-radios-sparkle.md b/.changeset/lemon-radios-sparkle.md new file mode 100644 index 000000000000..c28ed4a29da7 --- /dev/null +++ b/.changeset/lemon-radios-sparkle.md @@ -0,0 +1,9 @@ +--- +"wrangler": patch +--- + +refactor: initialize the user auth state synchronously + +We can now initialize the user state synchronously, which means that +we can remove the checks for whether it has been done or not in each +of the user auth functions. diff --git a/packages/wrangler/src/__tests__/helpers/mock-account-id.ts b/packages/wrangler/src/__tests__/helpers/mock-account-id.ts index 2c32d069ba78..b7d4fea0811c 100644 --- a/packages/wrangler/src/__tests__/helpers/mock-account-id.ts +++ b/packages/wrangler/src/__tests__/helpers/mock-account-id.ts @@ -1,3 +1,5 @@ +import { reinitialiseAuthTokens } from "../../user"; + const ORIGINAL_CLOUDFLARE_API_TOKEN = process.env.CLOUDFLARE_API_TOKEN; const ORIGINAL_CLOUDFLARE_ACCOUNT_ID = process.env.CLOUDFLARE_ACCOUNT_ID; @@ -16,6 +18,8 @@ export function mockApiToken({ } else { process.env.CLOUDFLARE_API_TOKEN = apiToken; } + // Now we have updated the environment, we must reinitialize the user auth state. + reinitialiseAuthTokens(); }); afterEach(() => { process.env.CLOUDFLARE_API_TOKEN = ORIGINAL_CLOUDFLARE_API_TOKEN; diff --git a/packages/wrangler/src/__tests__/helpers/mock-user.ts b/packages/wrangler/src/__tests__/helpers/mock-user.ts deleted file mode 100644 index 0278303e727b..000000000000 --- a/packages/wrangler/src/__tests__/helpers/mock-user.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { mkdirSync, writeFileSync } from "node:fs"; -import os from "node:os"; -import path from "node:path"; - -export function writeUserConfig( - oauth_token?: string, - refresh_token?: string, - expiration_time?: string -) { - const lines: string[] = []; - if (oauth_token) { - lines.push(`oauth_token = "${oauth_token}"`); - } - if (refresh_token) { - lines.push(`refresh_token = "${refresh_token}"`); - } - if (expiration_time) { - lines.push(`expiration_time = "${expiration_time}"`); - } - const configPath = path.join(os.homedir(), ".wrangler/config"); - mkdirSync(configPath, { recursive: true }); - writeFileSync( - path.join(configPath, "default.toml"), - lines.join("\n"), - "utf-8" - ); -} diff --git a/packages/wrangler/src/__tests__/helpers/run-in-tmp.ts b/packages/wrangler/src/__tests__/helpers/run-in-tmp.ts index 5e0ace8caffb..9c4809192b50 100644 --- a/packages/wrangler/src/__tests__/helpers/run-in-tmp.ts +++ b/packages/wrangler/src/__tests__/helpers/run-in-tmp.ts @@ -1,6 +1,7 @@ import * as fs from "node:fs"; import os from "node:os"; import * as path from "node:path"; +import { reinitialiseAuthTokens } from "../../user"; const originalCwd = process.cwd(); @@ -19,6 +20,8 @@ export function runInTempDir({ homedir }: { homedir?: string } = {}) { // This is because the module gets transpiled so that the "method" `homedir()` gets converted to a // getter that is not configurable (and so cannot be spied upon). jest.spyOn(os, "homedir").mockReturnValue(homedir); + // Now that we have changed the home directory location, we must reinitialize the user auth state + reinitialiseAuthTokens(); } }); diff --git a/packages/wrangler/src/__tests__/logout.test.ts b/packages/wrangler/src/__tests__/logout.test.ts index 06fcff0745e2..a65f00d6b719 100644 --- a/packages/wrangler/src/__tests__/logout.test.ts +++ b/packages/wrangler/src/__tests__/logout.test.ts @@ -2,9 +2,12 @@ import { existsSync } from "node:fs"; import os from "node:os"; import path from "node:path"; import fetchMock from "jest-fetch-mock"; -import { initialise } from "../user"; +import { + reinitialiseAuthTokens, + USER_AUTH_CONFIG_FILE, + writeAuthConfigFile, +} from "../user"; import { mockConsoleMethods } from "./helpers/mock-console"; -import { writeUserConfig } from "./helpers/mock-user"; import { runInTempDir } from "./helpers/run-in-tmp"; import { runWrangler } from "./helpers/run-wrangler"; @@ -14,14 +17,15 @@ describe("wrangler", () => { describe("logout", () => { it("should exit with a message stating the user is not logged in", async () => { - await initialise(); await runWrangler("logout"); expect(std.out).toMatchInlineSnapshot(`"Not logged in, exiting..."`); }); it("should logout user that has been properly logged in", async () => { - writeUserConfig("some-oauth-tok", "some-refresh-tok"); - + writeAuthConfigFile({ + oauth_token: "some-oauth-tok", + refresh_token: "some-refresh-tok", + }); // Mock out the response for a request to revoke the auth tokens, // checking the form of the request is as expected. fetchMock.mockResponseOnce(async (req) => { @@ -30,7 +34,8 @@ describe("wrangler", () => { return ""; }); - await initialise(); + // Now that we have changed the auth tokens in the wrangler.toml we must reinitialize the user auth state. + reinitialiseAuthTokens(); await runWrangler("logout"); expect(std.out).toMatchInlineSnapshot(` @@ -42,9 +47,9 @@ describe("wrangler", () => { expect(fetchMock).toHaveBeenCalledTimes(1); // Make sure that logout removed the config file containing the auth tokens. - expect( - existsSync(path.join(os.homedir(), ".wrangler/config/default.toml")) - ).toBe(false); + expect(existsSync(path.join(os.homedir(), USER_AUTH_CONFIG_FILE))).toBe( + false + ); }); }); }); diff --git a/packages/wrangler/src/__tests__/whoami.test.tsx b/packages/wrangler/src/__tests__/whoami.test.tsx index 45b874d92c55..b54ec5e558e7 100644 --- a/packages/wrangler/src/__tests__/whoami.test.tsx +++ b/packages/wrangler/src/__tests__/whoami.test.tsx @@ -1,9 +1,8 @@ import { render } from "ink-testing-library"; import React from "react"; -import { initialise } from "../user"; +import { reinitialiseAuthTokens, writeAuthConfigFile } from "../user"; import { getUserInfo, WhoAmI } from "../whoami"; import { setMockResponse } from "./helpers/mock-cfetch"; -import { writeUserConfig } from "./helpers/mock-user"; import { runInTempDir } from "./helpers/run-in-tmp"; import type { UserInfo } from "../whoami"; @@ -11,20 +10,23 @@ describe("getUserInfo()", () => { runInTempDir({ homedir: "./home" }); it("should return undefined if there is no config file", async () => { - await initialise(); const userInfo = await getUserInfo(); expect(userInfo).toBeUndefined(); }); it("should return undefined if there is an empty config file", async () => { - writeUserConfig(); - await initialise(); + writeAuthConfigFile({}); + // Now that we have changed the auth tokens in the wrangler.toml we must reinitialize the user auth state. + reinitialiseAuthTokens(); const userInfo = await getUserInfo(); expect(userInfo).toBeUndefined(); }); it("should return the user's email and accounts if authenticated via config token", async () => { - writeUserConfig("some-oauth-token"); + writeAuthConfigFile({ oauth_token: "some-oauth-token" }); + // Now that we have changed the auth tokens in the wrangler.toml we must reinitialize the user auth state. + reinitialiseAuthTokens(); + setMockResponse("/user", () => { return { email: "user@example.com" }; }); @@ -36,7 +38,6 @@ describe("getUserInfo()", () => { ]; }); - await initialise(); const userInfo = await getUserInfo(); expect(userInfo).toEqual({ diff --git a/packages/wrangler/src/index.tsx b/packages/wrangler/src/index.tsx index 7674543ef943..0b503ed3e626 100644 --- a/packages/wrangler/src/index.tsx +++ b/packages/wrangler/src/index.tsx @@ -47,7 +47,6 @@ import { login, logout, listScopes, - initialise as initialiseUserConfig, loginOrRefreshIfRequired, getAccountId, validateScopeKeys, @@ -2340,7 +2339,6 @@ export async function main(argv: string[]): Promise { wrangler.exitProcess(false); try { - await initialiseUserConfig(); await wrangler.parse(); } catch (e) { if (e instanceof CommandLineArgsError) { diff --git a/packages/wrangler/src/user.tsx b/packages/wrangler/src/user.tsx index efa063e17b35..8ee474ce2dd5 100644 --- a/packages/wrangler/src/user.tsx +++ b/packages/wrangler/src/user.tsx @@ -207,7 +207,7 @@ import assert from "node:assert"; import { webcrypto as crypto } from "node:crypto"; -import { readFile, writeFile, rm, mkdir } from "node:fs/promises"; +import { writeFileSync, mkdirSync, rmSync } from "node:fs"; import http from "node:http"; import os from "node:os"; import path from "node:path"; @@ -222,6 +222,7 @@ import { fetch } from "undici"; import { getCloudflareApiBaseUrl } from "./cfetch"; import { getEnvironmentVariableFactory } from "./environment-variables"; import openInBrowser from "./open-in-browser"; +import { readFileSync } from "./parse"; import type { Item as SelectInputItem } from "ink-select-input/build/SelectInput"; import type { ParsedUrlQuery } from "node:querystring"; import type { Response } from "undici"; @@ -251,17 +252,42 @@ interface PKCECodes { codeVerifier: string; } -interface State { - accessToken?: AccessToken; // persist +/** + * The module level state of the authentication flow. + */ +interface State extends AuthTokens { authorizationCode?: string; codeChallenge?: string; codeVerifier?: string; hasAuthCodeBeenExchangedForAccessToken?: boolean; - refreshToken?: RefreshToken; // persist stateQueryParam?: string; scopes?: Scope[]; } +/** + * The tokens related to authentication. + */ +interface AuthTokens { + accessToken?: AccessToken; + refreshToken?: RefreshToken; +} + +/** + * The path to the config file that holds user authentication data, + * relative to the user's home directory. + */ +export const USER_AUTH_CONFIG_FILE = ".wrangler/config/default.toml"; + +/** + * The data that may be read from the `USER_CONFIG_FILE`. + */ +export interface UserAuthConfig { + oauth_token?: string; + refresh_token?: string; + expiration_time?: string; + // api_token?: string; +} + interface RefreshToken { value: string; } @@ -311,60 +337,54 @@ const TOKEN_URL = "https://dash.cloudflare.com/oauth2/token"; const CALLBACK_URL = "http://localhost:8976/oauth/callback"; const REVOKE_URL = "https://dash.cloudflare.com/oauth2/revoke"; -const LocalState: State = {}; -let initialised = false; +let LocalState: State = getAuthTokens(); -// we do this because we have some async stuff -// TODO: this should just happen in the top level -// and we should figure out how to do top level await -export async function initialise(): Promise { +/** + * Compute the current auth tokens. + */ +function getAuthTokens(): AuthTokens { // get refreshToken/accessToken from fs if exists try { // if the environment variable is available, use that const apiTokenFromEnv = getCloudflareAPITokenFromEnv(); if (apiTokenFromEnv) { - LocalState.accessToken = { - value: apiTokenFromEnv, - expiry: "3021-12-31T23:59:59+00:00", + return { + accessToken: { + value: apiTokenFromEnv, + expiry: "3021-12-31T23:59:59+00:00", + }, }; - initialised = true; - return; } - const toml = TOML.parse( - await readFile(path.join(os.homedir(), ".wrangler/config/default.toml"), { - encoding: "utf-8", - }) - ); - const { oauth_token, refresh_token, expiration_time } = toml as { - oauth_token: string; - refresh_token: string; - expiration_time: string; - }; + // otherwise try loading from the user auth config file. + const { oauth_token, refresh_token, expiration_time } = + readAuthConfigFile(); + if (oauth_token) { - LocalState.accessToken = { value: oauth_token, expiry: expiration_time }; - } - if (refresh_token) { - LocalState.refreshToken = { value: refresh_token }; + return { + accessToken: { + value: oauth_token, + // If there is no `expiration_time` field then set it to an old date, to cause it to expire immediately. + expiry: expiration_time ?? "2000-01-01:00:00:00+00:00", + }, + refreshToken: { value: refresh_token ?? "" }, + }; + } else { + return {}; } - } catch (err) { - // no config yet, let's chill - // console.error(err); + } catch { + return {}; } - initialised = true; } -// ugh. TODO: see fix from above. -function throwIfNotInitialised() { - if (initialised === false) { - throw new Error( - "did you forget to call initialise() from the user module?" - ); - } +/** + * Run the initialisation of the auth state, in the case that something changed. + */ +export function reinitialiseAuthTokens(): void { + LocalState = getAuthTokens(); } export function getAPIToken(): string | undefined { - throwIfNotInitialised(); return LocalState.accessToken?.value; } @@ -800,21 +820,24 @@ function generateRandomState(lengthOfState: number): string { .join(""); } -async function writeToConfigFile(tokenData: AccessContext) { - await mkdir(path.join(os.homedir(), ".wrangler/config/"), { +export function writeAuthConfigFile(config: UserAuthConfig) { + mkdirSync(path.join(os.homedir(), ".wrangler/config/"), { recursive: true, }); - await writeFile( - path.join(os.homedir(), ".wrangler/config/default.toml"), - ` -oauth_token = "${tokenData.token?.value || ""}" -refresh_token = "${tokenData.refreshToken?.value}" -expiration_time = "${tokenData.token?.expiry}" -`, + writeFileSync( + path.join(os.homedir(), USER_AUTH_CONFIG_FILE), + TOML.stringify(config as TOML.JsonMap), { encoding: "utf-8" } ); } +function readAuthConfigFile(): UserAuthConfig { + const toml = TOML.parse( + readFileSync(path.join(os.homedir(), USER_AUTH_CONFIG_FILE)) + ); + return toml; +} + type LoginProps = { scopes?: Scope[]; }; @@ -824,7 +847,7 @@ export async function loginOrRefreshIfRequired( ): Promise { // TODO: if there already is a token, then try refreshing // TODO: ask permission before opening browser - if (!LocalState.accessToken) { + if (LocalState.accessToken === undefined) { // Not logged in. // If we are not interactive, we cannot ask the user to login return isInteractive && (await login()); @@ -894,8 +917,12 @@ export async function login(props?: LoginProps): Promise { finish(false, new ErrorNoAuthCode()); return; } else { - const tokenData = await exchangeAuthCodeForAccessToken(); - await writeToConfigFile(tokenData); + const exchange = await exchangeAuthCodeForAccessToken(); + writeAuthConfigFile({ + oauth_token: exchange.token?.value ?? "", + expiration_time: exchange.token?.expiry, + refresh_token: exchange.refreshToken?.value, + }); res.writeHead(307, { Location: "https://welcome.developers.workers.dev/wrangler-oauth-consent-granted", @@ -904,7 +931,7 @@ export async function login(props?: LoginProps): Promise { finish(true); }); console.log( - `Successfully configured. You can find your configuration file at: ${os.homedir()}/.wrangler/config/default.toml` + `Successfully configured. You can find your configuration file at: ${os.homedir()}/${USER_AUTH_CONFIG_FILE}` ); return; @@ -922,18 +949,22 @@ export async function login(props?: LoginProps): Promise { /** * Checks to see if the access token has expired. */ -export function isAccessTokenExpired(): boolean { - throwIfNotInitialised(); +function isAccessTokenExpired(): boolean { const { accessToken } = LocalState; return Boolean(accessToken && new Date() >= new Date(accessToken.expiry)); } -export async function refreshToken(): Promise { - throwIfNotInitialised(); +async function refreshToken(): Promise { // refresh try { - const refreshed = await exchangeRefreshTokenForAccessToken(); - await writeToConfigFile(refreshed); + const { + token: { value: oauth_token, expiry: expiration_time } = { + value: "", + expiry: "", + }, + refreshToken: { value: refresh_token } = {}, + } = await exchangeRefreshTokenForAccessToken(); + writeAuthConfigFile({ oauth_token, expiration_time, refresh_token }); return true; } catch (err) { console.error(err); @@ -942,10 +973,28 @@ export async function refreshToken(): Promise { } export async function logout(): Promise { - throwIfNotInitialised(); - if (!LocalState.refreshToken) { - console.log("Not logged in, exiting..."); - return; + if (!LocalState.accessToken) { + if (!LocalState.refreshToken) { + console.log("Not logged in, exiting..."); + return; + } + + const body = + `client_id=${encodeURIComponent(CLIENT_ID)}&` + + `token_type_hint=refresh_token&` + + `token=${encodeURIComponent(LocalState.refreshToken?.value || "")}`; + + const response = await fetch(REVOKE_URL, { + method: "POST", + body, + headers: { + "Content-Type": "application/x-www-form-urlencoded", + }, + }); + await response.text(); // blank text? would be nice if it was something meaningful + console.log( + "💁 Wrangler is configured with an OAuth token. The token has been successfully revoked" + ); } const body = `client_id=${encodeURIComponent(CLIENT_ID)}&` + @@ -964,15 +1013,12 @@ export async function logout(): Promise { "💁 Wrangler is configured with an OAuth token. The token has been successfully revoked" ); // delete the file - await rm(path.join(os.homedir(), ".wrangler/config/default.toml")); - console.log( - `Removing ${os.homedir()}/.wrangler/config/default.toml.. success!` - ); + rmSync(path.join(os.homedir(), USER_AUTH_CONFIG_FILE)); + console.log(`Removing ${os.homedir()}/${USER_AUTH_CONFIG_FILE}.. success!`); } -export function listScopes(): void { - throwIfNotInitialised(); - console.log("💁 Available scopes:"); +export function listScopes(message = "💁 Available scopes:"): void { + console.log(message); const data = ScopeKeys.map((scope: Scope) => ({ Scope: scope, Description: Scopes[scope],