From f784ed3c3e47c758d2643ed00646f61dc5aa351f Mon Sep 17 00:00:00 2001 From: Jake Urban <10968980+JakeUrban@users.noreply.github.com> Date: Mon, 4 Apr 2022 09:58:55 -0700 Subject: [PATCH] bug fixes: don't require SEP-12 'account' request parameter and always pass 'memo' as a string (#606) --- polaris/polaris/integrations/customers.py | 5 +- polaris/polaris/sep12/customer.py | 220 +++++++------------ polaris/polaris/tests/sep12/test_customer.py | 12 +- 3 files changed, 90 insertions(+), 147 deletions(-) diff --git a/polaris/polaris/integrations/customers.py b/polaris/polaris/integrations/customers.py index 76575749..b48c054d 100644 --- a/polaris/polaris/integrations/customers.py +++ b/polaris/polaris/integrations/customers.py @@ -42,10 +42,7 @@ def get( .. _ObjectDoesNotExist: https://docs.djangoproject.com/en/3.1/ref/exceptions/#objectdoesnotexist Return a dictionary matching the response schema outlined in `SEP-12 GET /customer`_ - based on the `params` passed. The key-value pairs in `params` match the arguments - sent in the request with the exception of ``sep10_client_account``. This parameter - was added in preparation for a future change. For now, ``sep10_client_account`` will - always match ``account``. + based on the `params` passed. Raise a ``ValueError`` if the parameters are invalid, or raise an ObjectDoesNotExist_ exception if the customer specified via the ``id`` parameter diff --git a/polaris/polaris/sep12/customer.py b/polaris/polaris/sep12/customer.py index 6e8212b0..b8813a45 100644 --- a/polaris/polaris/sep12/customer.py +++ b/polaris/polaris/sep12/customer.py @@ -1,4 +1,4 @@ -from typing import Dict +from typing import Dict, Optional, Tuple from django.utils.translation import gettext as _ from django.core.validators import URLValidator @@ -34,50 +34,18 @@ class CustomerAPIView(APIView): @staticmethod @validate_sep10_token() def get(token: SEP10Token, request: Request) -> Response: - if request.GET.get("account") and request.GET.get("account") != ( - token.muxed_account or token.account - ): - return render_error_response( - _("The account specified does not match authorization token"), - status_code=403, - ) - elif ( - token.memo - and request.GET.get("memo") - and ( - str(token.memo) != request.GET.get("memo") - or request.GET.get("memo_type") != Transaction.MEMO_TYPES.id - ) - ): - return render_error_response( - _( - "The memo specified does not match the memo ID authorized via SEP-10" - ), - status_code=403, - ) - elif request.GET.get("id") and ( - request.GET.get("account") - or request.GET.get("memo") - or request.GET.get("memo_type") - ): - return render_error_response( - _( - "requests with 'id' cannot also have 'account', 'memo', or 'memo_type'" - ) - ) + maybe_response = validate_id_parameters( + token=token, request_data=request.query_params + ) + if maybe_response: + return maybe_response try: - # validate memo and memo_type - make_memo(request.GET.get("memo"), request.GET.get("memo_type")) - except (ValueError, TypeError): - return render_error_response(_("invalid 'memo' for 'memo_type'")) - - memo = request.GET.get("memo") or token.memo - memo_type = None - if memo: - memo_type = request.GET.get("memo_type") or Transaction.MEMO_TYPES.id - if memo_type == Transaction.MEMO_TYPES.id: - memo = str(memo) + memo, memo_type = validate_memo_parameters( + token=token, request_data=request.query_params + ) + except ValueError as e: + return render_error_response(str(e)) try: response_data = rci.get( @@ -85,7 +53,7 @@ def get(token: SEP10Token, request: Request) -> Response: request=request, params={ "id": request.GET.get("id"), - "account": request.GET.get("account"), + "account": token.muxed_account or token.account, "memo": memo, "memo_type": memo_type, "type": request.GET.get("type"), @@ -112,51 +80,16 @@ def get(token: SEP10Token, request: Request) -> Response: @staticmethod @validate_sep10_token() def put(token: SEP10Token, request: Request) -> Response: - if request.data.get("id"): - if not isinstance(request.data.get("id"), str): - return render_error_response(_("bad ID value, expected str")) - elif ( - request.data.get("account") - or request.data.get("memo") - or request.data.get("memo_type") - ): - return render_error_response( - _( - "requests with 'id' cannot also have 'account', 'memo', or 'memo_type'" - ) - ) - elif request.data.get("account") != (token.muxed_account or token.account): - return render_error_response( - _("The account specified does not match authorization token"), - status_code=403, - ) - elif ( - token.memo - and request.data.get("memo") - and ( - str(token.memo) != str(request.data.get("memo")) - or request.data.get("memo_type") != Transaction.MEMO_TYPES.id - ) - ): - return render_error_response( - _( - "The memo specified does not match the memo ID authorized via SEP-10" - ), - status_code=403, - ) + maybe_response = validate_id_parameters(token=token, request_data=request.data) + if maybe_response: + return maybe_response try: - # validate memo and memo_type - make_memo(request.data.get("memo"), request.data.get("memo_type")) - except (ValueError, TypeError): - return render_error_response(_("invalid 'memo' for 'memo_type'")) - - memo = request.data.get("memo") or token.memo - memo_type = None - if memo: - memo_type = request.data.get("memo_type") or Transaction.MEMO_TYPES.id - if memo_type == Transaction.MEMO_TYPES.id: - memo = int(memo) + memo, memo_type = validate_memo_parameters( + token=token, request_data=request.data + ) + except ValueError as e: + return render_error_response(str(e)) try: customer_id = rci.put( @@ -190,49 +123,16 @@ def put(token: SEP10Token, request: Request) -> Response: @parser_classes([MultiPartParser, FormParser, JSONParser]) @validate_sep10_token() def callback(token: SEP10Token, request: Request) -> Response: - if request.data.get("id"): - if not isinstance(request.data.get("id"), str): - return render_error_response(_("bad ID value, expected str")) - elif ( - request.data.get("account") - or request.data.get("memo") - or request.data.get("memo_type") - ): - return render_error_response( - _( - "requests with 'id' cannot also have 'account', 'memo', or 'memo_type'" - ) - ) - elif request.data.get("account") != (token.muxed_account or token.account): - return render_error_response( - _("The account specified does not match authorization token"), - status_code=403, - ) - elif ( - token.memo - and request.data.get("memo") - and ( - str(token.memo) != str(request.data.get("memo")) - or request.data.get("memo_type") != Transaction.MEMO_TYPES.id - ) - ): - return render_error_response( - _("The memo specified does not match the memo ID authorized via SEP-10"), - status_code=403, - ) + maybe_response = validate_id_parameters(token=token, request_data=request.data) + if maybe_response: + return maybe_response try: - # validate memo and memo_type - make_memo(request.data.get("memo"), request.data.get("memo_type")) - except (ValueError, TypeError): - return render_error_response(_("invalid 'memo' for 'memo_type'")) - - memo = request.data.get("memo") or token.memo - memo_type = None - if memo: - memo_type = request.data.get("memo_type") or Transaction.MEMO_TYPES.id - if memo_type == Transaction.MEMO_TYPES.id: - memo = int(memo) + memo, memo_type = validate_memo_parameters( + token=token, request_data=request.data + ) + except ValueError as e: + return render_error_response(str(e)) callback_url = request.data.get("url") if not callback_url: @@ -282,15 +182,11 @@ def delete(token: SEP10Token, request: Request, account: str,) -> Response: ): return render_error_response(_("account not found"), status_code=404) try: - make_memo(request.data.get("memo"), request.data.get("memo_type")) - except (ValueError, TypeError): - return render_error_response(_("invalid 'memo' for 'memo_type'")) - memo = request.data.get("memo") or token.memo - memo_type = None - if memo: - memo_type = request.data.get("memo_type") or Transaction.MEMO_TYPES.id - if memo_type == Transaction.MEMO_TYPES.id: - memo = int(memo) + memo, memo_type = validate_memo_parameters( + token=token, request_data=request.data + ) + except ValueError as e: + return render_error_response(str(e)) try: rci.delete( token=token, @@ -380,6 +276,56 @@ def validate_response_data(data: Dict): ) +def validate_id_parameters(token: SEP10Token, request_data: dict) -> Optional[Response]: + if request_data.get("id"): + if not isinstance(request_data.get("id"), str): + return render_error_response(_("bad ID value, expected str")) + elif ( + request_data.get("account") + or request_data.get("memo") + or request_data.get("memo_type") + ): + return render_error_response( + _( + "requests with 'id' cannot also have 'account', 'memo', or 'memo_type'" + ) + ) + elif request_data.get("account") and request_data.get("account") != ( + token.muxed_account or token.account + ): + return render_error_response( + _("The account specified does not match authorization token"), + status_code=403, + ) + elif ( + token.memo + and request_data.get("memo") + and ( + str(token.memo) != str(request_data.get("memo")) + or request_data.get("memo_type") != Transaction.MEMO_TYPES.id + ) + ): + return render_error_response( + _("The memo specified does not match the memo ID authorized via SEP-10"), + status_code=403, + ) + + +def validate_memo_parameters( + token: SEP10Token, request_data: dict +) -> Tuple[Optional[str], Optional[str]]: + try: + make_memo(request_data.get("memo"), request_data.get("memo_type")) + except (ValueError, TypeError): + raise ValueError(_("invalid 'memo' for 'memo_type'")) + memo = request_data.get("memo") or token.memo + memo_type = None + if memo: + memo = str(memo) + memo_type = request_data.get("memo_type") or Transaction.MEMO_TYPES.id + return memo, memo_type + + def validate_fields(fields: Dict, provided=False): if not isinstance(fields, Dict): raise ValueError( diff --git a/polaris/polaris/tests/sep12/test_customer.py b/polaris/polaris/tests/sep12/test_customer.py index bfeaebc1..f52c0198 100644 --- a/polaris/polaris/tests/sep12/test_customer.py +++ b/polaris/polaris/tests/sep12/test_customer.py @@ -64,7 +64,7 @@ def test_put_success_auth_memo(mock_rci, client): assert kwargs["token"].muxed_account is None assert kwargs["token"].memo == TEST_ACCOUNT_MEMO assert kwargs["params"]["account"] == kwargs["token"].account - assert kwargs["params"]["memo"] == kwargs["token"].memo + assert kwargs["params"]["memo"] == str(kwargs["token"].memo) assert kwargs["params"]["memo_type"] == "id" assert response.status_code == 202, content assert content == {"id": "123"} @@ -92,7 +92,7 @@ def test_put_success_auth_memo_and_body(mock_rci, client): assert kwargs["token"].muxed_account is None assert kwargs["token"].memo == TEST_ACCOUNT_MEMO assert kwargs["params"]["account"] == kwargs["token"].account - assert kwargs["params"]["memo"] == kwargs["token"].memo + assert kwargs["params"]["memo"] == str(kwargs["token"].memo) assert kwargs["params"]["memo_type"] == "id" assert response.status_code == 202, content assert content == {"id": "123"} @@ -120,7 +120,7 @@ def test_put_success_memo_in_body_no_auth(mock_rci, client): assert kwargs["token"].muxed_account is None assert kwargs["token"].memo is None assert kwargs["params"]["account"] == kwargs["token"].account - assert kwargs["params"]["memo"] == TEST_ACCOUNT_MEMO + assert kwargs["params"]["memo"] == str(TEST_ACCOUNT_MEMO) assert kwargs["params"]["memo_type"] == "id" assert response.status_code == 202, content assert content == {"id": "123"} @@ -827,7 +827,7 @@ def test_delete_success_with_memo_auth(mock_delete, client): assert kwargs["token"].muxed_account is None assert kwargs["token"].memo is TEST_ACCOUNT_MEMO assert kwargs["account"] == kwargs["token"].account - assert kwargs["memo"] == kwargs["token"].memo + assert kwargs["memo"] == str(kwargs["token"].memo) assert kwargs["memo_type"] == "id" @@ -846,7 +846,7 @@ def test_delete_success_with_memo_auth_and_body(mock_delete, client): assert kwargs["token"].muxed_account is None assert kwargs["token"].memo is TEST_ACCOUNT_MEMO assert kwargs["account"] == kwargs["token"].account - assert kwargs["memo"] == kwargs["token"].memo + assert kwargs["memo"] == str(kwargs["token"].memo) assert kwargs["memo_type"] == "id" @@ -865,7 +865,7 @@ def test_delete_success_with_memo_in_body_no_auth(mock_delete, client): assert kwargs["token"].muxed_account is None assert kwargs["token"].memo is None assert kwargs["account"] == kwargs["token"].account - assert kwargs["memo"] == TEST_ACCOUNT_MEMO + assert kwargs["memo"] == str(TEST_ACCOUNT_MEMO) assert kwargs["memo_type"] == "id"