diff --git a/apps/website/.eslintrc.json b/apps/website/.eslintrc.json index 647647e15..424c3eefc 100644 --- a/apps/website/.eslintrc.json +++ b/apps/website/.eslintrc.json @@ -73,6 +73,7 @@ ], "rules": { "gridpilot-rules/mutation-contract": "error", + "gridpilot-rules/mutation-must-use-builders": "error", "gridpilot-rules/filename-service-match": "error" } }, @@ -117,7 +118,20 @@ "gridpilot-rules/rsc-no-di": "error", "gridpilot-rules/rsc-no-local-helpers": "error", "gridpilot-rules/rsc-no-object-construction": "error", - "gridpilot-rules/rsc-no-container-manager-calls": "error" + "gridpilot-rules/rsc-no-container-manager-calls": "error", + "gridpilot-rules/no-hardcoded-search-params": "error", + "gridpilot-rules/no-next-cookies-in-pages": "error" + } + }, + { + "files": [ + "lib/services/**/*.ts", + "lib/page-queries/**/*.ts", + "lib/mutations/**/*.ts", + "middleware.ts" + ], + "rules": { + "gridpilot-rules/no-direct-process-env": "error" } }, { @@ -150,17 +164,6 @@ "gridpilot-rules/no-hardcoded-search-params": "error" } }, - { - "files": [ - "lib/mutations/**/*.ts" - ], - "rules": { - "gridpilot-rules/mutation-contract": "error", - "gridpilot-rules/clean-error-handling": "error", - "gridpilot-rules/single-export-per-file": "error", - "gridpilot-rules/filename-matches-export": "error" - } - }, { "files": [ "templates/**/*.ts", @@ -190,7 +193,9 @@ "rules": { "gridpilot-rules/client-only-no-server-code": "error", "gridpilot-rules/client-only-must-have-directive": "error", - "gridpilot-rules/server-actions-must-use-mutations": "error" + "gridpilot-rules/server-actions-must-use-mutations": "error", + "gridpilot-rules/server-actions-return-result": "error", + "gridpilot-rules/server-actions-interface": "error" } }, { @@ -220,6 +225,18 @@ "gridpilot-rules/lib-no-next-imports": "error" } }, + { + "files": [ + "app/onboarding/**/*.ts", + "app/onboarding/**/*.tsx", + "lib/auth/RouteGuard.ts", + "lib/auth/AuthFlowRouter.ts", + "middleware.ts" + ], + "rules": { + "gridpilot-rules/no-console": "error" + } + }, { "files": [ "app/**/*.tsx", @@ -237,7 +254,9 @@ "app/**/actions/*.ts" ], "rules": { - "gridpilot-rules/no-hardcoded-routes": "error" + "gridpilot-rules/no-hardcoded-routes": "error", + "gridpilot-rules/server-actions-return-result": "error", + "gridpilot-rules/server-actions-interface": "error" } }, { @@ -268,7 +287,6 @@ ], "rules": { "gridpilot-rules/service-function-format": "error", - "gridpilot-rules/services-must-be-marked": "error", "gridpilot-rules/services-must-be-pure": "error", "gridpilot-rules/services-no-external-api": "error", "gridpilot-rules/services-implement-contract": "error", diff --git a/apps/website/lib/contracts/services/Service.ts b/apps/website/lib/contracts/services/Service.ts index b71a45037..ffb123dd1 100644 --- a/apps/website/lib/contracts/services/Service.ts +++ b/apps/website/lib/contracts/services/Service.ts @@ -18,7 +18,6 @@ * - They return Result */ -import { Result } from '@/lib/contracts/Result'; /** * Domain error type for services @@ -35,16 +34,19 @@ export type DomainError = /** * Service interface for orchestration operations - * All service methods must return Result with domain errors + * + * Design Decision: Services with multiple methods CANNOT use a single generic type + * because each method may return different DTOs. Instead: + * + * 1. Single-method services (PageQueries, Mutations): Use Service + * 2. Multi-method services: Don't implement this interface, just follow the pattern + * + * All service methods must return Promise> for type-safe error handling. * * Type Parameters: * - TApiDto: The API Transport DTO type returned on success * - TError: The domain error type (defaults to DomainError) */ -export interface Service { - /** - * Execute a service operation - * Returns Result with API DTO or Domain Error - */ - execute(...args: unknown[]): Promise>; +export interface Service { + // No specific methods - just a marker type } \ No newline at end of file diff --git a/apps/website/middleware.ts b/apps/website/middleware.ts index c3372ee8e..67e9142a3 100644 --- a/apps/website/middleware.ts +++ b/apps/website/middleware.ts @@ -4,6 +4,7 @@ import { SessionGateway } from '@/lib/gateways/SessionGateway'; import { handleAuthFlow } from '@/lib/auth/AuthFlowRouter'; import { ConsoleLogger } from '@/lib/infrastructure/logging/ConsoleLogger'; import { routeMatchers } from '@/lib/routing/RouteConfig'; +import { SearchParamBuilder } from '@/lib/routing/search-params/SearchParamBuilder'; const logger = new ConsoleLogger(); @@ -78,7 +79,7 @@ export async function middleware(request: NextRequest) { } catch (error) { logger.error('[MIDDLEWARE] Error in auth flow', error instanceof Error ? error : new Error(String(error))); // Fallback: redirect to login if there's an error - return NextResponse.redirect(new URL(`/auth/login?returnTo=${encodeURIComponent(pathname)}`, request.url)); + return NextResponse.redirect(new URL(`/auth/login${SearchParamBuilder.auth(pathname)}`, request.url)); } logger.info('[MIDDLEWARE] Decision summary', { @@ -121,4 +122,4 @@ export const config = { */ '/((?!_next/static|_next/image|_next/data|favicon.ico|.*\\.(?:svg|png|jpg|jpeg|gif|webp|mp4|webm|mov|avi)$).*)', ], -}; \ No newline at end of file +}; diff --git a/docs/architecture/website/SESSION.md b/docs/architecture/website/SESSION.md new file mode 100644 index 000000000..d99fbc6c5 --- /dev/null +++ b/docs/architecture/website/SESSION.md @@ -0,0 +1,272 @@ +# Next.js RSC Session Best Practices + +This document defines the authoritative pattern for handling session/authentication in Next.js Server Components and Server Actions within the `apps/website` layer. + +## Core Principle + +**Server Actions should NOT fetch session separately.** The API handles authentication automatically via cookies. + +## The Problem with Manual Session Fetching + +### ❌ Anti-Pattern (Current Implementation) +```typescript +// apps/website/app/onboarding/actions.ts +'use server'; + +import { SessionGateway } from '@/lib/gateways/SessionGateway'; + +async function getCurrentUserId(): Promise { + const gateway = new SessionGateway(); + const session = await gateway.getSession(); // ❌ Extra API call + return session?.user?.userId || null; +} + +export async function completeOnboardingAction(input: CompleteOnboardingInputDTO) { + const userId = await getCurrentUserId(); // ❌ Performance overhead + if (!userId) { + return Result.err('Not authenticated'); + } + + const mutation = new CompleteOnboardingMutation(); + // ... rest of logic +} +``` + +**Problems:** +1. **Performance**: Makes extra API call on every action invocation +2. **Redundancy**: Manual auth check when API handles it automatically +3. **Coupling**: Actions depend on session infrastructure +4. **Inconsistency**: Doesn't match pattern used elsewhere in codebase + +## The Correct Pattern + +### ✅ Server Actions (Thin Wrappers) +```typescript +// apps/website/app/onboarding/actions.ts +'use server'; + +import { Result } from '@/lib/contracts/Result'; +import { CompleteOnboardingMutation } from '@/lib/mutations/onboarding/CompleteOnboardingMutation'; +import { GenerateAvatarsMutation } from '@/lib/mutations/onboarding/GenerateAvatarsMutation'; +import { CompleteOnboardingInputDTO } from '@/lib/types/generated/CompleteOnboardingInputDTO'; +import { revalidatePath } from 'next/cache'; + +/** + * Complete onboarding - thin wrapper around mutation + * + * Pattern: Server Action → Mutation → Service → API Client + * + * Authentication is handled automatically by the API via cookies. + * The BaseApiClient includes credentials: 'include', so cookies are sent automatically. + * If authentication fails, the API returns 401/403 which gets converted to domain errors. + */ +export async function completeOnboardingAction( + input: CompleteOnboardingInputDTO +): Promise> { + const mutation = new CompleteOnboardingMutation(); + const result = await mutation.execute(input); + + if (result.isErr()) { + return Result.err(result.getError()); + } + + revalidatePath('/dashboard'); + return Result.ok({ success: true }); +} + +/** + * Generate avatars - thin wrapper around mutation + * + * Note: This action requires userId to be passed from the client. + * The client should get userId from session and pass it as a parameter. + */ +export async function generateAvatarsAction(params: { + userId: string; + facePhotoData: string; + suitColor: string; +}): Promise> { + const mutation = new GenerateAvatarsMutation(); + const result = await mutation.execute(params); + + if (result.isErr()) { + return Result.err(result.getError()); + } + + const data = result.unwrap(); + return Result.ok({ success: data.success, avatarUrls: data.avatarUrls }); +} +``` + +### ✅ Client Component Pattern +```typescript +// apps/website/app/onboarding/OnboardingWizardClient.tsx +'use client'; + +import { completeOnboardingAction, generateAvatarsAction } from '@/app/onboarding/actions'; +import { useAuth } from '@/lib/hooks/auth/useAuth'; +import { useState } from 'react'; + +export function OnboardingWizardClient() { + const { session } = useAuth(); // Get userId from session (client-side) + const [isSubmitting, setIsSubmitting] = useState(false); + + const handleCompleteOnboarding = async (input: CompleteOnboardingInputDTO) => { + setIsSubmitting(true); + try { + const result = await completeOnboardingAction(input); + + if (result.isErr()) { + // Handle error (show toast, etc.) + console.error('Onboarding failed:', result.getError()); + return; + } + + // Success - redirect or show success message + console.log('Onboarding completed successfully'); + } finally { + setIsSubmitting(false); + } + }; + + const handleGenerateAvatars = async (facePhotoData: string, suitColor: string) => { + if (!session?.user?.userId) { + console.error('User not authenticated'); + return Result.err('Not authenticated'); + } + + setIsSubmitting(true); + try { + const result = await generateAvatarsAction({ + userId: session.user.userId, // Pass userId from session + facePhotoData, + suitColor, + }); + + if (result.isErr()) { + console.error('Avatar generation failed:', result.getError()); + return; + } + + const data = result.unwrap(); + console.log('Avatars generated:', data.avatarUrls); + } finally { + setIsSubmitting(false); + } + }; + + return ( +
+ {/* Wizard implementation */} + +
+ ); +} +``` + +## Why This Pattern Works + +### 1. Automatic Authentication +```typescript +// apps/website/lib/api/base/BaseApiClient.ts +protected async request(method: string, path: string, data?: object, options = {}): Promise { + const config: RequestInit = { + method, + headers, + credentials: 'include', // ✅ Automatically sends cookies + signal, + }; + // ... +} +``` + +The `BaseApiClient` automatically includes `credentials: 'include'`, so: +- Cookies are sent with every API request +- The backend API can authenticate the user +- No manual session fetching needed in server actions + +### 2. Error Handling Flow +``` +User not authenticated + ↓ +API returns 401/403 + ↓ +BaseApiClient throws HttpUnauthorizedError + ↓ +Service catches and converts to DomainError + ↓ +Mutation returns Result.err() + ↓ +Server Action returns error to client + ↓ +Client shows appropriate message +``` + +### 3. Performance Benefits +- **No extra API calls**: Session fetched once on client, reused +- **Reduced latency**: Server actions don't wait for session lookup +- **Better scalability**: Fewer API calls per user action + +### 4. Consistency with Architecture +This pattern matches your existing codebase: +```typescript +// apps/website/app/admin/actions.ts - ✅ Already follows this pattern +export async function updateUserStatus(userId: string, status: string) { + const mutation = new UpdateUserStatusMutation(); + const result = await mutation.execute({ userId, status }); + // ... no manual session fetching +} +``` + +## When Manual Session Fetching IS Needed + +There are rare cases where you might need to fetch session in a server action: + +### Case 1: User ID Required in Request Body +If the API endpoint requires userId in the request body (not just cookies): + +```typescript +// Client passes userId +const result = await generateAvatarsAction({ + userId: session.user.userId, // ✅ Pass from client + facePhotoData, + suitColor, +}); +``` + +### Case 2: Server-Side Authorization Check +If you need to check permissions before calling a mutation: + +```typescript +export async function adminAction(input: AdminInput) { + // Only fetch session if you need to check permissions server-side + const gateway = new SessionGateway(); + const session = await gateway.getSession(); + + if (!session?.user?.roles?.includes('admin')) { + return Result.err('Forbidden'); + } + + const mutation = new AdminMutation(); + return await mutation.execute(input); +} +``` + +**Note**: This is rare. Usually, the API handles authorization and returns 403 if unauthorized. + +## Summary + +| Aspect | Manual Session Fetching | Automatic (Recommended) | +|--------|------------------------|------------------------| +| **Performance** | ❌ Extra API call | ✅ No overhead | +| **Complexity** | ❌ More code | ✅ Simpler | +| **Coupling** | ❌ Depends on session infra | ✅ Decoupled | +| **Consistency** | ❌ Inconsistent | ✅ Matches codebase | +| **Security** | ⚠️ Manual checks | ✅ API enforces | + +## Golden Rule + +**Server Actions are thin wrappers. Mutations handle logic. API enforces security.** + +Never fetch session in server actions unless you have a specific server-side authorization requirement that cannot be handled by the API. \ No newline at end of file