From 99206236c042f45e53c2028db50411408adad016 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Tue, 11 Feb 2025 19:08:41 +0100 Subject: [PATCH] fix: support user.id placeholder for conditions in rules (#171) see https://github.com/Aam-Digital/ndb-core/issues/2847 --- src/auth/cookie/cookie.service.spec.ts | 2 +- .../combined-auth/combined-auth.guard.spec.ts | 4 ++-- .../guards/jwt-bearer/jwt-bearer.strategy.ts | 1 + .../jwt-cookie/jwt-cookie-strategy.service.ts | 1 + .../permission/permission.service.spec.ts | 2 +- src/permissions/rules/rules.service.spec.ts | 16 +++++++++++----- .../attachment/attachment.controller.spec.ts | 16 ++++++++++------ .../document/document.controller.spec.ts | 3 ++- .../bulk-doc-endpoints.controller.spec.ts | 8 ++++---- .../bulk-document/bulk-document.service.spec.ts | 2 +- .../changes/changes.controller.spec.ts | 7 ++++++- .../session/session.controller.spec.ts | 4 ++-- .../session/user-auth.dto.ts | 7 +++++++ 13 files changed, 49 insertions(+), 24 deletions(-) diff --git a/src/auth/cookie/cookie.service.spec.ts b/src/auth/cookie/cookie.service.spec.ts index b067e9d..4184a57 100644 --- a/src/auth/cookie/cookie.service.spec.ts +++ b/src/auth/cookie/cookie.service.spec.ts @@ -26,7 +26,7 @@ describe('CookieService', () => { }); it('should set a cookie containing the user info on the response', () => { - const user = new UserInfo('Username', ['user_app']); + const user = new UserInfo('user-id', 'Username', ['user_app']); const request = { user: user }; const cookies = {}; const setCookieMock = jest.fn( diff --git a/src/auth/guards/combined-auth/combined-auth.guard.spec.ts b/src/auth/guards/combined-auth/combined-auth.guard.spec.ts index 325e4d1..8b9db33 100644 --- a/src/auth/guards/combined-auth/combined-auth.guard.spec.ts +++ b/src/auth/guards/combined-auth/combined-auth.guard.spec.ts @@ -52,7 +52,7 @@ describe('CombinedAuthGuard', () => { it('[MIDDLEWARE/GUARD] should assign user to sentry after a successful authentication', async () => { jest.spyOn(basicAuthGuard, 'canActivate').mockResolvedValue(true); - const user = new UserInfo('testUser', []); + const user = new UserInfo('user-id', 'testUser', []); const setUserSpy = jest.spyOn(Sentry, 'setUser'); await guard.use({ user } as any, undefined, () => undefined); @@ -72,7 +72,7 @@ describe('CombinedAuthGuard', () => { it('[MIDDLEWARE] should call next if authentication passes', async () => { jest.spyOn(jwtCookieGuard, 'canActivate').mockResolvedValue(true); let funCalled = false; - const user = new UserInfo('testUser', []); + const user = new UserInfo('user-id', 'testUser', []); await guard.use({ user } as any, undefined, () => (funCalled = true)); diff --git a/src/auth/guards/jwt-bearer/jwt-bearer.strategy.ts b/src/auth/guards/jwt-bearer/jwt-bearer.strategy.ts index 93dd0be..8a71d0d 100644 --- a/src/auth/guards/jwt-bearer/jwt-bearer.strategy.ts +++ b/src/auth/guards/jwt-bearer/jwt-bearer.strategy.ts @@ -31,6 +31,7 @@ export class JwtBearerStrategy extends PassportStrategy( ).catch(() => {}); return new UserInfo( + data.sub, data.username, data['_couchdb.roles'], user && user['projects'] ? user['projects'] : [], diff --git a/src/auth/guards/jwt-cookie/jwt-cookie-strategy.service.ts b/src/auth/guards/jwt-cookie/jwt-cookie-strategy.service.ts index a5b9e1b..a8a7962 100644 --- a/src/auth/guards/jwt-cookie/jwt-cookie-strategy.service.ts +++ b/src/auth/guards/jwt-cookie/jwt-cookie-strategy.service.ts @@ -33,6 +33,7 @@ export class JwtCookieStrategy extends PassportStrategy( ).catch(() => {}); return new UserInfo( + data.sub, data.name, data.sub, user && user['projects'] ? user['projects'] : [], diff --git a/src/permissions/permission/permission.service.spec.ts b/src/permissions/permission/permission.service.spec.ts index 2f6ed25..b48a682 100644 --- a/src/permissions/permission/permission.service.spec.ts +++ b/src/permissions/permission/permission.service.spec.ts @@ -30,7 +30,7 @@ describe('PermissionService', () => { service = module.get(PermissionService); - normalUser = new UserInfo('normalUser', ['user_app']); + normalUser = new UserInfo('user-id', 'normalUser', ['user_app']); }); it('should be defined', () => { diff --git a/src/permissions/rules/rules.service.spec.ts b/src/permissions/rules/rules.service.spec.ts index e1924cf..dee8614 100644 --- a/src/permissions/rules/rules.service.spec.ts +++ b/src/permissions/rules/rules.service.spec.ts @@ -18,8 +18,11 @@ describe('RulesService', () => { let testPermission: Permission; let changesResponse: ChangesResponse; - const normalUser = new UserInfo('normalUser', ['user_app']); - const adminUser = new UserInfo('superUser', ['user_app', 'admin_app']); + const normalUser = new UserInfo('user-normal', 'normalUser', ['user_app']); + const adminUser = new UserInfo('user-super', 'superUser', [ + 'user_app', + 'admin_app', + ]); const DATABASE_NAME = 'app'; beforeEach(async () => { @@ -131,7 +134,10 @@ describe('RulesService', () => { it('should not fail if no rules exist for a given role', () => { const result = service.getRulesForUser( - new UserInfo('specialUser', ['user_app', 'non_existing_role']), + new UserInfo('user-special', 'specialUser', [ + 'user_app', + 'non_existing_role', + ]), ); expect(result).toEqual(userRules); }); @@ -152,7 +158,7 @@ describe('RulesService', () => { { subject: 'User', action: 'read', - conditions: { name: '${user.name}' }, + conditions: { name: '${user.name}', id: '${user.id}' }, }, ]; @@ -162,7 +168,7 @@ describe('RulesService', () => { { subject: 'User', action: 'read', - conditions: { name: normalUser.name }, + conditions: { name: normalUser.name, id: normalUser.id }, }, ]); }); diff --git a/src/restricted-endpoints/attachment/attachment/attachment.controller.spec.ts b/src/restricted-endpoints/attachment/attachment/attachment.controller.spec.ts index e816c0a..590b42e 100644 --- a/src/restricted-endpoints/attachment/attachment/attachment.controller.spec.ts +++ b/src/restricted-endpoints/attachment/attachment/attachment.controller.spec.ts @@ -16,7 +16,11 @@ describe('AttachmentController', () => { let mockCouchDB: CouchdbService; let mockPermissions: PermissionService; + let user: UserInfo; + beforeEach(async () => { + user = new UserInfo('user-id', 'user', []); + mockCouchDB = { get: () => of(undefined), delete: () => of(undefined), @@ -68,7 +72,7 @@ describe('AttachmentController', () => { 'docId', 'prop', { rev: '1' }, - new UserInfo('user', []), + user, undefined, undefined, ), @@ -89,7 +93,7 @@ describe('AttachmentController', () => { 'docId', 'prop', { rev: '1' }, - new UserInfo('user', []), + user, undefined, undefined, ); @@ -110,7 +114,7 @@ describe('AttachmentController', () => { 'db', 'docId', 'prop', - new UserInfo('user', []), + user, undefined, undefined, ), @@ -130,7 +134,7 @@ describe('AttachmentController', () => { 'db', 'docId', 'prop', - new UserInfo('user', []), + user, undefined, undefined, ); @@ -150,7 +154,7 @@ describe('AttachmentController', () => { 'docId', 'prop', { rev: '1-rev' }, - new UserInfo('user', []), + user, ), ).rejects.toThrow(ForbiddenException); }); @@ -168,7 +172,7 @@ describe('AttachmentController', () => { 'docId', 'prop', { rev: '1-rev' }, - new UserInfo('user', []), + user, ); expect(mockCouchDB.delete).toHaveBeenCalledWith('db', 'docId/prop', { diff --git a/src/restricted-endpoints/document/document.controller.spec.ts b/src/restricted-endpoints/document/document.controller.spec.ts index fdced05..9b6f7b4 100644 --- a/src/restricted-endpoints/document/document.controller.spec.ts +++ b/src/restricted-endpoints/document/document.controller.spec.ts @@ -33,7 +33,7 @@ describe('DocumentController', () => { roles: [], type: 'user', }; - const requestingUser: UserInfo = new UserInfo('testUser', []); + const requestingUser: UserInfo = new UserInfo('user-id', 'testUser', []); const SUCCESS_RESPONSE: DocSuccess = { ok: true, id: userDoc._id, @@ -329,6 +329,7 @@ describe('DocumentController', () => { it('should throw exception if the update permission is not given', async () => { const otherUser: UserInfo = { + id: 'user-other', name: 'anotherUser', roles: [], projects: [], diff --git a/src/restricted-endpoints/replication/bulk-document/bulk-doc-endpoints.controller.spec.ts b/src/restricted-endpoints/replication/bulk-document/bulk-doc-endpoints.controller.spec.ts index 1364e28..6ccc686 100644 --- a/src/restricted-endpoints/replication/bulk-document/bulk-doc-endpoints.controller.spec.ts +++ b/src/restricted-endpoints/replication/bulk-document/bulk-doc-endpoints.controller.spec.ts @@ -59,7 +59,7 @@ describe('BulkDocEndpointsController', () => { jest .spyOn(documentFilter, 'filterBulkGetResponse') .mockReturnValue(filteredResponse); - const user = new UserInfo('username', ['user']); + const user = new UserInfo('user-id', 'username', ['user']); const result = await firstValueFrom( controller.bulkGetPost(null, null, null, user), @@ -107,7 +107,7 @@ describe('BulkDocEndpointsController', () => { jest .spyOn(documentFilter, 'filterAllDocsResponse') .mockReturnValue(filteredResponse); - const user = new UserInfo('username', ['user']); + const user = new UserInfo('user-id', 'username', ['user']); const result = await firstValueFrom( controller.allDocs('db', null, user, null), @@ -153,7 +153,7 @@ describe('BulkDocEndpointsController', () => { jest .spyOn(documentFilter, 'filterBulkDocsRequest') .mockReturnValue(Promise.resolve(filteredRequest)); - const user = new UserInfo('username', ['admin']); + const user = new UserInfo('user-id', 'username', ['admin']); await firstValueFrom(controller.bulkDocs('db', request, user)); @@ -197,7 +197,7 @@ describe('BulkDocEndpointsController', () => { jest .spyOn(documentFilter, 'filterFindResponse') .mockReturnValue(filteredResponse); - const user = new UserInfo('username', ['admin']); + const user = new UserInfo('user-id', 'username', ['admin']); await firstValueFrom(controller.find('db', request, user)); diff --git a/src/restricted-endpoints/replication/bulk-document/bulk-document.service.spec.ts b/src/restricted-endpoints/replication/bulk-document/bulk-document.service.spec.ts index a0ab283..90c8bd9 100644 --- a/src/restricted-endpoints/replication/bulk-document/bulk-document.service.spec.ts +++ b/src/restricted-endpoints/replication/bulk-document/bulk-document.service.spec.ts @@ -30,7 +30,7 @@ describe('BulkDocumentService', () => { mockCouchDBService = { post: () => of({}), } as any; - normalUser = new UserInfo('normalUser', ['user']); + normalUser = new UserInfo('user-id', 'normalUser', ['user']); schoolDoc = getSchoolDoc(); childDoc = getChildDoc(); diff --git a/src/restricted-endpoints/replication/changes/changes.controller.spec.ts b/src/restricted-endpoints/replication/changes/changes.controller.spec.ts index ef8a8fa..98ffdd3 100644 --- a/src/restricted-endpoints/replication/changes/changes.controller.spec.ts +++ b/src/restricted-endpoints/replication/changes/changes.controller.spec.ts @@ -24,7 +24,12 @@ describe('ChangesController', () => { childDoc, deletedChildDoc, ]); - const user: UserInfo = { name: 'username', roles: [], projects: [] }; + const user: UserInfo = { + id: 'user-id', + name: 'username', + roles: [], + projects: [], + }; const mockCouchdbService = { get: () => undefined } as CouchdbService; const getSpy = jest.spyOn(mockCouchdbService, 'get'); const mockRulesService = { diff --git a/src/restricted-endpoints/session/session.controller.spec.ts b/src/restricted-endpoints/session/session.controller.spec.ts index 0921fed..61ec282 100644 --- a/src/restricted-endpoints/session/session.controller.spec.ts +++ b/src/restricted-endpoints/session/session.controller.spec.ts @@ -20,7 +20,7 @@ describe('SessionController', () => { }); it('should return the user object on the request', () => { - const user = new UserInfo('user', ['user_app']); + const user = new UserInfo('user-id', 'user', ['user_app']); const response = controller.login(user); @@ -28,7 +28,7 @@ describe('SessionController', () => { }); it('should return user object from combinedAuth middleware if user is authenticated', async () => { - const user = new UserInfo('user', ['user_app']); + const user = new UserInfo('user-id', 'user', ['user_app']); const res = await controller.session(user); diff --git a/src/restricted-endpoints/session/user-auth.dto.ts b/src/restricted-endpoints/session/user-auth.dto.ts index 4a12be1..589c794 100644 --- a/src/restricted-endpoints/session/user-auth.dto.ts +++ b/src/restricted-endpoints/session/user-auth.dto.ts @@ -10,7 +10,14 @@ export class UserCredentials { * User object as used by CouchDB */ export class UserInfo { + /** + * @param id The account id (Keycloak user id) + * @param name The entityId of the user profile (e.g. User:123 -> CouchDB) + * @param roles The roles the user has (via the Keycloak user) + * @param projects The projects the user is linked to (via the user profile entity) + */ constructor( + public id: string, public name: string, public roles: string[], public projects: string[] = [],