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.

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

Code refactoring: Remove kobo_service_account dependency #5045

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
72ea390
Remove kobo_service_account dependency
noliveleger Jul 31, 2024
d40c334
Merge branch 'kobocat-django-app-part-2-remove-kobocat-rest-service' …
noliveleger Aug 6, 2024
276d8e9
Fix bug on redeploy
noliveleger Aug 6, 2024
726fb9a
Refactor MockDeploymentBackend: fix unit tests
noliveleger Aug 6, 2024
e6139ca
Fix last tests
noliveleger Aug 8, 2024
244c38e
Add pytest-xdist pip dependency
noliveleger Aug 8, 2024
be7505e
Fix unit tests with FileSystemStorage
noliveleger Aug 8, 2024
9c9a300
Remove deprecated methods
noliveleger Aug 8, 2024
735817d
Improve error message when XForm does not exist
noliveleger Aug 22, 2024
425d624
Refactor delete action and bulk update
noliveleger Aug 22, 2024
821686f
Use contextmanager to handle create_instance errors
noliveleger Aug 22, 2024
3698143
Remove unused import
noliveleger Aug 23, 2024
a056a74
Fix unit tests
noliveleger Aug 23, 2024
78eb204
Remove deprecated documentation
noliveleger Aug 28, 2024
2dabaec
Fix wrong attribute
noliveleger Sep 4, 2024
57eb2b2
Merge branch 'kobocat-django-app-part-2-refactor-mock-deployment-back…
noliveleger Sep 4, 2024
b062996
Merge branch 'kobocat-django-app-part-2-handle-missing-xform' into ko…
noliveleger Sep 4, 2024
ae91f5c
Fix DDA bug without service account
noliveleger Sep 5, 2024
b1d5000
Merge branch 'kobocat-django-app-part-2' into kobocat-django-app-part…
noliveleger Sep 5, 2024
a27edc3
Merge branch 'kobocat-django-app-part-2-remove-service-account' into …
noliveleger Sep 5, 2024
74b82e2
Merge branch 'kobocat-django-app-part-2-refactor-mock-deployment-back…
noliveleger Sep 5, 2024
0fb498b
Merge branch 'kobocat-django-app-part-2-handle-missing-xform' into ko…
noliveleger Sep 5, 2024
d7bd068
Fix tests
noliveleger Sep 5, 2024
89efcdf
Merge branch 'kobocat-django-app-part-2-refactor-mock-deployment-back…
noliveleger Sep 5, 2024
569efe7
Merge branch 'kobocat-django-app-part-2-handle-missing-xform' into ko…
noliveleger Sep 5, 2024
9e10341
Add reverse operation when deleting old tables
noliveleger Sep 12, 2024
fde9526
Do not use "safe_create_instance" anymore in DeploymentBackend classes
noliveleger Sep 12, 2024
5b4173f
Remove import
noliveleger Sep 12, 2024
65d81b6
Apply requested changes
noliveleger Oct 3, 2024
14d57a2
Remove useless imports
noliveleger Oct 3, 2024
41c6b73
Apply requested changes
noliveleger Sep 12, 2024
78ee7d8
Merge pull request #5056 from kobotoolbox/kobocat-django-app-part-2-r…
jnm Oct 3, 2024
32c6446
Merge branch 'kobocat-django-app-part-2-remove-service-account' into …
noliveleger Oct 3, 2024
99e3b1b
Merge branch 'kobocat-django-app-part-2-handle-missing-xform' into ko…
noliveleger Oct 3, 2024
7665656
Removed redundant call of get_queryset()
noliveleger Oct 3, 2024
5408a75
Make MCH test pass - PyXForm is case sensitive
noliveleger Oct 3, 2024
3ad8b85
Merge pull request #5075 from kobotoolbox/kobocat-django-app-part-2-h…
jnm Oct 3, 2024
d5b2f7b
Merge pull request #5078 from kobotoolbox/kobocat-django-app-part-2-b…
jnm Oct 3, 2024
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
4 changes: 2 additions & 2 deletions dependencies/pip/dev_requirements.in
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ pytest
pytest-cov
pytest-django
pytest-env
pytest-xdist


# Kobocat
# KoboCAT
httmock
simplejson
10 changes: 5 additions & 5 deletions dependencies/pip/dev_requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
# via -r dependencies/pip/requirements.in
-e git+https://github.com/kobotoolbox/formpack.git@451df4cd2a0d614be69a3b3309259c67369f7efb#egg=formpack
# via -r dependencies/pip/requirements.in
-e git+https://github.com/kobotoolbox/kobo-service-account.git@cb52c6221b68af9b13237d0a1157e3f1965a82b1#egg=kobo-service-account
# via -r dependencies/pip/requirements.in
-e git+https://github.com/dimagi/python-digest@5c94bb74516b977b60180ee832765c0695ff2b56#egg=python_digest
# via -r dependencies/pip/requirements.in
-e git+https://github.com/kobotoolbox/ssrf-protect@9b97d3f0fd8f737a38dd7a6b64efeffc03ab3cdd#egg=ssrf_protect
Expand Down Expand Up @@ -160,7 +158,6 @@ django==4.2.11
# django-timezone-field
# djangorestframework
# jsonfield
# kobo-service-account
# model-bakery
django-allauth==0.61.1
# via -r dependencies/pip/requirements.in
Expand Down Expand Up @@ -227,7 +224,6 @@ djangorestframework==3.15.1
# -r dependencies/pip/requirements.in
# djangorestframework-csv
# drf-extensions
# kobo-service-account
djangorestframework-csv==3.0.2
# via -r dependencies/pip/requirements.in
djangorestframework-jsonp==1.0.2
Expand All @@ -246,6 +242,8 @@ et-xmlfile==1.1.0
# via openpyxl
exceptiongroup==1.2.0
# via pytest
execnet==2.1.1
# via pytest-xdist
executing==2.0.1
# via stack-data
fabric==3.2.2
Expand Down Expand Up @@ -473,12 +471,15 @@ pytest==8.1.1
# pytest-cov
# pytest-django
# pytest-env
# pytest-xdist
pytest-cov==5.0.0
# via -r dependencies/pip/dev_requirements.in
pytest-django==4.8.0
# via -r dependencies/pip/dev_requirements.in
pytest-env==1.1.3
# via -r dependencies/pip/dev_requirements.in
pytest-xdist==3.6.1
# via -r dependencies/pip/dev_requirements.in
python-crontab==3.0.0
# via django-celery-beat
python-dateutil==2.9.0.post0
Expand All @@ -505,7 +506,6 @@ redis==5.0.3
# celery
# django-redis
# django-redis-sessions
# kobo-service-account
referencing==0.34.0
# via
# jsonschema
Expand Down
3 changes: 0 additions & 3 deletions dependencies/pip/requirements.in
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@
# formpack
-e git+https://github.com/kobotoolbox/formpack.git@451df4cd2a0d614be69a3b3309259c67369f7efb#egg=formpack

# service-account
-e git+https://github.com/kobotoolbox/kobo-service-account.git@cb52c6221b68af9b13237d0a1157e3f1965a82b1#egg=kobo-service-account

# More up-to-date version of django-digest than PyPI seems to have.
# Also, python-digest is an unlisted dependency thereof.
-e git+https://github.com/dimagi/python-digest@5c94bb74516b977b60180ee832765c0695ff2b56#egg=python_digest
Expand Down
5 changes: 0 additions & 5 deletions dependencies/pip/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
# via -r dependencies/pip/requirements.in
-e git+https://github.com/kobotoolbox/formpack.git@451df4cd2a0d614be69a3b3309259c67369f7efb#egg=formpack
# via -r dependencies/pip/requirements.in
-e git+https://github.com/kobotoolbox/kobo-service-account.git@cb52c6221b68af9b13237d0a1157e3f1965a82b1#egg=kobo-service-account
# via -r dependencies/pip/requirements.in
-e git+https://github.com/dimagi/python-digest@5c94bb74516b977b60180ee832765c0695ff2b56#egg=python_digest
# via -r dependencies/pip/requirements.in
-e git+https://github.com/kobotoolbox/ssrf-protect@9b97d3f0fd8f737a38dd7a6b64efeffc03ab3cdd#egg=ssrf_protect
Expand Down Expand Up @@ -138,7 +136,6 @@ django==4.2.11
# django-timezone-field
# djangorestframework
# jsonfield
# kobo-service-account
django-allauth==0.61.1
# via -r dependencies/pip/requirements.in
django-amazon-ses==4.0.1
Expand Down Expand Up @@ -204,7 +201,6 @@ djangorestframework==3.15.1
# -r dependencies/pip/requirements.in
# djangorestframework-csv
# drf-extensions
# kobo-service-account
djangorestframework-csv==3.0.2
# via -r dependencies/pip/requirements.in
djangorestframework-jsonp==1.0.2
Expand Down Expand Up @@ -420,7 +416,6 @@ redis==5.0.3
# celery
# django-redis
# django-redis-sessions
# kobo-service-account
referencing==0.34.0
# via
# jsonschema
Expand Down
30 changes: 16 additions & 14 deletions kobo/apps/hook/tests/hook_test_case.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
# coding: utf-8
import json
import uuid

import pytest
import responses
from django.conf import settings
from django.urls import reverse
from ipaddress import ip_address
from rest_framework import status

from kpi.constants import SUBMISSION_FORMAT_TYPE_JSON, SUBMISSION_FORMAT_TYPE_XML
Expand All @@ -23,16 +23,16 @@ def setUp(self):
self.asset = self.create_asset(
"some_asset",
content=json.dumps({'survey': [
{'type': 'text', 'name': 'q1'},
{'type': 'begin_group', 'name': 'group1'},
{'type': 'text', 'name': 'q2'},
{'type': 'text', 'name': 'q3'},
{'type': 'text', 'label': 'q1', 'name': 'q1'},
{'type': 'begin_group', 'label': 'group1', 'name': 'group1'},
{'type': 'text', 'label': 'q2', 'name': 'q2'},
{'type': 'text', 'label': 'q3', 'name': 'q3'},
{'type': 'end_group'},
{'type': 'begin_group', 'name': 'group2'},
{'type': 'begin_group', 'name': 'subgroup1'},
{'type': 'text', 'name': 'q4'},
{'type': 'text', 'name': 'q5'},
{'type': 'text', 'name': 'q6'},
{'type': 'begin_group', 'label': 'group2', 'name': 'group2'},
{'type': 'begin_group', 'label': 'subgroup1', 'name': 'subgroup1'},
{'type': 'text', 'label': 'q4', 'name': 'q4'},
{'type': 'text', 'label': 'q5', 'name': 'q5'},
{'type': 'text', 'label': 'q6', 'name': 'q6'},
{'type': 'end_group'},
{'type': 'end_group'},
]}),
Expand Down Expand Up @@ -76,8 +76,9 @@ def _create_hook(self, return_response_only=False, **kwargs):
if return_response_only:
return response
else:
self.assertEqual(response.status_code, status.HTTP_201_CREATED,
msg=response.data)
self.assertEqual(
response.status_code, status.HTTP_201_CREATED, msg=response.data
)
hook = self.asset.hooks.last()
self.assertTrue(hook.active)
return hook
Expand Down Expand Up @@ -151,14 +152,15 @@ def _send_and_wait_for_retry(self):

def __prepare_submission(self):
v_uid = self.asset.latest_deployed_version.uid
submission = {
self.submission = {
'__version__': v_uid,
'q1': '¿Qué tal?',
'_uuid': str(uuid.uuid4()),
'group1/q2': '¿Cómo está en el grupo uno la primera vez?',
'group1/q3': '¿Cómo está en el grupo uno la segunda vez?',
'group2/subgroup1/q4': '¿Cómo está en el subgrupo uno la primera vez?',
'group2/subgroup1/q5': '¿Cómo está en el subgrupo uno la segunda vez?',
'group2/subgroup1/q6': '¿Cómo está en el subgrupo uno la tercera vez?',
'group2/subgroup11/q1': '¿Cómo está en el subgrupo once?',
}
self.asset.deployment.mock_submissions([submission])
self.asset.deployment.mock_submissions([self.submission])
49 changes: 29 additions & 20 deletions kobo/apps/hook/tests/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ def test_json_parser(self):

ServiceDefinition = hook.get_service_definition()
submissions = hook.asset.deployment.get_submissions(hook.asset.owner)
uuid = submissions[0]['_id']
service_definition = ServiceDefinition(hook, uuid)
submission_id = submissions[0]['_id']
service_definition = ServiceDefinition(hook, submission_id)
expected_data = {
'_id': 1,
'_id': submission_id,
'group1/q3': u'¿Cómo está en el grupo uno la segunda vez?',
'group2/subgroup1/q4': u'¿Cómo está en el subgrupo uno la primera vez?',
'group2/subgroup1/q5': u'¿Cómo está en el subgrupo uno la segunda vez?',
Expand All @@ -29,26 +29,25 @@ def test_json_parser(self):

def test_xml_parser(self):
self.asset = self.create_asset(
"some_asset_with_xml_submissions",
'some_asset_with_xml_submissions',
content=json.dumps(self.asset.content),
format="json")
format='json',
)
self.asset.deploy(backend='mock', active=True)
self.asset.save()

hook = self._create_hook(subset_fields=['_id', 'subgroup1', 'q3'],
format_type=SUBMISSION_FORMAT_TYPE_XML)
hook = self._create_hook(
subset_fields=['meta', 'subgroup1', 'q3'],
format_type=SUBMISSION_FORMAT_TYPE_XML,
)

ServiceDefinition = hook.get_service_definition()
submissions = hook.asset.deployment.get_submissions(
self.asset.owner, format_type=SUBMISSION_FORMAT_TYPE_XML)
xml_doc = etree.fromstring(submissions[0].encode())
tree = etree.ElementTree(xml_doc)
uuid = tree.find('_id').text

service_definition = ServiceDefinition(hook, uuid)
submissions = hook.asset.deployment.get_submissions(self.asset.owner)
submission_id = submissions[0]['_id']
submission_uuid = submissions[0]['_uuid']
service_definition = ServiceDefinition(hook, submission_id)
expected_etree = etree.fromstring(
f'<{self.asset.uid}>'
f' <_id>{uuid}</_id>'
f'<{self.asset.uid} id="{self.asset.uid}">'
f' <group1>'
f' <q3>¿Cómo está en el grupo uno la segunda vez?</q3>'
f' </group1>'
Expand All @@ -59,13 +58,23 @@ def test_xml_parser(self):
f' <q6>¿Cómo está en el subgrupo uno la tercera vez?</q6>'
f' </subgroup1>'
f' </group2>'
f' <meta>'
f' <instanceID>uuid:{submission_uuid}</instanceID>'
f' </meta>'
f'</{self.asset.uid}>'
)
expected_xml = etree.tostring(expected_etree, pretty_print=True,
xml_declaration=True, encoding='utf-8')

expected_xml = etree.tostring(
expected_etree,
pretty_print=True,
xml_declaration=True,
encoding='utf-8',
)

def remove_whitespace(str_):
return re.sub(r'>\s+<', '><', to_str(str_))

self.assertEqual(remove_whitespace(service_definition._get_data()),
remove_whitespace(expected_xml.decode()))
self.assertEqual(
remove_whitespace(service_definition._get_data()),
remove_whitespace(expected_xml.decode()),
)
1 change: 1 addition & 0 deletions kobo/apps/kobo_auth/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ class KoboAuthAppConfig(AppConfig):
verbose_name = 'Authentication and authorization'

def ready(self):
from . import signals
super().ready()
58 changes: 58 additions & 0 deletions kobo/apps/kobo_auth/signals.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
from django.conf import settings
from django.db.models.signals import post_save
from django.dispatch import receiver
from rest_framework.authtoken.models import Token

from kobo.apps.kobo_auth.shortcuts import User
from kobo.apps.openrosa.apps.main.models.user_profile import UserProfile
from kpi.deployment_backends.kc_access.utils import (
grant_kc_model_level_perms,
kc_transaction_atomic,
)
from kpi.utils.permissions import (
grant_default_model_level_perms,
is_user_anonymous,
)


@receiver(post_save, sender=User)
def create_auth_token(sender, instance=None, created=False, **kwargs):
if is_user_anonymous(instance):
return

if created:
Token.objects.get_or_create(user_id=instance.pk)


@receiver(post_save, sender=User)
def default_permissions_post_save(sender, instance, created, raw, **kwargs):
"""
Users must have both model-level and object-level permissions to satisfy
DRF, so assign the newly-created user all available collection and asset
permissions at the model level
"""
if raw:
# `raw` means we can't touch (so make sure your fixtures include
# all necessary permissions!)
return
if not created:
# We should only grant default permissions when the user is first
# created
return
grant_default_model_level_perms(instance)


@receiver(post_save, sender=User)
def save_kobocat_user(sender, instance, created, raw, **kwargs):
"""
Sync auth_user table between KPI and KC, and, if the user is newly created,
grant all KoboCAT model-level permissions for the content types listed in
`settings.KOBOCAT_DEFAULT_PERMISSION_CONTENT_TYPES`
"""

if not settings.TESTING:
with kc_transaction_atomic():
instance.sync_to_openrosa_db()
if created:
grant_kc_model_level_perms(instance)
UserProfile.objects.get_or_create(user=instance)
28 changes: 7 additions & 21 deletions kobo/apps/openrosa/apps/api/permissions.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# coding: utf-8
from django.http import Http404
from kobo_service_account.models import ServiceAccountUser
from rest_framework.permissions import (
BasePermission,
DjangoObjectPermissions,
Expand Down Expand Up @@ -90,7 +89,6 @@ def has_permission(self, request, view):
request.method not in SAFE_METHODS
and view.action
and view.action in ['create', 'update', 'partial_update', 'destroy']
and not isinstance(request.user, ServiceAccountUser)
):
raise LegacyAPIException

Expand Down Expand Up @@ -174,23 +172,14 @@ def has_object_permission(self, request, view, obj):
except KeyError:
pass
else:
# Only service account is allowed to bulk delete submissions.
# Even KoBoCAT superusers are not allowed
if (
view.action == 'bulk_delete'
and not isinstance(user, ServiceAccountUser)
):
# return False
# Deleting submissions is not allowed with KoboCAT API
if view.action == 'bulk_delete':
raise LegacyAPIException

return user.has_perms(required_perms, obj)

# Only service account is allowed to delete submissions.
# Even KoBoCAT superusers are not allowed
if (
view.action == 'destroy'
and not isinstance(user, ServiceAccountUser)
):
# Deleting submissions is not allowed with KoboCAT API
if view.action == 'destroy':
raise LegacyAPIException

return super().has_object_permission(request, view, obj)
Expand All @@ -205,7 +194,7 @@ def has_object_permission(self, request, view, obj):
user = request.user
required_perms = [f'logger.{CAN_CHANGE_XFORM}']

# Grant access if user is owner or super user
# Grant access if user is owner or superuser
if user.is_superuser or user == obj.user:
return True

Expand Down Expand Up @@ -370,11 +359,8 @@ class UserDeletePermission(BasePermission):
perms_map = {}

def has_permission(self, request, view):
if not isinstance(request.user, ServiceAccountUser):
# Do not reveal user's existence
raise Http404

return True
# Do not reveal user's existence
raise Http404

def has_object_permission(self, request, view, obj):
# Always return True because it must pass `has_permission()` first
Expand Down
Loading
Loading