Merge pull request #904 from kobotoolbox/require-auth-at-project-level

Require authentication at the project level instead of the account level

.gitlab-ci.yml

@@ -33,6 +33,7 @@ test:
     POSTGRES_PASSWORD: kobo
     POSTGRES_DB: kobocat_test
     SERVICE_ACCOUNT_BACKEND_URL: redis://redis_cache:6379/4
+    ENKETO_REDIS_MAIN_URL: redis://redis_cache:6379/0
   services:
     - name: postgis/postgis:14-3.2
       alias: postgres

onadata/apps/api/tests/fixtures/formList_w_require_auth.xml

@@ -0,0 +1,2 @@
+<?xml version="1.0" encoding="utf-8"?>
+<xforms xmlns="http://openrosa.org/xforms/xformsList"><xform><formID>transportation_2011_07_25</formID><name>transportation_2011_07_25</name><majorMinorVersion></majorMinorVersion><version></version><hash>md5:%(hash)s</hash><descriptionText>transportation_2011_07_25</descriptionText><downloadUrl>http://testserver/forms/%(pk)s/form.xml</downloadUrl><manifestUrl>http://testserver/xformsManifest/%(pk)s</manifestUrl></xform></xforms>

onadata/apps/api/tests/viewsets/test_abstract_viewset.py

@@ -9,7 +9,6 @@
 )
 from django.test import TestCase
 from django.test.client import Client
-from django_digest.test import Client as DigestClient
 from django_digest.test import DigestAuth
 from kobo_service_account.utils import get_request_headers
 from rest_framework.reverse import reverse
@@ -170,7 +169,6 @@ def _create_user_profile(self, extra_post_data={}):
             organization=self.profile_data['organization'],
             home_page=self.profile_data['home_page'],
             twitter=self.profile_data['twitter'],
-            require_auth=False
         )
 
         return new_profile
@@ -290,12 +288,3 @@ def _add_form_metadata(
             response = self._post_form_metadata(data, test)
 
         return response
-
-    def _get_digest_client(self):
-        self.user.profile.require_auth = True
-        self.user.profile.save()
-        client = DigestClient()
-        client.set_authorization(self.profile_data['username'],
-                                 self.profile_data['password1'],
-                                 'Digest')
-        return client

onadata/apps/api/tests/viewsets/test_attachment_viewset.py

@@ -43,6 +43,8 @@ def setUp(self):
 
         alice_profile = self._create_user_profile(alice_profile_data)
         self.alice = alice_profile.user
+        # re-assign `self.user` and `self.profile_data` to bob
+        self._login_user_and_profile(self.default_profile_data.copy())
 
     def _retrieve_view(self, auth_headers):
         self._submit_transport_instance_w_attachment()

onadata/apps/api/tests/viewsets/test_briefcase_api.py

@@ -33,15 +33,10 @@ def setUp(self):
         self.form_def_path = os.path.join(
             self.main_directory, 'fixtures', 'transportation',
             'transportation.xml')
-        self._submission_list_url = reverse(
-            'view-submission-list', kwargs={'username': self.user.username})
-        self._submission_url = reverse(
-            'submissions', kwargs={'username': self.user.username})
-        self._download_submission_url = reverse(
-            'view-download-submission',
-            kwargs={'username': self.user.username})
-        self._form_upload_url = reverse(
-            'form-upload', kwargs={'username': self.user.username})
+        self._submission_list_url = reverse('view-submission-list')
+        self._submission_url = reverse('submissions')
+        self._download_submission_url = reverse('view-download-submission')
+        self._form_upload_url = reverse('form-upload')
 
     def test_view_submission_list(self):
         view = BriefcaseApi.as_view({'get': 'list'})
@@ -50,11 +45,11 @@ def test_view_submission_list(self):
         request = self.factory.get(
             self._submission_list_url,
             data={'formId': self.xform.id_string})
-        response = view(request, username=self.user.username)
+        response = view(request)
         self.assertEqual(response.status_code, 401)
         auth = DigestAuth(self.login_username, self.login_password)
         request.META.update(auth(request.META, response))
-        response = view(request, username=self.user.username)
+        response = view(request)
         self.assertEqual(response.status_code, 200)
         submission_list_path = os.path.join(
             self.main_directory, 'fixtures', 'transportation',
@@ -71,6 +66,21 @@ def test_view_submission_list(self):
                     '{{resumptionCursor}}', str(last_index))
             self.assertContains(response, expected_submission_list)
 
+    def test_cannot_view_submission_list_with_username(self):
+        view = BriefcaseApi.as_view({'get': 'list'})
+        self._publish_xml_form()
+        self._make_submissions()
+
+        request = self.factory.get(
+            self._submission_list_url, data={'formId': self.xform.id_string}
+        )
+        response = view(request, username=self.user.username)
+        self.assertEqual(response.status_code, 401)
+        auth = DigestAuth(self.login_username, self.login_password)
+        request.META.update(auth(request.META, response))
+        response = view(request, username=self.user.username)
+        self.assertEqual(response.status_code, 403)
+
     def test_view_submission_list_w_deleted_submission(self):
         view = BriefcaseApi.as_view({'get': 'list'})
         self._publish_xml_form()
@@ -80,11 +90,11 @@ def test_view_submission_list_w_deleted_submission(self):
         request = self.factory.get(
             self._submission_list_url,
             data={'formId': self.xform.id_string})
-        response = view(request, username=self.user.username)
+        response = view(request)
         self.assertEqual(response.status_code, 401)
         auth = DigestAuth(self.login_username, self.login_password)
         request.META.update(auth(request.META, response))
-        response = view(request, username=self.user.username)
+        response = view(request)
         self.assertEqual(response.status_code, 200)
         submission_list_path = os.path.join(
             self.main_directory, 'fixtures', 'transportation',
@@ -102,18 +112,18 @@ def test_view_submission_list_w_deleted_submission(self):
             self.assertContains(response, expected_submission_list)
 
         view = BriefcaseApi.as_view({'get': 'retrieve'})
-        formId = '%(formId)s[@version=null and @uiVersion=null]/' \
-                 '%(formId)s[@key=uuid:%(instanceId)s]' % {
-                     'formId': self.xform.id_string,
-                     'instanceId': uuid}
-        params = {'formId': formId}
-        request = self.factory.get(
-            self._download_submission_url, data=params)
-        response = view(request, username=self.user.username)
+        form_id = (
+            '%(formId)s[@version=null and @uiVersion=null]/'
+            '%(formId)s[@key=uuid:%(instanceId)s]'
+            % {'formId': self.xform.id_string, 'instanceId': uuid}
+        )
+        params = {'formId': form_id}
+        request = self.factory.get(self._download_submission_url, data=params)
+        response = view(request)
         self.assertEqual(response.status_code, 401)
         auth = DigestAuth(self.login_username, self.login_password)
         request.META.update(auth(request.META, response))
-        response = view(request, username=self.user.username)
+        response = view(request)
         self.assertTrue(response.status_code, 404)
 
     def test_view_submission_list_other_user(self):
@@ -132,10 +142,10 @@ def test_view_submission_list_other_user(self):
         request = self.factory.get(
             self._submission_list_url,
             data={'formId': self.xform.id_string})
-        response = view(request, username=self.user.username)
+        response = view(request)
         self.assertEqual(response.status_code, 401)
         request.META.update(auth(request.META, response))
-        response = view(request, username=self.user.username)
+        response = view(request)
         self.assertEqual(response.status_code, 404)
 
     def test_view_submission_list_num_entries(self):
@@ -169,10 +179,10 @@ def get_last_index(xform, last_index=None):
             request = self.factory.get(
                 self._submission_list_url,
                 data=params)
-            response = view(request, username=self.user.username)
+            response = view(request)
             self.assertEqual(response.status_code, 401)
             request.META.update(auth(request.META, response))
-            response = view(request, username=self.user.username)
+            response = view(request)
             self.assertEqual(response.status_code, 200)
             if index > 2:
                 last_index = get_last_index(self.xform, last_index)
@@ -208,10 +218,10 @@ def test_view_download_submission(self):
         auth = DigestAuth(self.login_username, self.login_password)
         request = self.factory.get(
             self._download_submission_url, data=params)
-        response = view(request, username=self.user.username)
+        response = view(request)
         self.assertEqual(response.status_code, 401)
         request.META.update(auth(request.META, response))
-        response = view(request, username=self.user.username)
+        response = view(request)
         download_submission_path = os.path.join(
             self.main_directory, 'fixtures', 'transportation',
             'view', 'downloadSubmission.xml')
@@ -239,10 +249,10 @@ def test_view_download_submission_no_xmlns(self):
         auth = DigestAuth(self.login_username, self.login_password)
         request = self.factory.get(
             self._download_submission_url, data=params)
-        response = view(request, username=self.user.username)
+        response = view(request)
         self.assertEqual(response.status_code, 401)
         request.META.update(auth(request.META, response))
-        response = view(request, username=self.user.username)
+        response = view(request)
         download_submission_path = os.path.join(
             self.main_directory, 'fixtures', 'transportation',
             'view', 'downloadSubmission.xml')
@@ -259,9 +269,9 @@ def test_view_download_submission_no_xmlns(self):
         with override_settings(SUPPORT_BRIEFCASE_SUBMISSION_DATE=False):
             request = self.factory.get(
                 self._download_submission_url, data=params)
-            response = view(request, username=self.user.username)
+            response = view(request)
             request.META.update(auth(request.META, response))
-            response = view(request, username=self.user.username)
+            response = view(request)
             response.render()
             self.assertNotIn(
                 'transportation id="transportation_2011_07_25" '
@@ -293,19 +303,19 @@ def test_view_download_submission_other_user(self):
         auth = DigestAuth('alice', 'alicealice')
         url = self._download_submission_url  # aliasing long name
         request = self.factory.get(url, data=params)
-        response = view(request, username=self.user.username)
+        response = view(request)
         self.assertEqual(response.status_code, 401)
 
         # Rewind the file to avoid the xml parser to get an empty string
         # and throw and parsing error
         request = request = self.factory.get(url, data=params)
         request.META.update(auth(request.META, response))
-        response = view(request, username=self.user.username)
+        response = view(request)
         self.assertEqual(response.status_code, 404)
 
     def test_publish_xml_form_other_user(self):
         view = BriefcaseApi.as_view({'post': 'create'})
-        # deno cannot publish form to bob's account
+        # alice cannot publish form to bob's account
         alice_data = {
             'username': 'alice',
             'password1': 'alicealice',
@@ -336,7 +346,7 @@ def test_publish_xml_form_where_filename_is_not_id_string(self):
             params = {'form_def_file': f, 'dataFile': ''}
             auth = DigestAuth(self.login_username, self.login_password)
             request = self.factory.post(self._form_upload_url, data=params)
-            response = view(request, username=self.user.username)
+            response = view(request)
             self.assertEqual(response.status_code, 401)
 
             # Rewind the file to avoid the xml parser to get an empty string
@@ -345,7 +355,7 @@ def test_publish_xml_form_where_filename_is_not_id_string(self):
             # Create a new requests to avoid request.FILES to be empty
             request = self.factory.post(self._form_upload_url, data=params)
             request.META.update(auth(request.META, response))
-            response = view(request, username=self.user.username)
+            response = view(request)
             self.assertEqual(XForm.objects.count(), count + 1)
             self.assertContains(
                 response, "successfully published.", status_code=201)
@@ -358,7 +368,7 @@ def _publish_xml_form(self, auth=None):
             params = {'form_def_file': f, 'dataFile': ''}
             auth = auth or DigestAuth(self.login_username, self.login_password)
             request = self.factory.post(self._form_upload_url, data=params)
-            response = view(request, username=self.user.username)
+            response = view(request)
             self.assertEqual(response.status_code, 401)
 
             # Rewind the file to avoid the xml parser to get an empty string
@@ -367,7 +377,7 @@ def _publish_xml_form(self, auth=None):
             # Create a new requests to avoid request.FILES to be empty
             request = self.factory.post(self._form_upload_url, data=params)
             request.META.update(auth(request.META, response))
-            response = view(request, username=self.user.username)
+            response = view(request)
             self.assertEqual(XForm.objects.count(), count + 1)
             self.assertContains(
                 response, "successfully published.", status_code=201)
@@ -381,7 +391,7 @@ def test_form_upload(self):
             params = {'form_def_file': f, 'dataFile': ''}
             auth = DigestAuth(self.login_username, self.login_password)
             request = self.factory.post(self._form_upload_url, data=params)
-            response = view(request, username=self.user.username)
+            response = view(request)
             self.assertEqual(response.status_code, 401)
 
             # Rewind the file to avoid the xml parser to get an empty string
@@ -390,7 +400,7 @@ def test_form_upload(self):
             # Create a new requests to avoid request.FILES to be empty
             request = self.factory.post(self._form_upload_url, data=params)
             request.META.update(auth(request.META, response))
-            response = view(request, username=self.user.username)
+            response = view(request)
             self.assertEqual(response.status_code, 400)
             # SQLite returns `UNIQUE constraint failed` whereas PostgreSQL
             # returns 'duplicate key ... violates unique constraint'
@@ -404,10 +414,10 @@ def test_upload_head_request(self):
 
         auth = DigestAuth(self.login_username, self.login_password)
         request = self.factory.head(self._form_upload_url)
-        response = view(request, username=self.user.username)
+        response = view(request)
         self.assertEqual(response.status_code, 401)
         request.META.update(auth(request.META, response))
-        response = view(request, username=self.user.username)
+        response = view(request)
         self.assertEqual(response.status_code, 204)
         self.assertTrue(response.has_header('X-OpenRosa-Version'))
         self.assertTrue(
@@ -438,7 +448,7 @@ def test_submission_with_instance_id_on_root_node(self):
             # Create a new requests to avoid request.FILES to be empty
             request = self.factory.post(self._submission_list_url, post_data)
             request.META.update(auth(request.META, response))
-            response = view(request, username=self.user.username)
+            response = view(request)
             self.assertContains(response, message, status_code=201)
             self.assertContains(response, instanceId, status_code=201)
             self.assertEqual(Instance.objects.count(), count + 1)

onadata/apps/api/tests/viewsets/test_connect_viewset.py

@@ -32,7 +32,7 @@ def setUp(self):
             'website': user_profile_data['website'],
             'twitter': user_profile_data['twitter'],
             'gravatar': user_profile_data['gravatar'],
-            'require_auth': False,
+            'require_auth': True,
             'api_token': self.user.auth_token.key,
             'temp_token': self.client.session.session_key,
         }

onadata/apps/api/tests/viewsets/test_data_viewset.py

@@ -145,11 +145,6 @@ def test_data_with_service_account(self):
                          response.data)
 
     def test_data_anon(self):
-        # By default, `_create_user_and_login()` creates users without
-        # authentication required. We force it when submitting data to persist
-        # collector's username because this test expects it.
-        # See `_submitted_data` in `data` below
-        self._set_require_auth(True)
         self._make_submissions()
         view = DataViewSet.as_view({'get': 'list'})
         request = self.factory.get('/')
@@ -341,30 +336,31 @@ def test_cannot_get_enketo_edit_url_without_require_auth(self):
         less-bad option is to reject edit requests with an explicit error
         message when anonymous submissions are enabled.
         """
-        self.assertFalse(self.user.profile.require_auth)
+        self.xform.require_auth = False
+        self.xform.save(update_fields=['require_auth'])
+        self.assertFalse(self.xform.require_auth)
         self._make_submissions()
+
         for view_ in ['enketo', 'enketo_edit']:
             view = DataViewSet.as_view({'get': view_})
             formid = self.xform.pk
             dataid = self.xform.instances.all().order_by('id')[0].pk
             request = self.factory.get(
                 '/',
-                data={'return_url': "http://test.io/test_url"},
+                data={'return_url': 'http://test.io/test_url'},
                 **self.extra
             )
             response = view(request, pk=formid, dataid=dataid)
             self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
             self.assertTrue(
                 response.data[0].startswith(
                     'Cannot edit submissions while "Require authentication '
-                    'to see forms and submit data" is disabled for your '
-                    'account'
+                    'to see form and submit data" is disabled for your '
+                    'project'
                 )
             )
 
     def test_get_enketo_edit_url(self):
-        self.user.profile.require_auth = True
-        self.user.profile.save()
         self._make_submissions()
         for view_ in ['enketo', 'enketo_edit']:
             # ensure both legacy `/enketo` and the new `/enketo_edit` endpoints
@@ -448,7 +444,6 @@ def test_get_form_public_data(self):
                          response_first_element)
 
     def test_data_w_attachment(self):
-        self._set_require_auth(auth=True)
         self._submit_transport_instance_w_attachment()
 
         view = DataViewSet.as_view({'get': 'list'})