Skip to content

Commit

Permalink
fix(permissions): restore edit functionality for users with partial p…
Browse files Browse the repository at this point in the history
…ermissions TASK-1366 (#5351)

### 📣 Summary
Fixed an issue where users with partial permissions could no longer edit
submissions they were allowed to.


### 📖 Description
Users with partial permissions faced an issue where they were unable to
edit submissions they had proper access to. This fix resolves the
problem, allowing these users to edit submissions within their
permissions scope, while maintaining the integrity of restricted
resources.
  • Loading branch information
noliveleger authored Dec 16, 2024
1 parent 953f2c9 commit 534edaf
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 8 deletions.
12 changes: 8 additions & 4 deletions kpi/deployment_backends/openrosa_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,6 @@ def edit_submission(
The returned Response should be in XML (expected format by Enketo Express)
"""
user = request.user

submission_xml = xml_submission_file.read()
try:
xml_root = fromstring_preserve_root_xmlns(submission_xml)
Expand Down Expand Up @@ -355,10 +354,17 @@ def edit_submission(
)

# Validate write access for users with partial permissions
self.validate_access_with_partial_perms(
submission_ids = self.validate_access_with_partial_perms(
user=user, perm=PERM_CHANGE_SUBMISSIONS, submission_ids=[instance.pk]
)

if submission_ids:
# If `submission_ids` is not empty, it indicates the user has partial
# permissions and has successfully passed validation. Therefore, set the
# `has_partial_perms` attribute on `request.user` to grant the necessary
# permissions when invoking `logger_tool.py::_has_edit_xform_permission()`.
user.has_partial_perms = True

# Set the In-Memory file’s current position to 0 before passing it to
# Request.
xml_submission_file.seek(0)
Expand Down Expand Up @@ -979,8 +985,6 @@ def set_validation_status(
submission_ids=[submission_id],
)

# TODO simplify response when KobocatDeploymentBackend
# and MockDeploymentBackend are gone
try:
instance = Instance.objects.only('validation_status', 'date_modified').get(
pk=submission_id
Expand Down
4 changes: 4 additions & 0 deletions kpi/tests/api/v1/test_api_submissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@ def test_edit_submission_snapshot_missing(self):
def test_edit_submission_snapshot_missing_unauthenticated(self):
pass

@pytest.mark.skip(reason='Only usable in v2')
def test_edit_submission_with_partial_perms(self):
pass


class SubmissionValidationStatusApiTests(test_api_submissions.SubmissionValidationStatusApiTests): # noqa: E501

Expand Down
72 changes: 71 additions & 1 deletion kpi/tests/api/v2/test_api_submissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import pytest
import responses
from django.conf import settings
from django.core.files.base import ContentFile
from django.urls import reverse
from django_digest.test import Client as DigestClient
from rest_framework import status
Expand Down Expand Up @@ -49,7 +50,11 @@
from kpi.tests.utils.xml import get_form_and_submission_tag_names
from kpi.urls.router_api_v2 import URL_NAMESPACE as ROUTER_URL_NAMESPACE
from kpi.utils.object_permission import get_anonymous_user
from kpi.utils.xml import fromstring_preserve_root_xmlns, xml_tostring
from kpi.utils.xml import (
edit_submission_xml,
fromstring_preserve_root_xmlns,
xml_tostring,
)


def dict2xform_with_namespace(submission: dict, xform_id_string: str) -> str:
Expand Down Expand Up @@ -1718,6 +1723,71 @@ def test_edit_submission_snapshot_missing_unauthenticated(self):
req = client.post(url)
self.assertEqual(req.status_code, status.HTTP_404_NOT_FOUND)

def test_edit_submission_with_partial_perms(self):
# Use Digest authentication; testing SessionAuth is not required.
# The purpose of this test is to validate partial permissions.
submission = self.submissions_submitted_by_anotheruser[0]
instance_xml = self.asset.deployment.get_submission(
submission['_id'], self.asset.owner, format_type='xml'
)
xml_parsed = fromstring_preserve_root_xmlns(instance_xml)
edit_submission_xml(
xml_parsed, 'meta/deprecatedID', submission['meta/instanceID']
)
edit_submission_xml(xml_parsed, 'meta/instanceID', 'foo')
edited_submission = xml_tostring(xml_parsed)

url = reverse(
self._get_endpoint('assetsnapshot-submission-alias'),
args=(self.asset.snapshot().uid,),
)
self.client.logout()
client = DigestClient()
req = client.post(url) # Retrieve www-challenge

client.set_authorization('anotheruser', 'anotheruser', 'Digest')
self.anotheruser.set_password('anotheruser')
self.anotheruser.save()
req = client.post(url)
self.assertEqual(req.status_code, status.HTTP_401_UNAUTHORIZED)

self.asset.assign_perm(
self.anotheruser,
PERM_PARTIAL_SUBMISSIONS,
partial_perms={
PERM_VIEW_SUBMISSIONS: [{'_submitted_by': 'anotheruser'}]
},
)
req = client.post(url)
self.assertEqual(req.status_code, status.HTTP_401_UNAUTHORIZED)

# Give anotheruser permissions to edit submissions someuser's data.
self.asset.assign_perm(
self.anotheruser,
PERM_PARTIAL_SUBMISSIONS,
partial_perms={
PERM_CHANGE_SUBMISSIONS: [{'_submitted_by': 'someuser'}]
},
)

data = {'xml_submission_file': ContentFile(edited_submission)}
response = client.post(url, data)
# Receive a 403 because we are trying to edit anotheruser's data
assert response.status_code == status.HTTP_403_FORBIDDEN

# Give anotheruser permissions to edit submissions their data.
self.asset.assign_perm(
self.anotheruser,
PERM_PARTIAL_SUBMISSIONS,
partial_perms={
PERM_CHANGE_SUBMISSIONS: [{'_submitted_by': 'anotheruser'}]
},
)

data = {'xml_submission_file': ContentFile(edited_submission)}
response = client.post(url, data)
assert response.status_code == status.HTTP_201_CREATED


class SubmissionViewApiTests(BaseSubmissionTestCase):

Expand Down
3 changes: 2 additions & 1 deletion kpi/tests/test_deployment_backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ def test_sync_media_files(self):
).first()

assert default_kobocat_storage.exists(str(meta_data.data_file))
assert not default_storage.exists(str(meta_data.data_file))
if default_storage.__class__.__name__ == 'FileSystemStorage':
assert not default_storage.exists(str(meta_data.data_file))

with default_kobocat_storage.open(
str(meta_data.data_file), 'r'
Expand Down
2 changes: 0 additions & 2 deletions kpi/views/v2/asset_snapshot.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,12 +220,10 @@ def submission(self, request, *args, **kwargs):
return self.get_response_for_head_request()

asset_snapshot = self.get_object()

xml_submission_file = request.data['xml_submission_file']

# Remove 'xml_submission_file' since it is already handled
request.FILES.pop('xml_submission_file')

try:
with http_open_rosa_error_handler(
lambda: asset_snapshot.asset.deployment.edit_submission(
Expand Down

0 comments on commit 534edaf

Please sign in to comment.