Skip to content

Commit

Permalink
fix: improve authentication logging and warnings
Browse files Browse the repository at this point in the history
- If a user has previously logged in via Wrangler 1 with an API token, we now display a helpful warning.
- When logging in and out, we no longer display the path to the internal user auh config file.
- When logging in, we now display an initial message to indicate the authentication flow is starting.

Fixes [#526](#526)
  • Loading branch information
petebacondarwin committed Mar 19, 2022
1 parent 06f9278 commit 0708070
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 27 deletions.
11 changes: 11 additions & 0 deletions .changeset/large-pillows-clap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"wrangler": patch
---

fix: improve authentication logging and warnings

- If a user has previously logged in via Wrangler 1 with an API token, we now display a helpful warning.
- When logging in and out, we no longer display the path to the internal user auh config file.
- When logging in, we now display an initial message to indicate the authentication flow is starting.

Fixes [#526](https://github.com/cloudflare/wrangler2/issues/526)
19 changes: 14 additions & 5 deletions packages/wrangler/src/__tests__/helpers/mock-user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,17 @@ 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
) {
export function writeUserConfig({
oauth_token,
refresh_token,
expiration_time,
api_token,
}: {
oauth_token?: string;
refresh_token?: string;
expiration_time?: string;
api_token?: string;
}) {
const lines: string[] = [];
if (oauth_token) {
lines.push(`oauth_token = "${oauth_token}"`);
Expand All @@ -17,6 +23,9 @@ export function writeUserConfig(
if (expiration_time) {
lines.push(`expiration_time = "${expiration_time}"`);
}
if (api_token) {
lines.push(`api_token = "${api_token}"`);
}
const configPath = path.join(os.homedir(), ".wrangler/config");
mkdirSync(configPath, { recursive: true });
writeFileSync(
Expand Down
10 changes: 5 additions & 5 deletions packages/wrangler/src/__tests__/logout.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ describe("wrangler", () => {
});

it("should logout user that has been properly logged in", async () => {
writeUserConfig("some-oauth-tok", "some-refresh-tok");
writeUserConfig({
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.
Expand All @@ -33,10 +36,7 @@ describe("wrangler", () => {
await initialise();
await runWrangler("logout");

expect(std.out).toMatchInlineSnapshot(`
"💁 Wrangler is configured with an OAuth token. The token has been successfully revoked
Removing ./home/.wrangler/config/default.toml.. success!"
`);
expect(std.out).toMatchInlineSnapshot(`"Successfully logged out."`);

// Make sure that we made the request to logout.
expect(fetchMock).toHaveBeenCalledTimes(1);
Expand Down
17 changes: 15 additions & 2 deletions packages/wrangler/src/__tests__/whoami.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@ import React from "react";
import { initialise } from "../user";
import { getUserInfo, WhoAmI } from "../whoami";
import { setMockResponse } from "./helpers/mock-cfetch";
import { mockConsoleMethods } from "./helpers/mock-console";
import { writeUserConfig } from "./helpers/mock-user";
import { runInTempDir } from "./helpers/run-in-tmp";
import type { UserInfo } from "../whoami";

describe("getUserInfo()", () => {
runInTempDir({ homedir: "./home" });
const std = mockConsoleMethods();

it("should return undefined if there is no config file", async () => {
await initialise();
Expand All @@ -17,14 +19,14 @@ describe("getUserInfo()", () => {
});

it("should return undefined if there is an empty config file", async () => {
writeUserConfig();
writeUserConfig({});
await initialise();
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");
writeUserConfig({ oauth_token: "some-oauth-token" });
setMockResponse("/user", () => {
return { email: "user@example.com" };
});
Expand All @@ -50,6 +52,17 @@ describe("getUserInfo()", () => {
],
});
});

it("should display a warning message if the config file contains a legacy api_token field", async () => {
writeUserConfig({ api_token: "API_TOKEN" });
await initialise();
await getUserInfo();
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."
`);
});
});

describe("WhoAmI component", () => {
Expand Down
37 changes: 22 additions & 15 deletions packages/wrangler/src/user.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,8 @@ interface State {
refreshToken?: RefreshToken; // persist
stateQueryParam?: string;
scopes?: Scope[];
/** @deprecated */
api_token?: string;
}

interface RefreshToken {
Expand Down Expand Up @@ -336,16 +338,21 @@ export async function initialise(): Promise<void> {
encoding: "utf-8",
})
);
const { oauth_token, refresh_token, expiration_time } = toml as {

const { oauth_token, refresh_token, expiration_time, api_token } = toml as {
oauth_token: string;
refresh_token: string;
expiration_time: string;
/** @deprecated */
api_token?: string;
};
if (oauth_token) {
LocalState.accessToken = { value: oauth_token, expiry: expiration_time };
}
if (refresh_token) {
LocalState.refreshToken = { value: refresh_token };
if (refresh_token) {
LocalState.refreshToken = { value: refresh_token };
}
} else if (api_token) {
LocalState.api_token = api_token;
}
} catch (err) {
// no config yet, let's chill
Expand All @@ -365,6 +372,13 @@ function throwIfNotInitialised() {

export function getAPIToken(): string | undefined {
throwIfNotInitialised();
if (LocalState.api_token) {
console.warn(
"It looks like you have used Wrangler 1's `config` command to login with an API token.\n" +
"This is no longer supported in the current version of Wrangler.\n" +
"If you wish to authenticate via an API token then please set the `CLOUDFLARE_API_TOKEN` environment variable."
);
}
return LocalState.accessToken?.value;
}

Expand Down Expand Up @@ -824,7 +838,7 @@ export async function loginOrRefreshIfRequired(
): Promise<boolean> {
// TODO: if there already is a token, then try refreshing
// TODO: ask permission before opening browser
if (!LocalState.accessToken) {
if (!getAPIToken()) {
// Not logged in.
// If we are not interactive, we cannot ask the user to login
return isInteractive && (await login());
Expand All @@ -836,6 +850,7 @@ export async function loginOrRefreshIfRequired(
}

export async function login(props?: LoginProps): Promise<boolean> {
console.log("Attempting to login via OAuth...");
const urlToOpen = await getAuthURL(props?.scopes);
await openInBrowser(urlToOpen);
let server: http.Server;
Expand Down Expand Up @@ -903,9 +918,7 @@ export async function login(props?: LoginProps): Promise<boolean> {
res.end(() => {
finish(true);
});
console.log(
`Successfully configured. You can find your configuration file at: ${os.homedir()}/.wrangler/config/default.toml`
);
console.log(`Successfully logged in.`);

return;
}
Expand Down Expand Up @@ -960,14 +973,8 @@ export async function logout(): Promise<void> {
},
});
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"
);
// delete the file
await rm(path.join(os.homedir(), ".wrangler/config/default.toml"));
console.log(
`Removing ${os.homedir()}/.wrangler/config/default.toml.. success!`
);
console.log(`Successfully logged out.`);
}

export function listScopes(): void {
Expand Down

0 comments on commit 0708070

Please sign in to comment.