diff --git a/README.md b/README.md index 1b286f7..e3f65de 100644 --- a/README.md +++ b/README.md @@ -110,8 +110,11 @@ short process lock; a delayed renewal cannot overwrite a newer sign-in. A lost renewal response can be retried with the saved recovery identifier. `dataplicity auth logout` clears saved credentials before contacting the server. -For a Core user login it also requests account-wide sign-out, including other -signed-in user clients. It never renews credentials just to sign out. A delayed +For a Core user login it revokes only that sign-in and its rotated credentials; +other apps/devices remain signed in. Use `dataplicity auth logout --everywhere` +to explicitly revoke all sign-ins on the account. That action exits unsuccessfully +if server revocation is unavailable or cannot be confirmed, even though local +credentials have been cleared. Neither action renews credentials to sign out. A delayed reply cannot overwrite a replacement login. The CLI reports if server sign-out could not be confirmed. Organisation API keys are only removed locally. Server revocation requires the matching backend session-authority release; diff --git a/dataplicity_cli/account_logout.py b/dataplicity_cli/account_logout.py index 8a169d5..835ec12 100644 --- a/dataplicity_cli/account_logout.py +++ b/dataplicity_cli/account_logout.py @@ -1,4 +1,4 @@ -"""Dedicated Core account revocation, independent of the detached local owner.""" +"""Explicit Core sign-in revocation, independent of the detached local owner.""" import time import requests @@ -8,7 +8,7 @@ from .config import LogoutProof -def revoke_account(proof: LogoutProof) -> str: +def revoke_account(proof: LogoutProof, *, everywhere: bool = False) -> str: """Never renew, follow redirects, borrow cookies, or write credentials. The paired backend accepts a consumed refresh only as revocation proof. @@ -17,7 +17,8 @@ def revoke_account(proof: LogoutProof) -> str: if not proof.access and not proof.refresh: return 'unavailable' try: - url = recipient_url(proof.base_url, '/api/auth/logout/') + path = '/api/auth/logout/everywhere/' if everywhere else '/api/auth/logout/' + url = recipient_url(proof.base_url, path) except ValueError: return 'unavailable' deadline = time.monotonic() + 3.0 diff --git a/dataplicity_cli/cli.py b/dataplicity_cli/cli.py index c8f6bd3..6c0a06a 100644 --- a/dataplicity_cli/cli.py +++ b/dataplicity_cli/cli.py @@ -2387,25 +2387,42 @@ def auth_api_key_set(ctx: typer.Context) -> None: @auth_app.command("logout") -def auth_logout(ctx: typer.Context) -> None: - """Clear saved credentials and revoke the captured Core account session. +def auth_logout( + ctx: typer.Context, + everywhere: bool = typer.Option( + False, "--everywhere", help="Revoke every Core user sign-in on this account." + ), +) -> None: + """Clear saved credentials and revoke this Core user sign-in. - Core sign-out is account-wide, including other signed-in user clients. + Other apps/devices remain signed in unless --everywhere is supplied. Organisation API keys are removed locally without revoking the key. Examples: dataplicity auth logout + dataplicity auth logout --everywhere """ state = _ctx(ctx) from .account_logout import revoke_account proof = state.config.detach_for_logout(state.config.capture_owner()) - result = revoke_account(proof) + result = revoke_account(proof, everywhere=everywhere) if state.json_output: - _print_json({"ok": True, "detail": "Logged out", "server_session_revocation": result}) + detail = 'Logged out' if not everywhere else ( + 'Logged out everywhere' if result == 'revoked' else 'Logged out locally' + ) + _print_json({"ok": not everywhere or result == 'revoked', "detail": detail, + "scope": "account" if everywhere else "sign_in", "server_session_revocation": result}) elif result == 'failed': - state.console.print('Saved credentials cleared. Account sign-out could not be confirmed.') + scope = 'Account-wide sign-out' if everywhere else 'Server sign-out' + state.console.print(f'Saved credentials cleared. {scope} could not be confirmed.') + elif everywhere and result == 'unavailable': + state.console.print('Saved credentials cleared. Sign out everywhere requires a Core user sign-in.') + elif everywhere: + state.console.print("[green]Logged out everywhere.[/green]") else: state.console.print("[green]Logged out.[/green]") + if everywhere and result != 'revoked': + raise typer.Exit(1) @auth_app.command("status") diff --git a/tests/test_account_logout.py b/tests/test_account_logout.py index 5aa60ad..468a15c 100644 --- a/tests/test_account_logout.py +++ b/tests/test_account_logout.py @@ -1,5 +1,6 @@ """Sign-out detaches before I/O and can never publish into a replacement owner.""" import json +import re from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer import threading from unittest.mock import Mock, patch @@ -60,14 +61,16 @@ def test_stale_logout_cannot_detach_a_replacement_primary_even_with_identical_cr assert Config.load(path).capture_owner() == replacement.capture_owner() -def test_cli_clears_before_server_io_and_late_success_preserves_a_new_primary(tmp_path): +@pytest.mark.parametrize('everywhere', [False, True]) +def test_cli_clears_before_server_io_and_late_success_preserves_a_new_primary(tmp_path, everywhere): path = tmp_path/'cli.json' signed_in(path) def send(session, url, **kwargs): detached = Config.load(path) assert detached.auth_method is None assert detached.access_token is None and detached.refresh_token is None - assert url == 'https://gateway.example.test/api/auth/logout/' + suffix = 'everywhere/' if everywhere else '' + assert url == 'https://gateway.example.test/api/auth/logout/' + suffix assert kwargs['headers'] == {'Authorization': 'Bearer owned-access'} assert kwargs['json'] == {} assert not kwargs['allow_redirects'] @@ -75,14 +78,17 @@ def send(session, url, **kwargs): detached.save(path) return response(200) with patch.object(requests.Session, 'post', send): - result = CliRunner().invoke(app, ['--config', str(path), '--json', 'auth', 'logout']) + result = CliRunner().invoke(app, ['--config', str(path), '--json', 'auth', 'logout', + *(['--everywhere'] if everywhere else [])]) assert result.exit_code == 0, result.output assert json.loads(result.output)['server_session_revocation'] == 'revoked' + assert json.loads(result.output)['scope'] == ('account' if everywhere else 'sign_in') assert Config.load(path).access_token == 'replacement-access' assert 'owned-access' not in result.output and 'owned-refresh' not in result.output -def test_explicit_access_rejection_uses_only_the_captured_refresh_and_no_ambient_auth(): +@pytest.mark.parametrize('everywhere', [False, True]) +def test_explicit_access_rejection_uses_only_the_captured_refresh_and_no_ambient_auth(everywhere): session = requests.Session() session.cookies.set('unrelated', 'private-cookie') session.auth = ('ambient', 'password') @@ -91,13 +97,15 @@ def test_explicit_access_rejection_uses_only_the_captured_refresh_and_no_ambient session.send = Mock(side_effect=responses) with patch('dataplicity_cli.account_logout.requests.Session', return_value=session), \ patch('requests.sessions.get_netrc_auth', return_value=('netrc', 'password')): - assert revoke_account(LogoutProof('https://gateway.example.test', 'owned-access', 'owned-refresh')) == 'revoked' + assert revoke_account(LogoutProof('https://gateway.example.test', 'owned-access', 'owned-refresh'), everywhere=everywhere) == 'revoked' first, second = [call.args[0] for call in session.send.call_args_list] assert first.headers['Authorization'] == 'Bearer owned-access' assert 'Authorization' not in second.headers assert json.loads(first.body) == {} assert json.loads(second.body) == {'refresh': 'owned-refresh'} assert 'Cookie' not in first.headers and 'Cookie' not in second.headers + suffix = 'everywhere/' if everywhere else '' + assert first.url == second.url == 'https://gateway.example.test/api/auth/logout/' + suffix assert session.trust_env for call in session.send.call_args_list: assert call.kwargs['allow_redirects'] is False @@ -108,9 +116,10 @@ def test_explicit_access_rejection_uses_only_the_captured_refresh_and_no_ambient @pytest.mark.parametrize('status', [302, 307, 400, 429, 500, 503]) -def test_ambiguous_or_non_auth_failure_never_starts_another_mutation(status): +@pytest.mark.parametrize('everywhere', [False, True]) +def test_ambiguous_or_non_auth_failure_never_starts_another_mutation(status, everywhere): with patch.object(requests.Session, 'post', return_value=response(status)) as send: - assert revoke_account(LogoutProof('https://gateway.example.test', 'access', 'refresh')) == 'failed' + assert revoke_account(LogoutProof('https://gateway.example.test', 'access', 'refresh'), everywhere=everywhere) == 'failed' assert send.call_count == 1 @@ -162,7 +171,8 @@ def test_refresh_only_and_exhausted_budget_are_bounded_without_renewal(): send.assert_not_called() -def test_actual_http_fallback_never_adopts_server_cookies_or_a_new_saved_account(tmp_path): +@pytest.mark.parametrize('everywhere', [False, True]) +def test_actual_http_fallback_never_adopts_server_cookies_or_a_new_saved_account(tmp_path, everywhere): path = tmp_path/'cli.json' calls = [] @@ -191,15 +201,57 @@ def log_message(self, *_args): access_token='owned-access', refresh_token='owned-refresh') config.save(path) with patch('requests.sessions.get_netrc_auth', return_value=('ambient', 'password')): - result = CliRunner().invoke(app, ['--config', str(path), '--json', 'auth', 'logout']) + result = CliRunner().invoke(app, ['--config', str(path), '--json', 'auth', 'logout', + *(['--everywhere'] if everywhere else [])]) assert result.exit_code == 0, result.output assert json.loads(result.output)['server_session_revocation'] == 'revoked' + endpoint = '/api/auth/logout/everywhere/' if everywhere else '/api/auth/logout/' assert calls == [ - ('/api/auth/logout/', 'Bearer owned-access', None, {}), - ('/api/auth/logout/', None, None, {'refresh': 'owned-refresh'}), + (endpoint, 'Bearer owned-access', None, {}), + (endpoint, None, None, {'refresh': 'owned-refresh'}), ] assert Config.load(path).access_token == 'replacement-access' finally: server.shutdown() server.server_close() worker.join(timeout=5) + + +@pytest.mark.parametrize('json_output', [False, True]) +@pytest.mark.parametrize('user_login', [False, True]) +def test_everywhere_failure_is_explicit_after_local_detachment(tmp_path, json_output, user_login): + path = tmp_path/'cli.json' + if user_login: + signed_in(path) + else: + Config(auth_method='api_key', api_key='organisation-secret').save(path) + with patch.object(requests.Session, 'post', side_effect=requests.Timeout('private-proof')) as send: + result = CliRunner().invoke(app, ['--config', str(path), + *(['--json'] if json_output else []), 'auth', 'logout', '--everywhere']) + assert result.exit_code == 1, result.output + assert Config.load(path).auth_method is None + if json_output: + output = json.loads(result.output) + assert output['ok'] is False and output['scope'] == 'account' + assert output['server_session_revocation'] == ('failed' if user_login else 'unavailable') + else: + assert ('could not be confirmed' if user_login else 'requires a Core user sign-in') in result.output + assert 'private-proof' not in result.output and 'organisation-secret' not in result.output + assert send.call_count == (1 if user_login else 0) + + +def test_successful_everywhere_command_names_its_scope(tmp_path): + path = tmp_path/'cli.json' + signed_in(path) + with patch.object(requests.Session, 'post', return_value=response(200)): + result = CliRunner().invoke(app, ['--config', str(path), 'auth', 'logout', '--everywhere']) + assert result.exit_code == 0 and 'Logged out everywhere.' in result.output + + +def test_logout_help_describes_explicit_global_scope(): + result = CliRunner().invoke(app, ['auth', 'logout', '--help']) + assert result.exit_code == 0 + # Rich splits option text with ANSI styles on CI's coloured terminal. + plain = re.sub(r'\x1b\[[0-9;]*m', '', result.output).replace('│', ' ') + assert '--everywhere' in plain + assert 'Other apps/devices remain signed in unless' in ' '.join(plain.split())