From cb2357f589ee2ab8158b421d76be64e4ecf277ab Mon Sep 17 00:00:00 2001 From: Sebastian Leidig Date: Tue, 19 Mar 2024 11:44:35 +0100 Subject: [PATCH 1/4] fix: automatically trigger clear_local when permissions changed see #7 --- src/admin/admin.controller.spec.ts | 29 ++++++++++++ src/admin/admin.controller.ts | 30 ++++++++++++ src/admin/admin.module.ts | 4 +- ...ntroller.spec.ts => admin.service.spec.ts} | 19 ++++---- src/admin/admin.service.ts | 27 +++++++++++ src/admin/admin/admin.controller.ts | 46 ------------------- src/permissions/rules/rules.service.spec.ts | 46 +++++++++++++++++++ src/permissions/rules/rules.service.ts | 14 +++++- 8 files changed, 155 insertions(+), 60 deletions(-) create mode 100644 src/admin/admin.controller.spec.ts create mode 100644 src/admin/admin.controller.ts rename src/admin/{admin/admin.controller.spec.ts => admin.service.spec.ts} (68%) create mode 100644 src/admin/admin.service.ts delete mode 100644 src/admin/admin/admin.controller.ts diff --git a/src/admin/admin.controller.spec.ts b/src/admin/admin.controller.spec.ts new file mode 100644 index 0000000..9dc4c32 --- /dev/null +++ b/src/admin/admin.controller.spec.ts @@ -0,0 +1,29 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { AdminController } from './admin.controller'; +import { CouchdbService } from '../couchdb/couchdb.service'; +import { authGuardMockProviders } from '../auth/auth-guard-mock.providers'; +import { AdminService } from './admin.service'; + +describe('AdminController', () => { + let controller: AdminController; + let mockAdminService: CouchdbService; + + beforeEach(async () => { + mockAdminService = { + clearLocal: () => Promise.resolve(), + } as any; + const module: TestingModule = await Test.createTestingModule({ + controllers: [AdminController], + providers: [ + ...authGuardMockProviders, + { provide: AdminService, useValue: mockAdminService }, + ], + }).compile(); + + controller = module.get(AdminController); + }); + + it('should be defined', () => { + expect(controller).toBeDefined(); + }); +}); diff --git a/src/admin/admin.controller.ts b/src/admin/admin.controller.ts new file mode 100644 index 0000000..15b54cc --- /dev/null +++ b/src/admin/admin.controller.ts @@ -0,0 +1,30 @@ +import { Controller, Param, Post, UseGuards } from '@nestjs/common'; +import { CombinedAuthGuard } from '../auth/guards/combined-auth/combined-auth.guard'; +import { OnlyAuthenticated } from '../auth/only-authenticated.decorator'; +import { AdminService } from './admin.service'; + +/** + * This service provides some general administrativ endpoints. + */ +@OnlyAuthenticated() +@UseGuards(CombinedAuthGuard) +@Controller('admin') +export class AdminController { + constructor(private adminService: AdminService) {} + + /** + * Deletes all local documents of the remote database. + * These document hold meta-information about the replication process. + * Deleting them forces clients to re-run sync and check which documents are different. + * See {@link https://docs.couchdb.org/en/stable/replication/protocol.html#retrieve-replication-logs-from-source-and-target} + * + * @param db name of the database where the local documents should be deleted from + * + * This function should be called whenever the permissions change to re-trigger sync + */ + @Post('/clear_local/:db') + async clearLocal(@Param('db') db: string): Promise { + await this.adminService.clearLocal(db); + return true; + } +} diff --git a/src/admin/admin.module.ts b/src/admin/admin.module.ts index b7c2dfb..550de6d 100644 --- a/src/admin/admin.module.ts +++ b/src/admin/admin.module.ts @@ -1,11 +1,13 @@ import { Module } from '@nestjs/common'; -import { AdminController } from './admin/admin.controller'; +import { AdminController } from './admin.controller'; import { PermissionModule } from '../permissions/permission.module'; import { CouchdbModule } from '../couchdb/couchdb.module'; import { AuthModule } from '../auth/auth.module'; +import { AdminService } from './admin.service'; @Module({ controllers: [AdminController], imports: [PermissionModule, CouchdbModule, AuthModule], + providers: [AdminService], }) export class AdminModule {} diff --git a/src/admin/admin/admin.controller.spec.ts b/src/admin/admin.service.spec.ts similarity index 68% rename from src/admin/admin/admin.controller.spec.ts rename to src/admin/admin.service.spec.ts index da60afd..fac3da5 100644 --- a/src/admin/admin/admin.controller.spec.ts +++ b/src/admin/admin.service.spec.ts @@ -1,11 +1,10 @@ import { Test, TestingModule } from '@nestjs/testing'; -import { AdminController } from './admin.controller'; +import { AdminService } from './admin.service'; +import { CouchdbService } from '../couchdb/couchdb.service'; import { of } from 'rxjs'; -import { CouchdbService } from '../../couchdb/couchdb.service'; -import { authGuardMockProviders } from '../../auth/auth-guard-mock.providers'; -describe('AdminController', () => { - let controller: AdminController; +describe('AdminService', () => { + let service: AdminService; let mockCouchDBService: CouchdbService; beforeEach(async () => { @@ -14,18 +13,17 @@ describe('AdminController', () => { delete: () => of({}), } as any; const module: TestingModule = await Test.createTestingModule({ - controllers: [AdminController], providers: [ - ...authGuardMockProviders, + AdminService, { provide: CouchdbService, useValue: mockCouchDBService }, ], }).compile(); - controller = module.get(AdminController); + service = module.get(AdminService); }); it('should be defined', () => { - expect(controller).toBeDefined(); + expect(service).toBeDefined(); }); it('should delete all docs in the _local db', async () => { @@ -42,12 +40,11 @@ describe('AdminController', () => { jest.spyOn(mockCouchDBService, 'delete').mockReturnValue(of(undefined)); const dbName = 'app'; - const result = await controller.clearLocal(dbName); + await service.clearLocal(dbName); expect(mockCouchDBService.get).toHaveBeenCalledWith(dbName, '_local_docs'); mockAllDocsResponse.rows.forEach((row) => { expect(mockCouchDBService.delete).toHaveBeenCalledWith(dbName, row.id); }); - expect(result).toBe(true); }); }); diff --git a/src/admin/admin.service.ts b/src/admin/admin.service.ts new file mode 100644 index 0000000..95532f7 --- /dev/null +++ b/src/admin/admin.service.ts @@ -0,0 +1,27 @@ +import { Injectable } from '@nestjs/common'; +import { firstValueFrom } from 'rxjs'; +import { AllDocsResponse } from '../restricted-endpoints/replication/bulk-document/couchdb-dtos/all-docs.dto'; +import { CouchdbService } from '../couchdb/couchdb.service'; + +@Injectable() +export class AdminService { + constructor(private couchdbService: CouchdbService) {} + + async clearLocal(db: string) { + const localDocsResponse = await firstValueFrom( + this.couchdbService.get(db, '_local_docs'), + ); + + // Get IDs of the replication checkpoints + const ids = localDocsResponse.rows + .map((doc) => doc.id) + .filter( + (id) => !id.includes('purge-mrview') && !id.includes('shard-sync'), + ); + const deletePromises = ids.map((id) => + firstValueFrom(this.couchdbService.delete(db, id)), + ); + + await Promise.all(deletePromises); + } +} diff --git a/src/admin/admin/admin.controller.ts b/src/admin/admin/admin.controller.ts deleted file mode 100644 index 8e92a17..0000000 --- a/src/admin/admin/admin.controller.ts +++ /dev/null @@ -1,46 +0,0 @@ -import { Controller, Param, Post, UseGuards } from '@nestjs/common'; -import { firstValueFrom } from 'rxjs'; -import { AllDocsResponse } from '../../restricted-endpoints/replication/bulk-document/couchdb-dtos/all-docs.dto'; -import { CouchdbService } from '../../couchdb/couchdb.service'; -import { CombinedAuthGuard } from '../../auth/guards/combined-auth/combined-auth.guard'; -import { OnlyAuthenticated } from '../../auth/only-authenticated.decorator'; - -/** - * This service provides some general administrativ endpoints. - */ -@OnlyAuthenticated() -@UseGuards(CombinedAuthGuard) -@Controller('admin') -export class AdminController { - constructor(private couchdbService: CouchdbService) {} - - /** - * Deletes all local documents of the remote database. - * These document hold meta-information about the replication process. - * Deleting them forces clients to re-run sync and check which documents are different. - * See {@link https://docs.couchdb.org/en/stable/replication/protocol.html#retrieve-replication-logs-from-source-and-target} - * - * @param db name of the database where the local documents should be deleted from - * - * This function should be called whenever the permissions change to re-trigger sync - */ - @Post('/clear_local/:db') - async clearLocal(@Param('db') db: string): Promise { - const localDocsResponse = await firstValueFrom( - this.couchdbService.get(db, '_local_docs'), - ); - - // Get IDs of the replication checkpoints - const ids = localDocsResponse.rows - .map((doc) => doc.id) - .filter( - (id) => !id.includes('purge-mrview') && !id.includes('shard-sync'), - ); - const deletePromises = ids.map((id) => - firstValueFrom(this.couchdbService.delete(db, id)), - ); - - await Promise.all(deletePromises); - return true; - } -} diff --git a/src/permissions/rules/rules.service.spec.ts b/src/permissions/rules/rules.service.spec.ts index 9d97eb2..e1924cf 100644 --- a/src/permissions/rules/rules.service.spec.ts +++ b/src/permissions/rules/rules.service.spec.ts @@ -6,14 +6,18 @@ import { Permission } from './permission'; import { ConfigService } from '@nestjs/config'; import { CouchdbService } from '../../couchdb/couchdb.service'; import { ChangesResponse } from '../../restricted-endpoints/replication/bulk-document/couchdb-dtos/changes.dto'; +import { AdminService } from '../../admin/admin.service'; describe('RulesService', () => { let service: RulesService; let adminRules: DocumentRule[]; let userRules: DocumentRule[]; let mockCouchDBService: CouchdbService; + let mockAdminService: AdminService; + let testPermission: Permission; let changesResponse: ChangesResponse; + const normalUser = new UserInfo('normalUser', ['user_app']); const adminUser = new UserInfo('superUser', ['user_app', 'admin_app']); const DATABASE_NAME = 'app'; @@ -28,6 +32,7 @@ describe('RulesService', () => { }); userRules = testPermission.data[normalUser.roles[0]]; adminRules = testPermission.data[adminUser.roles[1]]; + changesResponse = { last_seq: 'initial_seq', results: [ @@ -43,6 +48,10 @@ describe('RulesService', () => { .mockReturnValueOnce(of(changesResponse)) .mockReturnValueOnce(NEVER); + mockAdminService = { + clearLocal: jest.fn().mockResolvedValue(undefined), + } as any; + const module = await Test.createTestingModule({ providers: [ RulesService, @@ -53,6 +62,7 @@ describe('RulesService', () => { }), }, { provide: CouchdbService, useValue: mockCouchDBService }, + { provide: AdminService, useValue: mockAdminService }, ], }).compile(); @@ -178,4 +188,40 @@ describe('RulesService', () => { expect(result).toEqual([publicRule]); expect(result).not.toContain(testPermission.data.default); }); + + it('should update rules and call clear_local when permission doc changed', () => { + jest.useFakeTimers(); + + const updatedPermission = new Permission({ + user_app: [{ action: 'manage', subject: 'all' }], + }); + const updatedPermissionChange = { + last_seq: '1', + results: [ + { + doc: updatedPermission, + seq: '1', + changes: [], + id: updatedPermission._id, + }, + ], + pending: 0, + }; + + jest + .spyOn(mockCouchDBService, 'get') + .mockReturnValueOnce(of(changesResponse)) + .mockReturnValueOnce(of(updatedPermissionChange)) + .mockReturnValue(NEVER); + + service.loadRulesContinuously('app'); + jest.advanceTimersByTime(1500); + + expect(service.getRulesForUser(normalUser)).toEqual([ + { action: 'manage', subject: 'all' }, + ]); + expect(mockAdminService.clearLocal).toHaveBeenCalled(); + + jest.useRealTimers(); + }); }); diff --git a/src/permissions/rules/rules.service.ts b/src/permissions/rules/rules.service.ts index bf201ee..f8af8e8 100644 --- a/src/permissions/rules/rules.service.ts +++ b/src/permissions/rules/rules.service.ts @@ -8,6 +8,7 @@ import { CouchdbService } from '../../couchdb/couchdb.service'; import { ConfigService } from '@nestjs/config'; import { ChangesResponse } from '../../restricted-endpoints/replication/bulk-document/couchdb-dtos/changes.dto'; import { get } from 'lodash'; +import { AdminService } from '../../admin/admin.service'; export type DocumentRule = RawRuleOf; @@ -24,6 +25,7 @@ export class RulesService { constructor( private couchdbService: CouchdbService, private configService: ConfigService, + private adminService: AdminService, ) { const permissionDbName = this.configService.get( RulesService.ENV_PERMISSION_DB, @@ -60,8 +62,16 @@ export class RulesService { ) .subscribe((changes) => { this.lastSeq = changes.last_seq; - if (changes.results.length > 0) { - this.permission = changes.results[0].doc.data; + if (changes.results?.length > 0) { + const newPermissions = changes.results[0].doc.data; + this.permission = newPermissions; + + if ( + this.permission !== undefined && // do not clear upon restart of the API + JSON.stringify(this.permission) !== JSON.stringify(newPermissions) + ) { + this.adminService.clearLocal(db); + } } }); } From 43284efb205b83c7e4a728a9b03d9ac846244fba Mon Sep 17 00:00:00 2001 From: Sebastian Date: Tue, 19 Mar 2024 11:47:00 +0100 Subject: [PATCH 2/4] Update src/admin/admin.controller.ts --- src/admin/admin.controller.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/admin/admin.controller.ts b/src/admin/admin.controller.ts index 15b54cc..cd1a030 100644 --- a/src/admin/admin.controller.ts +++ b/src/admin/admin.controller.ts @@ -4,7 +4,7 @@ import { OnlyAuthenticated } from '../auth/only-authenticated.decorator'; import { AdminService } from './admin.service'; /** - * This service provides some general administrativ endpoints. + * This controller provides some general administrative endpoints. */ @OnlyAuthenticated() @UseGuards(CombinedAuthGuard) From 9a376c3ecc5d7fa39c99ea2d2c7c5262d16dfd0b Mon Sep 17 00:00:00 2001 From: Sebastian Leidig Date: Tue, 19 Mar 2024 11:48:45 +0100 Subject: [PATCH 3/4] updated code docs --- src/admin/admin.controller.ts | 10 ---------- src/admin/admin.service.ts | 13 +++++++++++++ 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/admin/admin.controller.ts b/src/admin/admin.controller.ts index cd1a030..c71f709 100644 --- a/src/admin/admin.controller.ts +++ b/src/admin/admin.controller.ts @@ -12,16 +12,6 @@ import { AdminService } from './admin.service'; export class AdminController { constructor(private adminService: AdminService) {} - /** - * Deletes all local documents of the remote database. - * These document hold meta-information about the replication process. - * Deleting them forces clients to re-run sync and check which documents are different. - * See {@link https://docs.couchdb.org/en/stable/replication/protocol.html#retrieve-replication-logs-from-source-and-target} - * - * @param db name of the database where the local documents should be deleted from - * - * This function should be called whenever the permissions change to re-trigger sync - */ @Post('/clear_local/:db') async clearLocal(@Param('db') db: string): Promise { await this.adminService.clearLocal(db); diff --git a/src/admin/admin.service.ts b/src/admin/admin.service.ts index 95532f7..b92e93d 100644 --- a/src/admin/admin.service.ts +++ b/src/admin/admin.service.ts @@ -3,10 +3,23 @@ import { firstValueFrom } from 'rxjs'; import { AllDocsResponse } from '../restricted-endpoints/replication/bulk-document/couchdb-dtos/all-docs.dto'; import { CouchdbService } from '../couchdb/couchdb.service'; +/** + * Service providing some general functionalities that are required in the context of administering the db and permissions. + */ @Injectable() export class AdminService { constructor(private couchdbService: CouchdbService) {} + /** + * Deletes all local documents of the remote database. + * These document hold meta-information about the replication process. + * Deleting them forces clients to re-run sync and check which documents are different. + * See {@link https://docs.couchdb.org/en/stable/replication/protocol.html#retrieve-replication-logs-from-source-and-target} + * + * @param db name of the database where the local documents should be deleted from + * + * This function should be called whenever the permissions change to re-trigger sync + */ async clearLocal(db: string) { const localDocsResponse = await firstValueFrom( this.couchdbService.get(db, '_local_docs'), From f8db36ced987b2a0ae85339a03b6a24dfefafd12 Mon Sep 17 00:00:00 2001 From: Sebastian Leidig Date: Tue, 19 Mar 2024 14:38:58 +0100 Subject: [PATCH 4/4] fix --- src/permissions/rules/rules.service.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/permissions/rules/rules.service.ts b/src/permissions/rules/rules.service.ts index f8af8e8..ba22d9a 100644 --- a/src/permissions/rules/rules.service.ts +++ b/src/permissions/rules/rules.service.ts @@ -63,14 +63,17 @@ export class RulesService { .subscribe((changes) => { this.lastSeq = changes.last_seq; if (changes.results?.length > 0) { + const prevPermissions = this.permission; const newPermissions = changes.results[0].doc.data; + this.permission = newPermissions; if ( - this.permission !== undefined && // do not clear upon restart of the API - JSON.stringify(this.permission) !== JSON.stringify(newPermissions) + prevPermissions !== undefined && // do not clear upon restart of the API + JSON.stringify(prevPermissions) !== JSON.stringify(newPermissions) ) { this.adminService.clearLocal(db); + console.log('Permissions changed - triggered clearLocal:'); } } });