Fix organization admin check in firestore.rules

CHANGELOG.md

@@ -42,6 +42,9 @@ All notable changes to this project will be documented in this file. The format
   of parent measurements when switching between items.
 - The edit button for key result values is now only visible to users with the
   appropriate permissions.
+- Organization admins should now be allowed to edit own organizations including
+  child departments and products.
+
 
 ### Security
 

firestore.rules

@@ -34,7 +34,7 @@ service cloud.firestore {
       let isAdminFromOrgOfProdOrDep = userIsAdmin && hasOrgProperty && getAfter(request.resource.data.organization).id in userDoc.data.admin;
 
       // Check if the user has access to the document which is an organization (given that the first check is false)
-      let isAdminOfOrg = userIsAdmin && request.resource.data.id in userDoc.data.admin;
+      let isAdminOfOrg = userIsAdmin && request.resource.id in userDoc.data.admin;
 
       return isSignedIn() && (isAdminOfOrg || isAdminFromOrgOfProdOrDep);
     }

tests/firebase/firestore.test.js

@@ -40,6 +40,9 @@ describe('Test Firestore rules', () => {
       await users
         .doc('[email protected]')
         .set({ name: 'Super Admin', superAdmin: true });
+      await users
+        .doc('[email protected]')
+        .set({ name: 'Org Y Admin', admin: ['organization-y'] });
       await users
         .doc('[email protected]')
         .set({ name: 'User', admin: ['organization-x'] });
@@ -67,6 +70,11 @@ describe('Test Firestore rules', () => {
         organization: organizations.doc('organization-x'),
         department: departments.doc('department-x1'),
       });
+      await products.doc('product-y1').set({
+        name: 'Product Y1',
+        organization: organizations.doc('organization-y'),
+        department: departments.doc('department-y1'),
+      });
 
       await objectives.doc('department-x1-objective-1').set({
         name: 'Department X1 - Objective 1',
@@ -159,6 +167,48 @@ describe('Test Firestore rules', () => {
     });
   });
 
+  test('users cannot update organizations', async () => {
+    await withAuthenticatedUser(testEnv, '[email protected]', async (db) => {
+      const organization = db.collection('organizations').doc('organization-x');
+      await expectPermissionDenied(organization.update({ name: 'Org X' }));
+    });
+  });
+
+  test('users cannot delete organizations', async () => {
+    await withAuthenticatedUser(testEnv, '[email protected]', async (db) => {
+      const organization = db.collection('organizations').doc('organization-x');
+      await expectPermissionDenied(organization.delete());
+    });
+  });
+
+  test('organization admin can update own organization', async () => {
+    await withAuthenticatedUser(testEnv, '[email protected]', async (db) => {
+      const organizationX = db.collection('organizations').doc('organization-x');
+      const organizationY = db.collection('organizations').doc('organization-y');
+      await expectPermissionDenied(organizationX.update({ name: 'Org X' }));
+      await expectUpdateSucceeds(organizationY.update({ name: 'Org Y' }));
+      expect((await organizationX.get()).data().name).toBe('Organization X');
+      expect((await organizationY.get()).data().name).toBe('Org Y');
+    });
+  });
+
+  test('organization admin can update own child departments and products', async () => {
+    await withAuthenticatedUser(testEnv, '[email protected]', async (db) => {
+      const departmentX1 = db.collection('departments').doc('department-x1');
+      const departmentY1 = db.collection('departments').doc('department-y1');
+      const productX1 = db.collection('products').doc('product-x1');
+      const productY1 = db.collection('products').doc('product-y1');
+      await expectPermissionDenied(departmentX1.update({ name: 'Dep X1' }));
+      await expectUpdateSucceeds(departmentY1.update({ name: 'Dep Y1' }));
+      await expectPermissionDenied(productX1.update({ name: 'Prod X1' }));
+      await expectUpdateSucceeds(productY1.update({ name: 'Prod Y1' }));
+      expect((await departmentX1.get()).data().name).toBe('Department X1');
+      expect((await departmentY1.get()).data().name).toBe('Dep Y1');
+      expect((await productX1.get()).data().name).toBe('Product X1');
+      expect((await productY1.get()).data().name).toBe('Prod Y1');
+    });
+  });
+
   test('anonymous users cannot read objective contributors', async () => {
     await withUnauthenticatedUser(testEnv, async (db) => {
       const oc = db.collection('objectiveContributors').doc('objective-contributor-1');