From 5f522b53246ddb034515802e1c716873f92fb0f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20D=C5=BEoi=C4=87?= <37804613+mariodz95@users.noreply.github.com> Date: Wed, 30 Jul 2025 00:46:23 +0200 Subject: [PATCH] fix: allow admin users to delete other household recipes (#5767) Co-authored-by: Michael Genson <71845777+michael-genson@users.noreply.github.com> --- .../Domain/Recipe/RecipeContextMenu.vue | 43 +++++++++++++----- frontend/lang/messages/en-US.json | 1 + mealie/services/recipe/recipe_service.py | 8 +++- tests/fixtures/fixture_users.py | 7 +++ .../test_recipe_cross_household.py | 36 +++++++++++++++ .../user_recipe_tests/test_recipe_owner.py | 45 +++++++++++++++++++ 6 files changed, 128 insertions(+), 12 deletions(-) diff --git a/frontend/components/Domain/Recipe/RecipeContextMenu.vue b/frontend/components/Domain/Recipe/RecipeContextMenu.vue index 6d86c3658..38f6f56c3 100644 --- a/frontend/components/Domain/Recipe/RecipeContextMenu.vue +++ b/frontend/components/Domain/Recipe/RecipeContextMenu.vue @@ -12,7 +12,12 @@ @confirm="deleteRecipe()" > - {{ $t("recipe.delete-confirmation") }} + + recipeRef.value ? { scale: props.recipeScale, ...recipeRef.value } : undefined, ); + const isAdminAndNotOwner = computed(() => { + return ( + $auth.user.value?.admin + && $auth.user.value?.id !== recipeRef.value?.userId + ); + }); + const canDelete = computed(() => { + const user = $auth.user.value; + const recipe = recipeRef.value; + return user && recipe && (user.admin || user.id === recipe.userId); + }); + + // Get Default Menu Items Specified in Props + for (const [key, value] of Object.entries(props.useItems)) { + if (!value) continue; + + // Skip delete if not allowed + if (key === "delete" && !canDelete.value) continue; + + const item = defaultItems[key]; + if (item && (item.isPublic || isOwnGroup.value)) { + state.menuItems.push(item); + } + } async function getShoppingLists() { const { data } = await api.shopping.lists.getAll(1, -1, { orderBy: "name", orderDirection: "asc" }); @@ -521,6 +540,8 @@ export default defineNuxtComponent({ icon, planTypeOptions, firstDayOfWeek, + isAdminAndNotOwner, + canDelete, }; }, }); diff --git a/frontend/lang/messages/en-US.json b/frontend/lang/messages/en-US.json index f3e3c2485..3a174488a 100644 --- a/frontend/lang/messages/en-US.json +++ b/frontend/lang/messages/en-US.json @@ -472,6 +472,7 @@ "comment": "Comment", "comments": "Comments", "delete-confirmation": "Are you sure you want to delete this recipe?", + "admin-delete-confirmation": "You're about to delete a recipe that isn't yours using admin permissions. Are you sure?", "delete-recipe": "Delete Recipe", "description": "Description", "disable-amount": "Disable Ingredient Amounts", diff --git a/mealie/services/recipe/recipe_service.py b/mealie/services/recipe/recipe_service.py index 0040ff88a..af827feff 100644 --- a/mealie/services/recipe/recipe_service.py +++ b/mealie/services/recipe/recipe_service.py @@ -64,6 +64,12 @@ class RecipeService(RecipeServiceBase): raise exceptions.NoEntryFound("Recipe not found.") return recipe + def can_delete(self, recipe: Recipe) -> bool: + if self.user.admin: + return True + else: + return self.can_update(recipe) + def can_update(self, recipe: Recipe) -> bool: if recipe.settings is None: raise exceptions.UnexpectedNone("Recipe Settings is None") @@ -423,7 +429,7 @@ class RecipeService(RecipeServiceBase): def delete_one(self, slug_or_id: str | UUID) -> Recipe: recipe = self.get_one(slug_or_id) - if not self.can_update(recipe): + if not self.can_delete(recipe): raise exceptions.PermissionDenied("You do not have permission to delete this recipe.") data = self.group_recipes.delete(recipe.id, "id") diff --git a/tests/fixtures/fixture_users.py b/tests/fixtures/fixture_users.py index 0c621acb8..1eccf79e0 100644 --- a/tests/fixtures/fixture_users.py +++ b/tests/fixtures/fixture_users.py @@ -226,6 +226,13 @@ def unique_user(session: Session, api_client: TestClient): yield from _unique_user(session, api_client) +@fixture(scope="module") +def unique_admin(session: Session, api_client: TestClient, unique_user: utils.TestUser): + admin_user = next(_unique_user(session, api_client)) + admin_user.repos.users.patch(admin_user.user_id, {"admin": True, "group_id": unique_user.group_id}) + yield admin_user + + @fixture(scope="module") def user_tuple(session: Session, admin_token, api_client: TestClient) -> Generator[list[utils.TestUser], None, None]: group_name = utils.random_string() diff --git a/tests/integration_tests/user_recipe_tests/test_recipe_cross_household.py b/tests/integration_tests/user_recipe_tests/test_recipe_cross_household.py index cd587d795..4334c0e55 100644 --- a/tests/integration_tests/user_recipe_tests/test_recipe_cross_household.py +++ b/tests/integration_tests/user_recipe_tests/test_recipe_cross_household.py @@ -215,6 +215,42 @@ def test_delete_recipes_from_other_households( assert response.status_code == 404 +@pytest.mark.parametrize("is_private_household", [True, False]) +@pytest.mark.parametrize("household_lock_recipe_edits", [True, False]) +def test_admin_delete_recipes_from_other_households( + api_client: TestClient, + unique_admin: TestUser, + h2_user: TestUser, + is_private_household: bool, + household_lock_recipe_edits: bool, +): + household = h2_user.repos.households.get_one(h2_user.household_id) + assert household and household.preferences + household.preferences.private_household = is_private_household + household.preferences.lock_recipe_edits_from_other_households = household_lock_recipe_edits + h2_user.repos.household_preferences.update(household.id, household.preferences) + + response = api_client.post(api_routes.recipes, json={"name": random_string()}, headers=h2_user.token) + assert response.status_code == 201 + h2_recipe = h2_user.repos.recipes.get_one(response.json()) + assert h2_recipe and h2_recipe.id + h2_recipe_id = str(h2_recipe.id) + + response = api_client.get(api_routes.recipes_slug(h2_recipe_id), headers=unique_admin.token) + assert response.status_code == 200 + recipe_json = response.json() + assert recipe_json["id"] == h2_recipe_id + + # Admin users should always be able to delete recipes from other households + # regardless of household_lock_recipe_edits setting + response = api_client.delete(api_routes.recipes_slug(recipe_json["slug"]), headers=unique_admin.token) + assert response.status_code == 200 + + # confirm the recipe was deleted + response = api_client.get(api_routes.recipes_slug(h2_recipe_id), headers=unique_admin.token) + assert response.status_code == 404 + + @pytest.mark.parametrize("is_private_household", [True, False]) @pytest.mark.parametrize("household_lock_recipe_edits", [True, False]) def test_user_can_update_last_made_on_other_household( diff --git a/tests/integration_tests/user_recipe_tests/test_recipe_owner.py b/tests/integration_tests/user_recipe_tests/test_recipe_owner.py index 3b6372281..f1b1c1ec9 100644 --- a/tests/integration_tests/user_recipe_tests/test_recipe_owner.py +++ b/tests/integration_tests/user_recipe_tests/test_recipe_owner.py @@ -1,7 +1,11 @@ from datetime import UTC, datetime +from uuid import UUID from fastapi.testclient import TestClient +from mealie.repos.repository_factory import AllRepositories +from mealie.schema.recipe.recipe import Recipe +from mealie.schema.recipe.recipe_settings import RecipeSettings from tests.utils import api_routes from tests.utils.factories import random_string from tests.utils.fixture_schemas import TestUser @@ -135,3 +139,44 @@ def test_other_user_cant_lock_recipe(api_client: TestClient, user_tuple: list[Te recipe["settings"]["locked"] = True response = api_client.put(api_routes.recipes + f"/{recipe_name}", json=recipe, headers=usr_2.token) assert response.status_code == 403 + + +def test_other_user_cant_delete_recipe(api_client: TestClient, user_tuple: list[TestUser]): + slug = random_string(10) + unique_user, other_user = user_tuple + + unique_user.repos.recipes.create( + Recipe( + user_id=unique_user.user_id, + group_id=unique_user.group_id, + name=slug, + settings=RecipeSettings(locked=True), + ) + ) + + response = api_client.delete(api_routes.recipes_slug(slug), headers=other_user.token) + assert response.status_code == 403 + + +def test_admin_can_delete_locked_recipe_owned_by_another_user( + api_client: TestClient, unfiltered_database: AllRepositories, unique_user: TestUser, admin_user: TestUser +): + slug = random_string(10) + unique_user.repos.recipes.create( + Recipe( + user_id=unique_user.user_id, + group_id=unique_user.group_id, + name=slug, + settings=RecipeSettings(locked=True), + ) + ) + + # Make sure admin belongs to same group/household as user + admin_data = unfiltered_database.users.get_one(admin_user.user_id) + assert admin_data + admin_data.group_id = UUID(unique_user.group_id) + admin_data.household_id = UUID(unique_user.household_id) + unfiltered_database.users.update(admin_user.user_id, admin_data) + + response = api_client.delete(api_routes.recipes_slug(slug), headers=admin_user.token) + assert response.status_code == 200