Skip to content

Commit 4cbcdd2

Browse files
Marfuenclaude
andauthored
Merge commit from fork
Resolve organizationId from the authenticated request context in the document and manual-answer handlers (consistent with the list/save handlers), and remove the unused run-token endpoint. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent c3ffd85 commit 4cbcdd2

2 files changed

Lines changed: 108 additions & 58 deletions

File tree

‎apps/api/src/knowledge-base/knowledge-base.controller.spec.ts‎

Lines changed: 76 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,17 @@ jest.mock('../auth/auth.server', () => ({
88
auth: { api: { getSession: jest.fn() } },
99
}));
1010

11+
// The controller pulls in the auth guard (and therefore the Prisma client and
12+
// the @trycompai/auth permission definitions) at import time. The service and
13+
// guards are fully mocked/overridden below, so none of this is exercised — stub
14+
// them out to keep this a hermetic unit test (matches the convention used by
15+
// the other controller specs in this app).
16+
jest.mock('@db', () => ({ db: {} }));
17+
jest.mock('@trycompai/auth', () => ({
18+
statement: {},
19+
BUILT_IN_ROLE_PERMISSIONS: {},
20+
}));
21+
1122
describe('KnowledgeBaseController', () => {
1223
let controller: KnowledgeBaseController;
1324
let service: jest.Mocked<KnowledgeBaseService>;
@@ -89,10 +100,16 @@ describe('KnowledgeBaseController', () => {
89100
});
90101
});
91102

103+
// These handlers scope to the caller's active organization from the auth
104+
// context. Each test passes one org in the body and a different authenticated
105+
// org, and asserts the authenticated org is what reaches the service.
106+
const AUTH_ORG = 'org_authenticated';
107+
const OTHER_ORG = 'org_supplied_in_body';
108+
92109
describe('uploadDocument', () => {
93-
it('should delegate to service', async () => {
110+
it('uses the authenticated organization, not the body organizationId', async () => {
94111
const dto = {
95-
organizationId: 'org_1',
112+
organizationId: OTHER_ORG,
96113
fileName: 'doc.pdf',
97114
fileType: 'application/pdf',
98115
fileData: 'base64',
@@ -103,50 +120,73 @@ describe('KnowledgeBaseController', () => {
103120
s3Key: 'key',
104121
});
105122

106-
const result = await controller.uploadDocument(dto as any);
123+
const result = await controller.uploadDocument(AUTH_ORG, dto as any);
107124

108125
expect(result.id).toBe('d1');
109-
expect(service.uploadDocument).toHaveBeenCalledWith(dto);
126+
expect(service.uploadDocument).toHaveBeenCalledWith({
127+
...dto,
128+
organizationId: AUTH_ORG,
129+
});
110130
});
111131
});
112132

113133
describe('getDownloadUrl', () => {
114-
it('should merge documentId param with dto', async () => {
115-
const dto = { organizationId: 'org_1' };
134+
it('scopes to the authenticated organization and merges documentId', async () => {
135+
const dto = { organizationId: OTHER_ORG };
116136
mockService.getDownloadUrl.mockResolvedValue({
117137
signedUrl: 'https://example.com/signed',
118138
fileName: 'doc.pdf',
119139
});
120140

121-
const result = await controller.getDownloadUrl('d1', dto as any);
141+
const result = await controller.getDownloadUrl('d1', AUTH_ORG, dto as any);
122142

123143
expect(result.signedUrl).toBe('https://example.com/signed');
124144
expect(service.getDownloadUrl).toHaveBeenCalledWith({
125-
...dto,
126145
documentId: 'd1',
146+
organizationId: AUTH_ORG,
147+
});
148+
});
149+
});
150+
151+
describe('getViewUrl', () => {
152+
it('scopes to the authenticated organization and merges documentId', async () => {
153+
const dto = { organizationId: OTHER_ORG };
154+
mockService.getViewUrl.mockResolvedValue({
155+
signedUrl: 'https://example.com/view',
156+
fileName: 'doc.pdf',
157+
fileType: 'application/pdf',
158+
viewableInBrowser: true,
159+
});
160+
161+
const result = await controller.getViewUrl('d1', AUTH_ORG, dto as any);
162+
163+
expect(result.signedUrl).toBe('https://example.com/view');
164+
expect(service.getViewUrl).toHaveBeenCalledWith({
165+
documentId: 'd1',
166+
organizationId: AUTH_ORG,
127167
});
128168
});
129169
});
130170

131171
describe('deleteDocument', () => {
132-
it('should merge documentId param with dto', async () => {
133-
const dto = { organizationId: 'org_1' };
172+
it('scopes to the authenticated organization and merges documentId', async () => {
173+
const dto = { organizationId: OTHER_ORG };
134174
mockService.deleteDocument.mockResolvedValue({ success: true });
135175

136-
const result = await controller.deleteDocument('d1', dto as any);
176+
const result = await controller.deleteDocument('d1', AUTH_ORG, dto as any);
137177

138178
expect(result).toEqual({ success: true });
139179
expect(service.deleteDocument).toHaveBeenCalledWith({
140-
...dto,
141180
documentId: 'd1',
181+
organizationId: AUTH_ORG,
142182
});
143183
});
144184
});
145185

146186
describe('processDocuments', () => {
147-
it('should delegate to service', async () => {
187+
it('uses the authenticated organization, not the body organizationId', async () => {
148188
const dto = {
149-
organizationId: 'org_1',
189+
organizationId: OTHER_ORG,
150190
documentIds: ['d1', 'd2'],
151191
};
152192
mockService.processDocuments.mockResolvedValue({
@@ -155,56 +195,49 @@ describe('KnowledgeBaseController', () => {
155195
message: 'Processing 2 documents in parallel...',
156196
});
157197

158-
const result = await controller.processDocuments(dto as any);
198+
const result = await controller.processDocuments(AUTH_ORG, dto as any);
159199

160200
expect(result.success).toBe(true);
161-
expect(service.processDocuments).toHaveBeenCalledWith(dto);
162-
});
163-
});
164-
165-
describe('createRunToken', () => {
166-
it('should return token when created', async () => {
167-
mockService.createRunReadToken.mockResolvedValue('token_123');
168-
169-
const result = await controller.createRunToken('run_1');
170-
171-
expect(result).toEqual({ success: true, token: 'token_123' });
172-
expect(service.createRunReadToken).toHaveBeenCalledWith('run_1');
173-
});
174-
175-
it('should return success false when token creation fails', async () => {
176-
mockService.createRunReadToken.mockResolvedValue(undefined);
177-
178-
const result = await controller.createRunToken('run_1');
179-
180-
expect(result).toEqual({ success: false, token: undefined });
201+
expect(service.processDocuments).toHaveBeenCalledWith({
202+
...dto,
203+
organizationId: AUTH_ORG,
204+
});
181205
});
182206
});
183207

184208
describe('deleteManualAnswer', () => {
185-
it('should merge manualAnswerId param with dto', async () => {
186-
const dto = { organizationId: 'org_1' };
209+
it('scopes to the authenticated organization and merges manualAnswerId', async () => {
210+
const dto = { organizationId: OTHER_ORG };
187211
mockService.deleteManualAnswer.mockResolvedValue({ success: true });
188212

189-
const result = await controller.deleteManualAnswer('ma1', dto as any);
213+
const result = await controller.deleteManualAnswer(
214+
'ma1',
215+
AUTH_ORG,
216+
dto as any,
217+
);
190218

191219
expect(result).toEqual({ success: true });
192220
expect(service.deleteManualAnswer).toHaveBeenCalledWith({
193-
...dto,
194221
manualAnswerId: 'ma1',
222+
organizationId: AUTH_ORG,
195223
});
196224
});
197225
});
198226

199227
describe('deleteAllManualAnswers', () => {
200-
it('should delegate to service', async () => {
201-
const dto = { organizationId: 'org_1' };
228+
it('uses the authenticated organization, not the body organizationId', async () => {
229+
const dto = { organizationId: OTHER_ORG };
202230
mockService.deleteAllManualAnswers.mockResolvedValue({ success: true });
203231

204-
const result = await controller.deleteAllManualAnswers(dto as any);
232+
const result = await controller.deleteAllManualAnswers(
233+
AUTH_ORG,
234+
dto as any,
235+
);
205236

206237
expect(result).toEqual({ success: true });
207-
expect(service.deleteAllManualAnswers).toHaveBeenCalledWith(dto);
238+
expect(service.deleteAllManualAnswers).toHaveBeenCalledWith({
239+
organizationId: AUTH_ORG,
240+
});
208241
});
209242
});
210243
});

‎apps/api/src/knowledge-base/knowledge-base.controller.ts‎

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,19 @@ export class KnowledgeBaseController {
7070
});
7171
}
7272

73+
// All handlers scope operations to the caller's active organization, resolved
74+
// from the authenticated request context via @OrganizationId.
75+
7376
@Post('documents/upload')
7477
@RequirePermission('questionnaire', 'create')
7578
@ApiOperation({ summary: 'Upload a knowledge base document' })
7679
@ApiConsumes('application/json')
7780
@ApiOkResponse({ description: 'Document uploaded successfully' })
78-
async uploadDocument(@Body() dto: UploadDocumentDto) {
79-
return this.knowledgeBaseService.uploadDocument(dto);
81+
async uploadDocument(
82+
@OrganizationId() organizationId: string,
83+
@Body() dto: UploadDocumentDto,
84+
) {
85+
return this.knowledgeBaseService.uploadDocument({ ...dto, organizationId });
8086
}
8187

8288
@Post('documents/:documentId/download')
@@ -87,11 +93,13 @@ export class KnowledgeBaseController {
8793
@ApiOkResponse({ description: 'Signed download URL generated' })
8894
async getDownloadUrl(
8995
@Param('documentId') documentId: string,
96+
@OrganizationId() organizationId: string,
9097
@Body() dto: Omit<GetDocumentUrlDto, 'documentId'>,
9198
) {
9299
return this.knowledgeBaseService.getDownloadUrl({
93100
...dto,
94101
documentId,
102+
organizationId,
95103
});
96104
}
97105

@@ -103,11 +111,13 @@ export class KnowledgeBaseController {
103111
@ApiOkResponse({ description: 'Signed view URL generated' })
104112
async getViewUrl(
105113
@Param('documentId') documentId: string,
114+
@OrganizationId() organizationId: string,
106115
@Body() dto: Omit<GetDocumentUrlDto, 'documentId'>,
107116
) {
108117
return this.knowledgeBaseService.getViewUrl({
109118
...dto,
110119
documentId,
120+
organizationId,
111121
});
112122
}
113123

@@ -119,11 +129,13 @@ export class KnowledgeBaseController {
119129
@ApiOkResponse({ description: 'Document deleted successfully' })
120130
async deleteDocument(
121131
@Param('documentId') documentId: string,
132+
@OrganizationId() organizationId: string,
122133
@Body() dto: Omit<DeleteDocumentDto, 'documentId'>,
123134
) {
124135
return this.knowledgeBaseService.deleteDocument({
125136
...dto,
126137
documentId,
138+
organizationId,
127139
});
128140
}
129141

@@ -132,17 +144,14 @@ export class KnowledgeBaseController {
132144
@ApiOperation({ summary: 'Trigger processing of knowledge base documents' })
133145
@ApiConsumes('application/json')
134146
@ApiOkResponse({ description: 'Document processing triggered' })
135-
async processDocuments(@Body() dto: ProcessDocumentsDto) {
136-
return this.knowledgeBaseService.processDocuments(dto);
137-
}
138-
139-
@Post('runs/:runId/token')
140-
@RequirePermission('questionnaire', 'read')
141-
@ApiOperation({ summary: 'Create a public access token for a run' })
142-
@ApiOkResponse({ description: 'Public access token created' })
143-
async createRunToken(@Param('runId') runId: string) {
144-
const token = await this.knowledgeBaseService.createRunReadToken(runId);
145-
return { success: !!token, token };
147+
async processDocuments(
148+
@OrganizationId() organizationId: string,
149+
@Body() dto: ProcessDocumentsDto,
150+
) {
151+
return this.knowledgeBaseService.processDocuments({
152+
...dto,
153+
organizationId,
154+
});
146155
}
147156

148157
@Post('manual-answers/:manualAnswerId/delete')
@@ -153,11 +162,13 @@ export class KnowledgeBaseController {
153162
@ApiOkResponse({ description: 'Manual answer deleted' })
154163
async deleteManualAnswer(
155164
@Param('manualAnswerId') manualAnswerId: string,
165+
@OrganizationId() organizationId: string,
156166
@Body() dto: DeleteManualAnswerDto,
157167
) {
158168
return this.knowledgeBaseService.deleteManualAnswer({
159169
...dto,
160170
manualAnswerId,
171+
organizationId,
161172
});
162173
}
163174

@@ -167,7 +178,13 @@ export class KnowledgeBaseController {
167178
@ApiOperation({ summary: 'Delete all manual answers for an organization' })
168179
@ApiConsumes('application/json')
169180
@ApiOkResponse({ description: 'All manual answers deleted' })
170-
async deleteAllManualAnswers(@Body() dto: DeleteAllManualAnswersDto) {
171-
return this.knowledgeBaseService.deleteAllManualAnswers(dto);
181+
async deleteAllManualAnswers(
182+
@OrganizationId() organizationId: string,
183+
@Body() dto: DeleteAllManualAnswersDto,
184+
) {
185+
return this.knowledgeBaseService.deleteAllManualAnswers({
186+
...dto,
187+
organizationId,
188+
});
172189
}
173190
}

0 commit comments

Comments
 (0)