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 5161e1e commit 61aea30
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 18 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)
5 changes: 1 addition & 4 deletions packages/wrangler/src/__tests__/logout.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,7 @@ describe("wrangler", () => {
reinitialiseAuthTokens();
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
13 changes: 13 additions & 0 deletions packages/wrangler/src/__tests__/whoami.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ import React from "react";
import { reinitialiseAuthTokens, writeAuthConfigFile } from "../user";
import { getUserInfo, WhoAmI } from "../whoami";
import { setMockResponse } from "./helpers/mock-cfetch";
import { mockConsoleMethods } from "./helpers/mock-console";
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 () => {
const userInfo = await getUserInfo();
Expand Down Expand Up @@ -51,6 +53,17 @@ describe("getUserInfo()", () => {
],
});
});

it("should display a warning message if the config file contains a legacy api_token field", async () => {
writeAuthConfigFile({ api_token: "API_TOKEN" });
await reinitialiseAuthTokens();
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
6 changes: 3 additions & 3 deletions packages/wrangler/src/cfetch/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,11 @@ async function requireLoggedIn(): Promise<void> {
}

function requireApiToken(): string {
const apiToken = getAPIToken();
if (!apiToken) {
const authToken = getAPIToken();
if (!authToken) {
throw new Error("No API token found.");
}
return apiToken;
return authToken;
}

function addAuthorizationHeader(
Expand Down
29 changes: 18 additions & 11 deletions packages/wrangler/src/user.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,8 @@ interface State extends AuthTokens {
interface AuthTokens {
accessToken?: AccessToken;
refreshToken?: RefreshToken;
/** @deprecated - this field was only provided by the deprecated `wrangler1 config` command. */
apiToken?: string;
}

/**
Expand All @@ -285,7 +287,8 @@ export interface UserAuthConfig {
oauth_token?: string;
refresh_token?: string;
expiration_time?: string;
// api_token?: string;
/** @deprecated - this field was only provided by the deprecated `wrangler1 config` command. */
api_token?: string;
}

interface RefreshToken {
Expand Down Expand Up @@ -357,7 +360,7 @@ function getAuthTokens(): AuthTokens {
}

// otherwise try loading from the user auth config file.
const { oauth_token, refresh_token, expiration_time } =
const { oauth_token, refresh_token, expiration_time, api_token } =
readAuthConfigFile();

if (oauth_token) {
Expand All @@ -369,6 +372,8 @@ function getAuthTokens(): AuthTokens {
},
refreshToken: { value: refresh_token ?? "" },
};
} else if (api_token) {
return { apiToken: api_token };
} else {
return {};
}
Expand All @@ -385,6 +390,13 @@ export function reinitialiseAuthTokens(): void {
}

export function getAPIToken(): string | undefined {
if (LocalState.apiToken) {
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 @@ -847,7 +859,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 === undefined) {
if (!getAPIToken()) {
// Not logged in.
// If we are not interactive, we cannot ask the user to login
return isInteractive && (await login());
Expand All @@ -859,6 +871,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 @@ -930,9 +943,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()}/${USER_AUTH_CONFIG_FILE}`
);
console.log(`Successfully logged in.`);

return;
}
Expand Down Expand Up @@ -1009,12 +1020,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
rmSync(path.join(os.homedir(), USER_AUTH_CONFIG_FILE));
console.log(`Removing ${os.homedir()}/${USER_AUTH_CONFIG_FILE}.. success!`);
console.log(`Successfully logged out.`);
}

export function listScopes(message = "💁 Available scopes:"): void {
Expand Down

0 comments on commit 61aea30

Please sign in to comment.