Form with truthy expression displayed despite permissions (#7466)

medic · Jan 26, 2022 · 15ef2b6 · 15ef2b6
1 parent 4980b8b
commit 15ef2b6
Show file tree

Hide file tree

Showing 2 changed files with 103 additions and 21 deletions.
diff --git a/webapp/src/ts/services/xml-forms.service.ts b/webapp/src/ts/services/xml-forms.service.ts
@@ -137,6 +137,32 @@ export class XmlFormsService {
     });
   }
 
+  private checkFormExpression(form, doc, user, contactSummary) {
+    if (!form.context.expression) {
+      return true;
+    }
+
+    try {
+      return this.evaluateExpression(
+        form.context.expression,
+        doc,
+        user,
+        contactSummary
+      );
+    } catch(err) {
+      console.error(`Unable to evaluate expression for form: ${form._id}`, err);
+      return false;
+    }
+  }
+
+  private checkFormPermissions(form) {
+    if (!form.context.permission) {
+      return true;
+    }
+
+    return this.authService.has(form.context.permission);
+  }
+
   private filter(form, options, user) {
     if (!options.includeCollect && form.context && form.context.collect) {
       return false;
@@ -158,27 +184,10 @@ export class XmlFormsService {
       return true;
     }
 
-    return this.filterContactTypes(form.context, options.doc).then(validSoFar => {
-      if (!validSoFar) {
-        return false;
-      }
-      if (form.context.expression) {
-        try {
-          return this.evaluateExpression(form.context.expression, options.doc, user, options.contactSummary);
-        } catch(err) {
-          console.error(`Unable to evaluate expression for form: ${form._id}`, err);
-          return false;
-        }
-      }
-      if (form.context.expression &&
-        !this.evaluateExpression(form.context.expression, options.doc, user, options.contactSummary)) {
-        return false;
-      }
-      if (!form.context.permission) {
-        return true;
-      }
-      return this.authService.has(form.context.permission);
-    });
+    return this
+      .filterContactTypes(form.context, options.doc)
+      .then(valid => valid && this.checkFormPermissions(form))
+      .then(valid => valid && this.checkFormExpression(form, options.doc, user, options.contactSummary));
   }
 
   private notify(error, forms?) {

diff --git a/webapp/tests/karma/ts/services/xml-forms.service.spec.ts b/webapp/tests/karma/ts/services/xml-forms.service.spec.ts
@@ -777,6 +777,79 @@ describe('XmlForms service', () => {
 
     });
 
+    it('does not return a form with a truthy expression if the user does not have relevant permissions', () => {
+      const given = [
+        {
+          id: 'visit',
+          doc: {
+            _id: 'visit',
+            internalId: 'visit',
+            _attachments: { xml: { something: true } },
+            context: {
+              expression: 'true',
+              permission: [ 'national_admin' ]
+            },
+          },
+        },
+        {
+          id: 'registration',
+          doc: {
+            _id: 'visit',
+            internalId: 'visit',
+            _attachments: { xml: { something: true } },
+            context: {
+              expression: 'true',
+              permission: [ 'district_admin' ]
+            },
+          },
+        }
+      ];
+      dbQuery.resolves({ rows: given });
+      hasAuth.withArgs([ 'national_admin' ]).resolves(true);
+      UserContact.resolves();
+      const service = getService();
+      return service.list().then(actual => {
+        expect(actual.length).to.equal(1);
+        expect(actual[0]).to.deep.equal(given[0].doc);
+      });
+    });
+
+    it('does not return a form with a false expression if the user has the relevant permissions', () => {
+      const given = [
+        {
+          id: 'visit',
+          doc: {
+            _id: 'visit',
+            internalId: 'visit',
+            _attachments: { xml: { something: true } },
+            context: {
+              expression: 'false',
+              permission: [ 'national_admin' ]
+            },
+          },
+        },
+        {
+          id: 'registration',
+          doc: {
+            _id: 'visit',
+            internalId: 'visit',
+            _attachments: { xml: { something: true } },
+            context: {
+              expression: 'false',
+              permission: [ 'district_admin' ]
+            },
+          },
+        }
+      ];
+      dbQuery.resolves({ rows: given });
+      hasAuth.withArgs([ 'national_admin' ]).resolves(true);
+      UserContact.resolves();
+      const service = getService();
+      return service.list().then(actual => {
+        expect(actual.length).to.equal(0);
+      });
+    });
+
   });
 
   describe('listen', () => {