Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 36 additions & 2 deletions src/fulfil_cli/auth/keyring_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,46 @@
from __future__ import annotations

import contextlib
import sys

import keyring
from keyring.errors import KeyringLocked, PasswordSetError

from fulfil_cli.client.errors import KeyringError

SERVICE_NAME = "fulfil-cli"

_MACOS_HINT = (
"The macOS Keychain refused to store the credential. This usually means an "
"existing 'fulfil-cli' entry was created by a different binary (e.g. a previous "
"pip/pipx install) and the current binary lacks the entitlement to update it. "
"Delete the existing entry and retry:\n\n"
" security delete-generic-password -s fulfil-cli\n\n"
"Run it repeatedly until it reports 'The specified item could not be found'."
)

_GENERIC_HINT = (
"The system keyring refused to store the credential. Ensure your keyring is "
"unlocked and accessible (e.g. gnome-keyring/KWallet on Linux, Credential "
"Manager on Windows), then retry."
)


def _raise_store_error(exc: Exception) -> None:
"""Translate a keyring storage failure into a KeyringError with remediation."""
hint = _MACOS_HINT if sys.platform == "darwin" else _GENERIC_HINT
raise KeyringError(
message=f"Failed to store credential in system keyring: {exc}",
hint=hint,
) from exc


def store_api_key(workspace: str, api_key: str) -> None:
"""Store an API key in the system keyring."""
keyring.set_password(SERVICE_NAME, workspace, api_key)
try:
keyring.set_password(SERVICE_NAME, workspace, api_key)
except (PasswordSetError, KeyringLocked) as exc:
_raise_store_error(exc)


def get_api_key(workspace: str) -> str | None:
Expand All @@ -29,7 +60,10 @@ def delete_api_key(workspace: str) -> None:

def store_oauth_tokens(workspace: str, tokens_json: str) -> None:
"""Store OAuth tokens in the system keyring."""
keyring.set_password(SERVICE_NAME, f"oauth:{workspace}", tokens_json)
try:
keyring.set_password(SERVICE_NAME, f"oauth:{workspace}", tokens_json)
except (PasswordSetError, KeyringLocked) as exc:
_raise_store_error(exc)


def get_oauth_tokens(workspace: str) -> str | None:
Expand Down
11 changes: 9 additions & 2 deletions src/fulfil_cli/cli/commands/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
store_oauth_tokens,
)
from fulfil_cli.auth.oauth import OAuthTokens, run_oauth_flow
from fulfil_cli.cli.commands.common import handle_error
from fulfil_cli.client.errors import EXIT_CONFIG, EXIT_GENERAL, AuthError, FulfilError
from fulfil_cli.client.http import FulfilClient
from fulfil_cli.config.manager import ConfigManager
Expand All @@ -45,7 +46,10 @@ def _login_api_key(workspace: str, api_key: str | None, config: ConfigManager) -
"(v3 API may not be deployed yet).[/yellow]"
)

store_api_key(workspace, api_key)
try:
store_api_key(workspace, api_key)
except FulfilError as exc:
handle_error(exc, context="auth")
config.set_auth_method(workspace, "api_key")
config.add_workspace(workspace)
config.workspace = workspace
Expand Down Expand Up @@ -77,7 +81,10 @@ def _login_oauth(workspace: str, config: ConfigManager) -> None:
"(v3 API may not be deployed yet).[/yellow]"
)

store_oauth_tokens(workspace, tokens.to_json().decode())
try:
store_oauth_tokens(workspace, tokens.to_json().decode())
except FulfilError as exc:
handle_error(exc, context="auth")
config.set_auth_method(workspace, "oauth")
config.add_workspace(workspace)
config.workspace = workspace
Expand Down
7 changes: 7 additions & 0 deletions src/fulfil_cli/client/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ class ConfigError(FulfilError):
exit_code: int = field(default=EXIT_CONFIG)


@dataclass
class KeyringError(ConfigError):
"""System keyring storage is unavailable or rejected a write."""

exit_code: int = field(default=EXIT_CONFIG)


@dataclass
class AuthError(FulfilError):
"""Authentication failed."""
Expand Down
47 changes: 47 additions & 0 deletions tests/test_cli/test_auth_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,53 @@ def test_login_auth_failure(self, httpx_mock, tmp_config):
assert result.exit_code == 4
assert "failed" in result.stdout.lower()

def test_login_keyring_rejected_macos(self, httpx_mock, jsonrpc_success, tmp_config):
"""macOS keyring rejection should surface the keychain-delete remediation."""
from keyring.errors import PasswordSetError

httpx_mock.add_response(json=jsonrpc_success("1.0.0"))

with (
_patch_config(tmp_config),
patch("fulfil_cli.auth.keyring_store.sys.platform", "darwin"),
patch(
"fulfil_cli.auth.keyring_store.keyring.set_password",
side_effect=PasswordSetError("(-25244, 'Unknown Error')"),
),
):
result = runner.invoke(
app, ["auth", "login", "--workspace", "acme.fulfil.io", "--api-key", "sk_test"]
)

assert result.exit_code == 3
output = result.stdout + result.stderr
assert "security delete-generic-password" in output
assert "acme.fulfil.io" not in tmp_config.workspaces

def test_login_keyring_rejected_linux(self, httpx_mock, jsonrpc_success, tmp_config):
"""Non-macOS keyring rejection should surface the generic unlock remediation."""
from keyring.errors import KeyringLocked

httpx_mock.add_response(json=jsonrpc_success("1.0.0"))

with (
_patch_config(tmp_config),
patch("fulfil_cli.auth.keyring_store.sys.platform", "linux"),
patch(
"fulfil_cli.auth.keyring_store.keyring.set_password",
side_effect=KeyringLocked("keyring is locked"),
),
):
result = runner.invoke(
app, ["auth", "login", "--workspace", "acme.fulfil.io", "--api-key", "sk_test"]
)

assert result.exit_code == 3
output = result.stdout + result.stderr
assert "unlocked" in output.lower()
assert "security delete-generic-password" not in output
assert "acme.fulfil.io" not in tmp_config.workspaces


class TestLogout:
def test_logout_current(self, tmp_config):
Expand Down
Loading