From 59baa1c579a2d2ecb88b0a12c19140cf8af55177 Mon Sep 17 00:00:00 2001 From: ian nuttall <6681919+iannuttall@users.noreply.github.com> Date: Mon, 31 Aug 2026 13:54:25 +0100 Subject: [PATCH] fix(auth): clarify Google login failures Show browser instructions before Google sign-in and return clear auth errors for timeout, cancellation, and missing permissions. Release the fix as version 0.2.40. Fixes #68 --- .claude-plugin/plugin.json | 2 +- package.json | 2 +- packages/cli/src/commands/auth.ts | 4 +- .../cli/src/commands/google-login.test.ts | 14 ++++ packages/cli/src/commands/google-login.ts | 11 +++ packages/cli/src/commands/setup/prompts.ts | 10 +-- packages/core/src/gsc/auth/callback-page.ts | 15 +++- .../src/gsc/auth/loopback-callback.test.ts | 78 +++++++++++++++++++ .../core/src/gsc/auth/loopback-callback.ts | 38 ++++++++- packages/core/src/gsc/auth/loopback.test.ts | 11 ++- packages/core/src/gsc/auth/loopback.ts | 2 +- 11 files changed, 169 insertions(+), 18 deletions(-) create mode 100644 packages/cli/src/commands/google-login.test.ts create mode 100644 packages/cli/src/commands/google-login.ts diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 91393de5..b78d4d82 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -2,7 +2,7 @@ "name": "seo", "displayName": "seo", "description": "Local-first SEO and AI-search diagnostics. Bundles the seo MCP server plus one SEO skill that gives an agent 50+ audit and report tools without filling its context window.", - "version": "0.2.39", + "version": "0.2.40", "author": { "name": "Ian Nuttall" }, diff --git a/package.json b/package.json index 9505c96a..b288a469 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "seo", - "version": "0.2.39", + "version": "0.2.40", "description": "The SEO command for AI agents. Audit sites and research search opportunities with local, evidence-backed reports.", "type": "module", "license": "Apache-2.0", diff --git a/packages/cli/src/commands/auth.ts b/packages/cli/src/commands/auth.ts index 17ff91ac..4d2d3add 100644 --- a/packages/cli/src/commands/auth.ts +++ b/packages/cli/src/commands/auth.ts @@ -6,7 +6,6 @@ import { getSeoCliPaths, getTokenStorageStatus, listGoogleAccounts, - loginWithLoopback, readTokens, refreshAuthToken, SeoError, @@ -22,6 +21,7 @@ import { printJson, printKeyValue, } from '../utils.js' +import { loginWithGuidance } from './google-login.js' function serviceAccountSource(source?: string): string { if (!source) return 'invalid or conflicting configuration' @@ -91,7 +91,7 @@ export const authCommand = defineCommand({ 'A service account is active from the environment. Unset its credential variable before running browser OAuth.', ) } - const tokens = await loginWithLoopback() + const tokens = await loginWithGuidance() printKeyValue( oauthIdentityRows({ accountEmail: tokens.account_email, diff --git a/packages/cli/src/commands/google-login.test.ts b/packages/cli/src/commands/google-login.test.ts new file mode 100644 index 00000000..29e0cc30 --- /dev/null +++ b/packages/cli/src/commands/google-login.test.ts @@ -0,0 +1,14 @@ +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { googleLoginGuidance } from './google-login.js' + +test('Google login guidance explains every browser step and the time limit', () => { + const guidance = googleLoginGuidance() + + assert.match(guidance, /browser will open/i) + assert.match(guidance, /choose your Google account/i) + assert.match(guidance, /select all permission boxes/i) + assert.match(guidance, /read-only access/i) + assert.match(guidance, /keep this terminal open/i) + assert.match(guidance, /within five minutes/i) +}) diff --git a/packages/cli/src/commands/google-login.ts b/packages/cli/src/commands/google-login.ts new file mode 100644 index 00000000..d0404df6 --- /dev/null +++ b/packages/cli/src/commands/google-login.ts @@ -0,0 +1,11 @@ +import { note } from '@clack/prompts' +import { loginWithLoopback, type StoredTokens } from '@seo/core' + +export function googleLoginGuidance(): string { + return 'Your browser will open. Choose your Google account and select all permission boxes for read-only access. Keep this terminal open and finish the browser steps within five minutes.' +} + +export async function loginWithGuidance(): Promise { + note(googleLoginGuidance(), 'Google sign-in') + return loginWithLoopback() +} diff --git a/packages/cli/src/commands/setup/prompts.ts b/packages/cli/src/commands/setup/prompts.ts index ca14a2bd..da40e440 100644 --- a/packages/cli/src/commands/setup/prompts.ts +++ b/packages/cli/src/commands/setup/prompts.ts @@ -18,7 +18,6 @@ import { listGa4DataStreams, listGoogleAccounts, loadProviderExtensions, - loginWithLoopback, matchGa4WebStreams, type RegisteredProviderExtension, readClickySiteKey, @@ -30,6 +29,7 @@ import { writeProviderExtensionCredentials, } from '@seo/core' import { canPrompt, maybeExitCancelled } from '../../utils.js' +import { loginWithGuidance } from '../google-login.js' import { detectMcpClients } from '../mcp-clients.js' import { installMcpConfig } from '../mcp-config.js' import { installSeoSkill } from '../skill-install.js' @@ -390,7 +390,7 @@ export async function maybeConnectAuth( } if (choice.type === 'skip') return { status: 'skipped' } if (status.sharedConfigured || status.byoConfigured) { - const tokens = await loginWithLoopback() + const tokens = await loginWithGuidance() note( `Connected as ${tokens.account_email}. seo has read-only access and cannot change your site.`, 'Google connected', @@ -427,7 +427,7 @@ export async function maybeConnectAuth( writeOauthClient({ clientId, clientSecret }) } - const tokens = await loginWithLoopback() + const tokens = await loginWithGuidance() note( `Connected as ${tokens.account_email}. seo has read-only access and cannot change your site.`, 'Google connected', @@ -638,7 +638,7 @@ async function chooseGoogleAnalyticsAccount(input: { return accounts.find((account) => account.active)?.accountEmail } if (accounts.length === 0) { - const tokens = await loginWithLoopback() + const tokens = await loginWithGuidance() return tokens.account_email } const choice = maybeExitCancelled( @@ -661,7 +661,7 @@ async function chooseGoogleAnalyticsAccount(input: { }), ) if (choice.type === 'account') return choice.accountEmail - const tokens = await loginWithLoopback() + const tokens = await loginWithGuidance() note( `Connected as ${tokens.account_email}. seo has read-only access and cannot change your site.`, 'Google connected', diff --git a/packages/core/src/gsc/auth/callback-page.ts b/packages/core/src/gsc/auth/callback-page.ts index a853cb19..f443475f 100644 --- a/packages/core/src/gsc/auth/callback-page.ts +++ b/packages/core/src/gsc/auth/callback-page.ts @@ -1,4 +1,5 @@ type OAuthCallbackPageOptions = + | { status: 'cancelled' } | { status: 'connected' } | { status: 'failed' } | { status: 'permissions-missing'; missing: string[] } @@ -15,11 +16,19 @@ function callbackCopy(options: OAuthCallbackPageOptions): { detail: 'This tab can be closed.', } } + if (options.status === 'cancelled') { + return { + title: 'Google sign-in cancelled', + heading: 'Google sign-in was cancelled.', + detail: + 'No account was connected. Return to your terminal when you are ready.', + } + } if (options.status === 'permissions-missing') { return { - title: 'Google permissions not granted', - heading: 'Google permissions were not granted.', - detail: `Return to your terminal and run seo auth login again. Select all permission boxes for ${options.missing.join(' and ')}.`, + title: 'Google permissions not selected', + heading: 'Required Google permissions were not selected.', + detail: `The login needs read-only access for ${options.missing.join(' and ')}. Return to your terminal and run seo auth login again. On Google's permissions screen, choose Select all, then continue.`, } } return { diff --git a/packages/core/src/gsc/auth/loopback-callback.test.ts b/packages/core/src/gsc/auth/loopback-callback.test.ts index 311ef61c..f82b7c7c 100644 --- a/packages/core/src/gsc/auth/loopback-callback.test.ts +++ b/packages/core/src/gsc/auth/loopback-callback.test.ts @@ -1,6 +1,7 @@ import assert from 'node:assert/strict' import http from 'node:http' import test from 'node:test' +import { SeoError } from '../../errors.js' import { waitForCode } from './loopback-callback.js' async function loopbackServer(): Promise<{ @@ -42,3 +43,80 @@ test('a stale OAuth callback does not stop the current login', async (t) => { assert.equal(response.status, 200) assert.equal(await response.text(), 'Connected.') }) + +test('an OAuth timeout is an expected auth failure with clear retry steps', async (t) => { + const { server, redirectUri } = await loopbackServer() + t.after(() => server.close()) + + await assert.rejects( + waitForCode({ + server, + redirectUri, + state: 'current-state', + timeoutMs: 5, + }), + (error: unknown) => { + assert.ok(error instanceof SeoError) + assert.equal(error.code, 'AUTH_REQUIRED') + assert.match(error.message, /No account was connected/) + assert.match(error.message, /seo auth login/) + assert.match(error.message, /within five minutes/) + assert.match(error.message, /Keep the terminal open/) + return true + }, + ) +}) + +test('a cancelled Google login returns a clear access error', async (t) => { + const { server, redirectUri } = await loopbackServer() + t.after(() => server.close()) + + const rejected = assert.rejects( + waitForCode({ + server, + redirectUri, + state: 'current-state', + }), + (error: unknown) => { + assert.ok(error instanceof SeoError) + assert.equal(error.code, 'ACCESS_DENIED') + assert.match(error.message, /sign-in was cancelled/i) + assert.match(error.message, /No account was connected/) + return true + }, + ) + const response = await fetch( + `${redirectUri}?state=current-state&error=access_denied`, + ) + + assert.equal(response.status, 400) + assert.match(await response.text(), /Google sign-in was cancelled/) + await rejected +}) + +test('another OAuth callback error returns a retry step instead of an internal error', async (t) => { + const { server, redirectUri } = await loopbackServer() + t.after(() => server.close()) + + const rejected = assert.rejects( + waitForCode({ + server, + redirectUri, + state: 'current-state', + }), + (error: unknown) => { + assert.ok(error instanceof SeoError) + assert.equal(error.code, 'AUTH_REQUIRED') + assert.match(error.message, /could not finish/i) + assert.match(error.message, /seo auth login/) + return true + }, + ) + const response = await fetch( + `${redirectUri}?state=current-state&error=temporarily_unavailable`, + ) + + assert.equal(response.status, 400) + assert.match(await response.text(), /Google connection failed/) + await rejected +}) diff --git a/packages/core/src/gsc/auth/loopback-callback.ts b/packages/core/src/gsc/auth/loopback-callback.ts index ae4a2ba8..60b16c7c 100644 --- a/packages/core/src/gsc/auth/loopback-callback.ts +++ b/packages/core/src/gsc/auth/loopback-callback.ts @@ -1,10 +1,34 @@ import type http from 'node:http' +import { SeoError } from '../../errors.js' import { oauthCallbackPage } from './callback-page.js' +const OAUTH_CALLBACK_TIMEOUT_MS = 300_000 + +function timeoutError(): SeoError { + return new SeoError( + 'AUTH_REQUIRED', + 'Google sign-in timed out. No account was connected. Run `seo auth login` again and finish the browser steps within five minutes. Keep the terminal open while you sign in.', + ) +} + +function callbackError(error: string): SeoError { + if (error === 'access_denied') { + return new SeoError( + 'ACCESS_DENIED', + 'Google sign-in was cancelled. No account was connected. Run `seo auth login` again when you are ready.', + ) + } + return new SeoError( + 'AUTH_REQUIRED', + 'Google sign-in could not finish. No account was connected. Run `seo auth login` again.', + ) +} + export function waitForCode(input: { server: http.Server redirectUri: string state: string + timeoutMs?: number }): Promise<{ code: string respond: (status: number, page: string) => void @@ -15,8 +39,8 @@ export function waitForCode(input: { }>((resolve, reject) => { const callbackPath = new URL(input.redirectUri).pathname const timer = setTimeout( - () => reject(new Error('OAuth flow timed out after 5 minutes.')), - 300_000, + () => reject(timeoutError()), + input.timeoutMs ?? OAUTH_CALLBACK_TIMEOUT_MS, ) input.server.on('request', (req, res) => { @@ -35,7 +59,15 @@ export function waitForCode(input: { const error = reqUrl.searchParams.get('error') if (error) { - throw new Error(`OAuth error: ${error}`) + res.writeHead(400, { 'content-type': 'text/html; charset=utf-8' }) + res.end( + oauthCallbackPage({ + status: error === 'access_denied' ? 'cancelled' : 'failed', + }), + ) + clearTimeout(timer) + reject(callbackError(error)) + return } const incomingCode = reqUrl.searchParams.get('code') diff --git a/packages/core/src/gsc/auth/loopback.test.ts b/packages/core/src/gsc/auth/loopback.test.ts index 67c95a6f..d605272c 100644 --- a/packages/core/src/gsc/auth/loopback.test.ts +++ b/packages/core/src/gsc/auth/loopback.test.ts @@ -23,7 +23,14 @@ test('OAuth callback page explains when required permissions were not granted', missing: ['Search Console', 'Google Analytics'], }) - assert.match(page, /Google permissions were not granted/) - assert.match(page, /Select all permission boxes/) + assert.match(page, /Required Google permissions were not selected/) + assert.match(page, /choose Select all/) assert.match(page, /Search Console and Google Analytics/) }) + +test('OAuth callback page confirms when the user cancelled sign-in', () => { + const page = oauthCallbackPage({ status: 'cancelled' }) + + assert.match(page, /Google sign-in was cancelled/) + assert.match(page, /No account was connected/) +}) diff --git a/packages/core/src/gsc/auth/loopback.ts b/packages/core/src/gsc/auth/loopback.ts index b95f77d5..ac302301 100644 --- a/packages/core/src/gsc/auth/loopback.ts +++ b/packages/core/src/gsc/auth/loopback.ts @@ -84,7 +84,7 @@ export async function loginWithLoopback( ) throw new SeoError( 'ACCESS_DENIED', - `Google login did not grant ${labels.join(' and ')} read-only access. Run \`seo auth login\` again, choose Select all, and approve both permission boxes.`, + `Google sign-in finished without the required read-only access for ${labels.join(' and ')}. No account was saved. Run \`seo auth login\` again. On Google's permissions screen, choose Select all, then continue.`, ) }