From 870317586fb3220a0a2e0c4b44b813b37f7470ee Mon Sep 17 00:00:00 2001 From: Aleksander Maksimeniuk <78569692+alexhook@users.noreply.github.com> Date: Tue, 26 Mar 2024 10:02:21 +0300 Subject: [PATCH] fix: invalid issuer error during incoming request verification if the bot host contains url parts other than the domain (#460) --- README.md | 2 +- pybotx/bot/bot.py | 4 ++-- pybotx/bot/bot_accounts_storage.py | 8 ++++---- pybotx/client/botx_method.py | 12 ++--------- pybotx/models/bot_account.py | 22 ++++++++++++++++++-- pyproject.toml | 2 +- tests/client/test_botx_method.py | 32 +++++++++++++++--------------- tests/conftest.py | 9 +++++++-- tests/models/test_bot_account.py | 20 +++++++++++++++++++ tests/test_end_to_end.py | 2 +- 10 files changed, 74 insertions(+), 39 deletions(-) create mode 100644 tests/models/test_bot_account.py diff --git a/README.md b/README.md index e676f861..53a7b653 100644 --- a/README.md +++ b/README.md @@ -88,7 +88,7 @@ bot = Bot( # Не забудьте заменить эти учётные данные на настоящие, # когда создадите бота в панели администратора. id=UUID("123e4567-e89b-12d3-a456-426655440000"), - host="cts.example.com", + cts_url="https://cts.example.com", secret_key="e29b417773f2feab9dac143ee3da20c5", ), ], diff --git a/pybotx/bot/bot.py b/pybotx/bot/bot.py index 7705121d..b1efdd4e 100644 --- a/pybotx/bot/bot.py +++ b/pybotx/bot/bot.py @@ -217,7 +217,7 @@ from pybotx.missing import Missing, MissingOptional, Undefined from pybotx.models.async_files import File from pybotx.models.attachments import IncomingFileAttachment, OutgoingAttachment -from pybotx.models.bot_account import BotAccount, BotAccountWithSecret +from pybotx.models.bot_account import BotAccountWithSecret from pybotx.models.bot_catalog import BotsListItem from pybotx.models.chats import ChatInfo, ChatListItem from pybotx.models.commands import BotAPICommand, BotCommand @@ -384,7 +384,7 @@ async def wait_botx_method_callback( return await self._callbacks_manager.wait_botx_method_callback(sync_id, timeout) @property - def bot_accounts(self) -> Iterator[BotAccount]: + def bot_accounts(self) -> Iterator[BotAccountWithSecret]: yield from self._bot_accounts_storage.iter_bot_accounts() async def fetch_tokens(self) -> None: diff --git a/pybotx/bot/bot_accounts_storage.py b/pybotx/bot/bot_accounts_storage.py index 1ae97d6b..5fcff38a 100644 --- a/pybotx/bot/bot_accounts_storage.py +++ b/pybotx/bot/bot_accounts_storage.py @@ -5,7 +5,7 @@ from uuid import UUID from pybotx.bot.exceptions import UnknownBotAccountError -from pybotx.models.bot_account import BotAccount, BotAccountWithSecret +from pybotx.models.bot_account import BotAccountWithSecret class BotAccountsStorage: @@ -20,12 +20,12 @@ def get_bot_account(self, bot_id: UUID) -> BotAccountWithSecret: raise UnknownBotAccountError(bot_id) - def iter_bot_accounts(self) -> Iterator[BotAccount]: + def iter_bot_accounts(self) -> Iterator[BotAccountWithSecret]: yield from self._bot_accounts - def get_host(self, bot_id: UUID) -> str: + def get_cts_url(self, bot_id: UUID) -> str: bot_account = self.get_bot_account(bot_id) - return bot_account.host + return bot_account.cts_url def set_token(self, bot_id: UUID, token: str) -> None: self._auth_tokens[bot_id] = token diff --git a/pybotx/client/botx_method.py b/pybotx/client/botx_method.py index 95074957..33302768 100644 --- a/pybotx/client/botx_method.py +++ b/pybotx/client/botx_method.py @@ -12,7 +12,6 @@ Type, TypeVar, ) -from urllib.parse import urlparse, urlunparse from uuid import UUID import httpx @@ -88,15 +87,8 @@ async def execute(self, *args: Any, **kwargs: Any) -> Any: # type: ignore raise NotImplementedError("You should define `execute` method") def _build_url(self, path: str) -> str: - host = self._bot_accounts_storage.get_host(self._bot_id) - - if "://" not in host: - host = f"https://{host}" - - host_parts = urlparse(host) - path = host_parts.path.rstrip("/") + path - - return urlunparse((host_parts.scheme, host_parts.netloc, path, "", "", "")) + cts_url = self._bot_accounts_storage.get_cts_url(self._bot_id) + return "/".join(part.strip("/") for part in (cts_url, path)) def _verify_and_extract_api_model( self, diff --git a/pybotx/models/bot_account.py b/pybotx/models/bot_account.py index 5c201ea7..48200525 100644 --- a/pybotx/models/bot_account.py +++ b/pybotx/models/bot_account.py @@ -1,6 +1,10 @@ from dataclasses import dataclass +from functools import cached_property +from urllib.parse import urlparse from uuid import UUID +from pydantic import AnyHttpUrl, BaseModel + @dataclass class BotAccount: @@ -8,6 +12,20 @@ class BotAccount: host: str -@dataclass -class BotAccountWithSecret(BotAccount): +class BotAccountWithSecret(BaseModel): + id: UUID + cts_url: AnyHttpUrl secret_key: str + + class Config: + allow_mutation = False + keep_untouched = (cached_property,) + + @cached_property + def host(self) -> str: + hostname = urlparse(self.cts_url).hostname + + if hostname is None: + raise ValueError("Could not parse host from cts_url.") + + return hostname diff --git a/pyproject.toml b/pyproject.toml index ece39115..8baca946 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "pybotx" -version = "0.64.0" +version = "0.65.0" description = "A python library for interacting with eXpress BotX API" authors = [ "Sidnev Nikolay ", diff --git a/tests/client/test_botx_method.py b/tests/client/test_botx_method.py index 0c9b2b2a..be2a63e2 100644 --- a/tests/client/test_botx_method.py +++ b/tests/client/test_botx_method.py @@ -1,6 +1,5 @@ from http import HTTPStatus from typing import Literal -from urllib.parse import urlunparse from uuid import UUID import httpx @@ -244,25 +243,27 @@ async def test__botx_method__succeed( @pytest.mark.parametrize( - argnames="protocol", - argvalues=["http", "https"], - ids=["http", "https"], + "cts_url", + ( + "http://127.0.0.1", + "http://localhost", + "http://cts.ru", + "https://cts.ru", + "http://cts.ru:8000", + "http://cts.ru/foo/bar", + "http://cts.ru:8000/foo/bar/", + ), ) -async def test__botx_method__host_with_protocol__succeed( - httpx_client: httpx.AsyncClient, - respx_mock: MockRouter, - host: str, +async def test__build_botx_url_with_different_bot_cts_urls( bot_id: UUID, + cts_url: str, + respx_mock: MockRouter, + httpx_client: httpx.AsyncClient, bot_account: BotAccountWithSecret, - protocol: str, ) -> None: # - Arrange - - bot_account.host = urlunparse(components=(protocol, host, "", "", "", "")) - endpoint = respx_mock.post( - f"{protocol}://{host}/foo/bar", - json={"baz": 1}, - headers={"Content-Type": "application/json"}, + "/".join(parts.strip("/") for parts in (cts_url, "/foo/bar")), ).mock( return_value=httpx.Response( HTTPStatus.OK, @@ -281,8 +282,7 @@ async def test__botx_method__host_with_protocol__succeed( payload = BotXAPIFooBarRequestPayload.from_domain(baz=1) # - Act - - botx_api_foo_bar = await method.execute(payload) + await method.execute(payload) # - Assert - - assert botx_api_foo_bar.to_domain() == UUID("21a9ec9e-f21f-4406-ac44-1a78d2ccf9e3") assert endpoint.called diff --git a/tests/conftest.py b/tests/conftest.py index 35611745..8e47dde9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -57,16 +57,21 @@ def host() -> str: return "cts.example.com" +@pytest.fixture +def cts_url() -> str: + return "https://cts.example.com" + + @pytest.fixture def bot_id() -> UUID: return UUID("24348246-6791-4ac0-9d86-b948cd6a0e46") @pytest.fixture -def bot_account(host: str, bot_id: UUID) -> BotAccountWithSecret: +def bot_account(cts_url: str, bot_id: UUID) -> BotAccountWithSecret: return BotAccountWithSecret( id=bot_id, - host=host, + cts_url=cts_url, secret_key="bee001", ) diff --git a/tests/models/test_bot_account.py b/tests/models/test_bot_account.py new file mode 100644 index 00000000..f9aff896 --- /dev/null +++ b/tests/models/test_bot_account.py @@ -0,0 +1,20 @@ +from uuid import UUID + +import pytest + +from pybotx import BotAccountWithSecret + + +def test__bot_account__could_not_parse_host(bot_id: UUID, cts_url: str) -> None: + # - Arrange - + bot_account = BotAccountWithSecret( + id=bot_id, + cts_url=cts_url, + secret_key="secret_key", + ) + bot_account.Config.allow_mutation = True + bot_account.cts_url = "cts_url" # type: ignore + + # - Assert - + with pytest.raises(ValueError): + bot_account.host diff --git a/tests/test_end_to_end.py b/tests/test_end_to_end.py index dc1524f0..6ecf1ae4 100644 --- a/tests/test_end_to_end.py +++ b/tests/test_end_to_end.py @@ -136,7 +136,7 @@ def asgi_factory() -> FastAPI: bot_accounts = [ BotAccountWithSecret( id=UUID("123e4567-e89b-12d3-a456-426655440000"), - host="cts.example.com", + cts_url="https://cts.example.com", secret_key="e29b417773f2feab9dac143ee3da20c5", ), ]