Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Limit shopping list owners to current group #3305

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions frontend/lib/api/types/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,10 @@ export interface UserIn {
canOrganize?: boolean;
password: string;
}
export interface UserSummary {
id: string;
fullName?: string;
}
export interface ValidateResetToken {
token: string;
}
9 changes: 8 additions & 1 deletion frontend/lib/api/user/users.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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`,
Expand All @@ -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, {});
}
Expand Down
6 changes: 3 additions & 3 deletions frontend/pages/shopping-lists/_id.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions frontend/types/components.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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" {
Expand All @@ -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;
Expand All @@ -71,7 +72,6 @@ declare module "vue" {
SafeMarkdown: typeof SafeMarkdown;
StatsCards: typeof StatsCards;
ToggleState: typeof ToggleState;
BaseWizard: typeof BaseWizard;
// Layout Components
DefaultLayout: typeof DefaultLayout;
}
Expand Down
16 changes: 15 additions & 1 deletion mealie/routes/users/crud.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
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"])
Expand All @@ -25,6 +25,8 @@ def mixins(self) -> HttpRepo:

@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,
Expand Down Expand Up @@ -56,6 +58,18 @@ def delete_user(self, item_id: UUID4):

@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
Expand Down
7 changes: 6 additions & 1 deletion mealie/schema/user/__init__.py
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -18,6 +18,7 @@
UserIn,
UserOut,
UserPagination,
UserSummary,
)
from .user_passwords import (
ForgotPassword,
Expand All @@ -30,6 +31,9 @@

__all__ = [
"CreateUserRegistration",
"CredentialsRequest",
"CredentialsRequestForm",
"OIDCRequest",
"Token",
"TokenData",
"UnlockResults",
Expand All @@ -55,4 +59,5 @@
"UserIn",
"UserOut",
"UserPagination",
"UserSummary",
]
10 changes: 10 additions & 0 deletions mealie/schema/user/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,20 @@ def convert_favorite_recipes_to_slugs(cls, v: Any):
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)
Expand Down
98 changes: 98 additions & 0 deletions tests/integration_tests/user_tests/test_user_crud.py
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions tests/utils/api_routes/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@
"""`/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"
Expand Down