Refactor Cognito auth into an async, persistence-aware token lifecycle
Chưa có ai nhận issue này.
Đánh giá
- Độ khó
- 5/5
- Thời gian dự kiến
- Hơn một tuần
- Mức phù hợp với người mới
- 35/100
- Loại issue
- Tái cấu trúc
- Độ rõ ràng
- Đặc tả rõ ràng
- Mức độ hoạt động
- Ít trao đổi
- Công nghệ
- python
- Lĩnh vực
- authentication, backend-api-design
Hướng nghiên cứu
Bắt đầu bằng cách đọc luồng xác thực trong evnex/api.py, đặc biệt là các dòng 120-157, 167-207, 218-273, 275-285 và 302-310, sau đó xem lại các dòng 162-197 trong tests/test_auth.py và các ví dụ xác thực trong README. Được coi là hoàn tất khi các tiêu chí chấp nhận được liệt kê đã được đáp ứng, bao gồm xác thực bất đồng bộ, các cập nhật token được lưu bền vững, cơ chế khôi phục có giới hạn đối với 401, xác thực tập trung và các bài kiểm thử xác thực mới.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Mô tả
Context
MFA support in #85 / v0.6.0 adds the missing Cognito challenge flow and fixes several immediate reliability problems. The current API now works reasonably for a CLI or a single long-lived Evnex object.
It is still the wrong abstraction for a Home Assistant integration or another long-running application that must persist credentials safely across restarts.
Problems
1. Refresh-token resumption still requires and retains the password
Evnex.__init__() requires both username and password, even when a refresh token is supplied:
https://github.com/hardbyte/python-evnex/blob/main/evnex/api.py#L120-L157
The README says "the refresh token alone is enough", but callers still need to provide a real or dummy password:
https://github.com/hardbyte/python-evnex/blob/main/README.md#L104-L113
For Home Assistant, the password should ideally be used only during interactive bootstrap/reauthentication and then discarded. Normal startup should require only the durable refresh-token session.
2. Refreshed token state cannot be reliably persisted
pycognito mutates access_token, id_token, and refresh_token internally during authentication and refresh. python-evnex exposes properties for reading them, but provides no notification or async callback when the token set changes:
https://github.com/hardbyte/python-evnex/blob/main/evnex/api.py#L167-L207
https://github.com/hardbyte/python-evnex/blob/main/evnex/api.py#L275-L285
A caller can manually inspect the properties after a request, but it cannot reliably or atomically persist every replacement token set. This becomes a correctness issue if Cognito refresh-token rotation is enabled, and is awkward even when only access/ID tokens change.
Token persistence should be owned by the application, with the library explicitly publishing each newly issued token set before other requests proceed.
3. The public authentication API is synchronous
authenticate() and respond_to_mfa_challenge() perform network operations synchronously:
https://github.com/hardbyte/python-evnex/blob/main/evnex/api.py#L218-L273
Internal API calls correctly wrap Cognito operations in asyncio.to_thread(), but a Home Assistant config flow calling the documented public methods must know to add its own executor/thread boundary. This is easy to get wrong and inconsistent with the otherwise async client.
All public methods that can perform network I/O should be async.
4. Public consumers are coupled to pycognito
MFA is surfaced through pycognito exception classes and challenge.get_tokens():
https://github.com/hardbyte/python-evnex/blob/main/README.md#L62-L102
This leaks the implementation dependency into integrations and makes it harder to change the Cognito implementation later. It also models MFA as a special mode + code call rather than an explicit Cognito challenge containing the challenge name, opaque session, and parameters.
Even if Evnex currently uses only SMS and TOTP, python-evnex should expose its own typed challenge/result API.
5. Authentication retries are coupled to generic request retries
A 401 calls _ensure_valid_token(). If check_token() considers the JWT unexpired, the request fails without forcing one refresh. Conversely, if renewal keeps reporting success, TokenRefreshedError can consume the full five-attempt generic retry budget:
https://github.com/hardbyte/python-evnex/blob/main/evnex/api.py#L302-L310
https://github.com/hardbyte/python-evnex/blob/main/tests/test_auth.py#L162-L197
Authentication recovery should have independent semantics:
- Send with the current valid token.
- On 401, force one single-flight refresh.
- Retry the request once.
- Surface a reauthentication-required error if it still fails.
It should not share the generic transient-network retry counter, particularly for command endpoints that may not be safe to send repeatedly.
6. Authentication is applied separately to every API method
@refresh_token_if_expired is repeated across resource methods. This is easy to omit when adding a new endpoint and mixes transport/auth concerns into each operation.
Authentication and one-time 401 recovery should happen in one central request path or an injected httpx.Auth-like component.
Proposed direction
Introduce a separate async authentication component, while retaining a backwards-compatible convenience constructor initially.
For example:
@dataclass(frozen=True)
class TokenSet:
access_token: str
id_token: str | None
refresh_token: str
expires_at: datetime | None
@dataclass(frozen=True)
class AuthChallenge:
name: str
session: str
parameters: Mapping[str, str]
class EvnexAuth:
async def get_access_token(self) -> str: ...
async def force_refresh(self) -> TokenSet: ...
async def start_authentication(self, username: str, password: str) -> TokenSet | AuthChallenge: ...
async def respond_to_challenge(self, challenge: AuthChallenge, response: str) -> TokenSet | AuthChallenge: ...
The application should be able to supply an async token-update callback or token store:
async def save_tokens(tokens: TokenSet) -> None: ...
Evnex would then accept an auth object and an httpx.AsyncClient, without needing to retain a password itself.
Acceptance criteria
- A client can resume using a refresh token without supplying or retaining a password.
- Every successful authentication/refresh returns or publishes the complete current token set.
- Token updates can be persisted atomically by the caller.
- All public network operations, including initial auth and MFA responses, are async.
- Public exceptions and challenge objects do not expose
pycognitotypes. - Concurrent API calls perform at most one refresh.
- A 401 triggers at most one forced refresh and one request retry.
- Auth retry behaviour is separate from generic transient HTTP retries.
- Authentication is enforced through one central request path rather than per-method decorators.
- Tests cover refresh-token-only startup, token-update persistence, concurrent refresh, MFA challenge round-tripping, and bounded 401 recovery.
This does not need to block the current MFA release. It is a follow-up refactor to make the new support robust for Home Assistant rather than merely functional in a single-process example.
- Ngôn ngữ chính
- Python
- Star
- 13
- Fork
- 8
- Merge trung bình
- 5 giờ 58 phút
- Pull request đã merge (30 ngày)
- 2
Hướng dẫn đóng góp
Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này
Bắt đầu từ đâu
- Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
- Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
- Fork repository và làm thay đổi trên một nhánh.
- Mở pull request có tham chiếu số hiệu của issue.
Issue khác của hardbyte/python-evnex
-
Độ khó 3/5 1-2 ngày Mức phù hợp với người mới 68/100
hardbyte/python-evnex#129 · 2 reaction ·
-
Độ khó 4/5 3-5 ngày Mức phù hợp với người mới 48/100
hardbyte/python-evnex#119 ·
-
Live Load data not available Đang mở
Độ khó 3/5 1-2 ngày Mức phù hợp với người mới 52/100
hardbyte/python-evnex#109 · 3 bình luận ·
Tất cả issue của hardbyte/python-evnex
Issue tương tự
-
货币战争手改优先级配置缺少列表元素类型校验(P3) Đang mở
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 78/100
syfoud/Simulated_Scepter#172 ·
-
A cancelled tests run makes the coverage comment workflow fail and reports it as a red check on main Đang mởarea: ci bug perceived difficulty: 3
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 78/100
Nitjsefnie-Harness-Commons/daedalus#921 · 1 bình luận ·
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 86/100
EleutherAI/lm-evaluation-harness#4207 ·
-
Độ khó 1/5 Dưới một giờ Mức phù hợp với người mới 92/100
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 78/100
ClickHouse/clickhouse-connect#1057 ·