From 6f924a8dcc8ace8c75f6ed660f26c2662003e04c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 11:59:35 +0000 Subject: [PATCH 1/2] feat(auth): route unprovisioned users to a welcome screen An Auth0 identity with no row in business_users resolved to a null auth context, so every @requiresAuth field threw UNAUTHENTICATED. The client read that as a token failure and ran refreshAuth (which succeeds, the token was never bad), optionally prompting an interactive re-login, while every failed operation raised a toast behind an empty dashboard. Server: - @requiresAuth now distinguishes the two cases: a verified JWT identity that maps to no membership throws ONBOARDING_REQUIRED instead of UNAUTHENTICATED. Missing or invalid credentials keep the old code. - New unauthenticated-safe `viewer` query reporting the caller's provisioning state (ACTIVE / EMAIL_UNVERIFIED / NO_WORKSPACE) from its own JWT claims. Client: - OnboardingGuard, composed into ProtectedRoute, redirects a non-ACTIVE viewer to the new /welcome screen; a viewer query error renders the app instead of trapping the user there. - /welcome explains the invitation-only model, offers re-check and sign-out, and returns to the app once a membership appears. - ONBOARDING_REQUIRED no longer raises an error toast per failed operation. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01344D5q1uyAHuwRJ3yyDX46 --- .../client/src/__tests__/urql-client.test.ts | 12 +++ .../src/__tests__/urql-error-handler.test.ts | 37 +++++++++ .../__tests__/protected-route.test.ts | 81 ++++++++++++++++++- .../client/src/components/screens/welcome.tsx | 80 ++++++++++++++++++ packages/client/src/hooks/use-viewer.ts | 34 ++++++++ .../src/providers/urql-error-handler.ts | 6 ++ packages/client/src/router/config.tsx | 12 +++ .../client/src/router/guards/auth-guards.tsx | 32 +++++++- packages/client/src/router/routes.ts | 1 + .../__tests__/auth-directives.test.ts | 29 ++++++- .../auth/directives/auth-directives.ts | 14 ++++ .../common/__tests__/viewer.resolver.test.ts | 78 ++++++++++++++++++ packages/server/src/modules/common/index.ts | 6 +- .../common/resolvers/viewer.resolver.ts | 40 +++++++++ .../modules/common/typeDefs/viewer.graphql.ts | 25 ++++++ 15 files changed, 481 insertions(+), 6 deletions(-) create mode 100644 packages/client/src/__tests__/urql-error-handler.test.ts create mode 100644 packages/client/src/components/screens/welcome.tsx create mode 100644 packages/client/src/hooks/use-viewer.ts create mode 100644 packages/server/src/modules/common/__tests__/viewer.resolver.test.ts create mode 100644 packages/server/src/modules/common/resolvers/viewer.resolver.ts create mode 100644 packages/server/src/modules/common/typeDefs/viewer.graphql.ts diff --git a/packages/client/src/__tests__/urql-client.test.ts b/packages/client/src/__tests__/urql-client.test.ts index ff9938da8a..8485df9f30 100644 --- a/packages/client/src/__tests__/urql-client.test.ts +++ b/packages/client/src/__tests__/urql-client.test.ts @@ -140,6 +140,18 @@ describe('URQL auth exchange hardening', () => { expect(authConfig.didAuthError(forbiddenError)).toBe(false); }); + it('does not treat ONBOARDING_REQUIRED as an auth error', async () => { + const { authConfig } = await initializeAuth(async () => null); + + // The token is valid — the account just has no workspace. Refreshing it + // cannot help, and doing so drags the user through a pointless re-login. + const onboardingError = { + graphQLErrors: [{ extensions: { code: 'ONBOARDING_REQUIRED' } }], + }; + + expect(authConfig.didAuthError(onboardingError)).toBe(false); + }); + it('does not eagerly trigger auth refresh before a server auth error', async () => { const { authConfig } = await initializeAuth(async () => null); diff --git a/packages/client/src/__tests__/urql-error-handler.test.ts b/packages/client/src/__tests__/urql-error-handler.test.ts new file mode 100644 index 0000000000..6edd388af5 --- /dev/null +++ b/packages/client/src/__tests__/urql-error-handler.test.ts @@ -0,0 +1,37 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { OperationResult } from 'urql'; +import { handleUrqlError } from '../providers/urql-error-handler.js'; + +const { toastErrorMock } = vi.hoisted(() => ({ + toastErrorMock: vi.fn(), +})); + +vi.mock('sonner', () => ({ + toast: { error: toastErrorMock }, +})); + +function resultWithCode(code: string): OperationResult { + return { + error: { graphQLErrors: [{ message: 'nope', extensions: { code } }] }, + } as unknown as OperationResult; +} + +describe('handleUrqlError', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('toasts ordinary GraphQL errors', () => { + handleUrqlError(resultWithCode('FORBIDDEN')); + + expect(toastErrorMock).toHaveBeenCalledTimes(1); + }); + + it('stays silent for ONBOARDING_REQUIRED', () => { + // Every guarded operation fails at once for an unprovisioned account; the + // /welcome screen is the message, so toasts would only pile up behind it. + handleUrqlError(resultWithCode('ONBOARDING_REQUIRED')); + + expect(toastErrorMock).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/client/src/components/__tests__/protected-route.test.ts b/packages/client/src/components/__tests__/protected-route.test.ts index 430a2e2d42..6c3daefbb8 100644 --- a/packages/client/src/components/__tests__/protected-route.test.ts +++ b/packages/client/src/components/__tests__/protected-route.test.ts @@ -8,27 +8,47 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { ProtectedRoute, PublicOnlyGuard } from '../../router/guards/auth-guards.js'; import { ROUTES } from '../../router/routes.js'; -const { useAuth0Mock } = vi.hoisted(() => ({ +const { useAuth0Mock, useViewerMock } = vi.hoisted(() => ({ useAuth0Mock: vi.fn(), + useViewerMock: vi.fn(), })); vi.mock('@auth0/auth0-react', () => ({ useAuth0: useAuth0Mock, })); +// ProtectedRoute composes OnboardingGuard, which queries `viewer` through urql. +// Stubbing the hook keeps these cases about auth state alone. +vi.mock('../../hooks/use-viewer.js', () => ({ + useViewer: useViewerMock, +})); + +const ACTIVE_VIEWER = { + fetching: false, + error: undefined, + viewer: { email: 'member@example.com', emailVerified: true, status: 'ACTIVE' }, +}; + type AuthState = { isAuthenticated: boolean; isLoading: boolean; }; +type ViewerState = typeof ACTIVE_VIEWER | Record; + ( globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean; } ).IS_REACT_ACT_ENVIRONMENT = true; -async function renderProtectedPath(pathname: string, authState: AuthState) { +async function renderProtectedPath( + pathname: string, + authState: AuthState, + viewerState: ViewerState = ACTIVE_VIEWER, +) { useAuth0Mock.mockReturnValue(authState); + useViewerMock.mockReturnValue(viewerState); const router = createMemoryRouter( [ @@ -36,6 +56,10 @@ async function renderProtectedPath(pathname: string, authState: AuthState) { path: ROUTES.LOGIN, element: React.createElement('div', null, 'Login Page'), }, + { + path: ROUTES.WELCOME, + element: React.createElement('div', null, 'Welcome Page'), + }, { path: ROUTES.CHARGES.ROOT, element: React.createElement( @@ -167,6 +191,59 @@ describe('ProtectedRoute', () => { }); }); +describe('OnboardingGuard', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + const authenticated = { isAuthenticated: true, isLoading: false }; + + it('redirects an authenticated user with no workspace to /welcome', async () => { + const { router, cleanup } = await renderProtectedPath(ROUTES.CHARGES.ROOT, authenticated, { + fetching: false, + error: undefined, + viewer: { email: 'new@example.com', emailVerified: true, status: 'NO_WORKSPACE' }, + }); + + expect(router.state.location.pathname).toBe(ROUTES.WELCOME); + await cleanup(); + }); + + it('redirects an unverified-email user to /welcome', async () => { + const { router, cleanup } = await renderProtectedPath(ROUTES.CHARGES.ROOT, authenticated, { + fetching: false, + error: undefined, + viewer: { email: 'new@example.com', emailVerified: false, status: 'EMAIL_UNVERIFIED' }, + }); + + expect(router.state.location.pathname).toBe(ROUTES.WELCOME); + await cleanup(); + }); + + it('holds the app shell back while the viewer query is in flight', async () => { + const { html, cleanup } = await renderProtectedPath(ROUTES.CHARGES.ROOT, authenticated, { + fetching: true, + error: undefined, + viewer: null, + }); + + expect(html).not.toContain('Charges Page'); + await cleanup(); + }); + + it('renders the app on a viewer query error rather than trapping the user', async () => { + const { html, router, cleanup } = await renderProtectedPath(ROUTES.CHARGES.ROOT, authenticated, { + fetching: false, + error: new Error('network down'), + viewer: null, + }); + + expect(router.state.location.pathname).toBe(ROUTES.CHARGES.ROOT); + expect(html).toContain('Charges Page'); + await cleanup(); + }); +}); + describe('PublicOnlyGuard', () => { beforeEach(() => { vi.clearAllMocks(); diff --git a/packages/client/src/components/screens/welcome.tsx b/packages/client/src/components/screens/welcome.tsx new file mode 100644 index 0000000000..f60ebcb7b2 --- /dev/null +++ b/packages/client/src/components/screens/welcome.tsx @@ -0,0 +1,80 @@ +import { useEffect, type ReactElement } from 'react'; +import { Loader2 } from 'lucide-react'; +import { Navigate, useNavigate } from 'react-router-dom'; +import { useAuth0 } from '@auth0/auth0-react'; +import { useLogout } from '../../hooks/use-logout.js'; +import { useViewer } from '../../hooks/use-viewer.js'; +import { ROUTES } from '../../router/routes.js'; +import { Button } from '../ui/button.js'; + +/** + * Terminal screen for an authenticated identity that cannot use the app yet. + * + * Accounter is invitation-only, so reaching this screen is expected for anyone + * who signed up directly, whose invitation expired before they accepted, or who + * was removed from their last business. It replaces what used to be an empty + * dashboard buried under failing queries. + */ +export function WelcomePage(): ReactElement { + const { fetching, viewer } = useViewer(); + const { isAuthenticated, isLoading } = useAuth0(); + const handleLogout = useLogout(); + const navigate = useNavigate(); + + // Provisioning can complete out-of-band (an admin adds the membership), so + // send an already-active viewer back to the app rather than stranding them. + useEffect(() => { + if (viewer?.status === 'ACTIVE') { + navigate(ROUTES.HOME, { replace: true }); + } + }, [viewer?.status, navigate]); + + if (isLoading || fetching) { + return ( +
+ +
+ ); + } + + if (!isAuthenticated) { + return ; + } + + const isEmailUnverified = viewer?.status === 'EMAIL_UNVERIFIED'; + + return ( +
+
+
+

+ {isEmailUnverified ? 'Verify your email' : 'No workspace yet'} +

+ {viewer?.email ? ( +

Signed in as {viewer.email}

+ ) : null} +
+ + {isEmailUnverified ? ( +

+ We sent a verification link to your email address. Open it, then reload this page to + continue. +

+ ) : ( +

+ Accounter is invitation-only. Your account is not linked to any business yet — ask an + administrator of the business you should belong to for an invitation, then open the link + they send you. +

+ )} + +
+ + +
+
+
+ ); +} diff --git a/packages/client/src/hooks/use-viewer.ts b/packages/client/src/hooks/use-viewer.ts new file mode 100644 index 0000000000..ae183f5e47 --- /dev/null +++ b/packages/client/src/hooks/use-viewer.ts @@ -0,0 +1,34 @@ +import { useQuery, type CombinedError } from 'urql'; +import { ViewerDocument, type ViewerQuery } from '../gql/graphql.js'; + +// eslint-disable-next-line @typescript-eslint/no-unused-expressions -- used by codegen +/* GraphQL */ ` + query Viewer { + viewer { + email + emailVerified + status + } + } +`; + +type UseViewer = { + fetching: boolean; + error: CombinedError | undefined; + viewer: ViewerQuery['viewer']; +}; + +/** + * The caller's identity and provisioning state. + * + * `viewer` is unauthenticated-safe on the server, so this is the one query that + * still answers for a user who is logged in to Auth0 but linked to no business. + */ +export const useViewer = (options?: { pause?: boolean }): UseViewer => { + const [{ data, fetching, error }] = useQuery({ + query: ViewerDocument, + pause: options?.pause ?? false, + }); + + return { fetching, error, viewer: data?.viewer ?? null }; +}; diff --git a/packages/client/src/providers/urql-error-handler.ts b/packages/client/src/providers/urql-error-handler.ts index 9d28ab2ab1..6ad6cb14a8 100644 --- a/packages/client/src/providers/urql-error-handler.ts +++ b/packages/client/src/providers/urql-error-handler.ts @@ -17,6 +17,12 @@ export function handleUrqlError(result: OperationResult) { const graphqlError = result.error.graphQLErrors[0]; const { message } = graphqlError; + // An unprovisioned account fails every guarded operation at once. The + // /welcome screen explains it; a toast per failed query would only be noise. + if (graphqlError.extensions?.code === 'ONBOARDING_REQUIRED') { + return; + } + // Show toast for common GraphQL errors console.error('GraphQL Error:', graphqlError); toast.error('Operation Error', { diff --git a/packages/client/src/router/config.tsx b/packages/client/src/router/config.tsx index 6171e26b96..9f2c930bbe 100644 --- a/packages/client/src/router/config.tsx +++ b/packages/client/src/router/config.tsx @@ -206,6 +206,9 @@ const AcceptInvitationPage = lazy(() => default: m.AcceptInvitationPage, })), ); +const WelcomePage = lazy(() => + import('../components/screens/welcome.js').then(m => ({ default: m.WelcomePage })), +); /** * Helper to wrap components with Suspense @@ -254,6 +257,15 @@ export const routes: RouteObject[] = [ title: 'Accept Invitation', }, }, + // Authenticated but not linked to a business: outside the dashboard shell, + // so it must not sit under ProtectedRoute (its guard redirects here). + { + path: ROUTES.WELCOME, + element: withSuspense(WelcomePage), + handle: { + title: 'Welcome', + }, + }, // Protected routes (require authentication) { diff --git a/packages/client/src/router/guards/auth-guards.tsx b/packages/client/src/router/guards/auth-guards.tsx index 52a7997531..b0361e23e6 100644 --- a/packages/client/src/router/guards/auth-guards.tsx +++ b/packages/client/src/router/guards/auth-guards.tsx @@ -2,6 +2,7 @@ import type { ReactElement } from 'react'; import { Navigate, useLocation } from 'react-router-dom'; import { useAuth0 } from '@auth0/auth0-react'; import { PageSkeleton } from '../../components/layout/page-skeleton.js'; +import { useViewer } from '../../hooks/use-viewer.js'; import { ROUTES } from '../routes.js'; type GuardProps = { @@ -15,7 +16,11 @@ export function ProtectedRoute({ children }: GuardProps): ReactElement { return children; } - return {children}; + return ( + + {children} + + ); } function Auth0ProtectedRoute({ children }: GuardProps): ReactElement { @@ -39,6 +44,31 @@ function Auth0ProtectedRoute({ children }: GuardProps): ReactElement { return children; } +/** + * Keeps an authenticated-but-unprovisioned user off the app shell. + * + * Being signed in to Auth0 is not enough to use Accounter — the identity must be + * linked to at least one business. Without this guard such a user renders the + * dashboard over queries that all fail, with no explanation and no way forward. + * + * Renders children on a query error rather than trapping the user on /welcome: + * a network blip should not look like a missing workspace, and every underlying + * screen handles its own failures. + */ +export function OnboardingGuard({ children }: GuardProps): ReactElement { + const { fetching, error, viewer } = useViewer(); + + if (fetching) { + return ; + } + + if (!error && viewer && viewer.status !== 'ACTIVE') { + return ; + } + + return children; +} + export function PublicOnlyGuard({ children }: GuardProps): ReactElement { if (isDevAuthEnabled) { return children; diff --git a/packages/client/src/router/routes.ts b/packages/client/src/router/routes.ts index be5f418100..ed1028fe53 100644 --- a/packages/client/src/router/routes.ts +++ b/packages/client/src/router/routes.ts @@ -68,6 +68,7 @@ export const ROUTES = { HOME: '/', LOGIN: '/login', AUTH_CALLBACK: '/auth/callback', + WELCOME: '/welcome', ACCEPT_INVITATION: (token = ':token') => `/accept-invitation/${token}`, NETWORK_ERROR: '/network-error', diff --git a/packages/server/src/modules/auth/directives/__tests__/auth-directives.test.ts b/packages/server/src/modules/auth/directives/__tests__/auth-directives.test.ts index 2b75aa76a2..9ce8e0ff51 100644 --- a/packages/server/src/modules/auth/directives/__tests__/auth-directives.test.ts +++ b/packages/server/src/modules/auth/directives/__tests__/auth-directives.test.ts @@ -6,7 +6,9 @@ import { describe, expect, it } from 'vitest'; import { AuthContextProvider } from '../../providers/auth-context.provider.js'; import { authDirectiveTransformer } from '../auth-directives.js'; -function createUnitYoga(roleId: string | null) { +type JwtIdentity = { auth0UserId: string; email: string | null; emailVerified: boolean } | null; + +function createUnitYoga(roleId: string | null, jwtIdentity: JwtIdentity = null) { const testModule = createModule({ id: 'auth-directive-unit-test', typeDefs: [ @@ -46,6 +48,7 @@ function createUnitYoga(roleId: string | null) { tenant: { businessId: 'biz-1' }, }; }, + getJwtIdentity: async () => jwtIdentity, }), scope: Scope.Operation, }, @@ -76,6 +79,7 @@ function createUnitYoga(roleId: string | null) { tenant: { businessId: 'biz-1' }, }; }, + getJwtIdentity: async () => jwtIdentity, }; }, }, @@ -109,6 +113,29 @@ describe('authDirectiveTransformer', () => { expect(result.errors?.[0]?.extensions?.code).toBe('UNAUTHENTICATED'); }); + it('@requiresAuth throws ONBOARDING_REQUIRED when the JWT is valid but unlinked', async () => { + const yoga = createUnitYoga(null, { + auth0UserId: 'auth0|new-user', + email: 'new@example.com', + emailVerified: true, + }); + const result = await execute(yoga, '{ secure }'); + + expect(result.data).toBeNull(); + expect(result.errors?.[0]?.extensions?.code).toBe('ONBOARDING_REQUIRED'); + }); + + it('@requiresAnyRole throws ONBOARDING_REQUIRED when the JWT is valid but unlinked', async () => { + const yoga = createUnitYoga(null, { + auth0UserId: 'auth0|new-user', + email: 'new@example.com', + emailVerified: true, + }); + const result = await execute(yoga, '{ ownerOrAccountant }'); + + expect(result.errors?.[0]?.extensions?.code).toBe('ONBOARDING_REQUIRED'); + }); + it('@requiresRole passes when role matches', async () => { const yoga = createUnitYoga('business_owner'); const result = await execute(yoga, '{ ownerOnly }'); diff --git a/packages/server/src/modules/auth/directives/auth-directives.ts b/packages/server/src/modules/auth/directives/auth-directives.ts index a980a24e21..2ae3a93abd 100644 --- a/packages/server/src/modules/auth/directives/auth-directives.ts +++ b/packages/server/src/modules/auth/directives/auth-directives.ts @@ -139,6 +139,20 @@ export function authDirectiveTransformer(schema: GraphQLSchema): GraphQLSchema { const authContext = await authProvider.getAuthContext(); if (!authContext?.user) { + // A valid Auth0 identity that maps to no local membership is not an + // authentication failure — refreshing the token can never fix it. + // Report it distinctly so the client routes to onboarding instead of + // looping through a token refresh / re-login prompt. + // Optional call: the injector hands back whatever is registered for + // the token, and partial test doubles of this provider are common. + const identity = await authProvider.getJwtIdentity?.(); + + if (identity) { + throw new GraphQLError('No workspace is linked to this account', { + extensions: { code: 'ONBOARDING_REQUIRED' }, + }); + } + throw new GraphQLError('Authentication required', { extensions: { code: 'UNAUTHENTICATED' }, }); diff --git a/packages/server/src/modules/common/__tests__/viewer.resolver.test.ts b/packages/server/src/modules/common/__tests__/viewer.resolver.test.ts new file mode 100644 index 0000000000..2e8e7ed600 --- /dev/null +++ b/packages/server/src/modules/common/__tests__/viewer.resolver.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, it, vi } from 'vitest'; +import { AuthContextProvider } from '../../auth/providers/auth-context.provider.js'; +import { viewerResolvers } from '../resolvers/viewer.resolver.js'; + +type AuthContextValue = Awaited>; +type JwtIdentity = Awaited>; + +async function runResolver(authContext: AuthContextValue, jwtIdentity: JwtIdentity) { + const authProvider = { + getAuthContext: vi.fn().mockResolvedValue(authContext), + getJwtIdentity: vi.fn().mockResolvedValue(jwtIdentity), + }; + const injector = { get: vi.fn(() => authProvider) }; + const resolver = viewerResolvers.Query!.viewer as unknown as ( + parent: unknown, + args: unknown, + context: { injector: { get: (token: unknown) => unknown } }, + info: unknown, + ) => Promise | null>; + + return { + result: await resolver(undefined, undefined, { injector }, undefined), + authProvider, + }; +} + +const linkedContext = { + authType: 'jwt', + user: { email: 'member@example.com', emailVerified: true }, +} as unknown as AuthContextValue; + +describe('viewer resolver', () => { + it('reports ACTIVE when the identity resolves to an auth context', async () => { + const { result, authProvider } = await runResolver(linkedContext, null); + + expect(result).toEqual({ + email: 'member@example.com', + emailVerified: true, + status: 'ACTIVE', + }); + // An active member must not pay for a second JWT verification. + expect(authProvider.getJwtIdentity).not.toHaveBeenCalled(); + }); + + it('reports NO_WORKSPACE for a verified identity with no membership', async () => { + const { result } = await runResolver(null, { + auth0UserId: 'auth0|new-user', + email: 'new@example.com', + emailVerified: true, + }); + + expect(result).toEqual({ + email: 'new@example.com', + emailVerified: true, + status: 'NO_WORKSPACE', + }); + }); + + it('reports EMAIL_UNVERIFIED before the email is verified', async () => { + const { result } = await runResolver(null, { + auth0UserId: 'auth0|new-user', + email: 'new@example.com', + emailVerified: false, + }); + + expect(result).toEqual({ + email: 'new@example.com', + emailVerified: false, + status: 'EMAIL_UNVERIFIED', + }); + }); + + it('returns null when the request carries no valid credentials', async () => { + const { result } = await runResolver(null, null); + + expect(result).toBeNull(); + }); +}); diff --git a/packages/server/src/modules/common/index.ts b/packages/server/src/modules/common/index.ts index 2240d22527..6f3dfd4945 100644 --- a/packages/server/src/modules/common/index.ts +++ b/packages/server/src/modules/common/index.ts @@ -2,17 +2,19 @@ import { createModule } from 'graphql-modules'; import { AuditLogsProvider } from './providers/audit-logs.provider.js'; import { scalarsResolvers } from './resolvers/common.resolver.js'; import { userContextResolvers } from './resolvers/user-context.resolver.js'; +import { viewerResolvers } from './resolvers/viewer.resolver.js'; import common from './typeDefs/common.graphql.js'; import errors from './typeDefs/errors.graphql.js'; import userContext from './typeDefs/user-context.graphql.js'; +import viewer from './typeDefs/viewer.graphql.js'; const __dirname = import.meta.dirname; export const commonModule = createModule({ id: 'common', dirname: __dirname, - typeDefs: [common, errors, userContext], - resolvers: [scalarsResolvers, userContextResolvers], + typeDefs: [common, errors, userContext, viewer], + resolvers: [scalarsResolvers, userContextResolvers, viewerResolvers], providers: () => [AuditLogsProvider], }); diff --git a/packages/server/src/modules/common/resolvers/viewer.resolver.ts b/packages/server/src/modules/common/resolvers/viewer.resolver.ts new file mode 100644 index 0000000000..ac7b3d41ed --- /dev/null +++ b/packages/server/src/modules/common/resolvers/viewer.resolver.ts @@ -0,0 +1,40 @@ +import { AuthContextProvider } from '../../auth/providers/auth-context.provider.js'; +import type { CommonModule } from '../types.js'; + +/** + * `viewer` is intentionally not `@requiresAuth`: its whole purpose is to describe + * identities that have no auth context yet (a valid Auth0 login that is not linked + * to any business). It therefore verifies the JWT itself via `getJwtIdentity()` and + * returns nothing beyond the caller's own token claims. + */ +export const viewerResolvers: CommonModule.Resolvers = { + Query: { + viewer: async (_, __, { injector }) => { + const authProvider = injector.get(AuthContextProvider); + + // A resolvable auth context means the identity is linked to at least one + // business. This also covers the non-JWT auth types (API key, dev bypass). + const authContext = await authProvider.getAuthContext(); + if (authContext?.user) { + return { + email: authContext.user.email || null, + emailVerified: authContext.user.emailVerified, + status: 'ACTIVE', + }; + } + + // No context: fall back to the raw verified identity. Absent/invalid + // credentials resolve to null here, and `viewer` stays null. + const identity = await authProvider.getJwtIdentity(); + if (!identity) { + return null; + } + + return { + email: identity.email, + emailVerified: identity.emailVerified, + status: identity.emailVerified ? 'NO_WORKSPACE' : 'EMAIL_UNVERIFIED', + }; + }, + }, +}; diff --git a/packages/server/src/modules/common/typeDefs/viewer.graphql.ts b/packages/server/src/modules/common/typeDefs/viewer.graphql.ts new file mode 100644 index 0000000000..257efef8cb --- /dev/null +++ b/packages/server/src/modules/common/typeDefs/viewer.graphql.ts @@ -0,0 +1,25 @@ +import { gql } from 'graphql-modules'; + +export default gql` + extend type Query { + " the caller's own identity and provisioning state; null when the request carries no valid credentials " + viewer: Viewer + } + + " provisioning state of the calling identity " + enum ViewerStatus { + " linked to at least one business; the app is usable " + ACTIVE + " authenticated, but the identity provider has not verified the email address yet " + EMAIL_UNVERIFIED + " authenticated and verified, but not linked to any business " + NO_WORKSPACE + } + + " the calling identity, exposing nothing beyond the caller's own credentials " # eslint-disable-next-line @graphql-eslint/strict-id-in-types -- identity of the caller; has no addressable id + type Viewer { + email: String + emailVerified: Boolean! + status: ViewerStatus! + } +`; From 8b10307e6318341460dfd792b9dfe6955b8e1690 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 13:25:06 +0000 Subject: [PATCH 2/2] fix(auth): address PR review on onboarding phase 0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Memoize getJwtIdentity per operation. The auth directives call it on every guarded field that fails to resolve a context, so an unprovisioned user's multi-field query re-verified the same JWT once per field. Caches the promise, so concurrent field resolution collapses into one verification too. - Show an explicit "could not check your account" state on /welcome when the viewer query fails, instead of asserting "No workspace yet" on unknown state. - Document ViewerStatus precedence: membership decides before email verification. Also isolates the new getJwtIdentity tests on their own Auth0 domain — the provider's JWKS cache is module-global and keyed by domain, so warming it made an existing test that asserts createRemoteJWKSet was called depend on order. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01344D5q1uyAHuwRJ3yyDX46 --- .../screens/__tests__/welcome.test.ts | 121 ++++++++++++++++++ .../client/src/components/screens/welcome.tsx | 27 +++- .../providers/__tests__/auth-context.test.ts | 49 +++++++ .../auth/providers/auth-context.provider.ts | 27 +++- .../common/__tests__/viewer.resolver.test.ts | 15 +++ .../modules/common/typeDefs/viewer.graphql.ts | 6 +- 6 files changed, 236 insertions(+), 9 deletions(-) create mode 100644 packages/client/src/components/screens/__tests__/welcome.test.ts diff --git a/packages/client/src/components/screens/__tests__/welcome.test.ts b/packages/client/src/components/screens/__tests__/welcome.test.ts new file mode 100644 index 0000000000..fe8f4751eb --- /dev/null +++ b/packages/client/src/components/screens/__tests__/welcome.test.ts @@ -0,0 +1,121 @@ +// @vitest-environment happy-dom + +import React from 'react'; +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { createMemoryRouter, RouterProvider } from 'react-router-dom'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { ROUTES } from '../../../router/routes.js'; +import { WelcomePage } from '../welcome.js'; + +const { useAuth0Mock, useViewerMock, useLogoutMock } = vi.hoisted(() => ({ + useAuth0Mock: vi.fn(), + useViewerMock: vi.fn(), + useLogoutMock: vi.fn(), +})); + +vi.mock('@auth0/auth0-react', () => ({ + useAuth0: useAuth0Mock, +})); + +vi.mock('../../../hooks/use-viewer.js', () => ({ + useViewer: useViewerMock, +})); + +vi.mock('../../../hooks/use-logout.js', () => ({ + useLogout: useLogoutMock, +})); + +( + globalThis as typeof globalThis & { + IS_REACT_ACT_ENVIRONMENT?: boolean; + } +).IS_REACT_ACT_ENVIRONMENT = true; + +async function renderWelcome(viewerState: Record) { + useAuth0Mock.mockReturnValue({ isAuthenticated: true, isLoading: false }); + useViewerMock.mockReturnValue(viewerState); + useLogoutMock.mockReturnValue(vi.fn()); + + const router = createMemoryRouter( + [ + { path: ROUTES.WELCOME, element: React.createElement(WelcomePage) }, + { path: ROUTES.HOME, element: React.createElement('div', null, 'Home Page') }, + ], + { initialEntries: [ROUTES.WELCOME] }, + ); + + const container = document.createElement('div'); + document.body.append(container); + + let root: Root | null = null; + await act(async () => { + root = createRoot(container); + root.render(React.createElement(RouterProvider, { router })); + await Promise.resolve(); + }); + + const html = container.innerHTML; + + const cleanup = async () => { + await act(async () => { + root?.unmount(); + await Promise.resolve(); + }); + container.remove(); + }; + + return { html, router, cleanup }; +} + +describe('WelcomePage', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('explains the invitation-only model when the user has no workspace', async () => { + const { html, cleanup } = await renderWelcome({ + fetching: false, + error: undefined, + viewer: { email: 'new@example.com', emailVerified: true, status: 'NO_WORKSPACE' }, + }); + + expect(html).toContain('No workspace yet'); + expect(html).toContain('invitation-only'); + await cleanup(); + }); + + it('asks an unverified user to verify their email', async () => { + const { html, cleanup } = await renderWelcome({ + fetching: false, + error: undefined, + viewer: { email: 'new@example.com', emailVerified: false, status: 'EMAIL_UNVERIFIED' }, + }); + + expect(html).toContain('Verify your email'); + await cleanup(); + }); + + it('reports an unknown state instead of "no workspace" when the query fails', async () => { + const { html, cleanup } = await renderWelcome({ + fetching: false, + error: new Error('network down'), + viewer: null, + }); + + expect(html).toContain('Could not check your account'); + expect(html).not.toContain('No workspace yet'); + await cleanup(); + }); + + it('returns an active viewer to the app', async () => { + const { router, cleanup } = await renderWelcome({ + fetching: false, + error: undefined, + viewer: { email: 'member@example.com', emailVerified: true, status: 'ACTIVE' }, + }); + + expect(router.state.location.pathname).toBe(ROUTES.HOME); + await cleanup(); + }); +}); diff --git a/packages/client/src/components/screens/welcome.tsx b/packages/client/src/components/screens/welcome.tsx index f60ebcb7b2..f0873c422e 100644 --- a/packages/client/src/components/screens/welcome.tsx +++ b/packages/client/src/components/screens/welcome.tsx @@ -5,6 +5,7 @@ import { useAuth0 } from '@auth0/auth0-react'; import { useLogout } from '../../hooks/use-logout.js'; import { useViewer } from '../../hooks/use-viewer.js'; import { ROUTES } from '../../router/routes.js'; +import { Alert, AlertDescription, AlertTitle } from '../ui/alert.js'; import { Button } from '../ui/button.js'; /** @@ -16,7 +17,7 @@ import { Button } from '../ui/button.js'; * dashboard buried under failing queries. */ export function WelcomePage(): ReactElement { - const { fetching, viewer } = useViewer(); + const { fetching, error, viewer } = useViewer(); const { isAuthenticated, isLoading } = useAuth0(); const handleLogout = useLogout(); const navigate = useNavigate(); @@ -41,6 +42,30 @@ export function WelcomePage(): ReactElement { return ; } + // Provisioning state is unknown when the query fails, so say that rather than + // asserting the user has no workspace — a network blip is not an entitlement. + if (error) { + return ( +
+
+ + Could not check your account + + We could not reach the server to see which businesses you belong to. Check your + connection and try again. + + +
+ + +
+
+
+ ); + } + const isEmailUnverified = viewer?.status === 'EMAIL_UNVERIFIED'; return ( diff --git a/packages/server/src/modules/auth/providers/__tests__/auth-context.test.ts b/packages/server/src/modules/auth/providers/__tests__/auth-context.test.ts index 9185584f5d..00116dc02b 100644 --- a/packages/server/src/modules/auth/providers/__tests__/auth-context.test.ts +++ b/packages/server/src/modules/auth/providers/__tests__/auth-context.test.ts @@ -54,6 +54,55 @@ describe('AuthContextProvider', () => { expect(result).toBeNull(); }); + describe('getJwtIdentity', () => { + // The JWKS cache in the provider module is global and keyed by domain, and + // `vi.clearAllMocks()` does not clear it. Using a domain of our own keeps + // these cases from warming the entry other tests assert on. + let identityProvider: AuthContextProvider; + + beforeEach(() => { + identityProvider = new AuthContextProvider( + { auth0: { domain: 'jwt-identity.auth0.com', audience: 'test-audience' } } as any, + mockRawAuth, + mockDBProvider, + ); + }); + + it('verifies the JWT once per operation across repeated calls', async () => { + // The auth directives call this on every guarded field that fails to + // resolve a context, so an unprovisioned user's multi-field query would + // otherwise re-verify the same token once per field. + vi.mocked(jose.jwtVerify).mockResolvedValue({ + payload: { sub: 'auth0|123', email: 'test@example.com', email_verified: true }, + } as any); + + const first = await identityProvider.getJwtIdentity(); + const second = await identityProvider.getJwtIdentity(); + + expect(first).toEqual({ + auth0UserId: 'auth0|123', + email: 'test@example.com', + emailVerified: true, + }); + expect(second).toEqual(first); + expect(jose.jwtVerify).toHaveBeenCalledTimes(1); + }); + + it('verifies once when concurrent callers race', async () => { + vi.mocked(jose.jwtVerify).mockResolvedValue({ + payload: { sub: 'auth0|123', email: 'test@example.com', email_verified: true }, + } as any); + + const [first, second] = await Promise.all([ + identityProvider.getJwtIdentity(), + identityProvider.getJwtIdentity(), + ]); + + expect(second).toEqual(first); + expect(jose.jwtVerify).toHaveBeenCalledTimes(1); + }); + }); + describe('JWT Verification', () => { it('should verify JWT and return context if valid', async () => { const mockPayload = { diff --git a/packages/server/src/modules/auth/providers/auth-context.provider.ts b/packages/server/src/modules/auth/providers/auth-context.provider.ts index ef0b142545..1e7eaa1d64 100644 --- a/packages/server/src/modules/auth/providers/auth-context.provider.ts +++ b/packages/server/src/modules/auth/providers/auth-context.provider.ts @@ -19,6 +19,12 @@ const jwksCache = new Map>(); type QueryableDB = Pick; +type JwtIdentity = { + auth0UserId: string; + email: string | null; + emailVerified: boolean; +}; + export async function handleDevBypassAuth( db: QueryableDB, userId: string, @@ -84,6 +90,7 @@ export async function handleDevBypassAuth( export class AuthContextProvider { private cachedContext: AuthContext | null | undefined = undefined; private handlingAuth: Promise | null = null; + private jwtIdentity: Promise | null = null; constructor( @Inject(ENVIRONMENT) private env: Environment, @@ -91,11 +98,21 @@ export class AuthContextProvider { @Inject(DBProvider) private db: DBProvider, ) {} - public async getJwtIdentity(): Promise<{ - auth0UserId: string; - email: string | null; - emailVerified: boolean; - } | null> { + /** + * The verified identity behind this request's JWT, or null. + * + * Memoized for the operation: the auth directives call this on every guarded + * field that fails to resolve an auth context, so an unprovisioned user with a + * multi-field query would otherwise pay for one `jwtVerify` per field. Caching + * the promise (not the value) also collapses concurrent field resolution into a + * single verification. Safe because `rawAuth` is fixed for the operation. + */ + public getJwtIdentity(): Promise { + this.jwtIdentity ??= this.resolveJwtIdentity(); + return this.jwtIdentity; + } + + private async resolveJwtIdentity(): Promise { const token = this.rawAuth.token; if (this.rawAuth.authType !== 'jwt' || !token) { return null; diff --git a/packages/server/src/modules/common/__tests__/viewer.resolver.test.ts b/packages/server/src/modules/common/__tests__/viewer.resolver.test.ts index 2e8e7ed600..6f428cd775 100644 --- a/packages/server/src/modules/common/__tests__/viewer.resolver.test.ts +++ b/packages/server/src/modules/common/__tests__/viewer.resolver.test.ts @@ -70,6 +70,21 @@ describe('viewer resolver', () => { }); }); + it('keeps a linked member ACTIVE even when their email is unverified', async () => { + // Membership decides: the API already serves this caller's data, so routing + // them to /welcome would lock out a working account over a claim that is not + // enforced anywhere else. + const { result } = await runResolver( + { + authType: 'jwt', + user: { email: 'member@example.com', emailVerified: false }, + } as unknown as AuthContextValue, + null, + ); + + expect(result).toMatchObject({ status: 'ACTIVE', emailVerified: false }); + }); + it('returns null when the request carries no valid credentials', async () => { const { result } = await runResolver(null, null); diff --git a/packages/server/src/modules/common/typeDefs/viewer.graphql.ts b/packages/server/src/modules/common/typeDefs/viewer.graphql.ts index 257efef8cb..817645defc 100644 --- a/packages/server/src/modules/common/typeDefs/viewer.graphql.ts +++ b/packages/server/src/modules/common/typeDefs/viewer.graphql.ts @@ -6,13 +6,13 @@ export default gql` viewer: Viewer } - " provisioning state of the calling identity " + " provisioning state of the calling identity; membership decides first, so a linked caller is ACTIVE regardless of email verification, and the other states describe why an unlinked caller cannot be matched to a business yet " enum ViewerStatus { " linked to at least one business; the app is usable " ACTIVE - " authenticated, but the identity provider has not verified the email address yet " + " not linked, and the email address is unverified — so it cannot be matched to an invitation " EMAIL_UNVERIFIED - " authenticated and verified, but not linked to any business " + " not linked, with a verified email; awaiting an invitation " NO_WORKSPACE }