diff --git a/changelog.md b/changelog.md index 44275107..71ae6e06 100644 --- a/changelog.md +++ b/changelog.md @@ -10,6 +10,7 @@ Features --------- * Add `--ssh-jump` CLI argument, restoring SSH jump functionality. * Add a `/dsn` command to manage persisted DSN aliases. +* Add `auto` option to ignore keyring when inside an SSH session. Documentation diff --git a/mycli/TIPS b/mycli/TIPS index a0f45761..b70c5339 100644 --- a/mycli/TIPS +++ b/mycli/TIPS @@ -20,7 +20,7 @@ the --character-set option sets the character set for a single session! the --unbuffered flag can save memory when in batch mode! ---use-keyring=true lets you access the system keyring for passwords! +--use-keyring=auto lets you access the system keyring for passwords! --use-keyring=reset resets a password saved to the system keyring! diff --git a/mycli/cli_runner.py b/mycli/cli_runner.py index 585860fc..9251f278 100644 --- a/mycli/cli_runner.py +++ b/mycli/cli_runner.py @@ -304,9 +304,24 @@ def run_from_cli_args(cli_args: 'CliArgs', client_factory: ClientFactory) -> Non if cli_args.use_keyring is not None and cli_args.use_keyring.lower() == 'reset': use_keyring = True reset_keyring = True + elif cli_args.use_keyring is not None and cli_args.use_keyring.lower() == 'auto': + if os.environ.get('SSH_CONNECTION'): + use_keyring = False + reset_keyring = False + else: + use_keyring = True + reset_keyring = False elif cli_args.use_keyring is None: - use_keyring = str_to_bool(mycli.config['main'].get('use_keyring', 'False')) - reset_keyring = False + if mycli.config['main'].get('use_keyring', 'False').lower() == 'auto': + if os.environ.get('SSH_CONNECTION'): + use_keyring = False + reset_keyring = False + else: + use_keyring = True + reset_keyring = False + else: + use_keyring = str_to_bool(mycli.config['main'].get('use_keyring', 'False')) + reset_keyring = False else: use_keyring = str_to_bool(cli_args.use_keyring) reset_keyring = False diff --git a/mycli/main.py b/mycli/main.py index 66ac5a05..0806a77b 100755 --- a/mycli/main.py +++ b/mycli/main.py @@ -263,9 +263,9 @@ class CliArgs: help='Show progress on the standard error with --batch.', ) use_keyring: str | None = clickdc.option( - type=click.Choice(['true', 'false', 'reset']), + type=click.Choice(['auto', 'true', 'false', 'reset']), default=None, - help='Store and retrieve passwords from the system keyring: true/false/reset.', + help='Store and retrieve passwords from the system keyring. auto means true, unless within an SSH connection.', ) keepalive_ticks: int | None = clickdc.option( type=int, diff --git a/mycli/myclirc b/mycli/myclirc index 99ff0244..e41998f5 100644 --- a/mycli/myclirc +++ b/mycli/myclirc @@ -206,10 +206,14 @@ enable_pager = True pager = 'less' # Whether to store and retrieve passwords from the system keyring. +# * False - never use keyring +# * True - always use keyring +# * auto - use keyring unless currently in an SSH connection # See the documentation for https://pypi.org/project/keyring/ for your OS. # Note that the hostname is considered to be different if short or qualified. # This can be overridden with --use-keyring= at the CLI. # A password can be reset with --use-keyring=reset at the CLI. +# Recommanded: auto use_keyring = False [search] diff --git a/test/myclirc b/test/myclirc index 9795ba64..6a556ebc 100644 --- a/test/myclirc +++ b/test/myclirc @@ -206,10 +206,14 @@ enable_pager = True pager = python test/features/wrappager.py ---boundary--- # Whether to store and retrieve passwords from the system keyring. +# * False - never use keyring +# * True - always use keyring +# * auto - use keyring unless currently in an SSH connection # See the documentation for https://pypi.org/project/keyring/ for your OS. # Note that the hostname is considered to be different if short or qualified. # This can be overridden with --use-keyring= at the CLI. # A password can be reset with --use-keyring=reset at the CLI. +# Recommended: auto use_keyring = False [search] diff --git a/test/pytests/test_cli_runner.py b/test/pytests/test_cli_runner.py index 262c80c3..8a02d519 100644 --- a/test/pytests/test_cli_runner.py +++ b/test/pytests/test_cli_runner.py @@ -421,3 +421,56 @@ def test_run_from_cli_args_uses_explicit_keyring_flag(monkeypatch: pytest.Monkey assert client.connect_calls[-1]['use_keyring'] is True assert client.connect_calls[-1]['reset_keyring'] is False + + +@pytest.mark.parametrize( + ('ssh_connection', 'expected_use_keyring'), + ( + (None, True), + ('client-ip client-port server-ip server-port', False), + ), +) +def test_run_from_cli_args_uses_auto_keyring_flag( + monkeypatch: pytest.MonkeyPatch, + ssh_connection: str | None, + expected_use_keyring: bool, +) -> None: + cli_args = make_cli_args() + cli_args.use_keyring = 'auto' + client = DummyMyCli() + if ssh_connection is None: + monkeypatch.delenv('SSH_CONNECTION', raising=False) + else: + monkeypatch.setenv('SSH_CONNECTION', ssh_connection) + + run_with_client(monkeypatch, cli_args, client) + + assert client.connect_calls[-1]['use_keyring'] is expected_use_keyring + assert client.connect_calls[-1]['reset_keyring'] is False + + +@pytest.mark.parametrize( + ('ssh_connection', 'expected_use_keyring'), + ( + (None, True), + ('client-ip client-port server-ip server-port', False), + ), +) +def test_run_from_cli_args_uses_auto_keyring_config( + monkeypatch: pytest.MonkeyPatch, + ssh_connection: str | None, + expected_use_keyring: bool, +) -> None: + cli_args = make_cli_args() + config = default_config() + config['main'] = {**config['main'], 'use_keyring': 'auto'} + client = DummyMyCli(config=config) + if ssh_connection is None: + monkeypatch.delenv('SSH_CONNECTION', raising=False) + else: + monkeypatch.setenv('SSH_CONNECTION', ssh_connection) + + run_with_client(monkeypatch, cli_args, client) + + assert client.connect_calls[-1]['use_keyring'] is expected_use_keyring + assert client.connect_calls[-1]['reset_keyring'] is False