Merge pull request #35804 from dimagi/nh/mapping

Safe mapping

corehq/apps/integration/kyc/forms.py

@@ -45,7 +45,7 @@ class Meta:
     api_field_to_user_data_map = JsonField(
         label=_('API Field to User Data Map'),
         required=True,
-        expected_type=list,
+        expected_type=dict,
     )
     connection_settings = forms.ModelChoiceField(
         label=_('Connection Settings'),

corehq/apps/integration/kyc/models.py

@@ -34,7 +34,7 @@ class KycConfig(models.Model):
     domain = models.CharField(max_length=126, db_index=True)
     user_data_store = models.CharField(max_length=25, choices=UserDataStore.CHOICES)
     other_case_type = models.CharField(max_length=126, null=True)
-    api_field_to_user_data_map = jsonfield.JSONField(default=list)
+    api_field_to_user_data_map = jsonfield.JSONField(default=dict)
     provider = models.CharField(
         max_length=25,
         choices=KycProviders.choices,

corehq/apps/integration/kyc/services.py

@@ -10,6 +10,7 @@
 
 from corehq.apps.hqcase.utils import update_case
 from corehq.apps.integration.kyc.models import UserDataStore
+from corehq.apps.users.models import CommCareUser
 
 
 class UserCaseNotFound(Exception):
@@ -129,17 +130,33 @@ def get_user_data_for_api(source, config):
         Returns a dictionary of user data for the API.
         ``source`` is a CommCareUser or a CommCareCase.
     """
+    # CommCareUser properties that could map to API fields
+    safe_commcare_user_properties = {
+        'first_name',
+        'last_name',
+        'full_name',
+        'name',
+        'email',
+        'username',  # For CommCareUsers this is an email address
+        'phone_number',
+        'default_phone_number',
+    }
     source_data = _get_source_data(source, config)
     user_data_for_api = {}
-    for mapping in config.api_field_to_user_data_map:
-        try:
-            if mapping['source'] == 'standard':
-                user_data_for_api[mapping['fieldName']] = getattr(source, mapping['mapsTo'])
-            else:
-                user_data_for_api[mapping['fieldName']] = source_data[mapping['mapsTo']]
-        except (AttributeError, KeyError):
+    for api_field, user_data_property in config.api_field_to_user_data_map.items():
+        if user_data_property in source_data:
+            # Fetch value from usercase / custom user data by default
+            value = source_data[user_data_property]
+        elif (
+            isinstance(source, CommCareUser)
+            and user_data_property in safe_commcare_user_properties
+        ):
+            # Fall back to CommCareUser
+            value = getattr(source, user_data_property)
+        else:
             # Conservative approach to skip the API field if data is not available for the user
             continue
+        user_data_for_api[api_field] = value
     return user_data_for_api
 
 

corehq/apps/integration/kyc/tests/test_services.py

@@ -135,7 +135,11 @@ def tearDownClass(cls):
 
     def setUp(self):
         super().setUp()
-        self.user = CommCareUser.create(self.domain, 'test-kyc', '123', None, None, first_name='abc')
+        self.user = CommCareUser.create(
+            self.domain, 'test-kyc', '123',
+            None, None,
+            first_name='abc',
+        )
         self.config = KycConfig.objects.create(
             domain=self.domain,
             user_data_store=UserDataStore.CUSTOM_USER_DATA,
@@ -149,24 +153,25 @@ def tearDown(self):
         super().tearDown()
 
     def _sample_api_field_to_user_data_map(self):
-        return [
-            {
-                "fieldName": "first_name",
-                "mapsTo": "first_name",
-                "source": "standard"
-            },
-            {
-                "fieldName": "nationality",
-                "mapsTo": "nationality",
-                "source": "custom"
-            }
-        ]
+        return {
+            # API Field : User data
+            "first_name": "first_name",
+            "nationality": "nationality",
+        }
 
     def test_custom_user_data_store(self):
         self.user.get_user_data(self.domain).update({'nationality': 'Indian'})
         result = get_user_data_for_api(self.user, self.config)
         self.assertEqual(result, {'first_name': 'abc', 'nationality': 'Indian'})
 
+    def test_unsafe_custom_user_data_store(self):
+        self.config.api_field_to_user_data_map = {
+            # API field : Custom user data / CommCareUser property
+            "first_name": "password",
+        }
+        result = get_user_data_for_api(self.user, self.config)
+        self.assertEqual(result, {})
+
     def test_custom_user_data_store_with_no_data(self):
         result = get_user_data_for_api(self.user, self.config)
         self.assertEqual(result, {'first_name': 'abc'})
@@ -196,7 +201,6 @@ def test_user_case_data_store_with_no_case(self):
     def test_custom_case_data_store(self):
         self.config.user_data_store = UserDataStore.OTHER_CASE_TYPE
         self.config.other_case_type = 'other-case'
-        self.config.api_field_to_user_data_map[0]['source'] = 'custom'
         self.config.save()
         case = create_case(
             self.domain,
@@ -221,7 +225,7 @@ def test_custom_case_data_store_with_no_data(self):
 
     def test_incorrect_mapping_standard_field(self):
         api_field_to_user_data_map = self._sample_api_field_to_user_data_map()
-        api_field_to_user_data_map[0]['mapsTo'] = 'wrong-standard_field'
+        api_field_to_user_data_map['first_name'] = 'wrong-standard_field'
         self.config.api_field_to_user_data_map = api_field_to_user_data_map
         self.config.save()
 

corehq/apps/integration/kyc/tests/test_views.py

@@ -105,53 +105,18 @@ def setUpClass(cls):
         )
         cls.addClassCleanup(conn_settings.delete)
 
-        cls.kyc_mapping = [
-            {
-                'fieldName': 'first_name',
-                'mapsTo': 'first_name',
-                'source': 'standard',
-            },
-            {
-                'fieldName': 'last_name',
-                'mapsTo': 'last_name',
-                'source': 'standard'
-            },
-            {
-                'fieldName': 'email',
-                'mapsTo': 'email',
-                'source': 'standard'
-            },
-            {
-                'fieldName': 'phone_number',
-                'mapsTo': 'phone_number',
-                'source': 'custom',
-            },
-            {
-                'fieldName': 'national_id_number',
-                'mapsTo': 'national_id_number',
-                'source': 'custom',
-            },
-            {
-                'fieldName': 'street_address',
-                'mapsTo': 'street_address',
-                'source': 'custom',
-            },
-            {
-                'fieldName': 'city',
-                'mapsTo': 'city',
-                'source': 'custom',
-            },
-            {
-                'fieldName': 'post_code',
-                'mapsTo': 'post_code',
-                'source': 'custom',
-            },
-            {
-                'fieldName': 'country',
-                'mapsTo': 'country',
-                'source': 'custom',
-            },
-        ]
+        cls.kyc_mapping = {
+            # API field: User data
+            'first_name': 'first_name',
+            'last_name': 'last_name',
+            'email': 'email',
+            'phone_number': 'phone_number',
+            'national_id_number': 'national_id_number',
+            'street_address': 'street_address',
+            'city': 'city',
+            'post_code': 'post_code',
+            'country': 'country',
+        }
         cls.kyc_config = KycConfig.objects.create(
             domain=cls.domain,
             user_data_store=UserDataStore.CUSTOM_USER_DATA,
@@ -244,6 +209,7 @@ def test_response_data_users(self):
                     'has_invalid_data': True,
                     'first_name': 'Jane',
                     'last_name': 'Doe',
+                    'phone_number': None,
                     'email': '',
                 })
             else:
@@ -265,23 +231,11 @@ def test_response_data_users(self):
     def test_response_data_cases(self):
         self.kyc_config.user_data_store = UserDataStore.OTHER_CASE_TYPE
         self.kyc_config.other_case_type = 'other-case'
-        self.kyc_config.api_field_to_user_data_map[0:3] = [
-            {
-                'fieldName': 'first_name',
-                'mapsTo': 'first_name',
-                'source': 'custom'
-            },
-            {
-                'fieldName': 'last_name',
-                'mapsTo': 'last_name',
-                'source': 'custom'
-            },
-            {
-                'fieldName': 'email',
-                'mapsTo': 'email',
-                'source': 'custom'
-            }
-        ]
+        self.kyc_config.api_field_to_user_data_map.update({
+            'first_name': 'first_name',
+            'last_name': 'last_name',
+            'email': 'email',
+        })
         self.kyc_config.save()
 
         response = self._make_request()