feat: Add Azure CLI auth source

[shirasassoon]

• Resolves [FEATURE] Support Azure Cli credential #50

Summary

Add azure_cli as a new identity type that delegates token acquisition to Azure CLI via azure-identity's AzureCliCredential. This allows tools calling fab to reuse an existing az login session instead of requiring a separate interactive fab auth login.

Design Decisions

Following security review, the implementation was simplified:

• No drift detection — the user consciously manages their az login session. If identity changes, Fabric APIs naturally reject with 401/403.
• No client-side token cache — AzureCliCredential handles caching internally via Azure CLI's MSAL cache.
• No --tenant flag for Azure CLI mode (P0) — tenant is derived from the active az login session.
• No rollback on login failure — consistent with MSAL auth flows.

Changes

Core auth (src/fabric_cli/core/fab_auth.py)

• Add azure_cli to AUTH_KEYS identity type allowlist
• Add _acquire_token_from_azure_cli() using singleton AzureCliCredential
• One-time tenant extraction from JWT on first token call (stored for context)
• Graceful error handling: CredentialUnavailableError → clear "not installed/not logged in" message

Command handler (src/fabric_cli/commands/auth/fab_auth.py)

• Add --azure-cli flag to fab auth login
• Add "Azure CLI (existing 'az login' session)" option to interactive login menu
• Login flow: set_access_mode("azure_cli") → _acquire_default_access_tokens() (3 scopes) → set tenant context
• Consistent with MSAL login flow (no success message, no rollback)

Error handling (src/fabric_cli/errors/auth.py)

• azure_cli_not_available — Azure CLI not installed or not logged in
• azure_cli_auth_failed — authentication failure
• azure_cli_token_acquisition_failed — token acquisition failure with scope info

Documentation

• docs/commands/auth/index.md — --azure-cli flag in command reference
• docs/examples/auth_examples.md — Azure CLI auth examples

Dependencies

• Add azure-identity>=1.15.0

Tests

28 automated tests across 3 files:

File	Count	Coverage
test_fab_auth_azure_cli.py	24	Core: token acquisition, singleton, scopes, lifecycle, JWT decode, auth isolation
test_fab_msal_bridge_azure_cli.py	2	Bridge: MsalTokenCredential dispatch with azure_cli identity
test_auth.py (command)	4	Command: flag login, tenant ignored, interactive menu, failure propagation

Key test scenarios:

• Auth method isolation — user/SPN don't invoke AzureCliCredential, azure_cli doesn't invoke MSAL
• 3-scope token acquisition at login (Fabric, OneLake, Azure)
• Azure CLI not installed / not logged in error handling
• Singleton credential reuse
• Login/logout lifecycle
• MSAL regression verified (273 core tests pass)

Security

• Uses AzureCliCredential from azure-identity SDK — no direct az subprocess calls
• Error messages surfaced from SDK (pre-sanitized by azure-identity)
• Auth methods are mutually exclusive — no credential path overlap
• No tokens cached client-side; relies on Azure CLI's own MSAL cache
• Identity changes handled naturally by API-level authorization (401/403)

[Copilot]

Pull request overview

This PR adds a new authentication source (azure_cli) to Fabric CLI, delegating token acquisition to Azure CLI (via azure-identity’s AzureCliCredential) so fab can reuse an existing az login session. It also wires the new auth source into fab auth login (flag + interactive option) and exposes the selected auth source in fab auth status.

Changes:

• Add azure_cli as an allowed identity type and implement Azure CLI token acquisition in FabAuth.
• Add --azure-cli flag plus an “Azure CLI” interactive login option; include auth_source in auth status output.
• Add azure-identity>=1.15.0 dependency and a new unit test module for Azure CLI auth.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tests/test_core/test_fab_auth_azure_cli.py	Adds unit tests for the new Azure CLI auth flow, scopes, and error sanitization.
src/fabric_cli/parsers/fab_auth_parser.py	Adds --azure-cli to fab auth login and updates examples.
src/fabric_cli/core/fab_constant.py	Extends identity type allow-list to include azure_cli.
src/fabric_cli/core/fab_auth.py	Implements set_azure_cli() and _acquire_token_from_azure_cli() and dispatches in acquire_token().
src/fabric_cli/commands/auth/fab_auth.py	Wires Azure CLI auth into login flows and adds auth_source to status output.
pyproject.toml	Adds the azure-identity runtime dependency required for AzureCliCredential.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (6)

tests/test_core/test_fab_auth_azure_cli.py:36

• These fixtures are unused in this test module, and the singleton-reset logic inside them is brittle (it relies on non-existent __wrapped__ and decorator internals). After resetting the FabAuth singleton in the autouse fixture, these can be removed to keep the tests easier to maintain.

@pytest.fixture
def auth_instance(temp_dir_fixture):
    """Get a fresh FabAuth instance."""
    # Clear singleton for test isolation
    FabAuth.__wrapped__ = None  # type: ignore
    from fabric_cli.core import fab_auth as fab_auth_module

    if FabAuth in fab_auth_module.singleton.__wrapped__:  # type: ignore
        del fab_auth_module.singleton.__wrapped__[FabAuth]  # type: ignore
    return FabAuth()

tests/test_core/test_fab_auth_azure_cli.py:56

• This fixture is unused, and it attempts to monkeypatch FabAuth.__init__.__globals__, which is not a safe or reliable way to reset the singleton (and may raise if executed). With the singleton reset handled in the autouse fixture, this block can be deleted.

@pytest.fixture
def fresh_auth(temp_dir_fixture, monkeypatch):
    """Get a fresh FabAuth instance with singleton cleared."""
    # Reset singleton instances dict
    import fabric_cli.core.fab_auth as auth_module

    # Access the closure variable of the singleton decorator
    singleton_instances = auth_module.singleton.__code__.co_consts  # noqa
    # Simpler approach: just patch the module-level reference
    monkeypatch.setattr(
        "fabric_cli.core.fab_auth.FabAuth.__init__.__globals__",
        {},
        raising=False,
    )
    # Re-instantiate
    auth = FabAuth.__new__(FabAuth)
    auth.__init__()
    return auth

src/fabric_cli/core/fab_auth.py:444

• This line exceeds the repo’s Black line-length (88) and will be reformatted by CI (tox.toml:69-70). Please wrap the conditional instantiation so the formatted output is stable and easier to read.

        tenant_id = self.get_tenant_id()
        try:
            credential = AzureCliCredential(tenant_id=tenant_id) if tenant_id else AzureCliCredential()
            # AzureCliCredential.get_token expects scopes as positional args
            azure_token = credential.get_token(scope[0])

src/fabric_cli/core/fab_auth.py:461

• This sanitized error message is long enough to violate Black’s 88-char line length and will be reformatted by CI (tox.toml:69-70). Splitting it across adjacent string literals keeps formatting stable.

            error_msg = str(e)
            if "accessToken" in error_msg or "token" in error_msg.lower():
                error_msg = "Azure CLI token acquisition failed. Run 'az account get-access-token' manually to diagnose."
            raise FabricCLIError(

src/fabric_cli/commands/auth/fab_auth.py:37

• When --azure-cli is provided, other credential flags (e.g., -u/-p, --certificate, --federated-token, --identity) are silently ignored due to branch precedence. This can lead to confusing CLI behavior; please validate and fail fast on incompatible combinations.

    if getattr(args, "azure_cli", False):
        FabAuth().set_access_mode("azure_cli", args.tenant)
        FabAuth().set_azure_cli(args.tenant)
        FabAuth().get_access_token(scope=fab_constant.SCOPE_FABRIC_DEFAULT)
        FabAuth().get_access_token(scope=fab_constant.SCOPE_ONELAKE_DEFAULT)

tests/test_core/test_fab_auth_azure_cli.py:24

• FabAuth is a singleton (see @singleton in fabric_cli.core.fab_auth). These tests patch config_location() per-test, but without clearing the singleton, FabAuth() will reuse the first instance (and its first auth/cache paths), causing state leakage across tests and making the tmp_path isolation ineffective.

This issue also appears in the following locations of the same file:

• line 27
• line 39

@pytest.fixture(autouse=True)
def temp_dir_fixture(monkeypatch, tmp_path):
    """Create a temporary directory and configure FabAuth to use it."""
    monkeypatch.setattr(
        "fabric_cli.core.fab_state_config.config_location", lambda: str(tmp_path)
    )

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/fabric_cli/core/fab_auth.py:502

• _acquire_token_from_azure_cli() calls credential.get_token(scope[0]) even though the method signature allows an empty scope list. If scope is empty, this will raise IndexError instead of a FabricCLIError, and it also ignores additional scopes if ever provided.

            credential = AzureCliCredential(tenant_id=stored_tenant) if stored_tenant else AzureCliCredential()
            # AzureCliCredential.get_token expects scopes as positional args
            azure_token = credential.get_token(scope[0])
            token_result = {
                "access_token": azure_token.token,

src/fabric_cli/commands/auth/fab_auth.py:35

• --azure-cli is not validated as mutually exclusive with managed identity/service principal flags. Because the code checks azure_cli first, a user can accidentally pass conflicting flags and silently get Azure CLI auth instead of an error.

    if getattr(args, "azure_cli", False):
        FabAuth().set_access_mode("azure_cli", args.tenant)
        FabAuth().set_azure_cli(args.tenant)
        FabAuth().get_access_token(scope=fab_constant.SCOPE_FABRIC_DEFAULT)
        FabAuth().get_access_token(scope=fab_constant.SCOPE_ONELAKE_DEFAULT)

tests/test_core/test_fab_auth_azure_cli.py:18

• FabAuth is a singleton (decorator returns a cached instance), but this autouse fixture only patches config_location/env vars and doesn’t clear the singleton cache. If any other test module instantiates FabAuth before this fixture runs, these tests will share state and potentially write auth/cache files outside tmp_path, causing order-dependent failures.

@pytest.fixture(autouse=True)
def temp_dir_fixture(monkeypatch, tmp_path):
    """Create a temporary directory and configure FabAuth to use it."""
    monkeypatch.setattr(
        "fabric_cli.core.fab_state_config.config_location", lambda: str(tmp_path)

src/fabric_cli/core/fab_auth.py:522

• The generic exception handler includes the raw exception message in the CLI error unless it matches a small allow-list of substrings. That does not guarantee token material won’t leak (e.g., access tokens that don’t contain the current patterns), which contradicts the PR’s “never leak token content” claim.

        except Exception as e:
            # Sanitize: never include token content in error messages
            error_msg = str(e)
            if any(p.lower() in error_msg.lower() for p in self._SENSITIVE_PATTERNS):
                error_msg = "Azure CLI token acquisition failed. Run 'az account get-access-token' manually to diagnose."

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (5)

src/fabric_cli/core/fab_auth.py:477

• Azure CLI auth introduces new FabricCLIError messages as hardcoded strings. Elsewhere in this module, auth failures use ErrorMessages.Auth.*() helpers for consistent wording and easier localization/maintenance. Consider moving these new messages into ErrorMessages.Auth and reusing them here.

        try:
            from azure.identity import AzureCliCredential, CredentialUnavailableError
        except ImportError:
            raise FabricCLIError(
                "Azure CLI auth requires the 'azure-identity' package. "
                "Install it with: pip install azure-identity",
                status_code=con.ERROR_AUTHENTICATION_FAILED,
            )

tests/test_core/test_fab_auth_azure_cli.py:36

• These tests call FabAuth() directly, but FabAuth is a singleton (via the @singleton decorator). Without reliably clearing the singleton cache per test, state (auth_file paths, env-loaded tokens, tenant id) can leak between tests and make behavior depend on execution order. The current fixture attempts (__wrapped__, singleton.__wrapped__) don't match how the singleton decorator is implemented, so they won't actually reset anything.

@pytest.fixture
def auth_instance(temp_dir_fixture):
    """Get a fresh FabAuth instance."""
    # Clear singleton for test isolation
    FabAuth.__wrapped__ = None  # type: ignore
    from fabric_cli.core import fab_auth as fab_auth_module

    if FabAuth in fab_auth_module.singleton.__wrapped__:  # type: ignore
        del fab_auth_module.singleton.__wrapped__[FabAuth]  # type: ignore
    return FabAuth()

src/fabric_cli/commands/auth/fab_auth.py:39

• fab auth login --azure-cli currently only acquires the Fabric token, while other login flows (interactive, SPN, managed identity) also acquire OneLake and Azure management tokens. This makes Azure CLI login behave differently and can leave later commands without the required secondary tokens.

    if getattr(args, "azure_cli", False):
        FabAuth().set_access_mode("azure_cli", args.tenant)
        FabAuth().set_azure_cli(args.tenant)
        FabAuth().get_access_token(scope=fab_constant.SCOPE_FABRIC_DEFAULT)
        Context().context = FabAuth().get_tenant()

src/fabric_cli/core/fab_auth.py:500

• The Azure CLI token acquisition block includes lines that exceed the repo's Black line-length (88), which will cause formatting churn and makes the code harder to read (e.g., the inline conditional credential construction). Please wrap these statements in Black-friendly form.

        try:
            credential = AzureCliCredential(tenant_id=stored_tenant) if stored_tenant else AzureCliCredential()
            # AzureCliCredential.get_token expects scopes as positional args
            azure_token = credential.get_token(scope[0])
            token_result = {

src/fabric_cli/core/fab_auth.py:522

• This sanitization fallback message is on a single very long line (over Black's 88-char limit). Wrapping it will keep formatting stable and improve readability.

            error_msg = str(e)
            if any(p.lower() in error_msg.lower() for p in self._SENSITIVE_PATTERNS):
                error_msg = "Azure CLI token acquisition failed. Run 'az account get-access-token' manually to diagnose."
            raise FabricCLIError(
                f"Azure CLI authentication failed: {error_msg}",

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

src/fabric_cli/core/fab_auth.py:501

• _acquire_token_from_azure_cli() calls scope[0] unconditionally (including in credential.get_token(scope[0])). If a caller passes an empty scope list, this will raise IndexError instead of a structured FabricCLIError. Also, AzureCliCredential.get_token expects scopes as positional args; using only scope[0] silently drops additional scopes if they’re ever introduced.

        try:
            credential = AzureCliCredential(tenant_id=stored_tenant) if stored_tenant else AzureCliCredential()
            # AzureCliCredential.get_token expects scopes as positional args
            azure_token = credential.get_token(scope[0])
            token_result = {

tests/test_core/test_fab_auth_azure_cli.py:36

• The auth_instance fixture is unused and its singleton-reset logic is incorrect for FabAuth (FabAuth is a function returned by the singleton decorator, and singleton() doesn’t expose a wrapped instances map). Keeping this dead code is misleading and risks future test failures if someone starts using it.

@pytest.fixture
def auth_instance(temp_dir_fixture):
    """Get a fresh FabAuth instance."""
    # Clear singleton for test isolation
    FabAuth.__wrapped__ = None  # type: ignore
    from fabric_cli.core import fab_auth as fab_auth_module

    if FabAuth in fab_auth_module.singleton.__wrapped__:  # type: ignore
        del fab_auth_module.singleton.__wrapped__[FabAuth]  # type: ignore
    return FabAuth()

tests/test_core/test_fab_auth_azure_cli.py:56

• The fresh_auth fixture is unused and attempts to mutate FabAuth.init.globals / singleton internals, which is brittle and not a valid way to reset the singleton. This should be removed to keep the test module deterministic and maintainable.

@pytest.fixture
def fresh_auth(temp_dir_fixture, monkeypatch):
    """Get a fresh FabAuth instance with singleton cleared."""
    # Reset singleton instances dict
    import fabric_cli.core.fab_auth as auth_module

    # Access the closure variable of the singleton decorator
    singleton_instances = auth_module.singleton.__code__.co_consts  # noqa
    # Simpler approach: just patch the module-level reference
    monkeypatch.setattr(
        "fabric_cli.core.fab_auth.FabAuth.__init__.__globals__",
        {},
        raising=False,
    )
    # Re-instantiate
    auth = FabAuth.__new__(FabAuth)
    auth.__init__()
    return auth

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Suppressed comments (3)

src/fabric_cli/core/fab_auth.py:477

• New FabricCLIError messages here are hardcoded strings. Elsewhere in this module, auth errors consistently use ErrorMessages.Auth.* helpers (e.g., invalid_identity_type(), token_acquisition_failed(), access_token_error()), which centralizes wording and keeps UX consistent. Consider adding Azure CLI-specific ErrorMessages.Auth helpers and using them here (and for the other Azure CLI error branches) instead of inline strings.

            raise FabricCLIError(
                "Azure CLI auth requires the 'azure-identity' package. "
                "Install it with: pip install azure-identity",
                status_code=con.ERROR_AUTHENTICATION_FAILED,
            )

src/fabric_cli/core/fab_auth.py:441

• In set_azure_cli(), identity_type is set before calling set_tenant(). If set_tenant() detects a tenant change it calls logout(), which clears _auth_info and can wipe out the just-set identity_type. This can leave the auth config without identity_type when switching tenants via set_azure_cli(tenant_id=...). Reorder so tenant changes (and any logout) happen first, then set identity_type last.

        self._set_auth_properties(
            {
                con.IDENTITY_TYPE: "azure_cli",
            }
        )

tests/test_core/test_fab_auth_azure_cli.py:57

• The auth_instance and fresh_auth fixtures are defined but never used in this test module, and they contain brittle/incorrect attempts to reset the @singleton-decorated FabAuth (e.g., mutating FabAuth.wrapped and FabAuth.init.globals). Keeping these unused fixtures risks future accidental use and makes the tests harder to understand; remove them or refactor to a single, actually-used fixture.

@pytest.fixture
def auth_instance(temp_dir_fixture):
    """Get a fresh FabAuth instance."""
    # Clear singleton for test isolation
    FabAuth.__wrapped__ = None  # type: ignore
    from fabric_cli.core import fab_auth as fab_auth_module

    if FabAuth in fab_auth_module.singleton.__wrapped__:  # type: ignore
        del fab_auth_module.singleton.__wrapped__[FabAuth]  # type: ignore
    return FabAuth()


@pytest.fixture
def fresh_auth(temp_dir_fixture, monkeypatch):
    """Get a fresh FabAuth instance with singleton cleared."""

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (5)

tests/test_core/test_fab_auth_azure_cli.py:57

• The fresh_auth fixture contains fragile/incorrect singleton-reset logic (e.g., reading singleton.__code__.co_consts and patching FabAuth.__init__.__globals__ to {}), and it is unused in this test module. Leaving this in place makes the tests harder to understand and could break badly if someone starts using it later.

Consider removing it and relying on a single, correct autouse singleton-reset fixture instead.

@pytest.fixture
def fresh_auth(temp_dir_fixture, monkeypatch):
    """Get a fresh FabAuth instance with singleton cleared."""
    # Reset singleton instances dict
    import fabric_cli.core.fab_auth as auth_module

    # Access the closure variable of the singleton decorator
    singleton_instances = auth_module.singleton.__code__.co_consts  # noqa
    # Simpler approach: just patch the module-level reference
    monkeypatch.setattr(
        "fabric_cli.core.fab_auth.FabAuth.__init__.__globals__",
        {},
        raising=False,
    )
    # Re-instantiate
    auth = FabAuth.__new__(FabAuth)
    auth.__init__()
    return auth

src/fabric_cli/core/fab_auth.py:477

• This new ImportError path raises FabricCLIError with a hardcoded message. In this codebase, auth errors are consistently sourced from ErrorMessages.Auth.* (see e.g. fab_auth.py:590-636) so messages stay centralized and reusable.

Please add an AuthErrors.azure_cli_missing_dependency() (or similar) and use it here for consistency.

        try:
            from azure.identity import AzureCliCredential, CredentialUnavailableError
        except ImportError:
            raise FabricCLIError(
                "Azure CLI auth requires the 'azure-identity' package. "
                "Install it with: pip install azure-identity",
                status_code=con.ERROR_AUTHENTICATION_FAILED,
            )

src/fabric_cli/core/fab_auth.py:513

• The Azure CLI unavailable case uses a hardcoded user-facing message. Elsewhere in this module, user-facing auth errors come from ErrorMessages.Auth.*.

Consider adding a dedicated AuthErrors.azure_cli_unavailable() (and possibly a separate one for "not logged in") and using it here so error messaging stays consistent and maintainable.

        except CredentialUnavailableError:
            raise FabricCLIError(
                "Azure CLI is not installed or not logged in. "
                "Run 'az login' to authenticate, then retry.",
                status_code=con.ERROR_AUTHENTICATION_FAILED,
            )

src/fabric_cli/core/fab_auth.py:500

• This credential = ... if ... else ... line exceeds Black’s default line length and will likely fail formatting checks in CI.

Wrap it onto multiple lines so black src/ tests/ stays clean.

        try:
            credential = AzureCliCredential(tenant_id=stored_tenant) if stored_tenant else AzureCliCredential()
            # AzureCliCredential.get_token expects scopes as positional args
            azure_token = credential.get_token(scope[0])
            token_result = {

tests/test_core/test_fab_auth_azure_cli.py:37

• The auth_instance fixture tries to reset the FabAuth singleton via __wrapped__, but FabAuth is a custom @singleton wrapper (a closure) and neither FabAuth.__wrapped__ nor fabric_cli.core.fab_auth.singleton.__wrapped__ exist. If this fixture is ever used it will raise AttributeError, and the current tests also risk leaking singleton state between test modules.

Use a robust singleton reset that clears the wrapped closure’s instances dict, ideally as an autouse fixture so all tests in this module get isolation.

This issue also appears on line 39 of the same file.

@pytest.fixture
def auth_instance(temp_dir_fixture):
    """Get a fresh FabAuth instance."""
    # Clear singleton for test isolation
    FabAuth.__wrapped__ = None  # type: ignore
    from fabric_cli.core import fab_auth as fab_auth_module

    if FabAuth in fab_auth_module.singleton.__wrapped__:  # type: ignore
        del fab_auth_module.singleton.__wrapped__[FabAuth]  # type: ignore
    return FabAuth()

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Suppressed comments (2)

src/fabric_cli/commands/auth/fab_auth.py:35

• The PR summary says auth status reports auth_source, but the status payload still contains no auth_source field, so users cannot tell that the session uses Azure CLI authentication. Add the identity type to the status data and cover it in text and JSON output.

    elif args.identity:

src/fabric_cli/core/fab_auth.py:458

• Tenant identity is only recorded when no tenant is already persisted. Since AzureCliCredential is created without a tenant pin and follows the current az session, switching az login to another tenant causes subsequent calls to use the new tenant's token while Fabric CLI still retains the old tenant and context. Compare the current token/session tenant on every acquisition and reset or reject mismatched state on Azure CLI login.

            # Store tenant_id from first successful token if not already set
            if not self.get_tenant_id():
                claims = self._decode_jwt_token(azure_token.token)
                if claims.get("tid"):
                    self.set_tenant(claims["tid"])

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Suppressed comments (7)

Previously missed (1) — in code that hasn't changed since the last review.

src/fabric_cli/core/fab_auth.py:458

• Once fab_tenant_id has been persisted, this condition skips decoding the newly acquired Azure CLI token entirely. Therefore a later az login --tenant ... (or az account set) leaves the old tenant in Fabric CLI state and proceeds with a token for the new tenant; the advertised per-command drift check/re-authentication behavior is not implemented. Compare the current CLI/token tenant with the stored tenant on every acquisition and fail explicitly on mismatch.

            # Store tenant_id from first successful token if not already set
            if not self.get_tenant_id():
                claims = self._decode_jwt_token(azure_token.token)
                if claims.get("tid"):
                    self.set_tenant(claims["tid"])

src/fabric_cli/commands/auth/fab_auth.py:33

• --tenant is never read in this branch, so fab auth login --azure-cli --tenant X succeeds with any active az tenant and records that tenant from the token. This contradicts the documented validation behavior and removes the guard against authenticating against the wrong tenant; validate the requested tenant against the Azure CLI session/token instead of silently ignoring it.

    if getattr(args, "azure_cli", False):
        FabAuth().set_access_mode("azure_cli")
        _acquire_default_access_tokens(FabAuth())
        Context().context = FabAuth().get_tenant()

src/fabric_cli/commands/auth/fab_auth.py:31

• set_access_mode only calls logout() when the mode changes. Therefore rerunning fab auth login --azure-cli while already in this mode does not clear the stored tenant/credential state; after changing the Azure CLI session to another tenant, the subsequent token acquisition still retains the old tenant. Use the advertised dedicated Azure CLI login/reset path that clears state on every explicit login.

        FabAuth().set_access_mode("azure_cli")

src/fabric_cli/commands/auth/fab_auth.py:78

• When --tenant is supplied but the user selects Azure CLI from the interactive menu, this branch also drops the supplied tenant. Thus fab auth login --tenant X followed by the Azure CLI choice bypasses the documented tenant check; apply the same validation here as in the direct Azure CLI path.

            elif selected_auth.startswith("Azure CLI"):
                FabAuth().set_access_mode("azure_cli")
                _acquire_default_access_tokens(FabAuth())
                Context().context = FabAuth().get_tenant()

src/fabric_cli/core/fab_auth.py:452

• AzureCliCredential.get_token() is called unconditionally and its result is never cached. fab_api_client.py:111 requests a token for every API request, so commands with multiple requests repeatedly spawn Azure CLI subprocesses (and auth status repeats this for each scope), defeating the claimed 60-second in-memory cache and adding substantial latency. Cache tokens per scope until their expiry buffer, with invalidation on logout/login.

            if self._azure_cli_credential is None:
                self._azure_cli_credential = AzureCliCredential()
            # AzureCliCredential.get_token expects scopes as positional args
            azure_token = self._azure_cli_credential.get_token(scope[0])

src/fabric_cli/core/fab_auth.py:38

• This mutates the process-wide azure.identity and azure.core loggers at module import, even when the caller never selects Azure CLI authentication. It suppresses all Azure SDK diagnostics and prevents propagation to handlers configured by embedding applications, making unrelated Azure failures difficult to diagnose. Avoid changing shared logger configuration globally; scope any sensitive-output suppression to the credential operation instead.

for _azure_ns in ("azure.identity", "azure.core"):
    logging.getLogger(_azure_ns).setLevel(logging.CRITICAL)
    logging.getLogger(_azure_ns).propagate = False

tests/test_core/test_fab_auth_azure_cli.py:409

• Exception is a superclass of FabricCLIError, so both assertions pass for any unrelated exception and do not verify the expected error contract. Narrow these contexts to pytest.raises(FabricCLIError) so malformed-JWT regressions are detected.

        with pytest.raises((FabricCLIError, Exception)):
            auth._decode_jwt_token("not-a-jwt")
        with pytest.raises((FabricCLIError, Exception)):
            auth._decode_jwt_token("")

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Suppressed comments (8)

src/fabric_cli/commands/auth/fab_auth.py:33

• The new Azure CLI mode is persisted here, but status() still builds auth_data without an auth_source field. Consequently fab auth status cannot tell users that the active provider is azure_cli, despite the PR's stated status change. Add the identity type to both text and JSON status output.

    if getattr(args, "azure_cli", False):
        FabAuth().set_access_mode("azure_cli")
        _acquire_default_access_tokens(FabAuth())
        Context().context = FabAuth().get_tenant()

src/fabric_cli/core/fab_auth.py:452

• Every call here delegates directly to AzureCliCredential.get_token, so there is no 60-second in-memory token cache: the three calls in _acquire_default_access_tokens and the repeated calls in auth status each pay the Azure CLI subprocess/token-acquisition cost. Cache tokens per scope until expires_on - 60 before invoking the credential, and invalidate that cache on logout or tenant changes.

            azure_token = self._azure_cli_credential.get_token(scope[0])

src/fabric_cli/errors/auth.py:124

• azure_cli_not_available is decorated with @staticmethod twice. Remove the duplicate decorator; leaving a staticmethod descriptor wrapped in another descriptor is unnecessary and can interfere with introspection/tooling even though calls happen to work on supported Python versions.

    @staticmethod
    @staticmethod

tests/test_commands/test_auth.py:35

• The rewrapped calls in this changed test file are still not Black-formatted (for example, Black will rewrite the assert_called_with call at lines 34-35 and the patch call below). Because CI runs black . via tox.toml:39-43, format this file before merging.

            mock_fab_auth_instance.set_access_mode.assert_called_with(
                "user", None)

tests/test_core/test_fab_auth_azure_cli.py:409

• Because FabricCLIError subclasses Exception, each pytest.raises((FabricCLIError, Exception)) accepts any exception type. These tests would pass for an unrelated implementation error and do not verify the documented FabricCLIError contract; assert pytest.raises(FabricCLIError) instead.

        with pytest.raises((FabricCLIError, Exception)):
            auth._decode_jwt_token("not-a-jwt")
        with pytest.raises((FabricCLIError, Exception)):
            auth._decode_jwt_token("")

tests/test_core/test_fab_auth_azure_cli.py:389

• The test name says re-login resets state, but the body explicitly documents the opposite and asserts that tenant A is preserved after the second set_access_mode("azure_cli"). This would pass even though the PR description requires every Azure CLI login to clear/rediscover tenant and token state. Rename the test if preservation is intentional, or make it assert tenant B and the promised cache invalidation behavior.

    def test_re_login_resets_state(self, mock_credential_class, temp_dir_fixture):
        """Re-login (set_access_mode again) should reset state."""
        _mock_credential_with_jwt(mock_credential_class, tid="tenant-A")
        auth = FabAuth()
        auth.set_access_mode("azure_cli")
        auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT)
        assert auth.get_tenant_id() == "tenant-A"

        # Re-login — set_access_mode("azure_cli") when already azure_cli does NOT logout
        # but a different tenant in next token will not overwrite
        _mock_credential_with_jwt(mock_credential_class, tid="tenant-B")
        auth.set_access_mode("azure_cli")
        # Tenant is still A because it was already set
        assert auth.get_tenant_id() == "tenant-A"

tests/test_core/test_fab_auth_azure_cli.py:22

• This new test file is not Black-formatted (for example, the _make_jwt signature and payload expression exceed the configured 88-character layout). CI runs black . via tox.toml:39-43, so the PR will fail the lint job until the changed test files are formatted.

def _make_jwt(tid: str = "test-tenant", oid: str = "test-oid",
              iss: str = "https://sts.windows.net/test-tenant/", **extra_claims) -> str:
    """Create a fake JWT with specified claims (no signature validation needed)."""
    header = base64.urlsafe_b64encode(b'{"alg":"none"}').rstrip(b"=").decode()
    claims = {"tid": tid, "oid": oid, "iss": iss, **extra_claims}
    payload = base64.urlsafe_b64encode(_json.dumps(claims).encode()).rstrip(b"=").decode()

tests/test_core/test_fab_msal_bridge_azure_cli.py:27

• This new bridge test also contains Black-unformatted long expressions (notably the _make_jwt payload construction). Since the lint job formats the whole repository with black ., run Black on this file before merging so the CI lint check passes.

def _make_jwt(tid: str = "test-tenant", oid: str = "test-oid") -> str:
    """Create a fake JWT with specified claims."""
    header = base64.urlsafe_b64encode(b'{"alg":"none"}').rstrip(b"=").decode()
    claims = {"tid": tid, "oid": oid, "iss": f"https://sts.windows.net/{tid}/"}
    payload = base64.urlsafe_b64encode(
        _json.dumps(claims).encode()).rstrip(b"=").decode()
    return f"{header}.{payload}.fakesig"

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Suppressed comments (7)

Previously missed (1) — in code that hasn't changed since the last review.

tests/test_core/test_fab_auth_azure_cli.py:409

• Exception includes FabricCLIError, so this assertion accepts any exception and would pass if malformed input raises an unrelated TypeError or network error. It does not verify the documented error contract; assert pytest.raises(FabricCLIError) for both calls.

        with pytest.raises((FabricCLIError, Exception)):
            auth._decode_jwt_token("not-a-jwt")
        with pytest.raises((FabricCLIError, Exception)):
            auth._decode_jwt_token("")

docs/commands/auth/index.md:46

• This new per-method command reference omits the existing managed identity login path (fab auth login --identity and the user-assigned -u form). Since it replaces the previous combined usage block, readers can no longer discover a supported authentication mode here; add a managed identity usage block and parameter entry.

#### Workload identity

fab auth login -u <client_id> --federated-token --tenant <tenant_id>

**src/fabric_cli/commands/auth/fab_auth.py:33**
* The PR summary says `fab auth status` should show the auth source, but the status payload still contains only logged-in/account/principal/tenant/app/token fields. As a result, Azure CLI, user, and service-principal sessions are indistinguishable in status output. Add `auth_source` from `auth.get_identity_type()` and cover it in text and JSON output.

if getattr(args, "azure_cli", False):
    FabAuth().set_access_mode("azure_cli")
    _acquire_default_access_tokens(FabAuth())
    Context().context = FabAuth().get_tenant()

**src/fabric_cli/core/fab_auth.py:452**
* `AzureCliCredential.get_token` is invoked on every call, but this path has no token cache. `fab_api_client` requests a token for every API request and `auth status` reads the same scopes repeatedly, so multi-request commands pay repeated `az account get-access-token` subprocess startup costs. Add a per-scope cache with the advertised 60-second refresh buffer before invoking the credential.

        # AzureCliCredential.get_token expects scopes as positional args
        azure_token = self._azure_cli_credential.get_token(scope[0])

**src/fabric_cli/core/fab_auth.py:458**
* Once `fab_tenant_id` is populated, this condition skips decoding every later Azure CLI token, so the tenant is never checked again. If the user changes the active `az` account from tenant A to tenant B, the next request uses a B token while the persisted tenant/context remains A (and the new re-login test currently codifies that behavior). Compare the token/session tenant with the stored tenant and fail or reset on drift before returning the token.

        # Store tenant_id from first successful token if not already set
        if not self.get_tenant_id():
            claims = self._decode_jwt_token(azure_token.token)
            if claims.get("tid"):
                self.set_tenant(claims["tid"])

**src/fabric_cli/core/fab_auth.py:38**
* These import-time assignments mutate the process-global `azure.identity` and `azure.core` loggers, suppressing all messages below CRITICAL and disabling propagation. That also affects host applications and SDK consumers importing `FabAuth` (including the token bridge), so it hides legitimate diagnostics, not just token-bearing output. Apply redaction or isolated handlers to Fabric CLI logging instead of globally disabling the namespaces.

for _azure_ns in ("azure.identity", "azure.core"):
logging.getLogger(_azure_ns).setLevel(logging.CRITICAL)
logging.getLogger(_azure_ns).propagate = False

**tests/test_core/test_fab_auth_azure_cli.py:376**
* The test name says re-login "resets state", but the body and assertion intentionally verify that calling `set_access_mode("azure_cli")` again preserves tenant A. This misleading name makes the expected lifecycle behavior unclear and can hide the missing tenant/cache reset behavior; rename it to describe preserving state when the mode is unchanged.

def test_re_login_resets_state(self, mock_credential_class, temp_dir_fixture):

</details>

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

Suppressed comments (7)

Previously missed (1) — in code that hasn't changed since the last review.

src/fabric_cli/errors/auth.py:140

• All three login scopes use this helper, but it accepts no scope and the call site supplies none, so Fabric, OneLake, and Azure token failures produce the same generic message. Include the sanitized resource/scope in the error so users can identify which Azure CLI request failed.

    def azure_cli_token_acquisition_failed() -> str:
        return (
            "Unable to get a token from Azure CLI. "
            "Run 'az login' to authenticate, then retry"
        )

src/fabric_cli/commands/auth/fab_auth.py:31

• Because this branch runs before the existing managed-identity and service-principal branches, fab auth login --azure-cli --identity or --azure-cli with credential flags silently discards the other authentication inputs and logs in with Azure CLI instead. Validate conflicting authentication selectors before choosing a mode so a typo cannot authenticate as a different identity; apply the same validation consistently across the modes.

    if getattr(args, "azure_cli", False):
        FabAuth().set_access_mode("azure_cli")

src/fabric_cli/core/fab_auth.py:488

• The catch-all path drops the requested scope, even though login invokes this helper separately for Fabric, OneLake, and Azure management. A failure in the second or third request therefore produces the same generic message and does not provide the scope information described for this error, making diagnosis needlessly difficult. Include the requested resource when constructing the token-acquisition error.

        except Exception:
            # Unknown exceptions get a safe generic message — never leak raw details
            raise FabricCLIError(
                ErrorMessages.Auth.azure_cli_auth_failed(
                    ErrorMessages.Auth.azure_cli_token_acquisition_failed()

src/fabric_cli/core/fab_auth.py:38

• This mutates process-wide Azure logger levels and propagation at import time, so merely importing FabAuth disables all azure.identity/azure.core diagnostics for any embedding application, including code using the token-credential bridge, not just Fabric CLI output. Scope suppression to the CLI's own handlers or specific messages instead of changing global logging state.

for _azure_ns in ("azure.identity", "azure.core"):
    logging.getLogger(_azure_ns).setLevel(logging.CRITICAL)
    logging.getLogger(_azure_ns).propagate = False

src/fabric_cli/errors/auth.py:124

• There are two consecutive @staticmethod decorators here. The outer decorator wraps the descriptor produced by the inner one, which is unnecessary and can interfere with callable/introspection behavior; keep a single decorator.

    @staticmethod
    @staticmethod

tests/test_core/test_fab_auth_azure_cli.py:409

• Because Exception is a superclass of FabricCLIError, these assertions accept any exception and do not verify the structured error contract of malformed JWT handling. Use pytest.raises(FabricCLIError) so the test fails if an unrelated exception type escapes.

        with pytest.raises((FabricCLIError, Exception)):
            auth._decode_jwt_token("not-a-jwt")
        with pytest.raises((FabricCLIError, Exception)):
            auth._decode_jwt_token("")

tests/test_core/test_fab_msal_bridge_azure_cli.py:42

• Patching config_location does not update an already-created FabAuth singleton's auth_file and cache_file. If another test instantiated the singleton first, set_access_mode() calls logout() against those stale paths, potentially deleting a real or unrelated cache file; the other Azure CLI fixture explicitly redirects both paths. Set auth.auth_file and auth.cache_file to tmp_path here as well.

    auth = FabAuth()
    auth._azure_cli_credential = None
    auth._auth_info = {}

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Suppressed comments (10)

Previously missed (1) — in code that hasn't changed since the last review.

src/fabric_cli/core/fab_auth.py:492

• The generic fallback does not include the requested scope/resource, so failures during the three-scope login cannot identify whether Fabric, OneLake, or Azure token acquisition failed. The PR describes azure_cli_token_acquisition_failed as scope-aware; pass a safe scope/resource label to that error helper.

            raise FabricCLIError(
                ErrorMessages.Auth.azure_cli_auth_failed(
                    ErrorMessages.Auth.azure_cli_token_acquisition_failed()
                ),

src/fabric_cli/core/fab_auth.py:559

• env_var_token is resolved before identity_type dispatch, so Azure CLI mode still runs _get_access_token_from_env_vars_if_exist for every scope. With FAB_TOKEN/FAB_TOKEN_ONELAKE set but no FAB_TOKEN_AZURE (or with a stale env token), the Azure-scope preflight can raise before AzureCliCredential is called, so the new auth source is not isolated from environment-token state. Skip env-token resolution for azure_cli, or move it into only the environment-token path.

            elif identity_type == "azure_cli":
                token = self._acquire_token_from_azure_cli(scope)

src/fabric_cli/core/fab_auth.py:462

• This rewrites the persisted tenant on every token request, while the helper's docstring and PR contract describe discovering it once for context. A later az login or account switch will therefore silently change Fabric CLI's stored tenant during ordinary API calls; guard this assignment so it only runs when no tenant has been stored, or explicitly implement the intended drift behavior.

            if tid and tid != self.get_tenant_id():
                self._set_auth_properties({con.FAB_TENANT_ID: tid})

src/fabric_cli/core/fab_auth.py:38

• This changes the process-wide azure.identity and azure.core logger configuration at import time. The token-credential bridge can be used in a host process, so unrelated Azure SDK or fabric-cicd diagnostics will also be suppressed, making failures harder to diagnose. Prefer a credential-local logging option/filter rather than mutating global logger state.

for _azure_ns in ("azure.identity", "azure.core"):
    logging.getLogger(_azure_ns).setLevel(logging.CRITICAL)
    logging.getLogger(_azure_ns).propagate = False

src/fabric_cli/errors/auth.py:125

• Two @staticmethod decorators were added for this method. The outer decorator wraps a staticmethod object rather than the function; this is redundant and can confuse type checking/introspection. Keep a single decorator.

    @staticmethod
    @staticmethod
    def azure_cli_not_available() -> str:

tests/test_core/test_fab_auth_azure_cli.py:409

• FabricCLIError already derives from Exception, so including Exception in this tuple makes the assertion accept any exception. The test would pass for unrelated failures such as KeyError or TypeError and does not verify the intended malformed-JWT error type.

        with pytest.raises((FabricCLIError, Exception)):
            auth._decode_jwt_token("not-a-jwt")
        with pytest.raises((FabricCLIError, Exception)):
            auth._decode_jwt_token("")

tests/test_core/test_fab_auth_azure_cli.py:129

• _mock_credential_with_jwt updates mock_credential_class.return_value, but after the first call auth._azure_cli_credential still points at the original mock. The second call therefore returns the original tenant and this test passes even with the current overwrite bug. Update the cached mock's get_token.return_value to a token containing new-tenant before calling it.

        # Change mock to return different tenant — should not overwrite
        _mock_credential_with_jwt(mock_credential_class, tid="new-tenant")
        auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT)

tests/test_core/test_fab_auth_azure_cli.py:88

• The autouse fixture replaces auth._decode_jwt_token for every test, so the TestJwtClaimsDecoding tests below never exercise the production decoder; they only exercise _test_decode_jwt_token. Scope this monkeypatch to the Azure token-acquisition tests or use a separate fixture so the decoder contract is actually tested.

    monkeypatch.setattr(auth, "_decode_jwt_token", lambda token, expected_audience=None: _test_decode_jwt_token(auth, token, expected_audience))

tests/test_core/test_fab_auth_azure_cli.py:376

• This test is named as though re-login resets state, but it deliberately verifies the opposite: calling set_access_mode("azure_cli") in the same mode does not reset the tenant. The misleading name makes the lifecycle contract hard to understand and can hide regressions; rename it to describe tenant preservation for the same mode.

    def test_re_login_resets_state(self, mock_credential_class, temp_dir_fixture):

tests/test_core/test_fab_msal_bridge_azure_cli.py:42

• Because FabAuth is a singleton, patching config_location() does not change auth_file or cache_file on an instance created before this fixture. set_access_mode() calls logout() and writes auth state, so these tests can modify the real default ~/.config/fab/auth.json instead of the temporary directory. Patch both paths on the returned instance before exercising it.

    auth = FabAuth()
    auth._azure_cli_credential = None
    auth._auth_info = {}

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Suppressed comments (10)

Previously missed (2) — in code that hasn't changed since the last review.

src/fabric_cli/core/fab_auth.py:486

• The login helper requests three different resources, but this generic exception path reports only Unable to get a token from Azure CLI and drops the requested scope. When the second or third token fails, users cannot tell whether Fabric, OneLake, or Azure caused the login failure; include the scope in azure_cli_token_acquisition_failed (and its call sites).

                ErrorMessages.Auth.azure_cli_auth_failed(
                    ErrorMessages.Auth.azure_cli_token_acquisition_failed()
                ),

tests/test_core/test_fab_auth_azure_cli.py:405

• This test is named as though re-login resets state, but its assertions and comments verify the opposite: calling set_access_mode("azure_cli") again preserves the existing tenant. Rename it and its docstring so the test does not mislead future changes.

    def test_re_login_resets_state(self, mock_credential_class, temp_dir_fixture):
        """Re-login (set_access_mode again) should reset state."""

src/fabric_cli/commands/auth/fab_auth.py:32

• When --azure-cli is combined with another credential flag such as --identity, -u/-p, --certificate, or --federated-token, this branch silently discards the other inputs. A typo like fab auth login --azure-cli -u ... therefore succeeds while authenticating a different identity; validate mutually exclusive auth options before selecting a branch, while preserving the intentional --identity -u case.

    if getattr(args, "azure_cli", False):
        FabAuth().set_access_mode("azure_cli")
        _acquire_default_access_tokens(FabAuth())

src/fabric_cli/core/fab_auth.py:453

• This new path validates the Azure CLI token with _decode_jwt_token, which on a fresh process performs requests.get for AAD JWKS without a timeout (_fetch_public_key_from_aad). An unavailable AAD endpoint can therefore make fab auth login --azure-cli hang or fail even after Azure CLI returned a token; avoid network-backed validation just to read tid, or add bounded timeout and fallback handling.

            claims = self._decode_jwt_token(azure_token.token)

src/fabric_cli/core/fab_auth.py:41

• These module-level assignments change the process-wide parent logger configuration: all azure.identity and azure.core records are raised to CRITICAL and prevented from propagating. This suppresses warnings and diagnostics from unrelated Azure SDK/fabric-cicd operations, including when fab debug logging is enabled; filter or redact sensitive records locally rather than disabling the namespaces globally.

# Prevent Azure SDK logs from exposing tokens or subprocess output in console or file logs
for _azure_ns in ("azure.identity", "azure.core"):
    logging.getLogger(_azure_ns).setLevel(logging.CRITICAL)
    logging.getLogger(_azure_ns).propagate = False

src/fabric_cli/core/fab_auth.py:553

• _get_access_token_from_env_vars_if_exist runs before this dispatch. With FAB_TOKEN and FAB_TOKEN_ONELAKE set but no FAB_TOKEN_AZURE, the existing probe indexes the missing FAB_TOKEN_AZURE while preparing the third default scope and raises before AzureCliCredential is called. Thus fab auth login --azure-cli can fail due to unrelated environment-token settings; resolve the identity type before probing env-token auth, or skip that probe for azure_cli.

            elif identity_type == "azure_cli":
                token = self._acquire_token_from_azure_cli(scope)

src/fabric_cli/errors/auth.py:140

• The generic fallback does not identify which resource scope failed, even though login requests Fabric, OneLake, and Azure-management tokens sequentially. When an unknown or uninformative exception occurs, all three failures produce the same message, making diagnosis difficult; pass the requested scope/resource into this error and include it in the message.

    def azure_cli_token_acquisition_failed() -> str:
        return (
            "Unable to get a token from Azure CLI. "
            "Run 'az login' to authenticate, then retry"
        )

src/fabric_cli/errors/auth.py:124

• The new error helper has two consecutive @staticmethod decorators. The inner decorator already provides static binding, so remove the duplicate wrapper to avoid exposing a nested descriptor to introspection and tooling.

    @staticmethod
    @staticmethod

tests/test_commands/test_auth.py:986

• This new test locks in silent precedence for conflicting authentication flags. A typo such as fab auth login --azure-cli --identity or supplying an SP credential will authenticate with the active Azure CLI account while discarding the other input; validate conflicting auth options (or make the parser mutually exclusive) instead of asserting these combinations succeed.

    def test_init_with_azure_cli_flag_ignores_other_auth_args(
        self, mock_fab_auth, mock_fab_context, other_auth_arg
    ):
        """Arguments for other auth modes should not affect Azure CLI auth."""
        args = prepare_auth_args({"azure_cli": True, **other_auth_arg})

tests/test_core/test_fab_msal_bridge_azure_cli.py:42

• This fixture resets the singleton's in-memory state but leaves auth_file and cache_file pointing at the path from the first FabAuth() construction (normally ~/.config/fab). set_access_mode() then calls logout(), which can delete a real user's cache.bin; patch both paths to tmp_path before resetting the state, as the other Azure CLI fixture does.

    auth = FabAuth()
    auth._azure_cli_credential = None
    auth._auth_info = {}

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Suppressed comments (8)

Previously missed (2) — in code that hasn't changed since the last review.

src/fabric_cli/core/fab_auth.py:481

• This fallback is shared by all three login requests, but the resulting message contains no requested scope, so a failure cannot tell whether Fabric, OneLake, or management token acquisition failed. Include the scope/resource in the token-acquisition error so the failure is actionable.

        except Exception:
            # Unknown exceptions get a safe generic message — never leak raw details
            raise FabricCLIError(
                ErrorMessages.Auth.azure_cli_auth_failed(
                    ErrorMessages.Auth.azure_cli_token_acquisition_failed()
                ),
                status_code=con.ERROR_AUTHENTICATION_FAILED,

tests/test_core/test_fab_msal_bridge_azure_cli.py:42

• Because FabAuth is returned by a process-wide singleton, changing config_location before FabAuth() does not recompute its auth_file/cache_file paths. This fixture clears state but leaves those paths pointing at whichever test initialized the singleton, so these bridge tests can read or write another test's auth files; reset both paths to tmp_path as the other Azure CLI fixture does.

    auth = FabAuth()
    auth._azure_cli_credential = None
    auth._auth_info = {}

src/fabric_cli/commands/auth/fab_auth.py:32

• Because this branch runs before args.identity and the service-principal credential branches, commands such as fab auth login --azure-cli --identity -u <id> are accepted while the other authentication flags are silently ignored. The new command tests explicitly preserve this behavior, which can authenticate a different identity than the command appears to request; reject incompatible credential flags before selecting Azure CLI (ideally consistently for all auth modes).

    if getattr(args, "azure_cli", False):
        FabAuth().set_access_mode("azure_cli")
        _acquire_default_access_tokens(FabAuth())

src/fabric_cli/core/fab_auth.py:547

• Selecting azure_cli here does not isolate this source: acquire_token() probes _get_access_token_from_env_vars_if_exist(scope) before reaching this branch. If FAB_TOKEN/FAB_TOKEN_ONELAKE are present, that probe can raise or decode those tokens and AzureCliCredential is never called, so a stale or partial token environment can prevent Azure CLI login. Bypass the environment-token probe for this identity (or dispatch on identity before probing it) and cover the coexistence case.

            elif identity_type == "azure_cli":
                token = self._acquire_token_from_azure_cli(scope)

src/fabric_cli/core/fab_auth.py:450

• These lines overwrite the persisted tenant whenever a token is acquired, which conflicts with the PR's stated one-time tenant extraction and no-drift-detection design. In an interactive session, Context().context can still contain the previous tenant while FabAuth starts using the new one, so path resolution/cache keys and API tokens can refer to different tenants. Only initialize the tenant once, or explicitly update the context and document/test that drift behavior.

            if tid and tid != self.get_tenant_id():
                self._set_auth_properties({con.FAB_TENANT_ID: tid})

src/fabric_cli/core/fab_auth.py:479

• This broad catch also covers unrelated failures such as writing the discovered tenant to auth.json or programming errors, but reports all of them as Azure CLI token-acquisition failures. That hides actionable causes and makes a successful token look like an auth failure; catch the documented credential exceptions (plus only narrowly handled local errors) and preserve unrelated failures as their appropriate CLI errors.

        except Exception:
            # Unknown exceptions get a safe generic message — never leak raw details
            raise FabricCLIError(
                ErrorMessages.Auth.azure_cli_auth_failed(
                    ErrorMessages.Auth.azure_cli_token_acquisition_failed()

src/fabric_cli/errors/auth.py:140

• This generic failure message does not include the requested resource or scope, even though login performs separate Fabric, OneLake, and Azure token requests. A failure in any of the three is therefore indistinguishable and does not provide the scope information described for this error; accept the scope/resource in this helper and pass it from _acquire_token_from_azure_cli.

    def azure_cli_token_acquisition_failed() -> str:
        return (
            "Unable to get a token from Azure CLI. "
            "Run 'az login' to authenticate, then retry"
        )

tests/test_core/test_fab_auth_azure_cli.py:405

• The test name and docstring say that re-login resets state, but the assertions intentionally verify the opposite: setting the same identity preserves tenant A. Rename the test and description to match the behavior so a future failure is not misdiagnosed.

    def test_re_login_resets_state(self, mock_credential_class, temp_dir_fixture):
        """Re-login (set_access_mode again) should reset state."""

[Copilot]

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.

Suppressed comments (6)

src/fabric_cli/commands/auth/fab_auth.py:32

• When --azure-cli is combined with --identity, -u, -p, --certificate, or --federated-token, this branch silently wins and the other credential arguments are ignored. Reject conflicting auth selectors instead of authenticating with a different mode than the command appears to request; this should be handled consistently with the existing auth-mode precedence.

    if getattr(args, "azure_cli", False):
        FabAuth().set_access_mode("azure_cli")
        _acquire_default_access_tokens(FabAuth())

src/fabric_cli/core/fab_auth.py:445

• This compares the tenant on every token acquisition and synchronizes state by clearing caches and replacing the current context when it changes. That is drift detection and silently adopts a newly selected az tenant, contrary to the stated design of one-time tenant extraction with API-level 401/403 handling; only initialize the tenant when none is stored, or update the design description.

            if tid and tid != self.get_tenant_id():
                self._synchronize_azure_cli_tenant(tid)

src/fabric_cli/core/fab_auth.py:475

• The generic exception path drops the requested scope, although the PR describes azure_cli_token_acquisition_failed as including scope information. During the three-scope login probe, a failure is therefore not distinguishable as Fabric, OneLake, or Azure; include the requested resource in the helper message.

        except Exception:
            # Unknown exceptions get a safe generic message — never leak raw details
            raise FabricCLIError(
                ErrorMessages.Auth.azure_cli_auth_failed(
                    ErrorMessages.Auth.azure_cli_token_acquisition_failed()
                ),

src/fabric_cli/core/fab_auth.py:551

• Even when identity_type is azure_cli, env_var_token has already been resolved before this branch. That helper validates the documented FAB_TOKEN* values (and can index a missing FAB_TOKEN_AZURE) before AzureCliCredential is called, so an Azure CLI login can fail without using Azure CLI. Resolve environment-token auth only for its own path, or skip it when the identity type is azure_cli.

            elif identity_type == "azure_cli":
                token = self._acquire_token_from_azure_cli(scope)

tests/test_commands/test_auth.py:1032

• This test locks in silent precedence for conflicting auth sources. Because all auth flags are exposed by the same parser, fab auth login --azure-cli -u ... currently succeeds while ignoring -u and the other credentials; a typo can therefore select a different identity than the command implies. Reject conflicting combinations with ERROR_INVALID_INPUT and change this test to assert the failure, ideally applying the same validation to the existing modes.

    def test_init_with_azure_cli_flag_ignores_other_auth_args(
        self, mock_fab_auth, mock_fab_context, other_auth_arg
    ):
        """Arguments for other auth modes should not affect Azure CLI auth."""
        args = prepare_auth_args({"azure_cli": True, **other_auth_arg})

tests/test_core/test_fab_auth_azure_cli.py:416

• This test is named test_re_login_resets_state, but its assertions and comments verify that re-login preserves tenant state and does not log out. Rename it to reflect the behavior being tested so future failures are not misdiagnosed.

    def test_re_login_resets_state(self, mock_credential_class, temp_dir_fixture):

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.

Suppressed comments (3)

src/fabric_cli/core/fab_auth.py:552

• env_var_token is resolved before this dispatch (in acquire_token), even when identity_type is azure_cli. If a process inherits FAB_TOKEN and FAB_TOKEN_ONELAKE but has a missing or invalid FAB_TOKEN_AZURE, that probe raises before AzureCliCredential is called, so --azure-cli cannot use the selected provider. Skip environment-token resolution for this identity (or dispatch on the identity before probing environment tokens).

            elif identity_type == "azure_cli":
                token = self._acquire_token_from_azure_cli(scope)

src/fabric_cli/core/fab_constant.py:65

• Adding azure_cli as a distinct identity type leaves user-only path resolution broken: get_personal_workspace_name() still accepts only identity_type == "user", so a user authenticated by az login cannot use ~/the personal workspace and receives personal_workspace_user_auth_only. Azure CLI supports both user and service-principal sessions, so determine the principal type or update the user-only capability check rather than treating every Azure CLI session as non-user.

    IDENTITY_TYPE: ["user", "service_principal", "managed_identity", "azure_cli"],

src/fabric_cli/errors/auth.py:124

• azure_cli_not_available is decorated with @staticmethod twice. The outer decorator wraps an already-created descriptor, unlike every adjacent error helper, which can lead to confusing descriptor/introspection behavior; keep a single decorator here.

    @staticmethod
    @staticmethod

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.

Suppressed comments (7)

Previously missed (1) — in code that hasn't changed since the last review.

src/fabric_cli/utils/fab_ui.py:656

• This uses a substring replacement on the whole formatted string, so any key containing Cli is corrupted: client_id becomes CLIent ID (and similar words beginning with Cli). Apply the special cases to whole whitespace-delimited words instead of replacing arbitrary substrings.

        "Cli": "CLI",

docs/examples/auth_examples.md:32

• The new example is not reflected in the top-level authentication overview, which still says fab supports only user, service principal, and managed identity, and the generic parameter reference also omits --azure-cli. Please update those user-facing references so the new login source is not documented in only one subsection.

### Azure CLI Authentication

Reuse an _existing_ Azure CLI session instead of requiring a separate Fabric CLI login. Useful when tools or scripts already have `az login` done (e.g., in development environments or CI/CD pipelines with Azure CLI pre-authenticated).

!!! info "Requires Azure CLI to be installed and logged in (`az login`)"

src/fabric_cli/core/fab_auth.py:446

• This still performs tenant-drift detection on every token: when tid changes it persists the new tenant, clears caches, and resets Context. That conflicts with the PR's stated “No drift detection” design, which says identity changes should be left to API 401/403 handling. Either keep tenant extraction one-time as documented, or update the design and tests to make this synchronization intentional.

            if tid and tid != self.get_tenant_id():
                self._synchronize_azure_cli_tenant(tid)

src/fabric_cli/core/fab_auth.py:476

• The generic fallback drops the requested scope, so failures from the three sequential login requests all produce the same message and do not provide the scope context promised by this PR. Include a non-sensitive resource/scope label when constructing azure_cli_token_acquisition_failed so users can identify whether Fabric, OneLake, or Azure token acquisition failed.

            raise FabricCLIError(
                ErrorMessages.Auth.azure_cli_auth_failed(
                    ErrorMessages.Auth.azure_cli_token_acquisition_failed()
                ),

src/fabric_cli/core/fab_auth.py:552

• acquire_token resolves FAB_TOKEN* environment tokens before dispatching on the identity type, so this new Azure CLI branch is not isolated from the env-token path. For example, with FAB_TOKEN and FAB_TOKEN_ONELAKE set but no FAB_TOKEN_AZURE, the third default-scope probe indexes the missing variable and raises KeyError instead of using Azure CLI. Resolve environment tokens only for the environment-token path, or dispatch Azure CLI before that lookup.

            elif identity_type == "azure_cli":
                token = self._acquire_token_from_azure_cli(scope)

src/fabric_cli/core/fab_constant.py:65

• Adding azure_cli as a distinct identity type leaves user-only personal-workspace resolution unsupported: get_personal_workspace_name() still permits only identity_type == "user" (src/fabric_cli/core/fab_handle_context.py:237), so a normal user az login session gets personal_workspace_user_auth_only for ~/... paths. Because Azure CLI can represent both users and service principals, this source needs an underlying-account check (or an explicit documented limitation) before applying the user-only guard.

    IDENTITY_TYPE: ["user", "service_principal", "managed_identity", "azure_cli"],

src/fabric_cli/errors/auth.py:140

• This fallback error does not identify the failing scope, even though login acquires Fabric, OneLake, and Azure tokens sequentially. A failure in the second or third acquisition is therefore indistinguishable from a Fabric failure; pass the failing scope/resource into this message and its call site.

    def azure_cli_token_acquisition_failed() -> str:
        return (
            "Unable to get a token from Azure CLI. "
            "Run 'az login' to authenticate, then retry"
        )

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.

Suppressed comments (4)

Previously missed (2) — in code that hasn't changed since the last review.

src/fabric_cli/core/fab_auth.py:472

• This catch-all converts programming, persistence, or tenant-synchronization failures into an authentication failure, hiding the actual defect from callers and logs. Catch the Azure SDK exceptions (plus only explicitly expected process/OS errors) instead of Exception, while retaining a safe message for those expected failures.

        except Exception:

tests/test_core/test_fab_auth_azure_cli.py:372

• The test name and docstring say that re-login resets state, but the body explicitly verifies the opposite: calling set_access_mode("azure_cli") again preserves the tenant because it does not log out. Rename the test and description to state that the state is preserved, otherwise the test gives future maintainers a false expectation.

    def test_re_login_resets_state(self, mock_credential_class, temp_dir_fixture):
        """Re-login (set_access_mode again) should reset state."""

pyproject.toml:23

• The supported dev-container setup installs requirements-dev.txt directly (scripts/install_dev_container_dependencies.sh:10) rather than installing the project metadata, but that file does not include azure-identity. A clean dev/test container can therefore fail importing fabric_cli.core.fab_auth after this new top-level import; add the dependency to the development requirements or update the setup path to install the project.

    "azure-identity>=1.25.0",

src/fabric_cli/errors/auth.py:139

• This fallback is used while acquiring three different scopes (Fabric, OneLake, and Azure management), but the helper has no scope parameter, so a failure only reports a generic Azure CLI error and does not identify which resource failed. Include the requested scope in this message (or wrap each per-scope call) so users can troubleshoot the failing provider.

    def azure_cli_token_acquisition_failed() -> str:
        return (
            "Unable to get a token from Azure CLI. "
            "Run 'az login' to authenticate, then retry"
        )