diff --git a/ccproxy/cli/commands/auth.py b/ccproxy/cli/commands/auth.py index f72f4c2b..49aa55d6 100644 --- a/ccproxy/cli/commands/auth.py +++ b/ccproxy/cli/commands/auth.py @@ -16,6 +16,7 @@ from rich.table import Table from structlog import get_logger +from ccproxy.auth.exceptions import CredentialsNotFoundError from ccproxy.cli.helpers import get_rich_toolkit from ccproxy.config.settings import get_settings from ccproxy.core.async_utils import get_claude_docker_home_dir @@ -426,21 +427,25 @@ def login_command( # Check if already logged in manager = get_credentials_manager(custom_paths) - validation_result = asyncio.run(manager.validate()) - if validation_result.valid and not validation_result.expired: - console.print( - "[yellow]You are already logged in with valid credentials.[/yellow]" - ) - console.print( - "Use [cyan]ccproxy auth info[/cyan] to view current credentials." - ) + try: + validation_result = asyncio.run(manager.validate()) + if validation_result.valid and not validation_result.expired: + console.print( + "[yellow]You are already logged in with valid credentials.[/yellow]" + ) + console.print( + "Use [cyan]ccproxy auth info[/cyan] to view current credentials." + ) - overwrite = typer.confirm( - "Do you want to login again and overwrite existing credentials?" - ) - if not overwrite: - console.print("Login cancelled.") - return + overwrite = typer.confirm( + "Do you want to login again and overwrite existing credentials?" + ) + if not overwrite: + console.print("Login cancelled.") + return + except CredentialsNotFoundError: + # No credentials found, proceed with login + pass # Perform OAuth login console.print("Starting OAuth login process...") diff --git a/ccproxy/services/credentials/manager.py b/ccproxy/services/credentials/manager.py index 47ef158a..d8949ade 100644 --- a/ccproxy/services/credentials/manager.py +++ b/ccproxy/services/credentials/manager.py @@ -309,7 +309,7 @@ async def validate(self) -> ValidationResult: """ credentials = await self.load() if not credentials: - raise CredentialsNotFoundError() + raise CredentialsNotFoundError("No credentials found. Please login first.") return ValidationResult( valid=True, diff --git a/tests/unit/cli/test_cli_auth_login_credentials_handling.py b/tests/unit/cli/test_cli_auth_login_credentials_handling.py new file mode 100644 index 00000000..d31787a8 --- /dev/null +++ b/tests/unit/cli/test_cli_auth_login_credentials_handling.py @@ -0,0 +1,310 @@ +"""Tests for CLI auth login command credentials handling. + +This module specifically tests the new CredentialsNotFoundError handling +in the login command that was added to handle cases where no credentials +exist during the login process. +""" + +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from typer.testing import CliRunner + +from ccproxy.auth.exceptions import CredentialsNotFoundError +from ccproxy.auth.models import ( + ClaudeCredentials, + OAuthToken, + ValidationResult, +) +from ccproxy.cli.commands.auth import app +from ccproxy.services.credentials.manager import CredentialsManager + + +class TestLoginCommandCredentialsHandling: + """Test login command credentials handling for CredentialsNotFoundError.""" + + @pytest.fixture + def runner(self) -> CliRunner: + """Create CLI test runner.""" + return CliRunner(env={"NO_COLOR": "1"}) + + @pytest.fixture + def mock_credentials_manager(self) -> AsyncMock: + """Create mock credentials manager.""" + mock = AsyncMock(spec=CredentialsManager) + return mock + + @pytest.fixture + def mock_oauth_token(self) -> OAuthToken: + """Create mock OAuth token.""" + return OAuthToken( + accessToken="sk-test-token-123", + refreshToken="refresh-token-456", + expiresAt=None, + tokenType="Bearer", + subscriptionType="pro", + scopes=["chat", "completions"], + ) + + @pytest.fixture + def mock_credentials(self, mock_oauth_token: OAuthToken) -> ClaudeCredentials: + """Create mock Claude credentials.""" + return ClaudeCredentials(claudeAiOauth=mock_oauth_token) + + @pytest.fixture + def mock_validation_result_valid( + self, mock_credentials: ClaudeCredentials + ) -> ValidationResult: + """Create valid validation result.""" + return ValidationResult( + valid=True, + expired=False, + path="/home/user/.claude/credentials.json", + credentials=mock_credentials, + ) + + @pytest.fixture + def mock_validation_result_invalid(self) -> ValidationResult: + """Create invalid validation result.""" + return ValidationResult( + valid=False, + expired=False, + path=None, + credentials=None, + ) + + @patch("ccproxy.cli.commands.auth.get_credentials_manager") + def test_login_command_no_existing_credentials( + self, + mock_get_manager: MagicMock, + runner: CliRunner, + mock_credentials_manager: AsyncMock, + mock_validation_result_valid: ValidationResult, + ) -> None: + """Test login command when no existing credentials are found. + + This tests the new CredentialsNotFoundError handling that was added + to allow login to proceed when no credentials exist. + """ + mock_get_manager.return_value = mock_credentials_manager + + # First validate() call raises CredentialsNotFoundError (no existing credentials) + # Second validate() call returns valid result (after successful login) + mock_credentials_manager.validate.side_effect = [ + CredentialsNotFoundError("No credentials found. Please login first."), + mock_validation_result_valid, + ] + mock_credentials_manager.login.return_value = None + + result = runner.invoke(app, ["login"]) + + assert result.exit_code == 0 + assert "Successfully logged in to Claude!" in result.stdout + + # Verify that login was called despite the CredentialsNotFoundError + mock_credentials_manager.login.assert_called_once() + + # Verify that validate was called twice (once for check, once for final validation) + assert mock_credentials_manager.validate.call_count == 2 + + @patch("ccproxy.cli.commands.auth.get_credentials_manager") + def test_login_command_credentials_not_found_then_login_success( + self, + mock_get_manager: MagicMock, + runner: CliRunner, + mock_credentials_manager: AsyncMock, + mock_validation_result_valid: ValidationResult, + ) -> None: + """Test login flow when CredentialsNotFoundError is raised initially. + + This specifically tests that the try-catch block around manager.validate() + properly handles CredentialsNotFoundError and allows login to proceed. + """ + mock_get_manager.return_value = mock_credentials_manager + + # Mock the sequence: no credentials found -> login succeeds -> validation succeeds + mock_credentials_manager.validate.side_effect = [ + CredentialsNotFoundError("No credentials found. Please login first."), + mock_validation_result_valid, + ] + mock_credentials_manager.login.return_value = None + + result = runner.invoke(app, ["login"]) + + assert result.exit_code == 0 + assert "Starting OAuth login process..." in result.stdout + assert "Successfully logged in to Claude!" in result.stdout + + # Should not show the "already logged in" message + assert "You are already logged in with valid credentials" not in result.stdout + + # Verify login was attempted + mock_credentials_manager.login.assert_called_once() + + @patch("ccproxy.cli.commands.auth.get_credentials_manager") + def test_login_command_existing_valid_credentials_overwrite_yes( + self, + mock_get_manager: MagicMock, + runner: CliRunner, + mock_credentials_manager: AsyncMock, + mock_validation_result_valid: ValidationResult, + ) -> None: + """Test login command with existing valid credentials and user chooses to overwrite.""" + mock_get_manager.return_value = mock_credentials_manager + + # First validate() returns valid credentials (already logged in) + # Second validate() returns valid result (after re-login) + mock_credentials_manager.validate.side_effect = [ + mock_validation_result_valid, + mock_validation_result_valid, + ] + mock_credentials_manager.login.return_value = None + + # Simulate user saying "yes" to overwrite + result = runner.invoke(app, ["login"], input="y\n") + + assert result.exit_code == 0 + assert "You are already logged in with valid credentials" in result.stdout + assert ( + "Do you want to login again and overwrite existing credentials?" + in result.stdout + ) + assert "Successfully logged in to Claude!" in result.stdout + + # Verify login was called after user confirmed overwrite + mock_credentials_manager.login.assert_called_once() + + @patch("ccproxy.cli.commands.auth.get_credentials_manager") + def test_login_command_existing_valid_credentials_overwrite_no( + self, + mock_get_manager: MagicMock, + runner: CliRunner, + mock_credentials_manager: AsyncMock, + mock_validation_result_valid: ValidationResult, + ) -> None: + """Test login command with existing valid credentials and user chooses not to overwrite.""" + mock_get_manager.return_value = mock_credentials_manager + mock_credentials_manager.validate.return_value = mock_validation_result_valid + + # Simulate user saying "no" to overwrite + result = runner.invoke(app, ["login"], input="n\n") + + assert result.exit_code == 0 + assert "You are already logged in with valid credentials" in result.stdout + assert "Login cancelled" in result.stdout + + # Verify login was NOT called + mock_credentials_manager.login.assert_not_called() + + @patch("ccproxy.cli.commands.auth.get_credentials_manager") + def test_login_command_credentials_not_found_with_docker_flag( + self, + mock_get_manager: MagicMock, + runner: CliRunner, + mock_credentials_manager: AsyncMock, + mock_validation_result_valid: ValidationResult, + ) -> None: + """Test login command with --docker flag when no credentials exist.""" + mock_get_manager.return_value = mock_credentials_manager + + # CredentialsNotFoundError on first check, then successful login + mock_credentials_manager.validate.side_effect = [ + CredentialsNotFoundError("No credentials found. Please login first."), + mock_validation_result_valid, + ] + mock_credentials_manager.login.return_value = None + + result = runner.invoke(app, ["login", "--docker"]) + + assert result.exit_code == 0 + assert "Successfully logged in to Claude!" in result.stdout + + # Verify that get_credentials_manager was called with Docker paths + mock_get_manager.assert_called_once() + call_args = mock_get_manager.call_args + if len(call_args[0]) > 0: + custom_paths = call_args[0][0] + else: + custom_paths = call_args.kwargs.get("custom_paths") + assert custom_paths is not None + assert any(".claude" in str(path) for path in custom_paths) + + @patch("ccproxy.cli.commands.auth.get_credentials_manager") + def test_login_command_credentials_not_found_with_custom_file( + self, + mock_get_manager: MagicMock, + runner: CliRunner, + mock_credentials_manager: AsyncMock, + mock_validation_result_valid: ValidationResult, + ) -> None: + """Test login command with --credential-file flag when no credentials exist.""" + mock_get_manager.return_value = mock_credentials_manager + + # CredentialsNotFoundError on first check, then successful login + mock_credentials_manager.validate.side_effect = [ + CredentialsNotFoundError("No credentials found. Please login first."), + mock_validation_result_valid, + ] + mock_credentials_manager.login.return_value = None + custom_file = "/custom/credentials.json" + + result = runner.invoke(app, ["login", "--credential-file", custom_file]) + + assert result.exit_code == 0 + assert "Successfully logged in to Claude!" in result.stdout + + # Verify that get_credentials_manager was called with custom file path + mock_get_manager.assert_called_once() + call_args = mock_get_manager.call_args + if len(call_args[0]) > 0: + custom_paths = call_args[0][0] + else: + custom_paths = call_args.kwargs.get("custom_paths") + assert custom_paths == [Path(custom_file)] + + @patch("ccproxy.cli.commands.auth.get_credentials_manager") + def test_login_command_other_exception_during_validation( + self, + mock_get_manager: MagicMock, + runner: CliRunner, + mock_credentials_manager: AsyncMock, + ) -> None: + """Test that other exceptions during validation are not caught by the CredentialsNotFoundError handler.""" + mock_get_manager.return_value = mock_credentials_manager + + # Some other exception that should not be caught + mock_credentials_manager.validate.side_effect = RuntimeError("Some other error") + + result = runner.invoke(app, ["login"]) + + # Should exit with error code due to unhandled exception + assert result.exit_code == 1 + assert "Error during login: Some other error" in result.stdout + + # Login should not be called due to the exception + mock_credentials_manager.login.assert_not_called() + + @patch("ccproxy.cli.commands.auth.get_credentials_manager") + def test_login_command_login_fails_after_credentials_not_found( + self, + mock_get_manager: MagicMock, + runner: CliRunner, + mock_credentials_manager: AsyncMock, + ) -> None: + """Test login command when login fails after CredentialsNotFoundError.""" + mock_get_manager.return_value = mock_credentials_manager + + # No existing credentials, but login fails + mock_credentials_manager.validate.side_effect = CredentialsNotFoundError( + "No credentials found. Please login first." + ) + mock_credentials_manager.login.side_effect = Exception("Login failed") + + result = runner.invoke(app, ["login"]) + + assert result.exit_code == 1 + assert "Login failed. Please try again." in result.stdout + + # Verify login was attempted + mock_credentials_manager.login.assert_called_once() diff --git a/tests/unit/services/test_credentials_manager_validation.py b/tests/unit/services/test_credentials_manager_validation.py new file mode 100644 index 00000000..9603b770 --- /dev/null +++ b/tests/unit/services/test_credentials_manager_validation.py @@ -0,0 +1,254 @@ +"""Tests for credentials manager validation method. + +This module specifically tests the changes to the validate() method in +CredentialsManager that now raises CredentialsNotFoundError with a +descriptive error message when no credentials are found. +""" + +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from ccproxy.auth.exceptions import CredentialsNotFoundError +from ccproxy.auth.models import ClaudeCredentials, OAuthToken, ValidationResult +from ccproxy.config.auth import AuthSettings +from ccproxy.services.credentials.manager import CredentialsManager + + +class TestCredentialsManagerValidation: + """Test CredentialsManager validation method changes.""" + + @pytest.fixture + def auth_settings(self) -> AuthSettings: + """Create auth settings for testing.""" + return AuthSettings() + + @pytest.fixture + def mock_storage(self) -> AsyncMock: + """Create mock storage backend.""" + mock = AsyncMock() + # Make get_location return a string, not a coroutine + mock.get_location = MagicMock( + return_value="/home/user/.claude/credentials.json" + ) + return mock + + @pytest.fixture + def mock_oauth_client(self) -> AsyncMock: + """Create mock OAuth client.""" + mock = AsyncMock() + return mock + + @pytest.fixture + def credentials_manager( + self, + auth_settings: AuthSettings, + mock_storage: AsyncMock, + mock_oauth_client: AsyncMock, + ) -> CredentialsManager: + """Create credentials manager with mocked dependencies.""" + return CredentialsManager( + config=auth_settings, + storage=mock_storage, + oauth_client=mock_oauth_client, + ) + + @pytest.fixture + def mock_oauth_token(self) -> MagicMock: + """Create mock OAuth token.""" + mock_token = MagicMock(spec=OAuthToken) + mock_token.accessToken = "sk-test-token-123" + mock_token.refreshToken = "refresh-token-456" + mock_token.expiresAt = None + mock_token.tokenType = "Bearer" + mock_token.subscriptionType = "pro" + mock_token.scopes = ["chat", "completions"] + mock_token.is_expired = False # Default to not expired + return mock_token + + @pytest.fixture + def mock_credentials(self, mock_oauth_token: MagicMock) -> MagicMock: + """Create mock Claude credentials.""" + mock_creds = MagicMock(spec=ClaudeCredentials) + mock_creds.claude_ai_oauth = mock_oauth_token + mock_creds.claudeAiOauth = ( + mock_oauth_token # Both property names for compatibility + ) + return mock_creds + + async def test_validate_with_valid_credentials( + self, + credentials_manager: CredentialsManager, + mock_storage: AsyncMock, + mock_credentials: MagicMock, + ) -> None: + """Test validate method with valid credentials.""" + # Mock storage to return valid credentials + mock_storage.load.return_value = mock_credentials + mock_storage.get_location.return_value = "/home/user/.claude/credentials.json" + + result = await credentials_manager.validate() + + assert isinstance(result, ValidationResult) + assert result.valid is True + assert result.expired == mock_credentials.claude_ai_oauth.is_expired + assert result.credentials == mock_credentials + assert result.path == "/home/user/.claude/credentials.json" + + # Verify storage was called + mock_storage.load.assert_called_once() + mock_storage.get_location.assert_called_once() + + async def test_validate_with_no_credentials_raises_descriptive_error( + self, + credentials_manager: CredentialsManager, + mock_storage: AsyncMock, + ) -> None: + """Test validate method raises CredentialsNotFoundError with descriptive message when no credentials found. + + This tests the specific change where the validate() method now raises + CredentialsNotFoundError with the message "No credentials found. Please login first." + instead of just returning an invalid ValidationResult. + """ + # Mock storage to return None (no credentials) + mock_storage.load.return_value = None + + with pytest.raises(CredentialsNotFoundError) as exc_info: + await credentials_manager.validate() + + # Verify the specific error message that was added + assert str(exc_info.value) == "No credentials found. Please login first." + + # Verify storage was called + mock_storage.load.assert_called_once() + # get_location should not be called when no credentials exist + mock_storage.get_location.assert_not_called() + + async def test_validate_with_expired_credentials( + self, + credentials_manager: CredentialsManager, + mock_storage: AsyncMock, + mock_credentials: MagicMock, + ) -> None: + """Test validate method with expired credentials.""" + # Set the token to be expired + mock_credentials.claude_ai_oauth.is_expired = True + mock_storage.load.return_value = mock_credentials + mock_storage.get_location.return_value = "/home/user/.claude/credentials.json" + + result = await credentials_manager.validate() + + assert isinstance(result, ValidationResult) + assert result.valid is True # Still valid, just expired + assert result.expired is True + assert result.credentials == mock_credentials + assert result.path == "/home/user/.claude/credentials.json" + + async def test_validate_with_storage_exception( + self, + credentials_manager: CredentialsManager, + mock_storage: AsyncMock, + ) -> None: + """Test validate method when storage raises an exception.""" + # Mock storage to raise an exception + mock_storage.load.side_effect = Exception("Storage error") + + with pytest.raises(CredentialsNotFoundError) as exc_info: + await credentials_manager.validate() + + # Should still raise CredentialsNotFoundError with the descriptive message + # because load() returns None when it catches exceptions + assert str(exc_info.value) == "No credentials found. Please login first." + + async def test_validate_preserves_existing_behavior_for_valid_credentials( + self, + credentials_manager: CredentialsManager, + mock_storage: AsyncMock, + mock_credentials: MagicMock, + ) -> None: + """Test that validate method preserves existing behavior for valid credentials.""" + # Set up non-expired token + mock_credentials.claude_ai_oauth.is_expired = False + mock_storage.load.return_value = mock_credentials + mock_storage.get_location.return_value = "/home/user/.claude/credentials.json" + + result = await credentials_manager.validate() + + # Verify all the expected fields are set correctly + assert result.valid is True + assert result.expired is False + assert result.credentials is mock_credentials + assert result.path == "/home/user/.claude/credentials.json" + + async def test_validate_error_message_consistency( + self, + credentials_manager: CredentialsManager, + mock_storage: AsyncMock, + ) -> None: + """Test that the error message is consistent with other methods. + + This ensures the error message matches what's used in other methods + like get_valid_credentials() for consistency. + """ + mock_storage.load.return_value = None + + with pytest.raises(CredentialsNotFoundError) as exc_info: + await credentials_manager.validate() + + # The error message should match what's used in get_valid_credentials + expected_message = "No credentials found. Please login first." + assert str(exc_info.value) == expected_message + + @patch("ccproxy.services.credentials.manager.logger") + async def test_validate_no_credentials_logging( + self, + mock_logger: MagicMock, + credentials_manager: CredentialsManager, + mock_storage: AsyncMock, + ) -> None: + """Test that appropriate logging occurs when no credentials are found.""" + mock_storage.load.return_value = None + + with pytest.raises(CredentialsNotFoundError): + await credentials_manager.validate() + + # The load method should log the error when it returns None + # This verifies the existing logging behavior is preserved + mock_storage.load.assert_called_once() + + async def test_validate_integration_with_load_method( + self, + auth_settings: AuthSettings, + ) -> None: + """Test validate method integration with actual load method behavior.""" + # Create manager without mocked storage to test real integration + manager = CredentialsManager(config=auth_settings) + + # Mock the storage's load method to return None + with patch.object(manager.storage, "load", return_value=None): + with pytest.raises(CredentialsNotFoundError) as exc_info: + await manager.validate() + + assert str(exc_info.value) == "No credentials found. Please login first." + + async def test_validate_method_signature_unchanged( + self, + credentials_manager: CredentialsManager, + mock_storage: AsyncMock, + mock_credentials: MagicMock, + ) -> None: + """Test that the validate method signature and return type are unchanged for valid credentials.""" + mock_storage.load.return_value = mock_credentials + mock_storage.get_location.return_value = "/test/path" + + result = await credentials_manager.validate() + + # Verify the return type is still ValidationResult + assert isinstance(result, ValidationResult) + + # Verify all expected attributes exist + assert hasattr(result, "valid") + assert hasattr(result, "expired") + assert hasattr(result, "credentials") + assert hasattr(result, "path")