From a1e4314a2b82a667c6308490d72a293f2558ea5a Mon Sep 17 00:00:00 2001 From: Bala Vignesh S Date: Thu, 23 Apr 2026 21:08:14 +0530 Subject: [PATCH] feat: make OAuth device auth the default authentication method Closes #230 - Remove Confirm.prompt gate in onboard flow; OAuth device auth now starts automatically when credentials are missing - Gracefully fall back to manual instructions if device auth fails (no browser, network issues, etc.) - Add 'login' and 'upgrade' to auth-exempt commands list (also addresses #242: vt upgrade no longer requires authentication) - Fix token length validation in ensureGlobalVtConfig to accept 43-char OAuth access tokens alongside 32-33 char API keys - Update 401/permissions error messages to suggest 'vt login' as primary recovery action - Fix missing await on globalConfig.saveGlobalConfig() in onboard - Use API_KEY_KEY constant instead of hardcoded string - Update Zod schema validation error message to mention OAuth tokens - Add comprehensive test suite (8 tests) covering schema validation, auth-exempt commands, and error message improvements --- src/cmd/flows/onboard.ts | 51 ++++++---- src/cmd/tests/oauth_default_test.ts | 152 ++++++++++++++++++++++++++++ src/cmd/utils.ts | 4 +- src/vt/VTConfig.ts | 4 +- src/vt/vt/schemas.ts | 3 +- vt.ts | 7 +- 6 files changed, 194 insertions(+), 27 deletions(-) create mode 100644 src/cmd/tests/oauth_default_test.ts diff --git a/src/cmd/flows/onboard.ts b/src/cmd/flows/onboard.ts index 3311acd2..9d73d205 100644 --- a/src/cmd/flows/onboard.ts +++ b/src/cmd/flows/onboard.ts @@ -1,15 +1,13 @@ -import { Confirm } from "@cliffy/prompt"; import { colors } from "@cliffy/ansi/colors"; import { + API_KEY_KEY, DEFAULT_WRAP_WIDTH, - GET_API_KEY_URL, GLOBAL_VT_CONFIG_PATH, VT_README_URL, } from "~/consts.ts"; import { ensureDir } from "@std/fs"; import wrap from "word-wrap"; import { globalConfig } from "~/vt/VTConfig.ts"; -import { delay } from "@std/async"; import { oicdLoginFlow } from "../../oauth.ts"; /** @@ -40,13 +38,15 @@ function welcomeToVt(): void { /** * The onboarding flow for users using vt for the first time. This handles - * walking the user through setting their API key and informing them on how to - * get started. + * walking the user through authenticating via OAuth device authorization flow + * and informing them on how to get started. + * + * OAuth is the default authentication method. Users who need to use an API + * key directly (e.g., for CI environments) can use: + * `vt config set apiKey ` * * @param options Options for the onboarding flow * @param options.showWelcome Whether to show the welcome message - * @param options.showApiKeyPrompt Whether to show the API key prompt - * @param options.openBrowser Whether to open the browser to get the API key */ export async function onboardFlow( options?: { showWelcome?: boolean }, @@ -56,28 +56,22 @@ export async function onboardFlow( if (options.showWelcome) { welcomeToVt(); console.log(); - - console.log(" To get started, you need to authenticate with Val Town."); - console.log(); } - const goToWebsite: boolean = await Confirm.prompt({ - message: - `We can log you in automatically! Would you like to proceed to vt's login page in your browser?\n`, - }); + console.log(" To get started, you need to authenticate with Val Town."); + console.log(); - if (goToWebsite) { - await delay(300); + try { const tokens = await oicdLoginFlow(); // Set the API key in the environment for the current session - Deno.env.set("VAL_TOWN_API_KEY", tokens.access_token); + Deno.env.set(API_KEY_KEY, tokens.access_token); // Ensure the global config directory exists await ensureDir(GLOBAL_VT_CONFIG_PATH); // Add the API key to the config - globalConfig.saveGlobalConfig({ + await globalConfig.saveGlobalConfig({ apiKey: tokens.access_token, refreshToken: tokens.refresh_token, }); @@ -88,12 +82,25 @@ export async function onboardFlow( `head over to ${VT_README_URL}`, ); console.log(); - } else { + } catch (error) { + // If the device auth flow fails (e.g., no browser, network issues), + // fall back to showing the manual API key instructions. + console.log(); + console.log( + colors.yellow( + "Automatic login failed" + + (error instanceof Error ? `: ${error.message}` : "."), + ), + ); console.log(); console.log( - "You can get an API key at " + GET_API_KEY_URL + - " with user read, val read + write, telemetry read permissions, " + - "and come back and run `vt config set apiKey ` when you are ready.", + "You can authenticate manually by running:\n" + + colors.cyan(" vt login") + + "\n\nor by setting an API key directly:\n" + + colors.cyan(" vt config set apiKey ") + + "\n\nYou can generate an API key at " + + colors.cyan("https://www.val.town/settings/api") + + " with user read, val read + write, telemetry read permissions.", ); } } diff --git a/src/cmd/tests/oauth_default_test.ts b/src/cmd/tests/oauth_default_test.ts new file mode 100644 index 00000000..7a49182f --- /dev/null +++ b/src/cmd/tests/oauth_default_test.ts @@ -0,0 +1,152 @@ +import { assertEquals, assertStringIncludes } from "@std/assert"; +import { VTConfigSchema } from "~/vt/vt/schemas.ts"; +import { doWithTempDir } from "~/vt/lib/utils/misc.ts"; +import { runVtCommand } from "~/cmd/tests/utils.ts"; +import { join } from "@std/path"; +import { ensureDir } from "@std/fs"; +import { stringify as stringifyYaml } from "@std/yaml"; + +/** + * Tests for issue #230: OAuth as default authentication. + * + * These tests verify: + * 1. The config schema accepts OAuth token lengths (43 chars) + * 2. Auth-exempt commands (login, logout, upgrade) don't require auth + * 3. Error messages reference `vt login` as the primary recovery action + */ + +// --------------------------------------------------------------------------- +// Schema validation: OAuth token length acceptance +// --------------------------------------------------------------------------- + +Deno.test({ + name: "VTConfigSchema accepts 32-char API keys", + fn() { + const key = "a".repeat(32); + const result = VTConfigSchema.safeParse({ apiKey: key }); + assertEquals(result.success, true); + }, +}); + +Deno.test({ + name: "VTConfigSchema accepts 33-char API keys", + fn() { + const key = "a".repeat(33); + const result = VTConfigSchema.safeParse({ apiKey: key }); + assertEquals(result.success, true); + }, +}); + +Deno.test({ + name: "VTConfigSchema accepts 43-char OAuth access tokens", + fn() { + const key = "a".repeat(43); + const result = VTConfigSchema.safeParse({ apiKey: key }); + assertEquals(result.success, true); + }, +}); + +Deno.test({ + name: "VTConfigSchema rejects invalid token lengths", + fn() { + const key = "a".repeat(20); + const result = VTConfigSchema.safeParse({ apiKey: key }); + assertEquals(result.success, false); + }, +}); + +Deno.test({ + name: "VTConfigSchema accepts null apiKey", + fn() { + const result = VTConfigSchema.safeParse({ apiKey: null }); + assertEquals(result.success, true); + }, +}); + +Deno.test({ + name: "VTConfigSchema accepts refreshToken alongside apiKey", + fn() { + const result = VTConfigSchema.safeParse({ + apiKey: "a".repeat(43), + refreshToken: "some_refresh_token_value", + }); + assertEquals(result.success, true); + }, +}); + +Deno.test({ + name: "VTConfigSchema accepts null refreshToken", + fn() { + const result = VTConfigSchema.safeParse({ + apiKey: "a".repeat(32), + refreshToken: null, + }); + assertEquals(result.success, true); + }, +}); + +// --------------------------------------------------------------------------- +// Auth-exempt commands: login, logout, upgrade should not require auth +// --------------------------------------------------------------------------- + +Deno.test({ + name: "vt upgrade does not require authentication", + permissions: "inherit", + async fn() { + await doWithTempDir(async (tmpDir) => { + // Create a minimal config with a null (invalid) apiKey so that + // ensureValidApiKey() would normally trigger the onboard flow. + const configDir = join(tmpDir, "vt"); + await ensureDir(configDir); + await Deno.writeTextFile( + join(configDir, "config.yaml"), + stringifyYaml({ apiKey: null }), + ); + + // `vt upgrade` should run without triggering auth. It will fail + // because there's no real upgrade to do, but it should NOT prompt + // for login or crash with an auth error. + const [output, _code] = await runVtCommand( + ["upgrade"], + tmpDir, + { env: { "XDG_CONFIG_HOME": tmpDir }, autoConfirm: false }, + ); + + // The output should NOT contain the onboarding/auth prompt text + assertEquals( + output.includes("authenticate with Val Town"), + false, + "upgrade command should not trigger authentication flow", + ); + }); + }, + sanitizeResources: false, +}); + +// --------------------------------------------------------------------------- +// Error message improvements +// --------------------------------------------------------------------------- + +Deno.test({ + name: "sanitizeErrors for 401 mentions vt login", + async fn() { + // Import and test the sanitizeErrors function directly + const { sanitizeErrors } = await import("~/cmd/utils.ts"); + const ValTown = (await import("@valtown/sdk")).default; + + // Create a mock 401 error + const error = new ValTown.APIError( + 401, + { message: "Unauthorized" }, + "Unauthorized", + {}, + ); + + const message = sanitizeErrors(error); + assertStringIncludes( + message, + "vt login", + "401 error message should suggest `vt login`", + ); + }, +}); diff --git a/src/cmd/utils.ts b/src/cmd/utils.ts index 510bf230..c6b5302f 100644 --- a/src/cmd/utils.ts +++ b/src/cmd/utils.ts @@ -41,12 +41,12 @@ export function sanitizeErrors(error: unknown): string { } } else if (error.status === 401) { suffixedExtra = - "You may need to re-authenticate. To set a new API key, use `vt config set apiKey new_api_key`"; + "You may need to re-authenticate. Run `vt login` to log in again, or use `vt config set apiKey ` to set an API key directly."; } if (error.message.includes("required permissions")) { suffixedExtra += - "To set a new API key, use `vt config set apiKey new_api_key`"; + "Run `vt login` to re-authenticate, or use `vt config set apiKey ` to set an API key directly."; } // Remove leading numbers from error message diff --git a/src/vt/VTConfig.ts b/src/vt/VTConfig.ts index 534da617..87b7e86c 100644 --- a/src/vt/VTConfig.ts +++ b/src/vt/VTConfig.ts @@ -181,7 +181,9 @@ export async function ensureGlobalVtConfig(): Promise { // be overwriting anything) if (Deno.env.has(API_KEY_KEY)) { const apiKey = Deno.env.get(API_KEY_KEY)!; // (!, we just checked) - if (apiKey.length == 32 || apiKey.length == 33) { + if (apiKey.length == 32 || apiKey.length == 33 || apiKey.length == 43) { + // 32-33 chars: traditional API keys (vtwn_...) + // 43 chars: OAuth access tokens startingConfig.apiKey = apiKey; } else startingConfig.apiKey = "0".repeat(32); } diff --git a/src/vt/vt/schemas.ts b/src/vt/vt/schemas.ts index d2142c02..a6c37ed8 100644 --- a/src/vt/vt/schemas.ts +++ b/src/vt/vt/schemas.ts @@ -59,7 +59,8 @@ export const VTConfigSchema = z.object({ val === null || val.length === 32 || val.length === 33 || val.length === 43, { // 43 is for oauth - message: "API key must be 32-33 characters long when provided", + message: + "API key must be 32-33 characters, or a 43-character OAuth token", }, ) .nullable(), diff --git a/vt.ts b/vt.ts index 0b032a91..1794b0c3 100755 --- a/vt.ts +++ b/vt.ts @@ -98,7 +98,12 @@ if (import.meta.main) { await registerOutdatedWarning(); } - if (!["logout"].includes(Deno.args[0])) { + // Commands that don't require authentication: + // - logout: clearing credentials shouldn't need valid credentials + // - login: this IS the auth flow; requiring auth first is circular + // - upgrade: updating the CLI binary doesn't need API access + const authExemptCommands = ["logout", "login", "upgrade"]; + if (!authExemptCommands.includes(Deno.args[0])) { await ensureValidApiKey(); }