Disallow technical user modifications

gristlabs · Nov 29, 2024 · 8312aa9 · 8312aa9
1 parent 090398c
commit 8312aa9
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 19 deletions.
diff --git a/app/server/lib/scim/v2/ScimUserController.ts b/app/server/lib/scim/v2/ScimUserController.ts
@@ -77,6 +77,9 @@ class ScimUserController {
   public async overwriteUser(resource: any, data: any, context: RequestContext) {
     return this._runAndHandleErrors(context, async () => {
       const id = ScimUserController._getIdFromResource(resource);
+      if (this._dbManager.getSpecialUserIds().includes(id)) {
+        throw new SCIMMY.Types.Error(403, null!, 'Technical user modification not permitted.');
+      }
       await this._checkEmailCanBeUsed(data.userName, id);
       const updatedUser = await this._dbManager.overwriteUser(id, toUserProfile(data));
       return toSCIMMYUser(updatedUser);
@@ -93,7 +96,7 @@ class ScimUserController {
     return this._runAndHandleErrors(context, async () => {
       const id = ScimUserController._getIdFromResource(resource);
       if (this._dbManager.getSpecialUserIds().includes(id)) {
-        throw new SCIMMY.Types.Error(403, null!, 'Cannot delete technical user');
+        throw new SCIMMY.Types.Error(403, null!, 'Technical user deletion not permitted.');
       }
       const fakeScope: Scope = { userId: id };
       // FIXME: deleteUser should probably better not requiring a scope.

diff --git a/test/server/lib/Scim.ts b/test/server/lib/Scim.ts
@@ -1,4 +1,4 @@
-import axios from 'axios';
+import axios, { AxiosResponse } from 'axios';
 import capitalize from 'lodash/capitalize';
 import { assert } from 'chai';
 import Sinon from 'sinon';
@@ -129,6 +129,27 @@ describe('Scim', () => {
         await getDbManager().deleteUser({ userId: userId }, userId);
       }
     }
+    async function checkOperationOnTechUserDisallowed({op, opType}: {
+      op: (id: number) => Promise<AxiosResponse>,
+      opType: string
+    }) {
+      const db = getDbManager();
+      const specialUsers = {
+        'anonymous': db.getAnonymousUserId(),
+        'support': db.getSupportUserId(),
+        'everyone': db.getEveryoneUserId(),
+        'preview': db.getPreviewerUserId(),
+      };
+      for (const [label, id] of Object.entries(specialUsers)) {
+        const res = await op(id);
+        assert.equal(res.status, 403, `should forbid ${opType} of the special user ${label}`);
+        assert.deepEqual(res.data, {
+          schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ],
+          status: '403',
+          detail: `Technical user ${opType} not permitted.`
+        });
+      }
+    }
 
     function checkCommonErrors(
       method: 'get' | 'post' | 'put' | 'patch' | 'delete',
@@ -468,6 +489,15 @@ describe('Scim', () => {
         assert.equal(res.status, 404);
       });
 
+      it('should return 403 for technical users', async function () {
+        const data = toSCIMUserWithoutId('whoever');
+        await checkOperationOnTechUserDisallowed({
+          op: (id) => axios.put(scimUrl(`/Users/${id}`), data, chimpy),
+          opType: 'modification'
+        });
+      });
+
+
       it('should deduce the name from the displayEmail when not provided', async function () {
         const res = await axios.put(scimUrl(`/Users/${userToUpdateId}`), {
           schemas: ['urn:ietf:params:scim:schemas:core:2.0:User'],
@@ -580,23 +610,11 @@ describe('Scim', () => {
         assert.equal(res.status, 404);
       });
 
-      it('should return 404 for technical users', async function () {
-        const db = getDbManager();
-        const specialUsers = {
-          'anonymous': db.getAnonymousUserId(),
-          'support': db.getSupportUserId(),
-          'everyone': db.getEveryoneUserId(),
-          'preview': db.getPreviewerUserId(),
-        };
-        for (const [label, id] of Object.entries(specialUsers)) {
-          const res = await axios.delete(scimUrl(`/Users/${id}`), chimpy);
-          assert.equal(res.status, 403, 'should forbid deletion of the special user ' + label);
-          assert.deepEqual(res.data, {
-            schemas: [ 'urn:ietf:params:scim:api:messages:2.0:Error' ],
-            status: '403',
-            detail: 'Cannot delete technical user'
-          });
-        }
+      it('should return 403 for technical users', async function () {
+        await checkOperationOnTechUserDisallowed({
+          op: (id) => axios.delete(scimUrl(`/Users/${id}`), chimpy),
+          opType: 'deletion'
+        });
       });
 
       checkCommonErrors('delete', '/Users/1');