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

Consistent 400 Response for Invalid Input in Kolibri Public Content APIs #12818

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
9 changes: 9 additions & 0 deletions kolibri/core/content/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from collections import OrderedDict
from functools import reduce
from random import sample
from uuid import UUID

from django.core.cache import cache
from django.core.exceptions import ValidationError
Expand Down Expand Up @@ -862,6 +863,14 @@ class ContentNodeViewset(InternalContentNodeMixin, RemoteMixin, ReadOnlyValuesVi
def retrieve(self, request, pk=None):
if pk is None:
manzil-infinity180 marked this conversation as resolved.
Show resolved Hide resolved
raise Http404

try:
ozer550 marked this conversation as resolved.
Show resolved Hide resolved
UUID(pk)
except ValueError:
return Response(
{"error": "Invalid UUID format."}, status=status.HTTP_400_BAD_REQUEST
)

if self._should_proxy_request(request):
if self.get_queryset().filter(admin_imported=True, pk=pk).exists():
# Used in the update method for remote request retrieval
Expand Down
11 changes: 10 additions & 1 deletion kolibri/core/content/public_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
from django.db import connection
from django.db.models import Q
from django.http import HttpResponseBadRequest
from django.shortcuts import get_object_or_404
from rest_framework import status
from rest_framework.response import Response
from rest_framework.serializers import Serializer
from rest_framework.viewsets import GenericViewSet
Expand Down Expand Up @@ -50,6 +52,13 @@ def retrieve(self, request, pk=None):
:param pk: id parent node
:return: an object with keys for each content metadata table and a schema_version key
"""
try:
ozer550 marked this conversation as resolved.
Show resolved Hide resolved
UUID(pk)
except ValueError:
return Response(
{"error": "Invalid UUID format."},
status=status.HTTP_400_BAD_REQUEST,
)

content_schema = request.query_params.get(
"schema_version", self.default_content_schema
Expand All @@ -72,7 +81,7 @@ def retrieve(self, request, pk=None):

# Get the model for the target node here - we do this so that we trigger a 404 immediately if the node
# does not exist.
node = self.get_object()
node = get_object_or_404(models.ContentNode.objects.all(), pk=pk)

nodes = node.get_ancestors(include_self=True)

Expand Down
13 changes: 11 additions & 2 deletions kolibri/core/content/test/test_content_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -506,15 +506,24 @@ def test_contentnode_tree_filtered_queryset_node(self):
)
self.assertEqual(response.status_code, 404)

def test_contentnode_tree_bad_pk(self):
def test_contentnode_tree_none_pk(self):
ozer550 marked this conversation as resolved.
Show resolved Hide resolved
response = self.client.get(
reverse(
"kolibri:core:contentnode_tree-detail",
kwargs={"pk": "this is not a UUID"},
kwargs={"pk": None},
)
)
self.assertEqual(response.status_code, 404)
ozer550 marked this conversation as resolved.
Show resolved Hide resolved

def test_contentnode_tree_bad_pk(self):
response = self.client.get(
reverse(
"kolibri:core:contentnode_tree-detail",
kwargs={"pk": "this is not UUID"},
)
)
self.assertEqual(response.status_code, 400)

@unittest.skipIf(
getattr(settings, "DATABASES")["default"]["ENGINE"]
== "django.db.backends.postgresql",
Expand Down
19 changes: 19 additions & 0 deletions kolibri/core/content/test/test_public_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,25 @@ def test_schema_version_too_high(self):
)
self.assertEqual(response.status_code, 400)

def test_import_metadata_none_pk(self):
response = self.client.get(
reverse(
"kolibri:core:importmetadata-detail",
kwargs={"pk": None},
)
)
self.assertEqual(response.status_code, 404)
ozer550 marked this conversation as resolved.
Show resolved Hide resolved

def test_import_metadata_bad_pk(self):
response = self.client.get(
reverse(
"kolibri:core:importmetadata-detail",
kwargs={"pk": "this is not a UUID"},
)
)
self.assertEqual(response.status_code, 400)
self.assertEqual(response.data["error"], "Invalid UUID format.")

def test_schema_version_just_right(self):
response = self.client.get(
reverse("kolibri:core:importmetadata-detail", kwargs={"pk": self.node.id})
Expand Down
Loading