Skip to content

Commit

Permalink
Ericbrehault/sc 9116/secure access to the sync agent endpoints (#28)
Browse files Browse the repository at this point in the history
* secure get folders, update and delete

* secure how we get sync parameters

* no auth check if local

* fix unit tests
  • Loading branch information
ebrehault authored Mar 14, 2024
1 parent b9dfaef commit b5b926e
Show file tree
Hide file tree
Showing 8 changed files with 75 additions and 28 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "nuclia-sync-agent-app",
"version": "1.1.0",
"version": "1.2.0",
"description": "\"This is a Nuclia Sync Agent App\"",
"main": "build/index.js",
"scripts": {
Expand Down
4 changes: 4 additions & 0 deletions server/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# 1.2.0 (2024-03-13)

- Secure endpoints by validating the dashboard user token

# 1.1.0 (2024-03-11)

- Support filtering options
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ const props: any = {
},
kb: {
knowledgeBox: 'knowledgeBox',
zone: 'zone',
backend: 'backend',
apiKey: 'apiKey',
},
Expand Down Expand Up @@ -93,7 +92,6 @@ describe('Create Sync dto tests', () => {
id: undefined,
kb: {
knowledgeBox: 'knowledgeBox',
zone: '',
},
});

Expand All @@ -105,7 +103,6 @@ describe('Create Sync dto tests', () => {
id: undefined,
kb: {
knowledgeBox: 'knowledgeBox',
zone: 'zone',
},
});

Expand Down
24 changes: 23 additions & 1 deletion server/src/logic/sync/domain/sync.entity.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { catchError, forkJoin, map, Observable, of } from 'rxjs';
import { catchError, firstValueFrom, forkJoin, map, Observable, of } from 'rxjs';

import { z } from 'zod';
import { FileStatus, IConnector, SearchResults, SyncItem } from '../../connector/domain/connector';
import { getConnector } from '../../connector/infrastructure/factory';
import { Nuclia } from '@nuclia/core';
import { CustomError } from '../../errors';

export type Connector = {
name: 'gdrive' | 'folder';
Expand Down Expand Up @@ -162,4 +164,24 @@ export class SyncEntity {
hasAuthData() {
return this.sourceConnector!.hasAuthData();
}

async checkNucliaAuth(token: string) {
// If there is no zone, it is a local NucliaDB or a unit test, we do not check the auth
if (!this.kb.zone) {
return Promise.resolve(true);
}
try {
const nuclia = new Nuclia({ ...this.kb, apiKey: '' });
nuclia.auth.authenticate({ access_token: token, refresh_token: '' });
const req = await firstValueFrom(
nuclia.knowledgeBox.getConfiguration().pipe(
map(() => true),
catchError(() => of(false)),
),
);
return req;
} catch (err) {
return new CustomError('Error checking Nuclia auth', 500);
}
}
}
33 changes: 30 additions & 3 deletions server/src/logic/sync/presentation/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { SyncAllFolders } from '../domain/use-cases/sync-all-folders-data.use-ca
import { UpdateSync } from '../domain/use-cases/update-sync.use-case';
import { FileSystemSyncDatasource } from '../infrastructure/file-system.sync.datasource';
import { SyncRepository } from '../infrastructure/sync.repository';
import { SyncEntity } from '../domain/sync.entity';

export class SyncFileSystemRoutes {
private readonly basePath: string;
Expand Down Expand Up @@ -43,10 +44,17 @@ export class SyncFileSystemRoutes {
next();
});

router.get('/', async (_req, res) => {
router.get('/kb/:kb', async (_req, res) => {
try {
const data = await new GetAllSync(syncRepository).execute();
res.status(200).send(data);
const allSyncs = await new GetAllSync(syncRepository).execute();
const kbSyncs = Object.values(allSyncs)
.filter((sync) => sync.kb.knowledgeBox === _req.params.kb)
.map((sync) => ({
id: sync.id,
title: sync.title,
connector: sync.connector.name,
}));
res.status(200).send(kbSyncs);
} catch (error) {
this.handleError(res, error);
}
Expand Down Expand Up @@ -77,6 +85,7 @@ export class SyncFileSystemRoutes {
router.get('/:id', async (req, res) => {
const { id } = req.params;
try {
await this.checkAuth(id, req.headers.token as string, syncRepository);
const data = await new GetSync(syncRepository).execute(id);
res.status(200).send(data);
} catch (error) {
Expand All @@ -97,6 +106,7 @@ export class SyncFileSystemRoutes {
router.get('/:id/folders', async (req, res) => {
const { id } = req.params;
try {
await this.checkAuth(id, req.headers.token as string, syncRepository);
const data = await new GetSyncFolders(syncRepository).execute(id);
res.status(200).send(data);
} catch (error) {
Expand All @@ -110,6 +120,7 @@ export class SyncFileSystemRoutes {
if (error) return res.status(400).json({ message: error });

try {
await this.checkAuth(id, req.headers.token as string, syncRepository);
await new UpdateSync(syncRepository).execute(updateSyncDto!);
res.status(204).send(null);
} catch (error) {
Expand All @@ -120,6 +131,7 @@ export class SyncFileSystemRoutes {
router.delete('/:id', async (req, res) => {
const { id } = req.params;
try {
await this.checkAuth(id, req.headers.token as string, syncRepository);
await new DeleteSync(syncRepository).execute(id);
res.status(200).send(null);
} catch (error) {
Expand All @@ -129,4 +141,19 @@ export class SyncFileSystemRoutes {

return router;
}

private async checkAuth(id: string, auth: string, syncRepository: SyncRepository) {
if (!auth) {
throw new CustomError('Check auth: No auth token provided', 401);
}
const data = await syncRepository.getSync(id);
if (data === null) {
throw new CustomError(`Check auth: Sync with id ${id} not found`, 404);
}
const syncEntity = new SyncEntity(data);
const checkAuth = await syncEntity.checkNucliaAuth(auth);
if (!checkAuth) {
throw new CustomError(`Check auth: Auth for sync with id ${id} not valid`, 401);
}
}
}
3 changes: 0 additions & 3 deletions server/tests/fixtures.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ export const serverTest = test.extend({
},
kb: {
knowledgeBox: 'test',
zone: 'local',
backend: 'http://localhost:8000',
apiKey: 'apiKey',
},
Expand All @@ -56,7 +55,6 @@ export const serverTest = test.extend({
},
kb: {
knowledgeBox: 'test',
zone: 'local',
backend: 'http://localhost:8000',
apiKey: 'apiKey',
},
Expand All @@ -75,7 +73,6 @@ export const serverTest = test.extend({
},
kb: {
knowledgeBox: 'test',
zone: 'local',
backend: 'http://localhost:8000',
apiKey: 'apiKey',
},
Expand Down
8 changes: 4 additions & 4 deletions server/tests/sever.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ describe('Basic server', () => {
});

serverTest('Get sync - empty folder', async ({ server }) => {
const response = await request(server.app).get('/sync');
const response = await request(server.app).get('/sync/kb/test');
expect(response.status).toBe(200);
expect(Object.keys(response.body).length).toEqual(0);
expect(response.body.length).toEqual(0);
});

serverTest('Get empty logs', async ({ server }) => {
Expand All @@ -21,8 +21,8 @@ describe('Basic server', () => {
});

serverTest('Get connectors', async ({ server }) => {
const response = await request(server.app).get('/sync');
const response = await request(server.app).get('/sync/kb/test');
expect(response.status).toBe(200);
expect(Object.keys(response.body).length).toEqual(0);
expect(response.body.length).toEqual(0);
});
});
26 changes: 13 additions & 13 deletions server/tests/sync.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,19 +36,20 @@ describe('Test Sync object', () => {
});

serverTest('Add new sync', async ({ serverWithSync }) => {
let response = await request(serverWithSync.app).get('/sync');
let response = await request(serverWithSync.app).get('/sync/kb/test');
expect(response.status).toBe(200);
expect(Object.keys(response.body).length).toEqual(1);
expect(response.body.length).toEqual(1);
});

serverTest('Update a sync', async ({ serverWithSync }) => {
const response = await request(serverWithSync.app).get('/sync');
const response = await request(serverWithSync.app).get('/sync/kb/test');
expect(response.status).toBe(200);
expect(Object.keys(response.body).length).toEqual(1);
expect(response.body.length).toEqual(1);

const id = Object.keys(response.body)[0];
const id = response.body[0].id;
const responsePatch = await request(serverWithSync.app)
.patch(`/sync/${id}`)
.set('token', 'fake-token')
.send({
title: 'Sync1',
kb: {
Expand All @@ -57,12 +58,11 @@ describe('Test Sync object', () => {
});
expect(responsePatch.status).toBe(204);

const responseGet = await request(serverWithSync.app).get(`/sync/${id}`);
const responseGet = await request(serverWithSync.app).get(`/sync/${id}`).set('token', 'fake-token');
expect(responseGet.status).toBe(200);
expect(responseGet.body['title']).toBe('Sync1');
expect(responseGet.body['kb']).toEqual({
knowledgeBox: 'test',
zone: 'local',
backend: 'http://localhost:9000',
apiKey: 'apiKey',
});
Expand All @@ -81,16 +81,16 @@ describe('Test Sync object', () => {
});

serverTest('Delete a sync', async ({ serverWithSync }) => {
let response = await request(serverWithSync.app).get('/sync');
let response = await request(serverWithSync.app).get('/sync/kb/test');
expect(response.status).toBe(200);
expect(Object.keys(response.body).length).toEqual(3);
expect(response.body.length).toEqual(3);

const id = Object.keys(response.body)[0];
const responseDelete = await request(serverWithSync.app).delete(`/sync/${id}`);
const id = response.body[0].id;
const responseDelete = await request(serverWithSync.app).delete(`/sync/${id}`).set('token', 'fake-token');
expect(responseDelete.status).toBe(200);

response = await request(serverWithSync.app).get('/sync');
response = await request(serverWithSync.app).get('/sync/kb/test');
expect(response.status).toBe(200);
expect(Object.keys(response.body).length).toEqual(2);
expect(response.body.length).toEqual(2);
});
});

0 comments on commit b5b926e

Please sign in to comment.