fix: support user.id placeholder for conditions in rules (#171)

see Aam-Digital/ndb-core#2847
Aam-Digital · Feb 11, 2025 · 9920623 · 9920623
1 parent 1090b71
commit 9920623
Show file tree

Hide file tree

Showing 13 changed files with 49 additions and 24 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -30,7 +30,7 @@ describe('PermissionService', () => {
 
     service = module.get<PermissionService>(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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -20,15 +20,15 @@ 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);
 
     expect(response).toBe(user);
   });
 
   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
@@ -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[] = [],