From c9db89e6ed0e1722022b3bbaecd6f52cff33f6cb Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Mon, 14 Mar 2022 19:43:15 +0000 Subject: [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. --- .changeset/lemon-radios-sparkle.md | 9 + .../src/__tests__/helpers/mock-account-id.ts | 4 + .../src/__tests__/helpers/mock-user.ts | 27 --- .../src/__tests__/helpers/run-in-tmp.ts | 3 + .../wrangler/src/__tests__/logout.test.ts | 23 ++- .../wrangler/src/__tests__/whoami.test.tsx | 15 +- packages/wrangler/src/index.tsx | 2 - packages/wrangler/src/user.tsx | 188 +++++++++++------- 8 files changed, 155 insertions(+), 116 deletions(-) create mode 100644 .changeset/lemon-radios-sparkle.md delete mode 100644 packages/wrangler/src/__tests__/helpers/mock-user.ts 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],