CVE-2022-0932: Missing Authorization (CWE-862)
value: Missing Authorization in GitHub repository saleor/saleor prior to 3.1.2.
CWE description: The product does not perform an authorization check when an actor attempts to access a resource or perform an action.
CVSS v3 base score: 6.5
Published: 2022-03-11T15:15Z
Repository: https://github.com/saleor/saleor
Language: Python
Files changed: CHANGELOG.md, saleor/graphql/account/tests/benchmark/test_account.py, saleor/graphql/account/tests/test_account.py, saleor/graphql/account/types.py
Commit date: 2022-03-01 09:33:10 +0100
Commit message: Require manage orders for fetching `user.orders` (#9128)

* Require manage orders for fetching customer.orders

* Update changelog

Co-authored-by: Marcin Gębala <5421321+maarcingebala@users.noreply.github.com>
Diff: diff --git a/CHANGELOG.md b/CHANGELOG.md
index c378fde97..77360bc12 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,10 +4,13 @@ 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
 - Add possibility for plugins to execute code before each mutation - #9193 by @NyanKiyoshi
 - Add support for hiding plugins - #9219 by @NyanKiyoshi
diff --git a/saleor/graphql/account/tests/benchmark/test_account.py b/saleor/graphql/account/tests/benchmark/test_account.py
index 34d5b71da..187616a72 100644
--- a/saleor/graphql/account/tests/benchmark/test_account.py
+++ b/saleor/graphql/account/tests/benchmark/test_account.py
@@ -103,11 +103,13 @@ 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"]
     assert data
 
@@ -350,14 +352,17 @@ CUSTOMERS_QUERY = """
 @pytest.mark.django_db
 @pytest.mark.count_queries(autouse=False)
 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
 
 
 @pytest.mark.django_db
diff --git a/saleor/graphql/account/tests/test_account.py b/saleor/graphql/account/tests/test_account.py
index f816df34c..bd1cfd45b 100644
--- a/saleor/graphql/account/tests/test_account.py
+++ b/saleor/graphql/account/tests/test_account.py
@@ -179,10 +179,11 @@ def test_query_customer_user(
     customer_user,
     gift_card_used,
     gift_card_expiry_date,
     address,
     permission_manage_users,
+    permission_manage_orders,
     media_root,
     settings,
 ):
     user = customer_user
     user.default_shipping_address.country = "US"
@@ -197,11 +198,13 @@ def test_query_customer_user(
     Group.objects.create(name="empty group")
 
     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"]
     assert data["email"] == user.email
     assert data["firstName"] == user.first_name
@@ -266,10 +269,11 @@ def test_query_customer_user(
 def test_query_customer_user_with_orders(
     staff_api_client,
     customer_user,
     order_list,
     permission_manage_users,
+    permission_manage_orders,
 ):
     # given
     query = FULL_USER_QUERY
     order_unfulfilled = order_list[0]
     order_unfulfilled.user = customer_user
@@ -289,28 +293,65 @@ def test_query_customer_user_with_orders(
     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]
+        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,
 ):
     user = customer_user
     user.default_shipping_address.country = "US"
@@ -325,11 +366,13 @@ def test_query_customer_user_app(
     Group.objects.create(name="empty group")
 
     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)
     data = content["data"]["user"]
     assert data["email"] == user.email
@@ -363,10 +406,45 @@ def test_query_customer_user_app_no_permission(
     response = app_api_client.post_graphql(query, variables)
 
     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,
     address,
     permission_manage_users,
@@ -991,10 +1069,11 @@ def test_me_with_cancelled_fulfillments(
 
 def test_user_with_cancelled_fulfillments(
     staff_api_client,
     customer_user,
     permission_manage_users,
+    permission_manage_orders,
     fulfilled_order_with_cancelled_fulfillment,
 ):
     query = """
     query User($id: ID!) {
         user(id: $id) {
@@ -1011,11 +1090,13 @@ 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(
         "Order", fulfilled_order_with_cancelled_fulfillment.id
     )
diff --git a/saleor/graphql/account/types.py b/saleor/graphql/account/types.py
index 4fbe2d179..680d57e05 100644
--- a/saleor/graphql/account/types.py
+++ b/saleor/graphql/account/types.py
@@ -362,13 +362,17 @@ class User(ModelObjectType):
         from ..order.types import OrderCountableConnection
 
         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
             )
 

Vulnerable code:         query, variables, permissions=[permission_manage_staff]
    staff_api_client.user.user_permissions.set([permission_manage_users])
    staff_api_client.user.user_permissions.add(permission_manage_users)
        query, variables, permissions=[permission_manage_users]
        graphene.Node.to_global_id("Order", order.pk)
        for order in [order_unfulfilled, order_unconfirmed]
    app.permissions.add(permission_manage_staff, permission_manage_users)
    staff_api_client.user.user_permissions.add(permission_manage_users)
                orders = list(
                    filter(lambda order: order.status != OrderStatus.DRAFT, orders)
                )
Fixed code: - 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
        query,
        variables,
        permissions=[permission_manage_staff, permission_manage_orders],
    permission_manage_orders,
    staff_api_client.user.user_permissions.set(
        [permission_manage_users, permission_manage_orders]
    )
    permission_manage_orders,
    staff_api_client.user.user_permissions.add(
        permission_manage_users, permission_manage_orders
    )
    permission_manage_orders,
        query,
        variables,
        permissions=[permission_manage_users, permission_manage_orders],
        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)


    permission_manage_orders,
    app.permissions.add(
        permission_manage_staff, permission_manage_users, permission_manage_orders
    )
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)


    permission_manage_orders,
    staff_api_client.user.user_permissions.add(
        permission_manage_users, permission_manage_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()