MPT-21532 Add account-scoped authentication provider#350
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughIntroduces ChangesAccount-Scoped Authentication
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Add AccountScopedAuthentication, an httpx.Auth provider that fetches account-scoped installation tokens and shares them across provider and client instances through a process-wide cache keyed by (secret, account_id). Token refreshes are serialized per account with double-checked locking, so concurrent callers trigger at most one token request; refresh happens proactively before the JWT exp and reactively on 401, and expired cache entries are evicted on write. Extract the shared token-fetch machinery into a new InstallationTokenAuthentication base and reparent ExtensionFrameworkAuthentication onto it with no behavior change. This brings the client to feature parity with the extension SDK's AccountTokenProvider and AccountScopedAsyncHTTPClient. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
albertsola
force-pushed
the
feature/MPT-21532-authentication-providers
branch
from
ea3a0a4 to
dc9d992
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mpt_api_client/auth/account_scoped.py`:
- Around line 54-69: Add validation in the __init__ method of the
AccountScopedTokenProvider class to ensure the min_remaining_validity_seconds
parameter is non-negative. Perform this check after the super().__init__ call
but before assigning the value to self._min_remaining_validity_seconds. Raise a
ValueError if a negative value is provided to fail fast and prevent expired
tokens from being incorrectly treated as reusable in the expiry threshold
calculations downstream.
- Around line 152-171: The _evict_expired() method iterates over the shared
_token_cache dictionary while the _store() method (and potentially other
threads) concurrently mutate it from different accounts, causing
dictionary-size-change errors during iteration. The existing per-account locks
protect only individual cache keys, not the global cache itself. Add a global
lock (separate from the per-account sync/async locks) to serialize access to
_token_cache during the iteration in _evict_expired(), and acquire this same
lock in _store() before modifying _token_cache to ensure thread-safe concurrent
access across different account keys.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 9491c8e5-c077-488c-b87f-408a44180711
📒 Files selected for processing (8)
docs/usage.mdmpt_api_client/__init__.pympt_api_client/auth/__init__.pympt_api_client/auth/account_scoped.pympt_api_client/auth/base.pympt_api_client/auth/extension_framework.pypyproject.tomltests/unit/auth/test_account_scoped.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
softwareone-platform/mpt-extension-skills(manual)
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🧰 Additional context used
📓 Path-based instructions (6)
**/*
⚙️ CodeRabbit configuration file
**/*: For each subsequent commit in this PR, explicitly verify if previous review comments have been resolved
Files:
mpt_api_client/__init__.pympt_api_client/auth/__init__.pypyproject.tomldocs/usage.mdmpt_api_client/auth/base.pytests/unit/auth/test_account_scoped.pympt_api_client/auth/account_scoped.pympt_api_client/auth/extension_framework.py
**
⚙️ CodeRabbit configuration file
**: # AGENTS.mdWorking protocol for any task in this repository:
- Identify the task type and select only the local repository files that are relevant to that task.
- Read only those relevant local files before making changes.
- If any selected local file references shared standards or shared operational guidance that are relevant to the same task, read those shared documents too before proceeding.
- Treat repository-local documents as repository-specific additions, restrictions, or overrides to shared guidance.
- If a repository-local rule conflicts with a shared rule, the local repository rule takes precedence.
Python API client for the SoftwareONE Marketplace Platform (MPT) API. Provides synchronous
(MPTClient) and asynchronous (AsyncMPTClient) clients built on httpx, with typed
resource services, mixin-based HTTP operations, and an RQL query builder.Documentation Reading Order
When applicable, read the repository documentation in this order:
README.md— repository overview, quick start, and documentation mapdocs/usage.md— installation, configuration, Python usage examples, and supported Docker-based commandsdocs/architecture.md— layered architecture, directory structure, and key abstractionsdocs/local-development.md— Docker-only setup and execution modeldocs/testing.md— repository-specific testing strategy and command mappingdocs/contributing.md— repository-specific workflow and links to shared standardsdocs/documentation.md— repository-specific documentation rulesdocs/unit_tests.md— unit test structure and guidancedocs/e2e_tests.md— end-to-end test setup and executionThen inspect the code paths relevant to the task:
mpt_api_client/mpt_client.py— public sync and async client entry pointsmpt_api_client/http/— HTTP clients, services, query state, and reusable mixinsmpt_api_client/resources/— domain resource groups such as catalog, commerce, billing, and integrati...
Files:
mpt_api_client/__init__.pympt_api_client/auth/__init__.pypyproject.tomldocs/usage.mdmpt_api_client/auth/base.pytests/unit/auth/test_account_scoped.pympt_api_client/auth/account_scoped.pympt_api_client/auth/extension_framework.py
docs/**/*.md
📄 CodeRabbit inference engine (docs/contributing.md)
When repository behavior changes, update the narrowest relevant document under
docs/
Files:
docs/usage.md
docs/*.md
📄 CodeRabbit inference engine (docs/documentation.md)
docs/*.md: Topic-specific documentation must live in the matching file underdocs/directory
Shared engineering rules must be linked frommpt-extension-skillsinstead of copied into this repository
Files:
docs/usage.md
docs/usage.md
📄 CodeRabbit inference engine (docs/documentation.md)
docs/usage.mdis the source of truth for installation, configuration, examples, and supported command entry points
Files:
docs/usage.md
docs/**
📄 CodeRabbit inference engine (AGENTS.md)
docs/**: Put topic-specific documentation underdocs/directory instead of expandingREADME.md
Link shared engineering rules frommpt-extension-skillsinstead of duplicating them locally in documentation
Files:
docs/usage.md
⚙️ CodeRabbit configuration file
docs/**: Review documentation changes againstdocs/documentation.mdand the linked repository'sstandards/documentation.md.
Use those documents as the source of truth for structure, topic boundaries, navigation updates, and when to link shared rules instead of copying them.
Files:
docs/usage.md
⚙️ CodeRabbit configuration file
docs/**: # ArchitectureThis document describes the internal architecture of
mpt-api-python-client.Overview
mpt-api-python-clientis a Python API client that provides a typed, fluent interface for the
SoftwareONE Marketplace Platform (MPT) REST API. It supports both synchronous and asynchronous
usage and is built on top of httpx.API Reference: The full upstream API contract is described by the
MPT OpenAPI Spec.
The client mirrors this spec's resource structure.The client exposes every MPT API domain (catalog, commerce, billing, etc.) as a resource group,
where each resource is a service object composed from reusable HTTP operation mixins.Directory Structure
mpt_api_client/ ├── __init__.py # Public API: MPTClient, AsyncMPTClient, RQLQuery ├── mpt_client.py # Client entry points ├── constants.py # Shared constants (content types) ├── exceptions.py # Error hierarchy (MPTError, MPTHttpError, MPTAPIError) │ ├── http/ # HTTP transport layer │ ├── client.py # Sync HTTPClient (httpx.Client) │ ├── async_client.py # Async AsyncHTTPClient (httpx.AsyncClient) │ ├── base_service.py # ServiceBase — shared service logic │ ├── service.py # Service — sync service (extends ServiceBase) │ ├── async_service.py # AsyncService — async service (extends ServiceBase) │ ├── query_state.py # Query parameter accumulation │ ├── client_utils.py # URL validation helpers │ ├── types.py # Type aliases (Response, HeaderTypes, etc.) │ └── mixins/ # Composable HTTP operation mixins │ ├── collection_mixin.py │ ├── create_mixin.py │ ├── create_file_mixin.py │ ├── update_mixin.py │ ├── update_file_mixin.py │ ├── delete_mixin.py │ ├── get_mixin.py │ ├── enable_mixin.py │ ├── disable_mixin...
Files:
docs/usage.md
🧠 Learnings (5)
📚 Learning: 2026-02-17T10:04:00.873Z
Learnt from: jentyk
Repo: softwareone-platform/mpt-api-python-client PR: 210
File: mpt_api_client/rql/query_builder.py:18-18
Timestamp: 2026-02-17T10:04:00.873Z
Learning: In this repository, Ruff and flake8 with wemake-python-styleguide are used together. Do not remove WPS* noqa directives (e.g., WPS231) even if Ruff flags them as unknown in RUF102. Keep the directives to satisfy flake8 rules; ensure tooling configuration accounts for both linters to avoid false positives.
Applied to files:
mpt_api_client/__init__.pympt_api_client/auth/__init__.pympt_api_client/auth/base.pympt_api_client/auth/account_scoped.pympt_api_client/auth/extension_framework.py
📚 Learning: 2026-04-16T13:00:41.320Z
Learnt from: jentyk
Repo: softwareone-platform/mpt-api-python-client PR: 303
File: tests/e2e/helpdesk/chats/participants/conftest.py:25-31
Timestamp: 2026-04-16T13:00:41.320Z
Learning: In mpt-api-python-client, do not treat list-wrapped arguments to CreateMixin.create() / AsyncCreateMixin.create() as an error. ResourceData is intentionally typed as Resource | list[Resource] (see mpt_api_client/models/model.py), so create() should accept either a single resource dict or a list of resource dicts (e.g., create([chat_participant_data])) to perform a batch create and return a ModelCollection. Therefore, reviewers should only flag list-wrapped create() arguments when there is evidence they violate the expected API contract beyond this documented batch-create behavior.
Applied to files:
mpt_api_client/__init__.pympt_api_client/auth/__init__.pympt_api_client/auth/base.pytests/unit/auth/test_account_scoped.pympt_api_client/auth/account_scoped.pympt_api_client/auth/extension_framework.py
📚 Learning: 2025-12-12T15:02:20.732Z
Learnt from: robcsegal
Repo: softwareone-platform/mpt-api-python-client PR: 160
File: tests/e2e/commerce/agreement/attachment/test_async_agreement_attachment.py:55-58
Timestamp: 2025-12-12T15:02:20.732Z
Learning: In pytest with pytest-asyncio, if a test function uses async fixtures but contains no await, declare the test function as def (synchronous) instead of async def. Pytest-asyncio will resolve the async fixtures automatically; this avoids linter complaints about unnecessary async functions. This pattern applies to any test file under the tests/ directory that uses such fixtures.
Applied to files:
tests/unit/auth/test_account_scoped.py
📚 Learning: 2026-04-02T09:35:03.825Z
Learnt from: jentyk
Repo: softwareone-platform/mpt-api-python-client PR: 269
File: tests/e2e/helpdesk/chats/links/test_sync_links.py:18-18
Timestamp: 2026-04-02T09:35:03.825Z
Learning: In this repository’s test suite, flake8-aaa/flake8-aaa codes use short two-digit suffixes (e.g., `# noqa: AAA01`), not three-digit variants like `AAA001`. If you see `# noqa: AAA01` in a test file (e.g., when the Act step is performed via a pytest fixture rather than inline code), treat it as valid and intentional—do not flag it as an invalid/no-longer-needed noqa, and do not suggest removing it even if Ruff reports `RUF102`, since `AAA` is configured under `tool.ruff.lint.external` and these noqa directives are expected to be preserved.
Applied to files:
tests/unit/auth/test_account_scoped.py
📚 Learning: 2026-02-02T13:05:41.144Z
Learnt from: albertsola
Repo: softwareone-platform/mpt-api-python-client PR: 201
File: tests/unit/resources/accounts/mixins/test_activatable_mixin.py:132-139
Timestamp: 2026-02-02T13:05:41.144Z
Learning: In the mpt-api-python-client repository, tests are configured to use pytest asyncio mode auto, which auto-detects async test functions and runs them without requiring pytest.mark.asyncio. Reviewers should rely on this behavior for all Python test files under tests/, and avoid adding unnecessary asyncio markers in async tests. Ensure test files in tests/ adhere to this convention unless a specific test requires an explicit marker.
Applied to files:
tests/unit/auth/test_account_scoped.py
🪛 ast-grep (0.43.0)
tests/unit/auth/test_account_scoped.py
[info] 22-22: use jsonify instead of json.dumps for JSON output
Context: json.dumps(payload)
Note: Security best practice.
(use-jsonify)
mpt_api_client/auth/account_scoped.py
[warning] 83-83: Setting a response header name or value from request-derived input allows HTTP response splitting / header injection (CRLF); strip CR/LF or validate the value.
Context: request.headers["Authorization"] = f"Bearer {token}"
Note: [CWE-113] Improper Neutralization of CRLF Sequences in HTTP Headers ('HTTP Response Splitting'). OWASP A03:2021 Injection.
(http-response-splitting-python)
[warning] 87-87: Setting a response header name or value from request-derived input allows HTTP response splitting / header injection (CRLF); strip CR/LF or validate the value.
Context: request.headers["Authorization"] = f"Bearer {self._token_sync(rejected)}"
Note: [CWE-113] Improper Neutralization of CRLF Sequences in HTTP Headers ('HTTP Response Splitting'). OWASP A03:2021 Injection.
(http-response-splitting-python)
[warning] 96-96: Setting a response header name or value from request-derived input allows HTTP response splitting / header injection (CRLF); strip CR/LF or validate the value.
Context: request.headers["Authorization"] = f"Bearer {token}"
Note: [CWE-113] Improper Neutralization of CRLF Sequences in HTTP Headers ('HTTP Response Splitting'). OWASP A03:2021 Injection.
(http-response-splitting-python)
[warning] 101-101: Setting a response header name or value from request-derived input allows HTTP response splitting / header injection (CRLF); strip CR/LF or validate the value.
Context: request.headers["Authorization"] = f"Bearer {refreshed}"
Note: [CWE-113] Improper Neutralization of CRLF Sequences in HTTP Headers ('HTTP Response Splitting'). OWASP A03:2021 Injection.
(http-response-splitting-python)
🪛 Ruff (0.15.17)
mpt_api_client/auth/__init__.py
[warning] 9-9: Invalid rule code in # noqa: WPS410
Add non-Ruff rule codes to the lint.external configuration option
(RUF102)
tests/unit/auth/test_account_scoped.py
[error] 15-15: Possible hardcoded password assigned to: "SECRET"
(S105)
mpt_api_client/auth/account_scoped.py
[warning] 33-33: Invalid rule code in # noqa: WPS214
Add non-Ruff rule codes to the lint.external configuration option
(RUF102)
🔇 Additional comments (13)
docs/usage.md (2)
51-56: Documentation accurately describesAccountScopedAuthenticationsemantics.The description correctly captures the shared token cache (keyed by
secret, account_id), serialized per-account refresh with double-checked locking, proactive refresh with configurable leeway (default 60s), reactive 401 handling, and the use case for shared/concurrent scenarios. Verified against implementation inmpt_api_client/auth/account_scoped.pyand test coverage intests/unit/auth/test_account_scoped.py.
94-106: Code example follows existing patterns and accurately demonstrates usage.The example correctly imports
AccountScopedAuthenticationfrom the public API, passes the requiredsecretandaccount_idconstructor parameters, and integrates withMPTClient.from_configin the same pattern as prior examples. Placement afterExtensionFrameworkAuthenticationexamples provides a clear narrative progression from single-instance to shared-cache approaches. Consistent formatting and style.mpt_api_client/auth/base.py (3)
7-27: LGTM!
59-87: LGTM!
88-130: LGTM!mpt_api_client/auth/extension_framework.py (3)
14-17: LGTM!
20-54: LGTM!
84-106: LGTM!mpt_api_client/auth/account_scoped.py (1)
20-53: LGTM!Also applies to: 71-151
mpt_api_client/auth/__init__.py (1)
1-15: LGTM!mpt_api_client/__init__.py (1)
1-18: LGTM!tests/unit/auth/test_account_scoped.py (1)
1-268: LGTM!pyproject.toml (1)
160-160: Configuration already implements the recommended fix.The Ruff external rule-code configuration for WPS and AAA is already present in
pyproject.toml(line 244:external = ["AAA", "WPS"]). No changes are needed.> Likely an incorrect or invalid review comment.
| def __init__( | ||
| self, | ||
| secret: str, | ||
| account_id: str, | ||
| min_remaining_validity_seconds: int = DEFAULT_TOKEN_VALIDITY_LEEWAY_SECONDS, | ||
| ) -> None: | ||
| """Initialize the provider. | ||
|
|
||
| Args: | ||
| secret: Extension secret used to authenticate token requests. | ||
| account_id: Account the requested token is scoped to. | ||
| min_remaining_validity_seconds: Proactive refresh leeway before the JWT ``exp``. | ||
| """ | ||
| super().__init__(secret) | ||
| self._account_id = account_id | ||
| self._min_remaining_validity_seconds = min_remaining_validity_seconds |
There was a problem hiding this comment.
Validate non-negative refresh leeway at construction.
min_remaining_validity_seconds is used directly in expiry threshold math (Line 149). If a negative value is passed, expired tokens can be treated as reusable. Add a constructor guard so invalid input fails fast.
Proposed fix
def __init__(
self,
secret: str,
account_id: str,
min_remaining_validity_seconds: int = DEFAULT_TOKEN_VALIDITY_LEEWAY_SECONDS,
) -> None:
@@
"""
super().__init__(secret)
+ if min_remaining_validity_seconds < 0:
+ raise ValueError("min_remaining_validity_seconds must be >= 0")
self._account_id = account_id
self._min_remaining_validity_seconds = min_remaining_validity_secondsAlso applies to: 149-150
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mpt_api_client/auth/account_scoped.py` around lines 54 - 69, Add validation
in the __init__ method of the AccountScopedTokenProvider class to ensure the
min_remaining_validity_seconds parameter is non-negative. Perform this check
after the super().__init__ call but before assigning the value to
self._min_remaining_validity_seconds. Raise a ValueError if a negative value is
provided to fail fast and prevent expired tokens from being incorrectly treated
as reusable in the expiry threshold calculations downstream.
| def _store(self, token: str | None) -> str: | ||
| """Cache a freshly fetched token, evicting expired entries, and return it.""" | ||
| if not token: | ||
| raise MPTError("Installations token endpoint returned an empty token.") | ||
| self._token_cache[self._cache_key] = _CachedToken(token, self._read_expiry(token)) | ||
| self._evict_expired() | ||
| return token | ||
|
|
||
| def _evict_expired(self) -> None: | ||
| """Drop cache entries (and their locks) whose tokens have already expired.""" | ||
| now = dt.datetime.now(dt.UTC) | ||
| expired_keys = [ | ||
| key | ||
| for key, cached in self._token_cache.items() | ||
| if cached.expires_at is not None and cached.expires_at <= now | ||
| ] | ||
| for key in expired_keys: | ||
| self._token_cache.pop(key, None) | ||
| self._sync_locks.pop(key, None) | ||
| self._async_locks.pop(key, None) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, locate and read the AccountScopedAuthentication class
find . -name "account_scoped.py" -type f | head -20Repository: softwareone-platform/mpt-api-python-client
Length of output: 126
🏁 Script executed:
#!/bin/bash
# Read the account_scoped.py file to understand the implementation
cat -n mpt_api_client/auth/account_scoped.pyRepository: softwareone-platform/mpt-api-python-client
Length of output: 8785
🏁 Script executed:
#!/bin/bash
# Search for all references to _token_cache, _sync_locks, _async_locks across the codebase
rg "(_token_cache|_sync_locks|_async_locks)" -n --context 2Repository: softwareone-platform/mpt-api-python-client
Length of output: 4855
🏁 Script executed:
#!/bin/bash
# Look for threading patterns and Lock usage in the auth module
rg "threading\.|Lock\(\)|asyncio\.Lock\(\)" mpt_api_client/auth/ -n --context 1Repository: softwareone-platform/mpt-api-python-client
Length of output: 855
🏁 Script executed:
#!/bin/bash
# Check if the _cache_guard fix has been applied
grep -n "_cache_guard" mpt_api_client/auth/account_scoped.pyRepository: softwareone-platform/mpt-api-python-client
Length of output: 68
🏁 Script executed:
#!/bin/bash
# Look for any test coverage of concurrent account access
grep -r "concurrent\|thread\|async.*test" tests/unit/auth/test_account_scoped.py -iRepository: softwareone-platform/mpt-api-python-client
Length of output: 217
🏁 Script executed:
#!/bin/bash
# Check what the concurrency test actually tests
sed -n '210,280p' tests/unit/auth/test_account_scoped.pyRepository: softwareone-platform/mpt-api-python-client
Length of output: 2213
🏁 Script executed:
#!/bin/bash
# Check if there are any tests mixing different accounts with concurrent access
grep -A 30 "test_serialized_refresh" tests/unit/auth/test_account_scoped.pyRepository: softwareone-platform/mpt-api-python-client
Length of output: 1320
🏁 Script executed:
#!/bin/bash
# Check if there are any tests that exercise multiple accounts concurrently
grep -B 5 -A 15 "ACC-OLD\|ACC-NEW" tests/unit/auth/test_account_scoped.py | head -40Repository: softwareone-platform/mpt-api-python-client
Length of output: 715
🏁 Script executed:
#!/bin/bash
# Look for documentation or usage patterns showing multiple accounts
grep -r "AccountScopedAuthentication" --include="*.py" --include="*.md" | grep -v "test_" | grep -v ".pyc"Repository: softwareone-platform/mpt-api-python-client
Length of output: 752
🏁 Script executed:
#!/bin/bash
# Check if the concurrency test uses multiple accounts
grep -B 5 -A 20 "test_serialized_refresh_fetches_token_once_under_concurrency" tests/unit/auth/test_account_scoped.pyRepository: softwareone-platform/mpt-api-python-client
Length of output: 1065
🏁 Script executed:
#!/bin/bash
# Look at the full test file structure to understand concurrency coverage
grep "^def test_\|^async def test_" tests/unit/auth/test_account_scoped.pyRepository: softwareone-platform/mpt-api-python-client
Length of output: 670
🏁 Script executed:
#!/bin/bash
# Check the full test_different_accounts_fetch_separate_tokens to see if it's concurrent
grep -B 5 -A 25 "test_different_accounts_fetch_separate_tokens" tests/unit/auth/test_account_scoped.pyRepository: softwareone-platform/mpt-api-python-client
Length of output: 1136
🏁 Script executed:
#!/bin/bash
# Run the test suite to verify the current state of the code
# First, let's check if there's a way to run tests and see if they would catch this race
grep -r "pytest\|test" docs/testing.md | head -10Repository: softwareone-platform/mpt-api-python-client
Length of output: 715
Serialize global cache eviction to prevent concurrent-dict runtime failures.
Line 165 iterates over AccountScopedAuthentication._token_cache while lines 156/169 mutate that same shared dict from request threads handling different account keys. Per-account locks (one per cache key) do not protect this global iteration, so concurrent writes to _token_cache from different accounts can crash with dictionary-size-change errors during iteration.
The test test_different_accounts_fetch_separate_tokens() only exercises sequential access across accounts; test_serialized_refresh_fetches_token_once_under_concurrency() only covers concurrency within a single account (protected by its per-account lock). No test validates concurrent access across multiple accounts, where the race window exists.
Proposed fix
class AccountScopedAuthentication(InstallationTokenAuthentication): # noqa: WPS214
@@
_token_cache: ClassVar[dict[CacheKey, _CachedToken]] = {}
_sync_locks: ClassVar[dict[CacheKey, threading.Lock]] = {}
_async_locks: ClassVar[dict[CacheKey, asyncio.Lock]] = {}
+ _cache_guard: ClassVar[threading.Lock] = threading.Lock()
@@
def clear_cache(cls) -> None:
"""Clear all cached account tokens and refresh locks."""
- cls._token_cache.clear()
- cls._sync_locks.clear()
- cls._async_locks.clear()
+ with cls._cache_guard:
+ cls._token_cache.clear()
+ cls._sync_locks.clear()
+ cls._async_locks.clear()
@@
def _store(self, token: str | None) -> str:
"""Cache a freshly fetched token, evicting expired entries, and return it."""
if not token:
raise MPTError("Installations token endpoint returned an empty token.")
- self._token_cache[self._cache_key] = _CachedToken(token, self._read_expiry(token))
- self._evict_expired()
+ with self._cache_guard:
+ self._token_cache[self._cache_key] = _CachedToken(token, self._read_expiry(token))
+ self._evict_expired()
return token🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mpt_api_client/auth/account_scoped.py` around lines 152 - 171, The
_evict_expired() method iterates over the shared _token_cache dictionary while
the _store() method (and potentially other threads) concurrently mutate it from
different accounts, causing dictionary-size-change errors during iteration. The
existing per-account locks protect only individual cache keys, not the global
cache itself. Add a global lock (separate from the per-account sync/async locks)
to serialize access to _token_cache during the iteration in _evict_expired(),
and acquire this same lock in _store() before modifying _token_cache to ensure
thread-safe concurrent access across different account keys.
|
1 participant
🤖 AI-generated PR — Please review carefully.
What
Adds
AccountScopedAuthentication, an authentication provider that fetches account-scopedinstallation tokens and shares them across provider/client instances through a process-wide
cache keyed by
(secret, account_id).threading.Lockfor sync,asyncio.Lockfor async): concurrent callers for the same account trigger at most onetoken request.
exp(default 60s leeway) and reactive refreshon
401(a server-rejected token forces a re-fetch even when still un-expired).clear_cache()classmethod.Why
Brings the client to feature parity with the extension SDK's
AccountTokenProvider/AccountScopedAsyncHTTPClient, so many clients sharing an account scope — or many concurrentrequests — reuse a single token instead of each fetching their own.
How
InstallationTokenAuthenticationbase and reparented
ExtensionFrameworkAuthenticationonto it with no behavior change.mpt_api_client/auth/account_scoped.py; exported frommpt_api_clientand
mpt_api_client.auth.docs/usage.md.Testing
pytest tests/unit— all passing, incl. newtests/unit/auth/test_account_scoped.py(12 cases: shared cache, concurrent serialized refresh, proactive/reactive refresh,
repeated-401 surfacing, eviction).
ruff format --check,ruff check,flake8,mypy,uv lock --check— clean.Closes MPT-21532
AccountScopedAuthentication, a new authentication provider for account-scoped installation tokens cached process-wide by(secret, account_id)pairs, enabling multiple client instances to share tokensthreading.Lock(sync) andasyncio.Lock(async) with double-checked locking to ensure concurrent requests trigger at most one token fetch401unauthorized responsesclear_cache()classmethod for manual cache clearingInstallationTokenAuthenticationbase classExtensionFrameworkAuthenticationto inherit fromInstallationTokenAuthenticationwithout behavior changesAccountScopedAuthenticationfrom bothmpt_api_clientandmpt_api_client.authmodulesdocs/usage.mddemonstratingAccountScopedAuthenticationconfiguration