From 984dedeb5ee0eb157becd1c677bcedf8468d3977 Mon Sep 17 00:00:00 2001 From: Mikhail Mikheev Date: Thu, 6 Jan 2022 14:50:19 +0100 Subject: [PATCH] Bug: apps are filtered by the client if clientUrl param is empty (#387) * check if client_url param is not empty, rename networkid to chainid * remove default value for clientUrl param * simplify client_url param validation in the view --- src/chains/models.py | 4 ++-- src/safe_apps/tests/test_views.py | 31 +++++++++++++++++++++++-------- src/safe_apps/views.py | 12 ++++++------ 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/chains/models.py b/src/chains/models.py index a48bd8c2..dd457c97 100644 --- a/src/chains/models.py +++ b/src/chains/models.py @@ -1,6 +1,6 @@ import os import re -from typing import IO +from typing import IO, Union from django.core.exceptions import ValidationError from django.core.files.images import get_image_dimensions @@ -26,7 +26,7 @@ def native_currency_path(instance: "Chain", filename: str) -> str: return f"chains/{instance.id}/currency_logo{file_extension}" -def validate_native_currency_size(image: str | IO[bytes]) -> None: +def validate_native_currency_size(image: Union[str, IO[bytes]]) -> None: image_width, image_height = get_image_dimensions(image) if image_width > 512 or image_height > 512: raise ValidationError("Image width and height need to be at most 512 pixels") diff --git a/src/safe_apps/tests/test_views.py b/src/safe_apps/tests/test_views.py index 1d9e0c84..319d5049 100644 --- a/src/safe_apps/tests/test_views.py +++ b/src/safe_apps/tests/test_views.py @@ -232,24 +232,39 @@ def test_apps_returned_on_non_existent_client_url(self) -> None: self.assertEqual(response.json(), json_response) def test_apps_returned_on_empty_client_url(self) -> None: - safe_app = SafeAppFactory.create() + client_1 = ClientFactory.create(url="safe.com") + safe_app_1 = SafeAppFactory.create() + safe_app_2 = SafeAppFactory.create(exclusive_clients=(client_1,)) url = reverse("v1:safe-apps:list") + f'{"?clientUrl="}' response = self.client.get(path=url, data=None, format="json") json_response = [ { - "id": safe_app.app_id, - "url": safe_app.url, - "name": safe_app.name, - "iconUrl": safe_app.icon_url, - "description": safe_app.description, - "chainIds": safe_app.chain_ids, + "id": safe_app_1.app_id, + "url": safe_app_1.url, + "name": safe_app_1.name, + "iconUrl": safe_app_1.icon_url, + "description": safe_app_1.description, + "chainIds": safe_app_1.chain_ids, "provider": None, "accessControl": { "type": "NO_RESTRICTIONS", }, - } + }, + { + "id": safe_app_2.app_id, + "url": safe_app_2.url, + "name": safe_app_2.name, + "iconUrl": safe_app_2.icon_url, + "description": safe_app_2.description, + "chainIds": safe_app_2.chain_ids, + "provider": None, + "accessControl": { + "type": "DOMAIN_ALLOWLIST", + "value": [client_1.url], + }, + }, ] self.assertEqual(response.status_code, 200) self.assertEqual(response.json(), json_response) diff --git a/src/safe_apps/views.py b/src/safe_apps/views.py index 1c804fc5..e0fff1b0 100644 --- a/src/safe_apps/views.py +++ b/src/safe_apps/views.py @@ -42,14 +42,14 @@ def get(self, request: Request, *args: Any, **kwargs: Any) -> Response: def get_queryset(self) -> QuerySet[SafeApp]: queryset = SafeApp.objects.filter(visible=True) - network_id = self.request.query_params.get("chainId") - if network_id is not None and network_id.isdigit(): - queryset = queryset.filter(chain_ids__contains=[network_id]) + chain_id = self.request.query_params.get("chainId") + if chain_id is not None and chain_id.isdigit(): + queryset = queryset.filter(chain_ids__contains=[chain_id]) - host = self.request.query_params.get("clientUrl") - if host is not None: + client_url = self.request.query_params.get("clientUrl") + if client_url: queryset = queryset.filter( - Q(exclusive_clients__url=host) | Q(exclusive_clients__isnull=True) + Q(exclusive_clients__url=client_url) | Q(exclusive_clients__isnull=True) ) return queryset