Skip to content

Commit

Permalink
fix: invalid issuer error during incoming request verification if the…
Browse files Browse the repository at this point in the history
… bot host contains url parts other than the domain (#460)
  • Loading branch information
alexhook authored Mar 26, 2024
1 parent e6d0795 commit 8703175
Show file tree
Hide file tree
Showing 10 changed files with 74 additions and 39 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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",
),
],
Expand Down
4 changes: 2 additions & 2 deletions pybotx/bot/bot.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
8 changes: 4 additions & 4 deletions pybotx/bot/bot_accounts_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand Down
12 changes: 2 additions & 10 deletions pybotx/client/botx_method.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
Type,
TypeVar,
)
from urllib.parse import urlparse, urlunparse
from uuid import UUID

import httpx
Expand Down Expand Up @@ -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,
Expand Down
22 changes: 20 additions & 2 deletions pybotx/models/bot_account.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,31 @@
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:
id: UUID
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
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -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 <[email protected]>",
Expand Down
32 changes: 16 additions & 16 deletions tests/client/test_botx_method.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from http import HTTPStatus
from typing import Literal
from urllib.parse import urlunparse
from uuid import UUID

import httpx
Expand Down Expand Up @@ -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,
Expand All @@ -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
9 changes: 7 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)

Expand Down
20 changes: 20 additions & 0 deletions tests/models/test_bot_account.py
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion tests/test_end_to_end.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
),
]
Expand Down

0 comments on commit 8703175

Please sign in to comment.