Skip to content

Commit

Permalink
fix: let folks know the URL we're opening during login (#1267)
Browse files Browse the repository at this point in the history
* fix: let folks know the URL we're opening during login

* Create big-toes-obey.md

* chore: update snapshots

* getting my head around our magic mocks

* Update mock-oauth-flow.ts

* Update packages/wrangler/src/user.tsx

Co-authored-by: Pete Bacon Darwin <[email protected]>

* attempt to get mocks working

* make generateAuthUrl mockable

* add comment

* mock more

* Update generate-random-state.ts

Co-authored-by: Pete Bacon Darwin <[email protected]>
  • Loading branch information
rozenmd and petebacondarwin authored Jun 16, 2022
1 parent d8ee04f commit c667398
Show file tree
Hide file tree
Showing 8 changed files with 102 additions and 28 deletions.
7 changes: 7 additions & 0 deletions .changeset/big-toes-obey.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"wrangler": patch
---

fix: let folks know the URL we're opening during login

Closes #1259
30 changes: 30 additions & 0 deletions packages/wrangler/src/__tests__/jest.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,33 @@ jest.mock("../dev/dev", () => {
// Make sure that we don't accidentally try to open a browser window when running tests.
// We will actually provide a mock implementation for `openInBrowser()` within relevant tests.
jest.mock("../open-in-browser");

// Mock the functions involved in getAuthURL so we don't take snapshots of the constantly changing URL.
jest.mock("../generate-auth-url", () => {
return {
generateRandomState: jest.fn().mockImplementation(() => "MOCK_STATE_PARAM"),
generateAuthUrl: jest
.fn()
.mockImplementation(({ authUrl, clientId, callbackUrl, scopes }) => {
return (
authUrl +
`?response_type=code&` +
`client_id=${encodeURIComponent(clientId)}&` +
`redirect_uri=${encodeURIComponent(callbackUrl)}&` +
// we add offline_access manually for every request
`scope=${encodeURIComponent(
[...scopes, "offline_access"].join(" ")
)}&` +
`state=MOCK_STATE_PARAM&` +
`code_challenge=${encodeURIComponent("MOCK_CODE_CHALLENGE")}&` +
`code_challenge_method=S256`
);
}),
};
});

jest.mock("../generate-random-state", () => {
return {
generateRandomState: jest.fn().mockImplementation(() => "MOCK_STATE_PARAM"),
};
});
3 changes: 2 additions & 1 deletion packages/wrangler/src/__tests__/publish.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ describe("publish", () => {
writeWorkerSource();
mockSubDomainRequest();
mockUploadWorkerRequest();

mockOAuthServerCallback();
const accessTokenRequest = mockGrantAccessToken({ respondWith: "ok" });
mockGrantAuthorization({ respondWith: "success" });
Expand All @@ -79,6 +78,7 @@ describe("publish", () => {

expect(std.out).toMatchInlineSnapshot(`
"Attempting to login via OAuth...
Opening a link in your default browser: https://dash.cloudflare.com/oauth2/auth?response_type=code&client_id=54d11594-84e4-41aa-b438-e81b8fa78ee7&redirect_uri=http%3A%2F%2Flocalhost%3A8976%2Foauth%2Fcallback&scope=account%3Aread%20user%3Aread%20workers%3Awrite%20workers_kv%3Awrite%20workers_routes%3Awrite%20workers_scripts%3Awrite%20workers_tail%3Aread%20pages%3Awrite%20zone%3Aread%20offline_access&state=MOCK_STATE_PARAM&code_challenge=MOCK_CODE_CHALLENGE&code_challenge_method=S256
Successfully logged in.
Uploaded test-name (TIMINGS)
Published test-name (TIMINGS)
Expand Down Expand Up @@ -108,6 +108,7 @@ describe("publish", () => {

expect(std.out).toMatchInlineSnapshot(`
"Attempting to login via OAuth...
Opening a link in your default browser: https://dash.cloudflare.com/oauth2/auth?response_type=code&client_id=54d11594-84e4-41aa-b438-e81b8fa78ee7&redirect_uri=http%3A%2F%2Flocalhost%3A8976%2Foauth%2Fcallback&scope=account%3Aread%20user%3Aread%20workers%3Awrite%20workers_kv%3Awrite%20workers_routes%3Awrite%20workers_scripts%3Awrite%20workers_tail%3Aread%20pages%3Awrite%20zone%3Aread%20offline_access&state=MOCK_STATE_PARAM&code_challenge=MOCK_CODE_CHALLENGE&code_challenge_method=S256
Successfully logged in.
Uploaded test-name (TIMINGS)
Published test-name (TIMINGS)
Expand Down
1 change: 1 addition & 0 deletions packages/wrangler/src/__tests__/user.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ describe("User", () => {

expect(std.out).toMatchInlineSnapshot(`
"Attempting to login via OAuth...
Opening a link in your default browser: https://dash.cloudflare.com/oauth2/auth?response_type=code&client_id=54d11594-84e4-41aa-b438-e81b8fa78ee7&redirect_uri=http%3A%2F%2Flocalhost%3A8976%2Foauth%2Fcallback&scope=account%3Aread%20user%3Aread%20workers%3Awrite%20workers_kv%3Awrite%20workers_routes%3Awrite%20workers_scripts%3Awrite%20workers_tail%3Aread%20pages%3Awrite%20zone%3Aread%20offline_access&state=MOCK_STATE_PARAM&code_challenge=MOCK_CODE_CHALLENGE&code_challenge_method=S256
Successfully logged in."
`);

Expand Down
33 changes: 33 additions & 0 deletions packages/wrangler/src/generate-auth-url.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
interface GenerateAuthUrlProps {
authUrl: string;
clientId: string;
callbackUrl: string;
scopes: string[];
stateQueryParam: string;
codeChallenge: string;
}

/**
* generateAuthUrl was extracted from getAuthURL in user.tsx
* to make it possible to mock the generated URL
*/
export const generateAuthUrl = ({
authUrl,
clientId,
callbackUrl,
scopes,
stateQueryParam,
codeChallenge,
}: GenerateAuthUrlProps) => {
return (
authUrl +
`?response_type=code&` +
`client_id=${encodeURIComponent(clientId)}&` +
`redirect_uri=${encodeURIComponent(callbackUrl)}&` +
// we add offline_access manually for every request
`scope=${encodeURIComponent([...scopes, "offline_access"].join(" "))}&` +
`state=${stateQueryParam}&` +
`code_challenge=${encodeURIComponent(codeChallenge)}&` +
`code_challenge_method=S256`
);
};
16 changes: 16 additions & 0 deletions packages/wrangler/src/generate-random-state.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { webcrypto as crypto } from "node:crypto";
import { PKCE_CHARSET } from "./user";

/**
* Generates random state to be passed for anti-csrf.
* extracted from user.tsx to make it possible to
* mock the generated URL
*/
export function generateRandomState(lengthOfState: number): string {
const output = new Uint32Array(lengthOfState);
// @ts-expect-error crypto's types aren't there yet
crypto.getRandomValues(output);
return Array.from(output)
.map((num: number) => PKCE_CHARSET[num % PKCE_CHARSET.length])
.join("");
}
4 changes: 1 addition & 3 deletions packages/wrangler/src/open-in-browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,8 @@ import { logger } from "./logger";
* @param url the URL to point the browser at
*/
export default async function openInBrowser(url: string): Promise<void> {
const errorMessage = `Failed to open ${url} in a browser`;

const childProcess = await open(url);
childProcess.on("error", () => {
logger.warn(errorMessage);
logger.warn("Failed to open");
});
}
36 changes: 12 additions & 24 deletions packages/wrangler/src/user.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,8 @@ import { fetch } from "undici";
import { getCloudflareApiBaseUrl } from "./cfetch";
import { purgeConfigCaches } from "./config-cache";
import { getEnvironmentVariableFactory } from "./environment-variables";
import { generateAuthUrl } from "./generate-auth-url";
import { generateRandomState } from "./generate-random-state";
import { logger } from "./logger";
import openInBrowser from "./open-in-browser";
import { parseTOML, readFileSync } from "./parse";
Expand Down Expand Up @@ -591,7 +593,7 @@ const RECOMMENDED_STATE_LENGTH = 32;
/**
* Character set to generate code verifier defined in rfc7636.
*/
const PKCE_CHARSET =
export const PKCE_CHARSET =
"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~";

/**
Expand Down Expand Up @@ -641,17 +643,14 @@ export async function getAuthURL(scopes = ScopeKeys): Promise<string> {
stateQueryParam,
});

return (
AUTH_URL +
`?response_type=code&` +
`client_id=${encodeURIComponent(CLIENT_ID)}&` +
`redirect_uri=${encodeURIComponent(CALLBACK_URL)}&` +
// we add offline_access manually for every request
`scope=${encodeURIComponent([...scopes, "offline_access"].join(" "))}&` +
`state=${stateQueryParam}&` +
`code_challenge=${encodeURIComponent(codeChallenge)}&` +
`code_challenge_method=S256`
);
return generateAuthUrl({
authUrl: AUTH_URL,
clientId: CLIENT_ID,
callbackUrl: CALLBACK_URL,
scopes,
stateQueryParam,
codeChallenge,
});
}

type TokenResponse =
Expand Down Expand Up @@ -863,18 +862,6 @@ async function generatePKCECodes(): Promise<PKCECodes> {
return { codeChallenge, codeVerifier };
}

/**
* Generates random state to be passed for anti-csrf.
*/
function generateRandomState(lengthOfState: number): string {
const output = new Uint32Array(lengthOfState);
// @ts-expect-error crypto's types aren't there yet
crypto.getRandomValues(output);
return Array.from(output)
.map((num: number) => PKCE_CHARSET[num % PKCE_CHARSET.length])
.join("");
}

/**
* Writes a a wrangler config file (auth credentials) to disk,
* and updates the user auth state with the new credentials.
Expand Down Expand Up @@ -1013,6 +1000,7 @@ export async function login(props?: LoginProps): Promise<boolean> {
server.listen(8976);
});

logger.log(`Opening a link in your default browser: ${urlToOpen}`);
await openInBrowser(urlToOpen);

return Promise.race([timerPromise, loginPromise]);
Expand Down

0 comments on commit c667398

Please sign in to comment.