diff --git a/limacharlie/commands/auth.py b/limacharlie/commands/auth.py index c14409c..4205a93 100644 --- a/limacharlie/commands/auth.py +++ b/limacharlie/commands/auth.py @@ -70,11 +70,16 @@ def group() -> None: Two authentication methods are supported: - API Key: limacharlie auth login --oid --api-key - OAuth: limacharlie auth login --oauth [--oid ] + API Key (org-scoped): limacharlie auth login --oid --api-key + API Key (user-scoped): limacharlie auth login --uid --api-key + OAuth: limacharlie auth login --oauth [--oid ] -For API key login, supply --oid and --api-key. If you are using a -user-scoped API key, also pass --uid. +For API key login, supply --api-key together with either --oid (for an +Organization API Key generated under an org's settings) or --uid (for a +User API Key generated under your account profile). User-scoped keys +do not require --oid and are the right choice for brand-new accounts +that have not created an org yet -- run 'auth list-orgs' afterwards to +discover your OIDs. For OAuth login, pass --oauth to authenticate via your browser using Google or Microsoft. Use --provider to choose (default: google). @@ -155,10 +160,29 @@ def login(ctx: click.Context, oid: str | None, api_key: str | None, environment: return write_credentials(env_name, oid=oid, api_key=api_key, uid=uid or "") + + # User-scoped login (--uid + --api-key, no --oid): clear any stale oid + # left over from a previous org-scoped login in this env, otherwise the + # new user creds would silently inherit the old org context. + if not oid and uid: + _clear_stale_oid(env_name) + if not ctx.obj.quiet: click.echo(f"Credentials saved for environment '{env_name}'.") +def _clear_stale_oid(env_name: str) -> None: + """Remove any stale ``oid`` field from the given environment block.""" + config = load_config() or {} + if env_name == "default" or env_name is None: + if config.pop("oid", None) is not None: + save_config(config) + else: + env_data = config.get("env", {}).get(env_name, {}) + if env_data.pop("oid", None) is not None: + save_config(config) + + def _login_oauth(ctx: click.Context, oid: str | None, env_name: str, provider: str, no_browser: bool) -> None: """Perform OAuth login via browser.""" try: diff --git a/tests/unit/test_cli_auth_login.py b/tests/unit/test_cli_auth_login.py new file mode 100644 index 0000000..c5f4256 --- /dev/null +++ b/tests/unit/test_cli_auth_login.py @@ -0,0 +1,242 @@ +"""Tests for ``limacharlie auth login`` argument validation and credential +persistence. + +Covers the three login shapes supported by the CLI: + +* ``--oid + --api-key`` -- org-scoped API key +* ``--uid + --api-key`` (with optional --oid) -- user-scoped API key +* ``--oauth`` -- not exercised here + +and the error paths when required flags are missing. Also asserts that a +user-scoped login clears any stale oid persisted from a previous org-scoped +login in the same environment. +""" + +import os + +import pytest +from click.testing import CliRunner + +from limacharlie.cli import cli +from limacharlie.config import load_config + + +@pytest.fixture +def tmp_config_file(monkeypatch, tmp_path): + """Point config resolution at a fresh temp directory. + + Mirrors the fixture in ``test_config.py`` so each test starts with no + persisted credentials and a clean cache. + """ + from limacharlie.config import _reset_config_cache + from limacharlie.paths import _reset_path_cache + + config_dir = str(tmp_path / "lc_config") + os.makedirs(config_dir, exist_ok=True) + config_path = os.path.join(config_dir, "config.yaml") + + monkeypatch.setenv("LC_CONFIG_DIR", config_dir) + monkeypatch.delenv("LC_CREDS_FILE", raising=False) + monkeypatch.delenv("LC_LEGACY_CONFIG", raising=False) + monkeypatch.delenv("LC_OID", raising=False) + monkeypatch.delenv("LC_API_KEY", raising=False) + monkeypatch.delenv("LC_UID", raising=False) + monkeypatch.delenv("LC_CURRENT_ENV", raising=False) + monkeypatch.delenv("LC_EPHEMERAL_CREDS", raising=False) + _reset_path_cache() + _reset_config_cache() + yield config_path + _reset_path_cache() + _reset_config_cache() + + +class TestLoginValidator: + """The validator should accept any of the three valid flag shapes and + reject everything else with a clear error and exit code 4.""" + + def test_org_scoped_login_succeeds(self, tmp_config_file): + runner = CliRunner() + result = runner.invoke(cli, [ + "auth", "login", + "--oid", "org-1", + "--api-key", "key-1", + ]) + assert result.exit_code == 0, result.output + config = load_config() + assert config["oid"] == "org-1" + assert config["api_key"] == "key-1" + assert "uid" not in config + + def test_user_scoped_login_without_oid_succeeds(self, tmp_config_file): + runner = CliRunner() + result = runner.invoke(cli, [ + "auth", "login", + "--uid", "user-1", + "--api-key", "key-1", + ]) + assert result.exit_code == 0, result.output + config = load_config() + assert config["api_key"] == "key-1" + assert config["uid"] == "user-1" + assert "oid" not in config + + def test_service_account_login_with_all_three_flags(self, tmp_config_file): + runner = CliRunner() + result = runner.invoke(cli, [ + "auth", "login", + "--oid", "org-1", + "--uid", "user-1", + "--api-key", "key-1", + ]) + assert result.exit_code == 0, result.output + config = load_config() + assert config["oid"] == "org-1" + assert config["api_key"] == "key-1" + assert config["uid"] == "user-1" + + def test_missing_api_key_errors(self, tmp_config_file): + runner = CliRunner() + result = runner.invoke(cli, ["auth", "login", "--oid", "org-1"]) + assert result.exit_code == 4 + assert "--api-key is required" in result.output + assert load_config() is None + + def test_missing_oid_and_uid_errors(self, tmp_config_file): + runner = CliRunner() + result = runner.invoke(cli, ["auth", "login", "--api-key", "key-1"]) + assert result.exit_code == 4 + # The error should distinguish the two key types so the user can + # pick the right one from the LC web UI. + assert "--oid" in result.output + assert "--uid" in result.output + assert load_config() is None + + +class TestLoginNamedEnvironment: + """Login should write under the correct env block, not the default + block, when --env is supplied.""" + + def test_user_scoped_login_writes_to_named_env(self, tmp_config_file): + runner = CliRunner() + result = runner.invoke(cli, [ + "auth", "login", + "--env", "staging", + "--uid", "user-1", + "--api-key", "key-1", + ]) + assert result.exit_code == 0, result.output + config = load_config() + assert config["env"]["staging"]["api_key"] == "key-1" + assert config["env"]["staging"]["uid"] == "user-1" + assert "oid" not in config["env"]["staging"] + + +class TestStaleOidCleanup: + """Switching to user-scoped credentials must drop any stale oid left + behind by a previous org-scoped login in the same environment. + Otherwise subsequent commands would pair the new user-scoped api key + with the wrong org context.""" + + def test_user_scoped_relogin_clears_default_oid(self, tmp_config_file): + runner = CliRunner() + # First, an org-scoped login persists oid=org-old. + first = runner.invoke(cli, [ + "auth", "login", + "--oid", "org-old", + "--api-key", "key-old", + ]) + assert first.exit_code == 0, first.output + assert load_config()["oid"] == "org-old" + + # Then a user-scoped login on the same env (no --oid) must drop it. + second = runner.invoke(cli, [ + "auth", "login", + "--uid", "user-new", + "--api-key", "key-new", + ]) + assert second.exit_code == 0, second.output + config = load_config() + assert "oid" not in config + assert config["api_key"] == "key-new" + assert config["uid"] == "user-new" + + def test_user_scoped_relogin_clears_named_env_oid(self, tmp_config_file): + runner = CliRunner() + first = runner.invoke(cli, [ + "auth", "login", + "--env", "staging", + "--oid", "org-old", + "--api-key", "key-old", + ]) + assert first.exit_code == 0, first.output + assert load_config()["env"]["staging"]["oid"] == "org-old" + + second = runner.invoke(cli, [ + "auth", "login", + "--env", "staging", + "--uid", "user-new", + "--api-key", "key-new", + ]) + assert second.exit_code == 0, second.output + env_block = load_config()["env"]["staging"] + assert "oid" not in env_block + assert env_block["api_key"] == "key-new" + assert env_block["uid"] == "user-new" + + def test_user_scoped_relogin_does_not_touch_other_envs(self, tmp_config_file): + runner = CliRunner() + # Persist an oid in 'production' that should be left alone. + prod_login = runner.invoke(cli, [ + "auth", "login", + "--env", "production", + "--oid", "org-prod", + "--api-key", "key-prod", + ]) + assert prod_login.exit_code == 0, prod_login.output + + # User-scoped login in 'staging' must not affect 'production'. + staging_login = runner.invoke(cli, [ + "auth", "login", + "--env", "staging", + "--uid", "user-1", + "--api-key", "key-1", + ]) + assert staging_login.exit_code == 0, staging_login.output + + config = load_config() + assert config["env"]["production"]["oid"] == "org-prod" + assert config["env"]["production"]["api_key"] == "key-prod" + assert "oid" not in config["env"]["staging"] + + def test_org_scoped_relogin_overwrites_oid(self, tmp_config_file): + """Sanity check: org-scoped re-login should *update* the oid, not + leave the old one in place. (This already worked pre-fix; the test + guards against a regression where the cleanup helper accidentally + clobbers the new value.)""" + runner = CliRunner() + first = runner.invoke(cli, [ + "auth", "login", + "--oid", "org-old", + "--api-key", "key-old", + ]) + assert first.exit_code == 0, first.output + + second = runner.invoke(cli, [ + "auth", "login", + "--oid", "org-new", + "--api-key", "key-new", + ]) + assert second.exit_code == 0, second.output + config = load_config() + assert config["oid"] == "org-new" + assert config["api_key"] == "key-new" + + +class TestExplainText: + """The --ai-help text should advertise the user-scoped shape so users + discovering the CLI know the flag combo is supported.""" + + def test_explain_mentions_user_scoped_form(self): + from limacharlie.commands.auth import _EXPLAIN_LOGIN + assert "--uid --api-key " in _EXPLAIN_LOGIN + assert "user-scoped" in _EXPLAIN_LOGIN.lower()