Repository: leshchenko1979/fast_bitrix24
Pull request #243 (2024-07-17 09:34:51)

Авторизация по OAuth
Добавить авторизацию по токену.

При инициализации объекта Bitrix пользователь добавляет параметр token_func, содержащий ссылку на асинхронную функцию, которая вызывается библиотекой при необходимости получения или обновления токена. 

Эта функция должна возвращать токен авторизации.

Если сервер возвращает ошибку о том, что токен устарел, все прочие запросы, идущие в параллели, приостанавливаются, пока не будет обновлен токен.

sourcery-ai[bot]: <!-- Generated by sourcery-ai[bot]: start review_guide -->

🧙 Sourcery is reviewing your pull request!

---

<details>
<summary>Tips</summary>

- Trigger a new Sourcery review by commenting `@sourcery-ai review` on the pull request.
- Continue your discussion with Sourcery by replying directly to review comments.
- You can change your review settings at any time by accessing your [dashboard](https://app.sourcery.ai):
    - Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    - Change the review language;
- You can always [contact us](mailto:support@sourcery.ai) if you have any questions or feedback.

</details>

<!-- Generated by sourcery-ai[bot]: end review_guide -->
leshchenko1979: @MSITETOP,

1. А вы видели #149 ? 
2. Если я правильно понимаю, в вашем решении никак не решен вопрос обновления auth_token. Вы можете попробовать воспользоваться кодом из #149, возможно, он решает вашу проблему более полно?
MSITETOP: > @MSITETOP,
> 
> 1. А вы видели [Авторизация по OAuth #149](https://github.com/leshchenko1979/fast_bitrix24/pull/149) ?
> 2. Если я правильно понимаю, в вашем решении никак не решен вопрос обновления auth_token. Вы можете попробовать воспользоваться кодом из [Авторизация по OAuth #149](https://github.com/leshchenko1979/fast_bitrix24/pull/149), возможно, он решает вашу проблему более полно?

1,2 видел. насколько я понял это решение было примерно 5 лет назад. с тех пор уже много всего изменилось. более того я не вижу смысла в вашу библиотеку пихать механизм рефреша токена, так как он за собой влечет и хранение этих токенов, а это какая-то БД и тут опять не хотелось бы привязываться к конкретной БД и ее структуре. 
А вот дать возможность вместо хука указать токен авторизации на мой взгляд оптимальное решение
leshchenko1979: > с тех пор уже много всего изменилось.

Кажется, с точки зрения OAuth-авторизации не изменилось ничего.

> так как он за собой влечет и хранение этих токенов, а это какая-то БД и тут опять не хотелось бы привязываться к конкретной БД и ее структуре.

не вижу логики -- зачем хранить токены в БД?
MSITETOP: > 
> не вижу логики -- зачем хранить токены в БД?
> 
кажется я понял о чем ты "зачем делать реализацию хранения токена в fast_bitrix24, если для получения токена используется своя произвольная функция получения токена?" согласен, в этой реализации хранение токенов внутри fast_bitrix24 не требуется

leshchenko1979: > единственное мне наверное не нравится, что токен передается в виде GET параметра

Это можно поправить.

Ключевое, что мне хотелось бы видеть в релизе -- это какой-то механизм обновления токена. У меня в #149 это сделано через вызов пользовательской функции, которая будет возвращать свежий токен. То есть, удобным способом отдано на откуп пользователю.

Возможно, есть еще какие-то механизмы?

У меня предложение -- можете попробовать у себя покрутить #149 ? 

Если хотите, добавить туда упаковку auth в тело POST-запроса.

Либо в свой PR добавьте обновление токена.


leshchenko1979: Так, я вижу, что #149 содержит конфликты мерджа. Боюсь делать мердж -- можно нарубить дров. Давайте возьмем ваш код за основу и в нем попробуем наладить освежение токена?
MSITETOP: > Так, я вижу, что #149 содержит конфликты мерджа. Боюсь делать мердж -- можно нарубить дров. Давайте возьмем ваш код за основу и в нем попробуем наладить освежение токена?

ок. тоесть мне в этом pr сделать реализацию с передачей функции, а не токена напрямую?
MSITETOP: > > ок. тоесть мне в этом pr сделать реализацию с передачей функции, а не токена напрямую?
> 
> Я добавил обновление токена. Осталось:
> 
> * протестировать работоспособность
> * написать автотесты
> * обновить документацию

я чем сейчас могу помочь? протестировать работоспособность?
leshchenko1979: > я чем сейчас могу помочь? протестировать работоспособность?

Да.

У меня нет под рукой аккаунта, где требуется авторизация, и провести интеграционное тестирование я сам не смогу.

Ну и в отношении автотестов и документации помощь тоже приветствуется. :)
MSITETOP: > > я чем сейчас могу помочь? протестировать работоспособность?
> 
> Да.
> 
> У меня нет под рукой аккаунта, где требуется авторизация, и провести интеграционное тестирование я сам не смогу.
> 
> Ну и в отношении автотестов и документации помощь тоже приветствуется. :)

ок. в течении дня протестирую и отпишусь
leshchenko1979: Мне также нужно будет получить от вас образец ответа сервера, который отправляется при необходимости обновления токена.

Его можно получить, например, включив логирование.

Сможете прислать?
MSITETOP: > Мне также нужно будет получить от вас образец ответа сервера, который отправляется при необходимости обновления токена.
> 
> Его можно получить, например, включив логирование.
> 
> Сможете прислать?

да. все отправлю после тестов. 

в том числе (видимо в личку в телеге) стенд где можно проверить и креды приложения тестового. 

Patch:
diff --git a/API.md b/API.md
index d7ab460..a01061d 100644
--- a/API.md
+++ b/API.md
@@ -4,11 +4,12 @@
 
 Внутри объекта ведётся учёт скорости отправки запросов к серверу, поэтому важно, чтобы все запросы приложения в отношении одного аккаунта с одного IP-адреса отправлялись из одного экземпляра `Bitrix`.
 
-### Метод ` __init__(self, webhook: str, verbose: bool = True, respect_velocity_policy: bool = True, request_pool_size: int = 50, requests_per_second: float = 2.0, client: aiohttp.ClientSession = None):`
+### Метод ` __init__(self, webhook: str, token_func: Awaitable = None, verbose: bool = True, respect_velocity_policy: bool = True, request_pool_size: int = 50, requests_per_second: float = 2.0, ssl: bool = True, client: aiohttp.ClientSession = None):`
 Создаёт клиента для доступа к Битрикс24.
 
 #### Параметры
 - `webhook: str` - URL вебхука, полученного от сервера Битрикс.
+- `token_func: Awaitable = None` - ссылка на асинхронную функцию, возвращающаю OAuth-токен для запросов к серверу. Если не указана, то OAuth-авторизация не используется.
 - `verbose: bool = True` - показывать прогрессбар при выполнении запроса.
 - `respect_velocity_policy: bool = True` - соблюдать политику Битрикса о скорости запросов.
 - `request_pool_size: int = 50` - размер пула запросов, который
diff --git a/README.md b/README.md
index 8c53d93..a5465ec 100644
--- a/README.md
+++ b/README.md
@@ -35,6 +35,7 @@ API wrapper для Питона для быстрого получения да
 - Высокоуровневые списочные методы для сокращения количества необходимого кода. Большинство операций занимают только одну строку кода. Обработка параллельных запросов, упаковка запросов в батчи и многое другое убрано "под капот".
 - Позволяет задавать параметры запроса именно в таком виде, как они приведены в [документации к Bitrix24 REST API](https://dev.1c-bitrix.ru/rest_help/index.php). Параметры проверяются на корректность для облегчения отладки.
 - Выполнение запросов автоматически сопровождается прогресс-баром из пакета `tqdm`, иллюстрирующим не только количество обработанных элементов, но и прошедшее и оставшееся время выполнения запроса.
+- При работе с приложениями отслеживает устаревание токена авторизации и автоматически обновляет его через вызов функции, передаваемой пользователем.
 
 ### Синхронный и асинхронный клиенты
 - Наличие асинхронного клиента позволяет использовать библиотеку для написания веб-приложений (например, телеграм-ботов).
@@ -160,7 +161,7 @@ results = bx.call_batch ({
 
 ```
 ### Асинхронные вызовы
-Если требуется использование бибилиотеки в асинхронном коде, то вместо клиента `Bitrix()` создавайте клиент класса `BitrixAsync()`:
+Если требуется использование бибилиотеки в асинхронном коде (или в ноутбуках), то вместо клиента `Bitrix()` создавайте клиент класса `BitrixAsync()`:
 ```python
 from fast_bitrix24 import BitrixAsync
 bx = BitrixAsync(webhook)
@@ -169,7 +170,30 @@ bx = BitrixAsync(webhook)
 ```python
 leads = await bx.get_all('crm.lead.list')
 ```
-## Как это работает
+
+### Авторизация через OAuth
+Если требуется авторизация через OAuth, то при инициализации клиента `Bitrix()` необходимо передать в параметре `token_func` ссылку на асинхронную функцию, которая будет возвращать токен авторизации:
+```python
+from fast_bitrix24 import Bitrix
+import requests
+
+async def get_new_token() -> str:
+    oauth_url = 'https://oauth.bitrix.info/oauth/token/'
+    params={
+        'grant_type': 'refresh_token',
+        'client_id': ...,
+        'client_secret': ...,
+        'refresh_token': ...
+    }
+    result = requests.get(oauth_url, timeout=10, params=params)
+    return result.json()["access_token"]
+
+bx = Bitrix(webhook, token_func=get_new_token)
+...
+```
+`token_func` будет вызываться каждый раз, когда необходимо получить впервые или обновить токен авторизации.
+
+## Как работает библиотека
 1. Перед обращением к серверу во всех методах класса `Bitrix` происходит проверка корректности самых популярных параметров, передаваемых к серверу, и поднимаются исключения `TypeError` и `ValueError` при наличии ошибок.
 2. Cоздаются запросы на получение всех элементов из запрошенного списка.
 3. Созданные запросы упаковываются в батчи по 50 запросов в каждом.
diff --git a/fast_bitrix24/bitrix.py b/fast_bitrix24/bitrix.py
index 3514113..8b6173a 100644
--- a/fast_bitrix24/bitrix.py
+++ b/fast_bitrix24/bitrix.py
@@ -3,6 +3,7 @@
 import asyncio
 import functools as ft
 from contextlib import contextmanager
+from inspect import iscoroutinefunction
 from typing import Iterable, Union
 
 import aiohttp
@@ -25,9 +26,11 @@ from .user_request import (
 class BitrixAsync:
     """Клиент для асинхронных запросов к Битрикс24."""
 
+    @beartype
     def __init__(
         self,
         webhook: str,
+        token_func=None,
         verbose: bool = True,
         respect_velocity_policy: bool = True,
         request_pool_size: int = 50,
@@ -40,6 +43,8 @@ class BitrixAsync:
 
         Параметры:
         - `webhook: str` - URL вебхука, полученного от сервера Битрикс
+        - `token_func: Awaitable = None` - асинхронная функция для получения
+        и обновления токена в случае работы с приложениями и OAuth-авторизацией
         - `verbose: bool = True` - показывать ли прогрессбар при выполнении
         запроса
         - `respect_velocity_policy: bool = True` - соблюдать ли политику
@@ -55,8 +60,12 @@ class BitrixAsync:
         пользователем. Ожидаеется, что пользователь сам откроет и закроет сессию.
         """
 
+        if token_func is not None and not iscoroutinefunction(token_func):
+            raise ValueError("`token_func` must be an async function.")
+
         self.srh = ServerRequestHandler(
             webhook=webhook,
+            token_func=token_func,
             respect_velocity_policy=respect_velocity_policy,
             request_pool_size=request_pool_size,
             requests_per_second=requests_per_second,
diff --git a/fast_bitrix24/srh.py b/fast_bitrix24/srh.py
index ed901e7..b612fa0 100644
--- a/fast_bitrix24/srh.py
+++ b/fast_bitrix24/srh.py
@@ -32,6 +32,10 @@ class ServerError(Exception):
     pass
 
 
+class TokenRejectedError(Exception):
+    pass
+
+
 RETRIED_ERRORS = (
     ClientPayloadError,
     ClientConnectionError,
@@ -54,6 +58,7 @@ class ServerRequestHandler:
     def __init__(
         self,
         webhook: str,
+        token_func,
         respect_velocity_policy: bool,
         request_pool_size: int,
         requests_per_second: float,
@@ -61,6 +66,14 @@ class ServerRequestHandler:
         ssl: bool = True,
     ):
         self.webhook = self.standardize_webhook(webhook)
+
+        self.token_func = token_func
+        self.token = None
+
+        # token_received - флаг, что получение токена начало и не закончено
+        self.token_received = Event()
+        self.token_received.set()
+
         self.respect_velocity_policy = respect_velocity_policy
 
         self.active_runs = 0
@@ -145,6 +158,10 @@ class ServerRequestHandler:
         """Делает единичный запрос к серверу,
         с повторными попытками при необходимости."""
 
+        # начальное получение токена
+        if self.token_func and not self.token:
+            await self.ensure_new_token()
+
         while True:
 
             try:
@@ -152,9 +169,14 @@ class ServerRequestHandler:
                 self.success()
                 return result
 
-            except RETRIED_ERRORS as err:  # all other exceptions will propagate
+            except TokenRejectedError:
+                await self.ensure_new_token()
+
+            except RETRIED_ERRORS as err:
                 self.failure(err)
 
+            # all other exceptions will propagate
+
     async def request_attempt(self, method, params=None) -> dict:
         """Делает попытку запроса к серверу, ожидая при необходимости."""
 
@@ -162,8 +184,12 @@ class ServerRequestHandler:
             async with self.acquire(method):
                 logger.debug(f"Requesting {{'method': {method}, 'params': {params}}}")
 
+                params_with_auth = params.copy() if params else {}
+                if self.token:
+                    params_with_auth["auth"] = self.token
+
                 async with self.session.post(
-                    url=self.webhook + method, json=params, ssl=self.ssl
+                    url=self.webhook + method, json=params_with_auth, ssl=self.ssl
                 ) as response:
                     json = await response.json(encoding="utf-8")
 
@@ -174,10 +200,16 @@ class ServerRequestHandler:
                     return json
 
         except ClientResponseError as error:
+
             if error.status // 100 == 5:  # ошибки вида 5XX
                 raise ServerError("The server returned an error") from error
 
-            raise
+            elif error.status == 401 and self.token_func:
+                raise TokenRejectedError(
+                    "The server rejected the auth token"
+                ) from error
+
+            raise  # иначе повторяем полученное исключение
 
     def add_throttler_records(self, method, params: dict, json: dict):
         if "result_time" in json:
@@ -274,3 +306,16 @@ class ServerRequestHandler:
         finally:
             self.concurrent_requests -= 1
             self.request_complete.set()
+
+    async def ensure_new_token(self):
+        """Получает новый токен, если процесс получения токена еще не запущен,
+        или ждет его завершения."""
+
+        if self.token_received.is_set():
+            logger.debug("Requesting new token")
+
+            self.token_received.clear()
+            self.token = await self.token_func()
+            self.token_received.set()
+        else:
+            await self.token_received.wait()


Test patch:
diff --git a/tests/test_auth.py b/tests/test_auth.py
new file mode 100644
index 0000000..6ecb0a9
--- /dev/null
+++ b/tests/test_auth.py
@@ -0,0 +1,65 @@
+import pytest
+from fast_bitrix24 import Bitrix
+from fast_bitrix24.srh import ServerRequestHandler, TokenRejectedError
+
+from typing import Dict, List, Union
+
+class MockSRH(ServerRequestHandler):
+    def __init__(self, token_func, response: Union[Dict, List[Dict]]):
+        self.response = response if isinstance(response, list) else [response]
+        self.element_no = 0
+
+        super().__init__("https://google.com/path", token_func, False, 50, 2, None)
+
+    async def request_attempt(self, *args, **kwargs):
+        result = self.response[self.element_no]
+        try:
+            if isinstance(result, Exception):
+                raise result
+            else:
+                return result
+        finally:
+            self.element_no += 1
+
+
+@pytest.mark.skip(reason="TODO")
+def test_first_request():
+    # нужно проверить, что вызывается функция запроса токена при первом запросе
+
+    raise AssertionError
+
+@pytest.mark.skip(reason="TODO")
+def test_auth_success():
+    # нужно проверить, что серверу передается токен, полученный от token_func
+
+    raise AssertionError
+
+@pytest.mark.skip(reason="TODO")
+def test_auth_failure():
+    # нужно проверить, что вызывается функция запроса токена, если сервер вернул ошибку токена
+
+    raise AssertionError
+
+@pytest.mark.skip(reason="TODO")
+def test_abort_on_multiple_failures():
+    # нужно проверить, что если token_func регулярно возвращает токен, который отвергается сервером,
+    # то запрос оборвется после MAX_RETRIES неудачных попыток
+
+    raise AssertionError
+
+def test_expired_token(bx_dummy):
+    # нужно проверить, что вызывается функция запроса токена, если токен истек
+
+    called_count = 0
+
+    async def token_func(*args, **kwargs):
+        nonlocal called_count
+        called_count += 1
+        return "abc"
+
+    bx_dummy.srh = MockSRH(token_func, [{}, TokenRejectedError(), {}])
+
+    bx_dummy.call("test", raw=True)
+    bx_dummy.call("test", raw=True)
+
+    assert called_count == 2  # начальный запрос + запрос после истечения токена
diff --git a/tests/test_server_responses.py b/tests/test_server_responses.py
index 2b472ca..313df0e 100644
--- a/tests/test_server_responses.py
+++ b/tests/test_server_responses.py
@@ -17,7 +17,7 @@ class MockSRH(ServerRequestHandler):
         self.response = response if isinstance(response, list) else [response]
         self.element_no = -1
 
-        super().__init__("https://google.com/path", False, 50, 2, None)
+        super().__init__("https://google.com/path", None, False, 50, 2, None)
 
     async def single_request(self, *args, **kwargs):
         self.element_no += 1
diff --git a/tests/test_srh.py b/tests/test_srh.py
index d0b827d..e3cb535 100644
--- a/tests/test_srh.py
+++ b/tests/test_srh.py
@@ -18,7 +18,7 @@ async def test_request_attempt():
     mock_session = AsyncMock()
     mock_session.post = mock_post
 
-    handler = ServerRequestHandler('https://google.com/webhook', True, 50, 2, mock_session)
+    handler = ServerRequestHandler('https://google.com/webhook', None, True, 50, 2, mock_session)
 
     # Call the method
     result = await handler.request_attempt('method', {'param': 'value'})