From e0d25220fb1b786195132d29d7a4e3f790106c29 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Mon, 10 Jul 2023 14:31:04 -0400 Subject: [PATCH 01/23] Add validation check flag and last password datetime change fields to UserProfile model --- ...2_add_validate_password_flag_to_profile.py | 24 +++++++++++++++++++ onadata/apps/main/models/user_profile.py | 3 +++ 2 files changed, 27 insertions(+) create mode 100644 onadata/apps/main/migrations/0012_add_validate_password_flag_to_profile.py diff --git a/onadata/apps/main/migrations/0012_add_validate_password_flag_to_profile.py b/onadata/apps/main/migrations/0012_add_validate_password_flag_to_profile.py new file mode 100644 index 000000000..04398f22f --- /dev/null +++ b/onadata/apps/main/migrations/0012_add_validate_password_flag_to_profile.py @@ -0,0 +1,24 @@ +# Generated by Django 3.2.15 on 2023-07-10 18:00 + +from django.db import migrations, models +import django.utils.timezone + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0011_drop_old_kpi_tables'), + ] + + operations = [ + migrations.AddField( + model_name='userprofile', + name='password_date_changed', + field=models.DateTimeField(default=django.utils.timezone.now), + ), + migrations.AddField( + model_name='userprofile', + name='validated_password', + field=models.BooleanField(default=True), + ), + ] diff --git a/onadata/apps/main/models/user_profile.py b/onadata/apps/main/models/user_profile.py index 15fed14a4..daf7cca6b 100644 --- a/onadata/apps/main/models/user_profile.py +++ b/onadata/apps/main/models/user_profile.py @@ -3,6 +3,7 @@ from django.contrib.auth.models import User from django.db import models from django.db.models.signals import post_save +from django.utils import timezone from django.utils.translation import gettext_lazy from guardian.conf import settings as guardian_settings from guardian.shortcuts import get_perms_for_model, assign_perm @@ -39,6 +40,8 @@ class UserProfile(models.Model): attachment_storage_bytes = models.BigIntegerField(default=0) metadata = models.JSONField(default=dict, blank=True) is_mfa_active = LazyDefaultBooleanField(default=False) + validated_password = models.BooleanField(default=True) + password_date_changed = models.DateTimeField(default=timezone.now) def __str__(self): return '%s[%s]' % (self.name, self.user.username) From 74b7c864961546de1bd4c4a63bc03f0a32d0e337 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Tue, 11 Jul 2023 15:23:37 -0400 Subject: [PATCH 02/23] Remove UI and other old views --- .../templates/404_xls_analyzer_error.html | 12 - kobocat-template/templates/dashboard.html | 109 ---- kobocat-template/templates/export_list.html | 6 - kobocat-template/templates/home.html | 81 --- kobocat-template/templates/legacy_banner.html | 384 -------------- kobocat-template/templates/map.html | 290 ----------- .../templates/outdated_data_view.html | 25 - .../templates/outdated_link_guidance.html | 59 --- kobocat-template/templates/profile.html | 93 ---- .../published_forms__list--empty.html | 3 - .../templates/published_forms__list.html | 61 --- .../templates/published_surveys.html | 30 -- kobocat-template/templates/show.html | 235 --------- .../templates/show_form_settings.html | 303 ----------- kobocat-template/templates/topbar.html | 13 +- onadata/apps/api/viewsets/xform_viewset.py | 32 +- .../logger/tests/test_briefcase_client.py | 10 +- onadata/apps/logger/views.py | 14 +- onadata/apps/main/templates/form_gallery.html | 78 --- onadata/apps/main/templates/form_photos.html | 65 --- onadata/apps/main/tests/test_form_api.py | 174 ------- onadata/apps/main/tests/test_form_auth.py | 14 +- onadata/apps/main/tests/test_form_errors.py | 87 ---- onadata/apps/main/tests/test_form_exports.py | 1 - onadata/apps/main/tests/test_form_metadata.py | 126 ----- onadata/apps/main/tests/test_form_show.py | 135 +---- onadata/apps/main/tests/test_permissions.py | 15 - onadata/apps/main/tests/test_user_profile.py | 108 ---- onadata/apps/main/urls.py | 72 +-- onadata/apps/main/views.py | 471 +----------------- .../apps/viewer/templates/export_list.html | 2 +- onadata/koboform/redirect_middleware.py | 22 +- onadata/libs/templates/change_language.html | 12 - onadata/libs/templates/home_topbar.html | 21 - onadata/settings/base.py | 7 - 35 files changed, 71 insertions(+), 3099 deletions(-) delete mode 100644 kobocat-template/templates/404_xls_analyzer_error.html delete mode 100644 kobocat-template/templates/dashboard.html delete mode 100644 kobocat-template/templates/home.html delete mode 100644 kobocat-template/templates/legacy_banner.html delete mode 100644 kobocat-template/templates/map.html delete mode 100644 kobocat-template/templates/outdated_data_view.html delete mode 100644 kobocat-template/templates/outdated_link_guidance.html delete mode 100644 kobocat-template/templates/profile.html delete mode 100644 kobocat-template/templates/published_forms__list--empty.html delete mode 100644 kobocat-template/templates/published_forms__list.html delete mode 100644 kobocat-template/templates/published_surveys.html delete mode 100644 kobocat-template/templates/show.html delete mode 100644 kobocat-template/templates/show_form_settings.html delete mode 100644 onadata/apps/main/templates/form_gallery.html delete mode 100644 onadata/apps/main/templates/form_photos.html delete mode 100644 onadata/apps/main/tests/test_form_api.py delete mode 100644 onadata/apps/main/tests/test_form_errors.py delete mode 100644 onadata/apps/main/tests/test_form_metadata.py delete mode 100644 onadata/apps/main/tests/test_user_profile.py delete mode 100644 onadata/libs/templates/change_language.html delete mode 100644 onadata/libs/templates/home_topbar.html diff --git a/kobocat-template/templates/404_xls_analyzer_error.html b/kobocat-template/templates/404_xls_analyzer_error.html deleted file mode 100644 index ffecb56c6..000000000 --- a/kobocat-template/templates/404_xls_analyzer_error.html +++ /dev/null @@ -1,12 +0,0 @@ -{% extends 'base.html' %} -{% load i18n %} -{% block content %} -

- {% trans "Error" %} -

- -

- {{ error_message }} -

- -{% endblock %} diff --git a/kobocat-template/templates/dashboard.html b/kobocat-template/templates/dashboard.html deleted file mode 100644 index 2444f6959..000000000 --- a/kobocat-template/templates/dashboard.html +++ /dev/null @@ -1,109 +0,0 @@ -{% load humanize %} - -{% block content %} - -{% load i18n %} - -
-
-
-

{% blocktrans %}Publish a Form Upload XLSForm{% endblocktrans %}

- {% url "tutorial" as tutorial_url %} - {% blocktrans %}For a quick introduction on how to publish a form try do download tutorial.xls and publish it.{% endblocktrans %} -
-
- {% url "syntax" as syntax_url %} - {% blocktrans %}Learn about the XLSForm syntax here along with more form examples.{% endblocktrans %} -
-
-
- {% csrf_token %} -
- -
-
- - {{ form.as_table }} -
-
-
- - {{ form_url.as_table }} -
-
-
- -
-
-
-
-
- -
-
-
-
-
-   -
-
-
-
-
- {% if not profile.gravatar_exists %} - - {% endif %} - gravatar - {% if not profile.gravatar_exists %} - - {% endif %} -
-
-

- {{ content_user.username }} - {% if profile.name %} - | {{ profile.name }} - {% endif %} -

-
-
-
-
-

{{ all_forms|intcomma }}

-
-
-

{{ num_forms|intcomma }}

-
-
-

{{ user_instances|intcomma }}

-
-
-
-
- {% trans "FORMS" %} -
-
- {% trans "SHARED FORMS" %} -
-
- {% trans "SUBMISSIONS" %} -
-
-
-
-
-
-{% include "published_surveys.html" %} -

-{% blocktrans with url=url %} -The url "of" this web application {{ url }} -must be given to ODK Collect before it will get forms from and -submit data to formhub. In Collect's Main Menu, press the Menu -button. Select Server Preferences, then Server. Enter -{{ url }} as the server. {% endblocktrans %} -

-{% endblock %} diff --git a/kobocat-template/templates/export_list.html b/kobocat-template/templates/export_list.html index 91ef01861..b69cbc7eb 100644 --- a/kobocat-template/templates/export_list.html +++ b/kobocat-template/templates/export_list.html @@ -5,12 +5,6 @@ {% block before-content %} {% load i18n %} -
-
- {% trans "Return to" %} {{ xform.title }} -
-
-

{{ export_type_name|upper }} {% blocktrans %}Exports{% endblocktrans %}

diff --git a/kobocat-template/templates/home.html b/kobocat-template/templates/home.html deleted file mode 100644 index 732d41962..000000000 --- a/kobocat-template/templates/home.html +++ /dev/null @@ -1,81 +0,0 @@ -{% extends 'ona_base.html' %} -{% load humanize %} -{% load i18n %} -{% load static %} - -{% block styles %} - {{block.super}} - - {% if koboform_autoredirect %} - - {% endif %} -{% endblock %} - -{% block content %} - -
-

KoBo Coordinated Assessment Tool

-

KoBoCAT was built using the latest technologies for the most demanding situations. KoBoCAT is used by humanitarian actors for rapid post-disaster assements, project evaluations, or any other form of data collection.

-
-
-
- -
-
- {% url "login" as login_url %} -
- -
- -
- -
- {% csrf_token %} - -
-
-
- {% endblock %} diff --git a/kobocat-template/templates/legacy_banner.html b/kobocat-template/templates/legacy_banner.html deleted file mode 100644 index b5c590dc4..000000000 --- a/kobocat-template/templates/legacy_banner.html +++ /dev/null @@ -1,384 +0,0 @@ -{% load i18n %} -{% load static %} - - - - - - -
-
-
-

- - - - - - {% trans "Changes to the legacy interface on 14 June 2021" %} -

- -

- {% trans "On 14 June 2021 we removed a number of components of this legacy user interface for viewing project data (also known as kobocat). Thank you to everyone who has participated in our survey over the last months and for responding to the forum discussion. This updated version has allowed us to upgrade kobocat’s source code and dependencies to ensure proper data security in the long run, and lower the amount of work needed to maintain it. You can notice the following changes:"%} -

- -
    -
  • - {% trans "Form uploads are only supported through the regular interface. Note that even if a setting form configuration is not supported yet in the formbuilder, you can deploy the form from there and it will work the same way as deployed through the legacy interface."%} -
    • -
  • - {% trans "You are able to sync all your forms that were deployed directly in the legacy interface to make them available as projects in the regular interface (as requested on the forum)."%} -
    • -
  • - {% trans "Form media and all other settings are handled directly in the regular interface."%} -
    • -
  • - {% trans "You can continue to export data from the legacy interface (XLS, CSV, KML, ZIP)"%} -
    • -
- -

- {% trans "The API will continue to work as before. The legacy interface will continue to be available for a few more months. After this, the kobocat component will serve for its core responsibility, receiving and storing survey submission data."%} -

- - -
-
-
- - -
-
- - -

- - - - - - {% trans "Changes to the legacy interface on 14 June 2021" %} -

- -

- {% trans "On 14 June 2021 we removed a number of components of this legacy user interface for viewing project data (also known as kobocat). After this date, the legacy interface will continue to be available for a few more months, with a number of changes." %} -

-
-
diff --git a/kobocat-template/templates/map.html b/kobocat-template/templates/map.html deleted file mode 100644 index 9fad4a53a..000000000 --- a/kobocat-template/templates/map.html +++ /dev/null @@ -1,290 +0,0 @@ -{% extends 'base.html' %} -{% load i18n %} -{% load static %} - -{% block additional-headers %} - - - - - - -{% endblock %} - -{% block body %} - - - {% include "topbar.html" %} -
back - to project
- - - - -
- - {% block javascript %} - {{ block.super }} - - - - - - - - - - - - - - - - - - - {% endblock %} - -{% endblock %} diff --git a/kobocat-template/templates/outdated_data_view.html b/kobocat-template/templates/outdated_data_view.html deleted file mode 100644 index 03d56764c..000000000 --- a/kobocat-template/templates/outdated_data_view.html +++ /dev/null @@ -1,25 +0,0 @@ -{% extends 'base.html' %} -{% load static %} - - -{% block before-content %} -{% load i18n %} - -
-
- {% trans "Return to" %} {{ xform.title }} -
-
- -
-
- -{% endblock %} - -{% block content %} -{% load i18n %} - -
- {% include "outdated_link_guidance.html" %} -
-{% endblock %} diff --git a/kobocat-template/templates/outdated_link_guidance.html b/kobocat-template/templates/outdated_link_guidance.html deleted file mode 100644 index b9d5621e8..000000000 --- a/kobocat-template/templates/outdated_link_guidance.html +++ /dev/null @@ -1,59 +0,0 @@ -{% load i18n %} - - -
-

⚠ {% trans "Outdated Link" %}

- {% if not kpi_url %} - {% if user == xform.user %} -

- {% trans "Many features cannot be accessed at this outdated address." %} - {% trans "Please click here to synchronize your forms." %} -

- {% else %} -

- {% trans "Many features cannot be accessed at this outdated address." %} - {% trans "Please ask the owner to log into their account and synchronize their forms." %} -

- {% endif %} - {% else %} -

- {% trans "Many features cannot be accessed at this outdated address." %} - {% trans "Please use the following instead:" %} -

-

- {{ kpi_url }} - -

- {% endif %} -

- - {% trans "To learn more, please click here." %} - -

-
diff --git a/kobocat-template/templates/profile.html b/kobocat-template/templates/profile.html deleted file mode 100644 index b695dbf94..000000000 --- a/kobocat-template/templates/profile.html +++ /dev/null @@ -1,93 +0,0 @@ -{% extends 'base.html' %} -{% load i18n %} -{% load static %} - -{% block styles %} - {{ block.super }} - -{% endblock %} - -{% block additional-headers %} - {% load i18n %} - {% if show_dashboard %} -
-
-

{% trans "Projects" %}

-
-
- {% endif %} -{% endblock %} - -{% block before-content %} - {% include "legacy_banner.html" %} -{% endblock %} - -
- {% block content %} - {% if all_forms or forms_shared_with %} -
-
- -
-

- Click the Sync form button below to make sure all your projects - are available in the regular interface. - All settings, data visualizations, and exports are - available there.
-

- - - {% trans "Go to regular interface" %} - -
-

-
-
-
- {% if show_dashboard %} - {% include "published_surveys.html" %} - {% else %} - - - {% endif %} - {% endif %} - {% endblock %} -
- -{% block below-content %} - {% if show_dashboard and not all_forms and not forms_shared_with %} -
-
-

{% trans "You currently have no projects yet." %}

-

{% trans "You can create a project by deploying one of your forms." %}

- {% if koboform_url %} -

- {% trans "Go to regular interface" %} -

- {% endif %} -
-
- {% endif %} -{% endblock %} - -{% block javascript %} - {{ block.super }} -{% endblock %} diff --git a/kobocat-template/templates/published_forms__list--empty.html b/kobocat-template/templates/published_forms__list--empty.html deleted file mode 100644 index 5ae1a9afe..000000000 --- a/kobocat-template/templates/published_forms__list--empty.html +++ /dev/null @@ -1,3 +0,0 @@ -{% load humanize %} -{% load i18n %} - diff --git a/kobocat-template/templates/published_forms__list.html b/kobocat-template/templates/published_forms__list.html deleted file mode 100644 index 7c879ebad..000000000 --- a/kobocat-template/templates/published_forms__list.html +++ /dev/null @@ -1,61 +0,0 @@ -{% load humanize %} -{% load i18n %} - - - - - - - - - - - - - - - - -{% for xform in xform_list.xforms %} - {% with submission_count=xform.submission_count time_of_last_submission=xform.time_of_last_submission has_instances_with_geopoints=xform.has_instances_with_geopoints %} - - - - - - - - - - {% endwith %} -{% endfor %} - - - -
{% trans "Projects" %}{% trans "Active" %}{% trans "Shared By" %}{% trans "Date Created" %}{% trans "Last Modified" %}{% trans "Submissions" %}
- {{ xform.title }} - {% if xform.description %} -
- {{ xform.description }} -
- {% endif %} - - {% if xform.encrypted %} -  {% trans "ENCRYPTED" %}  - {% endif %} - 		- {% if not xform.downloadable %}{% endif %} - {% if xform.downloadable %}{% endif %} - - {% if xform.user != request.user %} - {{ xform.user.username }} - {% endif %} - - {{ xform.date_created|date:"N d, Y" }} - - {% if time_of_last_submission %} - {{ time_of_last_submission|date:"N d, Y" }} - {% endif %} - {{ submission_count|intcomma }}
diff --git a/kobocat-template/templates/published_surveys.html b/kobocat-template/templates/published_surveys.html deleted file mode 100644 index 871278948..000000000 --- a/kobocat-template/templates/published_surveys.html +++ /dev/null @@ -1,30 +0,0 @@ -{% load humanize %} -{% load i18n %} - -{% if all_forms or forms_shared_with %} - - {% if message and message.preview_url %} - - {% endif %} - - {% for xform_list in xforms_list %} - {% if xform_list.id == "published_or_shared" %} -
- {% if xform_list.xforms %} - {% include 'published_forms__list.html' %} - {% endif %} -
- {% else %} - - {% endif %} - {% endfor %} -{% endif %} diff --git a/kobocat-template/templates/show.html b/kobocat-template/templates/show.html deleted file mode 100644 index 7b02c7871..000000000 --- a/kobocat-template/templates/show.html +++ /dev/null @@ -1,235 +0,0 @@ -{% extends 'base.html' %} -{% load humanize %} -{% load static %} - -{% block additional-headers %} - {% load i18n %} - - -
-
-

{{ xform.title }}

-
- - {{ xform.description }} -
- -
-
- - - -{% endblock %} - -{% block content %} - {% load i18n %} - - - - {% include "outdated_link_guidance.html" %} -
- -
-
-

- Submissions ({{ xform.submission_count|intcomma }}) -

- - - Download data - - - -
- - {# if images|length #} -
-

- Photos -

- - - - View gallery - - - - Download all photos - -
- {# endif #} - -
-

- Form -

-
-
- {{ xform.id_string }} -
-
-
-
-
- {% if xform.has_instances_with_geopoints %} -
-

GPS Points

- - - Download GPS points - -
- {% endif %} -
- - - -{% endblock %} - -{% block styles %} - {{ block.super }} - - - - - -{% endblock %} - -{% block javascript %} - {{ block.super }} - - {% if user.is_authenticated %} - - - {% endif %} - - - - - - - - - - -{% endblock %} diff --git a/kobocat-template/templates/show_form_settings.html b/kobocat-template/templates/show_form_settings.html deleted file mode 100644 index 6b1a77bfb..000000000 --- a/kobocat-template/templates/show_form_settings.html +++ /dev/null @@ -1,303 +0,0 @@ -{% extends 'base.html' %} -{% load humanize %} -{% load static %} - -{% block additional-headers %} -{% load i18n %} - - -{% endblock %} - -{% block content %} -{% load i18n %} - - {% if is_owner %} -
-
-
-

- {% trans "Form Settings" %} -

- -
- -
    - {% for media in media_upload %} -
  • -
    {{ media.data_value }}
    - - {% if can_edit %} - - {% endif %} -
    • - {% endfor %} -
- - {% if can_edit %} - - - - {% endif %} -
-
- -
-
- {% endif %} - -{% endblock %} - -{% block styles %} -{{ block.super }} - - - - -{% endblock %} - -{% block javascript %} -{{ block.super }} - - {% if user.is_authenticated %} - - - {% endif %} - - - - - - - - -{% endblock %} diff --git a/kobocat-template/templates/topbar.html b/kobocat-template/templates/topbar.html index 63195d699..2d2f429b1 100644 --- a/kobocat-template/templates/topbar.html +++ b/kobocat-template/templates/topbar.html @@ -10,7 +10,6 @@ {% endif %} -

{% trans "Projects" %}

@@ -18,9 +17,9 @@

{
  • - + - {% trans "Forms" %} + {% trans "Projects" %}
  • @@ -30,14 +29,6 @@

    {

    • - {% url "user_profile" user.username as is_username %} -
  • - - - {% trans "Projects" %} - -
    • -
  • diff --git a/onadata/apps/api/viewsets/xform_viewset.py b/onadata/apps/api/viewsets/xform_viewset.py index dbb93c86c..f36fcec65 100644 --- a/onadata/apps/api/viewsets/xform_viewset.py +++ b/onadata/apps/api/viewsets/xform_viewset.py @@ -25,21 +25,22 @@ from onadata.libs import filters from onadata.libs.exceptions import NoRecordsFoundError from onadata.libs.mixins.anonymous_user_public_forms_mixin import ( - AnonymousUserPublicFormsMixin) + AnonymousUserPublicFormsMixin +) from onadata.libs.mixins.labels_mixin import LabelsMixin from onadata.libs.renderers import renderers from onadata.libs.serializers.xform_serializer import XFormSerializer from onadata.libs.utils import log from onadata.libs.utils.common_tags import SUBMISSION_TIME from onadata.libs.utils.csv_import import submit_csv -from onadata.libs.utils.export_tools import generate_export, \ - should_create_new_export +from onadata.libs.utils.export_tools import ( + generate_export, + should_create_new_export, +) from onadata.libs.utils.export_tools import newset_export_for from onadata.libs.utils.logger_tools import response_with_mimetype_and_name -from onadata.libs.utils.string import str2bool from onadata.libs.utils.storage import rmdir -from onadata.libs.utils.viewer_tools import _get_form_url -from onadata.libs.utils.viewer_tools import enketo_url, EnketoError +from onadata.libs.utils.string import str2bool EXPORT_EXT = { 'xls': Export.XLS_EXPORT, @@ -620,25 +621,6 @@ def create(self, request, *args, **kwargs): return Response(survey, status=status.HTTP_400_BAD_REQUEST) - @action(detail=True, methods=['GET']) - def enketo(self, request, **kwargs): - self.object = self.get_object() - form_url = _get_form_url(self.object.user.username) - - data = {'message': t("Enketo not properly configured.")} - http_status = status.HTTP_400_BAD_REQUEST - - try: - url = enketo_url(form_url, self.object.id_string) - except EnketoError: - pass - else: - if url: - http_status = status.HTTP_200_OK - data = {"enketo_url": url} - - return Response(data, http_status) - def get_queryset(self): if isinstance(self.request.user, ServiceAccountUser): # We need to get all xforms (even soft-deleted ones) to diff --git a/onadata/apps/logger/tests/test_briefcase_client.py b/onadata/apps/logger/tests/test_briefcase_client.py index 262cb3cdf..c9a09baa2 100644 --- a/onadata/apps/logger/tests/test_briefcase_client.py +++ b/onadata/apps/logger/tests/test_briefcase_client.py @@ -1,11 +1,11 @@ # coding: utf-8 import os.path -from io import StringIO, BytesIO +from io import BytesIO from urllib.parse import urljoin import requests from django.contrib.auth import authenticate -from django.core.files.storage import get_storage_class +from django.core.files.storage import default_storage as storage from django.core.files.uploadedfile import UploadedFile from django.urls import reverse from django.test import RequestFactory @@ -17,12 +17,10 @@ from onadata.apps.logger.views import download_xform from onadata.apps.main.models import MetaData from onadata.apps.main.tests.test_base import TestBase -from onadata.apps.main.views import profile, download_media_data +from onadata.apps.main.views import download_media_data from onadata.libs.utils.briefcase_client import BriefcaseClient from onadata.libs.utils.storage import rmdir -storage = get_storage_class()() - def formList(*args, **kwargs): # noqa view = XFormListApi.as_view({'get': 'list'}) @@ -114,7 +112,7 @@ def setUp(self): self.assertEqual(MetaData.objects.count(), count + 1) url = urljoin( self.base_url, - reverse(profile, kwargs={'username': self.user.username}) + reverse('user_profile', kwargs={'username': self.user.username}) ) self._logout() self._create_user_and_login('deno', 'deno') diff --git a/onadata/apps/logger/views.py b/onadata/apps/logger/views.py index a30a5c8d7..8ba5f3360 100644 --- a/onadata/apps/logger/views.py +++ b/onadata/apps/logger/views.py @@ -8,11 +8,12 @@ from django.contrib.auth.decorators import login_required, user_passes_test from django.contrib.auth.models import User from django.contrib import messages -from django.core.files.storage import get_storage_class +from django.core.files.storage import default_storage from django.http import ( HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, + HttpResponseNotFound, HttpResponseRedirect, StreamingHttpResponse, Http404, @@ -185,7 +186,6 @@ def download_xlsform(request, username, id_string): return HttpResponseForbidden('Not shared.') file_path = xform.xls.name - default_storage = get_storage_class()() if file_path != '' and default_storage.exists(file_path): audit = { @@ -220,12 +220,9 @@ def download_xlsform(request, username, id_string): return response else: - messages.add_message(request, messages.WARNING, - t('No XLS file for your form ' - '%(id)s') - % {'id': id_string}) - - return HttpResponseRedirect("/%s" % username) + return HttpResponseNotFound( + t('No XLS file for your form %(id)s') % {'id': id_string} + ) def download_jsonform(request, username, id_string): @@ -282,7 +279,6 @@ def retrieve_superuser_stats(request, username, base_filename): 'superuser_stats', base_filename ) - default_storage = get_storage_class()() if not default_storage.exists(filename): raise Http404 with default_storage.open(filename) as f: diff --git a/onadata/apps/main/templates/form_gallery.html b/onadata/apps/main/templates/form_gallery.html deleted file mode 100644 index dbc21bb76..000000000 --- a/onadata/apps/main/templates/form_gallery.html +++ /dev/null @@ -1,78 +0,0 @@ -{% extends 'base.html' %} - -{% block content %} - -{% load i18n %} - -
    -
    -

    {% blocktrans %}Form Gallery Shared Forms{% endblocktrans %}

    -
    -
     
    - - - - - - - - - - - - {% for data_dictionary in shared_forms %} - - - - - - - {% endfor %} - -
    {% trans "User" %}{% trans "Name" %}{% trans "Description" %}{% trans "XLSForm" %}
    - gravatar {{ data_dictionary.user.username }} - {{ data_dictionary.title }} - {{ data_dictionary.description }} - {% if data_dictionary.xls %} - {% trans "Excel" %} - {% if not data_dictionary.id_string in cloned %} - {% if loggedin_user and data_dictionary.xls|length > 0 and loggedin_user.username != data_dictionary.user.username %} - {% trans "Clone" %} - {% endif %} - {% endif %} - {% endif %} -
    - -
    -{% endblock %} - -{% block javascript %} -{{ block.super }} - - - - - -{% endblock %} diff --git a/onadata/apps/main/templates/form_photos.html b/onadata/apps/main/templates/form_photos.html deleted file mode 100644 index b18405852..000000000 --- a/onadata/apps/main/templates/form_photos.html +++ /dev/null @@ -1,65 +0,0 @@ -{% extends 'base.html' %} - -{% block additional-headers %} - - - -{% endblock %} - -{% block body %} -{% load i18n %} -{% load l10n %} - - {% include "topbar.html" %} - {% if images|length %} - - {% else %} -
    - {% trans "There are no photos in this project." %} -
    - {% endif %} - -{% block javascript %} -{{ block.super }} -{% if images|length %} - - - -{% endif %} -{% endblock %} - -{% endblock %} diff --git a/onadata/apps/main/tests/test_form_api.py b/onadata/apps/main/tests/test_form_api.py deleted file mode 100644 index 7f22f7117..000000000 --- a/onadata/apps/main/tests/test_form_api.py +++ /dev/null @@ -1,174 +0,0 @@ -# coding: utf-8 -import json - -from django.urls import reverse - -from onadata.apps.main.views import api -from onadata.apps.api.mongo_helper import MongoHelper -from onadata.apps.viewer.models.parsed_instance import ParsedInstance -from onadata.libs.utils.string import base64_encodestring -from .test_base import TestBase - - -def dict_for_mongo_without_userform_id(parsed_instance): - d = parsed_instance.to_dict_for_mongo() - # remove _userform_id since its not returned by the API - d.pop(ParsedInstance.USERFORM_ID) - return d - - -class TestFormAPI(TestBase): - - def setUp(self): - TestBase.setUp(self) - self._create_user_and_login() - self._publish_transportation_form_and_submit_instance() - self.api_url = reverse(api, kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string - }) - - def test_api(self): - # query string - response = self.client.get(self.api_url, {}) - self.assertEqual(response.status_code, 200) - d = dict_for_mongo_without_userform_id( - self.xform.instances.all()[0].parsed_instance) - find_d = json.loads(response.content)[0] - self.assertEqual(find_d, d) - - def test_api_with_query(self): - # query string - query = '{"transport/available_transportation_types_to_referral_facil'\ - 'ity":"none"}' - data = {'query': query} - response = self.client.get(self.api_url, data) - self.assertEqual(response.status_code, 200) - d = dict_for_mongo_without_userform_id( - self.xform.instances.all()[0].parsed_instance) - find_d = json.loads(response.content)[0] - self.assertEqual(find_d, d) - - def test_api_query_no_records(self): - # query string - query = '{"available_transporation_types_to_referral_facility": "bicy'\ - 'cle"}' - data = {'query': query} - response = self.client.get(self.api_url, data) - self.assertEqual(response.status_code, 200) - self.assertEqual(response.content.decode(), '[]') - - def test_handle_bad_json(self): - response = self.client.get(self.api_url, {'query': 'bad'}) - self.assertEqual(response.status_code, 400) - self.assertEqual(True, 'Expecting value' in response.content.decode()) - - def test_api_jsonp(self): - # query string - callback = 'jsonpCallback' - response = self.client.get(self.api_url, {'callback': callback}) - self.assertEqual(response.status_code, 200) - self.assertEqual(response.content.decode().startswith(callback + '('), True) - self.assertEqual(response.content.decode().endswith(')'), True) - start = callback.__len__() + 1 - end = response.content.__len__() - 1 - content = response.content[start: end] - d = dict_for_mongo_without_userform_id( - self.xform.instances.all()[0].parsed_instance) - find_d = json.loads(content)[0] - self.assertEqual(find_d, d) - - def test_api_with_query_start_limit(self): - # query string - query = '{"transport/available_transportation_types_to_referral_facil'\ - 'ity":"none"}' - data = {'query': query, 'start': 0, 'limit': 10} - response = self.client.get(self.api_url, data) - self.assertEqual(response.status_code, 200) - d = dict_for_mongo_without_userform_id( - self.xform.instances.all()[0].parsed_instance) - find_d = json.loads(response.content)[0] - self.assertEqual(find_d, d) - - def test_api_with_query_invalid_start_limit(self): - # query string - query = '{"transport/available_transportation_types_to_referral_facil'\ - 'ity":"none"}' - data = {'query': query, 'start': -100, 'limit': -100} - response = self.client.get(self.api_url, data) - self.assertEqual(response.status_code, 400) - - def test_api_count(self): - # query string - query = '{"transport/available_transportation_types_to_referral_facil'\ - 'ity":"none"}' - data = {'query': query, 'count': 1} - response = self.client.get(self.api_url, data) - self.assertEqual(response.status_code, 200) - find_d = json.loads(response.content)[0] - self.assertTrue('count' in find_d) - self.assertEqual(find_d.get('count'), 1) - - def test_api_column_select(self): - # query string - query = '{"transport/available_transportation_types_to_referral_facil'\ - 'ity":"none"}' - columns = '["transport/available_transportation_types_to_referral_fac'\ - 'ility"]' - data = {'query': query, 'fields': columns} - response = self.client.get(self.api_url, data) - self.assertEqual(response.status_code, 200) - find_d = json.loads(response.content)[0] - self.assertTrue( - 'transport/available_transportation_types_to_referral_facility' in - find_d) - self.assertFalse('_attachments' in find_d) - - def test_api_decode_from_mongo(self): - field = "$section1.group01.question1" - encoded = MongoHelper.encode(field) - self.assertEqual(encoded, ( - "%(dollar)ssection1%(dot)sgroup01%(dot)squestion1" % { - "dollar": base64_encodestring("$").strip(), - "dot": base64_encodestring(".").strip()})) - decoded = MongoHelper.decode(encoded) - self.assertEqual(field, decoded) - - def test_api_with_or_query(self): - """Test that an or query is interpreted correctly since we use an - internal or query to filter out deleted records""" - for i in range(1, 3): - self._submit_transport_instance(i) - # record 0: does NOT have the 'transport/loop_over_transport_types_freq - # uency/ambulance/frequency_to_referral_facility' field - # record 1: 'transport/loop_over_transport_types_frequency/ambulance/fr - # equency_to_referral_facility': 'daily' - # record 2: 'transport/loop_over_transport_types_frequency/ambulance/fr - # equency_to_referral_facility': 'weekly' - params = { - 'query': - '{"$or": [{"transport/loop_over_transport_types_frequency/ambulanc' - 'e/frequency_to_referral_facility": "weekly"}, {"transport/loop_ov' - 'er_transport_types_frequency/ambulance/frequency_to_referral_faci' - 'lity": "daily"}]}'} - response = self.client.get(self.api_url, params) - self.assertEqual(response.status_code, 200) - data = json.loads(response.content) - self.assertEqual(len(data), 2) - - # check that blank params give us all our records i.e. 3 - params = {} - response = self.client.get(self.api_url, params) - self.assertEqual(response.status_code, 200) - data = json.loads(response.content) - self.assertEqual(len(data), 3) - - def test_api_cors_options(self): - response = self.anon.options(self.api_url) - allowed_headers = ['Accept', 'Origin', 'X-Requested-With', - 'Authorization'] - provided_headers = [h.strip() for h in response[ - 'Access-Control-Allow-Headers'].split(',')] - self.assertListEqual(allowed_headers, provided_headers) - self.assertEqual(response['Access-Control-Allow-Methods'], 'GET') - self.assertEqual(response['Access-Control-Allow-Origin'], '*') diff --git a/onadata/apps/main/tests/test_form_auth.py b/onadata/apps/main/tests/test_form_auth.py index 0704c0183..a767afbc3 100644 --- a/onadata/apps/main/tests/test_form_auth.py +++ b/onadata/apps/main/tests/test_form_auth.py @@ -1,7 +1,6 @@ # coding: utf-8 from django.urls import reverse -from onadata.apps.main.views import login_redirect from .test_base import TestBase @@ -10,6 +9,15 @@ class TestFormAuth(TestBase): def setUp(self): TestBase.setUp(self) - def test_login_redirect_redirects(self): - response = self.client.get(reverse(login_redirect)) + def test_home_redirects(self): + self._create_user_and_login(username='bob', password='bob') + response = self.client.get(reverse('home')) self.assertEqual(response.status_code, 302) + print("response['Location']", response['Location'], flush=True) + + def test_profile_redirects(self): + self._create_user_and_login(username='bob', password='bob') + url = reverse('user_profile', kwargs={'username': self.user.username}) + response = self.client.get(url) + self.assertEqual(response.status_code, 302) + print("response['Location']", response['Location'], flush=True) diff --git a/onadata/apps/main/tests/test_form_errors.py b/onadata/apps/main/tests/test_form_errors.py deleted file mode 100644 index f94f743cb..000000000 --- a/onadata/apps/main/tests/test_form_errors.py +++ /dev/null @@ -1,87 +0,0 @@ -# coding: utf-8 -import os -from unittest import skip - -from django.urls import reverse -from django.core.files.storage import get_storage_class - -from onadata.apps.main.views import show -from onadata.apps.logger.models import XForm -from .test_base import TestBase - - -class TestFormErrors(TestBase): - - def _create_xform(self): - self.xls_path = os.path.join( - self.this_directory, "fixtures", - "transportation", "transportation.xls") - response = self._publish_xls_file(self.xls_path) - self.assertEqual(response.status_code, 201) - self.xform = XForm.objects.all()[0] - - def test_bad_id_string(self): - self._create_user_and_login() - count = XForm.objects.count() - xls_path = os.path.join(self.this_directory, "fixtures", - "transportation", "transportation.bad_id.xls") - response = self._publish_xls_file(xls_path) - self.assertEqual(response.status_code, 400) - self.assertEqual(XForm.objects.count(), count) - - @skip - def test_dl_no_xls(self): - """ - Exports are built from the JSON form structure so we dont need the - xls to generate an export - """ - self._create_xform() - self.xform.shared_data = True - self.xform.save() - default_storage = get_storage_class()() - path = self.xform.xls.name - self.assertEqual(default_storage.exists(path), True) - default_storage.delete(path) - self.assertEqual(default_storage.exists(path), False) - url = reverse('xls_export', kwargs={'username': self.user.username, - 'id_string': self.xform.id_string}) - response = self.anon.get(url) - self.assertEqual(response.status_code, 404) - - def test_dl_xls_not_file(self): - self._create_xform() - self.xform.xls = "blah" - self.xform.save() - url = reverse('xls_export', kwargs={'username': self.user.username, - 'id_string': self.xform.id_string}) - response = self.anon.get(url) - self.assertEqual(response.status_code, 403) - - def test_nonexist_id_string(self): - url = reverse(show, kwargs={'username': self.user.username, - 'id_string': '4444'}) - response = self.client.get(url) - self.assertEqual(response.status_code, 404) - response = self.anon.get(url) - self.assertEqual(response.status_code, 404) - - def test_empty_submission(self): - xls_path = os.path.join( - self.this_directory, "fixtures", - "transportation", "transportation.xls") - xml_path = os.path.join( - self.this_directory, "fixtures", - "transportation", "transportation_empty_submission.xml") - self._publish_xls_file(xls_path) - self._make_submission(xml_path) - self.assertTrue(self.response.status_code, 400) - - def test_submission_deactivated(self): - self._create_xform() - self.xform.downloadable = False - self.xform.save() - xml_path = os.path.join( - self.this_directory, "fixtures", - "transportation", "transportation_empty_submission.xml") - self._make_submission(xml_path) - self.assertTrue(self.response.status_code, 400) diff --git a/onadata/apps/main/tests/test_form_exports.py b/onadata/apps/main/tests/test_form_exports.py index c4407be23..0efc6e6e3 100644 --- a/onadata/apps/main/tests/test_form_exports.py +++ b/onadata/apps/main/tests/test_form_exports.py @@ -2,7 +2,6 @@ import os import time import csv -import tempfile from io import BytesIO from django.urls import reverse diff --git a/onadata/apps/main/tests/test_form_metadata.py b/onadata/apps/main/tests/test_form_metadata.py deleted file mode 100644 index 4b29ab1a2..000000000 --- a/onadata/apps/main/tests/test_form_metadata.py +++ /dev/null @@ -1,126 +0,0 @@ -# coding: utf-8 -import os - -from django.core.files.base import File -from django.core.files.uploadedfile import InMemoryUploadedFile -from django.urls import reverse - -from onadata.apps.main.models import MetaData -from onadata.apps.main.views import ( - show, - edit, - download_media_data, -) -from onadata.libs.utils.hash import get_hash -from .test_base import TestBase - - -class TestFormMetadata(TestBase): - - def setUp(self): - TestBase.setUp(self) - self._create_user_and_login() - self._publish_transportation_form_and_submit_instance() - self.url = reverse(show, kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string - }) - self.edit_url = reverse(edit, kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string - }) - - def _add_metadata(self): - name = 'screenshot.png' - data_type = 'media' - path = os.path.join(self.this_directory, "fixtures", - "transportation", name) - with open(path, 'rb') as doc_file: - self.post_data = { - data_type: doc_file - } - self.client.post(self.edit_url, self.post_data) - - self.doc = MetaData.objects.filter(data_type=data_type).reverse()[0] - self.doc_url = reverse(download_media_data, kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string, - 'data_id': self.doc.id}) - - return name - - def test_adds_supporting_media_on_submit(self): - count = len(MetaData.objects.filter(xform=self.xform, - data_type='media')) - self._add_metadata() - self.assertEquals(count + 1, len(MetaData.objects.filter( - xform=self.xform, data_type='media'))) - - def test_download_supporting_media(self): - self._add_metadata() - response = self.client.get(self.doc_url) - self.assertEqual(response.status_code, 200) - fileName, ext = os.path.splitext(response['Content-Disposition']) - self.assertEqual(ext, '.png') - - def test_shared_download_supporting_media_for_anon(self): - self._add_metadata() - self.xform.shared = True - self.xform.save() - response = self.anon.get(self.doc_url) - self.assertEqual(response.status_code, 200) - - def test_delete_supporting_media(self): - count = MetaData.objects.filter( - xform=self.xform, data_type='media').count() - self._add_metadata() - self.assertEqual(MetaData.objects.filter( - xform=self.xform, data_type='media').count(), count + 1) - response = self.client.get(self.doc_url + '?del=true') - self.assertEqual(MetaData.objects.filter( - xform=self.xform, data_type='media').count(), count) - self.assertEqual(response.status_code, 302) - self._add_metadata() - response = self.anon.get(self.doc_url + '?del=true') - self.assertEqual(MetaData.objects.filter( - xform=self.xform, data_type='media').count(), count + 1) - self.assertEqual(response.status_code, 403) - - def test_media_file_hash(self): - name = "screenshot.png" - media_file = os.path.join( - self.this_directory, 'fixtures', 'transportation', name) - m = MetaData.objects.create( - data_type='media', xform=self.xform, data_value=name, - data_file=File(open(media_file, 'rb'), name), - data_file_type='image/png') - with open(media_file, 'rb') as f: - media_hash = get_hash(f, prefix=True) - meta_hash = m.md5_hash - self.assertEqual(meta_hash, media_hash) - self.assertEqual(m.file_hash, media_hash) - - def test_add_media_url(self): - uri = 'https://devtrac.ona.io/fieldtrips.csv' - count = MetaData.objects.filter(data_type='media').count() - self.client.post(self.edit_url, {'media_url': uri}) - self.assertEqual(count + 1, - len(MetaData.objects.filter(data_type='media'))) - - def test_windows_csv_file_upload(self): - count = MetaData.objects.filter(data_type='media').count() - media_file = os.path.join( - self.this_directory, 'fixtures', 'transportation', - 'transportation.csv') - f = InMemoryUploadedFile(open(media_file, 'rb'), - 'media', - 'transportation.csv', - 'application/octet-stream', - 2625, - None) - MetaData.media_upload(self.xform, f) - media_list = MetaData.objects.filter(data_type='media') - new_count = media_list.count() - self.assertEqual(count + 1, new_count) - media = media_list.get(data_value='transportation.csv') - self.assertEqual(media.data_file_type, 'text/csv') diff --git a/onadata/apps/main/tests/test_form_show.py b/onadata/apps/main/tests/test_form_show.py index 2ec6d6bc5..28e06a521 100644 --- a/onadata/apps/main/tests/test_form_show.py +++ b/onadata/apps/main/tests/test_form_show.py @@ -1,17 +1,16 @@ # coding: utf-8 import os -from unittest import skip from django.core.files.base import ContentFile from django.urls import reverse from onadata import koboform -from onadata.apps.main.views import show, form_photos, \ - show_form_settings from onadata.apps.logger.models import XForm -from onadata.apps.logger.views import download_xlsform, download_jsonform, \ - download_xform -from onadata.apps.viewer.views import export_list +from onadata.apps.logger.views import ( + download_xlsform, + download_jsonform, + download_xform, +) from onadata.libs.utils.logger_tools import publish_xml_form from onadata.libs.utils.user_auth import http_auth_string from .test_base import TestBase @@ -23,30 +22,6 @@ def setUp(self): TestBase.setUp(self) self._create_user_and_login() self._publish_transportation_form() - self.url = reverse(show, kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string - }) - - def test_show_form_name(self): - response = self.client.get(self.url) - self.assertEqual(response.status_code, 200) - self.assertContains(response, self.xform.id_string) - - def test_hide_from_anon(self): - response = self.anon.get(self.url) - self.assertEqual(response.status_code, 302) - - def test_hide_from_not_user(self): - self._create_user_and_login("jo") - response = self.client.get(self.url) - self.assertEqual(response.status_code, 302) - - def test_show_to_anon_if_public(self): - self.xform.shared = True - self.xform.save() - response = self.anon.get(self.url) - self.assertEqual(response.status_code, 200) def test_dl_xlsx_xlsform(self): self._publish_xlsx_file() @@ -160,93 +135,19 @@ def test_dl_xform_for_authenticated_non_owner(self): })) self.assertEqual(response.status_code, 200) - def test_show_link_if_shared_and_data(self): - self.xform.shared = True - self.xform.shared_data = True - self.xform.save() - self._submit_transport_instance() - response = self.anon.get(self.url) - self.assertContains(response, reverse(export_list, kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string, - 'export_type': 'csv' - })) - - def test_show_link_if_owner(self): - self._submit_transport_instance() - response = self.client.get(self.url) - self.assertContains(response, reverse(export_list, kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string, - 'export_type': 'csv' - })) - self.assertContains(response, reverse(export_list, kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string, - 'export_type': 'xls' - })) - self.assertNotContains(response, 'GPS Points') - - # check that a form with geopoints has the map url - response = self._publish_xls_file( - os.path.join( - os.path.dirname(__file__), "fixtures", "gps", "gps.xls")) - self.assertEqual(response.status_code, 201) - self.xform = XForm.objects.latest('date_created') - - show_url = reverse(show, kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string - }) - - response = self.client.get(show_url) - # check that map url doesnt show before we have submissions - self.assertNotContains(response, 'GPS Points') - - # make a submission - self._make_submission( - os.path.join( - os.path.dirname(__file__), "fixtures", "gps", "instances", - "gps_1980-01-23_20-52-08.xml") - ) - self.assertEqual(self.response.status_code, 201) - # get new show view - response = self.client.get(show_url) - self.assertContains(response, 'GPS Points') - - def test_anon_no_toggle_data_share_btn(self): - self.xform.shared = True - self.xform.save() - response = self.anon.get(self.url) - self.assertNotContains(response, 'PUBLIC    ') - self.assertNotContains(response, 'PRIVATE') - - def test_show_add_supporting_media_if_owner(self): - url = reverse(show_form_settings, kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string - }) - response = self.client.get(url) - self.assertContains(response, 'Media Upload') - - def test_load_photo_page(self): - response = self.client.get(reverse(form_photos, kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string})) - self.assertEqual(response.status_code, 200) - - def test_load_from_uuid(self): - self.xform = XForm.objects.get(pk=self.xform.id) - response = self.client.get(reverse(show, kwargs={ - 'uuid': self.xform.uuid})) - self.assertEqual(response.status_code, 302) - self.assertEqual(response['Location'], self.url) - def test_publish_xml_xlsform_download(self): count = XForm.objects.count() path = os.path.join( - self.this_directory, '..', '..', 'api', 'tests', 'fixtures', - 'forms', 'contributions', 'contributions.xml') + self.this_directory, + '..', + '..', + 'api', + 'tests', + 'fixtures', + 'forms', + 'contributions', + 'contributions.xml', + ) f = open(path, 'rb') xml_file = ContentFile(f.read()) f.close() @@ -256,5 +157,7 @@ def test_publish_xml_xlsform_download(self): response = self.client.get(reverse(download_xlsform, kwargs={ 'username': self.user.username, 'id_string': 'contributions' - }), follow=True) - self.assertContains(response, 'No XLS file for your form ') + })) + self.assertContains( + response, 'No XLS file for your form ', status_code=404 + ) diff --git a/onadata/apps/main/tests/test_permissions.py b/onadata/apps/main/tests/test_permissions.py index ce4cdd384..35f4c7700 100644 --- a/onadata/apps/main/tests/test_permissions.py +++ b/onadata/apps/main/tests/test_permissions.py @@ -1,10 +1,8 @@ # coding: utf-8 import os -from django.urls import reverse from guardian.shortcuts import assign_perm, remove_perm -from onadata.apps.main.views import show from .test_base import TestBase @@ -19,10 +17,6 @@ def setUp(self): self.this_directory, 'fixtures', 'transportation', 'instances', s, s + '.xml')) self.submission = self.xform.instances.reverse()[0] - self.url = reverse(show, kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string - }) def test_set_permissions_for_user(self): # alice cannot view bob's forms @@ -47,12 +41,3 @@ def test_set_permissions_for_user(self): # Alice is the owner of `self.xform` self.assertEqual(self.user.has_perm('view_xform', self.xform), True) self.assertEqual(self.user.has_perm('change_xform', self.xform), True) - - def test_add_view_to_user(self): - self._create_user_and_login('alice', 'alice') - alice = self.user - response = self.client.get(self.url) - self.assertEqual(response.status_code, 302) - assign_perm('view_xform', alice, self.xform) - response = self.client.get(self.url) - self.assertEqual(response.status_code, 200) diff --git a/onadata/apps/main/tests/test_user_profile.py b/onadata/apps/main/tests/test_user_profile.py deleted file mode 100644 index d0bf67407..000000000 --- a/onadata/apps/main/tests/test_user_profile.py +++ /dev/null @@ -1,108 +0,0 @@ -# coding: utf-8 -import unittest - -from django.test import TestCase -from django.test.client import Client -from django.contrib.auth.models import User -from django.urls import reverse - -from onadata import koboform -from onadata.apps.main.views import profile - - -class TestUserProfile(TestCase): - - def setup(self): - self.client = Client() - self.assertEqual(len(User.objects.all()), 0) - - def _login_user_and_profile(self, extra_post_data={}): - post_data = { - 'username': 'bob', - 'email': 'bob@columbia.edu', - 'password1': 'bobbob', - 'password2': 'bobbob', - 'name': 'Bob', - 'city': 'Bobville', - 'country': 'US', - 'organization': 'Bob Inc.', - 'home_page': 'bob.com', - 'twitter': 'boberama' - } - url = '/accounts/register/' - post_data = dict(post_data.items() + extra_post_data.items()) - self.response = self.client.post(url, post_data) - try: - self.user = User.objects.get(username=post_data['username']) - except User.DoesNotExist: - pass - - @unittest.skip("User creation is deactivated on KC") - def test_create_user_with_given_name(self): - self._login_user_and_profile() - self.assertEqual(self.response.status_code, 302) - self.assertEqual(self.user.username, 'bob') - - @unittest.skip("User creation is deactivated on KC") - def test_create_user_profile_for_user(self): - self._login_user_and_profile() - self.assertEqual(self.response.status_code, 302) - user_profile = self.user.profile - self.assertEqual(user_profile.city, 'Bobville') - self.assertTrue(hasattr(user_profile, 'metadata')) - - @unittest.skip("User creation is deactivated on KC") - def test_disallow_non_alpha_numeric(self): - invalid_usernames = [ - 'b ob', - 'b.o.b.', - 'b-ob', - 'b!', - '@bob', - 'bob@bob.com', - 'bob$', - 'b&o&b', - 'bob?', - '#bob', - '(bob)', - 'b*ob', - '%s % bob', - ] - users_before = User.objects.count() - for username in invalid_usernames: - self._login_user_and_profile({'username': username}) - self.assertEqual(User.objects.count(), users_before) - - @unittest.skip("User creation is deactivated on KC") - def test_disallow_reserved_name(self): - users_before = User.objects.count() - self._login_user_and_profile({'username': 'admin'}) - self.assertEqual(User.objects.count(), users_before) - - def test_redirect_to_login_if_user_does_not_exist(self): - response = self.client.get(reverse(profile, - kwargs={'username': 'nonuser'})) - self.assertEqual(response.status_code, 302) - login_url = reverse('login') - if koboform.active and koboform.autoredirect: - redirect_to = koboform.login_url() - else: - redirect_to = login_url - self.assertEqual(response.url, redirect_to) - - @unittest.skip("We don't use twitter in kobocat tests") - def test_show_single_at_sign_in_twitter_link(self): - self._login_user_and_profile() - response = self.client.get( - reverse(profile, kwargs={ - 'username': "bob" - })) - self.assertContains(response, ">@boberama") - # add the @ sign - self.user.profile.twitter = "@boberama" - self.user.profile.save() - response = self.client.get( - reverse(profile, kwargs={ - 'username': "bob" - })) - self.assertContains(response, ">@boberama") diff --git a/onadata/apps/main/urls.py b/onadata/apps/main/urls.py index 2aee889f5..9d85af931 100644 --- a/onadata/apps/main/urls.py +++ b/onadata/apps/main/urls.py @@ -2,34 +2,19 @@ from django.conf import settings from django.contrib import admin -from django.urls import include, path, re_path +from django.urls import include, re_path from django.views.generic import RedirectView from django.views.i18n import JavaScriptCatalog +from onadata import koboform from onadata.apps.api.urls import BriefcaseApi from onadata.apps.api.urls import XFormListApi from onadata.apps.api.urls import XFormSubmissionApi from onadata.apps.api.urls import router, router_with_patch_list from onadata.apps.main.service_health import service_health from onadata.apps.main.views import ( - # main website views - home, - login_redirect, - profile, - api_token, - # form specific - show, - api, - edit, - form_photos, - delete_metadata, - download_metadata, download_media_data, - show_form_settings, - - # views that now exist only in KPI - make_kpi_data_redirect_view, ) # exporting stuff @@ -66,24 +51,21 @@ re_path('^api/v1/', include(router.urls)), re_path('^api/v1/', include(router_with_patch_list.urls)), re_path(r'^service_health/$', service_health), - re_path(r'^api-docs/', RedirectView.as_view(url='/api/v1/')), re_path(r'^api/', RedirectView.as_view(url='/api/v1/')), re_path(r'^api/v1', RedirectView.as_view(url='/api/v1/')), # django default stuff re_path(r'^accounts/', include('django.contrib.auth.urls')), re_path(r'^admin/', admin.site.urls), - re_path(r'^admin/doc/', include('django.contrib.admindocs.urls')), # oath2_provider re_path(r'^o/', include('oauth2_provider.urls', namespace='oauth2_provider')), # main website views - re_path(r'^$', home), - re_path(r'^forms/(?P[^/]+)$', show, name='show_form'), - re_path(r'^login_redirect/$', login_redirect), + re_path( + r'^$', RedirectView.as_view(url=koboform.redirect_url('/')), name='home' + ), # Bring back old url because it's still used by `kpi` - # ToDo Remove when `kpi#gallery-2` is merged into master re_path(r"^attachment/$", attachment_url, name='attachment_url'), re_path(r"^attachment/(?P[^/]+)$", attachment_url, name='attachment_url'), @@ -92,42 +74,16 @@ attachment_url, name='attachment_url'), re_path(r'^jsi18n/$', JavaScriptCatalog.as_view(packages=['onadata.apps.main', 'onadata.apps.viewer']), name='javascript-catalog'), - re_path(r'^(?P[^/]+)/$', - profile, name='user_profile'), - re_path(r'^(?P[^/]+)/api-token$', - api_token, name='api_token'), - - # form specific - re_path(r'^(?P[^/]+)/forms/(?P[^/]+)$', - show, name='show_form'), - re_path(r'^(?P[^/]+)/forms/(?P[^/]+)/api$', - api, name='mongo_view_api'), - re_path(r'^(?P[^/]+)/forms/(?P[^/]+)/edit$', - edit, name='edit_form'), - re_path(r'^(?P[^/]+)/forms/(?P[^/]+)/photos', - form_photos, name='form_photos'), - re_path(r'^(?P[^/]+)/forms/(?P[^/]+)/doc/(?P\d+)', download_metadata, name='download_metadata'), - re_path(r'^(?P[^/]+)/forms/(?P[^/]+)/delete-doc/(?P<' - 'data_id>\d+)', delete_metadata, name='delete_metadata'), - re_path(r'^(?P[^/]+)/forms/(?P[^/]+)/formid-media/' - r'(?P\d+)', download_media_data, name='download_media_data'), - re_path(r'^(?P[^/]+)/forms/(?P[^/]+)/form_settings$', - show_form_settings, name='show_form_settings'), - path( - '/reports//export.html', - make_kpi_data_redirect_view('table'), - name='redirect_view_data_in_table_to_kpi', + re_path( + r'^(?P[^/]+)/$', + RedirectView.as_view(url=koboform.redirect_url('/')), + name='user_profile', ), - path( - '/reports//digest.html', - make_kpi_data_redirect_view('report'), - name='redirect_analyze_data_to_kpi', - ), - path( - '/forms//map', - make_kpi_data_redirect_view('map'), - name='redirect_map_to_kpi', + # form specific + re_path( + r'^(?P[^/]+)/forms/(?P[^/]+)/formid-media/(?P\d+)', + download_media_data, + name='download_media_data' ), # briefcase api urls diff --git a/onadata/apps/main/views.py b/onadata/apps/main/views.py index 3f18597b1..c206ebfa8 100644 --- a/onadata/apps/main/views.py +++ b/onadata/apps/main/views.py @@ -2,358 +2,24 @@ import os import requests -from bson import json_util from django.conf import settings from django.contrib.auth.decorators import login_required -from django.contrib.auth.models import User -from django.contrib.contenttypes.models import ContentType from django.core.files.storage import get_storage_class from django.urls import reverse -from django.http import HttpResponse -from django.http import HttpResponseBadRequest from django.http import HttpResponseForbidden from django.http import HttpResponseNotFound from django.http import HttpResponseRedirect from django.http import HttpResponseServerError from django.shortcuts import get_object_or_404 -from django.shortcuts import render from django.utils.translation import gettext as t -from django.views.decorators.http import require_GET -from django.views.decorators.http import require_http_methods -from rest_framework.authtoken.models import Token -from rest_framework import status -from ssrf_protect.ssrf_protect import SSRFProtect, SSRFProtectException from onadata.apps.logger.models import XForm -from onadata.apps.main.forms import ( - MediaForm, -) -from onadata.apps.main.forms import QuickConverterForm -from onadata.apps.main.models import UserProfile, MetaData -from onadata.apps.viewer.models.parsed_instance import ParsedInstance +from onadata.apps.main.models import MetaData from onadata.libs.utils.log import audit_log, Actions from onadata.libs.utils.logger_tools import ( check_submission_permissions, response_with_mimetype_and_name, ) -from onadata.libs.utils.user_auth import ( - add_cors_headers, - check_and_set_user_and_form, - get_xform_and_perms, - has_permission, - helper_auth_helper, - set_profile_data, -) - - -def home(request): - if request.user.username: - return HttpResponseRedirect( - reverse(profile, kwargs={'username': request.user.username})) - - return render(request, 'home.html') - - -@login_required -def login_redirect(request): - return HttpResponseRedirect(reverse(profile, - kwargs={'username': request.user.username})) - - -def profile(request, username): - content_user = get_object_or_404(User, username__iexact=username) - form = QuickConverterForm() - data = {'form': form} - - # If the "Sync forms" button is pressed in the UI, a call to KPI's - # `/migrate` endpoint is made to sync kobocat and KPI. - if request.GET.get('sync_xforms') == 'true': - migrate_response = _make_authenticated_request(request, content_user) - message = {} - if migrate_response.status_code == status.HTTP_200_OK: - message['text'] = t( - 'The migration process has started and may take several ' - 'minutes. Please check the project list in the ' - 'regular interface and ensure your projects have ' - 'synced.' - ).format(settings.KOBOFORM_URL) - else: - message['text'] = t( - 'Something went wrong trying to migrate your forms. Please try ' - 'again or reach out on the community forum ' - 'for assistance.' - ) - - data['message'] = message - - # profile view... - # for the same user -> dashboard - if content_user == request.user: - show_dashboard = True - all_forms = content_user.xforms.count() - - request_url = request.build_absolute_uri( - "/%s" % request.user.username) - url = request_url.replace('http://', 'https://') - xforms = XForm.objects.filter(user=content_user)\ - .select_related('user', 'instances') - - user_xforms = xforms - # forms shared with user - xfct = ContentType.objects.get(app_label='logger', model='xform') - xfs = content_user.userobjectpermission_set.filter(content_type=xfct) - shared_forms_pks = list(set([xf.object_pk for xf in xfs])) - forms_shared_with = XForm.objects.filter( - pk__in=shared_forms_pks).exclude(user=content_user)\ - .select_related('user') - # all forms to which the user has access - published_or_shared = XForm.objects.filter( - pk__in=shared_forms_pks).select_related('user') - xforms_list = [ - { - 'id': 'published', - 'xforms': user_xforms, - 'title': t("Published Forms"), - 'small': t("Export, map, and view submissions.") - }, - { - 'id': 'shared', - 'xforms': forms_shared_with, - 'title': t("Shared Forms"), - 'small': t("List of forms shared with you.") - }, - { - 'id': 'published_or_shared', - 'xforms': published_or_shared, - 'title': t("Published Forms"), - 'small': t("Export, map, and view submissions.") - } - ] - data.update({ - 'all_forms': all_forms, - 'show_dashboard': show_dashboard, - 'form': form, - 'url': url, - 'user_xforms': user_xforms, - 'xforms_list': xforms_list, - 'forms_shared_with': forms_shared_with - }) - # for any other user -> profile - set_profile_data(data, content_user) - - return render(request, "profile.html", data) - - -def redirect_to_public_link(request, uuid): - xform = get_object_or_404(XForm, uuid=uuid) - request.session['public_link'] = \ - xform.uuid if MetaData.public_link(xform) else False - - return HttpResponseRedirect(reverse(show, kwargs={ - 'username': xform.user.username, - 'id_string': xform.id_string - })) - - -@require_GET -def show(request, username=None, id_string=None, uuid=None): - if uuid: - return redirect_to_public_link(request, uuid) - - xform, is_owner, can_edit, can_view, can_delete_data = get_xform_and_perms( - username, id_string, request) - # no access - if not (xform.shared or can_view or request.session.get('public_link')): - return HttpResponseRedirect(reverse(home)) - - data = {} - data['cloned'] = len( - XForm.objects.filter(user__username__iexact=request.user.username, - id_string__exact=id_string + XForm.CLONED_SUFFIX) - ) > 0 - data['public_link'] = MetaData.public_link(xform) - data['is_owner'] = is_owner - data['can_edit'] = can_edit - data['can_view'] = can_view or request.session.get('public_link') - data['can_delete_data'] = can_delete_data - data['xform'] = xform - data['content_user'] = xform.user - data['base_url'] = "https://%s" % request.get_host() - data['supporting_docs'] = MetaData.supporting_docs(xform) - data['media_upload'] = MetaData.media_upload(xform) - - if is_owner: - data['media_form'] = MediaForm() - - if xform.kpi_asset_uid: - data['kpi_url'] = ( - f'{settings.KOBOFORM_URL}/#/forms/{xform.kpi_asset_uid}' - ) - - return render(request, "show.html", data) - - -# SETTINGS SCREEN FOR KPI, LOADED IN IFRAME -@require_GET -def show_form_settings(request, username=None, id_string=None, uuid=None): - if uuid: - return redirect_to_public_link(request, uuid) - - xform, is_owner, can_edit, can_view, can_delete_data = get_xform_and_perms( - username, id_string, request) - # no access - if not (xform.shared or can_view or request.session.get('public_link')): - return HttpResponseRedirect(reverse(home)) - - data = {} - data['cloned'] = len( - XForm.objects.filter(user__username__iexact=request.user.username, - id_string__exact=id_string + XForm.CLONED_SUFFIX) - ) > 0 - data['public_link'] = MetaData.public_link(xform) - data['is_owner'] = is_owner - data['can_edit'] = can_edit - data['can_view'] = can_view or request.session.get('public_link') - data['can_delete_data'] = can_delete_data - data['xform'] = xform - data['content_user'] = xform.user - data['base_url'] = "https://%s" % request.get_host() - data['source'] = MetaData.source(xform) - data['media_upload'] = MetaData.media_upload(xform) - # https://html.spec.whatwg.org/multipage/input.html#attr-input-accept - # e.g. .csv,.xml,text/csv,text/xml - media_upload_types = [] - for supported_type in settings.SUPPORTED_MEDIA_UPLOAD_TYPES: - extension = '.{}'.format(supported_type.split('/')[-1]) - media_upload_types.append(extension) - media_upload_types.append(supported_type) - data['media_upload_types'] = ','.join(media_upload_types) - - if is_owner: - data['media_form'] = MediaForm() - - return render(request, "show_form_settings.html", data) - - -@require_http_methods(["GET", "OPTIONS"]) -def api(request, username=None, id_string=None): - """ - Returns all results as JSON. If a parameter string is passed, - it takes the 'query' parameter, converts this string to a dictionary, an - that is then used as a MongoDB query string. - - NOTE: only a specific set of operators are allow, currently $or and $and. - Please send a request if you'd like another operator to be enabled. - - NOTE: Your query must be valid JSON, double check it here, - http://json.parser.online.fr/ - - E.g. api?query='{"last_name": "Smith"}' - """ - if request.method == "OPTIONS": - response = HttpResponse() - add_cors_headers(response) - - return response - helper_auth_helper(request) - helper_auth_helper(request) - xform, owner = check_and_set_user_and_form(username, id_string, request) - - if not xform: - return HttpResponseForbidden(t('Not shared.')) - - try: - args = { - 'username': username, - 'id_string': id_string, - 'query': request.GET.get('query'), - 'fields': request.GET.get('fields'), - 'sort': request.GET.get('sort') - } - if 'start' in request.GET: - args["start"] = int(request.GET.get('start')) - if 'limit' in request.GET: - args["limit"] = int(request.GET.get('limit')) - if 'count' in request.GET: - args["count"] = True if int(request.GET.get('count')) > 0\ - else False - cursor = ParsedInstance.query_mongo(**args) - except ValueError as e: - return HttpResponseBadRequest(e.__str__()) - - records = list(record for record in cursor) - response_text = json_util.dumps(records) - - if 'callback' in request.GET and request.GET.get('callback') != '': - callback = request.GET.get('callback') - response_text = ("%s(%s)" % (callback, response_text)) - - response = HttpResponse(response_text, content_type='application/json') - add_cors_headers(response) - - return response - - -@login_required -@require_GET -def api_token(request, username=None): - if request.user.username == username: - user = get_object_or_404(User, username=username) - data = {} - data['token_key'], created = Token.objects.get_or_create(user=user) - - return render(request, "api_token.html", data) - - return HttpResponseForbidden(t('Permission denied.')) - - -# ToDo Remove when `form-media` features is released in KPI -@login_required -def edit(request, username, id_string): - xform = XForm.objects.get(user__username__iexact=username, - id_string__exact=id_string) - owner = xform.user - - if username == request.user.username or\ - request.user.has_perm('logger.change_xform', xform): - - if request.POST.get('media_url'): - uri = request.POST.get('media_url') - try: - SSRFProtect.validate(uri) - except SSRFProtectException: - return HttpResponseForbidden( - t('URL {uri} is forbidden.').format(uri=uri) - ) - MetaData.media_add_uri(xform, uri) - elif request.FILES.get('media'): - audit = { - 'xform': xform.id_string - } - audit_log( - Actions.FORM_UPDATED, request.user, owner, - t("Media added to '%(id_string)s'.") % - { - 'id_string': xform.id_string - }, audit, request) - for aFile in request.FILES.getlist("media"): - MetaData.media_upload(xform, aFile) - - xform.update() - - if request.is_ajax(): - return HttpResponse(t('Updated succeeded.')) - else: - if 'HTTP_REFERER' in request.META and request.META['HTTP_REFERER'].strip(): - return HttpResponseRedirect(request.META['HTTP_REFERER']) - - return HttpResponseRedirect(reverse(show, kwargs={ - 'username': username, - 'id_string': id_string - })) - - return HttpResponseForbidden(t('Update failed.')) def download_media_data(request, username, id_string, data_id): @@ -382,8 +48,11 @@ def download_media_data(request, username, id_string, data_id): 'id_string': xform.id_string, 'filename': os.path.basename(data.data_file.name) }, audit, request) - if 'HTTP_REFERER' in request.META and request.META['HTTP_REFERER'].strip(): - return HttpResponseRedirect(request.META['HTTP_REFERER']) + if ( + 'HTTP_REFERER' in request.META + and request.META['HTTP_REFERER'].strip() + ): + return HttpResponseRedirect(request.META['HTTP_REFERER']) return HttpResponseRedirect(reverse(show, kwargs={ 'username': username, @@ -426,117 +95,6 @@ def download_media_data(request, username, id_string, data_id): return HttpResponseForbidden(t('Permission denied.')) -def download_metadata(request, username, id_string, data_id): - xform = get_object_or_404(XForm, - user__username__iexact=username, - id_string__exact=id_string) - owner = xform.user - # FIXME: couldn't non-owner users be allowed to access these files even - # without the form being entirely public (shared=True)? - if username == request.user.username or xform.shared: - data = get_object_or_404(MetaData, pk=data_id) - file_path = data.data_file.name - filename, extension = os.path.splitext(file_path.split('/')[-1]) - extension = extension.strip('.') - dfs = get_storage_class()() - if dfs.exists(file_path): - audit = { - 'xform': xform.id_string - } - audit_log( - Actions.FORM_UPDATED, request.user, owner, - t("Document '%(filename)s' for '%(id_string)s' downloaded.") % - { - 'id_string': xform.id_string, - 'filename': "%s.%s" % (filename, extension) - }, audit, request) - response = response_with_mimetype_and_name( - data.data_file_type, - filename, extension=extension, show_date=False, - file_path=file_path) - return response - else: - return HttpResponseNotFound() - - return HttpResponseForbidden(t('Permission denied.')) - - -@login_required() -def delete_metadata(request, username, id_string, data_id): - xform = get_object_or_404(XForm, - user__username__iexact=username, - id_string__exact=id_string) - owner = xform.user - data = get_object_or_404(MetaData, pk=data_id) - dfs = get_storage_class()() - req_username = request.user.username - if request.GET.get('del', False) and username == req_username: - try: - dfs.delete(data.data_file.name) - data.delete() - audit = { - 'xform': xform.id_string - } - audit_log( - Actions.FORM_UPDATED, request.user, owner, - t("Document '%(filename)s' deleted from '%(id_string)s'.") % - { - 'id_string': xform.id_string, - 'filename': os.path.basename(data.data_file.name) - }, audit, request) - return HttpResponseRedirect(reverse(show, kwargs={ - 'username': username, - 'id_string': id_string - })) - except Exception: - return HttpResponseServerError() - - -def form_photos(request, username, id_string): - GALLERY_IMAGE_COUNT_LIMIT = 2500 - GALLERY_THUMBNAIL_CHUNK_SIZE = 25 - GALLERY_THUMBNAIL_CHUNK_DELAY = 5000 # ms - - xform, owner = check_and_set_user_and_form(username, id_string, request) - - if not xform: - return HttpResponseForbidden(t('Not shared.')) - - data = {} - data['form_view'] = True - data['content_user'] = owner - data['xform'] = xform - image_urls = [] - too_many_images = False - - # Show the most recent images first - for instance in xform.instances.all().order_by('-pk'): - attachments = instance.attachments.all() - # If we have to truncate, don't include a partial instance - if len(image_urls) + attachments.count() > GALLERY_IMAGE_COUNT_LIMIT: - too_many_images = True - break - for attachment in attachments: - # skip if not image e.g video or file - if not attachment.mimetype.startswith('image'): - continue - - data = {} - data['original'] = attachment.secure_url() - for suffix in settings.THUMB_CONF.keys(): - data[suffix] = attachment.secure_url(suffix) - - image_urls.append(data) - - data['images'] = image_urls - data['too_many_images'] = too_many_images - data['thumbnail_chunk_size'] = GALLERY_THUMBNAIL_CHUNK_SIZE - data['thumbnail_chunk_delay'] = GALLERY_THUMBNAIL_CHUNK_DELAY - data['profilei'], created = UserProfile.objects.get_or_create(user=owner) - - return render(request, 'form_photos.html', data) - - def _make_authenticated_request(request, user): """ Make an authenticated request to KPI using the current session. @@ -552,20 +110,3 @@ def _get_migrate_url(username): return '{kf_url}/api/v2/users/{username}/migrate/'.format( kf_url=settings.KOBOFORM_URL, username=username ) - - -def make_kpi_data_redirect_view(kpi_data_route): - def view_func(request, username, id_string): - owner = get_object_or_404(User, username__iexact=username) - xform = get_object_or_404(XForm, id_string__exact=id_string, user=owner) - if not has_permission(xform, owner, request): - return HttpResponseForbidden(t('Not shared.')) - data = {'xform': xform} - if xform.kpi_asset_uid: - data['kpi_url'] = ( - f'{settings.KOBOFORM_URL}/#/forms/{xform.kpi_asset_uid}/data/' - f'{kpi_data_route}' - ) - return render(request, 'outdated_data_view.html', data) - - return view_func diff --git a/onadata/apps/viewer/templates/export_list.html b/onadata/apps/viewer/templates/export_list.html index 39c0f3f84..4518bbb24 100644 --- a/onadata/apps/viewer/templates/export_list.html +++ b/onadata/apps/viewer/templates/export_list.html @@ -6,7 +6,7 @@
    -

    {{ export_type_name|upper }} {% blocktrans %}Exports{% endblocktrans %} ({{ xform.id_string }})

    +

    {{ export_type_name|upper }} {% blocktrans %}Exports{% endblocktrans %}

    {% if user.is_authenticated %} diff --git a/onadata/koboform/redirect_middleware.py b/onadata/koboform/redirect_middleware.py index ab58506b4..03fa4882c 100644 --- a/onadata/koboform/redirect_middleware.py +++ b/onadata/koboform/redirect_middleware.py @@ -6,18 +6,10 @@ from onadata import koboform from onadata.libs.utils.user_auth import helper_auth_helper -# Always redirect to user profile, -# regardless of koboform setting -DISABLED_VIEWS = [ - 'home', -] - # if user not logged in: # if kform server exists: redirect to kform login url # else: redirect to kc login url REDIRECT_IF_NOT_LOGGED_IN = [ - 'profile', - 'home', 'download_xlsform', ] @@ -46,17 +38,7 @@ def process_view(self, request, view, args, kwargs): koboform.redirect_url('/accounts/logout/') ) - if view_name in DISABLED_VIEWS: - if is_logged_in: - redirect_to = reverse( - 'user_profile', kwargs={'username': request.user.username} - ) - else: - if koboform.active and koboform.autoredirect: - redirect_to = koboform.login_url(next_kobocat_url=request.path) - else: - redirect_to = login_url - return HttpResponseRedirect(redirect_to) - elif not is_logged_in and (view_name in REDIRECT_IF_NOT_LOGGED_IN): + if not is_logged_in and (view_name in REDIRECT_IF_NOT_LOGGED_IN): return HttpResponseRedirect(login_url) + pass diff --git a/onadata/libs/templates/change_language.html b/onadata/libs/templates/change_language.html deleted file mode 100644 index 7840cddf1..000000000 --- a/onadata/libs/templates/change_language.html +++ /dev/null @@ -1,12 +0,0 @@ -{% load i18n %} -
    -
    - {% csrf_token %} - - -
    -
    diff --git a/onadata/libs/templates/home_topbar.html b/onadata/libs/templates/home_topbar.html deleted file mode 100644 index 4daa8bae4..000000000 --- a/onadata/libs/templates/home_topbar.html +++ /dev/null @@ -1,21 +0,0 @@ -{% load i18n %} - -
    -
    -
    -
    - onadata -
    -
    -
    - -
    -
    - -
    -
    -
    diff --git a/onadata/settings/base.py b/onadata/settings/base.py index 49f6a3e1e..2ba2a6d45 100644 --- a/onadata/settings/base.py +++ b/onadata/settings/base.py @@ -92,17 +92,10 @@ def skip_suspicious_operations(record): # Example: "http://media.lawrence.com/static/" STATIC_URL = '/static/' -# Login URLs -LOGIN_URL = '/accounts/login/' -LOGIN_REDIRECT_URL = '/login_redirect/' - - if os.environ.get('KOBOCAT_ROOT_URI_PREFIX'): KOBOCAT_ROOT_URI_PREFIX = '/' + os.environ['KOBOCAT_ROOT_URI_PREFIX'].strip('/') + '/' MEDIA_URL = KOBOCAT_ROOT_URI_PREFIX + MEDIA_URL.lstrip('/') STATIC_URL = KOBOCAT_ROOT_URI_PREFIX + STATIC_URL.lstrip('/') - LOGIN_URL = KOBOCAT_ROOT_URI_PREFIX + LOGIN_URL.lstrip('/') - LOGIN_REDIRECT_URL = KOBOCAT_ROOT_URI_PREFIX + LOGIN_REDIRECT_URL.lstrip('/') MEDIA_ROOT = os.path.join(PROJECT_ROOT, MEDIA_URL.lstrip('/')) From 61a14312d09ea3c2c7b6406e476a353c24c7999f Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Tue, 11 Jul 2023 15:39:28 -0400 Subject: [PATCH 03/23] Remove superuser stats reports --- onadata/apps/logger/views.py | 39 ------------------------------------ onadata/apps/main/urls.py | 14 ------------- 2 files changed, 53 deletions(-) diff --git a/onadata/apps/logger/views.py b/onadata/apps/logger/views.py index 8ba5f3360..7139e9b68 100644 --- a/onadata/apps/logger/views.py +++ b/onadata/apps/logger/views.py @@ -247,42 +247,3 @@ def download_jsonform(request, username, id_string): add_cors_headers(response) response.content = xform.json return response - - -@user_passes_test(lambda u: u.is_superuser) -def superuser_stats(request, username): - base_filename = '{}_{}_{}.zip'.format( - re.sub('[^a-zA-Z0-9]', '-', request.get_host()), - date.today(), - datetime.now().microsecond - ) - filename = os.path.join( - request.user.username, - 'superuser_stats', - base_filename - ) - generate_stats_zip.delay(filename) - template_ish = ( - 'Hello, superuser.' - 'Your report is being generated. Once finished, it will be ' - 'available at {0}. If you receive a 404, please ' - 'refresh your browser periodically until your request succeeds.' - '' - ).format(base_filename) - return HttpResponse(template_ish) - - -@user_passes_test(lambda u: u.is_superuser) -def retrieve_superuser_stats(request, username, base_filename): - filename = os.path.join( - request.user.username, - 'superuser_stats', - base_filename - ) - if not default_storage.exists(filename): - raise Http404 - with default_storage.open(filename) as f: - response = StreamingHttpResponse(f, content_type='application/zip') - response['Content-Disposition'] = 'attachment;filename="{}"'.format( - base_filename) - return response diff --git a/onadata/apps/main/urls.py b/onadata/apps/main/urls.py index 9d85af931..e998899fd 100644 --- a/onadata/apps/main/urls.py +++ b/onadata/apps/main/urls.py @@ -36,13 +36,6 @@ download_jsonform, ) -# Statistics for superusers. The username is irrelevant, but leave it as -# the first part of the path to avoid collisions -from onadata.apps.logger.views import ( - superuser_stats, - retrieve_superuser_stats -) - admin.autodiscover() urlpatterns = [ @@ -166,11 +159,4 @@ name="download_jsonform"), re_path(r'^favicon\.ico', RedirectView.as_view(url='/static/images/favicon.ico')), - - # Statistics for superusers. The username is irrelevant, but leave it as - # the first part of the path to avoid collisions - re_path(r"^(?P[^/]+)/superuser_stats/$", - superuser_stats), - re_path(r"^(?P[^/]+)/superuser_stats/(?P[^/]+)$", - retrieve_superuser_stats), ] From bff1db2228f6218b6e2876a1823b77839aa29947 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Thu, 13 Jul 2023 11:22:27 -0400 Subject: [PATCH 04/23] Allow only OpenRosa endpoints with untrusted passwords --- .../templates/restricted_access.html | 12 ++ onadata/koboform/redirect_middleware.py | 4 +- onadata/libs/http/__init__.py | 1 + onadata/libs/http/response.py | 21 +++ onadata/libs/middleware.py | 141 ++++++++++++++++++ onadata/settings/base.py | 1 + 6 files changed, 178 insertions(+), 2 deletions(-) create mode 100644 kobocat-template/templates/restricted_access.html create mode 100644 onadata/libs/http/__init__.py create mode 100644 onadata/libs/http/response.py create mode 100644 onadata/libs/middleware.py diff --git a/kobocat-template/templates/restricted_access.html b/kobocat-template/templates/restricted_access.html new file mode 100644 index 000000000..3d3660fcc --- /dev/null +++ b/kobocat-template/templates/restricted_access.html @@ -0,0 +1,12 @@ +{% extends 'base.html' %} +{% load i18n %} +{% block content %} +

    + {% trans "403 - Restricted access" %} +

    + + {% blocktrans %} + Your access is restricted. Please reclaim your access by changing your password. + {% endblocktrans %} + +{% endblock %} diff --git a/onadata/koboform/redirect_middleware.py b/onadata/koboform/redirect_middleware.py index 03fa4882c..bc431779a 100644 --- a/onadata/koboform/redirect_middleware.py +++ b/onadata/koboform/redirect_middleware.py @@ -21,7 +21,7 @@ class ConditionalRedirects(MiddlewareMixin): - def process_view(self, request, view, args, kwargs): + def process_view(self, request, view, view_args, view_kwargs): view_name = view.__name__ if request.user.is_anonymous: helper_auth_helper(request) @@ -41,4 +41,4 @@ def process_view(self, request, view, args, kwargs): if not is_logged_in and (view_name in REDIRECT_IF_NOT_LOGGED_IN): return HttpResponseRedirect(login_url) - pass + return diff --git a/onadata/libs/http/__init__.py b/onadata/libs/http/__init__.py new file mode 100644 index 000000000..7194a7aab --- /dev/null +++ b/onadata/libs/http/__init__.py @@ -0,0 +1 @@ +from .response import JsonResponseForbidden, XMLResponseForbidden diff --git a/onadata/libs/http/response.py b/onadata/libs/http/response.py new file mode 100644 index 000000000..95974001a --- /dev/null +++ b/onadata/libs/http/response.py @@ -0,0 +1,21 @@ +from django.http import HttpResponseForbidden, JsonResponse +from rest_framework import status + +from onadata.libs.renderers.renderers import TemplateXMLRenderer + + +class JsonResponseForbidden(JsonResponse): + + status_code = status.HTTP_403_FORBIDDEN + + +class XMLResponseForbidden(HttpResponseForbidden): + + def __init__(self, data, **kwargs): + xml_renderer = TemplateXMLRenderer() + kwargs.setdefault('content_type', 'application/xml') + renderer_context = kwargs.pop('renderer_context') + data = xml_renderer.render( + data=data, renderer_context=renderer_context + ), + super().__init__(content=data, **kwargs) diff --git a/onadata/libs/middleware.py b/onadata/libs/middleware.py new file mode 100644 index 000000000..488a363c9 --- /dev/null +++ b/onadata/libs/middleware.py @@ -0,0 +1,141 @@ +from typing import Union + +from django.conf import settings +from django.contrib.auth import get_user_model +from django.http import HttpResponseForbidden +from django.template.loader import get_template +from django.utils.deprecation import MiddlewareMixin +from django.utils.translation import gettext as t + +from onadata.libs.http import JsonResponseForbidden, XMLResponseForbidden + + +ALLOWED_VIEWS_WITH_WEAK_PASSWORD = { + 'XFormListApi': { + 'actions': { + 'GET': ['manifest', 'media', 'list', 'retrieve'], + } + }, + 'XFormSubmissionApi': { + 'actions': { + 'POST': ['create'], + } + }, + 'RedirectView': { + 'view_initkwargs': [ + '/static/images/favicon.ico' + ] + }, + # Allow media files + 'MetaDataViewSet': { + 'actions': { + 'GET': ['retrieve', 'list'] + } + }, +} + + +class RestrictedAccessMiddleware(MiddlewareMixin): + + def __init__(self, get_response): + super().__init__(get_response) + self._allowed_view = True + + def process_response(self, request, response): + if not request.user.is_authenticated: + return + + try: + profile = request.user.profile + except get_user_model().profile.RelatedObjectDoesNotExist: + # Consider user's password as weak + return self._render_response(response) + + if profile.validated_password: + return + + if not self._allowed_view: + return self._render_response(response) + + return response + + def process_view(self, request, view, view_args, view_kwargs): + """ + Validate if view is among allowed one with unsafe password. + If it is not, set `self._allowed_view` to False to alter the + response in `process_response()`. + + We cannot validate user's password here because DRF authentication + takes places after this method call. Thus, `request.user` is always + anonymous if user is authenticated with something else than the session. + """ + view_name = view.__name__ + + if hasattr(view, 'actions'): + # Allow HEAD requests all the time + if request.method == 'HEAD': + return + + try: + allowed_actions = ALLOWED_VIEWS_WITH_WEAK_PASSWORD[view_name][ + 'actions' + ][request.method] + except KeyError: + self._allowed_view = False + return + + view_action = view.actions[request.method.lower()] + if view_action not in allowed_actions: + self._allowed_view = False + return + + if hasattr(view, 'view_initkwargs'): + try: + allowed_urls = ALLOWED_VIEWS_WITH_WEAK_PASSWORD[view_name][ + 'view_initkwargs' + ] + except KeyError: + self._allowed_view = False + return + + url = view.view_initkwargs['url'] + if url not in allowed_urls: + self._allowed_view = False + return + + return + + def _render_response( + self, response + ) -> Union[ + HttpResponseForbidden, JsonResponseForbidden, XMLResponseForbidden + ]: + """ + Render response in the requested format: HTML, JSON or XML. + If content type is not detected, fallback on HTML. + """ + template = get_template('restricted_access.html') + format_ = None + try: + content_type, *_ = response.accepted_media_type.split(';') + except AttributeError: + pass + else: + *_, format_ = content_type.split('/') + + if format_ not in ['xml', 'json']: + return HttpResponseForbidden(template.render()) + else: + data = { + 'detail': t( + f'Your access is restricted. Please reclaim your access by ' + f'changing your password at ' + f'{settings.KOBOFORM_URL}/accounts/password/reset/.' + ) + } + if format_ == 'json': + return JsonResponseForbidden(data) + else: + return XMLResponseForbidden( + data, renderer_context=response.renderer_context + ) diff --git a/onadata/settings/base.py b/onadata/settings/base.py index 2ba2a6d45..e791f0b4b 100644 --- a/onadata/settings/base.py +++ b/onadata/settings/base.py @@ -122,6 +122,7 @@ def skip_suspicious_operations(record): 'django.middleware.csrf.CsrfViewMiddleware', 'corsheaders.middleware.CorsMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', + 'onadata.libs.middleware.RestrictedAccessMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'onadata.libs.utils.middleware.HTTPResponseNotAllowedMiddleware', 'readonly.middleware.DatabaseReadOnlyMiddleware', From 083f43c9362f1110f6e599db9f281c9a6b8fd564 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Thu, 13 Jul 2023 13:18:29 -0400 Subject: [PATCH 05/23] Bugfix - return response when password is valid --- onadata/libs/middleware.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/onadata/libs/middleware.py b/onadata/libs/middleware.py index 488a363c9..398599c7e 100644 --- a/onadata/libs/middleware.py +++ b/onadata/libs/middleware.py @@ -43,7 +43,7 @@ def __init__(self, get_response): def process_response(self, request, response): if not request.user.is_authenticated: - return + return response try: profile = request.user.profile @@ -52,7 +52,7 @@ def process_response(self, request, response): return self._render_response(response) if profile.validated_password: - return + return response if not self._allowed_view: return self._render_response(response) From 212b3a10331a572d3b79e542943a2a40ea342b4c Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Thu, 13 Jul 2023 16:33:06 -0400 Subject: [PATCH 06/23] Allow ServiceAccountUser to update user's profile on their behalf --- onadata/apps/api/viewsets/connect_viewset.py | 12 ++++++++++-- onadata/libs/middleware.py | 18 ++++++++++-------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/onadata/apps/api/viewsets/connect_viewset.py b/onadata/apps/api/viewsets/connect_viewset.py index 2e54f0d20..bcf273ea9 100644 --- a/onadata/apps/api/viewsets/connect_viewset.py +++ b/onadata/apps/api/viewsets/connect_viewset.py @@ -1,4 +1,6 @@ # coding: utf-8 +from kobo_service_account.models import ServiceAccountUser +from kobo_service_account.utils import get_real_user from rest_framework import viewsets from rest_framework.response import Response @@ -53,8 +55,14 @@ def list(self, request, *args, **kwargs): # login(request, request.user) session.set_expiry(DEFAULT_SESSION_EXPIRY_TIME) + user = ( + get_real_user(request) + if isinstance(request.user, ServiceAccountUser) + else request.user + ) + serializer = UserProfileWithTokenSerializer( - instance=UserProfile.objects.get_or_create(user=request.user)[0], - context={"request": request}) + instance=UserProfile.objects.get_or_create(user=user)[0], + context={'request': request}) return Response(serializer.data) diff --git a/onadata/libs/middleware.py b/onadata/libs/middleware.py index 398599c7e..85b8b656a 100644 --- a/onadata/libs/middleware.py +++ b/onadata/libs/middleware.py @@ -6,6 +6,7 @@ from django.template.loader import get_template from django.utils.deprecation import MiddlewareMixin from django.utils.translation import gettext as t +from kobo_service_account.models import ServiceAccountUser from onadata.libs.http import JsonResponseForbidden, XMLResponseForbidden @@ -26,12 +27,6 @@ '/static/images/favicon.ico' ] }, - # Allow media files - 'MetaDataViewSet': { - 'actions': { - 'GET': ['retrieve', 'list'] - } - }, } @@ -39,17 +34,21 @@ class RestrictedAccessMiddleware(MiddlewareMixin): def __init__(self, get_response): super().__init__(get_response) - self._allowed_view = True + self._allowed_view = None def process_response(self, request, response): if not request.user.is_authenticated: return response + if isinstance(request.user, ServiceAccountUser): + return response + try: profile = request.user.profile except get_user_model().profile.RelatedObjectDoesNotExist: # Consider user's password as weak - return self._render_response(response) + if not self._allowed_view: + return self._render_response(response) if profile.validated_password: return response @@ -71,6 +70,9 @@ def process_view(self, request, view, view_args, view_kwargs): """ view_name = view.__name__ + # Reset boolean for each processed view + self._allowed_view = True + if hasattr(view, 'actions'): # Allow HEAD requests all the time if request.method == 'HEAD': From b9f1959993961cc8906e3ee53b98557127b92b03 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Thu, 13 Jul 2023 17:01:28 -0400 Subject: [PATCH 07/23] Add unit tests --- onadata/apps/api/tests/viewsets/test_user.py | 178 ++++++++++++++++++ .../viewsets/test_xform_submission_api.py | 1 + 2 files changed, 179 insertions(+) diff --git a/onadata/apps/api/tests/viewsets/test_user.py b/onadata/apps/api/tests/viewsets/test_user.py index ef6ba7d62..379d676be 100644 --- a/onadata/apps/api/tests/viewsets/test_user.py +++ b/onadata/apps/api/tests/viewsets/test_user.py @@ -1,4 +1,7 @@ # coding: utf-8 +import base64 +import os + import pytest from django.conf import settings from django.urls.exceptions import NoReverseMatch @@ -110,3 +113,178 @@ def test_service_account_can_delete_user(self): service_account_meta['HTTP_HOST'] = settings.TEST_HTTP_HOST response = self.client.delete(url, **service_account_meta) assert response.status_code == status.HTTP_204_NO_CONTENT + + def test_only_open_rosa_endpoints_allowed_with_not_validated_password(self): + # log in as bob + self._login_user_and_profile() + self.user.profile.require_auth = True + self.user.profile.validated_password = True + self.user.profile.save() + + # Password is valid, bob should be able to create a new form, submit data + # and browse the API + self.publish_xls_form() + self._submit_data() + assert self.response.status_code == status.HTTP_201_CREATED + # Validate bob is allowed to access all endpoints + self._access_endpoints(access_granted=True) + + # Flag Bob's password as not trusted + self.user.profile.validated_password = False + self.user.profile.save() + # Access denied to API endpoints with not validated password - Session auth + self._access_endpoints(access_granted=False) + # Still able to submit data + self._submit_data() + # We are sending a duplicate, we should receive a 202 if not blocked + assert self.response.status_code == status.HTTP_202_ACCEPTED + + # Access denied to API endpoints with not validated password - Basic auth + self.client.logout() + headers = { + 'HTTP_AUTHORIZATION': 'Basic ' + + base64.b64encode(b'bob:bobbob').decode('ascii') + } + self._access_endpoints(access_granted=False, headers=headers) + # Still able to submit data + self._submit_data() + # We are sending a duplicate, we should receive a 202 if not blocked + assert self.response.status_code == status.HTTP_202_ACCEPTED + + # Access denied to API endpoints with not validated password - Token auth + headers = { + 'HTTP_AUTHORIZATION': f'Token {self.user.auth_token}' + } + self._access_endpoints(access_granted=False, headers=headers) + # Still able to submit data + self._submit_data() + # We are sending a duplicate, we should receive a 202 if not blocked + assert self.response.status_code == status.HTTP_202_ACCEPTED + + def _access_endpoints(self, access_granted: bool, headers: dict = {}): + """ + Validate if `GET` requests return expected status code. + + The list of endpoints is not exhaustive but should cover the main ones. + TODO: Support other methods + """ + status_code = status.HTTP_200_OK if access_granted else status.HTTP_403_FORBIDDEN + xform_id = self.xform.instances.all()[0].pk + instance_id = self.xform.instances.all()[0].pk + attachment_id = self.xform.instances.all()[0].attachments.all()[0].pk + + # /api/v1/forms + response = self.client.get(reverse('xform-list'), **headers) + assert response.status_code == status_code + + response = self.client.get( + reverse('xform-detail', kwargs={'pk': xform_id}), **headers + ) + assert response.status_code == status_code + + response = self.client.get( + reverse('xform-form', kwargs={'pk': xform_id}), **headers + ) + assert response.status_code == status_code + + response = self.client.get( + reverse('xform-labels', kwargs={'pk': xform_id}), **headers + ) + assert response.status_code == status_code + + # /api/v1/data + response = self.client.get(reverse('data-list'), **headers) + assert response.status_code == status_code + + response = self.client.get( + reverse( + 'data-detail', kwargs={'pk': xform_id, 'dataid': instance_id} + ), + **headers, + ) + assert response.status_code == status_code + + response = self.client.get( + reverse( + 'data-validation-status', + kwargs={'pk': xform_id, 'dataid': instance_id}, + ), + **headers, + ) + assert response.status_code == status_code + + # /api/v1/media + response = self.client.get(reverse('attachment-list'), **headers) + assert response.status_code == status_code + + response = self.client.get( + reverse('attachment-detail', kwargs={'pk': attachment_id}), + **headers, + ) + assert response.status_code == status_code + + # /api/v1/metadata + response = self.client.get(reverse('metadata-list'), **headers) + assert response.status_code == status_code + + # TODO add media file to xform + # response = self.client.get(reverse('metadata-list', kwargs={'pk': metadata_id}), **headers)) + # assert response.status_code == status_code + + # /api/v1/user + response = self.client.get(reverse('userprofile-list'), **headers) + assert response.status_code == status_code + + ######################################################### + # OpenRosa endpoints. Should be granted no matter what. # + ######################################################### + # Xforms list + response = self.client.get(reverse('form-list'), **headers) + assert response.status_code == status.HTTP_200_OK + + response = self.client.get( + reverse('form-list', kwargs={'username': 'bob'}), **headers + ) + assert response.status_code == status.HTTP_200_OK + + # XForm manifest + response = self.client.get( + reverse('manifest-url', kwargs={'pk': xform_id}), **headers + ) + assert response.status_code == status.HTTP_200_OK + + response = self.client.get( + reverse('manifest-url', kwargs={'pk': xform_id, 'username': 'bob'}), + **headers, + ) + assert response.status_code == status.HTTP_200_OK + + # XForm XML + response = self.client.get( + reverse( + 'download_xform', kwargs={'pk': xform_id, 'username': 'bob'} + ), + **headers, + ) + assert response.status_code == status.HTTP_200_OK + + def _submit_data(self): + survey_datetime = self.surveys[0] + xml_path = os.path.join( + self.main_directory, + 'fixtures', + 'transportation', + 'instances', + survey_datetime, + f'{survey_datetime}.xml', + ) + media_file_path = os.path.join( + self.main_directory, + 'fixtures', + 'transportation', + 'instances', + survey_datetime, + '1335783522563.jpg' + ) + with open(media_file_path, 'rb') as media_file: + self._make_submission(xml_path, media_file=media_file) diff --git a/onadata/apps/api/tests/viewsets/test_xform_submission_api.py b/onadata/apps/api/tests/viewsets/test_xform_submission_api.py index 7317d8d5f..ac7155131 100644 --- a/onadata/apps/api/tests/viewsets/test_xform_submission_api.py +++ b/onadata/apps/api/tests/viewsets/test_xform_submission_api.py @@ -441,6 +441,7 @@ def test_edit_submission_with_service_account(self): self.assertEqual( response['Location'], 'http://testserver/submission' ) + def test_submission_blocking_flag(self): # Set 'submissions_suspended' True in the profile metadata to test if # submission do fail with the flag set From 8f6d6f63fa5f944ff9f23f7cd4e213bc533f52f2 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Thu, 13 Jul 2023 17:38:30 -0400 Subject: [PATCH 08/23] Simplify rules for restricted middleware --- onadata/libs/middleware.py | 65 +++++++++++++------------------------- 1 file changed, 22 insertions(+), 43 deletions(-) diff --git a/onadata/libs/middleware.py b/onadata/libs/middleware.py index 85b8b656a..1d32748d8 100644 --- a/onadata/libs/middleware.py +++ b/onadata/libs/middleware.py @@ -10,22 +10,18 @@ from onadata.libs.http import JsonResponseForbidden, XMLResponseForbidden - +# Define views (and viewsets) below. +# Viewset actions must specify (as a list) for each method. +# Legacy Django views do not need a list of actions, it can be empty. +# E.g. : Allow exports +# > 'export_list': { 'GET': [] } +# > 'create_export': { 'POST': [] } ALLOWED_VIEWS_WITH_WEAK_PASSWORD = { 'XFormListApi': { - 'actions': { - 'GET': ['manifest', 'media', 'list', 'retrieve'], - } + 'GET': ['manifest', 'media', 'list', 'retrieve'], }, 'XFormSubmissionApi': { - 'actions': { - 'POST': ['create'], - } - }, - 'RedirectView': { - 'view_initkwargs': [ - '/static/images/favicon.ico' - ] + 'POST': ['create'], }, } @@ -73,37 +69,20 @@ def process_view(self, request, view, view_args, view_kwargs): # Reset boolean for each processed view self._allowed_view = True - if hasattr(view, 'actions'): - # Allow HEAD requests all the time - if request.method == 'HEAD': - return - - try: - allowed_actions = ALLOWED_VIEWS_WITH_WEAK_PASSWORD[view_name][ - 'actions' - ][request.method] - except KeyError: - self._allowed_view = False - return - - view_action = view.actions[request.method.lower()] - if view_action not in allowed_actions: - self._allowed_view = False - return - - if hasattr(view, 'view_initkwargs'): - try: - allowed_urls = ALLOWED_VIEWS_WITH_WEAK_PASSWORD[view_name][ - 'view_initkwargs' - ] - except KeyError: - self._allowed_view = False - return - - url = view.view_initkwargs['url'] - if url not in allowed_urls: - self._allowed_view = False - return + if request.method == 'HEAD': + return + + try: + allowed_actions = ALLOWED_VIEWS_WITH_WEAK_PASSWORD[view_name][ + request.method + ] + except KeyError: + self._allowed_view = False + else: + if hasattr(view, 'actions'): + view_action = view.actions[request.method.lower()] + if view_action not in allowed_actions: + self._allowed_view = False return From 04a26150701f089f16ea5b00467bc8ad8db085fb Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Thu, 13 Jul 2023 17:38:40 -0400 Subject: [PATCH 09/23] Add exports to unit tests --- onadata/apps/api/tests/viewsets/test_user.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/onadata/apps/api/tests/viewsets/test_user.py b/onadata/apps/api/tests/viewsets/test_user.py index 379d676be..c731e2113 100644 --- a/onadata/apps/api/tests/viewsets/test_user.py +++ b/onadata/apps/api/tests/viewsets/test_user.py @@ -235,6 +235,21 @@ def _access_endpoints(self, access_granted: bool, headers: dict = {}): response = self.client.get(reverse('userprofile-list'), **headers) assert response.status_code == status_code + # exports (old views). Test only csv. + # //exports/// + response = self.client.get( + reverse( + 'export_list', + kwargs={ + 'username': 'bob', + 'id_string': self.xform.id_string, + 'export_type': 'csv', + }, + ), + **headers, + ) + assert response.status_code == status_code + ######################################################### # OpenRosa endpoints. Should be granted no matter what. # ######################################################### From e469573e25d1a72e5ee6450b0c4878af5696a19f Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Fri, 14 Jul 2023 12:41:03 -0400 Subject: [PATCH 10/23] Remove synchronous data exports --- onadata/apps/api/viewsets/xform_viewset.py | 3 +- onadata/apps/main/tests/test_base.py | 2 +- onadata/apps/main/tests/test_csv_export.py | 4 - onadata/apps/main/tests/test_form_exports.py | 145 +--------- onadata/apps/main/tests/test_process.py | 45 --- onadata/apps/main/urls.py | 10 - onadata/apps/viewer/tests/test_export_list.py | 52 ---- onadata/apps/viewer/tests/test_exports.py | 258 ++++-------------- onadata/apps/viewer/tests/test_kml_export.py | 46 ---- .../viewer/tests/test_pandas_mongo_bridge.py | 37 --- onadata/apps/viewer/views.py | 136 +-------- onadata/libs/utils/export_tools.py | 26 +- onadata/libs/utils/viewer_tools.py | 7 + 13 files changed, 83 insertions(+), 688 deletions(-) delete mode 100644 onadata/apps/viewer/tests/test_kml_export.py diff --git a/onadata/apps/api/viewsets/xform_viewset.py b/onadata/apps/api/viewsets/xform_viewset.py index f36fcec65..d1405b701 100644 --- a/onadata/apps/api/viewsets/xform_viewset.py +++ b/onadata/apps/api/viewsets/xform_viewset.py @@ -41,6 +41,7 @@ from onadata.libs.utils.logger_tools import response_with_mimetype_and_name from onadata.libs.utils.storage import rmdir from onadata.libs.utils.string import str2bool +from onadata.libs.utils.viewer_tools import format_date_for_mongo EXPORT_EXT = { 'xls': Export.XLS_EXPORT, @@ -71,8 +72,6 @@ def _get_extension_from_export_type(export_type): def _set_start_end_params(request, query): - format_date_for_mongo = lambda x, datetime: datetime.strptime( - x, '%y_%m_%d_%H_%M_%S').strftime('%Y-%m-%dT%H:%M:%S') # check for start and end params if 'start' in request.GET or 'end' in request.GET: diff --git a/onadata/apps/main/tests/test_base.py b/onadata/apps/main/tests/test_base.py index 65fba0493..8db035980 100644 --- a/onadata/apps/main/tests/test_base.py +++ b/onadata/apps/main/tests/test_base.py @@ -188,7 +188,7 @@ def _get_authenticated_client( def _get_response_content(self, response): contents = '' - if response.streaming: + if getattr(response, 'streaming', None): actual_content = BytesIO() for content in response.streaming_content: actual_content.write(content) diff --git a/onadata/apps/main/tests/test_csv_export.py b/onadata/apps/main/tests/test_csv_export.py index 2801b7818..68377c197 100644 --- a/onadata/apps/main/tests/test_csv_export.py +++ b/onadata/apps/main/tests/test_csv_export.py @@ -18,10 +18,6 @@ def setUp(self): self.this_directory, 'fixtures', 'csv_export') self._submission_time = parse_datetime('2013-02-18 15:54:01Z') - def test_csv_export_url(self): - """TODO: test data csv export""" - pass - def test_csv_export_output(self): path = os.path.join(self.fixture_dir, 'tutorial_w_repeats.xls') self._publish_xls_file_and_set_xform(path) diff --git a/onadata/apps/main/tests/test_form_exports.py b/onadata/apps/main/tests/test_form_exports.py index 0efc6e6e3..3abb1cb44 100644 --- a/onadata/apps/main/tests/test_form_exports.py +++ b/onadata/apps/main/tests/test_form_exports.py @@ -10,7 +10,7 @@ from openpyxl import load_workbook from onadata.apps.viewer.models.export import Export -from onadata.apps.viewer.views import kml_export, export_download +from onadata.apps.viewer.views import export_download from onadata.libs.utils.export_tools import generate_export from onadata.libs.utils.user_auth import http_auth_string from .test_base import TestBase @@ -22,12 +22,6 @@ def setUp(self): TestBase.setUp(self) self._create_user_and_login() self._publish_transportation_form_and_submit_instance() - self.csv_url = reverse('csv_export', kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string}) - self.xls_url = reverse('xls_export', kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string}) def _num_rows(self, content, export_format): def xls_rows(f): @@ -42,143 +36,6 @@ def csv_rows(f): } return num_rows_fn[export_format](content) - def test_csv_raw_export_name(self): - response = self.client.get(self.csv_url + '?raw=1') - self.assertEqual(response.status_code, 200) - self.assertEqual(response['Content-Disposition'], 'attachment;') - - def _filter_export_test(self, url, export_format): - """ - Test filter exports. Use sleep to ensure we don't have unique seconds. - Number of rows equals number of surveys plus 1, the header row. - """ - time.sleep(1) - # 1 survey exists before this time - start_time = timezone.now().strftime('%y_%m_%d_%H_%M_%S') - time.sleep(1) - s = self.surveys[1] - self._make_submission( - os.path.join(self.this_directory, 'fixtures', - 'transportation', 'instances', s, s + '.xml')) - time.sleep(1) - # 2 surveys exist before this time - end_time = timezone.now().strftime('%y_%m_%d_%H_%M_%S') - time.sleep(1) - # 3 surveys exist in total - s = self.surveys[2] - self._make_submission( - os.path.join(self.this_directory, 'fixtures', - 'transportation', 'instances', s, s + '.xml')) - # test restricting to before end time - params = {'end': end_time} - response = self.client.get(url, params) - self.assertEqual(response.status_code, 200) - content = self._get_response_content(response) - self.assertEqual(self._num_rows(content, export_format), 3) - # test restricting to after start time, thus excluding the initial - # submission - params = {'start': start_time} - response = self.client.get(url, params) - self.assertEqual(response.status_code, 200) - content = self._get_response_content(response) - self.assertEqual(self._num_rows(content, export_format), 3) - # test no time restriction - response = self.client.get(url) - self.assertEqual(response.status_code, 200) - content = self._get_response_content(response) - self.assertEqual(self._num_rows(content, export_format), 4) - # test restricting to between start time and end time - params = {'start': start_time, 'end': end_time} - response = self.client.get(url, params) - self.assertEqual(response.status_code, 200) - content = self._get_response_content(response) - self.assertEqual(self._num_rows(content, export_format), 2) - - def test_filter_by_date_csv(self): - self._filter_export_test(self.csv_url, 'csv') - - def test_filter_by_date_xls(self): - self._filter_export_test(self.xls_url, 'xls') - - def test_restrict_csv_export_if_not_shared(self): - response = self.anon.get(self.csv_url) - self.assertEqual(response.status_code, 403) - - def test_xls_raw_export_name(self): - response = self.client.get(self.xls_url + '?raw=1') - self.assertEqual(response.status_code, 200) - self.assertEqual(response['Content-Disposition'], 'attachment;') - - def test_restrict_xls_export_if_not_shared(self): - response = self.anon.get(self.xls_url) - self.assertEqual(response.status_code, 403) - - def test_restrict_kml_export_if_not_shared(self): - url = reverse(kml_export, kwargs={'username': self.user.username, - 'id_string': self.xform.id_string}) - response = self.anon.get(url) - self.assertEqual(response.status_code, 403) - - def test_allow_csv_export_if_shared(self): - self.xform.shared_data = True - self.xform.save() - response = self.anon.get(self.csv_url) - self.assertEqual(response.status_code, 200) - - def test_allow_xls_export_if_shared(self): - self.xform.shared_data = True - self.xform.save() - response = self.anon.get(self.xls_url) - self.assertEqual(response.status_code, 200) - - def test_allow_kml_export_if_shared(self): - self.xform.shared_data = True - self.xform.save() - url = reverse(kml_export, kwargs={'username': self.user.username, - 'id_string': self.xform.id_string}) - response = self.anon.get(url) - self.assertEqual(response.status_code, 200) - - def test_allow_csv_export(self): - response = self.client.get(self.csv_url) - self.assertEqual(response.status_code, 200) - - def test_allow_xls_export(self): - response = self.client.get(self.xls_url) - self.assertEqual(response.status_code, 200) - - def test_allow_kml_export(self): - url = reverse(kml_export, kwargs={'username': self.user.username, - 'id_string': self.xform.id_string}) - response = self.client.get(url) - self.assertEqual(response.status_code, 200) - - def test_allow_csv_export_for_basic_auth(self): - extra = { - 'HTTP_AUTHORIZATION': http_auth_string(self.login_username, - self.login_password) - } - response = self.anon.get(self.csv_url, **extra) - self.assertEqual(response.status_code, 200) - - def test_allow_xls_export_for_basic_auth(self): - extra = { - 'HTTP_AUTHORIZATION': http_auth_string(self.login_username, - self.login_password) - } - response = self.anon.get(self.xls_url, **extra) - self.assertEqual(response.status_code, 200) - - def test_allow_kml_export_for_basic_auth(self): - extra = { - 'HTTP_AUTHORIZATION': http_auth_string(self.login_username, - self.login_password) - } - url = reverse(kml_export, kwargs={'username': self.user.username, - 'id_string': self.xform.id_string}) - response = self.anon.get(url, **extra) - self.assertEqual(response.status_code, 200) - def test_allow_export_download_for_basic_auth(self): extra = { 'HTTP_AUTHORIZATION': http_auth_string(self.login_username, diff --git a/onadata/apps/main/tests/test_process.py b/onadata/apps/main/tests/test_process.py index d166f9456..a921f81b5 100644 --- a/onadata/apps/main/tests/test_process.py +++ b/onadata/apps/main/tests/test_process.py @@ -52,16 +52,6 @@ def setUp(self): def tearDown(self): super().tearDown() - @unittest.skip('Fails under Django 1.6') - def test_process(self, username=None, password=None): - self._publish_xls_file() - self._check_formList() - self._download_xform() - self._make_submissions() - self._update_dynamic_data() - self._check_csv_export() - self._check_delete() - def _update_dynamic_data(self): """ Update stuff like submission time so we can compare within out fixtures @@ -359,41 +349,6 @@ def _check_csv_export_second_pass(self): l.append(("transport/" + k, v)) self.assertEqual(d, dict(l)) - def test_xls_export_content(self): - self._publish_xls_file() - self._make_submissions() - self._update_dynamic_data() - self._check_xls_export() - - def _check_xls_export(self): - xls_export_url = reverse( - 'xls_export', kwargs={'username': self.user.username, - 'id_string': self.xform.id_string}) - response = self.client.get(xls_export_url) - expected_xls = load_workbook(os.path.join( - self.this_directory, 'fixtures', 'transportation', - 'transportation_export.xlsx')) - content = self._get_response_content(response) - actual_xls = load_workbook(BytesIO(content)) - actual_sheet = actual_xls[actual_xls.sheetnames[0]] - expected_sheet = expected_xls[expected_xls.sheetnames[0]] - - # check headers - self.assertEqual([cell.value for cell in actual_sheet[1]], - [cell.value for cell in expected_sheet[1]]) - - # check cell data - self.assertEqual(actual_sheet.max_column, expected_sheet.max_column) - self.assertEqual(actual_sheet.max_row, expected_sheet.max_row) - for i in range(2, actual_sheet.max_row): - actual_row = [cell.value for cell in actual_sheet[i]] - expected_row = [cell.value for cell in expected_sheet[i]] - - # remove _id from result set, varies depending on the database - del actual_row[22] - del expected_row[22] - self.assertEqual(actual_row, expected_row) - def _check_delete(self): self.assertEqual(self.user.xforms.count(), 1) self.user.xforms.all()[0].delete() diff --git a/onadata/apps/main/urls.py b/onadata/apps/main/urls.py index e998899fd..71217acd8 100644 --- a/onadata/apps/main/urls.py +++ b/onadata/apps/main/urls.py @@ -20,13 +20,11 @@ # exporting stuff from onadata.apps.viewer.views import ( attachment_url, - data_export, create_export, delete_export, export_progress, export_list, export_download, - kml_export ) from onadata.apps.logger.views import ( bulksubmission, @@ -94,14 +92,6 @@ name='upload'), # exporting stuff - re_path(r"^(?P\w+)/forms/(?P[^/]+)/data\.csv$", - data_export, name='csv_export', - kwargs={'export_type': 'csv'}), - re_path(r"^(?P\w+)/forms/(?P[^/]+)/data\.xls", - data_export, name='xls_export', - kwargs={'export_type': 'xls'}), - re_path(r"^(?P\w+)/forms/(?P[^/]+)/data\.kml$", - kml_export), re_path(r"^(?P\w+)/exports/(?P[^/]+)/(?P\w+)" r"/new$", create_export, name='create_export'), re_path(r"^(?P\w+)/exports/(?P[^/]+)/(?P\w+)" diff --git a/onadata/apps/viewer/tests/test_export_list.py b/onadata/apps/viewer/tests/test_export_list.py index be68e207e..1a5519a13 100644 --- a/onadata/apps/viewer/tests/test_export_list.py +++ b/onadata/apps/viewer/tests/test_export_list.py @@ -70,55 +70,3 @@ def test_zip_export_list(self): url = reverse(export_list, kwargs=kwargs) response = self.client.get(url) self.assertEqual(response.status_code, 200) - - -class TestDataExportURL(TestBase): - - def setUp(self): - super().setUp() - self._publish_transportation_form() - - def _filename_from_disposition(self, content_disposition): - filename_pos = content_disposition.index('filename=') - self.assertTrue(filename_pos != -1) - return content_disposition[filename_pos + len('filename='):] - - def test_csv_export_url(self): - self._submit_transport_instance() - url = reverse('csv_export', kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string, - }) - response = self.client.get(url) - headers = dict(response.items()) - self.assertEqual(headers['Content-Type'], 'application/csv') - content_disposition = headers['Content-Disposition'] - filename = self._filename_from_disposition(content_disposition) - basename, ext = os.path.splitext(filename) - self.assertEqual(ext, '.csv') - - def test_csv_export_url_without_records(self): - # csv using the pandas path can throw a NoRecordsFound Exception - - # handle it gracefully - url = reverse('csv_export', kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string, - }) - response = self.client.get(url) - self.assertEqual(response.status_code, 404) - - @unittest.skip('Fails under Django 1.6') - def test_xls_export_url(self): - self._submit_transport_instance() - url = reverse('xls_export', kwargs={ - 'username': self.user.username.upper(), - 'id_string': self.xform.id_string.upper(), - }) - response = self.client.get(url) - headers = dict(response.items()) - self.assertEqual(headers['Content-Type'], - 'application/vnd.openxmlformats') - content_disposition = headers['Content-Disposition'] - filename = self._filename_from_disposition(content_disposition) - basename, ext = os.path.splitext(filename) - self.assertEqual(ext, '.xlsx') diff --git a/onadata/apps/viewer/tests/test_exports.py b/onadata/apps/viewer/tests/test_exports.py index c0883f663..296a94651 100644 --- a/onadata/apps/viewer/tests/test_exports.py +++ b/onadata/apps/viewer/tests/test_exports.py @@ -4,19 +4,23 @@ import json import os import io -import unittest from time import sleep +import requests from django.conf import settings -from django.core.files.storage import get_storage_class, FileSystemStorage +from django.core.files.storage import default_storage, FileSystemStorage from django.urls import reverse from django.utils.dateparse import parse_datetime from openpyxl import load_workbook from onadata.apps.main.tests.test_base import TestBase -from onadata.apps.viewer.tests.export_helpers import viewer_fixture_path -from onadata.apps.viewer.views import delete_export, export_list,\ - create_export, export_progress, export_download +from onadata.apps.viewer.views import ( + delete_export, + export_list, + create_export, + export_progress, + export_download, +) from onadata.apps.viewer.xls_writer import XlsWriter from onadata.apps.viewer.models.export import Export from onadata.apps.viewer.models.parsed_instance import ParsedInstance @@ -25,10 +29,12 @@ from onadata.libs.utils.export_tools import generate_export,\ increment_index_in_filename, dict_to_joined_export -AMBULANCE_KEY = 'transport/available_transportation_types_to_referral_fac'\ - 'ility/ambulance' -AMBULANCE_KEY_DOTS = 'transport.available_transportation_types_to_referra'\ - 'l_facility.ambulance' +AMBULANCE_KEY = ( + 'transport/available_transportation_types_to_referral_facility/ambulance' +) +AMBULANCE_KEY_DOTS = ( + 'transport.available_transportation_types_to_referral_facility.ambulance' +) def _main_fixture_path(instance_name): @@ -51,77 +57,19 @@ def test_unique_xls_sheet_name(self): sheet_names_set = set(xls_writer._sheets.keys()) self.assertEqual(len(sheet_names_set), 2) - def test_csv_http_response(self): - self._publish_transportation_form() - survey = self.surveys[0] - self._make_submission( - os.path.join( - self.this_directory, 'fixtures', 'transportation', - 'instances', survey, survey + '.xml'), - forced_submission_time=self._submission_time) - response = self.client.get(reverse( - 'csv_export', - kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string - })) - self.assertEqual(response.status_code, 200) - test_file_path = viewer_fixture_path('transportation.csv') - content = self._get_response_content(response) - with open(test_file_path, 'rb') as test_file: - self.assertEqual(content, test_file.read()) - - def test_csv_without_na_values(self): - self._publish_transportation_form() - survey = self.surveys[0] - self._make_submission( - os.path.join( - self.this_directory, 'fixtures', 'transportation', - 'instances', survey, survey + '.xml'), - forced_submission_time=self._submission_time) - na_rep_restore = settings.NA_REP - settings.NA_REP = '' - response = self.client.get(reverse( - 'csv_export', - kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string - })) - self.assertEqual(response.status_code, 200) - test_file_path = viewer_fixture_path('transportation_without_na.csv') - content = self._get_response_content(response) - with open(test_file_path, 'rb') as test_file: - self.assertEqual(content, test_file.read()) - settings.NA_REP = na_rep_restore - - def test_responses_for_empty_exports(self): - self._publish_transportation_form() - # test csv though xls uses the same view - url = reverse( - 'csv_export', - kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string - } - ) - self.response = self.client.get(url) - self.assertEqual(self.response.status_code, 404) - self.assertIn('text/html', self.response['content-type']) - def test_create_export(self): self._publish_transportation_form_and_submit_instance() - storage = get_storage_class()() # test xls export = generate_export(Export.XLS_EXPORT, 'xls', self.user.username, self.xform.id_string) - self.assertTrue(storage.exists(export.filepath)) + self.assertTrue(default_storage.exists(export.filepath)) path, ext = os.path.splitext(export.filename) self.assertEqual(ext, '.xls') # test csv export = generate_export(Export.CSV_EXPORT, 'csv', self.user.username, self.xform.id_string) - self.assertTrue(storage.exists(export.filepath)) + self.assertTrue(default_storage.exists(export.filepath)) path, ext = os.path.splitext(export.filename) self.assertEqual(ext, '.csv') @@ -137,21 +85,20 @@ def test_delete_file_on_export_delete(self): self._submit_transport_instance() export = generate_export(Export.XLS_EXPORT, 'xls', self.user.username, self.xform.id_string) - storage = get_storage_class()() - self.assertTrue(storage.exists(export.filepath)) + self.assertTrue(default_storage.exists(export.filepath)) # delete export object export.delete() - self.assertFalse(storage.exists(export.filepath)) + self.assertFalse(default_storage.exists(export.filepath)) def test_graceful_exit_on_export_delete_if_file_doesnt_exist(self): self._publish_transportation_form() self._submit_transport_instance() export = generate_export(Export.XLS_EXPORT, 'xls', self.user.username, self.xform.id_string) - storage = get_storage_class()() + # delete file - storage.delete(export.filepath) - self.assertFalse(storage.exists(export.filepath)) + default_storage.delete(export.filepath) + self.assertFalse(default_storage.exists(export.filepath)) # clear filename, like it would be in an incomplete export export.filename = None export.filedir = None @@ -361,34 +308,6 @@ def test_add_index_to_filename(self): expected_filename = "file_name-124.txt" self.assertEqual(new_filename, expected_filename) - @unittest.skip('Fails under Django 1.6') - def test_duplicate_export_filename_is_renamed(self): - self._publish_transportation_form() - self._submit_transport_instance() - - # TODO: mock the time - # only works if the time we time we generate the basename - # is exact to the second with the time the 2nd export is created - - # create an export object in the db - basename = "%s_%s" % ( - self.xform.id_string, - datetime.datetime.now().strftime("%Y_%m_%d_%H_%M_%S")) - filename = basename + ".csv" - export = Export.objects.create( - xform=self.xform, export_type=Export.CSV_EXPORT, filename=filename) - - # 2nd export - export_2 = generate_export( - Export.CSV_EXPORT, 'csv', self.user.username, self.xform.id_string) - - if export.created_on.timetuple() == export_2.created_on.timetuple(): - new_filename = increment_index_in_filename(filename) - self.assertEqual(new_filename, export_2.filename) - else: - self.skipTest("duplicate export filename test skipped " - "because export times differ.") - def test_export_download_url(self): self._publish_transportation_form() self._submit_transport_instance() @@ -401,7 +320,6 @@ def test_export_download_url(self): "filename": export.filename }) response = self.client.get(csv_export_url) - default_storage = get_storage_class()() if not isinstance(default_storage, FileSystemStorage): self.assertEqual(response.status_code, 302) else: @@ -466,12 +384,24 @@ def test_deleted_submission_not_in_export(self): self.user.username, self.xform.id_string, '{}', '[]', '{}', count=True)[0]['count'] self.assertEqual(count, initial_count + 1) + # create the export - csv_export_url = reverse( - 'csv_export', kwargs={"username": self.user.username, - "id_string": self.xform.id_string}) + export = generate_export( + Export.CSV_EXPORT, 'csv', self.user.username, self.xform.id_string + ) + csv_export_url = reverse(export_download, kwargs={ + 'username': self.user.username, + 'id_string': self.xform.id_string, + 'export_type': Export.CSV_EXPORT, + 'filename': export.filename + }) response = self.client.get(csv_export_url) - self.assertEqual(response.status_code, 200) + if not isinstance(default_storage, FileSystemStorage): + assert response.status_code == 302 + response = requests.get(response.headers['Location']) + + assert response.status_code == 200 + f = io.StringIO(self._get_response_content(response).decode()) csv_reader = csv.reader(f) num_rows = len([row for row in csv_reader]) @@ -500,11 +430,22 @@ def test_edited_submissions_in_exports(self): count=True)[0]['count'] self.assertEqual(count, initial_count + 1) # create the export - csv_export_url = reverse( - 'csv_export', kwargs={"username": self.user.username, - "id_string": self.xform.id_string}) + export = generate_export( + Export.CSV_EXPORT, 'csv', self.user.username, self.xform.id_string + ) + csv_export_url = reverse(export_download, kwargs={ + 'username': self.user.username, + 'id_string': self.xform.id_string, + 'export_type': Export.CSV_EXPORT, + 'filename': export.filename + }) response = self.client.get(csv_export_url) - self.assertEqual(response.status_code, 200) + if not isinstance(default_storage, FileSystemStorage): + assert response.status_code == 302 + response = requests.get(response.headers['Location']) + + assert response.status_code == 200 + f = io.StringIO(self._get_response_content(response).decode()) csv_reader = csv.DictReader(f) data = [row for row in csv_reader] @@ -512,8 +453,10 @@ def test_edited_submissions_in_exports(self): num_rows = len(data) # number of rows == initial_count + 1 self.assertEqual(num_rows, initial_count + 1) - key = 'transport/loop_over_transport_types_frequency/ambulance/'\ - 'frequency_to_referral_facility' + key = ( + 'transport/loop_over_transport_types_frequency/ambulance/' + 'frequency_to_referral_facility' + ) self.assertEqual(data[initial_count][key], "monthly") def test_export_ids_dont_have_comma_separation(self): @@ -584,87 +527,6 @@ def test_export_progress_updates(self): self.assertEqual(status["complete"], True) self.assertIsNotNone(status["filename"]) - def test_direct_export_returns_newset_export_if_not_updated_since(self): - self._publish_transportation_form() - self._submit_transport_instance() - self.assertEqual(self.response.status_code, 201) - sleep(1) - self._submit_transport_instance_w_uuid("transport_2011-07-25_19-05-36") - self.assertEqual(self.response.status_code, 201) - - initial_num_csv_exports = Export.objects.filter( - xform=self.xform, export_type=Export.CSV_EXPORT).count() - initial_num_xls_exports = Export.objects.filter( - xform=self.xform, export_type=Export.XLS_EXPORT).count() - # request a direct csv export - csv_export_url = reverse('csv_export', kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string - }) - xls_export_url = reverse('xls_export', kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string - }) - response = self.client.get(csv_export_url) - self.assertEqual(response.status_code, 200) - # we should have initial_num_exports + 1 exports - num_csv_exports = Export.objects.filter( - xform=self.xform, export_type=Export.CSV_EXPORT).count() - self.assertEqual(num_csv_exports, initial_num_csv_exports + 1) - - # request another export without changing the data - response = self.client.get(csv_export_url) - self.assertEqual(response.status_code, 200) - # we should still only have a single export object - num_csv_exports = Export.objects.filter( - xform=self.xform, export_type=Export.CSV_EXPORT).count() - self.assertEqual(num_csv_exports, initial_num_csv_exports + 1) - - # this should not affect a direct XLS export - # and XLS should still re-generate - response = self.client.get(xls_export_url) - self.assertEqual(response.status_code, 200) - num_xls_exports = Export.objects.filter( - xform=self.xform, export_type=Export.XLS_EXPORT).count() - self.assertEqual(num_xls_exports, initial_num_xls_exports + 1) - - # make sure xls doesnt re-generate if data hasn't changed - response = self.client.get(xls_export_url) - self.assertEqual(response.status_code, 200) - num_xls_exports = Export.objects.filter( - xform=self.xform, export_type=Export.XLS_EXPORT).count() - self.assertEqual(num_xls_exports, initial_num_xls_exports + 1) - - sleep(1) - # check that data edits cause a re-generation - self._submit_transport_instance_w_uuid( - "transport_2011-07-25_19-05-36-edited") - self.assertEqual(self.response.status_code, 201) - self.client.get(csv_export_url) - self.assertEqual(response.status_code, 200) - # we should have an extra export now that the data has been updated - num_csv_exports = Export.objects.filter( - xform=self.xform, export_type=Export.CSV_EXPORT).count() - self.assertEqual(num_csv_exports, initial_num_csv_exports + 2) - - sleep(1) - # and when we delete - instance = Instance.objects.filter().order_by('-pk')[0] - delete_url = reverse('data-detail', kwargs={ - 'pk': self.xform.pk, - 'dataid': instance.pk - }) - response = self.client.delete(delete_url) - self.assertEqual(response.status_code, 204) - response = self.client.get(csv_export_url) - self.assertEqual(response.status_code, 200) - # we should have an extra export now that the data - # has been updated by the delete - num_csv_exports = Export.objects.filter( - xform=self.xform, export_type=Export.CSV_EXPORT).count() - self.xform.refresh_from_db() - self.assertEqual(num_csv_exports, initial_num_csv_exports + 3) - def test_exports_outdated_doesnt_consider_failed_exports(self): self._publish_transportation_form() self._submit_transport_instance() @@ -686,16 +548,14 @@ def test_exports_outdated_considers_pending_exports(self): Export.exports_outdated(self.xform, export.export_type)) def _get_csv_data(self, filepath): - storage = get_storage_class()() - csv_file = storage.open(filepath, mode='r') + csv_file = default_storage.open(filepath, mode='r') reader = csv.DictReader(csv_file) data = next(reader) csv_file.close() return data def _get_xls_data(self, filepath): - storage = get_storage_class()() - with storage.open(filepath) as f: + with default_storage.open(filepath) as f: workbook = load_workbook(f) transportation_sheet = workbook['transportation_2011_07_25'] self.assertTrue(transportation_sheet.max_row > 1) diff --git a/onadata/apps/viewer/tests/test_kml_export.py b/onadata/apps/viewer/tests/test_kml_export.py deleted file mode 100644 index d6a963900..000000000 --- a/onadata/apps/viewer/tests/test_kml_export.py +++ /dev/null @@ -1,46 +0,0 @@ -# coding: utf-8 -import os -import unittest - -from django.urls import reverse - -from onadata.apps.main.tests.test_base import TestBase -from onadata.apps.logger.models.instance import Instance -from onadata.apps.viewer.views import kml_export - - -class TestKMLExport(TestBase): - - def _publish_survey(self): - self.this_directory = os.path.dirname(__file__) - xls_path = self._fixture_path("gps", "gps.xls") - TestBase._publish_xls_file(self, xls_path) - - @unittest.skip('Fails under Django 1.6') - def test_kml_export(self): - id_string = 'gps' - - self._publish_survey() - self._make_submissions_gps() - self.fixtures = os.path.join( - self.this_directory, 'fixtures', 'kml_export') - url = reverse( - kml_export, - kwargs={'username': self.user.username, 'id_string': id_string}) - response = self.client.get(url) - instances = Instance.objects.filter( - xform__user=self.user, xform__id_string=id_string, - geom__isnull=False - ).order_by('id') - - self.assertEqual(instances.count(), 2) - - first, second = [str(i.pk) for i in instances] - - with open(os.path.join(self.fixtures, 'export.kml')) as f: - expected_content = f.read() - expected_content = expected_content.replace('{{first}}', first) - expected_content = expected_content.replace('{{second}}', second) - - self.assertMultiLineEqual( - expected_content.strip(), response.content.strip()) diff --git a/onadata/apps/viewer/tests/test_pandas_mongo_bridge.py b/onadata/apps/viewer/tests/test_pandas_mongo_bridge.py index 8b134d552..2aad28469 100644 --- a/onadata/apps/viewer/tests/test_pandas_mongo_bridge.py +++ b/onadata/apps/viewer/tests/test_pandas_mongo_bridge.py @@ -174,43 +174,6 @@ def test_xls_columns_for_gps_within_groups(self): self.assertEqual(sorted(expected_default_columns), sorted(default_columns)) - def test_xlsx_output_when_data_exceeds_limits(self): - self._publish_xls_fixture_set_xform("xlsx_output") - self._submit_fixture_instance("xlsx_output", "01") - xls_builder = XLSDataFrameBuilder(username=self.user.username, - id_string=self.xform.id_string) - self.assertEqual(xls_builder.exceeds_xls_limits, True) - # test that the view returns an xlsx file instead - url = reverse('xls_export', kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string - }) - self.response = self.client.get(url) - self.assertEqual(self.response.status_code, 200) - self.assertEqual(self.response["content-type"], - 'application/vnd.openxmlformats') - - def test_xlsx_export_for_repeats(self): - """ - Make sure exports run fine when the xlsx file has multiple sheets - """ - self._publish_xls_fixture_set_xform("new_repeats") - self._submit_fixture_instance("new_repeats", "01") - XLSDataFrameBuilder(username=self.user.username, - id_string=self.xform.id_string) - # test that the view returns an xlsx file instead - url = reverse('xls_export', kwargs={ - 'username': self.user.username, - 'id_string': self.xform.id_string - }) - params = { - 'xlsx': 'true' # force xlsx - } - self.response = self.client.get(url, params) - self.assertEqual(self.response.status_code, 200) - self.assertEqual(self.response["content-type"], - 'application/vnd.openxmlformats') - def test_csv_dataframe_export_to(self): self._publish_nested_repeats_form() self._submit_fixture_instance( diff --git a/onadata/apps/viewer/views.py b/onadata/apps/viewer/views.py index f9b135f5c..3092d8f0b 100644 --- a/onadata/apps/viewer/views.py +++ b/onadata/apps/viewer/views.py @@ -3,7 +3,6 @@ import logging import os import re -from datetime import datetime import rest_framework.request from django.conf import settings @@ -28,17 +27,9 @@ from onadata.apps.viewer.models.export import Export from onadata.apps.viewer.tasks import create_async_export from onadata.libs.authentication import digest_authentication -from onadata.libs.exceptions import NoRecordsFoundError -from onadata.libs.utils.common_tags import SUBMISSION_TIME -from onadata.libs.utils.export_tools import ( - generate_export, - should_create_new_export, - kml_export_data, - newset_export_for) from onadata.libs.utils.image_tools import image_url from onadata.libs.utils.log import audit_log, Actions -from onadata.libs.utils.logger_tools import response_with_mimetype_and_name, \ - disposition_ext_and_date +from onadata.libs.utils.logger_tools import response_with_mimetype_and_name from onadata.libs.utils.user_auth import ( HttpResponseNotAuthorized, has_permission, @@ -49,97 +40,6 @@ media_file_logger = logging.getLogger('media_files') -def _set_submission_time_to_query(query, request): - query[SUBMISSION_TIME] = {} - try: - if request.GET.get('start'): - query[SUBMISSION_TIME]['$gte'] = format_date_for_mongo( - request.GET['start']) - if request.GET.get('end'): - query[SUBMISSION_TIME]['$lte'] = format_date_for_mongo( - request.GET['end']) - except ValueError: - return HttpResponseBadRequest( - t("Dates must be in the format YY_MM_DD_hh_mm_ss")) - - return query - - -def format_date_for_mongo(x): - return datetime.strptime(x, '%y_%m_%d_%H_%M_%S')\ - .strftime('%Y-%m-%dT%H:%M:%S') - - -def data_export(request, username, id_string, export_type): - owner = get_object_or_404(User, username__iexact=username) - xform = get_object_or_404(XForm, id_string__exact=id_string, user=owner) - helper_auth_helper(request) - if not has_permission(xform, owner, request): - return HttpResponseForbidden(t('Not shared.')) - query = request.GET.get("query") - extension = export_type - - # check if we should force xlsx - force_xlsx = request.GET.get('xls') != 'true' - if export_type == Export.XLS_EXPORT and force_xlsx: - extension = 'xlsx' - - audit = { - "xform": xform.id_string, - "export_type": export_type - } - # check if we need to re-generate, - # we always re-generate if a filter is specified - if should_create_new_export(xform, export_type) or query or\ - 'start' in request.GET or 'end' in request.GET: - # check for start and end params - if 'start' in request.GET or 'end' in request.GET: - if not query: - query = '{}' - query = json.dumps( - _set_submission_time_to_query(json.loads(query), request)) - try: - export = generate_export( - export_type, extension, username, id_string, None, query) - audit_log( - Actions.EXPORT_CREATED, request.user, owner, - t("Created %(export_type)s export on '%(id_string)s'.") % - { - 'id_string': xform.id_string, - 'export_type': export_type.upper() - }, audit, request) - except NoRecordsFoundError: - return HttpResponseNotFound(t("No records found to export")) - else: - export = newset_export_for(xform, export_type) - - # log download as well - audit_log( - Actions.EXPORT_DOWNLOADED, request.user, owner, - t("Downloaded %(export_type)s export on '%(id_string)s'.") % - { - 'id_string': xform.id_string, - 'export_type': export_type.upper() - }, audit, request) - - if not export.filename: - # tends to happen when using newset_export_for. - return HttpResponseNotFound("File does not exist!") - - # get extension from file_path, exporter could modify to - # xlsx if it exceeds limits - path, ext = os.path.splitext(export.filename) - ext = ext[1:] - if request.GET.get('raw'): - id_string = None - - response = response_with_mimetype_and_name( - Export.EXPORT_MIMES[ext], id_string, extension=ext, - file_path=export.filepath) - - return response - - @login_required @require_POST def create_export(request, username, id_string, export_type): @@ -331,40 +231,6 @@ def delete_export(request, username, id_string, export_type): })) -def kml_export(request, username, id_string): - # read the locations from the database - owner = get_object_or_404(User, username__iexact=username) - xform = get_object_or_404(XForm, id_string__exact=id_string, user=owner) - helper_auth_helper(request) - if not has_permission(xform, owner, request): - return HttpResponseForbidden(t('Not shared.')) - data = {'data': kml_export_data(id_string, user=owner)} - response = \ - render(request, "survey.kml", data, - content_type="application/vnd.google-earth.kml+xml") - response['Content-Disposition'] = \ - disposition_ext_and_date(id_string, 'kml') - audit = { - "xform": xform.id_string, - "export_type": Export.KML_EXPORT - } - audit_log( - Actions.EXPORT_CREATED, request.user, owner, - t("Created KML export on '%(id_string)s'.") % - { - 'id_string': xform.id_string, - }, audit, request) - # log download as well - audit_log( - Actions.EXPORT_DOWNLOADED, request.user, owner, - t("Downloaded KML export on '%(id_string)s'.") % - { - 'id_string': xform.id_string, - }, audit, request) - - return response - - def attachment_url(request, size='medium'): media_file = request.GET.get('media_file') diff --git a/onadata/libs/utils/export_tools.py b/onadata/libs/utils/export_tools.py index 4b6378da9..16abf2881 100644 --- a/onadata/libs/utils/export_tools.py +++ b/onadata/libs/utils/export_tools.py @@ -6,10 +6,10 @@ from bson import json_util from django.conf import settings -from django.core.files.base import File, ContentFile +from django.core.files.base import File from django.core.files.storage import FileSystemStorage from django.core.files.temp import NamedTemporaryFile -from django.core.files.storage import get_storage_class +from django.core.files.storage import default_storage from django.contrib.auth.models import User from django.shortcuts import render from django.utils.text import slugify @@ -523,16 +523,18 @@ def write_row(data, work_sheet, fields, work_sheet_titles): wb.save(filename=path) - def to_flat_csv_export( - self, path, data, username, id_string, filter_query): + def to_flat_csv_export(self, path, data, username, id_string, filter_query): # TODO resolve circular import - - from onadata.apps.viewer.pandas_mongo_bridge import\ - CSVDataFrameBuilder + from onadata.apps.viewer.pandas_mongo_bridge import CSVDataFrameBuilder csv_builder = CSVDataFrameBuilder( - username, id_string, filter_query, self.GROUP_DELIMITER, - self.SPLIT_SELECT_MULTIPLES, self.BINARY_SELECT_MULTIPLES) + username, + id_string, + filter_query, + self.GROUP_DELIMITER, + self.SPLIT_SELECT_MULTIPLES, + self.BINARY_SELECT_MULTIPLES, + ) csv_builder.export_to(path) @@ -570,7 +572,6 @@ def generate_export(export_type, extension, username, id_string, # get the export function by export type func = getattr(export_builder, export_type_func_map[export_type]) - func.__call__( temp_file.name, records, username, id_string, filter_query) @@ -591,10 +592,9 @@ def generate_export(export_type, extension, username, id_string, filename) # TODO: if s3 storage, make private - how will we protect local storage?? - storage = get_storage_class()() # seek to the beginning as required by storage classes temp_file.seek(0) - export_filename = storage.save( + export_filename = default_storage.save( file_path, File(temp_file, file_path)) temp_file.close() @@ -609,7 +609,7 @@ def generate_export(export_type, extension, username, id_string, export.filedir = dir_name export.filename = basename export.internal_status = Export.SUCCESSFUL - # dont persist exports that have a filter + # do not persist exports that have a filter if filter_query is None: export.save() return export diff --git a/onadata/libs/utils/viewer_tools.py b/onadata/libs/utils/viewer_tools.py index ccdf96448..657745ae2 100644 --- a/onadata/libs/utils/viewer_tools.py +++ b/onadata/libs/utils/viewer_tools.py @@ -4,6 +4,7 @@ import traceback import requests import zipfile +from datetime import datetime from tempfile import NamedTemporaryFile @@ -25,6 +26,12 @@ class EnketoError(Exception): pass +def format_date_for_mongo(x): + return datetime.strptime(x, '%y_%m_%d_%H_%M_%S').strftime( + '%Y-%m-%dT%H:%M:%S' + ) + + def get_path(path, suffix): fileName, fileExtension = os.path.splitext(path) return fileName + suffix + fileExtension From 5c7b247f1694bdcfe1bd2fd5d32e42600e151cac Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Fri, 14 Jul 2023 13:15:41 -0400 Subject: [PATCH 11/23] Replace get_storage_class()() with default_storage --- .../api/tests/viewsets/test_briefcase_api.py | 3 +-- onadata/apps/api/tools.py | 4 ++-- .../commands/change_s3_media_permissions.py | 2 +- .../commands/create_image_thumbnails.py | 3 +-- .../management/commands/move_media_to_s3.py | 3 +-- onadata/apps/logger/models/xform.py | 4 ++-- onadata/apps/logger/tasks.py | 3 +-- .../logger/tests/test_importing_database.py | 5 ++-- onadata/apps/main/tests/test_csv_export.py | 19 +++++++-------- onadata/apps/main/tests/test_form_exports.py | 3 +-- onadata/apps/main/views.py | 8 +++---- onadata/apps/viewer/models/export.py | 8 +++---- onadata/apps/viewer/views.py | 23 ++++++++++-------- onadata/libs/utils/export_tools.py | 18 +++++++------- onadata/libs/utils/image_tools.py | 5 +--- onadata/libs/utils/logger_tools.py | 9 +++---- onadata/libs/utils/viewer_tools.py | 24 +++++++++++++------ 17 files changed, 70 insertions(+), 74 deletions(-) diff --git a/onadata/apps/api/tests/viewsets/test_briefcase_api.py b/onadata/apps/api/tests/viewsets/test_briefcase_api.py index 031749406..5f6bc136f 100644 --- a/onadata/apps/api/tests/viewsets/test_briefcase_api.py +++ b/onadata/apps/api/tests/viewsets/test_briefcase_api.py @@ -2,7 +2,7 @@ import os from django.urls import reverse -from django.core.files.storage import get_storage_class +from django.core.files.storage import default_storage from django.test import override_settings from django_digest.test import DigestAuth from rest_framework.test import APIRequestFactory @@ -16,7 +16,6 @@ NUM_INSTANCES = 4 -storage = get_storage_class()() def ordered_instances(xform): diff --git a/onadata/apps/api/tools.py b/onadata/apps/api/tools.py index 13ce9a20f..4410cd6a8 100644 --- a/onadata/apps/api/tools.py +++ b/onadata/apps/api/tools.py @@ -11,7 +11,7 @@ from django.conf import settings from django.contrib.auth.models import User from django.contrib.sites.models import Site -from django.core.files.storage import get_storage_class +from django.core.files.storage import default_storage from django.http import ( HttpResponse, HttpResponseNotFound, @@ -195,7 +195,7 @@ def get_media_file_response( file_path = metadata.data_file.name filename, extension = os.path.splitext(file_path.split('/')[-1]) extension = extension.strip('.') - dfs = get_storage_class()() + dfs = default_storage if dfs.exists(file_path): response = response_with_mimetype_and_name( diff --git a/onadata/apps/logger/management/commands/change_s3_media_permissions.py b/onadata/apps/logger/management/commands/change_s3_media_permissions.py index 57d53c902..b83eeda32 100644 --- a/onadata/apps/logger/management/commands/change_s3_media_permissions.py +++ b/onadata/apps/logger/management/commands/change_s3_media_permissions.py @@ -4,7 +4,7 @@ import sys from django.core.management.base import BaseCommand, CommandError -from django.core.files.storage import get_storage_class +from django.core.files.storage import default_storage class Command(BaseCommand): diff --git a/onadata/apps/logger/management/commands/create_image_thumbnails.py b/onadata/apps/logger/management/commands/create_image_thumbnails.py index fc6ed3ddf..4ba620ca0 100644 --- a/onadata/apps/logger/management/commands/create_image_thumbnails.py +++ b/onadata/apps/logger/management/commands/create_image_thumbnails.py @@ -4,7 +4,7 @@ from django.contrib.auth.models import User from django.core.management.base import BaseCommand, CommandError -from django.core.files.storage import get_storage_class +from django.core.files.storage import default_storage from django.conf import settings from onadata.apps.logger.models.attachment import Attachment @@ -53,7 +53,6 @@ def handle(self, *args, **kwargs): for att in queryset_iterator(attachments_qs): filename = att.media_file.name - default_storage = get_storage_class()() full_path = get_path(filename, settings.THUMB_CONF['small']['suffix']) if kwargs.get('force') is not None: diff --git a/onadata/apps/logger/management/commands/move_media_to_s3.py b/onadata/apps/logger/management/commands/move_media_to_s3.py index dd6e77231..2e94721f3 100644 --- a/onadata/apps/logger/management/commands/move_media_to_s3.py +++ b/onadata/apps/logger/management/commands/move_media_to_s3.py @@ -3,7 +3,7 @@ # coding: utf-8 import sys -from django.core.files.storage import get_storage_class +from django.core.files.storage import default_storage from django.core.management.base import BaseCommand from onadata.apps.logger.models.attachment import Attachment @@ -27,7 +27,6 @@ def handle(self, *args, **kwargs): "requirements/s3.pip") sys.exit(1) - default_storage = get_storage_class()() if default_storage.__class__ != s3.__class__: print("You must first set your default storage to s3 in your " "local_settings.py file.") diff --git a/onadata/apps/logger/models/xform.py b/onadata/apps/logger/models/xform.py index 0e636e6f3..8852f9ba4 100644 --- a/onadata/apps/logger/models/xform.py +++ b/onadata/apps/logger/models/xform.py @@ -2,12 +2,13 @@ import json import os import re +from io import BytesIO from xml.sax.saxutils import escape as xml_escape from django.conf import settings from django.contrib.auth.models import User from django.core.exceptions import ObjectDoesNotExist -from django.core.files.storage import get_storage_class +from django.core.files.storage import default_storage from django.urls import reverse from django.db import models from django.db.models.signals import post_save, post_delete, pre_delete @@ -261,7 +262,6 @@ def _xls_file_io(self): this should be used sparingly """ file_path = self.xls.name - default_storage = get_storage_class()() if file_path != '' and default_storage.exists(file_path): with default_storage.open(file_path) as ff: diff --git a/onadata/apps/logger/tasks.py b/onadata/apps/logger/tasks.py index e12275fdd..b740e6246 100644 --- a/onadata/apps/logger/tasks.py +++ b/onadata/apps/logger/tasks.py @@ -10,7 +10,7 @@ from dateutil import relativedelta from django.conf import settings from django.contrib.auth.models import User -from django.core.files.storage import get_storage_class +from django.core.files.storage import default_storage from django.core.management import call_command from django.utils import timezone @@ -94,7 +94,6 @@ def list_created_by_month(model, date_field): return results - default_storage = get_storage_class()() with default_storage.open(output_filename, 'wb') as output_file: zip_file = zipfile.ZipFile(output_file, 'w', zipfile.ZIP_DEFLATED) for filename, report_settings in REPORTS.items(): diff --git a/onadata/apps/logger/tests/test_importing_database.py b/onadata/apps/logger/tests/test_importing_database.py index 80228d436..e43bc4b4f 100644 --- a/onadata/apps/logger/tests/test_importing_database.py +++ b/onadata/apps/logger/tests/test_importing_database.py @@ -1,7 +1,7 @@ # coding: utf-8 import os -from django.core.files.storage import get_storage_class +from django.core.files.storage import default_storage from django.urls import reverse from django.conf import settings @@ -32,8 +32,7 @@ def _images_count(self, instance): xform_uuid=instance.xform.uuid, instance_uuid=instance.uuid ) - storage = get_storage_class()() - _, images = storage.listdir(attachments_path) + _, images = default_storage.listdir(attachments_path) return len(images) def tearDown(self): diff --git a/onadata/apps/main/tests/test_csv_export.py b/onadata/apps/main/tests/test_csv_export.py index 68377c197..49a11e63b 100644 --- a/onadata/apps/main/tests/test_csv_export.py +++ b/onadata/apps/main/tests/test_csv_export.py @@ -1,7 +1,7 @@ # coding: utf-8 import os -from django.core.files.storage import get_storage_class +from django.core.files.storage import default_storage from django.utils.dateparse import parse_datetime from onadata.apps.viewer.models.data_dictionary import DataDictionary @@ -27,13 +27,13 @@ def test_csv_export_output(self): # test csv export = generate_export(Export.CSV_EXPORT, 'csv', self.user.username, 'tutorial_w_repeats') - storage = get_storage_class()() - self.assertTrue(storage.exists(export.filepath)) + + self.assertTrue(default_storage.exists(export.filepath)) path, ext = os.path.splitext(export.filename) self.assertEqual(ext, '.csv') with open(os.path.join( self.fixture_dir, 'tutorial_w_repeats.csv'), 'rb') as f1: - with storage.open(export.filepath) as f2: + with default_storage.open(export.filepath) as f2: expected_content = f1.read() actual_content = f2.read() self.assertEqual(actual_content, expected_content) @@ -57,12 +57,12 @@ def test_csv_nested_repeat_output(self): # test csv export = generate_export(Export.CSV_EXPORT, 'csv', self.user.username, 'double_repeat') - storage = get_storage_class()() - self.assertTrue(storage.exists(export.filepath)) + + self.assertTrue(default_storage.exists(export.filepath)) path, ext = os.path.splitext(export.filename) self.assertEqual(ext, '.csv') with open(os.path.join(self.fixture_dir, 'export.csv'), 'rb') as f1: - with storage.open(export.filepath) as f2: + with default_storage.open(export.filepath) as f2: expected_content = f1.read() actual_content = f2.read() self.assertEqual(actual_content, expected_content) @@ -78,14 +78,13 @@ def test_dotted_fields_csv_export_output(self): # test csv export = generate_export(Export.CSV_EXPORT, 'csv', self.user.username, 'userone') - storage = get_storage_class()() - self.assertTrue(storage.exists(export.filepath)) + self.assertTrue(default_storage.exists(export.filepath)) path, ext = os.path.splitext(export.filename) self.assertEqual(ext, '.csv') with open(os.path.join( os.path.dirname(__file__), 'fixtures', 'userone', 'userone_with_dot_name_fields.csv'), 'rb') as f1: - with storage.open(export.filepath) as f2: + with default_storage.open(export.filepath) as f2: expected_content = f1.read() actual_content = f2.read() self.assertEqual(actual_content, expected_content) diff --git a/onadata/apps/main/tests/test_form_exports.py b/onadata/apps/main/tests/test_form_exports.py index 3abb1cb44..ac1619fd8 100644 --- a/onadata/apps/main/tests/test_form_exports.py +++ b/onadata/apps/main/tests/test_form_exports.py @@ -5,7 +5,7 @@ from io import BytesIO from django.urls import reverse -from django.core.files.storage import get_storage_class, FileSystemStorage +from django.core.files.storage import default_storage, FileSystemStorage from django.utils import timezone from openpyxl import load_workbook @@ -52,7 +52,6 @@ def test_allow_export_download_for_basic_auth(self): 'filename': export.filename }) response = self.anon.get(url, **extra) - default_storage = get_storage_class()() if not isinstance(default_storage, FileSystemStorage): self.assertEqual(response.status_code, 302) else: diff --git a/onadata/apps/main/views.py b/onadata/apps/main/views.py index c206ebfa8..943e4df2d 100644 --- a/onadata/apps/main/views.py +++ b/onadata/apps/main/views.py @@ -4,7 +4,7 @@ import requests from django.conf import settings from django.contrib.auth.decorators import login_required -from django.core.files.storage import get_storage_class +from django.core.files.storage import default_storage from django.urls import reverse from django.http import HttpResponseForbidden from django.http import HttpResponseNotFound @@ -28,13 +28,13 @@ def download_media_data(request, username, id_string, data_id): id_string__exact=id_string) owner = xform.user data = get_object_or_404(MetaData, id=data_id) - dfs = get_storage_class()() + if request.GET.get('del', False): if username == request.user.username: try: # ensure filename is not an empty string if data.data_file.name != '': - dfs.delete(data.data_file.name) + default_storage.delete(data.data_file.name) data.delete() audit = { @@ -72,7 +72,7 @@ def download_media_data(request, username, id_string, data_id): file_path = data.data_file.name filename, extension = os.path.splitext(file_path.split('/')[-1]) extension = extension.strip('.') - if dfs.exists(file_path): + if default_storage.exists(file_path): audit = { 'xform': xform.id_string } diff --git a/onadata/apps/viewer/models/export.py b/onadata/apps/viewer/models/export.py index 063ebe383..f643a5c7f 100644 --- a/onadata/apps/viewer/models/export.py +++ b/onadata/apps/viewer/models/export.py @@ -2,7 +2,7 @@ import os from tempfile import NamedTemporaryFile -from django.core.files.storage import get_storage_class +from django.core.files.storage import default_storage from django.db import models from django.db.models.signals import post_delete from django.utils.translation import gettext as t @@ -12,9 +12,8 @@ def export_delete_callback(sender, **kwargs): export = kwargs['instance'] - storage = get_storage_class()() - if export.filepath and storage.exists(export.filepath): - storage.delete(export.filepath) + if export.filepath and default_storage.exists(export.filepath): + default_storage.delete(export.filepath) class Export(models.Model): @@ -136,7 +135,6 @@ def filepath(self): @property def full_filepath(self): if self.filepath: - default_storage = get_storage_class()() try: return default_storage.path(self.filepath) except NotImplementedError: diff --git a/onadata/apps/viewer/views.py b/onadata/apps/viewer/views.py index 3092d8f0b..1047a6712 100644 --- a/onadata/apps/viewer/views.py +++ b/onadata/apps/viewer/views.py @@ -8,13 +8,16 @@ from django.conf import settings from django.contrib.auth.decorators import login_required from django.contrib.auth.models import User -from django.core.files.storage import FileSystemStorage -from django.core.files.storage import get_storage_class +from django.core.files.storage import default_storage, FileSystemStorage from django.urls import reverse from django.db.models import Q from django.http import ( - HttpResponseForbidden, HttpResponseRedirect, HttpResponseNotFound, - HttpResponseBadRequest, HttpResponse) + HttpResponseForbidden, + HttpResponseRedirect, + HttpResponseNotFound, + HttpResponseBadRequest, + HttpResponse, +) from django.shortcuts import get_object_or_404 from django.shortcuts import render from django.utils.http import urlquote @@ -181,17 +184,18 @@ def export_download(request, username, id_string, export_type, filename): 'filename': export.filename, 'id_string': xform.id_string, }, audit, request) - if request.GET.get('raw'): - id_string = None - default_storage = get_storage_class()() if not isinstance(default_storage, FileSystemStorage): return HttpResponseRedirect(default_storage.url(export.filepath)) basename = os.path.splitext(export.filename)[0] response = response_with_mimetype_and_name( - mime_type, name=basename, extension=ext, - file_path=export.filepath, show_date=False) + mime_type, + name=basename, + extension=ext, + file_path=export.filepath, + show_date=False, + ) return response @@ -332,7 +336,6 @@ def attachment_url(request, size='medium'): # - When using S3 Storage, traffic is multiplied by 2. # S3 -> Nginx -> User response = HttpResponse() - default_storage = get_storage_class()() if not isinstance(default_storage, FileSystemStorage): # Double-encode the S3 URL to take advantage of NGINX's # otherwise troublesome automatic decoding diff --git a/onadata/libs/utils/export_tools.py b/onadata/libs/utils/export_tools.py index 16abf2881..9ed4e0230 100644 --- a/onadata/libs/utils/export_tools.py +++ b/onadata/libs/utils/export_tools.py @@ -678,7 +678,7 @@ def generate_attachments_zip_export( absolute_filename = _get_absolute_filename(file_path) - with get_storage_class()().open(absolute_filename, 'wb') as destination_file: + with default_storage.open(absolute_filename, 'wb') as destination_file: create_attachments_zipfile( attachments, output_file=destination_file, @@ -725,11 +725,10 @@ def generate_kml_export( export_type, filename) - storage = get_storage_class()() temp_file = NamedTemporaryFile(suffix=extension) temp_file.write(response.content) temp_file.seek(0) - export_filename = storage.save( + export_filename = default_storage.save( file_path, File(temp_file, file_path)) temp_file.close() @@ -779,8 +778,7 @@ def _get_absolute_filename(filename: str) -> str: Get absolute filename related to storage root. """ - storage_class = get_storage_class()() - filename = storage_class.generate_filename(filename) + filename = default_storage.generate_filename(filename) # We cannot call `self.result.save()` before reopening the file # in write mode (i.e. open(filename, 'wb')). because it does not work @@ -791,20 +789,20 @@ def _get_absolute_filename(filename: str) -> str: # - Get a unique filename if filename already exists on storage # Copied from `FileSystemStorage._save()` 😢 - if isinstance(storage_class, FileSystemStorage): - full_path = storage_class.path(filename) + if isinstance(default_storage, FileSystemStorage): + full_path = default_storage.path(filename) # Create any intermediate directories that do not exist. directory = os.path.dirname(full_path) if not os.path.exists(directory): try: - if storage_class.directory_permissions_mode is not None: + if default_storage.directory_permissions_mode is not None: # os.makedirs applies the global umask, so we reset it, # for consistency with file_permissions_mode behavior. old_umask = os.umask(0) try: os.makedirs( - directory, storage_class.directory_permissions_mode + directory, default_storage.directory_permissions_mode ) finally: os.umask(old_umask) @@ -821,4 +819,4 @@ def _get_absolute_filename(filename: str) -> str: # Store filenames with forward slashes, even on Windows. filename = filename.replace('\\', '/') - return storage_class.get_available_name(filename) + return default_storage.get_available_name(filename) diff --git a/onadata/libs/utils/image_tools.py b/onadata/libs/utils/image_tools.py index b3d9f09db..8215c12e5 100644 --- a/onadata/libs/utils/image_tools.py +++ b/onadata/libs/utils/image_tools.py @@ -4,7 +4,7 @@ import requests from django.conf import settings -from django.core.files.storage import get_storage_class +from django.core.files.storage import default_storage from django.core.files.base import ContentFile from PIL import Image @@ -39,7 +39,6 @@ def _save_thumbnails(image, original_path, size, suffix): # Thumbnail format will be set by original file extension. # Use same format to keep transparency of GIF/PNG nm = NamedTemporaryFile(suffix='.%s' % image.format) - default_storage = get_storage_class()() try: # Ensure conversion to float in operations image.thumbnail(get_dimensions(image.size, float(size)), Image.ANTIALIAS) @@ -67,7 +66,6 @@ def _save_thumbnails(image, original_path, size, suffix): def resize(filename): - default_storage = get_storage_class()() is_local = default_storage.__class__.__name__ == 'FileSystemStorage' image = None original_path = None @@ -101,7 +99,6 @@ def image_url(attachment, suffix): if suffix == 'original': return url else: - default_storage = get_storage_class()() if suffix in settings.THUMB_CONF: size = settings.THUMB_CONF[suffix]['suffix'] filename = attachment.media_file.name diff --git a/onadata/libs/utils/logger_tools.py b/onadata/libs/utils/logger_tools.py index 574456199..cce11209a 100644 --- a/onadata/libs/utils/logger_tools.py +++ b/onadata/libs/utils/logger_tools.py @@ -21,7 +21,7 @@ from dict2xml import dict2xml from django.conf import settings from django.core.exceptions import ValidationError, PermissionDenied -from django.core.files.storage import get_storage_class +from django.core.files.storage import default_storage from django.core.mail import mail_admins from django.db import IntegrityError, transaction from django.db.models import Q @@ -494,15 +494,12 @@ def response_with_mimetype_and_name( if file_path: try: if not use_local_filesystem: - default_storage = get_storage_class()() wrapper = FileWrapper(default_storage.open(file_path)) - response = StreamingHttpResponse(wrapper, - content_type=mimetype) + response = StreamingHttpResponse(wrapper, content_type=mimetype) response['Content-Length'] = default_storage.size(file_path) else: wrapper = FileWrapper(open(file_path)) - response = StreamingHttpResponse(wrapper, - content_type=mimetype) + response = StreamingHttpResponse(wrapper, content_type=mimetype) response['Content-Length'] = os.path.getsize(file_path) except IOError: response = HttpResponseNotFound( diff --git a/onadata/libs/utils/viewer_tools.py b/onadata/libs/utils/viewer_tools.py index 657745ae2..faf55bd3f 100644 --- a/onadata/libs/utils/viewer_tools.py +++ b/onadata/libs/utils/viewer_tools.py @@ -9,7 +9,7 @@ from tempfile import NamedTemporaryFile from django.conf import settings -from django.core.files.storage import get_storage_class, FileSystemStorage +from django.core.files.storage import default_storage, FileSystemStorage from django.core.files.uploadedfile import InMemoryUploadedFile from django.core.mail import mail_admins from django.utils.translation import gettext as t @@ -200,15 +200,25 @@ def no_seeking(*a, **kw): f'disabling seeking failed: {e}' ) - storage = get_storage_class()() - with zipfile.ZipFile(output_file, 'w', zipfile.ZIP_STORED, allowZip64=True) as zip_file: + with zipfile.ZipFile( + output_file, 'w', zipfile.ZIP_STORED, allowZip64=True + ) as zip_file: for attachment in attachments: - if storage.exists(attachment.media_file.name): + if default_storage.exists(attachment.media_file.name): try: - with storage.open(attachment.media_file.name, 'rb') as source_file: - zip_file.writestr(attachment.media_file.name, source_file.read()) + with default_storage.open( + attachment.media_file.name, 'rb' + ) as source_file: + zip_file.writestr( + attachment.media_file.name, source_file.read() + ) except Exception as e: - report_exception("Error adding file \"{}\" to archive.".format(attachment.media_file.name), e) + report_exception( + "Error adding file \"{}\" to archive.".format( + attachment.media_file.name + ), + e, + ) return output_file From 2da35615a476fcf8d9ae45c5c950151216bc158e Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Mon, 17 Jul 2023 09:06:18 -0400 Subject: [PATCH 12/23] Remove legacy media view --- .../logger/tests/test_briefcase_client.py | 16 ++- onadata/apps/main/urls.py | 10 -- onadata/apps/main/views.py | 104 ------------------ onadata/libs/utils/export_tools.py | 10 +- 4 files changed, 15 insertions(+), 125 deletions(-) diff --git a/onadata/apps/logger/tests/test_briefcase_client.py b/onadata/apps/logger/tests/test_briefcase_client.py index c9a09baa2..464e0a3cf 100644 --- a/onadata/apps/logger/tests/test_briefcase_client.py +++ b/onadata/apps/logger/tests/test_briefcase_client.py @@ -17,7 +17,6 @@ from onadata.apps.logger.views import download_xform from onadata.apps.main.models import MetaData from onadata.apps.main.tests.test_base import TestBase -from onadata.apps.main.views import download_media_data from onadata.libs.utils.briefcase_client import BriefcaseClient from onadata.libs.utils.storage import rmdir @@ -35,6 +34,11 @@ def xformsManifest(*args, **kwargs): # noqa response.render() return response +def xformsMedia(*args, **kwargs): # noqa + view = XFormListApi.as_view({'get': 'media'}) + response = view(*args, **kwargs) + return response + @urlmatch(netloc=r'(.*\.)?testserver$') def form_list_xml(url, request, **kwargs): @@ -60,10 +64,12 @@ def form_list_xml(url, request, **kwargs): res = download_xform(req, username='bob', id_string=id_string) elif url.path.find('xformsManifest') > -1: res = xformsManifest(req, username='bob', pk=xform_id) - elif url.path.find('formid-media') > -1: - data_id = url.path[url.path.rfind('/') + 1:] - res = download_media_data( - req, username='bob', id_string=id_string, data_id=data_id) + elif url.path.find('xformsMedia') > -1: + filename = url.path[url.path.rfind('/') + 1:] + metadata_id, _ = os.path.splitext(filename) + res = xformsMedia( + req, username='bob', pk=xform_id, metadata=metadata_id + ) response._content = get_streaming_content(res) else: res = formList(req, username='bob') diff --git a/onadata/apps/main/urls.py b/onadata/apps/main/urls.py index 71217acd8..11869ae1d 100644 --- a/onadata/apps/main/urls.py +++ b/onadata/apps/main/urls.py @@ -12,10 +12,6 @@ from onadata.apps.api.urls import XFormSubmissionApi from onadata.apps.api.urls import router, router_with_patch_list from onadata.apps.main.service_health import service_health -from onadata.apps.main.views import ( - # form specific - download_media_data, -) # exporting stuff from onadata.apps.viewer.views import ( @@ -70,12 +66,6 @@ RedirectView.as_view(url=koboform.redirect_url('/')), name='user_profile', ), - # form specific - re_path( - r'^(?P[^/]+)/forms/(?P[^/]+)/formid-media/(?P\d+)', - download_media_data, - name='download_media_data' - ), # briefcase api urls re_path(r"^(?P\w+)/view/submissionList$", diff --git a/onadata/apps/main/views.py b/onadata/apps/main/views.py index c206ebfa8..50004943e 100644 --- a/onadata/apps/main/views.py +++ b/onadata/apps/main/views.py @@ -1,109 +1,5 @@ # coding: utf-8 -import os - -import requests from django.conf import settings -from django.contrib.auth.decorators import login_required -from django.core.files.storage import get_storage_class -from django.urls import reverse -from django.http import HttpResponseForbidden -from django.http import HttpResponseNotFound -from django.http import HttpResponseRedirect -from django.http import HttpResponseServerError -from django.shortcuts import get_object_or_404 -from django.utils.translation import gettext as t - -from onadata.apps.logger.models import XForm -from onadata.apps.main.models import MetaData -from onadata.libs.utils.log import audit_log, Actions -from onadata.libs.utils.logger_tools import ( - check_submission_permissions, - response_with_mimetype_and_name, -) - - -def download_media_data(request, username, id_string, data_id): - xform = get_object_or_404( - XForm, user__username__iexact=username, - id_string__exact=id_string) - owner = xform.user - data = get_object_or_404(MetaData, id=data_id) - dfs = get_storage_class()() - if request.GET.get('del', False): - if username == request.user.username: - try: - # ensure filename is not an empty string - if data.data_file.name != '': - dfs.delete(data.data_file.name) - - data.delete() - audit = { - 'xform': xform.id_string - } - audit_log( - Actions.FORM_UPDATED, request.user, owner, - t("Media download '%(filename)s' deleted from " - "'%(id_string)s'.") % - { - 'id_string': xform.id_string, - 'filename': os.path.basename(data.data_file.name) - }, audit, request) - if ( - 'HTTP_REFERER' in request.META - and request.META['HTTP_REFERER'].strip() - ): - return HttpResponseRedirect(request.META['HTTP_REFERER']) - - return HttpResponseRedirect(reverse(show, kwargs={ - 'username': username, - 'id_string': id_string - })) - except Exception as e: - return HttpResponseServerError(e) - else: - if not xform.shared: - # raise an exception if the requesting user is not allowed to - # submit to this form (and therefore should not see this form media) - check_submission_permissions(request, xform) - - if data.data_file.name == '' and data.data_value is not None: - return HttpResponseRedirect(data.data_value) - - file_path = data.data_file.name - filename, extension = os.path.splitext(file_path.split('/')[-1]) - extension = extension.strip('.') - if dfs.exists(file_path): - audit = { - 'xform': xform.id_string - } - audit_log( - Actions.FORM_UPDATED, request.user, owner, - t("Media '%(filename)s' downloaded from " - "'%(id_string)s'.") % - { - 'id_string': xform.id_string, - 'filename': os.path.basename(file_path) - }, audit, request) - response = response_with_mimetype_and_name( - data.data_file_type, - filename, extension=extension, show_date=False, - file_path=file_path) - return response - else: - return HttpResponseNotFound() - - return HttpResponseForbidden(t('Permission denied.')) - - -def _make_authenticated_request(request, user): - """ - Make an authenticated request to KPI using the current session. - Returns response from KPI. - """ - return requests.get( - url=_get_migrate_url(user.username), - cookies={settings.SESSION_COOKIE_NAME: request.session.session_key} - ) def _get_migrate_url(username): diff --git a/onadata/libs/utils/export_tools.py b/onadata/libs/utils/export_tools.py index 16abf2881..96ca25ee0 100644 --- a/onadata/libs/utils/export_tools.py +++ b/onadata/libs/utils/export_tools.py @@ -678,7 +678,7 @@ def generate_attachments_zip_export( absolute_filename = _get_absolute_filename(file_path) - with get_storage_class()().open(absolute_filename, 'wb') as destination_file: + with default_storage.open(absolute_filename, 'wb') as destination_file: create_attachments_zipfile( attachments, output_file=destination_file, @@ -725,7 +725,7 @@ def generate_kml_export( export_type, filename) - storage = get_storage_class()() + storage = default_storage temp_file = NamedTemporaryFile(suffix=extension) temp_file.write(response.content) temp_file.seek(0) @@ -751,9 +751,7 @@ def generate_kml_export( def kml_export_data(id_string, user): - # TODO resolve circular import - from onadata.apps.viewer.models.data_dictionary import DataDictionary - dd = DataDictionary.objects.get(id_string=id_string, user=user) + instances = Instance.objects.filter( xform__user=user, xform__id_string=id_string, @@ -779,7 +777,7 @@ def _get_absolute_filename(filename: str) -> str: Get absolute filename related to storage root. """ - storage_class = get_storage_class()() + storage_class = default_storage filename = storage_class.generate_filename(filename) # We cannot call `self.result.save()` before reopening the file From 9b6baee2e001e5428ba30d3a3ff90550ae7217fe Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Mon, 17 Jul 2023 10:41:43 -0400 Subject: [PATCH 13/23] Move RestrictedAccessMiddleware to onadata.libs.utils.middleware.py --- onadata/libs/middleware.py | 122 ------------------------------- onadata/libs/utils/middleware.py | 122 ++++++++++++++++++++++++++++++- onadata/settings/base.py | 2 +- 3 files changed, 122 insertions(+), 124 deletions(-) delete mode 100644 onadata/libs/middleware.py diff --git a/onadata/libs/middleware.py b/onadata/libs/middleware.py deleted file mode 100644 index 1d32748d8..000000000 --- a/onadata/libs/middleware.py +++ /dev/null @@ -1,122 +0,0 @@ -from typing import Union - -from django.conf import settings -from django.contrib.auth import get_user_model -from django.http import HttpResponseForbidden -from django.template.loader import get_template -from django.utils.deprecation import MiddlewareMixin -from django.utils.translation import gettext as t -from kobo_service_account.models import ServiceAccountUser - -from onadata.libs.http import JsonResponseForbidden, XMLResponseForbidden - -# Define views (and viewsets) below. -# Viewset actions must specify (as a list) for each method. -# Legacy Django views do not need a list of actions, it can be empty. -# E.g. : Allow exports -# > 'export_list': { 'GET': [] } -# > 'create_export': { 'POST': [] } -ALLOWED_VIEWS_WITH_WEAK_PASSWORD = { - 'XFormListApi': { - 'GET': ['manifest', 'media', 'list', 'retrieve'], - }, - 'XFormSubmissionApi': { - 'POST': ['create'], - }, -} - - -class RestrictedAccessMiddleware(MiddlewareMixin): - - def __init__(self, get_response): - super().__init__(get_response) - self._allowed_view = None - - def process_response(self, request, response): - if not request.user.is_authenticated: - return response - - if isinstance(request.user, ServiceAccountUser): - return response - - try: - profile = request.user.profile - except get_user_model().profile.RelatedObjectDoesNotExist: - # Consider user's password as weak - if not self._allowed_view: - return self._render_response(response) - - if profile.validated_password: - return response - - if not self._allowed_view: - return self._render_response(response) - - return response - - def process_view(self, request, view, view_args, view_kwargs): - """ - Validate if view is among allowed one with unsafe password. - If it is not, set `self._allowed_view` to False to alter the - response in `process_response()`. - - We cannot validate user's password here because DRF authentication - takes places after this method call. Thus, `request.user` is always - anonymous if user is authenticated with something else than the session. - """ - view_name = view.__name__ - - # Reset boolean for each processed view - self._allowed_view = True - - if request.method == 'HEAD': - return - - try: - allowed_actions = ALLOWED_VIEWS_WITH_WEAK_PASSWORD[view_name][ - request.method - ] - except KeyError: - self._allowed_view = False - else: - if hasattr(view, 'actions'): - view_action = view.actions[request.method.lower()] - if view_action not in allowed_actions: - self._allowed_view = False - - return - - def _render_response( - self, response - ) -> Union[ - HttpResponseForbidden, JsonResponseForbidden, XMLResponseForbidden - ]: - """ - Render response in the requested format: HTML, JSON or XML. - If content type is not detected, fallback on HTML. - """ - template = get_template('restricted_access.html') - format_ = None - try: - content_type, *_ = response.accepted_media_type.split(';') - except AttributeError: - pass - else: - *_, format_ = content_type.split('/') - - if format_ not in ['xml', 'json']: - return HttpResponseForbidden(template.render()) - else: - data = { - 'detail': t( - f'Your access is restricted. Please reclaim your access by ' - f'changing your password at ' - f'{settings.KOBOFORM_URL}/accounts/password/reset/.' - ) - } - if format_ == 'json': - return JsonResponseForbidden(data) - else: - return XMLResponseForbidden( - data, renderer_context=response.renderer_context - ) diff --git a/onadata/libs/utils/middleware.py b/onadata/libs/utils/middleware.py index 773392524..c0d15ea77 100644 --- a/onadata/libs/utils/middleware.py +++ b/onadata/libs/utils/middleware.py @@ -1,11 +1,35 @@ # coding: utf-8 +from typing import Union +from django.conf import settings +from django.contrib.auth import get_user_model from django.db import connection +from django.http import HttpResponseForbidden from django.http import HttpResponseNotAllowed -from django.template import loader from django.middleware.locale import LocaleMiddleware +from django.template import loader +from django.template.loader import get_template from django.utils.deprecation import MiddlewareMixin +from django.utils.translation import gettext as t from django.utils.translation.trans_real import parse_accept_lang_header +from kobo_service_account.models import ServiceAccountUser + +from onadata.libs.http import JsonResponseForbidden, XMLResponseForbidden + +# Define views (and viewsets) below. +# Viewset actions must specify (as a list) for each method. +# Legacy Django views do not need a list of actions, it can be empty. +# E.g. : Allow exports +# > 'export_list': { 'GET': [] } +# > 'create_export': { 'POST': [] } +ALLOWED_VIEWS_WITH_WEAK_PASSWORD = { + 'XFormListApi': { + 'GET': ['manifest', 'media', 'list', 'retrieve'], + }, + 'XFormSubmissionApi': { + 'POST': ['create'], + }, +} class HTTPResponseNotAllowedMiddleware(MiddlewareMixin): @@ -49,6 +73,102 @@ def process_response(self, request, response): return response +class RestrictedAccessMiddleware(MiddlewareMixin): + + def __init__(self, get_response): + super().__init__(get_response) + self._allowed_view = None + + def process_response(self, request, response): + if not request.user.is_authenticated: + return response + + if isinstance(request.user, ServiceAccountUser): + return response + + try: + profile = request.user.profile + except get_user_model().profile.RelatedObjectDoesNotExist: + # Consider user's password as weak + if not self._allowed_view: + return self._render_response(response) + + if profile.validated_password: + return response + + if not self._allowed_view: + return self._render_response(response) + + return response + + def process_view(self, request, view, view_args, view_kwargs): + """ + Validate if view is among allowed one with unsafe password. + If it is not, set `self._allowed_view` to False to alter the + response in `process_response()`. + + We cannot validate user's password here because DRF authentication + takes places after this method call. Thus, `request.user` is always + anonymous if user is authenticated with something else than the session. + """ + view_name = view.__name__ + + # Reset boolean for each processed view + self._allowed_view = True + + if request.method == 'HEAD': + return + + try: + allowed_actions = ALLOWED_VIEWS_WITH_WEAK_PASSWORD[view_name][ + request.method + ] + except KeyError: + self._allowed_view = False + else: + if hasattr(view, 'actions'): + view_action = view.actions[request.method.lower()] + if view_action not in allowed_actions: + self._allowed_view = False + + return + + def _render_response( + self, response + ) -> Union[ + HttpResponseForbidden, JsonResponseForbidden, XMLResponseForbidden + ]: + """ + Render response in the requested format: HTML, JSON or XML. + If content type is not detected, fallback on HTML. + """ + template = get_template('restricted_access.html') + format_ = None + try: + content_type, *_ = response.accepted_media_type.split(';') + except AttributeError: + pass + else: + *_, format_ = content_type.split('/') + + if format_ not in ['xml', 'json']: + return HttpResponseForbidden(template.render()) + else: + data = { + 'detail': t( + f'Your access is restricted. Please reclaim your access by ' + f'changing your password at ' + f'{settings.KOBOFORM_URL}/accounts/password/reset/.' + ) + } + if format_ == 'json': + return JsonResponseForbidden(data) + else: + return XMLResponseForbidden( + data, renderer_context=response.renderer_context + ) + + class UsernameInResponseHeaderMiddleware(MiddlewareMixin): """ Record the authenticated user (if any) in the `X-KoBoNaUt` HTTP header diff --git a/onadata/settings/base.py b/onadata/settings/base.py index e791f0b4b..49b1ae84a 100644 --- a/onadata/settings/base.py +++ b/onadata/settings/base.py @@ -122,7 +122,7 @@ def skip_suspicious_operations(record): 'django.middleware.csrf.CsrfViewMiddleware', 'corsheaders.middleware.CorsMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', - 'onadata.libs.middleware.RestrictedAccessMiddleware', + 'onadata.libs.utils.middleware.RestrictedAccessMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'onadata.libs.utils.middleware.HTTPResponseNotAllowedMiddleware', 'readonly.middleware.DatabaseReadOnlyMiddleware', From e5a32f66152ebecfd2824dd83024f710ea084b9c Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Mon, 17 Jul 2023 10:42:30 -0400 Subject: [PATCH 14/23] Fix test unique together unit test with new middleware --- .../test_user_id_string_unique_together.py | 2 ++ onadata/libs/utils/logger_tools.py | 22 +++++++++++++------ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/onadata/apps/main/tests/test_user_id_string_unique_together.py b/onadata/apps/main/tests/test_user_id_string_unique_together.py index ac9279ec3..a1a444578 100644 --- a/onadata/apps/main/tests/test_user_id_string_unique_together.py +++ b/onadata/apps/main/tests/test_user_id_string_unique_together.py @@ -1,6 +1,8 @@ # coding: utf-8 import os +from django.db import transaction, IntegrityError + from onadata.apps.logger.models import XForm from .test_base import TestBase diff --git a/onadata/libs/utils/logger_tools.py b/onadata/libs/utils/logger_tools.py index cce11209a..6774b34b8 100644 --- a/onadata/libs/utils/logger_tools.py +++ b/onadata/libs/utils/logger_tools.py @@ -7,17 +7,12 @@ import traceback from datetime import date, datetime from xml.parsers.expat import ExpatError - -from onadata.apps.logger.signals import ( - update_user_profile_attachment_storage_bytes, - update_xform_attachment_storage_bytes, -) - try: from zoneinfo import ZoneInfo except ImportError: from backports.zoneinfo import ZoneInfo + from dict2xml import dict2xml from django.conf import settings from django.core.exceptions import ValidationError, PermissionDenied @@ -60,6 +55,10 @@ update_xform_submission_count, ) from onadata.apps.logger.models.xform import XLSFormError +from onadata.apps.logger.signals import ( + update_user_profile_attachment_storage_bytes, + update_xform_attachment_storage_bytes, +) from onadata.apps.logger.xform_instance_parser import ( InstanceEmptyError, InstanceInvalidUserError, @@ -441,7 +440,16 @@ def publish_xls_form(xls_file, user, id_string=None): dd.save() return dd else: - return DataDictionary.objects.create(user=user, xls=xls_file) + # Creation needs to be wrapped in a transaction because of unit tests. + # It raises `TransactionManagementError` on IntegrityError in + # `RestrictedAccessMiddleware` when accessing `request.user.profile`. + # See https://stackoverflow.com/a/23326971 + try: + with transaction.atomic(): + dd = DataDictionary.objects.create(user=user, xls=xls_file) + except IntegrityError as e: + raise e + return dd def publish_xml_form(xml_file, user, id_string=None): From 5633ae171f28405a06f14ec0c1e668e14bb53d67 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Tue, 18 Jul 2023 13:18:54 -0400 Subject: [PATCH 15/23] Remove "password_date_changed" from UserProfile model --- .../migrations/0012_add_validate_password_flag_to_profile.py | 5 ----- onadata/apps/main/models/user_profile.py | 1 - 2 files changed, 6 deletions(-) diff --git a/onadata/apps/main/migrations/0012_add_validate_password_flag_to_profile.py b/onadata/apps/main/migrations/0012_add_validate_password_flag_to_profile.py index 04398f22f..8a83afe9d 100644 --- a/onadata/apps/main/migrations/0012_add_validate_password_flag_to_profile.py +++ b/onadata/apps/main/migrations/0012_add_validate_password_flag_to_profile.py @@ -11,11 +11,6 @@ class Migration(migrations.Migration): ] operations = [ - migrations.AddField( - model_name='userprofile', - name='password_date_changed', - field=models.DateTimeField(default=django.utils.timezone.now), - ), migrations.AddField( model_name='userprofile', name='validated_password', diff --git a/onadata/apps/main/models/user_profile.py b/onadata/apps/main/models/user_profile.py index daf7cca6b..0b4217c53 100644 --- a/onadata/apps/main/models/user_profile.py +++ b/onadata/apps/main/models/user_profile.py @@ -41,7 +41,6 @@ class UserProfile(models.Model): metadata = models.JSONField(default=dict, blank=True) is_mfa_active = LazyDefaultBooleanField(default=False) validated_password = models.BooleanField(default=True) - password_date_changed = models.DateTimeField(default=timezone.now) def __str__(self): return '%s[%s]' % (self.name, self.user.username) From 632d0b127013519698d9cfbbacf1532a002cbc4f Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Mon, 7 Aug 2023 11:50:29 -0400 Subject: [PATCH 16/23] Make URL placeholder for invalid password message --- onadata/libs/utils/middleware.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/onadata/libs/utils/middleware.py b/onadata/libs/utils/middleware.py index c0d15ea77..d30a085dd 100644 --- a/onadata/libs/utils/middleware.py +++ b/onadata/libs/utils/middleware.py @@ -156,10 +156,10 @@ def _render_response( else: data = { 'detail': t( - f'Your access is restricted. Please reclaim your access by ' - f'changing your password at ' - f'{settings.KOBOFORM_URL}/accounts/password/reset/.' - ) + 'Your access is restricted. Please reclaim your access by ' + 'changing your password at ' + '##koboform_url##/accounts/password/reset/.' + ).replace('##koboform_url##', settings.KOBOFORM_URL) } if format_ == 'json': return JsonResponseForbidden(data) From c0b2afd9b8516153ed5dba4a5b6ac1080312f446 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Tue, 8 Aug 2023 14:30:04 -0400 Subject: [PATCH 17/23] Fix typo in unit tests --- onadata/apps/api/tests/viewsets/test_user.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/onadata/apps/api/tests/viewsets/test_user.py b/onadata/apps/api/tests/viewsets/test_user.py index c731e2113..bc8f8f78e 100644 --- a/onadata/apps/api/tests/viewsets/test_user.py +++ b/onadata/apps/api/tests/viewsets/test_user.py @@ -168,10 +168,14 @@ def _access_endpoints(self, access_granted: bool, headers: dict = {}): The list of endpoints is not exhaustive but should cover the main ones. TODO: Support other methods """ - status_code = status.HTTP_200_OK if access_granted else status.HTTP_403_FORBIDDEN - xform_id = self.xform.instances.all()[0].pk - instance_id = self.xform.instances.all()[0].pk - attachment_id = self.xform.instances.all()[0].attachments.all()[0].pk + status_code = ( + status.HTTP_200_OK if access_granted else status.HTTP_403_FORBIDDEN + ) + xform_id = self.xform.pk + instance_id = self.xform.instances.all().order_by('id')[0].pk + attachment_id = ( + self.xform.instances.all()[0].attachments.all().order_by('id')[0].pk + ) # /api/v1/forms response = self.client.get(reverse('xform-list'), **headers) From 3b294303d6ce1987763030ad09001328a9da5da3 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Tue, 25 Jul 2023 09:01:36 -0400 Subject: [PATCH 18/23] Bump pip-tools version to 7.x.x --- .github/workflows/pytest.yml | 2 +- Dockerfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 878a819ac..fc249eaba 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -39,7 +39,7 @@ jobs: with: python-version: ${{ matrix.python-version }} - name: Install pip-tools - run: python -m pip install pip-tools==6.\* + run: python -m pip install pip-tools==7.\* - name: Update apt package lists run: sudo apt update - name: Update Debian package lists diff --git a/Dockerfile b/Dockerfile index ed2e043b7..517917f61 100644 --- a/Dockerfile +++ b/Dockerfile @@ -6,7 +6,7 @@ ENV VIRTUAL_ENV=/opt/venv \ RUN python -m venv "$VIRTUAL_ENV" ENV PATH="$VIRTUAL_ENV/bin:$PATH" -RUN pip install --quiet pip-tools==6.\* +RUN pip install --quiet pip-tools==7.\* COPY ./dependencies/pip/requirements.txt "${TMP_DIR}/pip_dependencies.txt" RUN pip-sync "${TMP_DIR}/pip_dependencies.txt" 1>/dev/null From f6f2459e31ee563efa38836668d7e903c4ac5f02 Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Tue, 8 Aug 2023 14:45:16 -0400 Subject: [PATCH 19/23] Fix CI with branch name which contains # --- .gitlab-ci.yml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index e37c62d40..bdbfb3682 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -14,11 +14,12 @@ build: before_script: - docker login -u gitlab-ci-token -p $CI_JOB_TOKEN registry.gitlab.com script: - - docker pull $CI_REGISTRY_IMAGE:${CI_COMMIT_REF_NAME//\//-} || true - - docker build --cache-from $CI_REGISTRY_IMAGE:${CI_COMMIT_REF_NAME//\//-} --tag $CI_REGISTRY_IMAGE:$CI_COMMIT_SHORT_SHA --tag $CI_REGISTRY_IMAGE:${CI_COMMIT_REF_NAME//\//-} . + - CI_COMMIT_REF_NAME_SANITIZED=${CI_COMMIT_REF_NAME/\#/-} + - CI_COMMIT_REF_NAME_SANITIZED=${CI_COMMIT_REF_NAME_SANITIZED//\//-} + - docker pull $CI_REGISTRY_IMAGE:$CI_COMMIT_REF_NAME_SANITIZED || true + - docker build --cache-from $CI_REGISTRY_IMAGE:$CI_COMMIT_REF_NAME_SANITIZED --tag $CI_REGISTRY_IMAGE:$CI_COMMIT_SHORT_SHA --tag $CI_REGISTRY_IMAGE:$CI_COMMIT_REF_NAME_SANITIZED . - docker push $CI_REGISTRY_IMAGE:$CI_COMMIT_SHORT_SHA - - docker push $CI_REGISTRY_IMAGE:${CI_COMMIT_REF_NAME//\//-} - + - docker push $CI_REGISTRY_IMAGE:$CI_COMMIT_REF_NAME_SANITIZED test: stage: build_test image: python:3.10 From 14b8fd452b1787816884518359f5164d42804dca Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Wed, 16 Aug 2023 10:31:24 -0400 Subject: [PATCH 20/23] Remove unused imports --- onadata/apps/main/tests/test_form_exports.py | 3 --- onadata/apps/viewer/tests/test_export_list.py | 1 - onadata/apps/viewer/tests/test_pandas_mongo_bridge.py | 1 - 3 files changed, 5 deletions(-) diff --git a/onadata/apps/main/tests/test_form_exports.py b/onadata/apps/main/tests/test_form_exports.py index 3abb1cb44..20560302c 100644 --- a/onadata/apps/main/tests/test_form_exports.py +++ b/onadata/apps/main/tests/test_form_exports.py @@ -1,12 +1,9 @@ # coding: utf-8 -import os -import time import csv from io import BytesIO from django.urls import reverse from django.core.files.storage import get_storage_class, FileSystemStorage -from django.utils import timezone from openpyxl import load_workbook from onadata.apps.viewer.models.export import Export diff --git a/onadata/apps/viewer/tests/test_export_list.py b/onadata/apps/viewer/tests/test_export_list.py index 1a5519a13..7f1a6af84 100644 --- a/onadata/apps/viewer/tests/test_export_list.py +++ b/onadata/apps/viewer/tests/test_export_list.py @@ -6,7 +6,6 @@ from onadata.apps.main.tests.test_base import TestBase from onadata.apps.viewer.models.export import Export -from onadata.apps.main.models.meta_data import MetaData from onadata.apps.viewer.views import export_list diff --git a/onadata/apps/viewer/tests/test_pandas_mongo_bridge.py b/onadata/apps/viewer/tests/test_pandas_mongo_bridge.py index 2aad28469..e64195dee 100644 --- a/onadata/apps/viewer/tests/test_pandas_mongo_bridge.py +++ b/onadata/apps/viewer/tests/test_pandas_mongo_bridge.py @@ -4,7 +4,6 @@ from tempfile import NamedTemporaryFile from django.utils.dateparse import parse_datetime -from django.urls import reverse from onadata.apps.main.tests.test_base import TestBase from onadata.apps.logger.models.xform import XForm From 81766dbc1a8ffabb798275a0cca6803c4e0c55bb Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Thu, 17 Aug 2023 11:19:38 -0400 Subject: [PATCH 21/23] Apply requested changes --- onadata/apps/main/tests/test_user_id_string_unique_together.py | 2 -- onadata/libs/utils/middleware.py | 3 ++- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/onadata/apps/main/tests/test_user_id_string_unique_together.py b/onadata/apps/main/tests/test_user_id_string_unique_together.py index a1a444578..ac9279ec3 100644 --- a/onadata/apps/main/tests/test_user_id_string_unique_together.py +++ b/onadata/apps/main/tests/test_user_id_string_unique_together.py @@ -1,8 +1,6 @@ # coding: utf-8 import os -from django.db import transaction, IntegrityError - from onadata.apps.logger.models import XForm from .test_base import TestBase diff --git a/onadata/libs/utils/middleware.py b/onadata/libs/utils/middleware.py index d30a085dd..74129c784 100644 --- a/onadata/libs/utils/middleware.py +++ b/onadata/libs/utils/middleware.py @@ -103,7 +103,8 @@ def process_response(self, request, response): def process_view(self, request, view, view_args, view_kwargs): """ - Validate if view is among allowed one with unsafe password. + Validate if the view is among the ones allowed to be accessed with + an unsafe password. If it is not, set `self._allowed_view` to False to alter the response in `process_response()`. From 2c4981ebf7fee115ef89a43fd084c23d79578c0e Mon Sep 17 00:00:00 2001 From: Olivier Leger Date: Fri, 18 Aug 2023 08:26:46 -0400 Subject: [PATCH 22/23] Fix context_processor not being evaluated in restricted_access template --- onadata/libs/utils/middleware.py | 16 ++++++++++------ onadata/settings/base.py | 3 ++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/onadata/libs/utils/middleware.py b/onadata/libs/utils/middleware.py index 74129c784..96b8836df 100644 --- a/onadata/libs/utils/middleware.py +++ b/onadata/libs/utils/middleware.py @@ -4,8 +4,12 @@ from django.conf import settings from django.contrib.auth import get_user_model from django.db import connection -from django.http import HttpResponseForbidden -from django.http import HttpResponseNotAllowed +from django.http import ( + HttpResponseNotAllowed, + HttpResponseForbidden, + HttpRequest, + HttpResponse, +) from django.middleware.locale import LocaleMiddleware from django.template import loader from django.template.loader import get_template @@ -91,13 +95,13 @@ def process_response(self, request, response): except get_user_model().profile.RelatedObjectDoesNotExist: # Consider user's password as weak if not self._allowed_view: - return self._render_response(response) + return self._render_response(request, response) if profile.validated_password: return response if not self._allowed_view: - return self._render_response(response) + return self._render_response(request, response) return response @@ -135,7 +139,7 @@ def process_view(self, request, view, view_args, view_kwargs): return def _render_response( - self, response + self, request: HttpRequest, response: HttpResponse ) -> Union[ HttpResponseForbidden, JsonResponseForbidden, XMLResponseForbidden ]: @@ -153,7 +157,7 @@ def _render_response( *_, format_ = content_type.split('/') if format_ not in ['xml', 'json']: - return HttpResponseForbidden(template.render()) + return HttpResponseForbidden(template.render(request=request)) else: data = { 'detail': t( diff --git a/onadata/settings/base.py b/onadata/settings/base.py index d5c3b3f82..8bad1a5ad 100644 --- a/onadata/settings/base.py +++ b/onadata/settings/base.py @@ -149,7 +149,6 @@ def skip_suspicious_operations(record): ], 'OPTIONS': { 'context_processors': [ - 'onadata.koboform.context_processors.koboform_integration', 'django.contrib.auth.context_processors.auth', 'django.template.context_processors.debug', 'django.template.context_processors.i18n', @@ -159,6 +158,8 @@ def skip_suspicious_operations(record): 'django.template.context_processors.request', 'django.contrib.messages.context_processors.messages', 'readonly.context_processors.readonly', + # Additional processors + 'onadata.koboform.context_processors.koboform_integration', 'onadata.apps.main.context_processors.google_analytics', 'onadata.apps.main.context_processors.site_name', 'onadata.apps.main.context_processors.base_url' From 7cb02a2470d8da2c0638da109f5a31b698940180 Mon Sep 17 00:00:00 2001 From: "John N. Milner" Date: Tue, 29 Aug 2023 22:54:44 -0400 Subject: [PATCH 23/23] Assert redirect location is correct; remove print --- onadata/apps/main/tests/test_form_auth.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/onadata/apps/main/tests/test_form_auth.py b/onadata/apps/main/tests/test_form_auth.py index a767afbc3..48bec7566 100644 --- a/onadata/apps/main/tests/test_form_auth.py +++ b/onadata/apps/main/tests/test_form_auth.py @@ -1,4 +1,5 @@ # coding: utf-8 +from django.conf import settings from django.urls import reverse from .test_base import TestBase @@ -13,11 +14,17 @@ def test_home_redirects(self): self._create_user_and_login(username='bob', password='bob') response = self.client.get(reverse('home')) self.assertEqual(response.status_code, 302) - print("response['Location']", response['Location'], flush=True) + received, desired = ( + u.rstrip('/') for u in (response['Location'], settings.KOBOFORM_URL) + ) + assert received == desired def test_profile_redirects(self): self._create_user_and_login(username='bob', password='bob') url = reverse('user_profile', kwargs={'username': self.user.username}) response = self.client.get(url) self.assertEqual(response.status_code, 302) - print("response['Location']", response['Location'], flush=True) + received, desired = ( + u.rstrip('/') for u in (response['Location'], settings.KOBOFORM_URL) + ) + assert received == desired