diff --git a/apps/web/src/app/(app)/claw/components/CalendarConnectStep.tsx b/apps/web/src/app/(app)/claw/components/CalendarConnectStep.tsx new file mode 100644 index 0000000000..4585294191 --- /dev/null +++ b/apps/web/src/app/(app)/claw/components/CalendarConnectStep.tsx @@ -0,0 +1,147 @@ +'use client'; + +import Link from 'next/link'; +import { Calendar, Check, X } from 'lucide-react'; +import { Button } from '@/components/ui/button'; +import { cn } from '@/lib/utils'; +import { OnboardingStepView } from './OnboardingStepView'; + +type CalendarConnectStepViewProps = { + currentStep: number; + totalSteps: number; + connectUrl: string; + isConnected: boolean; + connectedAccountEmail?: string | null; + /** + * Whether the kiloclaw instance row is provisioned. The + * /api/integrations/google/connect route requires an active instance and + * bounces the user out of onboarding when it can't find one, so the Connect + * button stays disabled until the instance row exists. + */ + instanceReady: boolean; + onSkip: () => void; + onContinue: () => void; + onConnectClick?: () => void; +}; + +const FEATURES: Array<{ included: boolean; title: string; detail: string }> = [ + { + included: true, + title: 'Read your calendar events', + detail: 'Titles, attendees, locations, descriptions for the next 14 days.', + }, + { + included: true, + title: 'Read calendars you own and subscribe to', + detail: 'Including team calendars shared with you.', + }, + { + included: false, + title: 'Create, modify, or delete events', + detail: "We don't request write access.", + }, +]; + +export function CalendarConnectStepView({ + currentStep, + totalSteps, + connectUrl, + isConnected, + connectedAccountEmail, + instanceReady, + onSkip, + onContinue, + onConnectClick, +}: CalendarConnectStepViewProps) { + return ( + +
+
+
+
+ +
+
+

Google Calendar

+

+ {isConnected && connectedAccountEmail + ? `Connected as ${connectedAccountEmail}` + : 'via OAuth · read-only'} +

+
+
+ + {isConnected ? 'Connected' : 'Recommended'} + +
+ +
+ {FEATURES.map(feature => ( +
+
+ {feature.included ? : } +
+
+

+ {feature.title} +

+

{feature.detail}

+
+
+ ))} +
+ +
+ + {isConnected ? ( + + ) : instanceReady ? ( + + ) : ( + + )} +
+
+ + ); +} diff --git a/apps/web/src/app/(app)/claw/components/ClawOnboardingFakeWalkthrough.tsx b/apps/web/src/app/(app)/claw/components/ClawOnboardingFakeWalkthrough.tsx index 58b590a66b..f696357233 100644 --- a/apps/web/src/app/(app)/claw/components/ClawOnboardingFakeWalkthrough.tsx +++ b/apps/web/src/app/(app)/claw/components/ClawOnboardingFakeWalkthrough.tsx @@ -19,11 +19,13 @@ import { ChannelPairingStepView } from './ChannelPairingStep'; import { ChannelSelectionStepView } from './ChannelSelectionStep'; import { ClawConfigServiceBanner } from './ClawConfigServiceBanner'; import { ClawHeader } from './ClawHeader'; +import { CalendarConnectStepView } from './CalendarConnectStep'; import { ClawSetupCompleteStep, ClawSetupErrorStep } from './ClawOnboardingFlow'; import { ProvisioningStepView } from './ProvisioningStep'; const FAKE_STEP_LABELS: Record = { identity: 'Identity', + calendar: 'Calendar', channels: 'Channels', provisioning: 'Provisioning', pairing: 'Pairing', @@ -187,6 +189,7 @@ function getFakeStepProgress( function getFakeOnboardingStep(step: ClawOnboardingRenderStep): OnboardingStep { switch (step) { case 'identity': + case 'calendar': case 'channels': case 'provisioning': case 'pairing': @@ -208,7 +211,21 @@ function renderFakeStep({ }: RenderFakeStepInput) { switch (step) { case 'identity': { - return setStep('channels')} />; + return setStep('calendar')} />; + } + case 'calendar': { + return ( + setStep('channels')} + onSkip={() => setStep('channels')} + onContinue={() => setStep('channels')} + /> + ); } case 'channels': { return ( diff --git a/apps/web/src/app/(app)/claw/components/ClawOnboardingFlow.state.test.ts b/apps/web/src/app/(app)/claw/components/ClawOnboardingFlow.state.test.ts index 18868d5d55..04cb918559 100644 --- a/apps/web/src/app/(app)/claw/components/ClawOnboardingFlow.state.test.ts +++ b/apps/web/src/app/(app)/claw/components/ClawOnboardingFlow.state.test.ts @@ -121,6 +121,15 @@ describe('ClawOnboardingFlow state machine', () => { expect(getClawOnboardingFlowState(createInput({ createSetupStarted: true })).renderStep).toBe( 'identity' ); + expect( + getClawOnboardingFlowState( + createInput({ + createSetupStarted: true, + onboardingStep: 'calendar', + hasBotIdentity: true, + }) + ).renderStep + ).toBe('calendar'); expect( getClawOnboardingFlowState( createInput({ @@ -159,60 +168,68 @@ describe('ClawOnboardingFlow state machine', () => { ).toBe('complete'); }); - test('uses four steps only when the selected channel requires pairing', () => { + test('uses five steps only when the selected channel requires pairing', () => { const pairingTelegram = getClawOnboardingFlowState( createInput({ selectedChannelId: 'telegram' }) ); - expect(pairingTelegram.totalSteps).toBe(4); + expect(pairingTelegram.totalSteps).toBe(5); expect(pairingTelegram.currentStep).toBe(1); const pairingDiscord = getClawOnboardingFlowState( createInput({ selectedChannelId: 'discord' }) ); - expect(pairingDiscord.totalSteps).toBe(4); + expect(pairingDiscord.totalSteps).toBe(5); expect(pairingDiscord.currentStep).toBe(1); const noPairingSlack = getClawOnboardingFlowState(createInput({ selectedChannelId: 'slack' })); - expect(noPairingSlack.totalSteps).toBe(3); + expect(noPairingSlack.totalSteps).toBe(4); expect(noPairingSlack.currentStep).toBe(1); const defaultState = getClawOnboardingFlowState(createInput()); - expect(defaultState.totalSteps).toBe(3); + expect(defaultState.totalSteps).toBe(4); expect(defaultState.currentStep).toBe(1); }); test('getClawOnboardingStepProgress returns correct live current and total steps', () => { expect(getClawOnboardingStepProgress('identity', false)).toEqual({ currentStep: 1, - totalSteps: 3, + totalSteps: 4, }); - expect(getClawOnboardingStepProgress('channels', false)).toEqual({ + expect(getClawOnboardingStepProgress('calendar', false)).toEqual({ currentStep: 2, - totalSteps: 3, + totalSteps: 4, }); - expect(getClawOnboardingStepProgress('provisioning', false)).toEqual({ + expect(getClawOnboardingStepProgress('channels', false)).toEqual({ currentStep: 3, - totalSteps: 3, + totalSteps: 4, + }); + expect(getClawOnboardingStepProgress('provisioning', false)).toEqual({ + currentStep: 4, + totalSteps: 4, }); - expect(getClawOnboardingStepProgress('done', false)).toEqual({ currentStep: 3, totalSteps: 3 }); + expect(getClawOnboardingStepProgress('done', false)).toEqual({ currentStep: 4, totalSteps: 4 }); expect(getClawOnboardingStepProgress('identity', true)).toEqual({ currentStep: 1, - totalSteps: 4, + totalSteps: 5, }); - expect(getClawOnboardingStepProgress('channels', true)).toEqual({ + expect(getClawOnboardingStepProgress('calendar', true)).toEqual({ currentStep: 2, - totalSteps: 4, + totalSteps: 5, }); - expect(getClawOnboardingStepProgress('provisioning', true)).toEqual({ + expect(getClawOnboardingStepProgress('channels', true)).toEqual({ currentStep: 3, - totalSteps: 4, + totalSteps: 5, }); - expect(getClawOnboardingStepProgress('pairing', true)).toEqual({ + expect(getClawOnboardingStepProgress('provisioning', true)).toEqual({ currentStep: 4, - totalSteps: 4, + totalSteps: 5, + }); + expect(getClawOnboardingStepProgress('pairing', true)).toEqual({ + currentStep: 5, + totalSteps: 5, }); - expect(getClawOnboardingStepProgress('done', true)).toEqual({ currentStep: 4, totalSteps: 4 }); + expect(getClawOnboardingStepProgress('done', true)).toEqual({ currentStep: 5, totalSteps: 5 }); }); test.each(CLAW_ONBOARDING_PROVISIONING_STATUSES)( diff --git a/apps/web/src/app/(app)/claw/components/ClawOnboardingFlow.state.ts b/apps/web/src/app/(app)/claw/components/ClawOnboardingFlow.state.ts index 24f6fa4d5a..8b6643f76b 100644 --- a/apps/web/src/app/(app)/claw/components/ClawOnboardingFlow.state.ts +++ b/apps/web/src/app/(app)/claw/components/ClawOnboardingFlow.state.ts @@ -6,10 +6,17 @@ export type PopulatedClawStatus = KiloClawDashboardStatus & { export type ClawOnboardingMode = 'create-first' | 'post-provisioning'; -export type OnboardingStep = 'identity' | 'channels' | 'provisioning' | 'pairing' | 'done'; +export type OnboardingStep = + | 'identity' + | 'calendar' + | 'channels' + | 'provisioning' + | 'pairing' + | 'done'; export const CLAW_ONBOARDING_WIZARD_STEPS = [ 'identity', + 'calendar', 'channels', 'provisioning', 'pairing', @@ -19,6 +26,7 @@ export type ClawOnboardingWizardStep = (typeof CLAW_ONBOARDING_WIZARD_STEPS)[num export type ClawOnboardingRenderStep = | 'identity' + | 'calendar' | 'channels' | 'provisioning' | 'pairing' @@ -31,6 +39,7 @@ export const FAKE_ONBOARDING_STEP_PARAM = 'fakeOnboardingStep'; export const CLAW_ONBOARDING_FAKE_STEPS = [ 'identity', + 'calendar', 'channels', 'provisioning', 'pairing', @@ -280,6 +289,13 @@ function getRenderStepDecision({ }; } + if (onboardingStep === 'calendar') { + return { + renderStep: 'calendar', + reason: 'stored onboarding step is calendar', + }; + } + if (onboardingStep === 'channels') { return { renderStep: 'channels', diff --git a/apps/web/src/app/(app)/claw/components/ClawOnboardingFlow.tsx b/apps/web/src/app/(app)/claw/components/ClawOnboardingFlow.tsx index 19a33ba347..86b940e1b2 100644 --- a/apps/web/src/app/(app)/claw/components/ClawOnboardingFlow.tsx +++ b/apps/web/src/app/(app)/claw/components/ClawOnboardingFlow.tsx @@ -2,7 +2,7 @@ import { useCallback, useEffect, useRef, useState } from 'react'; import Link from 'next/link'; -import { useRouter } from 'next/navigation'; +import { usePathname, useRouter, useSearchParams } from 'next/navigation'; import { useFeatureFlagVariantKey, usePostHog } from 'posthog-js/react'; import { toast } from 'sonner'; import { Loader2, TriangleAlert, X } from 'lucide-react'; @@ -18,13 +18,14 @@ import { useOnboardingSaves } from '../hooks/useOnboardingSaves'; import { useGatewayUrl } from '../hooks/useGatewayUrl'; import { BillingWrapper } from './billing/BillingWrapper'; import { BotIdentityStep } from './BotIdentityStep'; +import { CalendarConnectStepView } from './CalendarConnectStep'; import { ChannelPairingStep } from './ChannelPairingStep'; import { ChannelSelectionStepView } from './ChannelSelectionStep'; import { ClawContextProvider, useClawContext } from './ClawContext'; import { ClawConfigServiceBanner } from './ClawConfigServiceBanner'; import { ClawHeader } from './ClawHeader'; import { ProvisioningStep, ProvisioningStepView } from './ProvisioningStep'; -import { DEFAULT_ONBOARDING_EXEC_PRESET } from './claw.types'; +import { DEFAULT_BOT_IDENTITY, DEFAULT_ONBOARDING_EXEC_PRESET } from './claw.types'; import type { BotIdentity, ExecPreset } from './claw.types'; import { getClawOnboardingFlowState, @@ -123,6 +124,7 @@ function ClawOnboardingFlowInner({ const [localCreateSetupStarted, setLocalCreateSetupStarted] = useState(false); const [onboardingSaveSession, setOnboardingSaveSession] = useState(0); const hasCapturedIdentityView = useRef(false); + const hasCapturedCalendarView = useRef(false); const hasCapturedDoneView = useRef(false); const createSetupStarted = createFlowStarted || localCreateSetupStarted; @@ -159,6 +161,8 @@ function ClawOnboardingFlowInner({ useFeatureFlagVariantKey('button-vs-card'); const posthog = usePostHog(); const router = useRouter(); + const pathname = usePathname(); + const searchParams = useSearchParams(); // Save bot identity, exec preset, and channel tokens as soon as the instance // row exists. This closes the tab-close window where customizations entered @@ -182,6 +186,16 @@ function ClawOnboardingFlowInner({ posthog?.capture('claw_setup_identity_viewed'); }, [flowState.renderStep, posthog]); + // Fire `claw_setup_calendar_viewed` when the calendar step actually + // renders, matching the "viewed = rendered" semantic of identity above + // (and unlike the older advance-fire pattern still used by channels and + // provisioning). Ref guard so re-renders inside the step don't re-fire. + useEffect(() => { + if (flowState.renderStep !== 'calendar' || hasCapturedCalendarView.current) return; + hasCapturedCalendarView.current = true; + posthog?.capture('claw_setup_calendar_viewed'); + }, [flowState.renderStep, posthog]); + useEffect(() => { if ( mode !== 'post-provisioning' || @@ -217,6 +231,99 @@ function ClawOnboardingFlowInner({ const basePath = organizationId ? `/organizations/${organizationId}/claw` : '/claw'; + // Hydrate local bot-identity state from the persisted instance status when + // the page reloads mid-onboarding (e.g. after the OAuth round-trip on the + // calendar step). useOnboardingSaves writes the user's identity selections + // to the backend; without this, a remount would force the user back to the + // identity step even though their picks were already saved. + useEffect(() => { + if (botIdentity !== null) return; + const persisted = flowState.instanceStatus; + if (!persisted?.botName) return; + setBotIdentity({ + botName: persisted.botName, + botNature: persisted.botNature ?? DEFAULT_BOT_IDENTITY.botNature, + botVibe: persisted.botVibe ?? DEFAULT_BOT_IDENTITY.botVibe, + botEmoji: persisted.botEmoji ?? DEFAULT_BOT_IDENTITY.botEmoji, + }); + }, [flowState.instanceStatus, botIdentity]); + + // Resume the wizard at a specific step when returning from a flow that + // leaves the page (e.g. the Google OAuth round-trip on the calendar step + // posts the user back to /claw/new?step=calendar). The effect only acts + // when stepParam === 'calendar' — otherwise stale `?error=` or `?success=` + // params from elsewhere would fire calendar-specific toasts on the wrong + // screen. Also waits until botIdentity has been hydrated before consuming + // `step`, otherwise the state machine would override us with identity. + const hasResumedFromQuery = useRef(false); + + // Allowlist of known OAuth error codes that the callback route can emit. + // Anything else from `?error=` is bucketed as 'unknown' before going to + // PostHog so an attacker can't pollute analytics with arbitrary strings. + const KNOWN_OAUTH_ERROR_CODES = [ + 'access_denied', + 'oauth_error', + 'missing_code', + 'missing_instance', + 'connection_failed', + 'invalid_state', + 'unauthorized', + ]; + const cleanupResumeQueryParams = useCallback(() => { + const next = new URLSearchParams(searchParams?.toString() ?? ''); + next.delete('step'); + next.delete('success'); + next.delete('error'); + const nextSearch = next.toString(); + router.replace(nextSearch ? `${pathname}?${nextSearch}` : (pathname ?? '/claw/new')); + }, [pathname, router, searchParams]); + + useEffect(() => { + if (hasResumedFromQuery.current) return; + const stepParam = searchParams?.get('step'); + if (stepParam !== 'calendar') return; + if (botIdentity === null) return; + const successParam = searchParams?.get('success'); + const errorParamRaw = searchParams?.get('error'); + const errorReason = errorParamRaw + ? KNOWN_OAUTH_ERROR_CODES.includes(errorParamRaw) + ? errorParamRaw + : 'unknown' + : null; + hasResumedFromQuery.current = true; + setOnboardingStep('calendar'); + posthog?.capture('claw_setup_calendar_resumed', { + outcome: + successParam === 'google_connected' ? 'connected' : errorParamRaw ? 'error' : 'unknown', + }); + if (successParam === 'google_connected') { + posthog?.capture('claw_setup_calendar_oauth_completed'); + toast.success('Calendar connected'); + } else if (errorParamRaw) { + posthog?.capture('claw_setup_calendar_oauth_failed', { reason: errorReason }); + toast.error('Could not connect calendar — please try again or skip for now.'); + } + cleanupResumeQueryParams(); + }, [searchParams, botIdentity, posthog, cleanupResumeQueryParams]); + + // Watchdog: if `?step=calendar` is in the URL but botIdentity hydration + // never completes (e.g. patchBotIdentity hadn't propagated to the DB + // before the OAuth round-trip), don't silently strand the user on the + // identity step with stale params lingering in the URL. After a short + // grace period, clean the URL and surface a soft warning. + useEffect(() => { + if (hasResumedFromQuery.current) return; + if (searchParams?.get('step') !== 'calendar') return; + const timeoutId = window.setTimeout(() => { + if (hasResumedFromQuery.current) return; + if (botIdentity !== null) return; + hasResumedFromQuery.current = true; + toast.error("Couldn't restore your onboarding progress — continuing from here."); + cleanupResumeQueryParams(); + }, 5000); + return () => window.clearTimeout(timeoutId); + }, [searchParams, botIdentity, cleanupResumeQueryParams]); + // NOTE: When mode === 'post-provisioning' (i.e. an existing instance is // already running) and the gateway is ready, renderStep is 'complete' on // first render and the redirect below fires immediately. This is intentional: @@ -295,9 +402,46 @@ function ClawOnboardingFlowInner({ preset: DEFAULT_ONBOARDING_EXEC_PRESET, defaulted: true, }); - posthog?.capture('claw_setup_channels_viewed'); setBotIdentity(identity); - setOnboardingStep('channels'); + setOnboardingStep('calendar'); + }} + /> + ); + } + + function renderCalendarStep() { + const returnTo = `${basePath}/new?step=calendar`; + const connectParams = new URLSearchParams({ returnTo }); + if (organizationId) { + connectParams.set('organizationId', organizationId); + } + const connectUrl = `/api/integrations/google/connect?${connectParams.toString()}`; + const isConnected = Boolean(flowState.instanceStatus?.googleOAuthConnected); + const connectedEmail = flowState.instanceStatus?.googleOAuthAccountEmail ?? null; + + function advanceToChannels() { + posthog?.capture('claw_setup_channels_viewed'); + setOnboardingStep('channels'); + } + + return ( + { + posthog?.capture('claw_setup_calendar_connect_clicked', { skipped: false }); + }} + onSkip={() => { + posthog?.capture('claw_setup_calendar_completed', { connected: false, skipped: true }); + advanceToChannels(); + }} + onContinue={() => { + posthog?.capture('claw_setup_calendar_completed', { connected: true, skipped: false }); + advanceToChannels(); }} /> ); @@ -402,6 +546,8 @@ function ClawOnboardingFlowInner({ switch (renderStep) { case 'identity': return renderIdentityStep(); + case 'calendar': + return renderCalendarStep(); case 'channels': return renderChannelsStep(); case 'provisioning': diff --git a/apps/web/src/app/api/integrations/google/callback/route.test.ts b/apps/web/src/app/api/integrations/google/callback/route.test.ts index 11b45c281d..b010e68faf 100644 --- a/apps/web/src/app/api/integrations/google/callback/route.test.ts +++ b/apps/web/src/app/api/integrations/google/callback/route.test.ts @@ -116,6 +116,42 @@ describe('GET /api/integrations/google/callback', () => { expectRedirectLocation(response, '/users/sign_in'); }); + test('redirects personal success flow to returnTo when state carries one', async () => { + mockedVerifyGoogleOAuthState.mockReturnValue({ + owner: { type: 'user', id: USER_ID }, + userId: USER_ID, + instanceId: INSTANCE_ID, + capabilities: ['calendar_read'], + returnTo: '/claw/new?step=calendar', + }); + + const { GET } = await import('./route'); + const response = await GET( + makeRequest('/api/integrations/google/callback?code=abc&state=signed') as never + ); + + expect(response.status).toBe(307); + expectRedirectLocation(response, '/claw/new?step=calendar&success=google_connected'); + }); + + test('redirects personal error flow to returnTo when state carries one', async () => { + mockedVerifyGoogleOAuthState.mockReturnValue({ + owner: { type: 'user', id: USER_ID }, + userId: USER_ID, + instanceId: INSTANCE_ID, + capabilities: ['calendar_read'], + returnTo: '/claw/new?step=calendar', + }); + + const { GET } = await import('./route'); + const response = await GET( + makeRequest('/api/integrations/google/callback?error=access_denied&state=signed') as never + ); + + expect(response.status).toBe(307); + expectRedirectLocation(response, '/claw/new?step=calendar&error=access_denied'); + }); + test('redirects personal success flow to personal claw settings', async () => { const { GET } = await import('./route'); const response = await GET( diff --git a/apps/web/src/app/api/integrations/google/callback/route.ts b/apps/web/src/app/api/integrations/google/callback/route.ts index 945ea64680..e6f0012fad 100644 --- a/apps/web/src/app/api/integrations/google/callback/route.ts +++ b/apps/web/src/app/api/integrations/google/callback/route.ts @@ -14,19 +14,38 @@ import { import { upsertKiloClawGoogleOAuthConnection } from '@/lib/kiloclaw/google-oauth-connections'; import { KiloClawInternalClient } from '@/lib/kiloclaw/kiloclaw-internal-client'; +/** + * Build the post-OAuth redirect path. The query-string fragment is appended + * verbatim — callers MUST pass an already-URL-safe `key=value` string. Static + * literals (e.g. `'error=missing_code'`) are safe; dynamic values must come + * from `sanitizeOAuthProviderError` (which encodes) or be similarly + * pre-encoded. For raw key/value pairs use `appendQueryParam` instead. + */ function buildGoogleRedirectPath( - owner: VerifiedGoogleOAuthState['owner'] | null | undefined, - queryParam: string + state: { owner: VerifiedGoogleOAuthState['owner']; returnTo?: string } | null | undefined, + preEncodedQueryFragment: string ): string { - if (owner?.type === 'org') { - return `/organizations/${owner.id}/claw/settings?${queryParam}`; + if (state?.returnTo) { + const separator = state.returnTo.includes('?') ? '&' : '?'; + return `${state.returnTo}${separator}${preEncodedQueryFragment}`; } - if (owner?.type === 'user') { - return `/claw/settings?${queryParam}`; + const owner = state?.owner; + if (owner?.type === 'org') { + return `/organizations/${owner.id}/claw/settings?${preEncodedQueryFragment}`; } - return `/claw/settings?${queryParam}`; + return `/claw/settings?${preEncodedQueryFragment}`; +} + +/** + * Append a key=value query param to the given path, URL-encoding both. Use + * this when the value is raw / unencoded. For pre-encoded `key=value` + * fragments, use `buildGoogleRedirectPath` instead. + */ +function appendQueryParam(path: string, key: string, value: string): string { + const separator = path.includes('?') ? '&' : '?'; + return `${path}${separator}${encodeURIComponent(key)}=${encodeURIComponent(value)}`; } function sanitizeOAuthProviderError( @@ -127,7 +146,7 @@ export async function GET(request: NextRequest) { }); return NextResponse.redirect( - new URL(buildGoogleRedirectPath(verifiedState.owner, `error=${oauthErrorCode}`), APP_URL) + new URL(buildGoogleRedirectPath(verifiedState, `error=${oauthErrorCode}`), APP_URL) ); } @@ -140,7 +159,7 @@ export async function GET(request: NextRequest) { }); return NextResponse.redirect( - new URL(buildGoogleRedirectPath(verifiedState.owner, 'error=missing_code'), APP_URL) + new URL(buildGoogleRedirectPath(verifiedState, 'error=missing_code'), APP_URL) ); } @@ -157,7 +176,7 @@ export async function GET(request: NextRequest) { }); return NextResponse.redirect( - new URL(buildGoogleRedirectPath(verifiedState.owner, 'error=missing_instance'), APP_URL) + new URL(buildGoogleRedirectPath(verifiedState, 'error=missing_instance'), APP_URL) ); } @@ -211,8 +230,9 @@ export async function GET(request: NextRequest) { verifiedState.instanceId ); - const successPath = - verifiedState.owner.type === 'org' + const successPath = verifiedState.returnTo + ? appendQueryParam(verifiedState.returnTo, 'success', 'google_connected') + : verifiedState.owner.type === 'org' ? `/organizations/${verifiedState.owner.id}/claw/settings?success=google_connected` : '/claw/settings?success=google_connected'; @@ -232,7 +252,7 @@ export async function GET(request: NextRequest) { return NextResponse.redirect( new URL( - buildGoogleRedirectPath(verifyGoogleOAuthState(state)?.owner, 'error=connection_failed'), + buildGoogleRedirectPath(verifyGoogleOAuthState(state), 'error=connection_failed'), APP_URL ) ); diff --git a/apps/web/src/app/api/integrations/google/connect/route.test.ts b/apps/web/src/app/api/integrations/google/connect/route.test.ts index e066718a54..d8fa086543 100644 --- a/apps/web/src/app/api/integrations/google/connect/route.test.ts +++ b/apps/web/src/app/api/integrations/google/connect/route.test.ts @@ -4,6 +4,7 @@ import { getUserFromAuth } from '@/lib/user.server'; import { getActiveInstance, getActiveOrgInstance } from '@/lib/kiloclaw/instance-registry'; import { buildGoogleOAuthUrl } from '@/lib/integrations/google-service'; import { createGoogleOAuthState } from '@/lib/integrations/google/oauth-state'; +import type * as OAuthStateModule from '@/lib/integrations/google/oauth-state'; import { captureException, captureMessage } from '@sentry/nextjs'; import { failureResult } from '@/lib/maybe-result'; @@ -14,7 +15,19 @@ jest.mock('@/routers/organizations/utils', () => ({ })); jest.mock('@/lib/kiloclaw/instance-registry'); jest.mock('@/lib/integrations/google-service'); -jest.mock('@/lib/integrations/google/oauth-state'); +// Partial-mock so GOOGLE_OAUTH_RETURN_TO_REGEX (and any other constants) keep +// their real values; only the createGoogleOAuthState / verifyGoogleOAuthState +// functions need to be jest.fn() for assertions. +jest.mock('@/lib/integrations/google/oauth-state', () => { + const actual = jest.requireActual( + '@/lib/integrations/google/oauth-state' + ); + return { + ...actual, + createGoogleOAuthState: jest.fn(), + verifyGoogleOAuthState: jest.fn(), + }; +}); jest.mock('@sentry/nextjs', () => ({ captureException: jest.fn(), captureMessage: jest.fn(), @@ -155,4 +168,103 @@ describe('GET /api/integrations/google/connect', () => { expectRedirectLocation(response, '/claw/settings?error=invalid_organization'); expect(mockedEnsureOrganizationAccess).not.toHaveBeenCalled(); }); + + test('passes a valid returnTo through to the OAuth state', async () => { + const { GET } = await import('./route'); + const response = await GET( + makeRequest( + '/api/integrations/google/connect?returnTo=%2Fclaw%2Fnew%3Fstep%3Dcalendar' + ) as never + ); + + expect(response.status).toBe(307); + expect(mockedCreateGoogleOAuthState).toHaveBeenCalledWith( + expect.objectContaining({ + owner: { type: 'user', id: USER_ID }, + instanceId: INSTANCE_ID, + capabilities: ['calendar_read'], + returnTo: '/claw/new?step=calendar', + }), + USER_ID + ); + }); + + test('passes organizationId and returnTo through to the OAuth state for org flow', async () => { + const { GET } = await import('./route'); + const response = await GET( + makeRequest( + `/api/integrations/google/connect?organizationId=${ORG_ID}&returnTo=%2Forganizations%2F${ORG_ID}%2Fclaw%2Fnew%3Fstep%3Dcalendar` + ) as never + ); + + expect(response.status).toBe(307); + expect(mockedEnsureOrganizationAccess).toHaveBeenCalledWith({ user: { id: USER_ID } }, ORG_ID); + expect(mockedGetActiveOrgInstance).toHaveBeenCalledWith(USER_ID, ORG_ID); + expect(mockedCreateGoogleOAuthState).toHaveBeenCalledWith( + expect.objectContaining({ + owner: { type: 'org', id: ORG_ID }, + instanceId: INSTANCE_ID, + capabilities: ['calendar_read'], + returnTo: `/organizations/${ORG_ID}/claw/new?step=calendar`, + }), + USER_ID + ); + }); + + test('drops protocol-relative returnTo values to prevent open redirects', async () => { + const { GET } = await import('./route'); + const response = await GET( + makeRequest('/api/integrations/google/connect?returnTo=%2F%2Fevil.example.com') as never + ); + + expect(response.status).toBe(307); + const stateArg = mockedCreateGoogleOAuthState.mock.calls.at(0)?.[0]; + expect(stateArg).toBeDefined(); + expect(stateArg).not.toHaveProperty('returnTo'); + }); + + test('drops absolute-URL returnTo values', async () => { + const { GET } = await import('./route'); + const response = await GET( + makeRequest( + '/api/integrations/google/connect?returnTo=https%3A%2F%2Fevil.example.com%2Fclaw%2Fnew' + ) as never + ); + + expect(response.status).toBe(307); + const stateArg = mockedCreateGoogleOAuthState.mock.calls.at(0)?.[0]; + expect(stateArg).toBeDefined(); + expect(stateArg).not.toHaveProperty('returnTo'); + }); + + test('drops returnTo values with path-traversal segments', async () => { + // /claw/../admin would normalize to /admin after URL resolution. Even + // though the regex permits it, we reject any `.` or `..` segment for + // defense in depth. + const { GET } = await import('./route'); + const response = await GET( + makeRequest('/api/integrations/google/connect?returnTo=%2Fclaw%2F..%2Fadmin') as never + ); + + expect(response.status).toBe(307); + const stateArg = mockedCreateGoogleOAuthState.mock.calls.at(0)?.[0]; + expect(stateArg).toBeDefined(); + expect(stateArg).not.toHaveProperty('returnTo'); + }); + + test('drops returnTo values with a URI fragment', async () => { + // Fragments are disallowed by RETURN_TO_REGEX because the helpers in + // callback/route.ts append the success/error param using a `?`/`&` + // separator; a fragment in the returnTo would push the param past the + // `#` where browsers ignore it. + const { GET } = await import('./route'); + const response = await GET( + makeRequest('/api/integrations/google/connect?returnTo=%2Fclaw%2Fnew%23section') as never + ); + + expect(response.status).toBe(307); + const stateArg = mockedCreateGoogleOAuthState.mock.calls.at(0)?.[0]; + expect(stateArg).toBeDefined(); + expect(stateArg).not.toHaveProperty('returnTo'); + }); }); diff --git a/apps/web/src/app/api/integrations/google/connect/route.ts b/apps/web/src/app/api/integrations/google/connect/route.ts index a581af658c..b8aad9e4fc 100644 --- a/apps/web/src/app/api/integrations/google/connect/route.ts +++ b/apps/web/src/app/api/integrations/google/connect/route.ts @@ -6,7 +6,11 @@ import { APP_URL } from '@/lib/constants'; import { ensureOrganizationAccess } from '@/routers/organizations/utils'; import { getActiveInstance, getActiveOrgInstance } from '@/lib/kiloclaw/instance-registry'; import { buildGoogleOAuthUrl } from '@/lib/integrations/google-service'; -import { createGoogleOAuthState } from '@/lib/integrations/google/oauth-state'; +import { + createGoogleOAuthState, + GOOGLE_OAUTH_RETURN_TO_REGEX, + returnToHasPathTraversal, +} from '@/lib/integrations/google/oauth-state'; import { DEFAULT_GOOGLE_CAPABILITIES } from '@/lib/integrations/google/capabilities'; import { captureException, captureMessage } from '@sentry/nextjs'; import type { Owner } from '@/lib/integrations/core/types'; @@ -74,11 +78,21 @@ export async function GET(request: NextRequest) { return NextResponse.redirect(new URL(redirectPath, APP_URL)); } + const returnToParam = request.nextUrl.searchParams.get('returnTo'); + const returnTo = + returnToParam && + GOOGLE_OAUTH_RETURN_TO_REGEX.test(returnToParam) && + returnToParam.length <= 2048 && + !returnToHasPathTraversal(returnToParam) + ? returnToParam + : undefined; + const state = createGoogleOAuthState( { owner, instanceId: instance.id, capabilities, + ...(returnTo ? { returnTo } : {}), }, user.id ); diff --git a/apps/web/src/lib/integrations/google/oauth-state.ts b/apps/web/src/lib/integrations/google/oauth-state.ts index f46c2f93a7..62015f5237 100644 --- a/apps/web/src/lib/integrations/google/oauth-state.ts +++ b/apps/web/src/lib/integrations/google/oauth-state.ts @@ -6,6 +6,27 @@ import { GoogleCapabilitySchema } from './capabilities'; const GOOGLE_OAUTH_STATE_PREFIX = 'google:'; +// Constrain returnTo to a relative path so it can never be hijacked into an +// open-redirect to an external host. Must start with `/`, may contain a +// non-protocol-style path, optionally followed by a query string. Fragments +// are disallowed — buildGoogleRedirectPath / appendQueryParam append the +// success/error param using a `?` or `&` separator, and a fragment in the +// returnTo would push the appended param past the `#` where browsers ignore +// it. Disallows `//` after the leading slash so we don't accidentally accept +// protocol-relative URLs like `//evil.example.com`. +const RETURN_TO_REGEX = /^\/(?!\/)[^?#]*(\?[^#]*)?$/; + +/** + * Defense-in-depth check: even though RETURN_TO_REGEX rejects external hosts, + * a crafted `/foo/../admin` would still resolve to `/admin` after URL + * normalization. Block any `..` or `.` path segment so the redirect target + * stays anchored to the path the caller actually wrote. + */ +export function returnToHasPathTraversal(value: string): boolean { + const pathOnly = value.split('?')[0] ?? ''; + return pathOnly.split('/').some(segment => segment === '..' || segment === '.'); +} + const GoogleOAuthStatePayloadSchema = z.object({ owner: z.discriminatedUnion('type', [ z.object({ type: z.literal('user'), id: z.string().min(1) }), @@ -13,8 +34,16 @@ const GoogleOAuthStatePayloadSchema = z.object({ ]), instanceId: z.string().uuid(), capabilities: z.array(GoogleCapabilitySchema).min(1), + returnTo: z + .string() + .regex(RETURN_TO_REGEX) + .max(2048) + .refine(v => !returnToHasPathTraversal(v), 'returnTo must not contain path traversal segments') + .optional(), }); +export const GOOGLE_OAUTH_RETURN_TO_REGEX = RETURN_TO_REGEX; + export type GoogleOAuthStatePayload = z.infer; export type VerifiedGoogleOAuthState = GoogleOAuthStatePayload & {