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 <noreply@anthropic.com>
This commit is contained in:
+69
-12
@@ -29,6 +29,11 @@ import threading
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Dict, List, Optional
|
from typing import Dict, List, Optional
|
||||||
|
|
||||||
|
try:
|
||||||
|
import requests as _requests
|
||||||
|
except ImportError:
|
||||||
|
_requests = None
|
||||||
|
|
||||||
logger = logging.getLogger('picell')
|
logger = logging.getLogger('picell')
|
||||||
|
|
||||||
_DISPATCH_PROVISION = {
|
_DISPATCH_PROVISION = {
|
||||||
@@ -42,6 +47,8 @@ _DISPATCH_DEPROVISION = {
|
|||||||
'file_manager': '_deprovision_files',
|
'file_manager': '_deprovision_files',
|
||||||
}
|
}
|
||||||
|
|
||||||
|
_HTTP_TIMEOUT = 10
|
||||||
|
|
||||||
|
|
||||||
class AccountManager:
|
class AccountManager:
|
||||||
|
|
||||||
@@ -105,10 +112,55 @@ class AccountManager:
|
|||||||
def _deprovision_files(manager, _svc: Dict, peer_username: str) -> bool:
|
def _deprovision_files(manager, _svc: Dict, peer_username: str) -> bool:
|
||||||
return manager.delete_user(peer_username)
|
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 ─────────────────────────────────────────
|
# ── Service validation helper ─────────────────────────────────────────
|
||||||
|
|
||||||
def _resolve_service(self, service_id: str):
|
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)
|
svc = self._registry.get(service_id)
|
||||||
if svc is None:
|
if svc is None:
|
||||||
raise ValueError(f'Unknown service: {service_id!r}')
|
raise ValueError(f'Unknown service: {service_id!r}')
|
||||||
@@ -116,6 +168,8 @@ class AccountManager:
|
|||||||
manager_name = accounts_cfg.get('manager')
|
manager_name = accounts_cfg.get('manager')
|
||||||
if not manager_name:
|
if not manager_name:
|
||||||
raise ValueError(f'Service {service_id!r} does not support accounts')
|
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)
|
manager = self._managers.get(manager_name)
|
||||||
if manager is None:
|
if manager is None:
|
||||||
raise ValueError(f'Manager {manager_name!r} is not registered with AccountManager')
|
raise ValueError(f'Manager {manager_name!r} is not registered with AccountManager')
|
||||||
@@ -135,12 +189,14 @@ class AccountManager:
|
|||||||
if password is None:
|
if password is None:
|
||||||
password = _secrets_mod.token_urlsafe(16)
|
password = _secrets_mod.token_urlsafe(16)
|
||||||
|
|
||||||
dispatch = _DISPATCH_PROVISION.get(manager_name)
|
if manager_name == 'http':
|
||||||
if dispatch is None:
|
ok = self._provision_http(svc, peer_username, password)
|
||||||
raise ValueError(f'No provision dispatch for manager: {manager_name!r}')
|
else:
|
||||||
fn = getattr(self, dispatch)
|
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:
|
if not ok:
|
||||||
raise RuntimeError(
|
raise RuntimeError(
|
||||||
f'Provision of {peer_username!r} on {service_id!r} returned False — '
|
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."""
|
"""Delete the peer's account on the service and clear stored credentials."""
|
||||||
svc, manager_name, manager = self._resolve_service(service_id)
|
svc, manager_name, manager = self._resolve_service(service_id)
|
||||||
|
|
||||||
dispatch = _DISPATCH_DEPROVISION.get(manager_name)
|
if manager_name == 'http':
|
||||||
if dispatch is None:
|
ok = self._deprovision_http(svc, peer_username)
|
||||||
raise ValueError(f'No deprovision dispatch for manager: {manager_name!r}')
|
else:
|
||||||
fn = getattr(self, dispatch)
|
dispatch = _DISPATCH_DEPROVISION.get(manager_name)
|
||||||
|
if dispatch is None:
|
||||||
ok = fn(manager, svc, peer_username)
|
raise ValueError(f'No deprovision dispatch for manager: {manager_name!r}')
|
||||||
|
ok = getattr(self, dispatch)(manager, svc, peer_username)
|
||||||
|
|
||||||
with self._lock:
|
with self._lock:
|
||||||
all_creds = self._load_creds()
|
all_creds = self._load_creds()
|
||||||
|
|||||||
@@ -437,5 +437,95 @@ class TestEdgeCases(unittest.TestCase):
|
|||||||
self.assertIsNone(self.am.get_credentials('email', 'alice'))
|
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__':
|
if __name__ == '__main__':
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
Reference in New Issue
Block a user