-
-
Notifications
You must be signed in to change notification settings - Fork 187
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
feat(accessLogsExport): create new endpoints TASK-1147 #5304
Changes from 23 commits
09373fb
4079ea0
eb67d2b
0f9ac39
2836ea3
b69a006
04dc2cd
4c46fb0
de7f7f5
ff45ba8
7837192
6c60768
43ce879
a0a2d5e
bd5c994
91fefc7
0640937
cd25b98
6c3e3af
dd5650d
cd35d96
039063b
33e1c0a
6ca21ec
13d31b2
fee5f37
6eac898
189a860
1d41380
c9cbb7e
8d8f763
7f88c37
0383afb
350c22e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -6,17 +6,14 @@ | |||||
from rest_framework.reverse import reverse | ||||||
|
||||||
from kobo.apps.audit_log.audit_actions import AuditAction | ||||||
from kobo.apps.audit_log.models import ( | ||||||
AccessLog, | ||||||
AuditLog, | ||||||
AuditType, | ||||||
) | ||||||
from kobo.apps.audit_log.models import AccessLog, AuditLog, AuditType | ||||||
from kobo.apps.audit_log.tests.test_signals import skip_login_access_log | ||||||
from kobo.apps.kobo_auth.shortcuts import User | ||||||
from kpi.constants import ( | ||||||
ACCESS_LOG_SUBMISSION_AUTH_TYPE, | ||||||
ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, | ||||||
) | ||||||
from kpi.models.import_export_task import AccessLogExportTask | ||||||
from kpi.tests.base_test_case import BaseTestCase | ||||||
from kpi.urls.router_api_v2 import URL_NAMESPACE as ROUTER_URL_NAMESPACE | ||||||
|
||||||
|
@@ -430,3 +427,148 @@ def test_can_search_access_logs_by_date_including_submission_groups(self): | |||||
group['metadata']['auth_type'], | ||||||
ACCESS_LOG_SUBMISSION_GROUP_AUTH_TYPE, | ||||||
) | ||||||
|
||||||
|
||||||
class ApiAccessLogsExportTestCase(BaseAuditLogTestCase): | ||||||
|
||||||
def get_endpoint_basename(self): | ||||||
return 'access-logs-export-list' | ||||||
|
||||||
def test_export_as_anonymous_returns_unauthorized(self): | ||||||
self.client.logout() | ||||||
response = self.client.post(self.url) | ||||||
assert response.status_code == status.HTTP_401_UNAUTHORIZED | ||||||
|
||||||
def test_export_for_user_commences(self): | ||||||
self.force_login_user(User.objects.get(username='anotheruser')) | ||||||
response = self.client.post(self.url) | ||||||
self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED) | ||||||
|
||||||
def test_export_for_superuser_commences(self): | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above |
||||||
self.force_login_user(User.objects.get(username='admin')) | ||||||
response = self.client.post(self.url) | ||||||
self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED) | ||||||
|
||||||
def test_create_export_task_on_post(self): | ||||||
test_user = User.objects.get(username='anotheruser') | ||||||
self.force_login_user(test_user) | ||||||
|
||||||
response = self.client.post(self.url) | ||||||
self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED) | ||||||
|
||||||
task = ( | ||||||
AccessLogExportTask.objects.filter(user=test_user) | ||||||
.order_by('-date_created') | ||||||
.first() | ||||||
) | ||||||
self.assertIsNotNone(task) | ||||||
self.assertEqual(task.status, 'complete') | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a tiny bit risky since we don't know exactly when the task will finish. Maybe test status in ['created', 'processing', 'complete']? |
||||||
|
||||||
def test_get_status_of_most_recent_task(self): | ||||||
self.force_login_user(User.objects.get(username='anotheruser')) | ||||||
response = self.client.post(self.url) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to make this test more unit-y, I think we should manually create a task with AccessLogExportTask.objects.create(...). That way, we're only testing the list functionality and not the create functionality as well, since that's already tested. |
||||||
self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED) | ||||||
|
||||||
response_status = self.client.get(self.url) | ||||||
self.assertEqual(response_status.status_code, status.HTTP_200_OK) | ||||||
self.assertIn('uid', response_status.json()) | ||||||
self.assertIn('status', response_status.json()) | ||||||
self.assertEqual(response_status.json()['status'], 'complete') | ||||||
|
||||||
def test_multiple_export_tasks_not_allowed(self): | ||||||
test_user = User.objects.get(username='anotheruser') | ||||||
self.force_login_user(test_user) | ||||||
|
||||||
response_first = self.client.post(self.url) | ||||||
self.assertEqual(response_first.status_code, status.HTTP_202_ACCEPTED) | ||||||
|
||||||
task = ( | ||||||
AccessLogExportTask.objects.filter(user=test_user) | ||||||
.order_by('-date_created') | ||||||
.first() | ||||||
) | ||||||
task.status = 'processing' | ||||||
task.save() | ||||||
|
||||||
response_second = self.client.post(self.url) | ||||||
self.assertEqual(response_second.status_code, status.HTTP_400_BAD_REQUEST) | ||||||
self.assertIn( | ||||||
'You already have a running export task for your own logs', | ||||||
response_second.json()['error'], | ||||||
) | ||||||
|
||||||
|
||||||
class AllApiAccessLogsExportTestCase(BaseAuditLogTestCase): | ||||||
|
||||||
def get_endpoint_basename(self): | ||||||
return 'all-access-logs-export-list' | ||||||
|
||||||
def test_export_as_anonymous_returns_unauthorized(self): | ||||||
self.client.logout() | ||||||
response = self.client.post(self.url) | ||||||
assert response.status_code == status.HTTP_401_UNAUTHORIZED | ||||||
|
||||||
def test_regular_user_cannot_export_access_logs(self): | ||||||
self.force_login_user(User.objects.get(username='anotheruser')) | ||||||
response = self.client.post(self.url) | ||||||
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) | ||||||
|
||||||
def test_export_access_logs_for_superuser_commences(self): | ||||||
self.force_login_user(User.objects.get(username='admin')) | ||||||
response = self.client.post(self.url) | ||||||
self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED) | ||||||
|
||||||
def test__superuser_create_export_task_on_post(self): | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
test_superuser = User.objects.get(username='admin') | ||||||
self.force_login_user(test_superuser) | ||||||
|
||||||
response = self.client.post(self.url) | ||||||
self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED) | ||||||
|
||||||
task = ( | ||||||
AccessLogExportTask.objects.filter(user=test_superuser) | ||||||
.order_by('-date_created') | ||||||
.first() | ||||||
) | ||||||
self.assertIsNotNone(task) | ||||||
self.assertEqual(task.status, 'complete') | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as previous test, the test should pass if the status is 'processing' or 'created' |
||||||
|
||||||
def test_superuser_get_status_of_most_recent_task(self): | ||||||
self.force_login_user(User.objects.get(username='admin')) | ||||||
response = self.client.post(self.url) | ||||||
self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED) | ||||||
|
||||||
response_status = self.client.get(self.url) | ||||||
self.assertEqual(response_status.status_code, status.HTTP_200_OK) | ||||||
self.assertIn('uid', response_status.json()) | ||||||
self.assertIn('status', response_status.json()) | ||||||
self.assertEqual(response_status.json()['status'], 'complete') | ||||||
|
||||||
def test_permission_denied_for_non_superusers_on_get_status(self): | ||||||
non_superuser = User.objects.get(username='anotheruser') | ||||||
self.force_login_user(non_superuser) | ||||||
|
||||||
response = self.client.get(self.url) | ||||||
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) | ||||||
|
||||||
def test_multiple_export_tasks_not_allowed(self): | ||||||
test_superuser = User.objects.get(username='admin') | ||||||
self.force_login_user(test_superuser) | ||||||
|
||||||
response_first = self.client.post(self.url) | ||||||
self.assertEqual(response_first.status_code, status.HTTP_202_ACCEPTED) | ||||||
|
||||||
task = ( | ||||||
AccessLogExportTask.objects.filter(user=test_superuser) | ||||||
.order_by('-date_created') | ||||||
.first() | ||||||
) | ||||||
task.status = 'processing' | ||||||
task.save() | ||||||
|
||||||
response_second = self.client.post(self.url) | ||||||
self.assertEqual(response_second.status_code, status.HTTP_400_BAD_REQUEST) | ||||||
self.assertIn( | ||||||
'You already have a running export task for this type.', | ||||||
response_second.json()['error'], | ||||||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,21 @@ | ||
from rest_framework.routers import DefaultRouter | ||
|
||
from .views import AccessLogViewSet, AllAccessLogViewSet, AuditLogViewSet | ||
from .views import ( | ||
AccessLogsExportViewSet, | ||
AccessLogViewSet, | ||
AllAccessLogViewSet, | ||
AuditLogViewSet, | ||
) | ||
|
||
router = DefaultRouter() | ||
router.register(r'audit-logs', AuditLogViewSet, basename='audit-log') | ||
router.register(r'access-logs', AllAccessLogViewSet, basename='all-access-logs') | ||
router.register(r'access-logs/me', AccessLogViewSet, basename='access-log') | ||
router.register( | ||
r'access-logs/export', AccessLogsExportViewSet, basename='all-access-logs-export' | ||
) | ||
router.register( | ||
r'access-logs/me/export', AccessLogsExportViewSet, basename='access-logs-export' | ||
) | ||
|
||
urlpatterns = [] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,11 @@ | ||
from rest_framework import mixins, viewsets | ||
from rest_framework import exceptions, mixins, status, viewsets | ||
from rest_framework.renderers import BrowsableAPIRenderer, JSONRenderer | ||
from rest_framework.response import Response | ||
|
||
from kpi.filters import SearchFilter | ||
from kpi.models.import_export_task import AccessLogExportTask | ||
from kpi.permissions import IsAuthenticated | ||
from kpi.tasks import export_task_in_background | ||
from .filters import AccessLogPermissionsFilter | ||
from .models import AccessLog, AuditLog | ||
from .permissions import SuperUserPermission | ||
|
@@ -321,3 +324,84 @@ class AccessLogViewSet(AuditLogViewSet): | |
permission_classes = (IsAuthenticated,) | ||
filter_backends = (AccessLogPermissionsFilter,) | ||
serializer_class = AccessLogSerializer | ||
|
||
|
||
class AccessLogsExportViewSet(viewsets.ViewSet): | ||
permission_classes = (IsAuthenticated,) | ||
lookup_field = 'uid' | ||
|
||
def create(self, request, uid=None, type=None, *args, **kwargs): | ||
if not request.user.is_superuser and 'access-logs/export' in request.path: | ||
raise exceptions.PermissionDenied( | ||
'Only superusers can export all access logs.' | ||
) | ||
|
||
get_all_logs = 'access-logs/export' in request.path | ||
|
||
# Superuser handling: one job for all logs and another for their own logs | ||
if request.user.is_superuser: | ||
# Check if the superuser has a task running for all or just their own logs | ||
if AccessLogExportTask.objects.filter( | ||
user=request.user, | ||
status=AccessLogExportTask.PROCESSING, | ||
get_all_logs=get_all_logs, | ||
).exists(): | ||
return Response( | ||
{'error': 'You already have a running export task for this type.'}, | ||
status=status.HTTP_400_BAD_REQUEST, | ||
) | ||
else: | ||
# Non-superusers can only run one task for their own logs at a time | ||
if AccessLogExportTask.objects.filter( | ||
user=request.user, | ||
status=AccessLogExportTask.PROCESSING, | ||
get_all_logs=False, | ||
).exists(): | ||
return Response( | ||
{ | ||
'error': ( | ||
'You already have a running export task for your own logs.' | ||
) | ||
}, | ||
status=status.HTTP_400_BAD_REQUEST, | ||
) | ||
|
||
export_task = AccessLogExportTask.objects.create( | ||
user=request.user, | ||
get_all_logs=get_all_logs, | ||
data={ | ||
'type': 'access_logs_export', | ||
}, | ||
) | ||
|
||
export_task_in_background.delay( | ||
export_task_uid=export_task.uid, | ||
username=export_task.user.username, | ||
export_task_name='kpi.AccessLogExportTask', | ||
) | ||
return Response( | ||
{f'status: {export_task.status}'}, | ||
status=status.HTTP_202_ACCEPTED, | ||
) | ||
|
||
def list(self, request, *args, **kwargs): | ||
if not request.user.is_superuser and 'access-logs/export' in request.path: | ||
raise exceptions.PermissionDenied( | ||
'Only superusers can export all access logs.' | ||
) | ||
|
||
task = ( | ||
AccessLogExportTask.objects.filter(user=request.user) | ||
.order_by('-date_created') | ||
.first() | ||
) | ||
|
||
if task: | ||
return Response( | ||
{'uid': task.uid, 'status': task.status}, status=status.HTTP_200_OK | ||
) | ||
else: | ||
return Response( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No 404 here. An empty list is not an error response |
||
{'error': 'No export task found for this user.'}, | ||
status=status.HTTP_404_NOT_FOUND, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
non-blocking nit, since this doesn't technically test that the task started, only that the request suceeded