diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a1d83619..b565c2c3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -109,8 +109,6 @@ jobs: PYTEST_CLOUDSMITH_API_KEY: ${{ secrets.PYTEST_CLOUDSMITH_API_KEY }} PYTEST_CLOUDSMITH_API_HOST: ${{ vars.PYTEST_CLOUDSMITH_API_HOST }} PYTEST_CLOUDSMITH_ORGANIZATION: ${{ vars.PYTEST_CLOUDSMITH_ORGANIZATION }} - PYTEST_CLOUDSMITH_USERNAME: ${{ secrets.PYTEST_CLOUDSMITH_USERNAME }} - PYTEST_CLOUDSMITH_PASSWORD: ${{ secrets.PYTEST_CLOUDSMITH_PASSWORD }} run: uv run pytest -m integration --junitxml=./reports/pytest.xml - name: Upload test results diff --git a/AGENTS.md b/AGENTS.md index 17fba264..9bde1885 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,7 +30,7 @@ The CLI is a Click app that wraps the auto-generated `cloudsmith-api` Python SDK ### Layered layout -- `cloudsmith_cli/cli/` — Click commands, decorators, output formatting, validators, config-file parsing, SAML browser-callback webserver. API **call sites** belong in `core/api/*`; commands invoke those wrappers rather than calling the SDK directly. (A few commands — `login`, `logout`, `check` — do import `cloudsmith_api` for `Configuration` defaults or exception classes; that's fine, but new request code should go through `core/api/*`.) +- `cloudsmith_cli/cli/` — Click commands, decorators, output formatting, validators, config-file parsing, SAML browser-callback webserver. API **call sites** belong in `core/api/*`; commands invoke those wrappers rather than calling the SDK directly. (A few commands — `logout`, `check` — do import `cloudsmith_api` for `Configuration` defaults or exception classes; that's fine, but new request code should go through `core/api/*`.) - `cli/commands/main.py` defines the top-level `main` Click group (`cls=AliasGroup`). Every other command module imports `main` and registers itself with `@main.command(...)` or `@main.group(...)`. Command modules are imported lazily: `cli/commands/registry.py` maps each command name (and alias) to its module, and `AliasGroup` imports the module on first use. When you add, rename or alias a top-level command, update the registry; `cli/tests/test_lazy_commands.py` fails on drift. - `cli/command.py` provides `AliasGroup` (DYM + alias support) and JSON-aware Click exception formatting — Click errors are serialized to JSON when `-F json|pretty_json` is in effect (checked from both Click context and `sys.argv`). - `cli/decorators.py` is the seam between CLI and API: `@common_cli_config_options`, `@common_cli_output_options`, `@common_api_auth_options`, and `@initialise_api` (which calls `core.api.init.initialise_api` to configure the `cloudsmith_api` SDK with key/host/proxy/SSL/retry/SAML token). `@initialise_mcp` wires up the MCP server. @@ -67,4 +67,4 @@ Tests live alongside code: `cloudsmith_cli/cli/tests/` and `cloudsmith_cli/core/ - `flake8` ignores `E203,E501,D107,D102,W503` and uses `max-line-length=100` (but `black` enforces 88). `isort` profile lists known third-party packages explicitly — when adding a new third-party import, add it to `.isort.cfg`'s `known_third_party` if isort puts it in the wrong group. - `pyupgrade --py310-plus` runs on every commit, so use modern syntax (`X | Y` unions, `dict[str, ...]` generics, etc.). - Adding a new subcommand: create a module under `cli/commands/`, register it in `cli/commands/__init__.py`, and attach it to `main` (or a subgroup) with the `AliasGroup` for alias support. -- Adding a new API call: put SDK wrapping in `core/api/.py`, translate exceptions to `ApiException`, and call it from the command layer. Avoid making fresh `cloudsmith_api` request calls from `cli/commands/*`; importing `cloudsmith_api` for `Configuration` defaults or exception types is OK and matches what `login`/`logout`/`check` already do. +- Adding a new API call: put SDK wrapping in `core/api/.py`, translate exceptions to `ApiException`, and call it from the command layer. Avoid making fresh `cloudsmith_api` request calls from `cli/commands/*`; importing `cloudsmith_api` for `Configuration` defaults or exception types is OK and matches what `logout`/`check` already do. diff --git a/CHANGELOG.md b/CHANGELOG.md index a61e03ee..c3b94625 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] +### Removed + +- Removed the `cloudsmith login` command and its `cloudsmith token` alias. The Cloudsmith API no longer supports username and password login, so these commands fail in all earlier CLI versions. Both commands now print a notice and exit with status 1. To authenticate, set the `CLOUDSMITH_API_KEY` environment variable, or run `cloudsmith auth` to authenticate with SAML SSO. + ### Fixed - The CLI no longer prompts for a keyring password when no user can answer. A run is non-interactive when `CI` is set or when no controlling terminal is available. In such a run, a keyring call that needs a password that was not supplied fails without a prompt. OIDC tokens then use the on-disk cache, and `cloudsmith auth` prints why it did not store the SSO tokens. Previously, headless Linux agents printed `GetPassWarning: Can not control echo on the terminal` after each OIDC exchange. To keep a keyring in CI, set `CLOUDSMITH_KEYRING_BACKEND` to a backend that does not prompt, or to an encrypted backend together with `CLOUDSMITH_KEYRING_KEY`. diff --git a/README.md b/README.md index f0784306..45b25197 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,6 @@ The CLI currently supports the following commands (and sub-commands): - `entitlements`|`ents`: List entitlements for a repository. - `packages`: List packages for a repository. (Aliases `repos list`) - `repos`: List repositories for a namespace (owner). -- `login`|`token`: Retrieve your API authentication token/key via login. - `logout`: Clear stored authentication credentials and SSO tokens (Keyring, API key from credential file and emit warning when `$CLOUDSMITH_API_KEY` is still set). - `metadata`: Manage arbitrary JSON metadata (SBOM, BuildInfo, custom) attached to a package. - `add`: Attach a new metadata entry to a package. @@ -336,29 +335,15 @@ cloudsmith auth --owner example --no-browser #### Getting Your API Key -You can retrieve your API key using the `cloudsmith login` command: +Get your API key from your user settings in the Cloudsmith web app. If you authenticated with `cloudsmith auth`, you can also use `cloudsmith tokens create` to create an API key. -``` -cloudsmith login -Login: you@example.com -Password: -Repeat for confirmation: -``` - -*Note:* Please ensure you use your email for the 'Login' prompt and not your user slug/identifier. - -The resulting output looks something like: - -``` -Retrieving API token for 'you@example.com' ... OK -Your API token is: 1234567890abcdef1234567890abcdef -``` - -Once you have your API key you can then put this into your `credentials.ini`, use it as an environment variable `export CLOUDSMITH_API_KEY=your_key_here` or pass it to the CLI using the `-k your_key_here` flag. +Give your API key to the CLI in one of these ways: -For convenience the CLI will ask you if you want to install the default configuration files, complete with your API key, if they don't already exist. Say 'y' or 'yes' to create the configuration files. +- Set the `CLOUDSMITH_API_KEY` environment variable: `export CLOUDSMITH_API_KEY=your_key_here`. +- Put the key into your `credentials.ini` file. +- Pass the key to the CLI with the `-k your_key_here` flag. -If the configuration files already exist, you'll have to manually put the API key into the configuration files, but the CLI will print out their locations. +*Note:* The `cloudsmith login` and `cloudsmith token` commands are no longer available. Username and password login is not supported. ## Uploading Packages diff --git a/cloudsmith_cli/cli/commands/login.py b/cloudsmith_cli/cli/commands/login.py index a2f73399..ce464e5e 100644 --- a/cloudsmith_cli/cli/commands/login.py +++ b/cloudsmith_cli/cli/commands/login.py @@ -1,102 +1,26 @@ -"""CLI/Commands - Get an API token.""" +"""CLI/Commands - Notice for the removed login and token commands.""" import click -import cloudsmith_api -from ...core.api.exceptions import TwoFactorRequiredException -from ...core.api.user import get_user_token -from ...core.config import create_config_files, new_config_messaging -from .. import decorators, utils -from ..exceptions import handle_api_exceptions -from ..utils import maybe_spinner from .main import main - -def validate_login(ctx, param, value): - """Ensure that login is not blank.""" - # pylint: disable=unused-argument - value = value.strip() - if not value: - raise click.BadParameter("The value cannot be blank.", param=param) - return value +REMOVAL_NOTICE = ( + "The 'cloudsmith login' and 'cloudsmith token' commands are no longer " + "available. Username and password login is not supported.\n" + "To authenticate, do one of these:\n" + " - Set the CLOUDSMITH_API_KEY environment variable to your API key.\n" + " - Run 'cloudsmith auth' to authenticate with SAML SSO." +) -@main.command(aliases=["token"]) -@click.option( - "-l", - "--login", - required=True, - callback=validate_login, - prompt=True, - help="Your Cloudsmith login account (email address).", +@main.command( + aliases=["token"], + hidden=True, + add_help_option=False, + context_settings={"ignore_unknown_options": True, "allow_extra_args": True}, ) -@click.password_option("-p", "--password", help="Your Cloudsmith login password.") -@decorators.common_cli_config_options -@decorators.common_cli_output_options -@decorators.initialise_api @click.pass_context -def login(ctx, opts, login, password): # pylint: disable=redefined-outer-name - """Retrieve your API authentication token/key via login.""" - use_stderr = utils.should_use_stderr(opts) - click.echo( - f"Retrieving API token for {click.style(login, bold=True)} ... ", - nl=False, - err=use_stderr, - ) - - context_msg = "Failed to retrieve the API token!" - try: - with ( - handle_api_exceptions(ctx, opts=opts, context_msg=context_msg), - maybe_spinner(opts), - ): - api_key = get_user_token(login=login, password=password) - except TwoFactorRequiredException as e: - click.echo("\r\033[K", nl=False, err=use_stderr) - click.echo("Two-factor authentication is required.", err=use_stderr) - - totp_token = click.prompt( - "Enter your two-factor authentication code", type=str, err=use_stderr - ) - click.echo( - f"Verifying two-factor code for {click.style(login, bold=True)} ... ", - nl=False, - err=use_stderr, - ) - - try: - with ( - handle_api_exceptions(ctx, opts=opts, context_msg=context_msg), - maybe_spinner(opts), - ): - api_key = get_user_token( - login=login, - password=password, - totp_token=totp_token, - two_factor_token=e.two_factor_token, - ) - except cloudsmith_api.rest.ApiException: - click.echo("\r\033[K", nl=False, err=use_stderr) - click.secho( - "Authentication failed: The entered TOTP token is not valid.", - fg="red", - err=use_stderr, - ) - ctx.exit(1) - - except cloudsmith_api.rest.ApiException as e: - click.echo("\r\033[K", nl=False, err=use_stderr) - click.secho(f"Authentication failed: {e!s}", fg="red", err=use_stderr) - ctx.exit(1) - - click.secho("OK", fg="green", err=use_stderr) - - if not utils.maybe_print_as_json(opts, {"token": api_key, "login": login}): - click.echo( - "Your API key/token is: {token}".format( - token=click.style(api_key, fg="magenta") - ) - ) - - create, has_errors = create_config_files(ctx, opts, api_key=api_key) - new_config_messaging(has_errors, opts, create, api_key=api_key) +def login(ctx): + """Show a notice that the login and token commands are removed.""" + click.echo(REMOVAL_NOTICE, err=True) + ctx.exit(1) diff --git a/cloudsmith_cli/cli/decorators.py b/cloudsmith_cli/cli/decorators.py index 98a2a66f..b38eba32 100644 --- a/cloudsmith_cli/cli/decorators.py +++ b/cloudsmith_cli/cli/decorators.py @@ -533,7 +533,7 @@ def wrapper(ctx, *args, **kwargs): opts.oidc_detector_order, oidc_disabled_detectors ) - is_auth_command = ctx.command.name in ("authenticate", "login") + is_auth_command = ctx.command.name == "authenticate" context = CredentialContext( session=opts.session, api_key_from_flag=opts.api_key_from_flag, diff --git a/cloudsmith_cli/cli/exceptions.py b/cloudsmith_cli/cli/exceptions.py index de7f01f4..f98e2171 100644 --- a/cloudsmith_cli/cli/exceptions.py +++ b/cloudsmith_cli/cli/exceptions.py @@ -210,18 +210,11 @@ def get_401_error_hint(ctx, opts, exc): "try again." ) - if ctx.info_name == "token": - # This is already the token command - return ( - "The login failed - Either your email address and/or " - "your password was incorrect. Please check them and " - "try again!" - ) - return ( "You don't have an API key or access token set, but it seems this action " - "requires authentication - Try getting your API key via " - "'cloudsmith token', or access token via 'cloudsmith auth', then try again." + "requires authentication - Try setting your API key via " + "$CLOUDSMITH_API_KEY, or getting an access token via 'cloudsmith auth', " + "then try again." ) diff --git a/cloudsmith_cli/cli/tests/commands/test_login.py b/cloudsmith_cli/cli/tests/commands/test_login.py index 724fa002..7672ae53 100644 --- a/cloudsmith_cli/cli/tests/commands/test_login.py +++ b/cloudsmith_cli/cli/tests/commands/test_login.py @@ -1,31 +1,35 @@ import pytest -from ...commands.login import login +from ...commands.main import main -@pytest.mark.usefixtures("set_api_host_env_var") -@pytest.mark.integration -class TestLoginCommand: - def test_login_via_prompt(self, runner, username, password, api_key): - """Test that a user can `cloudsmith login` with interactive prompts.""" - expected = f"Your API key/token is: {api_key}" - user_input = [ - username, # Login: - password, # Password: - password, # Repeat for confirmation: - "N", # No default config file(s) found, do you want to create them? [y/N]: - ] - result = runner.invoke(login, input="\n".join(user_input)) - assert not result.exception - assert result.exit_code == 0 - assert expected in result.stdout +@pytest.mark.parametrize( + "args", + [ + ["login"], + ["token"], + ["login", "-l", "you@example.com", "-p", "secret"], + ["token", "-l", "you@example.com", "-p", "secret"], + ["login", "--help"], + ["token", "-h"], + ], +) +def test_login_and_token_print_removal_notice(runner, args): + result = runner.invoke(main, args) - def test_login_via_args(self, runner, username, password, api_key): - """Test that a user can `cloudsmith login -l -p `.""" - expected = f"Your API key/token is: {api_key}" - # The "input" argument here answers the following prompt: - # No default config file(s) found, do you want to create them? [y/N]: - result = runner.invoke(login, ["-l", username, "-p", password], input="N\n") - assert not result.exception - assert result.exit_code == 0 - assert expected in result.stdout + assert result.return_value == 1 + assert "'cloudsmith login'" in result.stderr + assert "'cloudsmith token'" in result.stderr + assert "no longer available" in result.stderr + assert "CLOUDSMITH_API_KEY" in result.stderr + assert "cloudsmith auth" in result.stderr + assert result.stdout == "" + + +def test_help_lists_neither_login_nor_token(runner): + result = runner.invoke(main, ["--help"]) + command_names = [line.split()[0] for line in result.stdout.splitlines() if line] + + assert result.exit_code == 0 + assert "tokens" in command_names + assert not {"login", "token", "login|token"} & set(command_names) diff --git a/cloudsmith_cli/cli/tests/conftest.py b/cloudsmith_cli/cli/tests/conftest.py index 8e139130..1a9f6487 100644 --- a/cloudsmith_cli/cli/tests/conftest.py +++ b/cloudsmith_cli/cli/tests/conftest.py @@ -35,18 +35,6 @@ def runner(): return click.testing.CliRunner() -@pytest.fixture() -def username(): - """Return the PYTEST_CLOUDSMITH_USERNAME value.""" - return _get_env_var_or_skip("PYTEST_CLOUDSMITH_USERNAME") - - -@pytest.fixture() -def password(): - """Return the PYTEST_CLOUDSMITH_PASSWORD value.""" - return _get_env_var_or_skip("PYTEST_CLOUDSMITH_PASSWORD") - - @pytest.fixture() def api_key(): """Return the PYTEST_CLOUDSMITH_API_KEY value.""" diff --git a/cloudsmith_cli/cli/tests/test_decorators.py b/cloudsmith_cli/cli/tests/test_decorators.py index 14950bce..22da503c 100644 --- a/cloudsmith_cli/cli/tests/test_decorators.py +++ b/cloudsmith_cli/cli/tests/test_decorators.py @@ -2,7 +2,6 @@ import click import click.testing -import pytest from ..decorators import report_retry, resolve_credentials @@ -46,8 +45,7 @@ def reject(context): assert result.stdout == "command ran\n" -@pytest.mark.parametrize("command_name", ["authenticate", "login"]) -def test_auth_commands_skip_automatic_keyring_refresh(command_name): +def test_auth_command_skips_automatic_keyring_refresh(): def resolve(context): assert context.skip_keyring_refresh is True return None @@ -57,7 +55,7 @@ def resolve(context): side_effect=resolve, ): result = click.testing.CliRunner().invoke( - _credential_command(name=command_name) + _credential_command(name="authenticate") ) assert result.exit_code == 0 diff --git a/cloudsmith_cli/cli/tests/test_exceptions.py b/cloudsmith_cli/cli/tests/test_exceptions.py index 67f143d9..9a999db8 100644 --- a/cloudsmith_cli/cli/tests/test_exceptions.py +++ b/cloudsmith_cli/cli/tests/test_exceptions.py @@ -14,15 +14,13 @@ ) -def hint_for(credential, info_name="push"): +def hint_for(credential): """Return the 401 hint for a session holding the given credential. opts.api_key is left as an auto-Mock, so a hint that reads it rather than the resolved credential fails these tests. """ - return get_401_error_hint( - Mock(info_name=info_name), Mock(credential=credential), Mock() - ) + return get_401_error_hint(Mock(), Mock(credential=credential), Mock()) class TestGet401ErrorHint: @@ -45,10 +43,11 @@ def test_api_key_credential_does_not_assert_a_permissions_problem(self): assert hint_for(credential) == API_KEY_HINT def test_no_credential_suggests_authenticating(self): - assert "cloudsmith token" in hint_for(None) + hint = hint_for(None) - def test_no_credential_on_token_command_reports_a_failed_login(self): - assert "login failed" in hint_for(None, info_name="token") + assert "CLOUDSMITH_API_KEY" in hint + assert "cloudsmith auth" in hint + assert "cloudsmith token" not in hint def invoke_credentialed_401(runner, config_dir, output_format="pretty"): diff --git a/cloudsmith_cli/core/api/exceptions.py b/cloudsmith_cli/core/api/exceptions.py index 1d02d037..d0393678 100644 --- a/cloudsmith_cli/core/api/exceptions.py +++ b/cloudsmith_cli/core/api/exceptions.py @@ -60,9 +60,3 @@ def catch_raise_api_exception(): raise ApiException( exc.status, detail=detail, headers=exc.headers, body=exc.body, fields=fields ) - - -class TwoFactorRequiredException(Exception): - def __init__(self, two_factor_token): - self.two_factor_token = two_factor_token - super().__init__("Two-factor authentication is required") diff --git a/cloudsmith_cli/core/api/init.py b/cloudsmith_cli/core/api/init.py index 8a373635..7346474e 100644 --- a/cloudsmith_cli/core/api/init.py +++ b/cloudsmith_cli/core/api/init.py @@ -97,16 +97,3 @@ def get_api_client(cls: type[T]) -> T: client.api_client.set_default_header(k, v) return client - - -def unset_api_key(): - """Unset the API key.""" - config = cloudsmith_api.Configuration() - - try: - del config.api_key["X-Api-Key"] - except KeyError: - pass - - if hasattr(cloudsmith_api.Configuration, "set_default"): - cloudsmith_api.Configuration.set_default(config) diff --git a/cloudsmith_cli/core/api/user.py b/cloudsmith_cli/core/api/user.py index f6d36330..e4c2bc4c 100644 --- a/cloudsmith_cli/core/api/user.py +++ b/cloudsmith_cli/core/api/user.py @@ -1,13 +1,10 @@ """API - User endpoints.""" -import json - import cloudsmith_api -from cloudsmith_api.rest import ApiException from .. import ratelimits -from .exceptions import TwoFactorRequiredException, catch_raise_api_exception -from .init import get_api_client, unset_api_key +from .exceptions import catch_raise_api_exception +from .init import get_api_client def get_user_api(): @@ -15,46 +12,6 @@ def get_user_api(): return get_api_client(cloudsmith_api.UserApi) -def get_user_token(login, password, totp_token=None, two_factor_token=None): - """Retrieve user token from the API (via authentication).""" - client = get_user_api() - - # Never use API key for the token endpoint - unset_api_key() - - # Prepare data based on whether this is initial login or 2FA completion - if totp_token and two_factor_token: - data_dict = { - "totp_token": totp_token, - "two_factor_token": two_factor_token, - } - else: - data_dict = { - "email": login, - "password": password, - } - - try: - data, _, headers = client.user_token_create_with_http_info(data=data_dict) - ratelimits.maybe_rate_limit(client, headers) - return data.token - except ApiException as e: - # Check for 2FA requirement - if e.status == 422 and e.body: - try: - response_data = json.loads(e.body) - if response_data.get("two_factor_required"): - two_factor_token = response_data.get("two_factor_token") - # Raise custom exception for 2FA requirement - raise TwoFactorRequiredException(two_factor_token) - except (ValueError, KeyError): - pass - - # If not 2FA, use the context manager to handle other API exceptions - with catch_raise_api_exception(): - raise - - def create_user_token_saml() -> dict: """Create a new user API token using SAML.""" client = get_user_api()