tests: unit tests for CLI PAT parsing logic

Author: RMCrean

pytest.ini

@@ -11,6 +11,7 @@ env =
     # divbase-cli test settings
     DIVBASE_CLI_CONFIG_PATH=tests/fixtures/tmp/config.yaml
     DIVBASE_CLI_TOKENS_PATH=tests/fixtures/tmp/.fakesecrets
+    DIVBASE_CLI_PATS_PATH=tests/fixtures/tmp/.fakepat
     DIVBASE_KEYRING_SERVICE=divbase-cli-test
     DIVBASE_API_URL=http://localhost:8001/api
     # API and worker test settings (used in unit tests - as integration/e2e get the settings from docker-compose)

tests/unit/divbase_cli/test_user_auth.py

@@ -10,11 +10,17 @@
 from pydantic import SecretStr
 
 from divbase_cli.cli_config import cli_settings
+from divbase_cli.cli_exceptions import AuthenticationError
 from divbase_cli.user_auth import (
+    PATData,
     TokenData,
     _delete_stored_jwts,
     check_existing_session,
+    delete_stored_pat,
+    get_pat_for_authentication,
+    load_stored_user_pat,
     load_user_tokens,
+    make_authenticated_request,
 )
 
 # cli config set to use "divbase-cli-test" (via pytest.ini) as the keyring service name,
@@ -27,7 +33,9 @@ def clean_up_keyring_entries():
     """Clean up any test keyring entries added after each test"""
     yield
     with contextlib.suppress(NoKeyringError, PasswordDeleteError):
-        keyring.delete_password(service_name=_TEST_KEYRING_SERVICE, username=cli_settings.KEYRING_USERNAME)
+        keyring.delete_password(service_name=_TEST_KEYRING_SERVICE, username=cli_settings.KEYRING_TOKENS_USERNAME)
+    with contextlib.suppress(NoKeyringError, PasswordDeleteError):
+        keyring.delete_password(service_name=_TEST_KEYRING_SERVICE, username=cli_settings.KEYRING_PATS_USERNAME)
 
 
 @pytest.fixture
@@ -105,7 +113,7 @@ def test_dump_tokens_falls_back_to_file_when_no_keyring(mock_set_password, mock_
     If attempt to dump tokens with keyring fails, tokens should be written to a file with 0600 permissions."""
     output = tmp_path / ".secrets"
 
-    mock_token_data.dump_tokens(output_path=output)
+    mock_token_data.dump_tokens(fallback_output_path=output)
 
     assert output.exists()
     file_mode = stat.S_IMODE(output.stat().st_mode)
@@ -117,7 +125,7 @@ def test_dump_tokens_falls_back_to_file_on_keyring_error(mock_set_password, mock
     """dump_tokens writes a 0600 file when an unexpected keyring error occurs."""
     output = tmp_path / ".secrets"
 
-    mock_token_data.dump_tokens(output_path=output)
+    mock_token_data.dump_tokens(fallback_output_path=output)
 
     assert output.exists()
     file_mode = stat.S_IMODE(output.stat().st_mode)
@@ -200,3 +208,151 @@ def test_load_user_tokens_keyring_takes_priority_over_file(mock_get_password, mo
     result = load_user_tokens(token_path=token_file)
     assert result is not None
     assert result.access_token.get_secret_value() == "keyring_access"
+
+
+@patch("divbase_cli.user_auth.httpx.request")
+@patch("divbase_cli.user_auth.load_user_tokens")
+def test_make_authenticated_request_uses_jwt_over_pat(mock_load_tokens, mock_request, mock_token_data, monkeypatch):
+    """JWT session should take priority over PAT when both are available."""
+    mock_load_tokens.return_value = mock_token_data
+    monkeypatch.setattr(cli_settings, "DIVBASE_API_PAT", SecretStr("divbase_pat_should_not_be_used"))
+    mock_response = MagicMock()
+    mock_response.raise_for_status = MagicMock()
+    mock_request.return_value = mock_response
+
+    make_authenticated_request(method="GET", divbase_base_url="https://example.com", api_route="v1/test")
+
+    auth_header = mock_request.call_args.kwargs["headers"]["Authorization"]
+    assert auth_header == f"Bearer {mock_token_data.access_token.get_secret_value()}"
+    assert "divbase_pat_should_not_be_used" not in auth_header
+
+
+@pytest.fixture
+def mock_pat_data():
+    return PATData(
+        name="test-pat",
+        pat=SecretStr("divbase_pat_faketoken"),
+        pat_expires_at=9999999999,
+    )
+
+
+@patch("divbase_cli.user_auth.keyring.set_password", side_effect=NoKeyringError)
+def test_dump_pat_data_falls_back_to_file_when_no_keyring(mock_set_password, mock_pat_data, tmp_path):
+    """dump_pat_data writes a 0600 file when keyring is unavailable."""
+    output = tmp_path / ".pat"
+
+    mock_pat_data.dump_pat_data(fallback_output_path=output)
+
+    assert output.exists()
+    file_mode = stat.S_IMODE(output.stat().st_mode)
+    # NOTE: this test has not been tried on windows, the test could fail there as windows doesn't use unix style permissions,
+    # we can consider dropping the 0600 part of the test if it becomes an issue.
+    # windows users will use keyring anyway so this is not a problem.
+    assert file_mode == 0o600
+
+
+@patch("divbase_cli.user_auth.keyring.get_password")
+def test_load_stored_user_pat_reads_from_keyring(mock_get_password, mock_pat_data, tmp_path):
+    """load_stored_user_pat returns PAT from keyring when present."""
+    mock_get_password.return_value = json.dumps(
+        {
+            "name": mock_pat_data.name,
+            "pat": mock_pat_data.pat.get_secret_value(),
+            "pat_expires_at": mock_pat_data.pat_expires_at,
+        }
+    )
+
+    result = load_stored_user_pat(pat_fallback_path=tmp_path / ".pat")
+
+    assert result is not None
+    assert result.name == "test-pat"
+    assert result.pat.get_secret_value() == "divbase_pat_faketoken"
+
+
+@patch("divbase_cli.user_auth.keyring.get_password", side_effect=NoKeyringError)
+def test_load_stored_user_pat_falls_back_to_file_when_no_keyring(mock_get_password, mock_pat_data, tmp_path):
+    """load_stored_user_pat reads from file when keyring is unavailable."""
+    pat_file = tmp_path / ".pat"
+    pat_file.write_text(
+        f"name: {mock_pat_data.name}\n"
+        f"pat: {mock_pat_data.pat.get_secret_value()}\n"
+        f"pat_expires_at: {mock_pat_data.pat_expires_at}\n"
+    )
+
+    result = load_stored_user_pat(pat_fallback_path=pat_file)
+
+    assert result is not None
+    assert result.name == "test-pat"
+
+
+@patch("divbase_cli.user_auth.keyring.get_password")
+def test_load_stored_user_pat_keyring_takes_priority_over_file(mock_get_password, mock_pat_data, tmp_path):
+    """When PAT exists in both keyring and fallback file, keyring takes priority."""
+    mock_get_password.return_value = json.dumps(
+        {
+            "name": "keyring-pat",
+            "pat": "divbase_pat_keyringtoken",
+            "pat_expires_at": mock_pat_data.pat_expires_at,
+        }
+    )
+    pat_file = tmp_path / ".pat"
+    pat_file.write_text(
+        f"name: {mock_pat_data.name}\n"
+        f"pat: {mock_pat_data.pat.get_secret_value()}\n"
+        f"pat_expires_at: {mock_pat_data.pat_expires_at}\n"
+    )
+
+    result = load_stored_user_pat(pat_fallback_path=pat_file)
+    assert result is not None
+    assert result.name == "keyring-pat"
+
+
+def test_get_pat_for_authentication_returns_env_var_pat(monkeypatch):
+    """get_pat_for_authentication returns env var PAT when set, without hitting keyring."""
+    env_pat = SecretStr("divbase_pat_from_env")
+    monkeypatch.setattr(cli_settings, "DIVBASE_API_PAT", env_pat)
+
+    result = get_pat_for_authentication()
+    assert result is env_pat
+
+
+@patch("divbase_cli.user_auth.load_stored_user_pat")
+def test_get_pat_for_authentication_raises_on_expired_pat(mock_load, mock_pat_data, monkeypatch):
+    """get_pat_for_authentication raises AuthenticationError when stored PAT is expired."""
+
+    monkeypatch.setattr(cli_settings, "DIVBASE_API_PAT", None)
+    mock_pat_data.pat_expires_at = int(time.time()) - 60
+    mock_load.return_value = mock_pat_data
+
+    with pytest.raises(AuthenticationError, match="expired"):
+        get_pat_for_authentication()
+
+
+@patch("divbase_cli.user_auth.keyring.delete_password", side_effect=PasswordDeleteError)
+def test_delete_stored_pat_can_run_multiple_times(mock_delete_password, tmp_path):
+    """delete_stored_pat should be idempotent."""
+    pat_file = tmp_path / ".pat"
+    delete_stored_pat(pat_file)
+    delete_stored_pat(pat_file)
+    delete_stored_pat(pat_file)
+
+
+@pytest.fixture
+def mock_pat_data_no_expiry():
+    return PATData(
+        name="no-expiry-pat",
+        pat=SecretStr("divbase_pat_noexpirytoken"),
+        pat_expires_at=None,
+    )
+
+
+@patch("divbase_cli.user_auth.load_stored_user_pat")
+def test_get_pat_for_authentication_no_expiry_pat_returns_pat(mock_load, mock_pat_data_no_expiry, monkeypatch):
+    """get_pat_for_authentication returns the PAT without raising when pat_expires_at is None."""
+    monkeypatch.setattr(cli_settings, "DIVBASE_API_PAT", None)
+    mock_load.return_value = mock_pat_data_no_expiry
+
+    result = get_pat_for_authentication()
+
+    assert result is not None
+    assert result.get_secret_value() == mock_pat_data_no_expiry.pat.get_secret_value()