diff --git a/docs/python/endpoint-style-guide.md b/docs/python/endpoint-style-guide.md index 705fdf6..5be39b8 100644 --- a/docs/python/endpoint-style-guide.md +++ b/docs/python/endpoint-style-guide.md @@ -1,13 +1,22 @@ +# Views + +Before importing from the default Django REST Framework views, check to see if we +have a custom base view in [our base views](/kirovy/views/base_views.py) first. + +They often have a lot of boilerplate, type hints, and custom request classes set up for you. + +For example, use `KirovyApiView` instead of `rest_framework.views.APIView`. + # API Errors in API class helpers Returning errors in the helper functions of your API endpoint can be annoying. -To avoid that annoyance, just raise one of the [view exceptions](kirovy/exceptions/view_exceptions.py) +To avoid that annoyance, just raise one of the [view exceptions](/kirovy/exceptions/view_exceptions.py) or write your own that subclasses `KirovyValidationError`. **Example where you annoy yourself with bubbling returns:** ```python -class MyView(APIView): +class MyView(KirovyApiView): ... def helper(self, request: KirovyRequest) -> MyObject | KirovyResponse: object_id = request.data.get("id") @@ -36,7 +45,7 @@ class MyView(APIView): **Example where you just raise the exception:** ```python -class MyView(APIView): +class MyView(KirovyApiView): ... def helper(self, request: KirovyRequest) -> MyObject: object_id = request.data.get("id") diff --git a/kirovy/migrations/0021_remove_cncmapfile_kirovy_cncm_cnc_map_a1e8af_idx_and_more.py b/kirovy/migrations/0021_remove_cncmapfile_kirovy_cncm_cnc_map_a1e8af_idx_and_more.py new file mode 100644 index 0000000..1347363 --- /dev/null +++ b/kirovy/migrations/0021_remove_cncmapfile_kirovy_cncm_cnc_map_a1e8af_idx_and_more.py @@ -0,0 +1,131 @@ +# Generated by Django 4.2.23 on 2026-01-28 04:46 + +from django.db import migrations, models +import uuid + + +class Migration(migrations.Migration): + + dependencies = [ + ("kirovy", "0020_alter_cncmapfile_file_alter_cncmapimagefile_file_and_more"), + ] + + operations = [ + migrations.RemoveIndex( + model_name="cncmapfile", + name="kirovy_cncm_cnc_map_a1e8af_idx", + ), + migrations.RemoveIndex( + model_name="cncmapimagefile", + name="kirovy_cncm_cnc_map_078511_idx", + ), + migrations.RemoveIndex( + model_name="cncmapimagefile", + name="kirovy_cncm_is_extr_0259c5_idx", + ), + migrations.RemoveIndex( + model_name="cncmapimagefile", + name="kirovy_cncm_cnc_use_b93113_idx", + ), + migrations.RemoveIndex( + model_name="cncmapimagefile", + name="kirovy_cncm_cnc_gam_241448_idx", + ), + migrations.AddField( + model_name="cncmapfile", + name="ip_address", + field=models.CharField(blank=True, db_index=True, max_length=50, null=True), + ), + migrations.AddField( + model_name="cncmapimagefile", + name="ip_address", + field=models.CharField(blank=True, db_index=True, max_length=50, null=True), + ), + migrations.AddField( + model_name="mappreview", + name="ip_address", + field=models.CharField(blank=True, db_index=True, max_length=50, null=True), + ), + migrations.AlterField( + model_name="cncfileextension", + name="id", + field=models.UUIDField( + db_index=True, default=uuid.uuid4, editable=False, primary_key=True, serialize=False + ), + ), + migrations.AlterField( + model_name="cncgame", + name="id", + field=models.UUIDField( + db_index=True, default=uuid.uuid4, editable=False, primary_key=True, serialize=False + ), + ), + migrations.AlterField( + model_name="cncmap", + name="id", + field=models.UUIDField( + db_index=True, default=uuid.uuid4, editable=False, primary_key=True, serialize=False + ), + ), + migrations.AlterField( + model_name="cncmap", + name="is_legacy", + field=models.BooleanField( + db_index=True, default=False, help_text="If true, this is an upload from the old cncnet database." + ), + ), + migrations.AlterField( + model_name="cncmap", + name="is_mapdb1_compatible", + field=models.BooleanField(db_index=True, default=False), + ), + migrations.AlterField( + model_name="cncmap", + name="is_published", + field=models.BooleanField( + db_index=True, default=False, help_text="If true, this map will show up in normal searches and feeds." + ), + ), + migrations.AlterField( + model_name="cncmap", + name="is_temporary", + field=models.BooleanField( + db_index=True, + default=False, + help_text="If true, this will be deleted eventually. This flag is to support sharing in multiplayer lobbies.", + ), + ), + migrations.AlterField( + model_name="cncmapfile", + name="id", + field=models.UUIDField( + db_index=True, default=uuid.uuid4, editable=False, primary_key=True, serialize=False + ), + ), + migrations.AlterField( + model_name="cncmapimagefile", + name="id", + field=models.UUIDField( + db_index=True, default=uuid.uuid4, editable=False, primary_key=True, serialize=False + ), + ), + migrations.AlterField( + model_name="cncmapimagefile", + name="is_extracted", + field=models.BooleanField(db_index=True, default=False), + ), + migrations.AlterField( + model_name="mapcategory", + name="id", + field=models.UUIDField( + db_index=True, default=uuid.uuid4, editable=False, primary_key=True, serialize=False + ), + ), + migrations.AlterField( + model_name="mappreview", + name="id", + field=models.UUIDField( + db_index=True, default=uuid.uuid4, editable=False, primary_key=True, serialize=False + ), + ), + ] diff --git a/kirovy/models/cnc_base_model.py b/kirovy/models/cnc_base_model.py index fc28cb2..240bc11 100644 --- a/kirovy/models/cnc_base_model.py +++ b/kirovy/models/cnc_base_model.py @@ -8,7 +8,7 @@ class CncNetBaseModel(models.Model): """Base model for all cnc net models to inherit from.""" - id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) + id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False, db_index=True) created = models.DateTimeField(auto_now_add=True, null=True) modified = models.DateTimeField(auto_now=True, null=True) @@ -18,6 +18,7 @@ class CncNetBaseModel(models.Model): on_delete=models.SET_NULL, null=True, related_name="modified_%(class)s_set", + db_index=True, ) """:attr: The last user to modify this entry, if applicable.""" diff --git a/kirovy/models/cnc_game.py b/kirovy/models/cnc_game.py index 9bdfc25..c072669 100644 --- a/kirovy/models/cnc_game.py +++ b/kirovy/models/cnc_game.py @@ -147,7 +147,7 @@ def __repr__(self) -> str: class GameScopedUserOwnedModel(CncNetUserOwnedModel): """A user owned object that is specific to a game. e.g. a map or image.""" - cnc_game = models.ForeignKey(CncGame, models.PROTECT, null=False, blank=False) + cnc_game = models.ForeignKey(CncGame, models.PROTECT, null=False, blank=False, db_index=True) class Meta: abstract = True diff --git a/kirovy/models/cnc_map.py b/kirovy/models/cnc_map.py index 1fc0b1a..f9a065d 100644 --- a/kirovy/models/cnc_map.py +++ b/kirovy/models/cnc_map.py @@ -64,6 +64,7 @@ class CncMap(GameScopedUserOwnedModel, Moderabile): is_legacy = models.BooleanField( default=False, help_text="If true, this is an upload from the old cncnet database.", + db_index=True, ) """:attr: This will be set for all maps that we bulk upload from the legacy cncnet map database. @@ -78,8 +79,7 @@ class CncMap(GameScopedUserOwnedModel, Moderabile): """:attr: Tracks the original upload dates for legacy maps, for historical reasons.""" is_published = models.BooleanField( - default=False, - help_text="If true, this map will show up in normal searches and feeds.", + default=False, help_text="If true, this map will show up in normal searches and feeds.", db_index=True ) """:attr: Did the map maker set this map to be published? Published maps show up in normal search and feeds. @@ -90,6 +90,7 @@ class CncMap(GameScopedUserOwnedModel, Moderabile): default=False, help_text="If true, this will be deleted eventually. " "This flag is to support sharing in multiplayer lobbies.", + db_index=True, ) """:attr: Whether this map is temporary. We don't want to keep storing every map that is shared in a multiplayer lobby, @@ -105,15 +106,10 @@ class CncMap(GameScopedUserOwnedModel, Moderabile): ) categories = models.ManyToManyField(MapCategory) - parent = models.ForeignKey( - "CncMap", - on_delete=models.SET_NULL, - null=True, - blank=True, - ) + parent = models.ForeignKey("CncMap", on_delete=models.SET_NULL, null=True, blank=True, db_index=True) """If set, then this map is a child of ``parent``. Used to track edits of other peoples' maps.""" - is_mapdb1_compatible = models.BooleanField(default=False) + is_mapdb1_compatible = models.BooleanField(default=False, db_index=True) """If true, then this map was uploaded by a legacy CnCNet client and is backwards compatible with map db 1.0. This should never be set for maps uploaded via the web UI. @@ -182,7 +178,7 @@ class CncMapFile(file_base.CncNetFileBaseModel): height = models.IntegerField() version = models.IntegerField(editable=False) - cnc_map = models.ForeignKey(CncMap, on_delete=models.CASCADE, null=False) + cnc_map = models.ForeignKey(CncMap, on_delete=models.CASCADE, null=False, db_index=True) ALLOWED_EXTENSION_TYPES = {game_models.CncFileExtension.ExtensionTypes.MAP.value} @@ -192,7 +188,6 @@ class Meta: constraints = [ models.UniqueConstraint(fields=["cnc_map_id", "version"], name="unique_map_version"), ] - indexes = [models.Index(fields=["cnc_map"])] def save(self, *args, **kwargs): if not self.version: @@ -234,7 +229,7 @@ class CncMapImageFile(file_base.CncNetFileBaseModel): width = models.IntegerField() height = models.IntegerField() - cnc_map = models.ForeignKey(CncMap, on_delete=models.CASCADE, null=False) + cnc_map = models.ForeignKey(CncMap, on_delete=models.CASCADE, null=False, db_index=True) ALLOWED_EXTENSION_TYPES = {game_models.CncFileExtension.ExtensionTypes.IMAGE.value} @@ -243,7 +238,7 @@ class CncMapImageFile(file_base.CncNetFileBaseModel): file = models.ImageField(null=False, upload_to=file_base.default_generate_upload_to, max_length=2048) """The actual file this object represent.""" - is_extracted = models.BooleanField(null=False, blank=False, default=False) + is_extracted = models.BooleanField(null=False, blank=False, default=False, db_index=True) """attr: If true, then this image was extracted from the uploaded map file, usually generated by FinalAlert. This will always be false for games released after Yuri's Revenge because Generals and beyond do not pack the @@ -257,14 +252,6 @@ class CncMapImageFile(file_base.CncNetFileBaseModel): If there are ``order`` collisions, then we fallback to the creation date. """ - class Meta: - indexes = [ - models.Index(fields=["cnc_map"]), - models.Index(fields=["is_extracted"]), - models.Index(fields=["cnc_user_id"]), - models.Index(fields=["cnc_game_id"]), - ] - def save(self, *args, **kwargs): if not self.name: self.name = self.cnc_map.map_name diff --git a/kirovy/models/cnc_user.py b/kirovy/models/cnc_user.py index 08cc2f1..9c659f1 100644 --- a/kirovy/models/cnc_user.py +++ b/kirovy/models/cnc_user.py @@ -176,7 +176,7 @@ def create_or_update_from_cncnet(user_dto: CncnetUserInfo) -> "CncUser": class CncNetUserOwnedModel(CncNetBaseModel): """A mixin model for any models that will be owned by a user.""" - cnc_user = models.ForeignKey(CncUser, on_delete=models.PROTECT, null=True) + cnc_user = models.ForeignKey(CncUser, on_delete=models.PROTECT, null=True, db_index=True) """:attr: The user that owns this object, if it has an owner.""" class Meta: diff --git a/kirovy/models/file_base.py b/kirovy/models/file_base.py index fed9b38..b11af77 100644 --- a/kirovy/models/file_base.py +++ b/kirovy/models/file_base.py @@ -60,6 +60,9 @@ class Meta: hash_sha1 = models.CharField(max_length=50, null=True, blank=False) """Backwards compatibility with the old CncNetClient.""" + ip_address = models.CharField(max_length=50, null=True, blank=True, db_index=True) + """IP address that uploaded the file. 50 is long enough for ipv6""" + def validate_file_extension(self, file_extension: game_models.CncFileExtension) -> None: """Validate that an extension is supported for a game. diff --git a/kirovy/request.py b/kirovy/request.py index ec0f7bb..49c725a 100644 --- a/kirovy/request.py +++ b/kirovy/request.py @@ -1,3 +1,5 @@ +from functools import cached_property + from rest_framework.request import Request as _DRFRequest from kirovy import models, typing as t, objects @@ -11,3 +13,13 @@ class KirovyRequest(_DRFRequest): user: t.Optional[models.CncUser] auth: t.Optional[objects.CncnetUserInfo] + + @cached_property + def client_ip_address(self) -> str: + if self.user.is_staff: + return "staff" + x_forwarded_for: str | None = self.META.get("HTTP_X_FORWARDED_FOR") + if x_forwarded_for: + return x_forwarded_for.split(",")[0].strip() + + return self.META.get("REMOTE_ADDR", "unknown") diff --git a/kirovy/serializers/__init__.py b/kirovy/serializers/__init__.py index 0b0a3e3..a063b52 100644 --- a/kirovy/serializers/__init__.py +++ b/kirovy/serializers/__init__.py @@ -1,3 +1,5 @@ +from functools import cached_property + from rest_framework import serializers from kirovy.constants import api_codes @@ -18,25 +20,33 @@ class KirovySerializer(serializers.Serializer): source="last_modified_by", queryset=CncUser.objects.all(), pk_field=serializers.UUIDField(), + allow_null=True, ) class Meta: exclude = ["last_modified_by"] - fields = "__all__" editable_fields: t.ClassVar[set[str]] = set() - def get_fields(self): + @cached_property + def permissioned_readable_fields(self): """Get fields based on permission level. Removes admin-only fields for non-admin requests. Will always remove the fields if the serializer doesn't have context. """ - fields = super().get_fields() + fields = self.fields request: t.Optional[KirovyRequest] = self.context.get("request") if not (request and request.user.is_authenticated and request.user.is_staff): fields.pop("last_modified_by_id", None) + fields.pop("ip_address", None) return fields + @property + def _readable_fields(self): + for field in self.permissioned_readable_fields.values(): + if not field.write_only: + yield field + def to_internal_value(self, data: dict) -> dict: """Convert the raw request data into data that can be used in a django model. @@ -72,4 +82,3 @@ class CncNetUserOwnedModelSerializer(KirovySerializer): class Meta: exclude = ["cnc_user"] - fields = "__all__" diff --git a/kirovy/serializers/cnc_map_serializers.py b/kirovy/serializers/cnc_map_serializers.py index 7ddd335..1e58bcf 100644 --- a/kirovy/serializers/cnc_map_serializers.py +++ b/kirovy/serializers/cnc_map_serializers.py @@ -30,7 +30,6 @@ class Meta: model = cnc_map.CncMapFile # We return the ID instead of the whole object. exclude = ["cnc_game", "cnc_map", "file_extension", "cnc_user"] - fields = "__all__" width = serializers.IntegerField() """attr: The map height. @@ -88,6 +87,8 @@ class Meta: hash_sha512 = serializers.CharField(required=True, allow_blank=False) hash_sha1 = serializers.CharField(required=True, allow_blank=False) + ip_address = serializers.CharField(required=True, allow_null=True) + def create(self, validated_data: t.DictStrAny) -> cnc_map.CncMapFile: map_file = cnc_map.CncMapFile(**validated_data) map_file.save() @@ -108,7 +109,6 @@ class Meta: model = cnc_map.CncMapImageFile # We return the ID instead of the whole object. exclude = ["cnc_user", "cnc_game", "cnc_map", "file_extension", "hash_md5", "hash_sha512", "hash_sha1"] - fields = "__all__" editable_fields: set[str] = {"name", "image_order"} width = serializers.IntegerField() @@ -167,6 +167,8 @@ class Meta: pk_field=serializers.UUIDField(), ) + ip_address = serializers.CharField(required=True, allow_null=True) + def create(self, validated_data: t.DictStrAny) -> cnc_map.CncMapImageFile: image_file = cnc_map.CncMapImageFile(**validated_data) image_file.save() diff --git a/kirovy/views/admin_views.py b/kirovy/views/admin_views.py index 618a4f5..e3f7edc 100644 --- a/kirovy/views/admin_views.py +++ b/kirovy/views/admin_views.py @@ -1,7 +1,6 @@ import pydantic from rest_framework import status from rest_framework.generics import get_object_or_404 -from rest_framework.views import APIView from kirovy import permissions, exceptions from kirovy.exceptions.view_exceptions import KirovyValidationError @@ -9,9 +8,10 @@ from kirovy.objects import ui_objects from kirovy.request import KirovyRequest from kirovy.response import KirovyResponse +from kirovy.views.base_views import KirovyApiView -class BanView(APIView): +class BanView(KirovyApiView): """The view for banning things. ``POST /admin/ban/`` diff --git a/kirovy/views/base_views.py b/kirovy/views/base_views.py index f902765..a3c4a59 100644 --- a/kirovy/views/base_views.py +++ b/kirovy/views/base_views.py @@ -32,6 +32,24 @@ _LOGGER = logging.get_logger(__name__) +class KirovyApiView(APIView): + request: KirovyRequest + + def initialize_request(self, request, *args, **kwargs): + """ + Returns the initial request object. + """ + parser_context = self.get_parser_context(request) + + return KirovyRequest( + request, + parsers=self.get_parsers(), + authenticators=self.get_authenticators(), + negotiator=self.get_content_negotiator(), + parser_context=parser_context, + ) + + class KirovyDefaultPagination(_pagination.LimitOffsetPagination): """Default pagination values.""" @@ -65,6 +83,20 @@ class KirovyListCreateView(_g.ListCreateAPIView): _paginator: t.Optional[KirovyDefaultPagination] request: KirovyRequest # Added for type hinting. Populated by DRF ``.setup()`` + def initialize_request(self, request, *args, **kwargs): + """ + Returns the initial request object. + """ + parser_context = self.get_parser_context(request) + + return KirovyRequest( + request, + parsers=self.get_parsers(), + authenticators=self.get_authenticators(), + negotiator=self.get_content_negotiator(), + parser_context=parser_context, + ) + def create(self, request: KirovyRequest, *args, **kwargs) -> KirovyResponse[ui_objects.ResultResponseData]: data = request.data if isinstance(data, dict) and issubclass(self.get_serializer_class(), KirovySerializer): @@ -112,6 +144,20 @@ class KirovyRetrieveUpdateView(_g.RetrieveUpdateAPIView): request: KirovyRequest # Added for type hinting. Populated by DRF ``.setup()`` permission_classes = [permissions.CanEdit | permissions.ReadOnly] + def initialize_request(self, request, *args, **kwargs): + """ + Returns the initial request object. + """ + parser_context = self.get_parser_context(request) + + return KirovyRequest( + request, + parsers=self.get_parsers(), + authenticators=self.get_authenticators(), + negotiator=self.get_content_negotiator(), + parser_context=parser_context, + ) + def retrieve(self, request, *args, **kwargs): instance = self.get_object() serializer = self.get_serializer(instance) @@ -153,8 +199,22 @@ class KirovyDestroyView(_g.DestroyAPIView): request: KirovyRequest # Added for type hinting. Populated by DRF ``.setup()`` permission_classes = [permissions.CanDelete | _p.IsAdminUser] + def initialize_request(self, request, *args, **kwargs): + """ + Returns the initial request object. + """ + parser_context = self.get_parser_context(request) + + return KirovyRequest( + request, + parsers=self.get_parsers(), + authenticators=self.get_authenticators(), + negotiator=self.get_content_negotiator(), + parser_context=parser_context, + ) + -class FileUploadBaseView(APIView, metaclass=ABCMeta): +class FileUploadBaseView(KirovyApiView, metaclass=ABCMeta): """A base class for uploading files that are **not** map files for a game.""" parser_classes = [MultiPartParser] @@ -225,8 +285,11 @@ def post(self, request: KirovyRequest, format=None) -> KirovyResponse[ui_objects "file": uploaded_file, "file_extension_id": extension_id, "cnc_user_id": self.request.user.id, + "ip_address": self.request.client_ip_address, + "last_modified_by_id": self.request.user.id, **self.extra_serializer_data(request, uploaded_file, parent_object), - } + }, + context={"request": self.request}, ) if not serializer.is_valid(raise_exception=False): diff --git a/kirovy/views/cnc_map_views.py b/kirovy/views/cnc_map_views.py index 3af0010..b3efdc2 100644 --- a/kirovy/views/cnc_map_views.py +++ b/kirovy/views/cnc_map_views.py @@ -9,7 +9,6 @@ from django_filters import rest_framework as filters from rest_framework.permissions import AllowAny from rest_framework.renderers import TemplateHTMLRenderer -from rest_framework.views import APIView from kirovy import permissions from kirovy.models import ( @@ -25,6 +24,7 @@ from kirovy.views import base_views from structlog import get_logger +from kirovy.views.base_views import KirovyApiView _LOGGER = get_logger(__name__) @@ -246,7 +246,7 @@ def perform_destroy(self, instance: CncMap): return super().perform_destroy(instance) -class BackwardsCompatibleMapView(APIView): +class BackwardsCompatibleMapView(KirovyApiView): """Match the legacy mapdb download endpoints. This is needed until the new UI is running for the clients CnCNet owns. @@ -272,7 +272,7 @@ def get(self, request, sha1_hash_filename: str, game_id: UUID, format=None): return FileResponse(map_file.file.open("rb"), as_attachment=True, filename=f"{map_file.hash_sha1}.zip") -class MapLegacyStaticUI(APIView): +class MapLegacyStaticUI(KirovyApiView): """Temporary upload page for backwards compatible upload testing. Map authors need an easy way to upload their maps to the database so that they diff --git a/kirovy/views/map_upload_views.py b/kirovy/views/map_upload_views.py index 035c546..67a35c3 100644 --- a/kirovy/views/map_upload_views.py +++ b/kirovy/views/map_upload_views.py @@ -10,7 +10,6 @@ from rest_framework import status from rest_framework.parsers import MultiPartParser from rest_framework.permissions import BasePermission, AllowAny -from rest_framework.views import APIView from kirovy import typing as t, permissions, exceptions, constants, logging from kirovy.constants.api_codes import UploadApiCodes @@ -25,7 +24,7 @@ from kirovy.services.cnc_gen_2_services import CncGen2MapParser, CncGen2MapSections from kirovy.services.file_extension_service import FileExtensionService from kirovy.utils import file_utils - +from kirovy.views.base_views import KirovyApiView _LOGGER = logging.get_logger(__name__) @@ -36,10 +35,9 @@ class MapHashes(t.NamedTuple): sha512: str -class _BaseMapFileUploadView(APIView, metaclass=ABCMeta): +class _BaseMapFileUploadView(KirovyApiView, metaclass=ABCMeta): parser_classes = [MultiPartParser] permission_classes: t.ClassVar[t.Iterable[BasePermission]] - request: KirovyRequest upload_is_temporary: t.ClassVar[bool] def __init__(self, **kwargs): @@ -80,6 +78,7 @@ def post(self, request: KirovyRequest, format=None) -> KirovyResponse: cnc_user_id=request.user.id, parent_id=parent_map.id if parent_map else None, last_modified_by_id=request.user.id, + is_temporary=self.upload_is_temporary, ), context={"request": self.request}, ) @@ -138,6 +137,7 @@ def post(self, request: KirovyRequest, format=None) -> KirovyResponse: hash_sha1=map_hashes_post_processing.sha1, cnc_user_id=self.request.user.id, last_modified_by_id=self.request.user.id, + ip_address=self.request.client_ip_address, ), context={"request": self.request}, ) @@ -183,6 +183,8 @@ def extract_preview(self, new_map_file: cnc_map.CncMapFile, map_parser: CncGen2M file_extension_id=image_extension.id, image_order=999, cnc_user_id=self.request.user.id, + ip_address=self.request.client_ip_address, + last_modified_by_id=self.request.user.id, ) ) image_serializer.is_valid(raise_exception=True) @@ -219,10 +221,9 @@ def get_map_parent(self, map_parser: CncGen2MapParser) -> cnc_map.CncMap | None: @cached_property def user_log_attrs(self) -> t.DictStrAny: # todo move to structlogger - naughty_ip_address = self.request.META.get("HTTP_X_FORWARDED_FOR", "unknown") user = self.request.user return { - "ip_address": naughty_ip_address, + "ip_address": self.request.client_ip_address, "user": f"[{user.cncnet_id}] {user.username}" if user.is_authenticated else "unauthenticated_upload", } @@ -400,6 +401,7 @@ def post(self, request: KirovyRequest, format=None) -> KirovyResponse: cnc_user=CncUser.objects.get_or_create_legacy_upload_user(), parent=None, is_mapdb1_compatible=True, + is_temporary=True, ) new_map.save() @@ -415,6 +417,8 @@ def post(self, request: KirovyRequest, format=None) -> KirovyResponse: hash_sha512=map_hashes.sha512, hash_sha1=map_hashes.sha1, cnc_user_id=CncUser.objects.get_or_create_legacy_upload_user().id, + ip_address=self.request.client_ip_address, + last_modified_by_id=None, ), context={"request": self.request}, ) diff --git a/kirovy/views/permission_views.py b/kirovy/views/permission_views.py index 84fa1d4..5b58343 100644 --- a/kirovy/views/permission_views.py +++ b/kirovy/views/permission_views.py @@ -1,13 +1,13 @@ from rest_framework import status -from rest_framework.views import APIView import kirovy.objects.ui_objects -from kirovy import permissions, typing as t +from kirovy import permissions from kirovy.request import KirovyRequest from kirovy.response import KirovyResponse +from kirovy.views.base_views import KirovyApiView -class ListPermissionForAuthUser(APIView): +class ListPermissionForAuthUser(KirovyApiView): """End point to check which buttons / views the UI should show. The UI showing the buttons / views will not guarantee access. The backend still checks permissions for all diff --git a/kirovy/views/test.py b/kirovy/views/test.py index d234ba9..b700f90 100644 --- a/kirovy/views/test.py +++ b/kirovy/views/test.py @@ -1,14 +1,13 @@ from django.conf import settings from rest_framework import status from rest_framework.permissions import IsAuthenticated -from rest_framework.request import Request from rest_framework.response import Response -from rest_framework.views import APIView from kirovy.request import KirovyRequest +from kirovy.views.base_views import KirovyApiView -class TestJwt(APIView): +class TestJwt(KirovyApiView): """ Test JWT tokens. Only for use in tests. """ diff --git a/tests/test_views/test_backwards_compatibility.py b/tests/test_views/test_backwards_compatibility.py index 1c4a441..2d254fd 100644 --- a/tests/test_views/test_backwards_compatibility.py +++ b/tests/test_views/test_backwards_compatibility.py @@ -76,6 +76,7 @@ def test_map_upload_single_file_backwards_compatible( game_dawn_of_the_tiberium_age, game_red_alert, ): + """Test uploads for backwards compatible map files that only have a map file in the zip.""" game_map_name = [ (game_yuri, file_map_desert, "desert"), (game_tiberian_sun, file_map_ts_woodland_hills, "Woodland Hills"), @@ -87,17 +88,24 @@ def test_map_upload_single_file_backwards_compatible( original_extension = pathlib.Path(file_map.name).suffix upload_file, file_sha1 = zip_map_for_legacy_upload(file_map) upload_response: KirovyResponse = client_anonymous.post( - url, {"file": upload_file, "game": game.slug}, format="multipart", content_type=None + url, {"file": upload_file, "game": game.slug}, format="multipart", content_type=None, REMOTE_ADDR="2.2.2.2" ) assert upload_response.status_code == status.HTTP_200_OK assert upload_response.data["result"]["download_url"] == f"/{game.slug}/{file_sha1}.zip" - _download_and_check_hash(client_anonymous, file_sha1, game, map_name, [original_extension]) + _download_and_check_hash( + client_anonymous, file_sha1, game, map_name, [original_extension], ip_address="2.2.2.2" + ) def _download_and_check_hash( - client: "KirovyClient", file_sha1: str, game: CncGame, expected_map_name: str, extensions_to_check: t.List[str] + client: "KirovyClient", + file_sha1: str, + game: CncGame, + expected_map_name: str, + extensions_to_check: t.List[str], + ip_address: str | None = None, ): response: FileResponse = client.get(f"/{game.slug}/{file_sha1}.zip") assert response.status_code == status.HTTP_200_OK @@ -112,3 +120,5 @@ def _download_and_check_hash( assert cnc_map_file assert cnc_map_file.cnc_game_id == game.id assert cnc_map_file.cnc_map.map_name == expected_map_name + if ip_address: + assert cnc_map_file.ip_address == ip_address diff --git a/tests/test_views/test_map_image_view.py b/tests/test_views/test_map_image_view.py index 9ca20ac..a035912 100644 --- a/tests/test_views/test_map_image_view.py +++ b/tests/test_views/test_map_image_view.py @@ -6,6 +6,8 @@ from kirovy.constants import api_codes from kirovy.constants.api_codes import FileUploadApiCodes from kirovy.models.cnc_map import CncMapImageFile +from kirovy.objects.ui_objects import ResultResponseData +from kirovy.response import KirovyResponse from kirovy.views.map_image_views import MapImageFileUploadView from rest_framework import status @@ -22,12 +24,13 @@ def test_map_image_upload__happy_path(create_cnc_map, file_map_image, client_use {"file": file_map_image, "cnc_map_id": str(cnc_map.id)}, format="multipart", content_type=None, + HTTP_X_FORWARDED_FOR="1.1.1.1", ) assert response.status_code == status.HTTP_201_CREATED assert response.data["message"] == MapImageFileUploadView.success_message file_id = response.data["result"]["file_id"] - get_response = client_user.get(f"/maps/img/{file_id}/") + get_response: KirovyResponse[ResultResponseData] = client_user.get(f"/maps/img/{file_id}/") assert get_response.status_code == status.HTTP_200_OK saved_file_raw = CncMapImageFile.objects.get(id=file_id) @@ -54,6 +57,9 @@ def test_map_image_upload__happy_path(create_cnc_map, file_map_image, client_use assert saved_file["cnc_user_id"] == str(client_user.kirovy_user.id) assert saved_file_raw.file.size < file_map_image.size, "Converting to jpeg should have shrunk the file size." assert saved_file["file"].endswith(expected_url_path) + assert "ip_address" not in saved_file.keys() + + assert CncMapImageFile.objects.get(id=file_id).ip_address == "1.1.1.1" def test_map_image_upload__jpg(create_cnc_map, file_map_image_jpg, client_user, get_file_path_for_uploaded_file_url): @@ -247,3 +253,29 @@ def test_map_image_delete(client_user, create_cnc_map, create_cnc_map_image_file with pytest.raises(CncMapImageFile.DoesNotExist): image_file.refresh_from_db() + + +def test_map_image_upload__admin_ip( + create_cnc_map, file_map_image, client_moderator, get_file_path_for_uploaded_file_url +): + """Test that admin IPs are just `staff`. Test that admins can see IPs.""" + cnc_map = create_cnc_map( + user_id=client_moderator.kirovy_user.id, is_legacy=False, is_published=True, is_temporary=False + ) + response = client_moderator.post( + "/maps/img/", + {"file": file_map_image, "cnc_map_id": str(cnc_map.id)}, + format="multipart", + content_type=None, + HTTP_X_FORWARDED_FOR="1.1.1.1", + ) + + assert response.status_code == status.HTTP_201_CREATED + assert response.data["message"] == MapImageFileUploadView.success_message + file_id = response.data["result"]["file_id"] + get_response: KirovyResponse[ResultResponseData] = client_moderator.get(f"/maps/img/{file_id}/") + assert get_response.status_code == status.HTTP_200_OK + + saved_file_raw = CncMapImageFile.objects.get(id=file_id) + saved_file = get_response.data["result"] + assert saved_file["ip_address"] == saved_file_raw.ip_address == "staff" diff --git a/tests/test_views/test_map_upload.py b/tests/test_views/test_map_upload.py index 778584a..3227250 100644 --- a/tests/test_views/test_map_upload.py +++ b/tests/test_views/test_map_upload.py @@ -5,6 +5,7 @@ from kirovy.constants.api_codes import UploadApiCodes, FileUploadApiCodes from kirovy.models.cnc_map import CncMapImageFile from kirovy.objects import ui_objects +from kirovy.objects.ui_objects import ResultResponseData from kirovy.utils import file_utils from kirovy.models import CncMap, CncMapFile, MapCategory, CncGame from kirovy.response import KirovyResponse @@ -17,9 +18,11 @@ def test_map_file_upload_happy_path( client_user, file_map_desert, game_uploadable, extension_map, tmp_media_root, get_file_path_for_uploaded_file_url ): + """Test uploading a map file via the UI endpoint.""" response = client_user.post_file( _UPLOAD_URL, {"file": file_map_desert, "game_id": str(game_uploadable.id)}, + HTTP_X_FORWARDED_FOR="117.117.117.117", ) assert response.status_code == status.HTTP_201_CREATED @@ -51,6 +54,7 @@ def test_map_file_upload_happy_path( assert file_object.cnc_user_id == client_user.kirovy_user.id assert image_object.cnc_user_id == client_user.kirovy_user.id assert map_object.cnc_user_id == client_user.kirovy_user.id + assert file_object.ip_address == image_object.ip_address == "117.117.117.117" # Note: These won't match a md5 from the commandline because we add the ID to the map file. assert file_object.hash_md5 == file_utils.hash_file_md5(uploaded_file.open()) @@ -73,6 +77,9 @@ def test_map_file_upload_happy_path( assert not response_map["is_temporary"], "Maps uploaded via a signed in user shouldn't be marked as temporary." assert not response_map["is_reviewed"], "Maps should not default to being reviewed." assert not response_map["is_banned"], "Happy path maps should not be banned on upload." + assert response_map[ + "incomplete_upload" + ], "Map files that have been uploaded via the UI should be marked as incomplete until the user sets map info." assert response_map["legacy_upload_date"] is None, "Non legacy maps should never have this field." assert response_map["id"] == str(map_object.id) @@ -88,6 +95,8 @@ def test_map_file_upload_happy_path( assert response_map["images"][0]["file"].endswith(uploaded_image_url) assert response_map["images"][0]["is_extracted"] is True + assert "ip_address" not in response_map.keys() + def test_map_file_upload_banned_user(file_map_desert, game_uploadable, client_banned): """Test that a banned user cannot upload a new map.""" @@ -145,3 +154,24 @@ def test_map_file_upload_game_allowances( {"file": file_map_desert, "game_id": str(game.id)}, ) assert response.status_code == status.HTTP_201_CREATED + + +def test_map_file_upload__staff_ip(client_moderator, file_map_desert, game_uploadable, tmp_media_root): + """Test uploading a map file via the UI endpoint has correct IP.""" + response = client_moderator.post_file( + _UPLOAD_URL, + {"file": file_map_desert, "game_id": str(game_uploadable.id)}, + HTTP_X_FORWARDED_FOR="117.117.117.117", + ) + + assert response.status_code == status.HTTP_201_CREATED + + map_id: str = response.data["result"]["cnc_map_id"] + get_response: KirovyResponse[ResultResponseData] = client_moderator.get(f"/maps/{map_id}/") + + assert len(get_response.data["result"]["files"]) == 1 + assert ( + get_response.data["result"]["files"][0]["ip_address"] + == CncMapFile.objects.get(cnc_map_id=map_id).ip_address + == "staff" + ) diff --git a/tests/test_views/test_search_view.py b/tests/test_views/test_search_view.py index bc930da..43e0005 100644 --- a/tests/test_views/test_search_view.py +++ b/tests/test_views/test_search_view.py @@ -19,6 +19,7 @@ def test_search_map_name(create_cnc_map, client_anonymous): assert response.status_code == status.HTTP_200_OK assert len(response.data["results"]) == 1 assert response.data["results"][0]["id"] == str(expected.id) + assert "ip_address" not in response.data["results"][0].keys() def test_search_map__categories(create_cnc_map, client_anonymous, create_cnc_map_category):