feat(clients): add AuthStrategy pattern for extensible client auth#1285
Open
AtMrun wants to merge 11 commits intochore/remove-deprecated-v2-modulesfrom
Open
feat(clients): add AuthStrategy pattern for extensible client auth#1285AtMrun wants to merge 11 commits intochore/remove-deprecated-v2-modulesfrom
AtMrun wants to merge 11 commits intochore/remove-deprecated-v2-modulesfrom
Conversation
Introduces a protocol-based auth strategy system that bridges typed credentials to client connection parameters, replacing the hardcoded match statement in BaseSQLClient.get_auth_token(). - AuthStrategy protocol with build_url_params, build_connect_args, build_url_query_params, and build_headers methods - Built-in strategies: Basic, Keypair, OAuth, ApiKey, BearerToken, IAM - BaseSQLClient.AUTH_STRATEGIES class var + load_with_credential() method - AsyncBaseSQLClient.load_with_credential() async counterpart - Full backward compat: empty AUTH_STRATEGIES falls back to legacy path
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Introduces AuthenticatedClient between ClientInterface and the protocol-specific clients (BaseSQLClient, BaseClient). Shared auth logic (_resolve_strategy, _build_url, add_url_params) now lives in one place and is inherited by all client types. - BaseSQLClient and BaseClient both extend AuthenticatedClient - AUTH_STRATEGIES, credentials, strategy resolution moved to parent - BaseSQLClient.add_connection_params kept as backward-compat alias - BaseClient gains auth strategy support for free (needed by Phase 4) - load_with_credential() simplified to use shared helpers
get_iam_user_token, get_iam_role_token, get_auth_token, and get_sqlalchemy_connection_string are now deprecated in favor of AUTH_STRATEGIES + load_with_credential(). They remain for backward compat until existing connectors migrate to typed credentials.
Removes get_iam_user_token, get_iam_role_token, get_auth_token, and get_sqlalchemy_connection_string — all replaced by AUTH_STRATEGIES + load_with_credential(). The load(dict) path now builds connection strings inline without the legacy dispatch chain. Deleted ~170 lines of code. The logic lives in auth strategies (IamUserAuthStrategy, IamRoleAuthStrategy, BasicAuthStrategy, etc.) and AuthenticatedClient._build_url().
…entInterface
Replaces the 3-level hierarchy (ClientInterface → AuthenticatedClient
→ BaseSQLClient/BaseClient) with a flat one:
BaseClient (root)
├── BaseSQLClient — SQLAlchemy
├── BaseHTTPClient — httpx (renamed from BaseClient)
└── AzureClient — Azure SDK
- ClientInterface ABC deleted (was just load + close stubs)
- AuthenticatedClient merged into BaseClient in clients/__init__.py
- Current BaseClient renamed to BaseHTTPClient in clients/base.py
- AzureClient now extends BaseClient (gains AUTH_STRATEGIES for free)
- All 93 connectors will extend one of these three
…PClient) - BaseClient → Client (root, in clients/client.py) - BaseSQLClient → SQLClient - AsyncBaseSQLClient → AsyncSQLClient - BaseHTTPClient → HTTPClient Backward-compat aliases preserved: - clients/__init__.py: BaseClient = Client, ClientInterface = Client - clients/sql.py: BaseSQLClient = SQLClient, AsyncBaseSQLClient = AsyncSQLClient - clients/base.py: BaseHTTPClient = HTTPClient, BaseClient = Client
File now matches its contents — HTTPClient lives in http.py.
clients/
__init__.py ← re-exports Client + backward-compat aliases
client.py ← Client (root)
http.py ← HTTPClient (httpx transport)
sql.py ← SQLClient (SQLAlchemy transport)
auth.py ← AuthStrategy protocol
auth_strategies/
azure/ ← AzureClient
…zure/ → azure.py - AzureClient now uses AUTH_STRATEGIES with ServicePrincipalAuthStrategy - Deleted AzureAuthProvider (~240 lines) — replaced by 37-line strategy - Added ServicePrincipalCredential to typed credentials + registry - Collapsed azure/ folder (3 files) into single azure.py - load(dict) legacy path parses raw dict into ServicePrincipalCredential then delegates to load_with_credential() Net: -319 lines. All auth now goes through the strategy pattern.
Remove BaseClient, ClientInterface, BaseSQLClient, AsyncBaseSQLClient, BaseHTTPClient aliases. Clean names only: Client, SQLClient, AsyncSQLClient, HTTPClient.
…RATEGIES - SQLClient renamed from BaseSQLClient - PostgresClient uses AUTH_STRATEGIES with BasicAuthStrategy - Shows the pattern connector authors will follow
…ar, assert→TypeError
- Remove mutable class-level `credentials` dict from Client; assign only in __init__
- Remove stale `resolved_credentials` class-level dict from SQLClient
- Fix mutable default arg `credentials={}` → `credentials=None` in SQLClient.__init__
- Annotate AUTH_STRATEGIES as ClassVar to signal it must not be mutated at runtime
- Replace all `assert isinstance(...)` in auth strategies with proper
`if not isinstance(...): raise TypeError(...)` so type checks survive `-O`
6 tasks
Member
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AuthStrategyprotocol (clients/auth.py) that maps typedCredentialinstances to client connection parameters (URL params, connect_args, query params, HTTP headers)BasicAuthStrategy,KeypairAuthStrategy,OAuthAuthStrategy,ApiKeyAuthStrategy,BearerTokenAuthStrategy,IamUserAuthStrategy,IamRoleAuthStrategy,ServicePrincipalAuthStrategyClientInterfaceABC → concreteClientbase class withAUTH_STRATEGIES(typed asClassVar) and_resolve_strategy/_build_urlhelpersBaseSQLClient→SQLClient,AsyncBaseSQLClient→AsyncSQLClient,BaseClient→HTTPClient,base.py→http.pyload_with_credential(credential, **connection_params)on bothSQLClientandAsyncSQLClientas the strategy-based alternative toload(dict)AzureClienttoAUTH_STRATEGIES, collapsesazure/package → singleazure.pyServicePrincipalCredentialto the credential type registryHardening (latest commit)
credentials/resolved_credentialsdicts — assigned only in__init__AUTH_STRATEGIESannotated asClassVarto prevent accidental runtime mutationassert isinstance(...)in strategies replaced withraise TypeError(...)so checks survive Python-OWhat a multi-auth client looks like
Three auth methods. Zero branching logic.
Test plan
test_auth_strategies.py)Clientbase class (test_authenticated_client.py)load_with_credentialon both sync and async clients (test_sql_auth_strategies.py)load(dict)still works unchanged🤖 Generated with Claude Code