refactor(browser): dedupe auth-flow names and sentinel identity
- Rename the broker's TicketInvalid to ControllerTicketInvalid: the same exception name already exists in hermes_cli/dashboard_auth/ws_tickets.py and BOTH are caught in the same WS auth flow this feature touches — two unrelated same-named exception types in one blast radius invited a wrong except clause. - Import the 'server-internal' sentinel identity from its canonical definition (ws_tickets.INTERNAL_USER_ID/INTERNAL_PROVIDER) instead of re-declaring the strings; drift would have silently broken the internal-peer exclusion in _is_authenticated_identity. Surfaced during review of PR #85351.
This commit is contained in:
@@ -22,7 +22,7 @@ Contract (each rule is exercised by tests/gateway/test_browser_control_broker.py
|
||||
(``secrets``-derived, >= 32 chars) plus an expiry derived from the injected
|
||||
clock; ``consume_ticket`` exchanges it exactly once for the
|
||||
:class:`ControllerScope` it was minted for, raising
|
||||
:class:`TicketInvalid` for unknown, already-consumed, or expired values.
|
||||
:class:`ControllerTicketInvalid` for unknown, already-consumed, or expired values.
|
||||
The ticket is the only cross-transport credential minted here; transports
|
||||
decide how to carry it.
|
||||
|
||||
@@ -207,7 +207,7 @@ class BrowserControlError(Exception):
|
||||
"""Base class for broker contract failures."""
|
||||
|
||||
|
||||
class TicketInvalid(BrowserControlError):
|
||||
class ControllerTicketInvalid(BrowserControlError):
|
||||
"""A registration ticket is unknown, already consumed, or expired."""
|
||||
|
||||
|
||||
@@ -372,7 +372,7 @@ class BrowserControlBroker:
|
||||
def consume_ticket(self, value: str) -> ControllerScope:
|
||||
"""Exchange a ticket for its scope, exactly once.
|
||||
|
||||
Raises :class:`TicketInvalid` for unknown, already-consumed, or
|
||||
Raises :class:`ControllerTicketInvalid` for unknown, already-consumed, or
|
||||
expired tickets. The expiry check happens against the live clock at
|
||||
consume time, so a ticket that outlived its TTL can never be used.
|
||||
"""
|
||||
@@ -380,11 +380,11 @@ class BrowserControlBroker:
|
||||
with self._lock:
|
||||
record = self._tickets.get(value)
|
||||
if record is None:
|
||||
raise TicketInvalid("unknown ticket")
|
||||
raise ControllerTicketInvalid("unknown ticket")
|
||||
if record.consumed:
|
||||
raise TicketInvalid("ticket already consumed")
|
||||
raise ControllerTicketInvalid("ticket already consumed")
|
||||
if now > record.expires_at:
|
||||
raise TicketInvalid("ticket expired")
|
||||
raise ControllerTicketInvalid("ticket expired")
|
||||
record.consumed = True
|
||||
return record.scope
|
||||
|
||||
|
||||
@@ -138,7 +138,7 @@ from gateway.browser_control_broker import (
|
||||
BROWSER_CONTROL_CAPABILITIES,
|
||||
BROWSER_CONTROL_DEVELOPER_CAPABILITIES,
|
||||
ControllerScope,
|
||||
TicketInvalid,
|
||||
ControllerTicketInvalid,
|
||||
browser_control_developer_mode,
|
||||
browser_control_protocol_supported,
|
||||
filter_browser_control_capabilities,
|
||||
@@ -3591,7 +3591,7 @@ class APIServerAdapter(BasePlatformAdapter):
|
||||
raise web.HTTPUnauthorized()
|
||||
try:
|
||||
scope = self._browser_control_broker.consume_ticket(ticket_value)
|
||||
except TicketInvalid:
|
||||
except ControllerTicketInvalid:
|
||||
raise web.HTTPUnauthorized() from None
|
||||
except Exception:
|
||||
logger.exception("browser-control WS ticket consumption failed")
|
||||
|
||||
@@ -7,7 +7,7 @@ from gateway.browser_control_broker import (
|
||||
BrowserControlBroker,
|
||||
ControllerCancelled,
|
||||
ControllerScope,
|
||||
TicketInvalid,
|
||||
ControllerTicketInvalid,
|
||||
)
|
||||
|
||||
|
||||
@@ -34,12 +34,12 @@ def test_registration_ticket_is_short_lived_single_use_and_identity_bound():
|
||||
assert len(ticket.value) >= 32
|
||||
assert ticket.expires_at == 130.0
|
||||
assert broker.consume_ticket(ticket.value) == scope
|
||||
with pytest.raises(TicketInvalid, match="unknown|consumed"):
|
||||
with pytest.raises(ControllerTicketInvalid, match="unknown|consumed"):
|
||||
broker.consume_ticket(ticket.value)
|
||||
|
||||
expired = broker.mint_ticket(scope)
|
||||
now[0] = 131.0
|
||||
with pytest.raises(TicketInvalid, match="expired"):
|
||||
with pytest.raises(ControllerTicketInvalid, match="expired"):
|
||||
broker.consume_ticket(expired.value)
|
||||
|
||||
|
||||
|
||||
@@ -41,6 +41,10 @@ from gateway.browser_control_broker import (
|
||||
browser_control_protocol_supported,
|
||||
filter_browser_control_capabilities,
|
||||
)
|
||||
from hermes_cli.dashboard_auth.ws_tickets import (
|
||||
INTERNAL_PROVIDER as _INTERNAL_PROVIDER,
|
||||
INTERNAL_USER_ID as _INTERNAL_USER_ID,
|
||||
)
|
||||
|
||||
from .method_ctx import HandlerRegistry
|
||||
|
||||
@@ -57,12 +61,6 @@ _CLOUD_TRANSPORT_FAMILY = "cloud-ticket-ws"
|
||||
#: JSON-RPC error code for identity / session / flag denials (forbidden).
|
||||
_ERR_FORBIDDEN = 4403
|
||||
|
||||
#: Identity recorded for server-spawned WS clients (see
|
||||
#: ``hermes_cli.dashboard_auth.ws_tickets``) — never allowed to act as a
|
||||
#: browser controller.
|
||||
_INTERNAL_USER_ID = "server-internal"
|
||||
_INTERNAL_PROVIDER = "server-internal"
|
||||
|
||||
|
||||
def _is_authenticated_identity(identity: object) -> bool:
|
||||
"""True for a server-minted, non-internal ``{user_id, provider}`` identity."""
|
||||
|
||||
Reference in New Issue
Block a user