Skip to content

Commit

Permalink
Require manage orders for fetching user.orders (#9128)
Browse files Browse the repository at this point in the history
* Require manage orders for fetching customer.orders

* Update changelog

Co-authored-by: Marcin Gębala <5421321+maarcingebala@users.noreply.github.com>
  • Loading branch information
IKarbowiak and maarcingebala committed Mar 1, 2022
1 parent 035bf70 commit 521dfd6
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 11 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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
Expand Down
9 changes: 7 additions & 2 deletions saleor/graphql/account/tests/benchmark/test_account.py
Expand Up @@ -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"]
Expand Down Expand Up @@ -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

Expand Down
93 changes: 87 additions & 6 deletions saleor/graphql/account/tests/test_account.py
Expand Up @@ -181,6 +181,7 @@ def test_query_customer_user(
gift_card_expiry_date,
address,
permission_manage_users,
permission_manage_orders,
media_root,
settings,
):
Expand All @@ -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"]
Expand Down Expand Up @@ -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
Expand All @@ -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,
):
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 = """
Expand All @@ -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(
Expand Down
10 changes: 7 additions & 3 deletions saleor/graphql/account/types.py
Expand Up @@ -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
Expand Down

1 comment on commit 521dfd6

@JamieSlome
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maarcingebala - hey 👋

The researcher that disclosed the report has requested a CVE for this.

Are you happy for us to assign and publish one for the report?

Please sign in to comment.