From 8fee03414bedaced9c2bfb835726ab6b8735c5ce Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Tue, 21 Apr 2026 12:54:39 -0400 Subject: [PATCH] fix(api,app): unblock onboarding frameworks list for users without active org MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The GET /v1/frameworks/available endpoint was changed on April 17 to read @OrganizationId() from the request. That decorator throws when the user has no active organization, producing HTTP 500 for every fresh signup on the first onboarding step — even though @SkipOrgCheck() is specifically meant to allow this case. - Add @OrganizationIdOptional() for @SkipOrgCheck()-decorated endpoints - Harden @OrganizationId() to throw InternalServerErrorException with a message pointing to the optional variant, so future misuse is visible in monitoring instead of a generic 500 - Swap findAvailable to use @OrganizationIdOptional() - Add regression tests covering both no-org and with-org cases - Surface API errors in FrameworkSelection.tsx so the next server-side failure doesn't produce a silent blank onboarding screen Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/api/src/auth/auth-context.decorator.ts | 28 ++++++++++++--- .../frameworks/frameworks.controller.spec.ts | 27 +++++++++++++++ .../src/frameworks/frameworks.controller.ts | 8 +++-- .../setup/components/FrameworkSelection.tsx | 34 +++++++++++++++++-- 4 files changed, 89 insertions(+), 8 deletions(-) diff --git a/apps/api/src/auth/auth-context.decorator.ts b/apps/api/src/auth/auth-context.decorator.ts index 1d5590a8a0..bdc1a6beb1 100644 --- a/apps/api/src/auth/auth-context.decorator.ts +++ b/apps/api/src/auth/auth-context.decorator.ts @@ -1,4 +1,8 @@ -import { createParamDecorator, ExecutionContext } from '@nestjs/common'; +import { + createParamDecorator, + ExecutionContext, + InternalServerErrorException, +} from '@nestjs/common'; import { AuthContext as AuthContextType, AuthenticatedRequest } from './types'; /** @@ -46,7 +50,10 @@ export const AuthContext = createParamDecorator( ); /** - * Parameter decorator to extract just the organization ID + * Parameter decorator to extract just the organization ID. + * Throws when no active organization is present on the request — only use this + * on endpoints that require an active organization. For endpoints decorated + * with @SkipOrgCheck() (e.g. onboarding), use @OrganizationIdOptional() instead. */ export const OrganizationId = createParamDecorator( (data: unknown, ctx: ExecutionContext): string => { @@ -54,8 +61,8 @@ export const OrganizationId = createParamDecorator( const { organizationId } = request; if (!organizationId) { - throw new Error( - 'Organization ID not found. Ensure HybridAuthGuard is applied.', + throw new InternalServerErrorException( + 'Organization ID missing on request. If this endpoint is @SkipOrgCheck()-decorated, use @OrganizationIdOptional() instead.', ); } @@ -63,6 +70,19 @@ export const OrganizationId = createParamDecorator( }, ); +/** + * Parameter decorator to extract the organization ID when it may be absent. + * Returns `undefined` instead of throwing when no active organization is + * present. Use this on endpoints decorated with @SkipOrgCheck() where the + * user may not yet have an active organization (e.g. during onboarding). + */ +export const OrganizationIdOptional = createParamDecorator( + (data: unknown, ctx: ExecutionContext): string | undefined => { + const request = ctx.switchToHttp().getRequest(); + return request.organizationId || undefined; + }, +); + /** * Parameter decorator to extract the user ID (only available for session auth) */ diff --git a/apps/api/src/frameworks/frameworks.controller.spec.ts b/apps/api/src/frameworks/frameworks.controller.spec.ts index a8250426f1..b8996ce7cf 100644 --- a/apps/api/src/frameworks/frameworks.controller.spec.ts +++ b/apps/api/src/frameworks/frameworks.controller.spec.ts @@ -15,6 +15,7 @@ describe('FrameworksController', () => { const mockService = { findAll: jest.fn(), + findAvailable: jest.fn(), delete: jest.fn(), }; @@ -68,6 +69,32 @@ describe('FrameworksController', () => { }); }); + describe('findAvailable', () => { + // Regression test for the onboarding 500 bug: this endpoint must not throw + // when the authenticated user has no active organization yet (fresh signups + // hitting the first onboarding step). Previously used @OrganizationId(), + // which threw when organizationId was empty → HTTP 500. + it('should return frameworks when user has no active organization', async () => { + const mockFrameworks = [ + { id: 'frk_1', name: 'soc2', visible: true, isCustom: false }, + ]; + mockService.findAvailable.mockResolvedValue(mockFrameworks); + + const result = await controller.findAvailable(undefined); + + expect(result).toEqual({ data: mockFrameworks, count: 1 }); + expect(service.findAvailable).toHaveBeenCalledWith(undefined); + }); + + it('should pass organizationId to service when user has an active org', async () => { + mockService.findAvailable.mockResolvedValue([]); + + await controller.findAvailable('org_1'); + + expect(service.findAvailable).toHaveBeenCalledWith('org_1'); + }); + }); + describe('delete', () => { it('should delegate to service and return result', async () => { mockService.delete.mockResolvedValue({ success: true }); diff --git a/apps/api/src/frameworks/frameworks.controller.ts b/apps/api/src/frameworks/frameworks.controller.ts index cee958679f..3276d8cc6a 100644 --- a/apps/api/src/frameworks/frameworks.controller.ts +++ b/apps/api/src/frameworks/frameworks.controller.ts @@ -18,7 +18,11 @@ import { HybridAuthGuard } from '../auth/hybrid-auth.guard'; import { PermissionGuard } from '../auth/permission.guard'; import { RequirePermission } from '../auth/require-permission.decorator'; import { SkipOrgCheck } from '../auth/skip-org-check.decorator'; -import { AuthContext, OrganizationId } from '../auth/auth-context.decorator'; +import { + AuthContext, + OrganizationId, + OrganizationIdOptional, +} from '../auth/auth-context.decorator'; import type { AuthContext as AuthContextType } from '../auth/types'; import { FrameworksService } from './frameworks.service'; import { AddFrameworksDto } from './dto/add-frameworks.dto'; @@ -57,7 +61,7 @@ export class FrameworksController { summary: 'List available frameworks (requires session, no active org needed — used during onboarding)', }) - async findAvailable(@OrganizationId() organizationId?: string) { + async findAvailable(@OrganizationIdOptional() organizationId?: string) { const data = await this.frameworksService.findAvailable(organizationId); return { data, count: data.length }; } diff --git a/apps/app/src/app/(app)/setup/components/FrameworkSelection.tsx b/apps/app/src/app/(app)/setup/components/FrameworkSelection.tsx index fe6172af72..8173ace9b1 100644 --- a/apps/app/src/app/(app)/setup/components/FrameworkSelection.tsx +++ b/apps/app/src/app/(app)/setup/components/FrameworkSelection.tsx @@ -19,11 +19,22 @@ export function FrameworkSelection({ value, onChange, onLoadingChange }: Framewo const onChangeRef = useRef(onChange); const valueRef = useRef(value); - const { data: frameworks = [], isLoading } = useSWR( + const { + data: frameworks = [], + isLoading, + error, + mutate, + } = useSWR( '/v1/frameworks/available', async (endpoint: string) => { const response = await api.get<{ data: Framework[] }>(endpoint); - return Array.isArray(response.data?.data) ? response.data.data : []; + if (response.error || !response.data) { + throw new Error( + response.error || + `Failed to load frameworks (HTTP ${response.status})`, + ); + } + return Array.isArray(response.data.data) ? response.data.data : []; }, ); @@ -52,6 +63,25 @@ export function FrameworkSelection({ value, onChange, onLoadingChange }: Framewo return null; } + if (error) { + const message = + error instanceof Error ? error.message : 'Something went wrong.'; + return ( +
+

+ We couldn't load the compliance frameworks. {message} +

+ +
+ ); + } + return (
{frameworks