roof/pic
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix post-deploy auth issues: best-effort service provisioning, integration test auth, test mock corrections

Browse Source 
- api/app.py: email/calendar/files provisioning now best-effort (non-fatal); fixed email_manager.create_email_user call to include domain argument
- tests/integration: added module-level auth sessions to all integration test files; added admin auth to api fixture and _resolve_admin_pass() helper; added TEST_PEER_PASSWORD constant; added password to peer creation calls
- tests/test_peer_provisioning.py: renamed rollback test to reflect new best-effort semantics (email failure no longer causes rollback)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Dmitrii Iurco
2026-04-25 15:42:03 -04:00
parent 975d05eef3
commit fc3cfc9741
10 changed files with 184 additions and 88 deletions
Download Patch File Download Diff File Expand all files Collapse all files
api/app.py
+17 -37
View File
@@ -1845,45 +1845,25 @@ def add_peer():
peer_name = data['name']
# --- Provision service accounts with rollback on failure ---
provisioned = []
try:
auth_manager.create_user(peer_name, password, 'peer')
provisioned.append('auth')
# --- Provision auth account (hard-required) ---
if not auth_manager.create_user(peer_name, password, 'peer'):
return jsonify({"error": f"Could not create auth account (duplicate name?)"}), 400
email_manager.create_email_user(peer_name, password)
provisioned.append('email')
calendar_manager.create_calendar_user(peer_name, password)
provisioned.append('calendar')
file_manager.create_user(peer_name, password)
provisioned.append('files')
except Exception as prov_err:
logger.error(f"Peer provisioning failed at step {provisioned}: {prov_err}")
# Rollback everything provisioned so far
if 'files' in provisioned:
# --- Provision service accounts (best-effort; failures logged but non-fatal) ---
provisioned = ['auth']
domain = _configured_domain()
for step_name, step_fn in [
('email', lambda: email_manager.create_email_user(peer_name, domain, password)),
('calendar', lambda: calendar_manager.create_calendar_user(peer_name, password)),
('files', lambda: file_manager.create_user(peer_name, password)),
]:
try:
file_manager.delete_user(peer_name)
except Exception:
pass
if 'calendar' in provisioned:
try:
calendar_manager.delete_calendar_user(peer_name)
except Exception:
pass
if 'email' in provisioned:
try:
email_manager.delete_email_user(peer_name)
except Exception:
pass
if 'auth' in provisioned:
try:
auth_manager.delete_user(peer_name)
except Exception:
pass
return jsonify({"error": f"Peer provisioning failed: {prov_err}"}), 500
if step_fn():
provisioned.append(step_name)
else:
logger.warning(f"Peer {peer_name}: {step_name} account creation returned False (service may not be ready)")
except Exception as e:
logger.warning(f"Peer {peer_name}: {step_name} account creation failed (non-fatal): {e}")
# Add peer to registry with all provided fields
peer_info = {
tests/integration/conftest.py
+31 -9
View File
@@ -6,6 +6,8 @@ Configure with environment variables:
PIC_API_PORT API port (default: 3000)
PIC_WEBUI_PORT WebUI port (default: 80)
PIC_WG_CONTAINER WireGuard container name (default: cell-wireguard)
PIC_ADMIN_USER Admin username (default: admin)
PIC_ADMIN_PASS Admin password (default: read from data/api/.admin_initial_password or env)
"""
import os
import json
@@ -17,6 +19,8 @@ PIC_HOST = os.environ.get('PIC_HOST', 'localhost')
API_PORT = int(os.environ.get('PIC_API_PORT', '3000'))
WEBUI_PORT = int(os.environ.get('PIC_WEBUI_PORT', '80'))
WG_CONTAINER = os.environ.get('PIC_WG_CONTAINER', 'cell-wireguard')
ADMIN_USER = os.environ.get('PIC_ADMIN_USER', 'admin')
ADMIN_PASS = os.environ.get('PIC_ADMIN_PASS', '')
API_BASE = f"http://{PIC_HOST}:{API_PORT}"
WEBUI_BASE = f"http://{PIC_HOST}:{WEBUI_PORT}"
@@ -28,11 +32,33 @@ TEST_PEERS = (
'bad-svc-peer', # guard against validation-test leak
)
TEST_PEER_PASSWORD = 'IntegrationTest123!'
def _resolve_admin_pass() -> str:
if ADMIN_PASS:
return ADMIN_PASS
# Try reading from the initial password file (present on first run before bootstrap)
candidate = os.path.join(
os.path.dirname(__file__), '..', '..', 'data', 'api', '.admin_initial_password'
)
candidate = os.path.normpath(candidate)
if os.path.exists(candidate):
return open(candidate).read().strip()
raise RuntimeError(
"Admin password unknown. Set PIC_ADMIN_PASS env var or run make setup first."
)
@pytest.fixture(scope='session')
def api():
"""Authenticated requests.Session logged in as admin."""
s = requests.Session()
s.headers['Content-Type'] = 'application/json'
password = _resolve_admin_pass()
r = s.post(f"{API_BASE}/api/auth/login", json={'username': ADMIN_USER, 'password': password})
if r.status_code != 200:
raise RuntimeError(f"Integration test login failed: {r.status_code} {r.text}")
return s
@@ -71,17 +97,13 @@ def peer_rules(peer_ip: str) -> list[str]:
return [line for line in iptables_forward().splitlines() if comment in line]
def get_live_service_vips() -> dict:
def get_live_service_vips(session: requests.Session = None) -> dict:
"""
Read virtual IPs from the config API.
The config API computes service_ips from the current ip_range at request time,
so it always matches what the running firewall_manager will use when applying
peer rules. Using docker exec on the API container is NOT reliable because
it spawns a fresh Python process that imports firewall_manager with its initial
hardcoded SERVICE_IPS, ignoring any update_service_ips() calls made at runtime.
Read virtual IPs from the config API using an authenticated session.
Falls back to a new unauthenticated request only if no session provided (legacy).
"""
cfg = requests.get(f"{API_BASE}/api/config").json()
s = session or requests.Session()
cfg = s.get(f"{API_BASE}/api/config").json()
sips = cfg.get('service_ips', {})
return {
'calendar': sips.get('vip_calendar', ''),
tests/integration/test_apply_propagation.py
+17 -5
View File
@@ -37,7 +37,7 @@ import requests
from requests.exceptions import ConnectionError, Timeout
sys.path.insert(0, os.path.dirname(__file__))
from conftest import API_BASE
from conftest import API_BASE, _resolve_admin_pass
# ---------------------------------------------------------------------------
# Constants
@@ -56,20 +56,32 @@ _CAL_PORT_B = 5233 # an alternate safe value used as the "changed" state
# Helpers
# ---------------------------------------------------------------------------
_S = None
@pytest.fixture(scope='module', autouse=True)
def _auth_session():
global _S
_S = requests.Session()
_S.headers['Content-Type'] = 'application/json'
r = _S.post(f"{API_BASE}/api/auth/login",
json={'username': 'admin', 'password': _resolve_admin_pass()})
assert r.status_code == 200, f"Login failed: {{r.text}}"
def get(path, **kw):
return requests.get(f"{API_BASE}{path}", **kw)
return _S.get(f"{API_BASE}{path}", **kw)
def put(path, **kw):
return requests.put(f"{API_BASE}{path}", **kw)
return _S.put(f"{API_BASE}{path}", **kw)
def post(path, **kw):
return requests.post(f"{API_BASE}{path}", **kw)
return _S.post(f"{API_BASE}{path}", **kw)
def delete(path, **kw):
return requests.delete(f"{API_BASE}{path}", **kw)
return _S.delete(f"{API_BASE}{path}", **kw)
def wait_for_healthy(timeout: int = _HEALTH_TIMEOUT) -> bool:
tests/integration/test_config_api.py
+16 -4
View File
@@ -16,19 +16,31 @@ import requests
import sys
import os
sys.path.insert(0, os.path.dirname(__file__))
from conftest import API_BASE
from conftest import API_BASE, _resolve_admin_pass
_S = None
@pytest.fixture(scope='module', autouse=True)
def _auth_session():
global _S
_S = requests.Session()
_S.headers['Content-Type'] = 'application/json'
r = _S.post(f"{API_BASE}/api/auth/login",
json={'username': 'admin', 'password': _resolve_admin_pass()})
assert r.status_code == 200, f"Login failed: {{r.text}}"
def get(path, **kw):
return requests.get(f"{API_BASE}{path}", **kw)
return _S.get(f"{API_BASE}{path}", **kw)
def put(path, **kw):
return requests.put(f"{API_BASE}{path}", **kw)
return _S.put(f"{API_BASE}{path}", **kw)
def post(path, **kw):
return requests.post(f"{API_BASE}{path}", **kw)
return _S.post(f"{API_BASE}{path}", **kw)
# ---------------------------------------------------------------------------
tests/integration/test_containers.py
+16 -4
View File
@@ -19,7 +19,7 @@ import requests
import sys
import os
sys.path.insert(0, os.path.dirname(__file__))
from conftest import API_BASE
from conftest import API_BASE, _resolve_admin_pass
# A non-critical container safe to restart during testing.
# cell-ntp has no write-side effects and recovers in seconds.
@@ -29,12 +29,24 @@ _SAFE_TO_RESTART = 'cell-ntp'
_NONEXISTENT = 'cell-does-not-exist-xyz'
_S = None
@pytest.fixture(scope='module', autouse=True)
def _auth_session():
global _S
_S = requests.Session()
_S.headers['Content-Type'] = 'application/json'
r = _S.post(f"{API_BASE}/api/auth/login",
json={'username': 'admin', 'password': _resolve_admin_pass()})
assert r.status_code == 200, f"Login failed: {{r.text}}"
def get(path, **kw):
return requests.get(f"{API_BASE}{path}", **kw)
return _S.get(f"{API_BASE}{path}", **kw)
def post(path, **kw):
return requests.post(f"{API_BASE}{path}", **kw)
return _S.post(f"{API_BASE}{path}", **kw)
# Skip the entire module if the container endpoint is access-denied.
@@ -42,7 +54,7 @@ def post(path, **kw):
# is_local_request(). Run `make update` to rebuild and re-enable these tests.
def _containers_accessible():
try:
return requests.get(f"{API_BASE}/api/containers", timeout=3).status_code != 403
return _S.get(f"{API_BASE}/api/containers", timeout=3).status_code != 403
except Exception:
return False
tests/integration/test_live_api.py
+15 -3
View File
@@ -7,16 +7,28 @@ Or: PIC_HOST=192.168.31.51 pytest tests/integration/test_live_api.py -v
import pytest
import sys, os
sys.path.insert(0, os.path.dirname(__file__))
from conftest import API_BASE
from conftest import API_BASE, _resolve_admin_pass
# Shorthand helpers — always hits the live API
import requests as _req
_S = None
@pytest.fixture(scope='module', autouse=True)
def _auth_session():
global _S
_S = requests.Session()
_S.headers['Content-Type'] = 'application/json'
r = _S.post(f"{API_BASE}/api/auth/login",
json={'username': 'admin', 'password': _resolve_admin_pass()})
assert r.status_code == 200, f"Login failed: {{r.text}}"
def get(path, **kw):
return _req.get(f"{API_BASE}{path}", **kw)
return _S.get(f"{API_BASE}{path}", **kw)
def post(path, **kw):
return _req.post(f"{API_BASE}{path}", **kw)
return _S.post(f"{API_BASE}{path}", **kw)
# ---------------------------------------------------------------------------
tests/integration/test_negative_scenarios.py
+17 -5
View File
@@ -22,27 +22,39 @@ import requests
import sys
import os
sys.path.insert(0, os.path.dirname(__file__))
from conftest import API_BASE
from conftest import API_BASE, _resolve_admin_pass
# Sentinel peer name that should never exist in the registry
_GHOST_PEER = 'ghost-peer-that-does-not-exist-xyz'
_GHOST_CONTAINER = 'cell-container-does-not-exist-xyz'
_S = None
@pytest.fixture(scope='module', autouse=True)
def _auth_session():
global _S
_S = requests.Session()
_S.headers['Content-Type'] = 'application/json'
r = _S.post(f"{API_BASE}/api/auth/login",
json={'username': 'admin', 'password': _resolve_admin_pass()})
assert r.status_code == 200, f"Login failed: {{r.text}}"
def get(path, **kw):
return requests.get(f"{API_BASE}{path}", **kw)
return _S.get(f"{API_BASE}{path}", **kw)
def post(path, **kw):
return requests.post(f"{API_BASE}{path}", **kw)
return _S.post(f"{API_BASE}{path}", **kw)
def put(path, **kw):
return requests.put(f"{API_BASE}{path}", **kw)
return _S.put(f"{API_BASE}{path}", **kw)
def delete(path, **kw):
return requests.delete(f"{API_BASE}{path}", **kw)
return _S.delete(f"{API_BASE}{path}", **kw)
# ---------------------------------------------------------------------------
tests/integration/test_network_services.py
+16 -4
View File
@@ -13,22 +13,34 @@ import requests
import sys
import os
sys.path.insert(0, os.path.dirname(__file__))
from conftest import API_BASE
from conftest import API_BASE, _resolve_admin_pass
# Test DNS hostname to use — must be cleaned up after tests
_TEST_DNS_HOSTNAME = 'inttest-dns-record'
_S: requests.Session = None
@pytest.fixture(scope='module', autouse=True)
def _auth_session():
global _S
_S = requests.Session()
_S.headers['Content-Type'] = 'application/json'
r = _S.post(f"{API_BASE}/api/auth/login",
json={'username': 'admin', 'password': _resolve_admin_pass()})
assert r.status_code == 200, f"Login failed: {r.text}"
def get(path, **kw):
return requests.get(f"{API_BASE}{path}", **kw)
return _S.get(f"{API_BASE}{path}", **kw)
def post(path, **kw):
return requests.post(f"{API_BASE}{path}", **kw)
return _S.post(f"{API_BASE}{path}", **kw)
def delete(path, **kw):
return requests.delete(f"{API_BASE}{path}", **kw)
return _S.delete(f"{API_BASE}{path}", **kw)
# ---------------------------------------------------------------------------
tests/integration/test_peer_lifecycle.py
+23 -6
View File
@@ -16,24 +16,37 @@ import pytest
import requests
import sys, os
sys.path.insert(0, os.path.dirname(__file__))
from conftest import API_BASE, peer_rules, iptables_forward, get_live_service_vips
from conftest import API_BASE, peer_rules, iptables_forward, get_live_service_vips, TEST_PEER_PASSWORD, _resolve_admin_pass
# Service → virtual IP mapping (mirrors firewall_manager.SERVICE_IPS)
ALL_SERVICES = {'calendar', 'files', 'mail', 'webdav'}
ALL_PEERS = ('integration-test-full', 'integration-test-restricted', 'integration-test-none')
# Module-level authenticated session — set once by the autouse fixture below
_S: requests.Session = None
@pytest.fixture(scope='module', autouse=True)
def _auth_session():
global _S
_S = requests.Session()
_S.headers['Content-Type'] = 'application/json'
r = _S.post(f"{API_BASE}/api/auth/login",
json={'username': 'admin', 'password': _resolve_admin_pass()})
assert r.status_code == 200, f"Login failed: {r.text}"
def api_post(path, **kw):
return requests.post(f"{API_BASE}{path}", **kw)
return _S.post(f"{API_BASE}{path}", **kw)
def api_get(path, **kw):
return requests.get(f"{API_BASE}{path}", **kw)
return _S.get(f"{API_BASE}{path}", **kw)
def api_put(path, **kw):
return requests.put(f"{API_BASE}{path}", **kw)
return _S.put(f"{API_BASE}{path}", **kw)
def api_delete(path, **kw):
return requests.delete(f"{API_BASE}{path}", **kw)
return _S.delete(f"{API_BASE}{path}", **kw)
# ---------------------------------------------------------------------------
@@ -109,6 +122,7 @@ class TestPeerFullAccess:
'name': self.PEER_NAME,
'public_key': keys['public_key'],
'service_access': list(ALL_SERVICES),
'password': TEST_PEER_PASSWORD,
})
assert r.status_code == 201, f"Peer creation failed: {r.text}"
data = r.json()
@@ -143,8 +157,9 @@ class TestPeerFullAccess:
r = api_post('/api/peers', json={
'name': self.PEER_NAME,
'public_key': keys['public_key'],
'password': TEST_PEER_PASSWORD,
})
assert r.status_code == 400, "Duplicate peer should be rejected"
assert r.status_code in (400, 409), "Duplicate peer should be rejected"
def test_delete_peer_full_access(self):
r = api_delete(f'/api/peers/{self.PEER_NAME}')
@@ -180,6 +195,7 @@ class TestPeerRestrictedAccess:
'public_key': keys['public_key'],
'service_access': ['calendar'],
'internet_access': False,
'password': TEST_PEER_PASSWORD,
})
assert r.status_code == 201, f"Peer creation failed: {r.text}"
@@ -254,6 +270,7 @@ class TestPeerNoAccess:
'service_access': [],
'internet_access': False,
'peer_access': False,
'password': TEST_PEER_PASSWORD,
})
assert r.status_code == 201, f"Peer creation failed: {r.text}"
tests/test_peer_provisioning.py
+11 -6
View File
@@ -221,10 +221,11 @@ def test_create_peer_requires_public_key(admin_client):
# ── POST /api/peers — rollback on failure ─────────────────────────────────────
def test_create_peer_rollback_on_email_failure(
def test_create_peer_email_failure_is_nonfatal(
auth_mgr, mock_email_mgr, mock_calendar_mgr,
mock_file_mgr, mock_wg_mgr, mock_peer_registry):
"""If email_manager.create_email_user raises, auth user must be deleted (rollback)."""
"""Email provisioning is best-effort: if create_email_user raises, peer creation
still succeeds (201) and the auth user is NOT rolled back."""
mock_email_mgr.create_email_user.side_effect = RuntimeError('SMTP server down')
app.config['TESTING'] = True
@@ -252,11 +253,15 @@ def test_create_peer_rollback_on_email_failure(
with app.test_client() as client:
r = _login(client)
assert r.status_code == 200
_post_peer(client)
# alice must not remain in the auth store (rolled back)
resp = _post_peer(client)
# Peer creation must succeed despite email failure (best-effort)
assert resp.status_code == 201, (
f'expected 201 but got {resp.status_code}: {resp.data}'
)
# Auth user must remain — no rollback for non-fatal service failures
alice = auth_mgr.get_user('alice')
assert alice is None, (
'auth user alice was not rolled back after email_manager failure'
assert alice is not None, (
'auth user alice was incorrectly rolled back after non-fatal email failure'
)
finally:
for p in patches: