diff --git a/apps/backend/src/libs/better-auth/auth.ts b/apps/backend/src/libs/better-auth/auth.ts index a39fea6c..c716cc2f 100644 --- a/apps/backend/src/libs/better-auth/auth.ts +++ b/apps/backend/src/libs/better-auth/auth.ts @@ -319,14 +319,17 @@ const authInstance: any = betterAuth({ ? { sameSite: 'none', // Allow cross-origin requests secure: true, // HTTPS only - // Shares the session cookie across sibling subdomains (e.g. - // web and api hosts) so the direct browser -> backend OAuth - // authorize/consent redirect (MCP login flow) can see the - // session set on the frontend host. Unset in envs where web - // and api share one origin. - ...(process.env.COOKIE_DOMAIN - ? { domain: process.env.COOKIE_DOMAIN } - : {}), + // No `domain` here on purpose. A previous COOKIE_DOMAIN env var + // tried to share the session cookie across the web and api hosts + // so the direct browser -> backend /oauth2/authorize hit would + // see it. Never needed: oauthProvider bounces an unauthenticated + // authorize request to `loginPage` and resumes the flow itself + // once a session cookie is set, which happens on the frontend + // origin via the /auth proxy — which also strips `domain=` from + // every Set-Cookie, so it never reached a browser anyway. If web + // and api ever share a registrable domain, reach for + // better-auth's `advanced.crossSubDomainCookies` rather than + // setting `domain` by hand. } : { sameSite: 'lax', // Standard for same-origin diff --git a/apps/web/__tests__/auth-proxy.test.ts b/apps/web/__tests__/auth-proxy.test.ts new file mode 100644 index 00000000..592b0df1 --- /dev/null +++ b/apps/web/__tests__/auth-proxy.test.ts @@ -0,0 +1,111 @@ +import { describe, it, expect } from 'vitest'; +import { + isTopLevelNavigation, + redirectTargetFromBody, + rewriteCookieForProxy, +} from '@/lib/auth-proxy'; + +const request = (method: string, headers: Record) => ({ + method, + headers: new Headers(headers), +}); + +const jsonResponse = (body: unknown) => + new Response(JSON.stringify(body), { + headers: { 'content-type': 'application/json' }, + }); + +describe('isTopLevelNavigation', () => { + it('recognises a document navigation', () => { + expect( + isTopLevelNavigation( + request('GET', { + 'sec-fetch-mode': 'navigate', + accept: 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8', + }), + ), + ).toBe(true); + }); + + it('leaves an XHR alone, which handles its own redirect', () => { + expect( + isTopLevelNavigation( + request('GET', { 'sec-fetch-mode': 'cors', accept: '*/*' }), + ), + ).toBe(false); + }); + + it('never treats a POST as a navigation', () => { + expect( + isTopLevelNavigation(request('POST', { 'sec-fetch-mode': 'navigate' })), + ).toBe(false); + }); + + it('falls back to Accept when sec-fetch-mode is absent', () => { + expect(isTopLevelNavigation(request('GET', { accept: 'text/html' }))).toBe( + true, + ); + expect( + isTopLevelNavigation(request('GET', { accept: 'application/json' })), + ).toBe(false); + }); +}); + +describe('redirectTargetFromBody', () => { + it("picks up oauth-provider's JSON redirect", async () => { + await expect( + redirectTargetFromBody( + jsonResponse({ + redirect: true, + url: 'http://localhost:9876/callback?code=abc', + }), + ), + ).resolves.toBe('http://localhost:9876/callback?code=abc'); + }); + + it('ignores a sign-in response that carries no redirect', async () => { + await expect( + redirectTargetFromBody( + jsonResponse({ redirect: false, token: 't', user: {} }), + ), + ).resolves.toBeNull(); + }); + + it('ignores an ordinary payload', async () => { + await expect( + redirectTargetFromBody(jsonResponse({ session: { id: 1 } })), + ).resolves.toBeNull(); + }); + + it('ignores a non-JSON body', async () => { + await expect( + redirectTargetFromBody( + new Response('hi', { headers: { 'content-type': 'text/html' } }), + ), + ).resolves.toBeNull(); + }); + + it('leaves the body readable for the pass-through path', async () => { + const response = jsonResponse({ redirect: false, token: 'abc' }); + await redirectTargetFromBody(response); + await expect(response.text()).resolves.toBe( + JSON.stringify({ redirect: false, token: 'abc' }), + ); + }); +}); + +describe('rewriteCookieForProxy', () => { + it('drops Domain so the cookie lands on this origin', () => { + expect( + rewriteCookieForProxy( + 'session=abc; Path=/; Domain=.example.com; HttpOnly; Secure', + ), + ).toBe('session=abc; Path=/; HttpOnly; Secure'); + }); + + it('relaxes SameSite=None now that it is same-site', () => { + expect(rewriteCookieForProxy('session=abc; SameSite=None; Secure')).toBe( + 'session=abc; SameSite=Lax; Secure', + ); + }); +}); diff --git a/apps/web/src/app/(app)/chat/layout.tsx b/apps/web/src/app/(app)/chat/layout.tsx index 24e81b57..4cde1974 100644 --- a/apps/web/src/app/(app)/chat/layout.tsx +++ b/apps/web/src/app/(app)/chat/layout.tsx @@ -5,33 +5,64 @@ export const metadata = { title: 'Chat', }; +// Use backend URL from environment, fallback to localhost for development +// Use NEXT_PUBLIC_BACKEND_URL for client-side consistency +const backendUrl = () => + process.env.NEXT_PUBLIC_BACKEND_URL || + process.env.BACKEND_URL || + 'http://localhost:3001'; + +async function authFetch(path: string) { + const headersList = await headers(); + const response = await fetch(`${backendUrl()}${path}`, { + headers: Object.fromEntries(headersList.entries()), + cache: 'no-store', + }); + + return response.ok ? await response.json() : null; +} + // Server Component layout guard - performs real session validation async function getSession() { try { - const headersList = await headers(); - // Use backend URL from environment, fallback to localhost for development - // Use NEXT_PUBLIC_BACKEND_URL for client-side consistency - const backendUrl = - process.env.NEXT_PUBLIC_BACKEND_URL || - process.env.BACKEND_URL || - 'http://localhost:3001'; - const response = await fetch(`${backendUrl}/auth/get-session`, { - headers: Object.fromEntries(headersList.entries()), - cache: 'no-store', - }); - - if (!response.ok) { - return null; - } - - const data = await response.json(); - return data.session || null; + const data = await authFetch('/auth/get-session'); + return data?.session || null; } catch (error) { console.error('Failed to get session:', error); return null; } } +/** + * Where to send someone who reaches the app without an organization. + * + * activeOrganizationId is stamped on the session when it is created + * (databaseHooks.session.create.before in the backend's better-auth config) + * and refreshed by setActive when an organization is created or an invitation + * accepted. So its absence is a dependable "no organization yet", and it + * flips the moment they have one — which is what stops this bouncing them + * straight back here. + */ +async function organizationlessDestination() { + // Retry a few times to handle session propagation timing for new accounts + for (let attempt = 0; attempt < 3; attempt++) { + if (attempt > 0) await new Promise((r) => setTimeout(r, 500)); + + try { + const invitations = await authFetch( + '/auth/organization/list-user-invitations', + ); + if (Array.isArray(invitations) && invitations.length > 0) { + return '/settings?tab=invitations'; + } + } catch (error) { + console.error('Failed to list invitations:', error); + } + } + + return '/organization/create'; +} + export default async function ChatLayout({ children, }: { @@ -44,5 +75,15 @@ export default async function ChatLayout({ redirect('/sign-in'); } + // better-auth can only redirect to a fixed callbackURL, but where a login + // belongs depends on state that exists only once there is a session. So + // every login lands here and the decision is made at the door. This also + // catches anyone navigating straight to /chat without an organization, who + // previously reached an app where every request failed with + // NO_ACTIVE_ORGANIZATION. + if (!session.activeOrganizationId) { + redirect(await organizationlessDestination()); + } + return <>{children}; } diff --git a/apps/web/src/app/auth/[...auth]/route.ts b/apps/web/src/app/auth/[...auth]/route.ts index c56a5fd0..45e2682f 100644 --- a/apps/web/src/app/auth/[...auth]/route.ts +++ b/apps/web/src/app/auth/[...auth]/route.ts @@ -1,5 +1,10 @@ import type { NextRequest } from 'next/server'; import { NextResponse } from 'next/server'; +import { + isTopLevelNavigation, + redirectTargetFromBody, + rewriteCookieForProxy, +} from '@/lib/auth-proxy'; const BACKEND_URL = process.env.BACKEND_URL || 'http://localhost:3001'; @@ -40,6 +45,19 @@ export async function PATCH( return proxyToBackend(request, context); } +/** Copy the backend's cookies across, re-scoped onto this origin. */ +function applyCookies(from: Response, to: NextResponse, label: string) { + from.headers.getSetCookie().forEach((cookie) => { + const rewrittenCookie = rewriteCookieForProxy(cookie); + + console.log(`[AUTH PROXY] Rewriting cookie on ${label}:`, { + original: cookie, + rewritten: rewrittenCookie, + }); + to.headers.append('Set-Cookie', rewrittenCookie); + }); +} + async function proxyToBackend( request: NextRequest, context: { params: Promise<{ auth: string[] }> }, @@ -130,32 +148,33 @@ async function proxyToBackend( response.status, ); - // Set cookies on redirect response - const cookies = response.headers.getSetCookie(); - cookies.forEach((cookie) => { - const rewrittenCookie = cookie - .split(';') - .map((part) => part.trim()) - .filter((part) => !part.toLowerCase().startsWith('domain=')) - .map((part) => { - if (part.toLowerCase() === 'samesite=none') { - return 'SameSite=Lax'; - } - return part; - }) - .join('; '); - - console.log('[AUTH PROXY] Rewriting cookie on redirect:', { - original: cookie, - rewritten: rewrittenCookie, - }); - redirectResponse.headers.append('Set-Cookie', rewrittenCookie); - }); + applyCookies(response, redirectResponse, 'redirect'); return redirectResponse; } } + // better-auth's oauth-provider content-negotiates its redirects: a browser + // fetch gets { redirect, url } to act on, anything else gets a real 302 + // (oauth-provider/dist/index.mjs handleRedirect). It decides by + // sec-fetch-mode === 'cors' — and Node's fetch sets that header on every + // request and will not let us override it, so the backend cannot tell this + // proxy apart from an XHR. Harmless when the browser is doing an XHR and + // will act on the JSON itself. Fatal on a top-level navigation, such as + // returning to /oauth2/authorize after login: the browser renders the JSON + // as text, the authorization code sits unused on screen, and the MCP + // client waits forever. So turn it back into the 302 the backend meant. + if (response.ok && isTopLevelNavigation(request)) { + const location = await redirectTargetFromBody(response); + + if (location) { + console.log('[AUTH PROXY] JSON redirect → 302:', location); + const redirectResponse = NextResponse.redirect(location, 302); + applyCookies(response, redirectResponse, 'json redirect'); + return redirectResponse; + } + } + // Forward the response const responseBody = await response.text(); const nextResponse = new NextResponse(responseBody, { @@ -179,30 +198,7 @@ async function proxyToBackend( }); // Forward all Set-Cookie headers, rewriting for same-site usage - const cookies = response.headers.getSetCookie(); - cookies.forEach((cookie) => { - // Parse cookie and: - // 1. Remove Domain attribute so it defaults to current domain (frontend) - // 2. Change SameSite=None to SameSite=Lax (we're now same-site, not cross-site) - const rewrittenCookie = cookie - .split(';') - .map((part) => part.trim()) - .filter((part) => !part.toLowerCase().startsWith('domain=')) - .map((part) => { - // Change SameSite=None to SameSite=Lax - if (part.toLowerCase() === 'samesite=none') { - return 'SameSite=Lax'; - } - return part; - }) - .join('; '); - - console.log('[AUTH PROXY] Rewriting cookie:', { - original: cookie, - rewritten: rewrittenCookie, - }); - nextResponse.headers.append('Set-Cookie', rewrittenCookie); - }); + applyCookies(response, nextResponse, 'response'); return nextResponse; } catch (error) { diff --git a/apps/web/src/components/auth/email-password-auth.tsx b/apps/web/src/components/auth/email-password-auth.tsx index 99a70aa2..722450f7 100644 --- a/apps/web/src/components/auth/email-password-auth.tsx +++ b/apps/web/src/components/auth/email-password-auth.tsx @@ -1,8 +1,7 @@ 'use client'; import Image from 'next/image'; -import { useState } from 'react'; -import { useRouter } from 'next/navigation'; +import { useEffect, useState } from 'react'; import { AuthLayout, Button, Field, Label, Input } from '@zuko/ui-kit'; import { authClient } from '@/lib/auth-client'; import Link from 'next/link'; @@ -16,7 +15,6 @@ export function EmailPasswordAuth({ mode = 'signin', emailPasswordEnabled = false, }: EmailPasswordAuthProps) { - const router = useRouter(); const [error, setError] = useState(null); const [isLoading, setIsLoading] = useState(false); @@ -27,35 +25,64 @@ export function EmailPasswordAuth({ const isSignup = mode === 'signup'; - /** Shared post-auth redirect: org → chat, invitations → settings, else → create org */ - const redirectAfterAuth = async () => { - const { data } = await authClient.organization.list(); - if (data && data.length > 0) { - router.push('/chat'); - } else { - // Retry a few times to handle session propagation timing for new accounts - let invitations = null; - for (let attempt = 0; attempt < 3; attempt++) { - if (attempt > 0) await new Promise((r) => setTimeout(r, 500)); - const { data } = await authClient.organization.listUserInvitations(); - if (data && data.length > 0) { - invitations = data; - break; - } - } - if (invitations && invitations.length > 0) { - router.push('/settings?tab=invitations'); - } else { - router.push('/organization/create'); - } - } + // A signed authorization query lands on whichever of /sign-in or /sign-up + // oauthProvider sent the user to. The cross-link between the two pages has + // to carry it, or a user who signs up mid-authorization arrives with no + // query for the client plugin to attach and the MCP client is left waiting. + // Read after mount so the server render and first client render agree. + const [authQuery, setAuthQuery] = useState(''); + useEffect(() => setAuthQuery(window.location.search), []); + + /** + * Where better-auth sends a successful login, via `callbackURL`. + * + * Normally /chat, whose layout decides whether this user actually belongs + * there or needs to create an organization first. But when oauthProvider + * bounced an MCP client's authorization here, the signed query is still on + * the URL and the login has to return to the authorize endpoint before a + * code can be minted. + * oauthProvider resumes that itself on any response it can rewrite, which + * covers email sign-in — a social login round-trips through Google first, + * so name the destination explicitly and let whichever lands first win. + * + * Deliberately the same-origin /auth proxy and not BACKEND_URL: web and api + * are separate origins with no shared cookie domain, so hitting the backend + * authorize endpoint directly would carry no session and bounce straight + * back to this page. The proxy forwards the cookie. Re-encoding + * the query on the way through is safe — the signature is verified over a + * canonicalised, re-sorted URLSearchParams on both sides + * (@better-auth/oauth-provider/dist/version-DaSfXJQ1.mjs:5). + */ + const postAuthURL = () => { + const search = window.location.search; + return new URLSearchParams(search).has('sig') + ? `${window.location.origin}/auth/oauth2/authorize${search}` + : `${window.location.origin}/chat`; }; + /** + * better-auth's redirectPlugin (client/fetch-plugins.mjs) navigates by + * itself whenever a response comes back as { redirect, url } — which is + * both how `callbackURL` is honoured and how oauthProvider hands back the + * consent screen after resuming an MCP authorization. + * + * signUp.email is the one endpoint that never sets those fields: it returns + * { token, user } and uses callbackURL only for the verification link + * (api/routes/sign-up.mjs). So that branch still has to navigate on its + * own, and has to check first or it races the plugin. + */ + const pluginWillRedirect = (data: unknown) => + Boolean((data as { redirect?: boolean } | null)?.redirect); + const handleEmailPasswordSubmit = async (e: React.FormEvent) => { e.preventDefault(); setIsLoading(true); setError(null); + // Set once we hand off to a full-page navigation; the button must stay + // disabled for the round trip rather than flicking back to enabled. + let leaving = false; + try { if (isSignup) { const result = await authClient.signUp.email({ @@ -70,12 +97,15 @@ export function EmailPasswordAuth({ 'Failed to create account. Please try again.', ); } else { - await redirectAfterAuth(); + leaving = true; + if (!pluginWillRedirect(result.data)) + window.location.href = postAuthURL(); } } else { const result = await authClient.signIn.email({ email, password, + callbackURL: postAuthURL(), }); if (result.error) { @@ -84,21 +114,21 @@ export function EmailPasswordAuth({ 'Failed to sign in. Please check your credentials.', ); } else { - await redirectAfterAuth(); + leaving = true; } } } catch (err) { setError('An unexpected error occurred. Please try again.'); console.error('Email/password auth error:', err); } finally { - setIsLoading(false); + if (!leaving) setIsLoading(false); } }; const handleGoogleSignIn = () => { authClient.signIn.social({ provider: 'google', - callbackURL: `${window.location.origin}/chat`, + callbackURL: postAuthURL(), }); }; @@ -208,7 +238,7 @@ export function EmailPasswordAuth({ <> Already have an account?{' '} Sign in @@ -218,7 +248,7 @@ export function EmailPasswordAuth({ <> Don't have an account?{' '} Sign up diff --git a/apps/web/src/components/auth/oauth-consent.tsx b/apps/web/src/components/auth/oauth-consent.tsx index 022ab69b..d1319ad0 100644 --- a/apps/web/src/components/auth/oauth-consent.tsx +++ b/apps/web/src/components/auth/oauth-consent.tsx @@ -35,9 +35,19 @@ const SCOPE_DESCRIPTIONS: Record = { // CRM - Tasks 'tasks:read': 'Read your tasks', 'tasks:write': 'Create and update tasks', - // CRM - Relations - 'relations:read': 'Read entity relations', - 'relations:write': 'Manage entity relations', + // CRM - Prospects and leads + 'prospects:read': 'Read your prospects', + 'prospects:write': 'Create and update prospects', + 'leads:read': 'Read your leads', + 'leads:write': 'Create and update leads', + // CRM - Targeting + 'icps:read': 'Read your ideal customer profiles', + 'icps:write': 'Create and update ideal customer profiles', + 'campaigns:read': 'Read your campaigns', + 'campaigns:write': 'Create and update campaigns', + // CRM - Comments + 'comments:read': 'Read comments', + 'comments:write': 'Write comments', }; /** diff --git a/apps/web/src/lib/auth-proxy.ts b/apps/web/src/lib/auth-proxy.ts new file mode 100644 index 00000000..14b68e5f --- /dev/null +++ b/apps/web/src/lib/auth-proxy.ts @@ -0,0 +1,58 @@ +/** + * Helpers for the /auth proxy (app/auth/[...auth]/route.ts). + * + * Kept out of the route module so they can be unit tested: Next only lets a + * route file export its HTTP handlers. + */ + +/** + * A document navigation, as opposed to an XHR the page will handle itself. + * Mirrors the test better-auth makes internally: trust sec-fetch-mode when the + * browser sends it, otherwise fall back to the Accept header. + */ +export function isTopLevelNavigation(request: { + method: string; + headers: Headers; +}) { + if (request.method !== 'GET') return false; + + const mode = request.headers.get('sec-fetch-mode')?.toLowerCase(); + if (mode) return mode === 'navigate'; + + const accept = request.headers.get('accept')?.toLowerCase() ?? ''; + return ( + accept.includes('text/html') || accept.includes('application/xhtml+xml') + ); +} + +/** + * The `url` of a `{ redirect: true, url }` body, or null for anything else. + * + * Reads a clone so the caller can still stream the original body through. + */ +export async function redirectTargetFromBody(response: Response) { + if (!response.headers.get('content-type')?.includes('application/json')) { + return null; + } + + try { + const body = await response.clone().json(); + return body?.redirect === true && typeof body.url === 'string' + ? body.url + : null; + } catch { + return null; + } +} + +/** Re-scope a backend cookie onto this origin. */ +export function rewriteCookieForProxy(cookie: string) { + return cookie + .split(';') + .map((part) => part.trim()) + .filter((part) => !part.toLowerCase().startsWith('domain=')) + .map((part) => + part.toLowerCase() === 'samesite=none' ? 'SameSite=Lax' : part, + ) + .join('; '); +} diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 9630527e..9a7da407 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -30,7 +30,7 @@ overrides: # pnpm blocks dependency install/postinstall scripts unless they are listed # here (`pnpm approve-builds` maintains this). @scarf/scarf is denied on -# purpose: it is analytics, not a build. +# purpose: it is analytics, not a build. sharp is denied too — see below. allowBuilds: '@parcel/watcher': true '@prisma/engines': true @@ -40,5 +40,14 @@ allowBuilds: esbuild: true nx: true prisma: true - sharp: true + # sharp's install script is `node install/check.js || npm run build`, and + # check.js exits 1 whenever it detects a compatible system libvips + # (lib/libvips.js useGlobalLibvips) — true on any machine with libvips-dev + # or Homebrew libvips. That exit triggers the source-build fallback, which + # dies on `sharp: Please add node-gyp to your dependencies` and prints a + # scary error on every install. We never want a source build: sharp 0.34 + # ships its binary as the @img/sharp-* optional deps, which pnpm installs + # as ordinary packages and which are what require('sharp') loads. Skipping + # the script loses nothing (check.js does nothing on success). + sharp: false unrs-resolver: true