Skip to content

Commit d3d2632

Browse files
Refactor: deduplicate Clients SSO logic and fix _relay_base edge cases
- Extract shared RemoteClients base class to eliminate duplicated auto-SSO login logic between edge.py and drive.py Clients classes - Fix potential bug in _relay_base: use startswith() instead of substring `in` check for device DNS name matching - Fix trailing-slash edge case in _relay_base URL construction - Add comprehensive unit tests for _relay_base (DNS fallback, hostname derivation, port handling, trailing slash) - Extract common test setup into _setup_remote_device_with_sso helper Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 0298206 commit d3d2632

6 files changed

Lines changed: 137 additions & 62 deletions

File tree

cterasdk/core/remote.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,13 @@ def _relay_base(Portal, device):
1515
Host header while still connecting to the same portal endpoint.
1616
"""
1717
device_dns = getattr(device, 'deviceDnsName', None)
18-
if device_dns and device.name in device_dns:
19-
portal_hostname = device_dns[len(device.name) + 1:] # strip "vGateway-7192."
18+
if device_dns and device_dns.startswith(f'{device.name}.'):
19+
portal_hostname = device_dns[len(device.name) + 1:]
2020
parsed = urlparse(Portal.ctera.baseurl)
2121
port = f':{parsed.port}' if parsed.port not in (None, 80, 443) else ''
22-
return f'{parsed.scheme}://{portal_hostname}{port}{parsed.path}/devices/{device.name}'
23-
return f'{Portal.ctera.baseurl}/devices/{device.name}'
22+
base_path = parsed.path.rstrip('/')
23+
return f'{parsed.scheme}://{portal_hostname}{port}{base_path}/devices/{device.name}'
24+
return f'{Portal.ctera.baseurl.rstrip("/")}/devices/{device.name}'
2425

2526

2627
def remote_command(Portal, device):

cterasdk/objects/synchronous/drive.py

Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,23 @@
1-
import logging
2-
31
import cterasdk.settings
42
from ...clients import clients
53
from ..services import Management
64
from ..endpoints import EndpointBuilder
7-
from ...common import parse_base_object_ref
85
from ...lib.session.edge import Session
96
from ...edge import backup, cli, logs, services, support, sync
7+
from .remote_clients import RemoteClients
108

119

12-
logger = logging.getLogger('cterasdk.drive')
13-
14-
15-
class Clients:
10+
class Clients(RemoteClients):
1611

1712
def __init__(self, drive, Portal):
18-
self._drive = drive
19-
self._Portal = Portal
20-
self._authenticated = False
2113
if Portal:
2214
drive._Portal = Portal
2315
drive.default.close()
2416
drive._ctera_session.start_remote_session(Portal.session())
25-
self._api = Portal.default.clone(clients.API, EndpointBuilder.new(drive.base, '/admingui/api'), authenticator=lambda *_: True)
17+
api_client = Portal.default.clone(clients.API, EndpointBuilder.new(drive.base, '/admingui/api'), authenticator=lambda *_: True)
2618
else:
27-
self._api = drive.default.clone(clients.API, EndpointBuilder.new(drive.base, '/admingui/api'))
28-
29-
@property
30-
def api(self):
31-
if self._Portal and not self._authenticated:
32-
tenant = parse_base_object_ref(self._drive.portal).name
33-
device_name = self._drive.name
34-
logger.debug('Auto-SSO login via relay channel. %s', {'tenant': tenant, 'device': device_name})
35-
token = self._Portal.api.execute(f'/portals/{tenant}/devices/{device_name}', 'singleSignOn')
36-
if token:
37-
self._api.get('/ssologin', params={'ticket': token})
38-
self._authenticated = True
39-
return self._api
19+
api_client = drive.default.clone(clients.API, EndpointBuilder.new(drive.base, '/admingui/api'))
20+
super().__init__(drive, Portal, api_client)
4021

4122

4223
class Drive(Management):

cterasdk/objects/synchronous/edge.py

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
1-
import logging
2-
31
import cterasdk.settings
42
from ...clients import clients
53
from ..services import Management
64
from ..endpoints import EndpointBuilder
75
from .. import authenticators
8-
from ...common import modules, parse_base_object_ref
6+
from ...common import modules
97
from ...lib.session.edge import Session
8+
from .remote_clients import RemoteClients
109

1110

1211
from ...edge import (
@@ -18,36 +17,19 @@
1817
)
1918

2019

21-
logger = logging.getLogger('cterasdk.edge')
22-
23-
24-
class Clients:
20+
class Clients(RemoteClients):
2521

2622
def __init__(self, edge, Portal):
27-
self._edge = edge
28-
self._Portal = Portal
29-
self._authenticated = False
3023
if Portal:
3124
edge._Portal = Portal
3225
edge.default.close()
3326
edge._ctera_session.start_remote_session(Portal.session())
34-
self._api = Portal.default.clone(clients.API, EndpointBuilder.new(edge.base, '/admingui/api'), authenticator=lambda *_: True)
27+
api_client = Portal.default.clone(clients.API, EndpointBuilder.new(edge.base, '/admingui/api'), authenticator=lambda *_: True)
3528
else:
3629
self.migrate = edge.default.clone(clients.Migrate, EndpointBuilder.new(edge.base, '/migration/rest/v1'))
37-
self._api = edge.default.clone(clients.API, EndpointBuilder.new(edge.base, '/admingui/api'))
30+
api_client = edge.default.clone(clients.API, EndpointBuilder.new(edge.base, '/admingui/api'))
3831
self.io = IO(edge)
39-
40-
@property
41-
def api(self):
42-
if self._Portal and not self._authenticated:
43-
tenant = parse_base_object_ref(self._edge.portal).name
44-
device_name = self._edge.name
45-
logger.debug('Auto-SSO login via relay channel. %s', {'tenant': tenant, 'device': device_name})
46-
token = self._Portal.api.execute(f'/portals/{tenant}/devices/{device_name}', 'singleSignOn')
47-
if token:
48-
self._api.get('/ssologin', params={'ticket': token})
49-
self._authenticated = True
50-
return self._api
32+
super().__init__(edge, Portal, api_client)
5133

5234

5335
class IO:
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import logging
2+
3+
from ...common import parse_base_object_ref
4+
5+
6+
logger = logging.getLogger('cterasdk.remote')
7+
8+
9+
class RemoteClients:
10+
"""
11+
Base class for remote device client management with lazy SSO authentication.
12+
13+
Subclasses must set ``self._device`` before calling ``super().__init__``.
14+
"""
15+
16+
def __init__(self, device, Portal, api_client):
17+
self._device = device
18+
self._Portal = Portal
19+
self._authenticated = False
20+
self._api = api_client
21+
22+
@property
23+
def api(self):
24+
if self._Portal and not self._authenticated:
25+
tenant = parse_base_object_ref(self._device.portal).name
26+
device_name = self._device.name
27+
logger.debug('Auto-SSO login via relay channel. %s', {'tenant': tenant, 'device': device_name})
28+
token = self._Portal.api.execute(f'/portals/{tenant}/devices/{device_name}', 'singleSignOn')
29+
if token:
30+
self._api.get('/ssologin', params={'ticket': token})
31+
self._authenticated = True
32+
return self._api
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import unittest
2+
from unittest import mock
3+
4+
from cterasdk.core.remote import _relay_base
5+
6+
7+
class TestRelayBase(unittest.TestCase):
8+
9+
def _make_portal(self, baseurl):
10+
portal = mock.MagicMock()
11+
portal.ctera.baseurl = baseurl
12+
return portal
13+
14+
def _make_device(self, name, device_dns_name=None):
15+
device = mock.MagicMock(spec=['name', 'deviceDnsName'] if device_dns_name is not None else ['name'])
16+
device.name = name
17+
if device_dns_name is not None:
18+
device.deviceDnsName = device_dns_name
19+
return device
20+
21+
def test_fallback_when_device_dns_is_none(self):
22+
portal = self._make_portal('https://portal.ctera.me')
23+
device = self._make_device('vGateway-7192')
24+
result = _relay_base(portal, device)
25+
self.assertEqual(result, 'https://portal.ctera.me/devices/vGateway-7192')
26+
27+
def test_fallback_when_device_dns_does_not_start_with_name(self):
28+
portal = self._make_portal('https://portal.ctera.me')
29+
device = self._make_device('vGateway-7192', 'other-device.portal.ctera.me')
30+
result = _relay_base(portal, device)
31+
self.assertEqual(result, 'https://portal.ctera.me/devices/vGateway-7192')
32+
33+
def test_hostname_derivation_from_dns_name(self):
34+
portal = self._make_portal('https://10.0.0.1')
35+
device = self._make_device('vGateway-7192', 'vGateway-7192.portal.ctera.me')
36+
result = _relay_base(portal, device)
37+
self.assertEqual(result, 'https://portal.ctera.me/devices/vGateway-7192')
38+
39+
def test_non_standard_port_preserved(self):
40+
portal = self._make_portal('https://10.0.0.1:8443')
41+
device = self._make_device('vGateway-7192', 'vGateway-7192.portal.ctera.me')
42+
result = _relay_base(portal, device)
43+
self.assertEqual(result, 'https://portal.ctera.me:8443/devices/vGateway-7192')
44+
45+
def test_standard_https_port_omitted(self):
46+
portal = self._make_portal('https://10.0.0.1:443')
47+
device = self._make_device('vGateway-7192', 'vGateway-7192.portal.ctera.me')
48+
result = _relay_base(portal, device)
49+
self.assertEqual(result, 'https://portal.ctera.me/devices/vGateway-7192')
50+
51+
def test_standard_http_port_omitted(self):
52+
portal = self._make_portal('http://10.0.0.1:80')
53+
device = self._make_device('vGateway-7192', 'vGateway-7192.portal.ctera.me')
54+
result = _relay_base(portal, device)
55+
self.assertEqual(result, 'http://portal.ctera.me/devices/vGateway-7192')
56+
57+
def test_trailing_slash_in_baseurl_no_double_slash(self):
58+
portal = self._make_portal('https://portal.ctera.me/')
59+
device = self._make_device('vGateway-7192', 'vGateway-7192.portal.ctera.me')
60+
result = _relay_base(portal, device)
61+
self.assertNotIn('//', result.split('://')[1])
62+
self.assertEqual(result, 'https://portal.ctera.me/devices/vGateway-7192')
63+
64+
def test_baseurl_with_path(self):
65+
portal = self._make_portal('https://10.0.0.1/api/v1')
66+
device = self._make_device('vGateway-7192', 'vGateway-7192.portal.ctera.me')
67+
result = _relay_base(portal, device)
68+
self.assertEqual(result, 'https://portal.ctera.me/api/v1/devices/vGateway-7192')
69+
70+
def test_substring_device_name_does_not_false_match(self):
71+
"""A device named 'gw' should NOT match dns 'other-gw.portal.ctera.me'."""
72+
portal = self._make_portal('https://portal.ctera.me')
73+
device = self._make_device('gw', 'other-gw.portal.ctera.me')
74+
result = _relay_base(portal, device)
75+
self.assertEqual(result, 'https://portal.ctera.me/devices/gw')
76+
77+
def test_fallback_strips_trailing_slash(self):
78+
portal = self._make_portal('https://portal.ctera.me/')
79+
device = self._make_device('vGateway-7192')
80+
result = _relay_base(portal, device)
81+
self.assertEqual(result, 'https://portal.ctera.me/devices/vGateway-7192')

tests/ut/core/admin/test_remote.py

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,8 @@ def _create_device_param(name, portal, device_type, remote_access_url):
7373
param.remoteAccessUrl = remote_access_url
7474
return param
7575

76-
def test_auto_sso_on_first_api_access(self):
77-
"""Verify that first access to edge.api triggers SSO login via relay channel."""
76+
def _setup_remote_device_with_sso(self):
77+
"""Common setup for SSO auto-login tests. Returns the remote device with a mocked _api."""
7878
remote_session = self.patch_call("cterasdk.lib.session.edge.Session.start_remote_session")
7979
remote_session.return_value = munch.Munch({'account': munch.Munch({'name': 'mickey', 'tenant': 'tenant'})})
8080
get_multi_response = TestCoreRemote._create_device_param(self._device_name, self._device_portal,
@@ -83,21 +83,19 @@ def test_auto_sso_on_first_api_access(self):
8383
self._activate_portal_session()
8484
device = devices.Devices(self._global_admin).device(self._device_name)
8585
device._ctera_clients._api = mock.MagicMock()
86+
return device
87+
88+
def test_auto_sso_on_first_api_access(self):
89+
"""Verify that first access to edge.api triggers SSO login via relay channel."""
90+
device = self._setup_remote_device_with_sso()
8691
_ = device.api
8792
self._global_admin.api.execute.assert_called_once_with(
8893
f'/portals/{self._tenant_name}/devices/{self._device_name}', 'singleSignOn')
8994
device._ctera_clients._api.get.assert_called_once_with('/ssologin', params={'ticket': self._sso_ticket})
9095

9196
def test_auto_sso_not_repeated_on_subsequent_api_access(self):
9297
"""Verify that subsequent api accesses do not re-trigger SSO."""
93-
remote_session = self.patch_call("cterasdk.lib.session.edge.Session.start_remote_session")
94-
remote_session.return_value = munch.Munch({'account': munch.Munch({'name': 'mickey', 'tenant': 'tenant'})})
95-
get_multi_response = TestCoreRemote._create_device_param(self._device_name, self._device_portal,
96-
'vGateway', self._device_remote_access_url)
97-
self._init_global_admin(get_multi_response=get_multi_response, execute_response=self._sso_ticket)
98-
self._activate_portal_session()
99-
device = devices.Devices(self._global_admin).device(self._device_name)
100-
device._ctera_clients._api = mock.MagicMock()
98+
device = self._setup_remote_device_with_sso()
10199
_ = device.api
102100
_ = device.api
103101
_ = device.api

0 commit comments

Comments
 (0)