diff --git a/CHANGELOG.md b/CHANGELOG.md index d076d1c9..72b50927 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added +- `CLOUDSMITH_KEYRING_BACKEND` is now accepted as an alias for `PYTHON_KEYRING_BACKEND`. If both are set, `PYTHON_KEYRING_BACKEND` takes precedence. +- `CLOUDSMITH_KEYRING_KEY` is now accepted as an alias for `KEYRING_PROPERTY_KEYRING_KEY`, letting the bundled `keyrings.cryptfile`/`keyrings.alt` encrypted backends be unlocked non-interactively (e.g. in headless containers) without an interactive `getpass()` prompt. If both are set, `KEYRING_PROPERTY_KEYRING_KEY` takes precedence. Note: both bundled backends override `KeyringBackend.__init__` without calling `super().__init__()`, so `keyring`'s own automatic `KEYRING_PROPERTY_*` handling never runs for them — we apply it ourselves after resolving the backend. - `cloudsmith auth --no-browser` skips the automatic browser launch and prints the SAML IDP URL to open manually, for shells where launching a browser is unwanted or unreliable. ### Fixed diff --git a/cloudsmith_cli/core/keyring.py b/cloudsmith_cli/core/keyring.py index e39fd98a..5e721098 100644 --- a/cloudsmith_cli/core/keyring.py +++ b/cloudsmith_cli/core/keyring.py @@ -24,7 +24,36 @@ def _get_username(): return getpass.getuser() +def _sync_keyring_backend_env(): + """Allow CLOUDSMITH_KEYRING_BACKEND to alias PYTHON_KEYRING_BACKEND.""" + alias_value = os.environ.get("CLOUDSMITH_KEYRING_BACKEND") + if alias_value and "PYTHON_KEYRING_BACKEND" not in os.environ: + os.environ["PYTHON_KEYRING_BACKEND"] = alias_value + + +def _sync_keyring_property_env(): + """Allow CLOUDSMITH_KEYRING_KEY to alias KEYRING_PROPERTY_KEYRING_KEY.""" + alias_value = os.environ.get("CLOUDSMITH_KEYRING_KEY") + if alias_value and "KEYRING_PROPERTY_KEYRING_KEY" not in os.environ: + os.environ["KEYRING_PROPERTY_KEYRING_KEY"] = alias_value + + +def _prepare_keyring_backend(): + """Resolve env var aliases and apply them to the keyring backend. + + keyrings.cryptfile and keyrings.alt override KeyringBackend.__init__ + without calling super(), so KEYRING_PROPERTY_* env vars (e.g. the + keyring_key password for those encrypted file backends) never reach + them through the library's own documented mechanism. Apply them here + instead, once the backend has been resolved. + """ + _sync_keyring_backend_env() + _sync_keyring_property_env() + keyring.get_keyring().set_properties_from_env() + + def _get_value(key): + _prepare_keyring_backend() username = _get_username() try: return keyring.get_password(key, username) @@ -33,6 +62,7 @@ def _get_value(key): def _set_value(key, value): + _prepare_keyring_backend() username = _get_username() keyring.set_password(key, username, value) @@ -112,6 +142,7 @@ def store_sso_tokens(api_host, access_token, refresh_token): def _delete_value(key): + _prepare_keyring_backend() username = _get_username() try: keyring.delete_password(key, username) diff --git a/cloudsmith_cli/core/tests/test_keyring.py b/cloudsmith_cli/core/tests/test_keyring.py index 863710b1..15bd7a4c 100644 --- a/cloudsmith_cli/core/tests/test_keyring.py +++ b/cloudsmith_cli/core/tests/test_keyring.py @@ -47,6 +47,12 @@ def mock_delete_password(): yield delete_password_mock +@pytest.fixture(autouse=True) +def mock_get_keyring(): + with patch.object(keyring, "get_keyring") as get_keyring_mock: + yield get_keyring_mock + + class TestKeyring: api_host = "https://example.com" @@ -254,6 +260,94 @@ def test_returns_true_when_env_var_is_falsy(self, env_value): assert should_use_keyring() is True +class TestKeyringBackendAlias: + """Tests for CLOUDSMITH_KEYRING_BACKEND aliasing PYTHON_KEYRING_BACKEND.""" + + api_host = "https://example.com" + + def test_sets_python_keyring_backend_when_unset( + self, mock_get_user, mock_get_password + ): + env = os.environ.copy() + env.pop("PYTHON_KEYRING_BACKEND", None) + env["CLOUDSMITH_KEYRING_BACKEND"] = "keyring.backends.null.Keyring" + with patch.dict(os.environ, env, clear=True): + get_access_token(self.api_host) + assert ( + os.environ["PYTHON_KEYRING_BACKEND"] == "keyring.backends.null.Keyring" + ) + + def test_does_not_override_existing_python_keyring_backend( + self, mock_get_user, mock_get_password + ): + env = os.environ.copy() + env["PYTHON_KEYRING_BACKEND"] = "keyring.backends.SecretService.Keyring" + env["CLOUDSMITH_KEYRING_BACKEND"] = "keyring.backends.null.Keyring" + with patch.dict(os.environ, env, clear=True): + get_access_token(self.api_host) + assert ( + os.environ["PYTHON_KEYRING_BACKEND"] + == "keyring.backends.SecretService.Keyring" + ) + + def test_no_op_when_alias_not_set(self, mock_get_user, mock_get_password): + env = os.environ.copy() + env.pop("PYTHON_KEYRING_BACKEND", None) + env.pop("CLOUDSMITH_KEYRING_BACKEND", None) + with patch.dict(os.environ, env, clear=True): + get_access_token(self.api_host) + assert "PYTHON_KEYRING_BACKEND" not in os.environ + + +class TestKeyringPropertyAlias: + """Tests for CLOUDSMITH_KEYRING_KEY aliasing KEYRING_PROPERTY_KEYRING_KEY. + + keyrings.cryptfile and keyrings.alt override KeyringBackend.__init__ + without calling super(), so KEYRING_PROPERTY_* env vars never reach + those backends via the library's own documented mechanism; we apply + them ourselves by calling set_properties_from_env() on the resolved + backend. + """ + + api_host = "https://example.com" + + def test_sets_keyring_property_when_unset( + self, mock_get_user, mock_get_password, mock_get_keyring + ): + env = os.environ.copy() + env.pop("KEYRING_PROPERTY_KEYRING_KEY", None) + env["CLOUDSMITH_KEYRING_KEY"] = "super-secret" + with patch.dict(os.environ, env, clear=True): + get_access_token(self.api_host) + assert os.environ["KEYRING_PROPERTY_KEYRING_KEY"] == "super-secret" + + def test_does_not_override_existing_keyring_property( + self, mock_get_user, mock_get_password, mock_get_keyring + ): + env = os.environ.copy() + env["KEYRING_PROPERTY_KEYRING_KEY"] = "native-secret" + env["CLOUDSMITH_KEYRING_KEY"] = "alias-secret" + with patch.dict(os.environ, env, clear=True): + get_access_token(self.api_host) + assert os.environ["KEYRING_PROPERTY_KEYRING_KEY"] == "native-secret" + + def test_no_op_when_alias_not_set( + self, mock_get_user, mock_get_password, mock_get_keyring + ): + env = os.environ.copy() + env.pop("KEYRING_PROPERTY_KEYRING_KEY", None) + env.pop("CLOUDSMITH_KEYRING_KEY", None) + with patch.dict(os.environ, env, clear=True): + get_access_token(self.api_host) + assert "KEYRING_PROPERTY_KEYRING_KEY" not in os.environ + + def test_applies_properties_to_resolved_backend( + self, mock_get_user, mock_get_password, mock_get_keyring + ): + get_access_token(self.api_host) + mock_get_keyring.return_value.set_properties_from_env.assert_called_once() + + class TestDeleteSsoTokens: """Tests for the delete_sso_tokens and has_sso_tokens functions."""