From 521dfd6394f3926a77c60d8633c058e16d0f916d Mon Sep 17 00:00:00 2001 From: Iga Karbowiak <40886528+IKarbowiak@users.noreply.github.com> Date: Tue, 1 Mar 2022 09:33:10 +0100 Subject: [PATCH] Require manage orders for fetching `user.orders` (#9128) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Require manage orders for fetching customer.orders * Update changelog Co-authored-by: Marcin Gębala <5421321+maarcingebala@users.noreply.github.com> --- CHANGELOG.md | 3 + .../account/tests/benchmark/test_account.py | 9 +- saleor/graphql/account/tests/test_account.py | 93 +++++++++++++++++-- saleor/graphql/account/types.py | 10 +- 4 files changed, 104 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c378fde9793..77360bc1274 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ All notable, unreleased changes to this project will be documented in this file. # Unreleased ### Breaking changes +- Require manage orders for fetching `user.orders` - #9128 by @IKarbowiak + - only staff with `manage orders` and can fetch customer orders + - the customer can fetch his own orders, except drafts ### Other changes - Filter Customer/Order/Sale/Product/ProductVariant by datetime of last modification - #9137 by @rafalp diff --git a/saleor/graphql/account/tests/benchmark/test_account.py b/saleor/graphql/account/tests/benchmark/test_account.py index 34d5b71da92..187616a7253 100644 --- a/saleor/graphql/account/tests/benchmark/test_account.py +++ b/saleor/graphql/account/tests/benchmark/test_account.py @@ -105,7 +105,9 @@ def test_query_staff_user( user_id = graphene.Node.to_global_id("User", staff_user.pk) variables = {"id": user_id} response = staff_api_client.post_graphql( - query, variables, permissions=[permission_manage_staff] + query, + variables, + permissions=[permission_manage_staff, permission_manage_orders], ) content = get_graphql_content(response) data = content["data"]["user"] @@ -352,10 +354,13 @@ def test_delete_staff_members( def test_customers_query( staff_api_client, permission_manage_users, + permission_manage_orders, users_for_customers_benchmarks, count_queries, ): - staff_api_client.user.user_permissions.set([permission_manage_users]) + staff_api_client.user.user_permissions.set( + [permission_manage_users, permission_manage_orders] + ) content = get_graphql_content(staff_api_client.post_graphql(CUSTOMERS_QUERY)) assert content["data"]["customers"] is not None diff --git a/saleor/graphql/account/tests/test_account.py b/saleor/graphql/account/tests/test_account.py index f816df34ce7..bd1cfd45b27 100644 --- a/saleor/graphql/account/tests/test_account.py +++ b/saleor/graphql/account/tests/test_account.py @@ -181,6 +181,7 @@ def test_query_customer_user( gift_card_expiry_date, address, permission_manage_users, + permission_manage_orders, media_root, settings, ): @@ -199,7 +200,9 @@ def test_query_customer_user( query = FULL_USER_QUERY ID = graphene.Node.to_global_id("User", customer_user.id) variables = {"id": ID} - staff_api_client.user.user_permissions.add(permission_manage_users) + staff_api_client.user.user_permissions.add( + permission_manage_users, permission_manage_orders + ) response = staff_api_client.post_graphql(query, variables) content = get_graphql_content(response) data = content["data"]["user"] @@ -268,6 +271,7 @@ def test_query_customer_user_with_orders( customer_user, order_list, permission_manage_users, + permission_manage_orders, ): # given query = FULL_USER_QUERY @@ -291,24 +295,61 @@ def test_query_customer_user_with_orders( # when response = staff_api_client.post_graphql( - query, variables, permissions=[permission_manage_users] + query, + variables, + permissions=[permission_manage_users, permission_manage_orders], ) # then content = get_graphql_content(response) user = content["data"]["user"] assert {order["node"]["id"] for order in user["orders"]["edges"]} == { - graphene.Node.to_global_id("Order", order.pk) - for order in [order_unfulfilled, order_unconfirmed] + graphene.Node.to_global_id("Order", order.pk) for order in order_list } +def test_query_customer_user_with_orders_no_manage_orders_perm( + staff_api_client, + customer_user, + order_list, + permission_manage_users, +): + # given + query = FULL_USER_QUERY + order_unfulfilled = order_list[0] + order_unfulfilled.user = customer_user + + order_unconfirmed = order_list[1] + order_unconfirmed.status = OrderStatus.UNCONFIRMED + order_unconfirmed.user = customer_user + + order_draft = order_list[2] + order_draft.status = OrderStatus.DRAFT + order_draft.user = customer_user + + Order.objects.bulk_update( + [order_unconfirmed, order_draft, order_unfulfilled], ["user", "status"] + ) + + id = graphene.Node.to_global_id("User", customer_user.id) + variables = {"id": id} + + # when + response = staff_api_client.post_graphql( + query, variables, permissions=[permission_manage_users] + ) + + # then + assert_no_permission(response) + + def test_query_customer_user_app( app_api_client, customer_user, address, permission_manage_users, permission_manage_staff, + permission_manage_orders, media_root, app, ): @@ -327,7 +368,9 @@ def test_query_customer_user_app( query = FULL_USER_QUERY ID = graphene.Node.to_global_id("User", customer_user.id) variables = {"id": ID} - app.permissions.add(permission_manage_staff, permission_manage_users) + app.permissions.add( + permission_manage_staff, permission_manage_users, permission_manage_orders + ) response = app_api_client.post_graphql(query, variables) content = get_graphql_content(response) @@ -365,6 +408,41 @@ def test_query_customer_user_app_no_permission( assert_no_permission(response) +def test_query_customer_user_with_orders_by_app_no_manage_orders_perm( + app_api_client, + customer_user, + order_list, + permission_manage_users, +): + # given + query = FULL_USER_QUERY + order_unfulfilled = order_list[0] + order_unfulfilled.user = customer_user + + order_unconfirmed = order_list[1] + order_unconfirmed.status = OrderStatus.UNCONFIRMED + order_unconfirmed.user = customer_user + + order_draft = order_list[2] + order_draft.status = OrderStatus.DRAFT + order_draft.user = customer_user + + Order.objects.bulk_update( + [order_unconfirmed, order_draft, order_unfulfilled], ["user", "status"] + ) + + id = graphene.Node.to_global_id("User", customer_user.id) + variables = {"id": id} + + # when + response = app_api_client.post_graphql( + query, variables, permissions=[permission_manage_users] + ) + + # then + assert_no_permission(response) + + def test_query_staff_user( staff_api_client, staff_user, @@ -993,6 +1071,7 @@ def test_user_with_cancelled_fulfillments( staff_api_client, customer_user, permission_manage_users, + permission_manage_orders, fulfilled_order_with_cancelled_fulfillment, ): query = """ @@ -1013,7 +1092,9 @@ def test_user_with_cancelled_fulfillments( """ user_id = graphene.Node.to_global_id("User", customer_user.id) variables = {"id": user_id} - staff_api_client.user.user_permissions.add(permission_manage_users) + staff_api_client.user.user_permissions.add( + permission_manage_users, permission_manage_orders + ) response = staff_api_client.post_graphql(query, variables) content = get_graphql_content(response) order_id = graphene.Node.to_global_id( diff --git a/saleor/graphql/account/types.py b/saleor/graphql/account/types.py index 4fbe2d179bf..680d57e055c 100644 --- a/saleor/graphql/account/types.py +++ b/saleor/graphql/account/types.py @@ -364,9 +364,13 @@ def resolve_orders(root: models.User, info, **kwargs): def _resolve_orders(orders): requester = get_user_or_app_from_context(info.context) if not requester.has_perm(OrderPermissions.MANAGE_ORDERS): - orders = list( - filter(lambda order: order.status != OrderStatus.DRAFT, orders) - ) + # allow fetch requestor orders (except drafts) + if root == info.context.user: + orders = list( + filter(lambda order: order.status != OrderStatus.DRAFT, orders) + ) + else: + raise PermissionDenied() return create_connection_slice( orders, info, kwargs, OrderCountableConnection