Skip to content

Commit

Permalink
Add bleach (#41) (#3204)
Browse files Browse the repository at this point in the history
* use shims for API view inheritation

* Add mixin for input sanitation

* fix clean operation to fix all string values

* Also clean up dicts
this is to future-proof this function

* Update docstirng

* proof custom methods against XSS through authenticated users
  • Loading branch information
matmair committed Jun 16, 2022
1 parent f8a2760 commit e83995b
Show file tree
Hide file tree
Showing 12 changed files with 310 additions and 178 deletions.
6 changes: 4 additions & 2 deletions InvenTree/InvenTree/api.py
Expand Up @@ -6,10 +6,12 @@
from django.utils.translation import gettext_lazy as _

from django_filters.rest_framework import DjangoFilterBackend
from rest_framework import filters, generics, permissions
from rest_framework import filters, permissions
from rest_framework.response import Response
from rest_framework.serializers import ValidationError

from InvenTree.mixins import ListCreateAPI

from .status import is_worker_running
from .version import (inventreeApiVersion, inventreeInstanceName,
inventreeVersion)
Expand Down Expand Up @@ -134,7 +136,7 @@ def delete(self, request, *args, **kwargs):
)


class ListCreateDestroyAPIView(BulkDeleteMixin, generics.ListCreateAPIView):
class ListCreateDestroyAPIView(BulkDeleteMixin, ListCreateAPI):
"""Custom API endpoint which provides BulkDelete functionality in addition to List and Create"""
...

Expand Down
90 changes: 90 additions & 0 deletions InvenTree/InvenTree/mixins.py
@@ -0,0 +1,90 @@
"""Mixins for (API) views in the whole project."""

from bleach import clean
from rest_framework import generics, status
from rest_framework.response import Response


class CleanMixin():
"""Model mixin class which cleans inputs."""

# Define a map of fields avaialble for import
SAFE_FIELDS = {}

def create(self, request, *args, **kwargs):
"""Override to clean data before processing it."""
serializer = self.get_serializer(data=self.clean_data(request.data))
serializer.is_valid(raise_exception=True)
self.perform_create(serializer)
headers = self.get_success_headers(serializer.data)
return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers)

def update(self, request, *args, **kwargs):
"""Override to clean data before processing it."""
partial = kwargs.pop('partial', False)
instance = self.get_object()
serializer = self.get_serializer(instance, data=self.clean_data(request.data), partial=partial)
serializer.is_valid(raise_exception=True)
self.perform_update(serializer)

if getattr(instance, '_prefetched_objects_cache', None):
# If 'prefetch_related' has been applied to a queryset, we need to
# forcibly invalidate the prefetch cache on the instance.
instance._prefetched_objects_cache = {}

return Response(serializer.data)

def clean_data(self, data: dict) -> dict:
"""Clean / snatize data.
This uses mozillas bleach under the hood to disable certain html tags by
encoding them - this leads to script tags etc. to not work.
The results can be longer then the input; might make some character combinations
`ugly`. Prevents XSS on the server-level.
Args:
data (dict): Data that should be sanatized.
Returns:
dict: Profided data sanatized; still in the same order.
"""
clean_data = {}
for k, v in data.items():
if isinstance(v, str):
ret = clean(v)
elif isinstance(v, dict):
ret = self.clean_data(v)
else:
ret = v
clean_data[k] = ret
return clean_data


class ListAPI(generics.ListAPIView):
"""View for list API."""


class ListCreateAPI(CleanMixin, generics.ListCreateAPIView):
"""View for list and create API."""


class CreateAPI(CleanMixin, generics.CreateAPIView):
"""View for create API."""


class RetrieveAPI(generics.RetrieveAPIView):
"""View for retreive API."""
pass


class RetrieveUpdateAPI(CleanMixin, generics.RetrieveUpdateAPIView):
"""View for retrieve and update API."""
pass


class RetrieveUpdateDestroyAPI(CleanMixin, generics.RetrieveUpdateDestroyAPIView):
"""View for retrieve, update and destroy API."""


class UpdateAPI(CleanMixin, generics.UpdateAPIView):
"""View for update API."""
29 changes: 15 additions & 14 deletions InvenTree/build/api.py
Expand Up @@ -3,7 +3,7 @@
from django.urls import include, re_path
from django.utils.translation import gettext_lazy as _

from rest_framework import filters, generics
from rest_framework import filters
from rest_framework.exceptions import ValidationError

from django_filters.rest_framework import DjangoFilterBackend
Expand All @@ -13,6 +13,7 @@
from InvenTree.helpers import str2bool, isNull, DownloadFile
from InvenTree.filters import InvenTreeOrderingFilter
from InvenTree.status_codes import BuildStatus
from InvenTree.mixins import CreateAPI, RetrieveUpdateDestroyAPI, ListCreateAPI

import build.admin
import build.serializers
Expand Down Expand Up @@ -65,7 +66,7 @@ def filter_assigned_to_me(self, queryset, name, value):
return queryset


class BuildList(APIDownloadMixin, generics.ListCreateAPIView):
class BuildList(APIDownloadMixin, ListCreateAPI):
"""API endpoint for accessing a list of Build objects.
- GET: Return list of objects (with filters)
Expand Down Expand Up @@ -200,7 +201,7 @@ def get_serializer(self, *args, **kwargs):
return self.serializer_class(*args, **kwargs)


class BuildDetail(generics.RetrieveUpdateDestroyAPIView):
class BuildDetail(RetrieveUpdateDestroyAPI):
"""API endpoint for detail view of a Build object."""

queryset = Build.objects.all()
Expand All @@ -219,7 +220,7 @@ def destroy(self, request, *args, **kwargs):
return super().destroy(request, *args, **kwargs)


class BuildUnallocate(generics.CreateAPIView):
class BuildUnallocate(CreateAPI):
"""API endpoint for unallocating stock items from a build order.
- The BuildOrder object is specified by the URL
Expand Down Expand Up @@ -263,23 +264,23 @@ def get_serializer_context(self):
return ctx


class BuildOutputCreate(BuildOrderContextMixin, generics.CreateAPIView):
class BuildOutputCreate(BuildOrderContextMixin, CreateAPI):
"""API endpoint for creating new build output(s)."""

queryset = Build.objects.none()

serializer_class = build.serializers.BuildOutputCreateSerializer


class BuildOutputComplete(BuildOrderContextMixin, generics.CreateAPIView):
class BuildOutputComplete(BuildOrderContextMixin, CreateAPI):
"""API endpoint for completing build outputs."""

queryset = Build.objects.none()

serializer_class = build.serializers.BuildOutputCompleteSerializer


class BuildOutputDelete(BuildOrderContextMixin, generics.CreateAPIView):
class BuildOutputDelete(BuildOrderContextMixin, CreateAPI):
"""API endpoint for deleting multiple build outputs."""

def get_serializer_context(self):
Expand All @@ -295,15 +296,15 @@ def get_serializer_context(self):
serializer_class = build.serializers.BuildOutputDeleteSerializer


class BuildFinish(BuildOrderContextMixin, generics.CreateAPIView):
class BuildFinish(BuildOrderContextMixin, CreateAPI):
"""API endpoint for marking a build as finished (completed)."""

queryset = Build.objects.none()

serializer_class = build.serializers.BuildCompleteSerializer


class BuildAutoAllocate(BuildOrderContextMixin, generics.CreateAPIView):
class BuildAutoAllocate(BuildOrderContextMixin, CreateAPI):
"""API endpoint for 'automatically' allocating stock against a build order.
- Only looks at 'untracked' parts
Expand All @@ -317,7 +318,7 @@ class BuildAutoAllocate(BuildOrderContextMixin, generics.CreateAPIView):
serializer_class = build.serializers.BuildAutoAllocationSerializer


class BuildAllocate(BuildOrderContextMixin, generics.CreateAPIView):
class BuildAllocate(BuildOrderContextMixin, CreateAPI):
"""API endpoint to allocate stock items to a build order.
- The BuildOrder object is specified by the URL
Expand All @@ -333,21 +334,21 @@ class BuildAllocate(BuildOrderContextMixin, generics.CreateAPIView):
serializer_class = build.serializers.BuildAllocationSerializer


class BuildCancel(BuildOrderContextMixin, generics.CreateAPIView):
class BuildCancel(BuildOrderContextMixin, CreateAPI):
"""API endpoint for cancelling a BuildOrder."""

queryset = Build.objects.all()
serializer_class = build.serializers.BuildCancelSerializer


class BuildItemDetail(generics.RetrieveUpdateDestroyAPIView):
class BuildItemDetail(RetrieveUpdateDestroyAPI):
"""API endpoint for detail view of a BuildItem object."""

queryset = BuildItem.objects.all()
serializer_class = build.serializers.BuildItemSerializer


class BuildItemList(generics.ListCreateAPIView):
class BuildItemList(ListCreateAPI):
"""API endpoint for accessing a list of BuildItem objects.
- GET: Return list of objects
Expand Down Expand Up @@ -442,7 +443,7 @@ class BuildAttachmentList(AttachmentMixin, ListCreateDestroyAPIView):
]


class BuildAttachmentDetail(AttachmentMixin, generics.RetrieveUpdateDestroyAPIView):
class BuildAttachmentDetail(AttachmentMixin, RetrieveUpdateDestroyAPI):
"""Detail endpoint for a BuildOrderAttachment object."""

queryset = BuildOrderAttachment.objects.all()
Expand Down
20 changes: 11 additions & 9 deletions InvenTree/common/api.py
Expand Up @@ -9,7 +9,7 @@

from django_filters.rest_framework import DjangoFilterBackend
from django_q.tasks import async_task
from rest_framework import filters, generics, permissions, serializers
from rest_framework import filters, permissions, serializers
from rest_framework.exceptions import NotAcceptable, NotFound
from rest_framework.response import Response
from rest_framework.views import APIView
Expand All @@ -18,6 +18,8 @@
import common.serializers
from InvenTree.api import BulkDeleteMixin
from InvenTree.helpers import inheritors
from InvenTree.mixins import (CreateAPI, ListAPI, RetrieveAPI,
RetrieveUpdateAPI, RetrieveUpdateDestroyAPI)
from plugin.models import NotificationUserSetting
from plugin.serializers import NotificationUserSettingSerializer

Expand Down Expand Up @@ -97,7 +99,7 @@ def _get_webhook(self, endpoint, request, *args, **kwargs):
raise NotFound()


class SettingsList(generics.ListAPIView):
class SettingsList(ListAPI):
"""Generic ListView for settings.
This is inheritted by all list views for settings.
Expand Down Expand Up @@ -145,7 +147,7 @@ def has_permission(self, request, view):
return False


class GlobalSettingsDetail(generics.RetrieveUpdateAPIView):
class GlobalSettingsDetail(RetrieveUpdateAPI):
"""Detail view for an individual "global setting" object.
- User must have 'staff' status to view / edit
Expand Down Expand Up @@ -203,7 +205,7 @@ def has_object_permission(self, request, view, obj):
return user == obj.user


class UserSettingsDetail(generics.RetrieveUpdateAPIView):
class UserSettingsDetail(RetrieveUpdateAPI):
"""Detail view for an individual "user setting" object.
- User can only view / edit settings their own settings objects
Expand Down Expand Up @@ -245,7 +247,7 @@ def filter_queryset(self, queryset):
return queryset


class NotificationUserSettingsDetail(generics.RetrieveUpdateAPIView):
class NotificationUserSettingsDetail(RetrieveUpdateAPI):
"""Detail view for an individual "notification user setting" object.
- User can only view / edit settings their own settings objects
Expand All @@ -259,7 +261,7 @@ class NotificationUserSettingsDetail(generics.RetrieveUpdateAPIView):
]


class NotificationList(BulkDeleteMixin, generics.ListAPIView):
class NotificationList(BulkDeleteMixin, ListAPI):
"""List view for all notifications of the current user."""

queryset = common.models.NotificationMessage.objects.all()
Expand Down Expand Up @@ -310,7 +312,7 @@ def filter_delete_queryset(self, queryset, request):
return queryset


class NotificationDetail(generics.RetrieveUpdateDestroyAPIView):
class NotificationDetail(RetrieveUpdateDestroyAPI):
"""Detail view for an individual notification object.
- User can only view / delete their own notification objects
Expand All @@ -323,7 +325,7 @@ class NotificationDetail(generics.RetrieveUpdateDestroyAPIView):
]


class NotificationReadEdit(generics.CreateAPIView):
class NotificationReadEdit(CreateAPI):
"""General API endpoint to manipulate read state of a notification."""

queryset = common.models.NotificationMessage.objects.all()
Expand Down Expand Up @@ -360,7 +362,7 @@ class NotificationUnread(NotificationReadEdit):
target = False


class NotificationReadAll(generics.RetrieveAPIView):
class NotificationReadAll(RetrieveAPI):
"""API endpoint to mark all notifications as read."""

queryset = common.models.NotificationMessage.objects.all()
Expand Down

0 comments on commit e83995b

Please sign in to comment.