Fix bug where encrypted XLSForm with choice filter has invalid XML (#2881)

* fix bug where encrypted XLSForm with choice filter has invalid XML

The bug arises from survey.to_json_dict() generating incorrect JSON schema when publishing the XLSForm. A SurveyElementBuilder object created from an existing XForm's JSON schema generates invalid XML

* fix failing tests

* refactor code

* remove redundant check

* refactor code

* refactor code

Author: kelvin-muchiri

onadata/apps/logger/models/xform.py

@@ -27,7 +27,7 @@
 from django.utils.translation import gettext_lazy
 
 from guardian.models import GroupObjectPermissionBase, UserObjectPermissionBase
-from pyxform import SurveyElementBuilder, constants, create_survey_element_from_dict
+from pyxform import SurveyElementBuilder, constants
 from pyxform.errors import PyXFormError
 from pyxform.question import Question
 from pyxform.section import RepeatingSection
@@ -1113,14 +1113,12 @@ def _set_encrypted_field(self):
             self.encrypted = "public_key" in json_dict
 
     def _set_public_key_field(self):
-        if self.json and self.json != "":
-            if self.num_of_submissions == 0 and self.public_key:
-                json_dict = self.json_dict()
-                json_dict["public_key"] = self.public_key
-                survey = create_survey_element_from_dict(json_dict)
-                self.json = survey.to_json_dict()
-                self.xml = survey.to_xml()
-                self._set_encrypted_field()
+        if self.public_key and self.num_of_submissions == 0:
+            survey = self.get_survey_from_xlsform()
+            survey.public_key = self.public_key
+            self.json = survey.to_json_dict()
+            self.xml = survey.to_xml()
+            self._set_encrypted_field()
 
     def json_dict(self):
         """Returns the `self.json` field data as a dict."""

onadata/apps/main/tests/test_base.py

@@ -21,6 +21,7 @@
 from django.test import RequestFactory, TransactionTestCase
 from django.test.client import Client
 from django.utils import timezone
+from django.utils.encoding import smart_str
 
 from django_digest.test import Client as DigestClient
 from django_digest.test import DigestAuth
@@ -152,9 +153,9 @@ def _publish_xlsx_file_with_external_choices(self, form_version="v1"):
         # make sure publishing the survey worked
         self.assertEqual(XForm.objects.count(), pre_count + 1)
 
-    def _publish_xls_file_and_set_xform(self, path):
+    def _publish_xls_file_and_set_xform(self, path, user=None):
         count = XForm.objects.count()
-        self._publish_xls_file(path)
+        self._publish_xls_file(path, user)
         self.assertEqual(XForm.objects.count(), count + 1)
         # pylint: disable=attribute-defined-outside-init
         self.xform = XForm.objects.order_by("pk").reverse()[0]
@@ -652,18 +653,20 @@ def _publish_entity_update_form(self, user, project=None):
     def _encrypt_xform(self, xform, kms_key, encrypted_by=None):
         version = timezone.now().strftime("%Y%m%d%H%M")
 
-        json_dict = xform.json_dict()
-        json_dict["public_key"] = kms_key.public_key
-        json_dict["version"] = version
-
-        survey = create_survey_element_from_dict(json_dict)
+        survey = xform.get_survey_from_xlsform()
+        survey.public_key = kms_key.public_key
+        survey.version = version
 
         xform.json = survey.to_json_dict()
         xform.xml = survey.to_xml()
         xform.version = version
         xform.public_key = kms_key.public_key
         xform.encrypted = True
+        xform.is_managed = True
         xform.save()
         xform.kms_keys.create(
             version=version, kms_key=kms_key, encrypted_by=encrypted_by
         )
+
+    def _clean_xml(self, xml):
+        return re.sub(r">\s+<", "><", smart_str(xml.strip()))

onadata/libs/kms/tools.py

@@ -42,7 +42,6 @@
     XForm,
     XFormKey,
 )
-from onadata.apps.logger.models.xform import create_survey_element_from_dict
 from onadata.libs.exceptions import DecryptionError, EncryptionError
 from onadata.libs.permissions import is_organization
 from onadata.libs.utils.cache_tools import (
@@ -310,11 +309,9 @@ def _invalidate_xform_list_cache(xform: XForm):
 def _encrypt_xform(xform, kms_key, encrypted_by=None):
     version = timezone.now().strftime("%Y%m%d%H%M")
 
-    json_dict = xform.json_dict()
-    json_dict["public_key"] = kms_key.public_key
-    json_dict["version"] = version
-
-    survey = create_survey_element_from_dict(json_dict)
+    survey = xform.get_survey_from_xlsform()
+    survey.public_key = kms_key.public_key
+    survey.version = version
 
     xform.json = survey.to_json_dict()
     xform.xml = survey.to_xml()
@@ -615,11 +612,9 @@ def disable_xform_encryption(xform, disabled_by=None) -> None:
 
     new_version = timezone.now().strftime("%Y%m%d%H%M")
 
-    json_dict = xform.json_dict()
-    json_dict["version"] = new_version
-    del json_dict["public_key"]
-
-    survey = create_survey_element_from_dict(json_dict)
+    survey = xform.get_survey_from_xlsform()
+    survey.public_key = None
+    survey.version = new_version
 
     xform.json = survey.to_json_dict()
     xform.xml = survey.to_xml()