diff --git a/onadata/apps/api/tests/viewsets/test_data_viewset.py b/onadata/apps/api/tests/viewsets/test_data_viewset.py index 6194e5bd5..3eba3e109 100644 --- a/onadata/apps/api/tests/viewsets/test_data_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_data_viewset.py @@ -355,7 +355,7 @@ def test_cannot_get_enketo_edit_url_without_require_auth(self): self.assertTrue( response.data[0].startswith( 'Cannot edit submissions while "Require authentication ' - 'to see forms and submit data" is disabled for your ' + 'to see form and submit data" is disabled for your ' 'project' ) ) diff --git a/onadata/apps/api/tests/viewsets/test_xform_list_api.py b/onadata/apps/api/tests/viewsets/test_xform_list_api.py index 40c4805ee..2663ac0a3 100644 --- a/onadata/apps/api/tests/viewsets/test_xform_list_api.py +++ b/onadata/apps/api/tests/viewsets/test_xform_list_api.py @@ -484,10 +484,11 @@ def test_retrieve_xform_manifest_as_anonymous(self): 'get': 'manifest' }) request = self.factory.get('/') - # Not auth required but XForm cannot be found except if `required_auth` - # is True. - # See `TestXFormListApiWithoutAuthRequired.test_retrieve_xform_manifest()` response = self.view(request, pk=self.xform.pk, username=self.user.username) + # The project (self.xform) requires auth by default. Anonymous user cannot + # access it. It is also True for the project manifest, thus anonymous + # should receive a 404. + # See `TestXFormListApiWithoutAuthRequired.test_retrieve_xform_manifest()` self.assertEqual(response.status_code, 404) def test_head_xform_manifest(self): diff --git a/onadata/apps/api/viewsets/briefcase_api.py b/onadata/apps/api/viewsets/briefcase_api.py index f00f747de..e7d6617a3 100644 --- a/onadata/apps/api/viewsets/briefcase_api.py +++ b/onadata/apps/api/viewsets/briefcase_api.py @@ -108,7 +108,7 @@ def get_object(self): uuid = _extract_uuid(form_id) obj = get_instance_or_404( - xform__id_string__exact=id_string, + xform__id_string=id_string, uuid=uuid, ) self.check_object_permissions(self.request, obj.xform) @@ -118,7 +118,8 @@ def get_object(self): def filter_queryset(self, queryset): username = self.kwargs.get('username') if username is None: - # If no username is specified, the request must be authenticated + # Briefcase does not allow anonymous access, user should always be + # authenticated if self.request.user.is_anonymous: # raises a permission denied exception, forces authentication self.permission_denied(self.request) @@ -127,20 +128,20 @@ def filter_queryset(self, queryset): # including those shared by other users queryset = super().filter_queryset(queryset) else: - raise exceptions.PermissionDenied( - detail=t( - 'Cannot access non secure URL. Remove username from ' - 'aggregate server URL in configuration settings.' - ), - code=status.HTTP_403_FORBIDDEN, - ) + # With the advent of project-level anonymous access in #904, Briefcase + # requests must no longer use endpoints whose URLs contain usernames. + # Ideally, Briefcase would display error messages returned by this method, + # but sadly that is not the case. + # Raise an empty PermissionDenied since it's impossible to have + # Briefcase display any guidance. + raise exceptions.PermissionDenied() form_id = self.request.GET.get('formId', '') if form_id.find('[') != -1: form_id = _extract_id_string(form_id) - xform = get_object_or_404(queryset, id_string__exact=form_id) + xform = get_object_or_404(queryset, id_string=form_id) self.check_object_permissions(self.request, xform) instances = Instance.objects.filter(xform=xform).order_by('pk') num_entries = self.request.GET.get('numEntries') @@ -172,17 +173,14 @@ def create(self, request, *args, **kwargs): xform_def = request.FILES.get('form_def_file', None) response_status = status.HTTP_201_CREATED - if username := kwargs.get('username'): - raise exceptions.PermissionDenied( - detail=t( - 'Cannot User %(user)s has no permission to add xforms to ' - 'account %(account)s' - % { - 'user': request.user.username, - 'account': username, - } - ) - ) + # With the advent of project-level anonymous access in #904, Briefcase + # requests must no longer use endpoints whose URLs contain usernames. + # Ideally, Briefcase would display error messages returned by this method, + # but sadly that is not the case. + # Raise an empty PermissionDenied since it's impossible to have + # Briefcase display any guidance. + if kwargs.get('username'): + raise exceptions.PermissionDenied() data = {} diff --git a/onadata/apps/api/viewsets/data_viewset.py b/onadata/apps/api/viewsets/data_viewset.py index 6d3aa22be..cbf97a69d 100644 --- a/onadata/apps/api/viewsets/data_viewset.py +++ b/onadata/apps/api/viewsets/data_viewset.py @@ -648,15 +648,6 @@ def enketo(self, request, *args, **kwargs): permission_classes=[EnketoSubmissionEditPermissions], ) def enketo_edit(self, request, *args, **kwargs): - """ - Trying to edit in Enketo while `profile.require_auth == False` leads to - an infinite authentication loop because Enketo never sends credentials - unless it receives a 401 response to an unauthenticated HEAD request. - There's no way to send such a response for editing only while - simultaneously allowing anonymous submissions to the same endpoint. - Avoid the infinite loop by blocking doomed requests here and returning - a helpful error message. - """ return self._enketo_request(request, action_='edit', *args, **kwargs) @action( @@ -678,9 +669,17 @@ def _enketo_request(self, request, action_, *args, **kwargs): raise ParseError(t('`return_url` not provided.')) if not object_.xform.require_auth and action_ == 'edit': + # Trying to edit in Enketo while `xform.require_auth == False` + # leads to an infinite authentication loop because Enketo never + # sends credentials unless it receives a 401 response to an + # unauthenticated HEAD request. + # There's no way to send such a response for editing only while + # simultaneously allowing anonymous submissions to the same endpoint. + # Avoid the infinite loop by blocking doomed requests here and + # returning a helpful error message. raise ValidationError(t( 'Cannot edit submissions while "Require authentication to ' - 'see forms and submit data" is disabled for your project' + 'see form and submit data" is disabled for your project' )) try: diff --git a/onadata/apps/api/viewsets/xform_list_api.py b/onadata/apps/api/viewsets/xform_list_api.py index 292106062..32bb14536 100644 --- a/onadata/apps/api/viewsets/xform_list_api.py +++ b/onadata/apps/api/viewsets/xform_list_api.py @@ -87,7 +87,8 @@ def filter_queryset(self, queryset): # including those shared by other users queryset = super().filter_queryset(queryset) else: - # Only returns non-secured projects with URL starts with username + # Only return projects that allow anonymous submissions when path + # starts with a username queryset = queryset.filter( user__username=username.lower(), require_auth=False ) @@ -111,8 +112,6 @@ def list(self, request, *args, **kwargs): if request.method == 'HEAD': return self.get_response_for_head_request() - - serializer = self.get_serializer( object_list, many=True, require_auth=not bool(kwargs.get('username')) ) diff --git a/onadata/apps/logger/__init__.py b/onadata/apps/logger/__init__.py index db7dc9515..7b6a6d772 100644 --- a/onadata/apps/logger/__init__.py +++ b/onadata/apps/logger/__init__.py @@ -1,5 +1,7 @@ # coding: utf-8 from django.apps import AppConfig +from django.conf import settings +from django.core.checks import register, Error class LoggerAppConfig(AppConfig): @@ -14,3 +16,26 @@ def ready(self): from kobo_service_account.utils import reversion_monkey_patch reversion_monkey_patch() super().ready() + + +@register() +def check_enketo_redis_main_url(app_configs, **kwargs): + """ + `ENKETO_REDIS_MAIN_URL` is required to make the app run properly. + """ + errors = [] + if not settings.CACHES.get('enketo_redis_main'): + # We need to set `BACKEND` property. Otherwise, this error is shadowed + # by Django CACHES system checks. + settings.CACHES['enketo_redis_main']['BACKEND'] = ( + 'django.core.cache.backends.dummy.DummyCache' + ) + errors.append( + Error( + f'Please set environment variable `ENKETO_REDIS_MAIN_URL`', + hint='Enketo Express Redis main URL is missing.', + obj=settings, + id='kobo.logger.enketo_redis_main.E001', + ) + ) + return errors diff --git a/onadata/apps/logger/migrations/0034_set_require_auth_at_project_level.py b/onadata/apps/logger/migrations/0034_set_require_auth_at_project_level.py index e7427cc09..652bc7dd6 100644 --- a/onadata/apps/logger/migrations/0034_set_require_auth_at_project_level.py +++ b/onadata/apps/logger/migrations/0034_set_require_auth_at_project_level.py @@ -4,13 +4,11 @@ from django.conf import settings from django.db import migrations -from redis import Redis +from django_redis import get_redis_connection CHUNK_SIZE = 2000 -redis_client = Redis.from_url(settings.ENKETO_REDIS_MAIN_URL) - def restore_open_rosa_server_in_redis(apps, schema_editor): @@ -28,7 +26,7 @@ def restore_open_rosa_server_in_redis(apps, schema_editor): XForm = apps.get_model('logger', 'XForm') # noqa parsed_url = urlparse(settings.KOBOCAT_URL) - server_url = f"{settings.KOBOCAT_URL.rstrip('/')}" + server_url = settings.KOBOCAT_URL.rstrip('/') xforms_iter = ( XForm.objects.filter(require_auth=False) @@ -59,6 +57,7 @@ def restore_open_rosa_server_in_redis(apps, schema_editor): end end """ + redis_client = get_redis_connection('enketo_redis_main') pipeline = redis_client.pipeline() pipeline.eval(lua_script, 0) pipeline.execute() @@ -66,18 +65,26 @@ def restore_open_rosa_server_in_redis(apps, schema_editor): def restore_require_auth_at_profile_level(apps, schema_editor): - if settings.SKIP_HEAVY_MIGRATIONS: - return - - XForm = apps.get_model('logger', 'XForm') # noqa - UserProfile = apps.get_model('main', 'UserProfile') # noqa - - UserProfile.objects.filter( - user_id__in=XForm.objects.filter(require_auth=False).values_list( - 'user_id' - ) - ).update(require_auth=False) - XForm.objects.filter(require_auth=True).update(require_auth=False) + print( + """ + Restoring authentication at the profile level cannot be done + automatically. + + This is an example of what can be done: + ⚠️ WARNING ⚠️ The example below makes all projects publicly + accessible when profile level is restored even if, at least, one project + was publicly accessible at project level. + + ```python + UserProfile.objects.filter( + user_id__in=XForm.objects.filter(require_auth=False).values_list( + 'user_id' + ) + ).update(require_auth=False) + XForm.objects.filter(require_auth=True).update(require_auth=False) + ``` + """ + ) def set_require_auth_at_project_level(apps, schema_editor): @@ -141,6 +148,7 @@ def update_open_rosa_server_in_redis(apps, schema_editor): end end """ + redis_client = get_redis_connection('enketo_redis_main') pipeline = redis_client.pipeline() pipeline.eval(lua_script, 0) pipeline.execute() diff --git a/onadata/apps/logger/models/xform.py b/onadata/apps/logger/models/xform.py index 7cc96e905..966543ca9 100644 --- a/onadata/apps/logger/models/xform.py +++ b/onadata/apps/logger/models/xform.py @@ -70,7 +70,7 @@ class XForm(BaseModel): user = models.ForeignKey(User, related_name='xforms', null=True, on_delete=models.CASCADE) require_auth = models.BooleanField( default=True, - verbose_name=t('Require authentication to see forms and submit data'), + verbose_name=t('Require authentication to see form and submit data'), ) shared = models.BooleanField(default=False) shared_data = models.BooleanField(default=False) diff --git a/onadata/apps/logger/tests/test_briefcase_client.py b/onadata/apps/logger/tests/test_briefcase_client.py index ef1432daa..f393914e2 100644 --- a/onadata/apps/logger/tests/test_briefcase_client.py +++ b/onadata/apps/logger/tests/test_briefcase_client.py @@ -27,6 +27,7 @@ def formList(*args, **kwargs): # noqa def xformsDownload(*args, **kwargs): # noqa view = XFormListApi.as_view({'get': 'retrieve'}) response = view(*args, **kwargs) + response.render() return response def xformsManifest(*args, **kwargs): # noqa @@ -61,7 +62,6 @@ def form_list_xml(url, request, **kwargs): res = formList(req) elif url.path.endswith('form.xml'): res = xformsDownload(req, pk=xform_id) - res.render() elif url.path.find('xformsManifest') > -1: res = xformsManifest(req, pk=xform_id) elif url.path.find('xformsMedia') > -1: diff --git a/onadata/apps/logger/tests/test_form_submission.py b/onadata/apps/logger/tests/test_form_submission.py index 9bccc6d1a..eba2687c8 100644 --- a/onadata/apps/logger/tests/test_form_submission.py +++ b/onadata/apps/logger/tests/test_form_submission.py @@ -438,8 +438,7 @@ def test_anonymous_cannot_edit_submissions(self): "..", "fixtures", "tutorial", "instances", "tutorial_2012-06-27_11-27-53_w_uuid_edited.xml" ) - # …without "Require authentication to see forms and submit data" - # self.assertFalse(self.user.profile.require_auth) + # …without "Require authentication to see form and submit data" self.xform.require_auth = False self.xform.save(update_fields=['require_auth']) @@ -476,8 +475,6 @@ def test_authorized_user_can_edit_submissions_without_require_auth(self): submissions at the same time. """ - # self.assertFalse(self.user.profile.require_auth) - xml_submission_file_path = os.path.join( os.path.dirname(os.path.abspath(__file__)), "..", "fixtures", "tutorial", "instances", @@ -576,7 +573,7 @@ def test_unauthorized_cannot_edit_submissions(self): "..", "fixtures", "tutorial", "instances", "tutorial_2012-06-27_11-27-53_w_uuid_edited.xml" ) - # …without "Require authentication to see forms and submit data" + # …without "Require authentication to see form and submit data" self.xform.require_auth = False self.xform.save(update_fields=['require_auth']) self._make_submission( diff --git a/onadata/apps/main/tests/test_base.py b/onadata/apps/main/tests/test_base.py index f9f2b1b54..9eaa9b163 100644 --- a/onadata/apps/main/tests/test_base.py +++ b/onadata/apps/main/tests/test_base.py @@ -78,7 +78,7 @@ def _create_user_and_login(self, username="bob", password="bob"): self.login_password = password self.user = self._create_user(username, password) - # create user profile and set require_auth to false for tests + # create user profile if it does not exist UserProfile.objects.get_or_create(user=self.user) self.client = self._login(username, password) diff --git a/onadata/libs/utils/briefcase_client.py b/onadata/libs/utils/briefcase_client.py index b58e40d0a..5922d58c3 100644 --- a/onadata/libs/utils/briefcase_client.py +++ b/onadata/libs/utils/briefcase_client.py @@ -346,10 +346,10 @@ def push(self): if form_xml in form_files: form_xml_path = os.path.join(dir_path, form_xml) x = self._upload_xform(form_xml_path, form_xml) - # if isinstance(x, dict): - # self.logger.error("Failed to publish %s" % form_dir) - # else: - # self.logger.debug("Successfully published %s" % form_dir) + if isinstance(x, dict): + self.logger.error("Failed to publish %s" % form_dir) + else: + self.logger.debug("Successfully published %s" % form_dir) if 'instances' in form_dirs: self.logger.debug("Uploading instances") c = self._upload_instances(os.path.join(dir_path, 'instances')) diff --git a/onadata/libs/utils/logger_tools.py b/onadata/libs/utils/logger_tools.py index c957338a6..454537253 100644 --- a/onadata/libs/utils/logger_tools.py +++ b/onadata/libs/utils/logger_tools.py @@ -98,13 +98,10 @@ def check_submission_permissions( """ Check that permission is required and the request user has permission. - The user does not have permissions if: - * the user is authed, - * the form require auth, - * the xform user is not submitting. + The user has permissions iff: + * the form does not require auth + * the user is authed, the form requires auth and user can submit data - Since we have a username, the Instance creation logic will - handle checking for the forms existence by its id_string. :returns: None. :raises: PermissionDenied based on the above criteria. diff --git a/onadata/settings/base.py b/onadata/settings/base.py index b751d435e..5f5031c63 100644 --- a/onadata/settings/base.py +++ b/onadata/settings/base.py @@ -203,11 +203,11 @@ def skip_suspicious_operations(record): 'django_digest', 'corsheaders', 'oauth2_provider', + 'onadata.apps.logger.LoggerAppConfig', 'rest_framework', 'rest_framework.authtoken', 'taggit', 'readonly', - 'onadata.apps.logger.LoggerAppConfig', 'onadata.apps.viewer', 'onadata.apps.main', 'onadata.apps.restservice', @@ -395,6 +395,7 @@ def skip_suspicious_operations(record): CACHES = { # Set CACHE_URL to override. Only redis is supported. 'default': env.cache(default='redis://redis_cache:6380/3'), + 'enketo_redis_main': env.cache('ENKETO_REDIS_MAIN_URL', None), } DIGEST_NONCE_BACKEND = 'onadata.apps.django_digest_backends.cache.RedisCacheNonceStorage' @@ -756,8 +757,6 @@ def skip_suspicious_operations(record): # All internal communications between containers must be HTTP only. ENKETO_PROTOCOL = os.environ.get('ENKETO_PROTOCOL', 'https') -ENKETO_REDIS_MAIN_URL = os.environ.get('ENKETO_REDIS_MAIN_URL', 'redis://localhost:6379/') - ################################ # MongoDB settings #