feat: Proxy authentication mode with mock token - #283
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
It changes security-sensitive authentication behavior and currently has a proxy-auth scope/TokenCredential mismatch that needs correction and team review.
Pull request overview
Adds a proxy authentication mode (gated by FAB_PROXY_AUTH_ENABLED) to let the CLI emit a placeholder Bearer mockToken that downstream proxies can replace, while bypassing MSAL/JWT validation and blocking interactive auth flows.
Changes:
- Introduces proxy-auth detection and placeholder token/expiry behavior in
FabAuth, including skipping env-var auth validation/loading when enabled. - Updates
fab authcommands to block login/logout with actionable errors and to returnproxy authentication modefromfab auth status. - Adds/extends tests to validate proxy-auth behavior across auth, API requests, and Azure TokenCredential bridging.
File summaries
| File | Description |
|---|---|
| tests/test_core/test_fab_msal_bridge_azure_cli.py | Adds a proxy-auth test for MsalTokenCredential behavior. |
| tests/test_core/test_fab_auth.py | Adds proxy-auth mode unit tests and adjusts auth singleton test setup/reset. |
| tests/test_core/test_fab_api_client.py | Verifies API requests send Bearer mockToken when proxy-auth is enabled. |
| tests/test_commands/test_auth.py | Ensures auth login/logout/status behave correctly in proxy-auth mode. |
| tests/conftest.py | Ensures proxy-auth env var is cleaned up in Azure CLI auth fixture. |
| src/fabric_cli/errors/auth.py | Adds new error messages for login/logout being unavailable in proxy-auth mode. |
| src/fabric_cli/core/fab_auth.py | Implements proxy-auth mode switch, placeholder token, and JWT/MSAL bypass behavior. |
| src/fabric_cli/commands/auth/fab_auth.py | Blocks login/logout and adjusts auth status output for proxy-auth mode. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| def test_bridge_returns_access_token_in_proxy_auth_mode( | ||
| self, monkeypatch, azure_cli_auth_fixture | ||
| ): | ||
| """Proxy auth placeholders should satisfy the TokenCredential contract.""" | ||
| monkeypatch.setenv("FAB_PROXY_AUTH_ENABLED", "true") | ||
| auth = FabAuth() | ||
|
|
||
| credential = MsalTokenCredential(auth) | ||
| result = credential.get_token(con.SCOPE_FABRIC_DEFAULT[0]) | ||
|
|
Co-authored-by: ayeshurun <98805507+ayeshurun@users.noreply.github.com>
Co-authored-by: ayeshurun <98805507+ayeshurun@users.noreply.github.com>
| from fabric_cli.utils.fab_secure_io import restrict_existing_file | ||
|
|
||
| try: | ||
| if self.is_proxy_auth_mode(): |
There was a problem hiding this comment.
tarostok do we want/need to validate if env vars tokens exists and if one of env vars tokens exists & proxy mode env var exists, to raise an error?
There was a problem hiding this comment.
🔵 Needs a closer look
A newly added test mutates FabAuth singleton state (_auth_info) via direct assignment without teardown-safe patching, which can leak state across tests and cause order-dependent failures.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
tests/test_core/test_fab_api_client.py:358
FabAuthis a process-wide singleton, so directly assigningauth._auth_info = {}here can leak state into other tests (this module doesn’t resetFabAuthbetween tests). Usemonkeypatch.setattr(orauth.logout()) so the mutation is automatically reverted at test teardown and stays isolated.
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
It adds a new authentication bypass mode in security-sensitive code paths and needs final human review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
tests/test_core/test_fab_auth.py:39
- Because FabAuth is a process-wide singleton, using monkeypatch.setattr(...) for mutable state like _auth_info/app/_azure_cli_credential/aad_public_key will restore whatever state existed before the test at teardown (potentially reintroducing dirty/global state for later tests). Prefer directly resetting these attributes (no restore) while keeping monkeypatch only for per-test file paths.
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Lite
📥 Pull Request
Adds an explicit proxy authentication mode controlled by
FAB_PROXY_AUTH_ENABLED=trueor1.When enabled, the CLI:
mockTokenfor Fabric, OneLake, Azure, and custom scopes.TokenCredentialcompatibility.proxy authentication modefromfab auth status.Bearer mockTokenthrough the API client for downstream proxy replacement.This enables environments where an authentication proxy replaces the placeholder
token with the appropriate credential.
No additional dependencies are required.
Tests cover mode detection, authentication precedence, supported scopes, API
headers, JWT bypass, auth commands, and the MSAL credential bridge.