fix: Limit shopping list owners to current group (#3305)

* add route for getting group-only users

* add new api route to frontend

* update shopping list user getAll call

* tests

* fixed bad import

* replace UserOut with UserSummary

* fix params

frontend/lib/api/types/user.ts

@@ -238,6 +238,10 @@ export interface UserIn {
   canOrganize?: boolean;
   password: string;
 }
+export interface UserSummary {
+  id: string;
+  fullName?: string;
+}
 export interface ValidateResetToken {
   token: string;
 }

frontend/lib/api/user/users.ts

@@ -1,5 +1,6 @@
 import { BaseCRUDAPI } from "../base/base-clients";
-import { RequestResponse } from "../types/non-generated";
+import { QueryValue, route } from "~/lib/api/base/route";
+import { PaginationData, RequestResponse } from "~/lib/api/types/non-generated";
 import {
   ChangePassword,
   DeleteTokenResponse,
@@ -11,11 +12,13 @@ import {
   UserFavorites,
   UserIn,
   UserOut,
+  UserSummary,
 } from "~/lib/api/types/user";
 
 const prefix = "/api";
 
 const routes = {
+  groupUsers: `${prefix}/users/group-users`,
   usersSelf: `${prefix}/users/self`,
   groupsSelf: `${prefix}/users/self/group`,
   passwordReset: `${prefix}/users/reset-password`,
@@ -36,6 +39,10 @@ export class UserApi extends BaseCRUDAPI<UserIn, UserOut, UserBase> {
   baseRoute: string = routes.users;
   itemRoute = (itemid: string) => routes.usersId(itemid);
 
+  async getGroupUsers(page = 1, perPage = -1, params = {} as Record<string, QueryValue>) {
+    return await this.requests.get<PaginationData<UserSummary>>(route(routes.groupUsers, { page, perPage, ...params }));
+  }
+
   async getSelfGroup(): Promise<RequestResponse<GroupInDB>> {
     return await this.requests.get(routes.groupsSelf, {});
   }

frontend/pages/shopping-lists/_id.vue

@@ -245,7 +245,7 @@ import { useUserApi } from "~/composables/api";
 import MultiPurposeLabelSection from "~/components/Domain/ShoppingList/MultiPurposeLabelSection.vue"
 import ShoppingListItem from "~/components/Domain/ShoppingList/ShoppingListItem.vue";
 import { ShoppingListItemCreate, ShoppingListItemOut, ShoppingListMultiPurposeLabelOut, ShoppingListOut } from "~/lib/api/types/group";
-import { UserOut } from "~/lib/api/types/user";
+import { UserSummary } from "~/lib/api/types/user";
 import RecipeList from "~/components/Domain/Recipe/RecipeList.vue";
 import ShoppingListItemEditor from "~/components/Domain/ShoppingList/ShoppingListItemEditor.vue";
 import { useFoodStore, useLabelStore, useUnitStore } from "~/composables/store";
@@ -817,10 +817,10 @@ export default defineComponent({
     // ===============================================================
     // Shopping List Settings
 
-    const allUsers = ref<UserOut[]>([]);
+    const allUsers = ref<UserSummary[]>([]);
     const currentUserId = ref<string | undefined>();
     async function fetchAllUsers() {
-      const { data } = await userApi.users.getAll(1, -1, { orderBy: "full_name", orderDirection: "asc" });
+      const { data } = await userApi.users.getGroupUsers(1, -1, { orderBy: "full_name", orderDirection: "asc" });
       if (!data) {
         return;
       }

frontend/types/components.d.ts

@@ -14,6 +14,7 @@ import BaseDivider from "@/components/global/BaseDivider.vue";
 import BaseOverflowButton from "@/components/global/BaseOverflowButton.vue";
 import BasePageTitle from "@/components/global/BasePageTitle.vue";
 import BaseStatCard from "@/components/global/BaseStatCard.vue";
+import BaseWizard from "@/components/global/BaseWizard.vue";
 import ButtonLink from "@/components/global/ButtonLink.vue";
 import ContextMenu from "@/components/global/ContextMenu.vue";
 import CrudTable from "@/components/global/CrudTable.vue";
@@ -32,7 +33,6 @@ import ReportTable from "@/components/global/ReportTable.vue";
 import SafeMarkdown from "@/components/global/SafeMarkdown.vue";
 import StatsCards from "@/components/global/StatsCards.vue";
 import ToggleState from "@/components/global/ToggleState.vue";
-import BaseWizard from "@/components/global/BaseWizard.vue";
 import DefaultLayout from "@/components/layout/DefaultLayout.vue";
 
 declare module "vue" {
@@ -53,6 +53,7 @@ declare module "vue" {
     BaseOverflowButton: typeof BaseOverflowButton;
     BasePageTitle: typeof BasePageTitle;
     BaseStatCard: typeof BaseStatCard;
+    BaseWizard: typeof BaseWizard;
     ButtonLink: typeof ButtonLink;
     ContextMenu: typeof ContextMenu;
     CrudTable: typeof CrudTable;
@@ -71,7 +72,6 @@ declare module "vue" {
     SafeMarkdown: typeof SafeMarkdown;
     StatsCards: typeof StatsCards;
     ToggleState: typeof ToggleState;
-    BaseWizard: typeof BaseWizard;
     // Layout Components
     DefaultLayout: typeof DefaultLayout;
   }

mealie/routes/users/crud.py

@@ -11,7 +11,7 @@ from mealie.routes.users._helpers import assert_user_change_allowed
 from mealie.schema.response import ErrorResponse, SuccessResponse
 from mealie.schema.response.pagination import PaginationQuery
 from mealie.schema.user import ChangePassword, UserBase, UserIn, UserOut
-from mealie.schema.user.user import GroupInDB, UserPagination
+from mealie.schema.user.user import GroupInDB, UserPagination, UserSummary, UserSummaryPagination
 
 user_router = UserAPIRouter(prefix="/users", tags=["Users: CRUD"])
 admin_router = AdminAPIRouter(prefix="/users", tags=["Users: Admin CRUD"])
@@ -25,6 +25,8 @@ class AdminUserController(BaseAdminController):
 
     @admin_router.get("", response_model=UserPagination)
     def get_all(self, q: PaginationQuery = Depends(PaginationQuery)):
+        """Returns all users from all groups"""
+
         response = self.repos.users.page_all(
             pagination=q,
             override=UserOut,
@@ -56,6 +58,18 @@ class AdminUserController(BaseAdminController):
 
 @controller(user_router)
 class UserController(BaseUserController):
+    @user_router.get("/group-users", response_model=UserSummaryPagination)
+    def get_all_group_users(self, q: PaginationQuery = Depends(PaginationQuery)):
+        """Returns all users from the current group"""
+
+        response = self.repos.users.by_group(self.group_id).page_all(
+            pagination=q,
+            override=UserSummary,
+        )
+
+        response.set_pagination_guides(user_router.url_path_for("get_all_group_users"), q.model_dump())
+        return response
+
     @user_router.get("/self", response_model=UserOut)
     def get_logged_in_user(self):
         return self.user

mealie/schema/user/__init__.py

@@ -1,5 +1,5 @@
 # This file is auto-generated by gen_schema_exports.py
-from .auth import Token, TokenData, UnlockResults
+from .auth import CredentialsRequest, CredentialsRequestForm, OIDCRequest, Token, TokenData, UnlockResults
 from .registration import CreateUserRegistration
 from .user import (
     ChangePassword,
@@ -18,6 +18,7 @@ from .user import (
     UserIn,
     UserOut,
     UserPagination,
+    UserSummary,
 )
 from .user_passwords import (
     ForgotPassword,
@@ -30,6 +31,9 @@ from .user_passwords import (
 
 __all__ = [
     "CreateUserRegistration",
+    "CredentialsRequest",
+    "CredentialsRequestForm",
+    "OIDCRequest",
     "Token",
     "TokenData",
     "UnlockResults",
@@ -55,4 +59,5 @@ __all__ = [
     "UserIn",
     "UserOut",
     "UserPagination",
+    "UserSummary",
 ]

mealie/schema/user/user.py

@@ -139,10 +139,20 @@ class UserOut(UserBase):
         return slugs
 
 
+class UserSummary(MealieModel):
+    id: UUID4
+    full_name: str
+    model_config = ConfigDict(from_attributes=True)
+
+
 class UserPagination(PaginationBase):
     items: list[UserOut]
 
 
+class UserSummaryPagination(PaginationBase):
+    items: list[UserSummary]
+
+
 class UserFavorites(UserBase):
     favorite_recipes: list[RecipeSummary] = []  # type: ignore
     model_config = ConfigDict(from_attributes=True)

tests/integration_tests/user_tests/test_user_crud.py

@@ -0,0 +1,98 @@
+import pytest
+from fastapi.testclient import TestClient
+
+from mealie.repos.repository_factory import AllRepositories
+from tests.utils import TestUser, api_routes
+from tests.utils.factories import random_email, random_int, random_string
+
+
+@pytest.mark.parametrize("use_admin_user", [True, False])
+def test_get_all_users_admin(
+    request: pytest.FixtureRequest, database: AllRepositories, api_client: TestClient, use_admin_user: bool
+):
+    user: TestUser
+    if use_admin_user:
+        user = request.getfixturevalue("admin_user")
+    else:
+        user = request.getfixturevalue("unique_user")
+
+    user_ids: set[str] = set()
+    for _ in range(random_int(2, 5)):
+        group = database.groups.create({"name": random_string()})
+        for _ in range(random_int(2, 5)):
+            new_user = database.users.create(
+                {
+                    "username": random_string(),
+                    "email": random_email(),
+                    "group": group.name,
+                    "full_name": random_string(),
+                    "password": random_string(),
+                    "admin": False,
+                }
+            )
+            user_ids.add(str(new_user.id))
+
+    response = api_client.get(api_routes.admin_users, params={"perPage": -1}, headers=user.token)
+    if not use_admin_user:
+        assert response.status_code == 403
+        return
+
+    assert response.status_code == 200
+
+    # assert all users from all groups are returned
+    response_user_ids = set(user["id"] for user in response.json()["items"])
+    for user_id in user_ids:
+        assert user_id in response_user_ids
+
+
+@pytest.mark.parametrize("use_admin_user", [True, False])
+def test_get_all_group_users(
+    request: pytest.FixtureRequest, database: AllRepositories, api_client: TestClient, use_admin_user: bool
+):
+    user: TestUser
+    if use_admin_user:
+        user = request.getfixturevalue("admin_user")
+    else:
+        user = request.getfixturevalue("unique_user")
+
+    other_group_user_ids: set[str] = set()
+    for _ in range(random_int(2, 5)):
+        group = database.groups.create({"name": random_string()})
+        for _ in range(random_int(2, 5)):
+            new_user = database.users.create(
+                {
+                    "username": random_string(),
+                    "email": random_email(),
+                    "group": group.name,
+                    "full_name": random_string(),
+                    "password": random_string(),
+                    "admin": False,
+                }
+            )
+            other_group_user_ids.add(str(new_user.id))
+
+    user_group = database.groups.get_by_slug_or_id(user.group_id)
+    assert user_group
+    same_group_user_ids: set[str] = set([str(user.user_id)])
+    for _ in range(random_int(2, 5)):
+        new_user = database.users.create(
+            {
+                "username": random_string(),
+                "email": random_email(),
+                "group": user_group.name,
+                "full_name": random_string(),
+                "password": random_string(),
+                "admin": False,
+            }
+        )
+        same_group_user_ids.add(str(new_user.id))
+
+    response = api_client.get(api_routes.users_group_users, params={"perPage": -1}, headers=user.token)
+    assert response.status_code == 200
+    response_user_ids = set(user["id"] for user in response.json()["items"])
+
+    # assert only users from the same group are returned
+    for user_id in other_group_user_ids:
+        assert user_id not in response_user_ids
+    for user_id in same_group_user_ids:
+        assert user_id in response_user_ids

tests/utils/api_routes/__init__.py

@@ -171,6 +171,8 @@ users_api_tokens = "/api/users/api-tokens"
 """`/api/users/api-tokens`"""
 users_forgot_password = "/api/users/forgot-password"
 """`/api/users/forgot-password`"""
+users_group_users = "/api/users/group-users"
+"""`/api/users/group-users`"""
 users_password = "/api/users/password"
 """`/api/users/password`"""
 users_register = "/api/users/register"