Add domain conflict validation when changing domain or accepting heal invite
Two gaps allowed a cell to take a domain already in use by a connected cell: 1. PUT /api/config domain change: added check against cell_link_manager's connected cells list before saving — returns 409 if the new domain collides with any connected cell's domain. 2. accept_invite healing path: a remote cell changing its domain via a re-invite was not validated against other connected cells' domains. Now calls _check_invite_conflicts(invite, exclude_cell=name) before applying any change. Also: the healing path now detects domain changes (alongside dns_ip/ vpn_subnet/endpoint), updates the stored domain, and refreshes the DNS forward rule when the domain changes. Tests: 3 new domain-conflict tests in test_config_validation.py; 3 new accept_invite healing tests in test_cell_link_manager.py. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -511,26 +511,37 @@ class CellLinkManager:
|
|||||||
links = self._load()
|
links = self._load()
|
||||||
name = invite['cell_name']
|
name = invite['cell_name']
|
||||||
|
|
||||||
# Already connected — check whether the remote's endpoint or subnet changed
|
# Already connected — check whether the remote's endpoint, subnet, or domain changed
|
||||||
# (e.g. the remote cell changed its WireGuard address) and heal if so.
|
# (e.g. the remote cell changed its WireGuard address or domain) and heal if so.
|
||||||
existing = next((l for l in links if l['cell_name'] == name), None)
|
existing = next((l for l in links if l['cell_name'] == name), None)
|
||||||
if existing:
|
if existing:
|
||||||
dns_changed = existing.get('dns_ip') != invite['dns_ip']
|
dns_changed = existing.get('dns_ip') != invite['dns_ip']
|
||||||
subnet_changed = existing.get('vpn_subnet') != invite['vpn_subnet']
|
subnet_changed = existing.get('vpn_subnet') != invite['vpn_subnet']
|
||||||
endpoint_changed = (invite.get('endpoint') and
|
endpoint_changed = (invite.get('endpoint') and
|
||||||
invite['endpoint'] != existing.get('endpoint'))
|
invite['endpoint'] != existing.get('endpoint'))
|
||||||
if dns_changed or subnet_changed or endpoint_changed:
|
domain_changed = (invite.get('domain') and
|
||||||
|
invite['domain'] != existing.get('domain'))
|
||||||
|
if dns_changed or subnet_changed or endpoint_changed or domain_changed:
|
||||||
|
# Before healing, verify the updated invite doesn't conflict with
|
||||||
|
# other connected cells (exclude this cell by name so it's not
|
||||||
|
# self-blocking when only endpoint/dns_ip changed).
|
||||||
|
self._check_invite_conflicts(invite, exclude_cell=name)
|
||||||
|
|
||||||
logger.info(
|
logger.info(
|
||||||
f"accept_invite: updating existing cell '{name}' "
|
f"accept_invite: updating existing cell '{name}' "
|
||||||
f"(dns_ip: {existing.get('dns_ip')} → {invite['dns_ip']}, "
|
f"(dns_ip: {existing.get('dns_ip')} → {invite['dns_ip']}, "
|
||||||
f"vpn_subnet: {existing.get('vpn_subnet')} → {invite['vpn_subnet']})"
|
f"vpn_subnet: {existing.get('vpn_subnet')} → {invite['vpn_subnet']}, "
|
||||||
|
f"domain: {existing.get('domain')} → {invite.get('domain')})"
|
||||||
)
|
)
|
||||||
old_subnet = existing.get('vpn_subnet', '')
|
old_subnet = existing.get('vpn_subnet', '')
|
||||||
|
old_domain = existing.get('domain', '')
|
||||||
existing['dns_ip'] = invite['dns_ip']
|
existing['dns_ip'] = invite['dns_ip']
|
||||||
existing['vpn_subnet'] = invite['vpn_subnet']
|
existing['vpn_subnet'] = invite['vpn_subnet']
|
||||||
existing['remote_api_url'] = f"http://{invite['dns_ip']}:3000"
|
existing['remote_api_url'] = f"http://{invite['dns_ip']}:3000"
|
||||||
if invite.get('endpoint'):
|
if invite.get('endpoint'):
|
||||||
existing['endpoint'] = invite['endpoint']
|
existing['endpoint'] = invite['endpoint']
|
||||||
|
if domain_changed:
|
||||||
|
existing['domain'] = invite['domain']
|
||||||
self._save(links)
|
self._save(links)
|
||||||
|
|
||||||
# Update WG peer AllowedIPs to the new subnet
|
# Update WG peer AllowedIPs to the new subnet
|
||||||
@@ -538,10 +549,10 @@ class CellLinkManager:
|
|||||||
self.wireguard_manager.update_peer_ip(
|
self.wireguard_manager.update_peer_ip(
|
||||||
existing['public_key'], invite['vpn_subnet'])
|
existing['public_key'], invite['vpn_subnet'])
|
||||||
|
|
||||||
# Update DNS forward rule (remove old, add new)
|
# Update DNS forward rule — triggers on dns_ip OR domain change
|
||||||
if dns_changed:
|
if dns_changed or domain_changed:
|
||||||
try:
|
try:
|
||||||
self.network_manager.remove_cell_dns_forward(existing['domain'])
|
self.network_manager.remove_cell_dns_forward(old_domain)
|
||||||
except Exception:
|
except Exception:
|
||||||
pass
|
pass
|
||||||
self.network_manager.add_cell_dns_forward(
|
self.network_manager.add_cell_dns_forward(
|
||||||
|
|||||||
@@ -158,6 +158,13 @@ def update_config():
|
|||||||
return jsonify({'error': 'domain must be 255 characters or fewer'}), 400
|
return jsonify({'error': 'domain must be 255 characters or fewer'}), 400
|
||||||
if not _DOMAIN_RE.match(v):
|
if not _DOMAIN_RE.match(v):
|
||||||
return jsonify({'error': 'Invalid domain: use only letters, digits, hyphens, dots'}), 400
|
return jsonify({'error': 'Invalid domain: use only letters, digits, hyphens, dots'}), 400
|
||||||
|
from app import cell_link_manager as _clm
|
||||||
|
for _link in _clm.list_connections():
|
||||||
|
if _link.get('domain') == v:
|
||||||
|
return jsonify({'error': (
|
||||||
|
f"Domain {v!r} is already used by connected cell "
|
||||||
|
f"'{_link['cell_name']}' — each cell must use a unique domain"
|
||||||
|
)}), 409
|
||||||
|
|
||||||
if 'ip_range' in identity_updates:
|
if 'ip_range' in identity_updates:
|
||||||
_rfc1918 = [
|
_rfc1918 = [
|
||||||
|
|||||||
@@ -645,6 +645,56 @@ class TestAcceptInviteNew(unittest.TestCase):
|
|||||||
self.mgr.accept_invite(SAMPLE_INVITE)
|
self.mgr.accept_invite(SAMPLE_INVITE)
|
||||||
self.assertEqual(len(self.mgr.list_connections()), 1)
|
self.assertEqual(len(self.mgr.list_connections()), 1)
|
||||||
|
|
||||||
|
def test_accept_invite_domain_change_updates_stored_domain(self):
|
||||||
|
"""accept_invite with a changed domain updates the stored domain."""
|
||||||
|
with patch('firewall_manager.apply_cell_rules'):
|
||||||
|
self.mgr.accept_invite(SAMPLE_INVITE)
|
||||||
|
updated = {**SAMPLE_INVITE, 'domain': 'office-new.cell'}
|
||||||
|
import sys
|
||||||
|
fake_cfg = MagicMock()
|
||||||
|
fake_cfg.configs = {'_identity': {'domain': 'home.cell'}}
|
||||||
|
with patch.dict(sys.modules, {'app': MagicMock(config_manager=fake_cfg)}):
|
||||||
|
with patch('firewall_manager.apply_cell_rules'):
|
||||||
|
result = self.mgr.accept_invite(updated)
|
||||||
|
self.assertEqual(result['domain'], 'office-new.cell')
|
||||||
|
|
||||||
|
def test_accept_invite_domain_change_updates_dns_forward(self):
|
||||||
|
"""accept_invite with a changed domain removes old DNS forward and adds new."""
|
||||||
|
with patch('firewall_manager.apply_cell_rules'):
|
||||||
|
self.mgr.accept_invite(SAMPLE_INVITE)
|
||||||
|
self.nm.reset_mock()
|
||||||
|
updated = {**SAMPLE_INVITE, 'domain': 'office-new.cell'}
|
||||||
|
import sys
|
||||||
|
fake_cfg = MagicMock()
|
||||||
|
fake_cfg.configs = {'_identity': {'domain': 'home.cell'}}
|
||||||
|
with patch.dict(sys.modules, {'app': MagicMock(config_manager=fake_cfg)}):
|
||||||
|
with patch('firewall_manager.apply_cell_rules'):
|
||||||
|
self.mgr.accept_invite(updated)
|
||||||
|
self.nm.remove_cell_dns_forward.assert_called_with('office.cell')
|
||||||
|
self.nm.add_cell_dns_forward.assert_called_with(
|
||||||
|
domain='office-new.cell', dns_ip=SAMPLE_INVITE['dns_ip'])
|
||||||
|
|
||||||
|
def test_accept_invite_healing_domain_conflict_raises(self):
|
||||||
|
"""Healing must reject a domain update that conflicts with another connected cell."""
|
||||||
|
import sys
|
||||||
|
fake_cfg = MagicMock()
|
||||||
|
fake_cfg.configs = {'_identity': {'domain': 'home.cell'}}
|
||||||
|
# Add two cells: 'office' and 'branch'
|
||||||
|
branch_invite = {**SAMPLE_INVITE,
|
||||||
|
'cell_name': 'branch', 'public_key': 'branchpubkey1234567890ABCDEFGH=',
|
||||||
|
'vpn_subnet': '10.9.0.0/24', 'dns_ip': '10.9.0.1',
|
||||||
|
'domain': 'branch.cell'}
|
||||||
|
with patch.dict(sys.modules, {'app': MagicMock(config_manager=fake_cfg)}):
|
||||||
|
with patch('firewall_manager.apply_cell_rules'):
|
||||||
|
self.mgr.accept_invite(SAMPLE_INVITE)
|
||||||
|
self.mgr.accept_invite(branch_invite)
|
||||||
|
# Now 'office' tries to heal its domain to 'branch.cell' — must fail
|
||||||
|
conflicting = {**SAMPLE_INVITE, 'domain': 'branch.cell'}
|
||||||
|
with patch.dict(sys.modules, {'app': MagicMock(config_manager=fake_cfg)}):
|
||||||
|
with self.assertRaises(ValueError) as ctx:
|
||||||
|
self.mgr.accept_invite(conflicting)
|
||||||
|
self.assertIn('branch.cell', str(ctx.exception))
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# TestAddConnectionMutualPairing
|
# TestAddConnectionMutualPairing
|
||||||
|
|||||||
@@ -170,5 +170,44 @@ class TestBodyValidation(unittest.TestCase):
|
|||||||
self.assertEqual(r.status_code, 200)
|
self.assertEqual(r.status_code, 200)
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Domain conflict validation
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
class TestDomainConflictValidation(unittest.TestCase):
|
||||||
|
"""Changing this cell's domain to one already used by a connected cell → 409."""
|
||||||
|
|
||||||
|
def setUp(self):
|
||||||
|
self.client = _make_client()
|
||||||
|
|
||||||
|
def test_domain_matching_connected_cell_returns_409(self):
|
||||||
|
"""PUT /api/config with domain='other.cell' conflicts with a connected cell."""
|
||||||
|
connected = [{'cell_name': 'remote', 'domain': 'other.cell',
|
||||||
|
'vpn_subnet': '10.5.0.0/24', 'dns_ip': '10.5.0.1'}]
|
||||||
|
with patch('app.cell_link_manager') as mock_clm:
|
||||||
|
mock_clm.list_connections.return_value = connected
|
||||||
|
r = _put(self.client, {'domain': 'other.cell'})
|
||||||
|
self.assertEqual(r.status_code, 409)
|
||||||
|
import json
|
||||||
|
data = json.loads(r.data)
|
||||||
|
self.assertIn('remote', data['error'])
|
||||||
|
|
||||||
|
def test_domain_not_matching_any_cell_is_accepted(self):
|
||||||
|
"""PUT /api/config with a domain not used by any connected cell → 200."""
|
||||||
|
connected = [{'cell_name': 'remote', 'domain': 'other.cell',
|
||||||
|
'vpn_subnet': '10.5.0.0/24', 'dns_ip': '10.5.0.1'}]
|
||||||
|
with patch('app.cell_link_manager') as mock_clm:
|
||||||
|
mock_clm.list_connections.return_value = connected
|
||||||
|
r = _put(self.client, {'domain': 'unique.cell'})
|
||||||
|
self.assertNotEqual(r.status_code, 409)
|
||||||
|
|
||||||
|
def test_domain_no_connected_cells_is_accepted(self):
|
||||||
|
"""PUT /api/config with domain change when no cells are connected → 200."""
|
||||||
|
with patch('app.cell_link_manager') as mock_clm:
|
||||||
|
mock_clm.list_connections.return_value = []
|
||||||
|
r = _put(self.client, {'domain': 'any.cell'})
|
||||||
|
self.assertNotEqual(r.status_code, 409)
|
||||||
|
|
||||||
|
|
||||||
if __name__ == '__main__':
|
if __name__ == '__main__':
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
Reference in New Issue
Block a user