From 823b938a2c1680e7fd760a9c174a909b1a50a0ba Mon Sep 17 00:00:00 2001 From: Hayden <64056131+hay-kot@users.noreply.github.com> Date: Sat, 23 May 2026 00:32:57 -0500 Subject: [PATCH] fix: enforce organize-group-data permission on food/tag/category mutations (#7651) --- .../organizers/controller_categories.py | 3 + mealie/routes/organizers/controller_tags.py | 3 + mealie/routes/unit_and_foods/foods.py | 4 + .../test_organize_permissions.py | 115 ++++++++++++++++++ 4 files changed, 125 insertions(+) create mode 100644 tests/integration_tests/user_household_tests/test_organize_permissions.py diff --git a/mealie/routes/organizers/controller_categories.py b/mealie/routes/organizers/controller_categories.py index 61338f448..5d686132e 100644 --- a/mealie/routes/organizers/controller_categories.py +++ b/mealie/routes/organizers/controller_categories.py @@ -51,6 +51,7 @@ class RecipeCategoryController(BaseCrudController): @router.post("", status_code=201) def create_one(self, category: CategoryIn): """Creates a Category in the database""" + self.checks.can_organize() save_data = mapper.cast(category, CategorySave, group_id=self.group_id) new_category = self.mixins.create_one(save_data) if new_category: @@ -83,6 +84,7 @@ class RecipeCategoryController(BaseCrudController): @router.put("/{item_id}", response_model=CategorySummary) def update_one(self, item_id: UUID4, update_data: CategoryIn): """Updates an existing Tag in the database""" + self.checks.can_organize() save_data = mapper.cast(update_data, CategorySave, group_id=self.group_id) category = self.mixins.update_one(save_data, item_id) @@ -108,6 +110,7 @@ class RecipeCategoryController(BaseCrudController): category does not impact a recipe. The category will be removed from any recipes that contain it """ + self.checks.can_organize() if category := self.mixins.delete_one(item_id): self.publish_event( event_type=EventTypes.category_deleted, diff --git a/mealie/routes/organizers/controller_tags.py b/mealie/routes/organizers/controller_tags.py index 09ab3e486..ef4c167a8 100644 --- a/mealie/routes/organizers/controller_tags.py +++ b/mealie/routes/organizers/controller_tags.py @@ -51,6 +51,7 @@ class TagController(BaseCrudController): @router.post("", status_code=201) def create_one(self, tag: TagIn): """Creates a Tag in the database""" + self.checks.can_organize() save_data = mapper.cast(tag, TagSave, group_id=self.group_id) new_tag = self.mixins.create_one(save_data) @@ -72,6 +73,7 @@ class TagController(BaseCrudController): @router.put("/{item_id}", response_model=RecipeTagResponse) def update_one(self, item_id: UUID4, new_tag: TagIn): """Updates an existing Tag in the database""" + self.checks.can_organize() save_data = mapper.cast(new_tag, TagSave, group_id=self.group_id) tag = self.repo.update(item_id, save_data) @@ -97,6 +99,7 @@ class TagController(BaseCrudController): tag does not impact a recipe. The tag will be removed from any recipes that contain it """ + self.checks.can_organize() try: tag = self.repo.delete(item_id) diff --git a/mealie/routes/unit_and_foods/foods.py b/mealie/routes/unit_and_foods/foods.py index 58b2c5d79..ffbcc4f26 100644 --- a/mealie/routes/unit_and_foods/foods.py +++ b/mealie/routes/unit_and_foods/foods.py @@ -48,11 +48,13 @@ class IngredientFoodsController(BaseUserController): @router.post("", response_model=IngredientFood, status_code=201) def create_one(self, data: CreateIngredientFood): + self.checks.can_organize() save_data = mapper.cast(data, SaveIngredientFood, group_id=self.group_id) return self.mixins.create_one(save_data) @router.put("/merge", response_model=SuccessResponse) def merge_one(self, data: MergeFood): + self.checks.can_organize() try: self.repo.merge(data.from_food, data.to_food) return SuccessResponse.respond("Successfully merged foods") @@ -66,9 +68,11 @@ class IngredientFoodsController(BaseUserController): @router.put("/{item_id}", response_model=IngredientFood) def update_one(self, item_id: UUID4, data: CreateIngredientFood): + self.checks.can_organize() data = mapper.cast(data, SaveIngredientFood, group_id=self.group_id) return self.mixins.update_one(data, item_id) @router.delete("/{item_id}", response_model=IngredientFood) def delete_one(self, item_id: UUID4): + self.checks.can_organize() return self.mixins.delete_one(item_id) diff --git a/tests/integration_tests/user_household_tests/test_organize_permissions.py b/tests/integration_tests/user_household_tests/test_organize_permissions.py new file mode 100644 index 000000000..61bd0d2a3 --- /dev/null +++ b/tests/integration_tests/user_household_tests/test_organize_permissions.py @@ -0,0 +1,115 @@ +"""Tests that the "User can organize group data" permission gates the +mutation endpoints for foods, tags, and categories (GHSA / issue #6883). + +Read endpoints remain open; only POST/PUT/DELETE require the permission. +""" + +import pytest +from fastapi.testclient import TestClient + +from tests.utils import api_routes +from tests.utils.factories import random_string +from tests.utils.fixture_schemas import TestUser + +# (id, collection route, item-id route builder) +RESOURCES = [ + ("foods", api_routes.foods, api_routes.foods_item_id), + ("tags", api_routes.organizers_tags, api_routes.organizers_tags_item_id), + ("categories", api_routes.organizers_categories, api_routes.organizers_categories_item_id), +] +RESOURCE_IDS = [resource[0] for resource in RESOURCES] + + +def set_can_organize(user: TestUser, value: bool) -> None: + db_user = user.repos.users.get_one(user.user_id) + assert db_user + db_user.can_organize = value + user.repos.users.update(db_user.id, db_user) + + +def create_item(api_client: TestClient, user: TestUser, collection: str) -> str: + """Creates an item as a user with the organize permission and returns its id.""" + set_can_organize(user, True) + response = api_client.post(collection, json={"name": random_string(10)}, headers=user.token) + assert response.status_code == 201 + return response.json()["id"] + + +@pytest.mark.parametrize("name, collection, item", RESOURCES, ids=RESOURCE_IDS) +def test_create_requires_organize_permission( + api_client: TestClient, unique_user_fn_scoped: TestUser, name: str, collection: str, item +): + user = unique_user_fn_scoped + payload = {"name": random_string(10)} + + set_can_organize(user, False) + response = api_client.post(collection, json=payload, headers=user.token) + assert response.status_code == 403 + + set_can_organize(user, True) + response = api_client.post(collection, json=payload, headers=user.token) + assert response.status_code == 201 + + +@pytest.mark.parametrize("name, collection, item", RESOURCES, ids=RESOURCE_IDS) +def test_update_requires_organize_permission( + api_client: TestClient, unique_user_fn_scoped: TestUser, name: str, collection: str, item +): + user = unique_user_fn_scoped + item_id = create_item(api_client, user, collection) + payload = {"id": item_id, "name": random_string(10)} + + set_can_organize(user, False) + response = api_client.put(item(item_id), json=payload, headers=user.token) + assert response.status_code == 403 + + set_can_organize(user, True) + response = api_client.put(item(item_id), json=payload, headers=user.token) + assert response.status_code == 200 + + +@pytest.mark.parametrize("name, collection, item", RESOURCES, ids=RESOURCE_IDS) +def test_delete_requires_organize_permission( + api_client: TestClient, unique_user_fn_scoped: TestUser, name: str, collection: str, item +): + user = unique_user_fn_scoped + item_id = create_item(api_client, user, collection) + + set_can_organize(user, False) + response = api_client.delete(item(item_id), headers=user.token) + assert response.status_code == 403 + + set_can_organize(user, True) + response = api_client.delete(item(item_id), headers=user.token) + assert response.status_code == 200 + + +def test_food_merge_requires_organize_permission(api_client: TestClient, unique_user_fn_scoped: TestUser): + user = unique_user_fn_scoped + from_food = create_item(api_client, user, api_routes.foods) + to_food = create_item(api_client, user, api_routes.foods) + payload = {"fromFood": from_food, "toFood": to_food} + + set_can_organize(user, False) + response = api_client.put(api_routes.foods_merge, json=payload, headers=user.token) + assert response.status_code == 403 + + set_can_organize(user, True) + response = api_client.put(api_routes.foods_merge, json=payload, headers=user.token) + assert response.status_code == 200 + + +@pytest.mark.parametrize("name, collection, item", RESOURCES, ids=RESOURCE_IDS) +def test_read_endpoints_do_not_require_organize_permission( + api_client: TestClient, unique_user_fn_scoped: TestUser, name: str, collection: str, item +): + user = unique_user_fn_scoped + item_id = create_item(api_client, user, collection) + + set_can_organize(user, False) + + response = api_client.get(collection, headers=user.token) + assert response.status_code == 200 + + response = api_client.get(item(item_id), headers=user.token) + assert response.status_code == 200