From 5cbbfb41d9bec9dcd2391f258c158b3b077266db Mon Sep 17 00:00:00 2001 From: Dmitrii Iurco Date: Sat, 30 May 2026 00:46:54 -0400 Subject: [PATCH] feat: add HTTP dispatch to AccountManager for generic store services Services with accounts.manager='http' now use POST/DELETE to the service container's /service-api/accounts endpoint instead of requiring a named Python manager. _resolve_service allows 'http' without a registered Python object; _provision_http and _deprovision_http handle the HTTP calls with 404-as-success on delete. 9 new tests. Co-Authored-By: Claude Sonnet 4.6 --- api/account_manager.py | 81 ++++++++++++++++++++++++++----- tests/test_account_manager.py | 90 +++++++++++++++++++++++++++++++++++ 2 files changed, 159 insertions(+), 12 deletions(-) diff --git a/api/account_manager.py b/api/account_manager.py index 6f02fdd..4f2f5cd 100644 --- a/api/account_manager.py +++ b/api/account_manager.py @@ -29,6 +29,11 @@ import threading from pathlib import Path from typing import Dict, List, Optional +try: + import requests as _requests +except ImportError: + _requests = None + logger = logging.getLogger('picell') _DISPATCH_PROVISION = { @@ -42,6 +47,8 @@ _DISPATCH_DEPROVISION = { 'file_manager': '_deprovision_files', } +_HTTP_TIMEOUT = 10 + class AccountManager: @@ -105,10 +112,55 @@ class AccountManager: def _deprovision_files(manager, _svc: Dict, peer_username: str) -> bool: return manager.delete_user(peer_username) + # ── HTTP dispatch (manager == "http") ──────────────────────────────── + + @staticmethod + def _http_base_url(svc: Dict) -> str: + """Return the base URL for the service's /service-api endpoint.""" + backend = svc.get('backend', '') + if not backend: + raise ValueError(f"Service {svc.get('id')!r} has no 'backend' configured") + return f'http://{backend}' + + def _provision_http(self, svc: Dict, peer_username: str, password: str) -> bool: + if _requests is None: + raise RuntimeError('requests library is required for HTTP account dispatch') + url = self._http_base_url(svc) + '/service-api/accounts' + try: + resp = _requests.post( + url, + json={'username': peer_username, 'password': password}, + timeout=_HTTP_TIMEOUT, + ) + if resp.status_code in (200, 201): + return True + logger.warning('HTTP provision %s on %s returned %s: %s', + peer_username, svc.get('id'), resp.status_code, resp.text[:200]) + return False + except Exception as exc: + raise RuntimeError(f'HTTP provision request failed: {exc}') from exc + + def _deprovision_http(self, svc: Dict, peer_username: str) -> bool: + if _requests is None: + raise RuntimeError('requests library is required for HTTP account dispatch') + url = self._http_base_url(svc) + f'/service-api/accounts/{peer_username}' + try: + resp = _requests.delete(url, timeout=_HTTP_TIMEOUT) + if resp.status_code in (200, 204, 404): + return True + logger.warning('HTTP deprovision %s on %s returned %s: %s', + peer_username, svc.get('id'), resp.status_code, resp.text[:200]) + return False + except Exception as exc: + raise RuntimeError(f'HTTP deprovision request failed: {exc}') from exc + # ── Service validation helper ───────────────────────────────────────── def _resolve_service(self, service_id: str): - """Return (svc, manager_name, manager) or raise ValueError.""" + """Return (svc, manager_name, manager) or raise ValueError. + + manager is None when manager_name == 'http' — callers must check. + """ svc = self._registry.get(service_id) if svc is None: raise ValueError(f'Unknown service: {service_id!r}') @@ -116,6 +168,8 @@ class AccountManager: manager_name = accounts_cfg.get('manager') if not manager_name: raise ValueError(f'Service {service_id!r} does not support accounts') + if manager_name == 'http': + return svc, 'http', None manager = self._managers.get(manager_name) if manager is None: raise ValueError(f'Manager {manager_name!r} is not registered with AccountManager') @@ -135,12 +189,14 @@ class AccountManager: if password is None: password = _secrets_mod.token_urlsafe(16) - dispatch = _DISPATCH_PROVISION.get(manager_name) - if dispatch is None: - raise ValueError(f'No provision dispatch for manager: {manager_name!r}') - fn = getattr(self, dispatch) + if manager_name == 'http': + ok = self._provision_http(svc, peer_username, password) + else: + dispatch = _DISPATCH_PROVISION.get(manager_name) + if dispatch is None: + raise ValueError(f'No provision dispatch for manager: {manager_name!r}') + ok = getattr(self, dispatch)(manager, svc, peer_username, password) - ok = fn(manager, svc, peer_username, password) if not ok: raise RuntimeError( f'Provision of {peer_username!r} on {service_id!r} returned False — ' @@ -160,12 +216,13 @@ class AccountManager: """Delete the peer's account on the service and clear stored credentials.""" svc, manager_name, manager = self._resolve_service(service_id) - dispatch = _DISPATCH_DEPROVISION.get(manager_name) - if dispatch is None: - raise ValueError(f'No deprovision dispatch for manager: {manager_name!r}') - fn = getattr(self, dispatch) - - ok = fn(manager, svc, peer_username) + if manager_name == 'http': + ok = self._deprovision_http(svc, peer_username) + else: + dispatch = _DISPATCH_DEPROVISION.get(manager_name) + if dispatch is None: + raise ValueError(f'No deprovision dispatch for manager: {manager_name!r}') + ok = getattr(self, dispatch)(manager, svc, peer_username) with self._lock: all_creds = self._load_creds() diff --git a/tests/test_account_manager.py b/tests/test_account_manager.py index 5a5a06b..c3ddf4f 100644 --- a/tests/test_account_manager.py +++ b/tests/test_account_manager.py @@ -437,5 +437,95 @@ class TestEdgeCases(unittest.TestCase): self.assertIsNone(self.am.get_credentials('email', 'alice')) +# ── HTTP dispatch (manager == "http") ───────────────────────────────────────── + +class TestHttpDispatch(unittest.TestCase): + """AccountManager with manager='http' uses HTTP POST/DELETE to the service backend.""" + + def _make_http_registry(self, backend='cell-myapp:8080'): + reg = MagicMock() + reg.get.return_value = { + 'id': 'myapp', + 'backend': backend, + 'accounts': {'manager': 'http', 'credentials': ['password']}, + } + return reg + + def setUp(self): + import tempfile + self.tmp = Path(tempfile.mkdtemp()) + self.am = _make_am(self.tmp, registry=self._make_http_registry()) + + def test_provision_http_posts_to_service_api(self): + with patch('account_manager._requests') as mock_req: + mock_req.post.return_value = MagicMock(status_code=201) + creds = self.am.provision('myapp', 'alice', password='s3cret') + mock_req.post.assert_called_once_with( + 'http://cell-myapp:8080/service-api/accounts', + json={'username': 'alice', 'password': 's3cret'}, + timeout=10, + ) + self.assertEqual(creds['password'], 's3cret') + + def test_provision_http_stores_credentials_on_success(self): + with patch('account_manager._requests') as mock_req: + mock_req.post.return_value = MagicMock(status_code=200) + self.am.provision('myapp', 'alice', password='pw') + self.assertEqual(self.am.get_credentials('myapp', 'alice'), {'password': 'pw'}) + + def test_provision_http_returns_false_on_non_2xx(self): + with patch('account_manager._requests') as mock_req: + mock_req.post.return_value = MagicMock(status_code=409, text='conflict') + with self.assertRaises(RuntimeError): + self.am.provision('myapp', 'alice', password='pw') + + def test_provision_http_raises_on_request_exception(self): + with patch('account_manager._requests') as mock_req: + mock_req.post.side_effect = Exception('connection refused') + with self.assertRaises(RuntimeError): + self.am.provision('myapp', 'alice', password='pw') + + def test_deprovision_http_deletes_to_service_api(self): + self.am.store_credentials('myapp', 'alice', {'password': 'pw'}) + with patch('account_manager._requests') as mock_req: + mock_req.delete.return_value = MagicMock(status_code=204) + ok = self.am.deprovision('myapp', 'alice') + mock_req.delete.assert_called_once_with( + 'http://cell-myapp:8080/service-api/accounts/alice', + timeout=10, + ) + self.assertTrue(ok) + + def test_deprovision_http_treats_404_as_success(self): + """404 means already deleted — still a clean deprovision.""" + self.am.store_credentials('myapp', 'alice', {'password': 'pw'}) + with patch('account_manager._requests') as mock_req: + mock_req.delete.return_value = MagicMock(status_code=404) + ok = self.am.deprovision('myapp', 'alice') + self.assertTrue(ok) + + def test_deprovision_http_removes_stored_credentials(self): + self.am.store_credentials('myapp', 'alice', {'password': 'pw'}) + with patch('account_manager._requests') as mock_req: + mock_req.delete.return_value = MagicMock(status_code=204) + self.am.deprovision('myapp', 'alice') + self.assertIsNone(self.am.get_credentials('myapp', 'alice')) + + def test_resolve_service_http_does_not_require_python_manager(self): + """manager='http' must not raise even with no named managers passed.""" + am = AccountManager( + service_registry=self._make_http_registry(), + data_dir=str(self.tmp), + ) + svc, manager_name, manager = am._resolve_service('myapp') + self.assertEqual(manager_name, 'http') + self.assertIsNone(manager) + + def test_http_base_url_raises_when_no_backend(self): + svc = {'id': 'nobackend', 'backend': ''} + with self.assertRaises(ValueError): + AccountManager._http_base_url(svc) + + if __name__ == '__main__': unittest.main()