Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: value error exception for non numeric project id #2741

Merged
merged 1 commit into from
Nov 25, 2024
Merged

fix: value error exception for non numeric project id #2741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 53 additions & 3 deletions onadata/apps/api/tests/viewsets/test_xform_submission_viewset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"""
Test XFormSubmissionViewSet module.
"""

import os
from builtins import open # pylint: disable=redefined-builtin
from unittest.mock import patch
Expand All @@ -11,6 +12,8 @@
from django.core.files.uploadedfile import InMemoryUploadedFile
from django.http import UnreadablePostError
from django.test import TransactionTestCase
from django.urls.exceptions import NoReverseMatch
from rest_framework.reverse import reverse

import simplejson as json
from django_digest.test import DigestAuth
Expand Down Expand Up @@ -843,9 +846,7 @@ def test_floip_format_multiple_rows_instance(self):
data_responses = [i[4] for i in json.loads(data)]
self.assertTrue(any(i in data_responses for i in instance_json.values()))

@patch(
"onadata.apps.api.viewsets.xform_submission_viewset.SubmissionSerializer"
) # noqa
@patch("onadata.apps.api.viewsets.xform_submission_viewset.SubmissionSerializer") # noqa
def test_post_submission_unreadable_post_error(self, MockSerializer):
"""
Test UnreadablePostError exception during submission..
Expand Down Expand Up @@ -1232,6 +1233,55 @@ def test_post_submission_using_project_pk_while_authenticated(self):
Instance.objects.filter(xform=self.xform).count(), count + 1
)

def test_post_submission_using_project_pk_exceptions(self):
"""
Test that one is able to submit data using the project
submission endpoint built for a particular form through
it's primary key while authenticated
"""
with self.assertRaises(NoReverseMatch):
_url = reverse("submission", kwargs={"project_id": "mission"})
s = self.surveys[0]
media_file = "1335783522563.jpg"
path = os.path.join(
self.main_directory,
"fixtures",
"transportation",
"instances",
s,
media_file,
)
with open(path, "rb") as f:
f = InMemoryUploadedFile(
f, "media_file", media_file, "image/jpg", os.path.getsize(path), None
)
submission_path = os.path.join(
self.main_directory,
"fixtures",
"transportation",
"instances",
s,
s + ".xml",
)
with open(submission_path, "rb") as sf:
data = {"xml_submission_file": sf, "media_file": f}
count = Instance.objects.filter(xform=self.xform).count()
request = self.factory.post(
f"/projects/{self.xform.project.pk}hello/submission", data
)
response = self.view(request)
self.assertEqual(response.status_code, 401)
auth = DigestAuth("bob", "bobbob")
request.META.update(auth(request.META, response))
response = self.view(request, project_pk=f"{self.project.pk}hello")
self.assertEqual(response.status_code, 400)
ukanga marked this conversation as resolved.
Show resolved Hide resolved
self.assertTrue(response.has_header("X-OpenRosa-Version"))
self.assertTrue(response.has_header("X-OpenRosa-Accept-Content-Length"))
self.assertTrue(response.has_header("Date"))
self.assertEqual(
Instance.objects.filter(xform=self.xform).count(), count
)

@patch.object(ServiceDefinition, "send")
def test_new_submission_sent_to_rapidpro(self, mock_send):
"""Submission created is sent to RapidPro"""
Expand Down
17 changes: 9 additions & 8 deletions onadata/apps/main/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"""
URLs path.
"""

import sys

import django
Expand Down Expand Up @@ -203,7 +204,7 @@
name="view-submission-list",
),
re_path(
r"^forms/(?P<xform_pk>\w+)/view/submissionList$",
r"^forms/(?P<xform_pk>\d+)/view/submissionList$",
BriefcaseViewset.as_view({"get": "list", "head": "list"}),
name="view-submission-list",
),
Expand All @@ -218,7 +219,7 @@
name="view-download-submission",
),
re_path(
r"^forms/(?P<xform_pk>\w+)/view/downloadSubmission$",
r"^forms/(?P<xform_pk>\d+)/view/downloadSubmission$",
BriefcaseViewset.as_view({"get": "retrieve", "head": "retrieve"}),
name="view-download-submission",
),
Expand Down Expand Up @@ -377,17 +378,17 @@
name="form-list",
),
re_path(
r"^enketo/(?P<xform_pk>\w+)/formList$",
r"^enketo/(?P<xform_pk>\d+)/formList$",
XFormListViewSet.as_view({"get": "list", "head": "list"}),
name="form-list",
),
re_path(
r"^forms/(?P<xform_pk>\w+)/formList$",
r"^forms/(?P<xform_pk>\d+)/formList$",
XFormListViewSet.as_view({"get": "list", "head": "list"}),
name="form-list",
),
re_path(
r"^enketo-preview/(?P<xform_pk>\w+)/formList$",
r"^enketo-preview/(?P<xform_pk>\d+)/formList$",
PreviewXFormListViewSet.as_view({"get": "list", "head": "list"}),
name="form-list",
),
Expand Down Expand Up @@ -444,17 +445,17 @@
name="submissions",
),
re_path(
r"^enketo/(?P<xform_pk>\w+)/submission$",
r"^enketo/(?P<xform_pk>\d+)/submission$",
XFormSubmissionViewSet.as_view({"post": "create", "head": "create"}),
name="submissions",
),
re_path(
r"^projects/(?P<project_pk>\w+)/submission$",
r"^projects/(?P<project_pk>\d+)/submission$",
XFormSubmissionViewSet.as_view({"post": "create", "head": "create"}),
name="submissions",
),
re_path(
r"^forms/(?P<xform_pk>\w+)/submission$",
r"^forms/(?P<xform_pk>\d+)/submission$",
XFormSubmissionViewSet.as_view({"post": "create", "head": "create"}),
name="submissions",
),
Expand Down
23 changes: 18 additions & 5 deletions onadata/libs/serializers/data_serializer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"""
Submission data serializers module.
"""

from io import BytesIO

from django.shortcuts import get_object_or_404
Expand All @@ -13,6 +14,7 @@

from onadata.apps.logger.models import Project, XForm
from onadata.apps.logger.models.instance import Instance, InstanceHistory
from onadata.libs.data import parse_int
from onadata.libs.serializers.fields.json_field import JsonField
from onadata.libs.utils.analytics import TrackObjectEvent
from onadata.libs.utils.common_tags import (
Expand Down Expand Up @@ -56,11 +58,19 @@
# get the username from the XForm object if form_id is
# present else utilize the request users username
if form_pk:
form = get_object_or_404(XForm, pk=form_pk)
username = form.user.username
form_pk = parse_int(form_pk)
if form_pk:
form = get_object_or_404(XForm, pk=form_pk)
username = form.user.username
else:
raise ValueError(_("Invalid XForm id."))
elif project_pk:
project = get_object_or_404(Project, pk=project_pk)
username = project.user.username
project_pk = parse_int(project_pk)
if project_pk:
project = get_object_or_404(Project, pk=project_pk)
username = project.user.username
else:
raise ValueError(_("Invalid Project id."))
else:
username = request.user and request.user.username

Expand Down Expand Up @@ -296,7 +306,10 @@
pass

def validate(self, attrs):
request, __ = get_request_and_username(self.context)
try:
request, __ = get_request_and_username(self.context)
except ValueError as exc:
raise serializers.ValidationError(str(exc))

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
Loading
flows to this location and may be exposed to an external user.

Copilot Autofix AI about 2 months ago

To fix the problem, we should avoid exposing the detailed exception message to the end user. Instead, we should log the detailed error message on the server and return a generic error message to the user. This can be achieved by using Python's logging module to log the exception details and raising a serializers.ValidationError with a generic message.

Suggested changeset 1
onadata/libs/serializers/data_serializer.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/onadata/libs/serializers/data_serializer.py b/onadata/libs/serializers/data_serializer.py
--- a/onadata/libs/serializers/data_serializer.py
+++ b/onadata/libs/serializers/data_serializer.py
@@ -311,3 +311,6 @@
         except ValueError as exc:
-            raise serializers.ValidationError(str(exc))
+            import logging
+            logger = logging.getLogger(__name__)
+            logger.error("Error in getting request and username", exc_info=True)
+            raise serializers.ValidationError(_("An internal error has occurred. Please try again later."))
         if not request.FILES or "xml_submission_file" not in request.FILES:
EOF
@@ -311,3 +311,6 @@
except ValueError as exc:
raise serializers.ValidationError(str(exc))
import logging
logger = logging.getLogger(__name__)
logger.error("Error in getting request and username", exc_info=True)
raise serializers.ValidationError(_("An internal error has occurred. Please try again later."))
if not request.FILES or "xml_submission_file" not in request.FILES:
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
if not request.FILES or "xml_submission_file" not in request.FILES:
raise serializers.ValidationError(_("No XML submission file."))

Expand Down
Loading