Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge pull request saleor#5192 from maarcingebala/fix-checkout-attach…
…-user-permissions

Fix permission for checkoutCustomerAttach mutation
  • Loading branch information
maarcingebala committed Jan 23, 2020
2 parents 0222f85 + 7df2877 commit 233b889
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -54,6 +54,7 @@ All notable, unreleased changes to this project will be documented in this file.
- Drop gettext occurrences - #5189 by @IKarbowiak
- Fix `product_created` webhook - #5187 by @dzkb
- Drop unused resolver `resolve_availability` - #5190 by @maarcingebala
- Fix permission for `checkoutCustomerAttach` mutation - #5192 by @maarcingebala

## 2.9.0

Expand Down
41 changes: 34 additions & 7 deletions saleor/graphql/checkout/mutations.py
Expand Up @@ -5,6 +5,7 @@
from django.core.exceptions import ObjectDoesNotExist, ValidationError
from django.db import transaction
from django.db.models import Prefetch
from graphql_jwt.exceptions import PermissionDenied

from ...account.error_codes import AccountErrorCode
from ...checkout import models
Expand Down Expand Up @@ -35,7 +36,7 @@
from ...product import models as product_models
from ...warehouse.availability import check_stock_quantity, get_available_quantity
from ..account.i18n import I18nMixin
from ..account.types import AddressInput, User
from ..account.types import AddressInput
from ..core.mutations import (
BaseMutation,
ClearMetaBaseMutation,
Expand Down Expand Up @@ -404,22 +405,39 @@ class CheckoutCustomerAttach(BaseMutation):

class Arguments:
checkout_id = graphene.ID(required=True, description="ID of the checkout.")
customer_id = graphene.ID(required=True, description="The ID of the customer.")
customer_id = graphene.ID(
required=False,
description=(
"The ID of the customer. DEPRECATED: This field is deprecated and will "
"be removed in Saleor 2.11. To identify a customer you should "
"authenticate with JWT token."
),
)

class Meta:
description = "Sets the customer as the owner of the checkout."
error_type_class = CheckoutError
error_type_field = "checkout_errors"

@classmethod
def perform_mutation(cls, _root, info, checkout_id, customer_id):
def check_permissions(cls, context):
return context.user.is_authenticated

@classmethod
def perform_mutation(cls, _root, info, checkout_id, customer_id=None):
checkout = cls.get_node_or_error(
info, checkout_id, only_type=Checkout, field="checkout_id"
)
customer = cls.get_node_or_error(
info, customer_id, only_type=User, field="customer_id"
)
checkout.user = customer

# Check if provided customer_id matches with the authenticated user and raise
# error if it doesn't. This part can be removed when `customer_id` field is
# removed.
if customer_id:
current_user_id = graphene.Node.to_global_id("User", info.context.user.id)
if current_user_id != customer_id:
raise PermissionDenied()

checkout.user = info.context.user
checkout.save(update_fields=["user", "last_change"])
return CheckoutCustomerAttach(checkout=checkout)

Expand All @@ -435,11 +453,20 @@ class Meta:
error_type_class = CheckoutError
error_type_field = "checkout_errors"

@classmethod
def check_permissions(cls, context):
return context.user.is_authenticated

@classmethod
def perform_mutation(cls, _root, info, checkout_id):
checkout = cls.get_node_or_error(
info, checkout_id, only_type=Checkout, field="checkout_id"
)

# Raise error if the current user doesn't own the checkout of the given ID.
if checkout.user and checkout.user != info.context.user:
raise PermissionDenied()

checkout.user = None
checkout.save(update_fields=["user", "last_change"])
return CheckoutCustomerDetach(checkout=checkout)
Expand Down
2 changes: 1 addition & 1 deletion saleor/graphql/schema.graphql
Expand Up @@ -2288,7 +2288,7 @@ type Mutation {
checkoutBillingAddressUpdate(billingAddress: AddressInput!, checkoutId: ID!): CheckoutBillingAddressUpdate
checkoutComplete(checkoutId: ID!, redirectUrl: String, storeSource: Boolean = false): CheckoutComplete
checkoutCreate(input: CheckoutCreateInput!): CheckoutCreate
checkoutCustomerAttach(checkoutId: ID!, customerId: ID!): CheckoutCustomerAttach
checkoutCustomerAttach(checkoutId: ID!, customerId: ID): CheckoutCustomerAttach
checkoutCustomerDetach(checkoutId: ID!): CheckoutCustomerDetach
checkoutEmailUpdate(checkoutId: ID, email: String!): CheckoutEmailUpdate
checkoutLineDelete(checkoutId: ID!, lineId: ID): CheckoutLineDelete
Expand Down
33 changes: 28 additions & 5 deletions tests/api/test_checkout.py
Expand Up @@ -8,6 +8,7 @@
from django.core.exceptions import ValidationError
from prices import Money, TaxedMoney

from saleor.account.models import User
from saleor.checkout import calculations
from saleor.checkout.error_codes import CheckoutErrorCode
from saleor.checkout.models import Checkout
Expand All @@ -25,7 +26,7 @@
from saleor.shipping import ShippingMethodType
from saleor.shipping.models import ShippingMethod
from saleor.warehouse.models import Stock
from tests.api.utils import get_graphql_content
from tests.api.utils import assert_no_permission, get_graphql_content


@pytest.fixture
Expand Down Expand Up @@ -851,7 +852,9 @@ def test_checkout_line_delete_by_zero_quantity(
mocked_update_shipping_method.assert_called_once_with(checkout, mock.ANY)


def test_checkout_customer_attach(user_api_client, checkout_with_item, customer_user):
def test_checkout_customer_attach(
api_client, user_api_client, checkout_with_item, customer_user
):
checkout = checkout_with_item
assert checkout.user is None

Expand All @@ -871,16 +874,26 @@ def test_checkout_customer_attach(user_api_client, checkout_with_item, customer_
"""
checkout_id = graphene.Node.to_global_id("Checkout", checkout.pk)
customer_id = graphene.Node.to_global_id("User", customer_user.pk)

variables = {"checkoutId": checkout_id, "customerId": customer_id}

# Mutation should fail for unauthenticated customers
response = api_client.post_graphql(query, variables)
assert_no_permission(response)

# Mutation should succeed for authenticated customer
response = user_api_client.post_graphql(query, variables)
content = get_graphql_content(response)

data = content["data"]["checkoutCustomerAttach"]
assert not data["errors"]
checkout.refresh_from_db()
assert checkout.user == customer_user

# Mutation with ID of a different user should fail as well
other_customer = User.objects.create_user("othercustomer@example.com", "password")
variables["customerId"] = graphene.Node.to_global_id("User", other_customer.pk)
response = user_api_client.post_graphql(query, variables)
assert_no_permission(response)


MUTATION_CHECKOUT_CUSTOMER_DETACH = """
mutation checkoutCustomerDetach($checkoutId: ID!) {
Expand All @@ -904,16 +917,26 @@ def test_checkout_customer_detach(user_api_client, checkout_with_item, customer_

checkout_id = graphene.Node.to_global_id("Checkout", checkout.pk)
variables = {"checkoutId": checkout_id}

# Mutation should succeed if the user owns this checkout.
response = user_api_client.post_graphql(
MUTATION_CHECKOUT_CUSTOMER_DETACH, variables
)
content = get_graphql_content(response)

data = content["data"]["checkoutCustomerDetach"]
assert not data["errors"]
checkout.refresh_from_db()
assert checkout.user is None

# Mutation should fail when user calling it doesn't own the checkout.
other_user = User.objects.create_user("othercustomer@example.com", "password")
checkout.user = other_user
checkout.save()
response = user_api_client.post_graphql(
MUTATION_CHECKOUT_CUSTOMER_DETACH, variables
)
assert_no_permission(response)


MUTATION_CHECKOUT_SHIPPING_ADDRESS_UPDATE = """
mutation checkoutShippingAddressUpdate(
Expand Down

0 comments on commit 233b889

Please sign in to comment.