diff --git a/README.md b/README.md index d0f1266..1b286f7 100644 --- a/README.md +++ b/README.md @@ -109,6 +109,14 @@ asks you to start again. Configuration updates use a private atomic file and a 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 +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; +an already-consumed refresh is accepted only as revocation proof. + Login with email/password: ``` diff --git a/dataplicity_cli/account_logout.py b/dataplicity_cli/account_logout.py new file mode 100644 index 0000000..8a169d5 --- /dev/null +++ b/dataplicity_cli/account_logout.py @@ -0,0 +1,46 @@ +"""Dedicated Core account revocation, independent of the detached local owner.""" +import time + +import requests + +from .api import _EXPLICIT_API_AUTH +from .api_recipient import recipient_url +from .config import LogoutProof + + +def revoke_account(proof: LogoutProof) -> str: + """Never renew, follow redirects, borrow cookies, or write credentials. + + The paired backend accepts a consumed refresh only as revocation proof. + A network/server failure is ambiguous and must not start another mutation. + """ + if not proof.access and not proof.refresh: + return 'unavailable' + try: + url = recipient_url(proof.base_url, '/api/auth/logout/') + except ValueError: + return 'unavailable' + deadline = time.monotonic() + 3.0 + session = requests.Session() + try: + for access in ((proof.access, None) if proof.access and proof.refresh else (proof.access,)): + remaining = deadline - time.monotonic() + if remaining <= 0: + break + session.cookies.clear() + response = session.post(url, + headers={'Authorization': 'Bearer ' + access} if access else {}, + json={} if access else {'refresh': proof.refresh}, + auth=_EXPLICIT_API_AUTH, allow_redirects=False, timeout=remaining, stream=True) + try: + if 200 <= response.status_code < 300: + return 'revoked' + if response.status_code not in (401, 403): + break + finally: + response.close() + except requests.RequestException: + pass + finally: + session.close() + return 'failed' diff --git a/dataplicity_cli/cli.py b/dataplicity_cli/cli.py index 0223959..c8f6bd3 100644 --- a/dataplicity_cli/cli.py +++ b/dataplicity_cli/cli.py @@ -2388,17 +2388,22 @@ def auth_api_key_set(ctx: typer.Context) -> None: @auth_app.command("logout") def auth_logout(ctx: typer.Context) -> None: - """Clear saved authentication credentials. + """Clear saved credentials and revoke the captured Core account session. + + Core sign-out is account-wide, including other signed-in user clients. + Organisation API keys are removed locally without revoking the key. Examples: dataplicity auth logout """ state = _ctx(ctx) - state.config.clear_tokens() - state.config.clear_api_key() - state.config.save(state.config_path) + from .account_logout import revoke_account + proof = state.config.detach_for_logout(state.config.capture_owner()) + result = revoke_account(proof) if state.json_output: - _print_json({"ok": True, "detail": "Logged out"}) + _print_json({"ok": True, "detail": "Logged out", "server_session_revocation": result}) + elif result == 'failed': + state.console.print('Saved credentials cleared. Account sign-out could not be confirmed.') else: state.console.print("[green]Logged out.[/green]") diff --git a/dataplicity_cli/config.py b/dataplicity_cli/config.py index 9d2511b..2540d90 100644 --- a/dataplicity_cli/config.py +++ b/dataplicity_cli/config.py @@ -59,6 +59,13 @@ class RefreshTicket: request_id: str +@dataclass(frozen=True) +class LogoutProof: + base_url: str = field(repr=False) + access: Optional[str] = field(default=None, repr=False) + refresh: Optional[str] = field(default=None, repr=False) + + _AUTH_FIELDS = {"base_url", "auth_method", "access_token", "refresh_token", "api_key"} _SECRET_FIELDS = {"auth_method", "access_token", "refresh_token", "api_key"} @@ -285,6 +292,22 @@ def invalidate_owned(self, owner, expected_refresh=None): self._persist_owned(record) return self.capture_owner() + def detach_for_logout(self, owner): + """Capture the latest owned proof and clear it in one file transaction. + + Network work must happen after this returns, without a config reference. + A waiting rotation or primary login cannot publish into the old owner. + """ + with self._owned_record(owner) as record: + jwt = record.auth_method == 'jwt' + proof = LogoutProof(record.base_url, + record.access_token if jwt and valid_api_credential(record.access_token) else None, + record.refresh_token if jwt and valid_api_credential(record.refresh_token) else None) + record.clear_tokens() + record.clear_api_key() + self._persist_owned(record) + return proof + def _new_session(self): self.local_session_id = str(uuid.uuid4()) self.refresh_attempt = None diff --git a/tests/test_account_logout.py b/tests/test_account_logout.py new file mode 100644 index 0000000..5aa60ad --- /dev/null +++ b/tests/test_account_logout.py @@ -0,0 +1,205 @@ +"""Sign-out detaches before I/O and can never publish into a replacement owner.""" +import json +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +import threading +from unittest.mock import Mock, patch + +import pytest +import requests +from typer.testing import CliRunner + +from dataplicity_cli.account_logout import revoke_account +from dataplicity_cli.cli import app +from dataplicity_cli.config import Config, LogoutProof +from dataplicity_cli.config_store import ConfigChangedError + + +def response(status): + result = requests.Response() + result.status_code = status + result._content = b'{}' + result.close = Mock() + return result + + +def signed_in(path): + config = Config(base_url='https://gateway.example.test', auth_method='jwt', + access_token='owned-access', refresh_token='owned-refresh') + config.save(path) + return config + + +def test_detachment_captures_latest_owned_rotation_and_invalidates_its_pending_work(tmp_path): + path = tmp_path/'cli.json' + config = signed_in(path) + owner = config.capture_owner() + other = Config.load(path) + ticket = other.claim_refresh(other.capture_owner()) + other.commit_rotation(ticket, 'rotated-access', 'rotated-refresh') + pending = other.claim_refresh(other.capture_owner()) + proof = config.detach_for_logout(owner) + assert (proof.access, proof.refresh) == ('rotated-access', 'rotated-refresh') + saved = Config.load(path) + assert saved.auth_method is None and saved.access_token is None and saved.refresh_token is None + assert saved.refresh_attempt is None and saved.capture_owner() != owner + with pytest.raises(ConfigChangedError): + other.commit_rotation(pending, 'late-access', 'late-refresh') + assert Config.load(path).access_token is None + assert 'rotated' not in repr(proof) + + +def test_stale_logout_cannot_detach_a_replacement_primary_even_with_identical_credentials(tmp_path): + path = tmp_path/'cli.json' + old = signed_in(path) + owner = old.capture_owner() + replacement = Config.load(path) + replacement.install_user_tokens('owned-access', 'owned-refresh') + replacement.save(path) + with pytest.raises(ConfigChangedError): + old.detach_for_logout(owner) + 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): + 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/' + assert kwargs['headers'] == {'Authorization': 'Bearer owned-access'} + assert kwargs['json'] == {} + assert not kwargs['allow_redirects'] + detached.install_user_tokens('replacement-access', 'replacement-refresh') + detached.save(path) + return response(200) + with patch.object(requests.Session, 'post', send): + result = CliRunner().invoke(app, ['--config', str(path), '--json', 'auth', 'logout']) + assert result.exit_code == 0, result.output + assert json.loads(result.output)['server_session_revocation'] == 'revoked' + 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(): + session = requests.Session() + session.cookies.set('unrelated', 'private-cookie') + session.auth = ('ambient', 'password') + # Explicit auth must beat session Basic and netrc, including refresh-only. + responses = [response(401), response(200)] + 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' + 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 + assert session.trust_env + for call in session.send.call_args_list: + assert call.kwargs['allow_redirects'] is False + assert call.kwargs['stream'] is True + assert 0 < call.kwargs['timeout'] <= 3 + for item in responses: + item.close.assert_called_once() + + +@pytest.mark.parametrize('status', [302, 307, 400, 429, 500, 503]) +def test_ambiguous_or_non_auth_failure_never_starts_another_mutation(status): + with patch.object(requests.Session, 'post', return_value=response(status)) as send: + assert revoke_account(LogoutProof('https://gateway.example.test', 'access', 'refresh')) == 'failed' + assert send.call_count == 1 + + +def test_connection_failure_never_renews_or_retries_and_cli_reports_local_completion(tmp_path): + path = tmp_path/'cli.json' + signed_in(path) + with patch.object(requests.Session, 'post', side_effect=requests.Timeout('private-credential')) as send: + result = CliRunner().invoke(app, ['--config', str(path), 'auth', 'logout']) + assert result.exit_code == 0 + assert 'credentials cleared' in result.output + assert 'could not be confirmed' in result.output + assert 'private-credential' not in result.output + assert Config.load(path).access_token is None + assert send.call_count == 1 + + +@pytest.mark.parametrize('method', ['api_key', None]) +def test_non_user_realms_clear_locally_without_sending_core_proof(tmp_path, method): + path = tmp_path/'cli.json' + config = Config(auth_method=method, api_key='organisation-secret', + access_token='inactive-user-access', refresh_token='inactive-user-refresh') + config.save(path) + with patch.object(requests.Session, 'post') as send: + result = CliRunner().invoke(app, ['--config', str(path), '--json', 'auth', 'logout']) + assert result.exit_code == 0 + assert json.loads(result.output)['server_session_revocation'] == 'unavailable' + saved = Config.load(path) + assert saved.api_key is None and saved.access_token is None and saved.auth_method is None + send.assert_not_called() + + +@pytest.mark.parametrize('base', ['https://user:private@gateway.example.test', 'http://foreign.example.test', 'file:///private']) +def test_invalid_recipient_never_receives_the_detached_proof(base): + proof = LogoutProof(base, 'private-access', 'private-refresh') + with patch.object(requests.Session, 'post') as send: + assert revoke_account(proof) == 'unavailable' + send.assert_not_called() + assert 'private' not in repr(proof) + + +def test_refresh_only_and_exhausted_budget_are_bounded_without_renewal(): + with patch.object(requests.Session, 'post', return_value=response(403)) as send: + assert revoke_account(LogoutProof('https://gateway.example.test', refresh='owned-refresh')) == 'failed' + assert send.call_args.kwargs['headers'] == {} + assert send.call_args.kwargs['json'] == {'refresh': 'owned-refresh'} + with patch('dataplicity_cli.account_logout.time.monotonic', side_effect=[0, 4]), \ + patch.object(requests.Session, 'post') as send: + assert revoke_account(LogoutProof('https://gateway.example.test', 'access')) == 'failed' + send.assert_not_called() + + +def test_actual_http_fallback_never_adopts_server_cookies_or_a_new_saved_account(tmp_path): + path = tmp_path/'cli.json' + calls = [] + + class Handler(BaseHTTPRequestHandler): + def do_POST(self): + body = json.loads(self.rfile.read(int(self.headers['Content-Length']))) + calls.append((self.path, self.headers.get('Authorization'), self.headers.get('Cookie'), body)) + if len(calls) == 1: + current = Config.load(path) + assert current.access_token is None and current.refresh_token is None + current.install_user_tokens('replacement-access', 'replacement-refresh') + current.save(path) + self.send_response(401 if len(calls) == 1 else 200) + self.send_header('Set-Cookie', 'ambient=must-not-be-forwarded; Path=/') + self.send_header('Content-Length', '0') + self.end_headers() + + def log_message(self, *_args): + pass + + server = ThreadingHTTPServer(('127.0.0.1', 0), Handler) + worker = threading.Thread(target=server.serve_forever, daemon=True) + worker.start() + try: + config = Config(base_url=f'http://127.0.0.1:{server.server_port}', auth_method='jwt', + 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']) + assert result.exit_code == 0, result.output + assert json.loads(result.output)['server_session_revocation'] == 'revoked' + assert calls == [ + ('/api/auth/logout/', 'Bearer owned-access', None, {}), + ('/api/auth/logout/', None, None, {'refresh': 'owned-refresh'}), + ] + assert Config.load(path).access_token == 'replacement-access' + finally: + server.shutdown() + server.server_close() + worker.join(timeout=5)