Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
22 changes: 22 additions & 0 deletions udata/api_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -995,6 +995,19 @@ def patch(obj: _T, request) -> _T:
document_type = db.resolve_model(value["class"])
except ValueError as e:
raise FieldValidationError(message=str(e), field=key)
# `resolve_model` resolves against the whole document registry, so
# without this the client picks which collection the lookup below
# queries — MongoEngine only enforces `choices` at save() time, long
# after that query ran. A field without `choices` accepts them all,
# by design (e.g. `Transfer.subject`).
if (
model_attribute.choices
and document_type._class_name not in model_attribute.choices
):
raise FieldValidationError(
message=f"Value must be one of {model_attribute.choices}",
field=key,
)
value = wrap_primary_key(
key,
model_attribute,
Expand Down Expand Up @@ -1134,6 +1147,15 @@ def wrap_primary_key(
if isinstance(value, dict) and "id" in value:
return wrap_primary_key(field_name, foreign_field, value["id"], document_type)

# `value` comes straight from the request body and goes straight into the query
# below, so anything but a scalar is a set of Mongo operators (`{"$ne": …}`) that
# selects an arbitrary document instead of the requested one. MongoEngine catches
# them only when the primary key is an `ObjectId`, whose `prepare_query_value`
# refuses the dict; on a `StringField` primary key (`License`, `GeoZone`…) the
# operators reach the database untouched.
if isinstance(value, (dict, list)):
raise FieldValidationError(field=field_name, message="Expected a reference id")

document_type = document_type or foreign_field.document_type().__class__
id_field_name = document_type._meta["id_field"]

Expand Down
35 changes: 34 additions & 1 deletion udata/tests/api/test_reports_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@

from udata.core.dataservices.factories import DataserviceFactory
from udata.core.dataset.factories import DatasetFactory
from udata.core.dataset.models import Dataset
from udata.core.dataset.models import Dataset, License
from udata.core.discussions.factories import DiscussionFactory, MessageDiscussionFactory
from udata.core.discussions.models import Discussion, Message
from udata.core.reports.constants import (
REASON_AUTO_SPAM,
REASON_ILLEGAL_CONTENT,
REASON_OTHERS,
REASON_SPAM,
reports_reasons_translations,
)
Expand Down Expand Up @@ -593,3 +594,35 @@ def test_reports_api_create_without_subject(self):
)
self.assert400(response)
self.assertEqual(Report.objects.count(), 0)


class ReportsSubjectClassAPITest(APITestCase):
def test_reports_api_create_with_a_non_reportable_subject_class(self):
"""`class` is resolved against the whole document registry, so a class outside
`REPORTABLE_MODELS` must be rejected before its collection is queried."""
License(id="fr-lo", title="Licence Ouverte").save()

response = self.post(
url_for("api.reports"),
{
"reason": REASON_OTHERS,
"subject": {"class": "License", "id": "fr-lo"},
},
)
self.assert400(response)
self.assertEqual(Report.objects.count(), 0)

def test_reports_api_create_with_mongo_operators_as_subject_id(self):
"""`License.id` being a `StringField`, the operators used to reach the database
and crash the query instead of being rejected as an invalid reference."""
License(id="fr-lo", title="Licence Ouverte").save()

response = self.post(
url_for("api.reports"),
{
"reason": REASON_OTHERS,
"subject": {"class": "License", "id": {"$where": "return true"}},
},
)
self.assert400(response)
self.assertEqual(Report.objects.count(), 0)
11 changes: 11 additions & 0 deletions udata/tests/test_api_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from udata.api import api
from udata.api_fields import field, generate_fields, patch, patch_and_save
from udata.core.dataset.api_fields import dataset_fields
from udata.core.dataset.models import License
from udata.core.organization import constants as org_constants
from udata.core.organization.factories import OrganizationFactory
from udata.core.organization.models import Organization
Expand Down Expand Up @@ -599,6 +600,16 @@ def test_write_field_takes_a_class_and_id_reference(self) -> None:
)
assert obj.subject == organization

@pytest.mark.parametrize("operators", [{"$where": "return true"}, {"$ne": "nope"}, {"$gt": ""}])
def test_write_field_rejects_mongo_operators_as_id(self, operators: dict) -> None:
"""An id made of Mongo operators would select an arbitrary document instead of
the requested one. MongoEngine catches it on an `ObjectId` primary key, but
`License.id` is a `StringField`, so the operators would reach the database."""
License(id="fr-lo", title="Licence Ouverte").save()

with pytest.raises(FieldValidationError):
patch(FakeWithGenericReference(), {"subject": {"class": "License", "id": operators}})


class RenameFieldTest(PytestOnlyDBTestCase):
def test_read_fields_use_the_api_key(self) -> None:
Expand Down