Skip to content

Commit 774c7e3

Browse files
authored
feat: IP addresses (#33)
- Log IP of uploader - Make kirovy base views use the custom KirovyRequest - Fix `last_modified_by_id` not being written for non-admins - Make the removal of non-admin fields not affect writes. - Use the KirovyApiSerializer everywhere - Don't log admin IPs - Make sure IPs are only shown to admins - Fix links to base views and view exceptions
1 parent 2be42fe commit 774c7e3

20 files changed

Lines changed: 349 additions & 56 deletions

docs/python/endpoint-style-guide.md

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,22 @@
1+
# Views
2+
3+
Before importing from the default Django REST Framework views, check to see if we
4+
have a custom base view in [our base views](/kirovy/views/base_views.py) first.
5+
6+
They often have a lot of boilerplate, type hints, and custom request classes set up for you.
7+
8+
For example, use `KirovyApiView` instead of `rest_framework.views.APIView`.
9+
110
# API Errors in API class helpers
211

312
Returning errors in the helper functions of your API endpoint can be annoying.
4-
To avoid that annoyance, just raise one of the [view exceptions](kirovy/exceptions/view_exceptions.py)
13+
To avoid that annoyance, just raise one of the [view exceptions](/kirovy/exceptions/view_exceptions.py)
514
or write your own that subclasses `KirovyValidationError`.
615

716
**Example where you annoy yourself with bubbling returns:**
817

918
```python
10-
class MyView(APIView):
19+
class MyView(KirovyApiView):
1120
...
1221
def helper(self, request: KirovyRequest) -> MyObject | KirovyResponse:
1322
object_id = request.data.get("id")
@@ -36,7 +45,7 @@ class MyView(APIView):
3645
**Example where you just raise the exception:**
3746

3847
```python
39-
class MyView(APIView):
48+
class MyView(KirovyApiView):
4049
...
4150
def helper(self, request: KirovyRequest) -> MyObject:
4251
object_id = request.data.get("id")
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
# Generated by Django 4.2.23 on 2026-01-28 04:46
2+
3+
from django.db import migrations, models
4+
import uuid
5+
6+
7+
class Migration(migrations.Migration):
8+
9+
dependencies = [
10+
("kirovy", "0020_alter_cncmapfile_file_alter_cncmapimagefile_file_and_more"),
11+
]
12+
13+
operations = [
14+
migrations.RemoveIndex(
15+
model_name="cncmapfile",
16+
name="kirovy_cncm_cnc_map_a1e8af_idx",
17+
),
18+
migrations.RemoveIndex(
19+
model_name="cncmapimagefile",
20+
name="kirovy_cncm_cnc_map_078511_idx",
21+
),
22+
migrations.RemoveIndex(
23+
model_name="cncmapimagefile",
24+
name="kirovy_cncm_is_extr_0259c5_idx",
25+
),
26+
migrations.RemoveIndex(
27+
model_name="cncmapimagefile",
28+
name="kirovy_cncm_cnc_use_b93113_idx",
29+
),
30+
migrations.RemoveIndex(
31+
model_name="cncmapimagefile",
32+
name="kirovy_cncm_cnc_gam_241448_idx",
33+
),
34+
migrations.AddField(
35+
model_name="cncmapfile",
36+
name="ip_address",
37+
field=models.CharField(blank=True, db_index=True, max_length=50, null=True),
38+
),
39+
migrations.AddField(
40+
model_name="cncmapimagefile",
41+
name="ip_address",
42+
field=models.CharField(blank=True, db_index=True, max_length=50, null=True),
43+
),
44+
migrations.AddField(
45+
model_name="mappreview",
46+
name="ip_address",
47+
field=models.CharField(blank=True, db_index=True, max_length=50, null=True),
48+
),
49+
migrations.AlterField(
50+
model_name="cncfileextension",
51+
name="id",
52+
field=models.UUIDField(
53+
db_index=True, default=uuid.uuid4, editable=False, primary_key=True, serialize=False
54+
),
55+
),
56+
migrations.AlterField(
57+
model_name="cncgame",
58+
name="id",
59+
field=models.UUIDField(
60+
db_index=True, default=uuid.uuid4, editable=False, primary_key=True, serialize=False
61+
),
62+
),
63+
migrations.AlterField(
64+
model_name="cncmap",
65+
name="id",
66+
field=models.UUIDField(
67+
db_index=True, default=uuid.uuid4, editable=False, primary_key=True, serialize=False
68+
),
69+
),
70+
migrations.AlterField(
71+
model_name="cncmap",
72+
name="is_legacy",
73+
field=models.BooleanField(
74+
db_index=True, default=False, help_text="If true, this is an upload from the old cncnet database."
75+
),
76+
),
77+
migrations.AlterField(
78+
model_name="cncmap",
79+
name="is_mapdb1_compatible",
80+
field=models.BooleanField(db_index=True, default=False),
81+
),
82+
migrations.AlterField(
83+
model_name="cncmap",
84+
name="is_published",
85+
field=models.BooleanField(
86+
db_index=True, default=False, help_text="If true, this map will show up in normal searches and feeds."
87+
),
88+
),
89+
migrations.AlterField(
90+
model_name="cncmap",
91+
name="is_temporary",
92+
field=models.BooleanField(
93+
db_index=True,
94+
default=False,
95+
help_text="If true, this will be deleted eventually. This flag is to support sharing in multiplayer lobbies.",
96+
),
97+
),
98+
migrations.AlterField(
99+
model_name="cncmapfile",
100+
name="id",
101+
field=models.UUIDField(
102+
db_index=True, default=uuid.uuid4, editable=False, primary_key=True, serialize=False
103+
),
104+
),
105+
migrations.AlterField(
106+
model_name="cncmapimagefile",
107+
name="id",
108+
field=models.UUIDField(
109+
db_index=True, default=uuid.uuid4, editable=False, primary_key=True, serialize=False
110+
),
111+
),
112+
migrations.AlterField(
113+
model_name="cncmapimagefile",
114+
name="is_extracted",
115+
field=models.BooleanField(db_index=True, default=False),
116+
),
117+
migrations.AlterField(
118+
model_name="mapcategory",
119+
name="id",
120+
field=models.UUIDField(
121+
db_index=True, default=uuid.uuid4, editable=False, primary_key=True, serialize=False
122+
),
123+
),
124+
migrations.AlterField(
125+
model_name="mappreview",
126+
name="id",
127+
field=models.UUIDField(
128+
db_index=True, default=uuid.uuid4, editable=False, primary_key=True, serialize=False
129+
),
130+
),
131+
]

kirovy/models/cnc_base_model.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
class CncNetBaseModel(models.Model):
99
"""Base model for all cnc net models to inherit from."""
1010

11-
id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False)
11+
id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False, db_index=True)
1212

1313
created = models.DateTimeField(auto_now_add=True, null=True)
1414
modified = models.DateTimeField(auto_now=True, null=True)
@@ -18,6 +18,7 @@ class CncNetBaseModel(models.Model):
1818
on_delete=models.SET_NULL,
1919
null=True,
2020
related_name="modified_%(class)s_set",
21+
db_index=True,
2122
)
2223
""":attr: The last user to modify this entry, if applicable."""
2324

kirovy/models/cnc_game.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ def __repr__(self) -> str:
147147
class GameScopedUserOwnedModel(CncNetUserOwnedModel):
148148
"""A user owned object that is specific to a game. e.g. a map or image."""
149149

150-
cnc_game = models.ForeignKey(CncGame, models.PROTECT, null=False, blank=False)
150+
cnc_game = models.ForeignKey(CncGame, models.PROTECT, null=False, blank=False, db_index=True)
151151

152152
class Meta:
153153
abstract = True

kirovy/models/cnc_map.py

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ class CncMap(GameScopedUserOwnedModel, Moderabile):
6464
is_legacy = models.BooleanField(
6565
default=False,
6666
help_text="If true, this is an upload from the old cncnet database.",
67+
db_index=True,
6768
)
6869
""":attr:
6970
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):
7879
""":attr: Tracks the original upload dates for legacy maps, for historical reasons."""
7980

8081
is_published = models.BooleanField(
81-
default=False,
82-
help_text="If true, this map will show up in normal searches and feeds.",
82+
default=False, help_text="If true, this map will show up in normal searches and feeds.", db_index=True
8383
)
8484
""":attr:
8585
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):
9090
default=False,
9191
help_text="If true, this will be deleted eventually. "
9292
"This flag is to support sharing in multiplayer lobbies.",
93+
db_index=True,
9394
)
9495
""":attr:
9596
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):
105106
)
106107

107108
categories = models.ManyToManyField(MapCategory)
108-
parent = models.ForeignKey(
109-
"CncMap",
110-
on_delete=models.SET_NULL,
111-
null=True,
112-
blank=True,
113-
)
109+
parent = models.ForeignKey("CncMap", on_delete=models.SET_NULL, null=True, blank=True, db_index=True)
114110
"""If set, then this map is a child of ``parent``. Used to track edits of other peoples' maps."""
115111

116-
is_mapdb1_compatible = models.BooleanField(default=False)
112+
is_mapdb1_compatible = models.BooleanField(default=False, db_index=True)
117113
"""If true, then this map was uploaded by a legacy CnCNet client and is backwards compatible with map db 1.0.
118114
119115
This should never be set for maps uploaded via the web UI.
@@ -182,7 +178,7 @@ class CncMapFile(file_base.CncNetFileBaseModel):
182178
height = models.IntegerField()
183179
version = models.IntegerField(editable=False)
184180

185-
cnc_map = models.ForeignKey(CncMap, on_delete=models.CASCADE, null=False)
181+
cnc_map = models.ForeignKey(CncMap, on_delete=models.CASCADE, null=False, db_index=True)
186182

187183
ALLOWED_EXTENSION_TYPES = {game_models.CncFileExtension.ExtensionTypes.MAP.value}
188184

@@ -192,7 +188,6 @@ class Meta:
192188
constraints = [
193189
models.UniqueConstraint(fields=["cnc_map_id", "version"], name="unique_map_version"),
194190
]
195-
indexes = [models.Index(fields=["cnc_map"])]
196191

197192
def save(self, *args, **kwargs):
198193
if not self.version:
@@ -234,7 +229,7 @@ class CncMapImageFile(file_base.CncNetFileBaseModel):
234229
width = models.IntegerField()
235230
height = models.IntegerField()
236231

237-
cnc_map = models.ForeignKey(CncMap, on_delete=models.CASCADE, null=False)
232+
cnc_map = models.ForeignKey(CncMap, on_delete=models.CASCADE, null=False, db_index=True)
238233

239234
ALLOWED_EXTENSION_TYPES = {game_models.CncFileExtension.ExtensionTypes.IMAGE.value}
240235

@@ -243,7 +238,7 @@ class CncMapImageFile(file_base.CncNetFileBaseModel):
243238
file = models.ImageField(null=False, upload_to=file_base.default_generate_upload_to, max_length=2048)
244239
"""The actual file this object represent."""
245240

246-
is_extracted = models.BooleanField(null=False, blank=False, default=False)
241+
is_extracted = models.BooleanField(null=False, blank=False, default=False, db_index=True)
247242
"""attr: If true, then this image was extracted from the uploaded map file, usually generated by FinalAlert.
248243
249244
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):
257252
If there are ``order`` collisions, then we fallback to the creation date.
258253
"""
259254

260-
class Meta:
261-
indexes = [
262-
models.Index(fields=["cnc_map"]),
263-
models.Index(fields=["is_extracted"]),
264-
models.Index(fields=["cnc_user_id"]),
265-
models.Index(fields=["cnc_game_id"]),
266-
]
267-
268255
def save(self, *args, **kwargs):
269256
if not self.name:
270257
self.name = self.cnc_map.map_name

kirovy/models/cnc_user.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ def create_or_update_from_cncnet(user_dto: CncnetUserInfo) -> "CncUser":
176176
class CncNetUserOwnedModel(CncNetBaseModel):
177177
"""A mixin model for any models that will be owned by a user."""
178178

179-
cnc_user = models.ForeignKey(CncUser, on_delete=models.PROTECT, null=True)
179+
cnc_user = models.ForeignKey(CncUser, on_delete=models.PROTECT, null=True, db_index=True)
180180
""":attr: The user that owns this object, if it has an owner."""
181181

182182
class Meta:

kirovy/models/file_base.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@ class Meta:
6060
hash_sha1 = models.CharField(max_length=50, null=True, blank=False)
6161
"""Backwards compatibility with the old CncNetClient."""
6262

63+
ip_address = models.CharField(max_length=50, null=True, blank=True, db_index=True)
64+
"""IP address that uploaded the file. 50 is long enough for ipv6"""
65+
6366
def validate_file_extension(self, file_extension: game_models.CncFileExtension) -> None:
6467
"""Validate that an extension is supported for a game.
6568

kirovy/request.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from functools import cached_property
2+
13
from rest_framework.request import Request as _DRFRequest
24
from kirovy import models, typing as t, objects
35

@@ -11,3 +13,13 @@ class KirovyRequest(_DRFRequest):
1113

1214
user: t.Optional[models.CncUser]
1315
auth: t.Optional[objects.CncnetUserInfo]
16+
17+
@cached_property
18+
def client_ip_address(self) -> str:
19+
if self.user.is_staff:
20+
return "staff"
21+
x_forwarded_for: str | None = self.META.get("HTTP_X_FORWARDED_FOR")
22+
if x_forwarded_for:
23+
return x_forwarded_for.split(",")[0].strip()
24+
25+
return self.META.get("REMOTE_ADDR", "unknown")

kirovy/serializers/__init__.py

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from functools import cached_property
2+
13
from rest_framework import serializers
24

35
from kirovy.constants import api_codes
@@ -18,25 +20,33 @@ class KirovySerializer(serializers.Serializer):
1820
source="last_modified_by",
1921
queryset=CncUser.objects.all(),
2022
pk_field=serializers.UUIDField(),
23+
allow_null=True,
2124
)
2225

2326
class Meta:
2427
exclude = ["last_modified_by"]
25-
fields = "__all__"
2628
editable_fields: t.ClassVar[set[str]] = set()
2729

28-
def get_fields(self):
30+
@cached_property
31+
def permissioned_readable_fields(self):
2932
"""Get fields based on permission level.
3033
3134
Removes admin-only fields for non-admin requests. Will always remove the fields if the serializer doesn't
3235
have context.
3336
"""
34-
fields = super().get_fields()
37+
fields = self.fields
3538
request: t.Optional[KirovyRequest] = self.context.get("request")
3639
if not (request and request.user.is_authenticated and request.user.is_staff):
3740
fields.pop("last_modified_by_id", None)
41+
fields.pop("ip_address", None)
3842
return fields
3943

44+
@property
45+
def _readable_fields(self):
46+
for field in self.permissioned_readable_fields.values():
47+
if not field.write_only:
48+
yield field
49+
4050
def to_internal_value(self, data: dict) -> dict:
4151
"""Convert the raw request data into data that can be used in a django model.
4252
@@ -72,4 +82,3 @@ class CncNetUserOwnedModelSerializer(KirovySerializer):
7282

7383
class Meta:
7484
exclude = ["cnc_user"]
75-
fields = "__all__"

0 commit comments

Comments
 (0)