Skip to content

Commit 6826150

Browse files
authored
Merge pull request #2623 from trycompai/main
[comp] Production Deploy
2 parents a7761dd + 4466792 commit 6826150

4 files changed

Lines changed: 89 additions & 8 deletions

File tree

apps/api/src/auth/auth-context.decorator.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
import { createParamDecorator, ExecutionContext } from '@nestjs/common';
1+
import {
2+
createParamDecorator,
3+
ExecutionContext,
4+
InternalServerErrorException,
5+
} from '@nestjs/common';
26
import { AuthContext as AuthContextType, AuthenticatedRequest } from './types';
37

48
/**
@@ -46,23 +50,39 @@ export const AuthContext = createParamDecorator(
4650
);
4751

4852
/**
49-
* Parameter decorator to extract just the organization ID
53+
* Parameter decorator to extract just the organization ID.
54+
* Throws when no active organization is present on the request — only use this
55+
* on endpoints that require an active organization. For endpoints decorated
56+
* with @SkipOrgCheck() (e.g. onboarding), use @OrganizationIdOptional() instead.
5057
*/
5158
export const OrganizationId = createParamDecorator(
5259
(data: unknown, ctx: ExecutionContext): string => {
5360
const request = ctx.switchToHttp().getRequest<AuthenticatedRequest>();
5461
const { organizationId } = request;
5562

5663
if (!organizationId) {
57-
throw new Error(
58-
'Organization ID not found. Ensure HybridAuthGuard is applied.',
64+
throw new InternalServerErrorException(
65+
'Organization ID missing on request. If this endpoint is @SkipOrgCheck()-decorated, use @OrganizationIdOptional() instead.',
5966
);
6067
}
6168

6269
return organizationId;
6370
},
6471
);
6572

73+
/**
74+
* Parameter decorator to extract the organization ID when it may be absent.
75+
* Returns `undefined` instead of throwing when no active organization is
76+
* present. Use this on endpoints decorated with @SkipOrgCheck() where the
77+
* user may not yet have an active organization (e.g. during onboarding).
78+
*/
79+
export const OrganizationIdOptional = createParamDecorator(
80+
(data: unknown, ctx: ExecutionContext): string | undefined => {
81+
const request = ctx.switchToHttp().getRequest<AuthenticatedRequest>();
82+
return request.organizationId || undefined;
83+
},
84+
);
85+
6686
/**
6787
* Parameter decorator to extract the user ID (only available for session auth)
6888
*/

apps/api/src/frameworks/frameworks.controller.spec.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ describe('FrameworksController', () => {
1515

1616
const mockService = {
1717
findAll: jest.fn(),
18+
findAvailable: jest.fn(),
1819
delete: jest.fn(),
1920
};
2021

@@ -68,6 +69,32 @@ describe('FrameworksController', () => {
6869
});
6970
});
7071

72+
describe('findAvailable', () => {
73+
// Regression test for the onboarding 500 bug: this endpoint must not throw
74+
// when the authenticated user has no active organization yet (fresh signups
75+
// hitting the first onboarding step). Previously used @OrganizationId(),
76+
// which threw when organizationId was empty → HTTP 500.
77+
it('should return frameworks when user has no active organization', async () => {
78+
const mockFrameworks = [
79+
{ id: 'frk_1', name: 'soc2', visible: true, isCustom: false },
80+
];
81+
mockService.findAvailable.mockResolvedValue(mockFrameworks);
82+
83+
const result = await controller.findAvailable(undefined);
84+
85+
expect(result).toEqual({ data: mockFrameworks, count: 1 });
86+
expect(service.findAvailable).toHaveBeenCalledWith(undefined);
87+
});
88+
89+
it('should pass organizationId to service when user has an active org', async () => {
90+
mockService.findAvailable.mockResolvedValue([]);
91+
92+
await controller.findAvailable('org_1');
93+
94+
expect(service.findAvailable).toHaveBeenCalledWith('org_1');
95+
});
96+
});
97+
7198
describe('delete', () => {
7299
it('should delegate to service and return result', async () => {
73100
mockService.delete.mockResolvedValue({ success: true });

apps/api/src/frameworks/frameworks.controller.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,11 @@ import { HybridAuthGuard } from '../auth/hybrid-auth.guard';
1818
import { PermissionGuard } from '../auth/permission.guard';
1919
import { RequirePermission } from '../auth/require-permission.decorator';
2020
import { SkipOrgCheck } from '../auth/skip-org-check.decorator';
21-
import { AuthContext, OrganizationId } from '../auth/auth-context.decorator';
21+
import {
22+
AuthContext,
23+
OrganizationId,
24+
OrganizationIdOptional,
25+
} from '../auth/auth-context.decorator';
2226
import type { AuthContext as AuthContextType } from '../auth/types';
2327
import { FrameworksService } from './frameworks.service';
2428
import { AddFrameworksDto } from './dto/add-frameworks.dto';
@@ -57,7 +61,7 @@ export class FrameworksController {
5761
summary:
5862
'List available frameworks (requires session, no active org needed — used during onboarding)',
5963
})
60-
async findAvailable(@OrganizationId() organizationId?: string) {
64+
async findAvailable(@OrganizationIdOptional() organizationId?: string) {
6165
const data = await this.frameworksService.findAvailable(organizationId);
6266
return { data, count: data.length };
6367
}

apps/app/src/app/(app)/setup/components/FrameworkSelection.tsx

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,22 @@ export function FrameworkSelection({ value, onChange, onLoadingChange }: Framewo
1919
const onChangeRef = useRef(onChange);
2020
const valueRef = useRef(value);
2121

22-
const { data: frameworks = [], isLoading } = useSWR<Framework[]>(
22+
const {
23+
data: frameworks = [],
24+
isLoading,
25+
error,
26+
mutate,
27+
} = useSWR<Framework[]>(
2328
'/v1/frameworks/available',
2429
async (endpoint: string) => {
2530
const response = await api.get<{ data: Framework[] }>(endpoint);
26-
return Array.isArray(response.data?.data) ? response.data.data : [];
31+
if (response.error || !response.data) {
32+
throw new Error(
33+
response.error ||
34+
`Failed to load frameworks (HTTP ${response.status})`,
35+
);
36+
}
37+
return Array.isArray(response.data.data) ? response.data.data : [];
2738
},
2839
);
2940

@@ -52,6 +63,25 @@ export function FrameworkSelection({ value, onChange, onLoadingChange }: Framewo
5263
return null;
5364
}
5465

66+
if (error) {
67+
const message =
68+
error instanceof Error ? error.message : 'Something went wrong.';
69+
return (
70+
<div className="flex flex-col items-start gap-2">
71+
<p className="text-sm text-destructive">
72+
We couldn't load the compliance frameworks. {message}
73+
</p>
74+
<button
75+
type="button"
76+
onClick={() => mutate()}
77+
className="text-sm underline hover:no-underline"
78+
>
79+
Try again
80+
</button>
81+
</div>
82+
);
83+
}
84+
5585
return (
5686
<div className="flex flex-wrap gap-3 overflow-y-auto pr-1">
5787
{frameworks

0 commit comments

Comments
 (0)