From 718e45438bcfd19d9e2a27f50a4c984ba343297f Mon Sep 17 00:00:00 2001 From: abrichr Date: Tue, 28 Jul 2026 02:24:48 -0400 Subject: [PATCH 1/2] feat: warn before hosted credentials expire --- README.md | 17 +- src/openadapt_tray/__init__.py | 11 +- src/openadapt_tray/app.py | 7 + src/openadapt_tray/hosted.py | 343 ++++++++++++++++++++++++++++++++- src/openadapt_tray/menu.py | 29 ++- src/openadapt_tray/state.py | 44 +++++ tests/test_hosted.py | 233 +++++++++++++++++++++- tests/test_menu.py | 29 ++- tests/test_state.py | 17 ++ 9 files changed, 714 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 4e54ee4..b8bf408 100644 --- a/README.md +++ b/README.md @@ -106,12 +106,15 @@ Recent Captures # only when count > 0 Open Desktop App Open Cloud Dashboard +Account: connected · days left Pause Sync / Sync (offline) -Login... Settings... Quit ``` +The account row changes to a sign-in action, an expiry warning, or an +unavailable status. Selecting it opens the credential settings page. + During a local operation, the recording item changes to Starting, Stop Recording, Stopping, or Compiling. These labels reflect events; the tray does not perform the work. @@ -137,12 +140,24 @@ The poller calls: ```text GET /api/needs-attention/count Authorization: Bearer +→ { "count": 0, "credential": { + "expires_at": "2026-08-05T12:00:00Z", + "expires_in_days": 8, + "expiring_soon": true, + "legacy_non_expiring": false, + "warning_days": 14 + } } ``` The token is resolved from `OPENADAPT_INGEST_TOKEN` or the OS keychain and is not written to `tray.json`. The default poll interval is 60 seconds, clamped to at least 30 seconds, with a slower offline retry. +The control plane decides when the credential enters its 14-day warning +window. The tray shows one actionable notification for each credential and +expiry, including after a tray restart. It stores only a non-secret identity +digest for notification deduplication. It never stores or logs the token. + This is a narrow status endpoint, not hosted execution. The tray does not upload screenshots, workflow bundles, or capture artifacts through this poller. diff --git a/src/openadapt_tray/__init__.py b/src/openadapt_tray/__init__.py index 83f4cc6..648e295 100644 --- a/src/openadapt_tray/__init__.py +++ b/src/openadapt_tray/__init__.py @@ -5,11 +5,20 @@ from openadapt_tray.app import TrayApplication, main from openadapt_tray.config import TrayConfig from openadapt_tray.hosted import CountResult, HostedPoller -from openadapt_tray.state import AppState, StateManager, SyncState, TrayState +from openadapt_tray.state import ( + AppState, + CredentialState, + CredentialStatus, + StateManager, + SyncState, + TrayState, +) __all__ = [ "AppState", "CountResult", + "CredentialState", + "CredentialStatus", "HostedPoller", "StateManager", "SyncState", diff --git a/src/openadapt_tray/app.py b/src/openadapt_tray/app.py index a4308b2..244d851 100644 --- a/src/openadapt_tray/app.py +++ b/src/openadapt_tray/app.py @@ -34,6 +34,7 @@ LANE_BYOC, LANE_CLOUD, AppState, + CredentialStatus, StateManager, SyncState, TrayState, @@ -85,6 +86,8 @@ def __init__(self, config: TrayConfig | None = None): on_count=self._on_hosted_count, notifier=self.notifications, on_break_clicked=self.open_needs_attention, + on_credential=self._on_hosted_credential, + on_credential_clicked=self.login, set_offline=self._on_hosted_offline, ) @@ -458,6 +461,10 @@ def _on_hosted_count(self, result: CountResult) -> None: """Apply a needs-attention count from the cloud poller.""" self.state.set_break_count(result.count) + def _on_hosted_credential(self, credential: CredentialStatus) -> None: + """Apply the privacy-safe credential status from the cloud poller.""" + self.state.set_credential_status(credential) + def _on_hosted_offline(self, offline: bool) -> None: """Reflect the cloud poller's online/offline status on the sync channel.""" if offline: diff --git a/src/openadapt_tray/hosted.py b/src/openadapt_tray/hosted.py index c34ea55..b8b9973 100644 --- a/src/openadapt_tray/hosted.py +++ b/src/openadapt_tray/hosted.py @@ -15,10 +15,16 @@ * byoc lane → IPC ``open_teach`` to the desktop (the fix stays local) """ +import hashlib +import json +import os +import re import threading import webbrowser -from collections.abc import Callable -from dataclasses import dataclass +from collections.abc import Callable, Mapping +from dataclasses import dataclass, field +from datetime import datetime, timezone +from pathlib import Path import httpx @@ -27,9 +33,21 @@ OFFLINE_POLL_INTERVAL_S, TrayConfig, ) +from openadapt_tray.state import CredentialState, CredentialStatus COUNT_ENDPOINT_PATH = "/api/needs-attention/count" REQUEST_TIMEOUT_S = 10.0 +CREDENTIAL_WARNING_DAYS = 14 +CREDENTIAL_CONTRACT_KEYS = { + "expires_at", + "expires_in_days", + "expiring_soon", + "legacy_non_expiring", + "warning_days", +} +CREDENTIAL_WARNING_STATE_VERSION = 1 +MAX_DELIVERED_CREDENTIAL_WARNINGS = 32 +INGEST_TOKEN_PATTERN = re.compile(r"^oai_ingest_[A-Za-z0-9_-]{43}$") class InvalidCountPayload(ValueError): @@ -41,6 +59,193 @@ class InvalidCountPayload(ValueError): """ +class InvalidCredentialPayload(ValueError): + """The additive credential block does not match the closed contract.""" + + +class CredentialWarningStateError(RuntimeError): + """The local warning-deduplication state could not be read or written.""" + + +def _is_json_int(value: object) -> bool: + """Return whether a value is a JSON integer rather than a boolean.""" + return isinstance(value, int) and not isinstance(value, bool) + + +def _parse_utc_expiry(value: object) -> str: + """Validate and return an ISO-8601 UTC expiry string.""" + if not isinstance(value, str) or not value: + raise InvalidCredentialPayload("expires_at must be a non-empty string") + try: + parsed = datetime.fromisoformat(value.replace("Z", "+00:00")) + except ValueError as e: + raise InvalidCredentialPayload("expires_at must be ISO-8601") from e + if parsed.tzinfo is None or parsed.utcoffset() != timezone.utc.utcoffset(parsed): + raise InvalidCredentialPayload("expires_at must use UTC") + return value + + +def _header_values(headers: Mapping[str, str] | None) -> dict[str, str] | None: + """Return case-insensitive response headers for contract validation.""" + if headers is None: + return None + return {str(key).lower(): str(value) for key, value in headers.items()} + + +def is_valid_ingest_token(token: object) -> bool: + """Return whether a value matches the closed hosted bearer format.""" + return isinstance(token, str) and INGEST_TOKEN_PATTERN.fullmatch(token) is not None + + +def parse_credential_status( + value: object, + *, + headers: Mapping[str, str] | None = None, +) -> CredentialStatus: + """Parse the control plane's closed credential-status contract. + + The server's ``expiring_soon`` value is authoritative. The tray validates + it, but never recomputes it from the local clock or rounded day count. + """ + if not isinstance(value, dict): + raise InvalidCredentialPayload("credential must be an object") + if set(value) != CREDENTIAL_CONTRACT_KEYS: + raise InvalidCredentialPayload("credential has unexpected or missing fields") + + warning_days = value["warning_days"] + if not _is_json_int(warning_days) or warning_days != CREDENTIAL_WARNING_DAYS: + raise InvalidCredentialPayload("warning_days does not match the contract") + expiring_soon = value["expiring_soon"] + legacy = value["legacy_non_expiring"] + if not isinstance(expiring_soon, bool) or not isinstance(legacy, bool): + raise InvalidCredentialPayload("credential flags must be booleans") + + normalized_headers = _header_values(headers) + if normalized_headers is not None: + if normalized_headers.get("cache-control") != "no-store": + raise InvalidCredentialPayload("cache-control header does not match") + if normalized_headers.get( + "x-openadapt-credential-warning-days" + ) != str(CREDENTIAL_WARNING_DAYS): + raise InvalidCredentialPayload("warning-days header does not match") + + if legacy: + if value["expires_at"] is not None or value["expires_in_days"] is not None: + raise InvalidCredentialPayload("legacy credential must not carry expiry") + if expiring_soon: + raise InvalidCredentialPayload("legacy credential cannot be expiring soon") + if normalized_headers is not None and ( + "x-openadapt-credential-expires-in-days" in normalized_headers + ): + raise InvalidCredentialPayload("legacy credential must omit expiry header") + return CredentialStatus( + state=CredentialState.LEGACY, + warning_days=CREDENTIAL_WARNING_DAYS, + ) + + expires_at = _parse_utc_expiry(value["expires_at"]) + expires_in_days = value["expires_in_days"] + if not _is_json_int(expires_in_days) or expires_in_days < 0: + raise InvalidCredentialPayload("expires_in_days must be a non-negative integer") + if expires_in_days < CREDENTIAL_WARNING_DAYS and not expiring_soon: + raise InvalidCredentialPayload("expiry status contradicts the day count") + if expires_in_days > CREDENTIAL_WARNING_DAYS and expiring_soon: + raise InvalidCredentialPayload("expiry status contradicts the day count") + if normalized_headers is not None and normalized_headers.get( + "x-openadapt-credential-expires-in-days" + ) != str(expires_in_days): + raise InvalidCredentialPayload("expiry-days header does not match") + + return CredentialStatus( + state=(CredentialState.EXPIRING if expiring_soon else CredentialState.ACTIVE), + expires_at=expires_at, + expires_in_days=expires_in_days, + warning_days=CREDENTIAL_WARNING_DAYS, + ) + + +def credential_identity(hosted_url: str, token: str) -> str: + """Return a non-secret identity digest without storing or logging a token.""" + material = f"{hosted_url.rstrip('/')}\0{token}".encode() + return hashlib.sha256(material).hexdigest() + + +def credential_warning_key(identity: str, credential: CredentialStatus) -> str: + """Bind a delivered warning to the credential identity and expiry version.""" + material = json.dumps( + { + "contract": CREDENTIAL_WARNING_STATE_VERSION, + "credential": identity, + "expires_at": credential.expires_at, + "warning_days": credential.warning_days, + }, + sort_keys=True, + separators=(",", ":"), + ).encode() + return hashlib.sha256(material).hexdigest() + + +class CredentialWarningStore: + """Small persistent store for delivered credential-warning identities.""" + + def __init__(self, path: Path | None = None): + self.path = path or TrayConfig.config_path().with_name( + "credential-warning-state.json" + ) + + def _read(self) -> list[str]: + if not self.path.exists(): + return [] + try: + payload = json.loads(self.path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError) as e: + raise CredentialWarningStateError("warning state is unreadable") from e + if ( + not isinstance(payload, dict) + or set(payload) != {"version", "delivered"} + or payload["version"] != CREDENTIAL_WARNING_STATE_VERSION + or not isinstance(payload["delivered"], list) + or any(not isinstance(item, str) for item in payload["delivered"]) + ): + raise CredentialWarningStateError("warning state has an invalid schema") + return payload["delivered"][-MAX_DELIVERED_CREDENTIAL_WARNINGS:] + + def was_delivered(self, warning_key: str) -> bool: + """Return whether the exact credential warning was delivered.""" + return warning_key in self._read() + + def mark_delivered(self, warning_key: str) -> None: + """Persist one delivered warning without storing the credential.""" + try: + delivered = self._read() + except CredentialWarningStateError: + # The failed read remains visible to the caller of ``was_delivered``. + # A confirmed new delivery can safely replace this non-secret cache. + delivered = [] + delivered = [item for item in delivered if item != warning_key] + delivered.append(warning_key) + delivered = delivered[-MAX_DELIVERED_CREDENTIAL_WARNINGS:] + payload = { + "version": CREDENTIAL_WARNING_STATE_VERSION, + "delivered": delivered, + } + temporary = self.path.with_name(f".{self.path.name}.tmp") + try: + self.path.parent.mkdir(parents=True, exist_ok=True) + temporary.write_text( + json.dumps(payload, sort_keys=True, separators=(",", ":")), + encoding="utf-8", + ) + os.chmod(temporary, 0o600) + os.replace(temporary, self.path) + except OSError as e: + try: + temporary.unlink(missing_ok=True) + except OSError: + pass + raise CredentialWarningStateError("warning state could not be saved") from e + + def _optional_int(payload: dict, key: str) -> int: """Read an optional integer subfield. @@ -62,9 +267,18 @@ class CountResult: count: int halts: int = 0 uncertain_dispatches: int = 0 + credential: CredentialStatus = field(default_factory=CredentialStatus.unknown) + credential_error: str | None = field(default=None, repr=False, compare=False) + credential_identity: str | None = field(default=None, repr=False, compare=False) @classmethod - def from_payload(cls, payload: object) -> "CountResult": + def from_payload( + cls, + payload: object, + *, + headers: Mapping[str, str] | None = None, + credential_id: str | None = None, + ) -> "CountResult": """Build a result from the JSON body. ``count`` is the safety-critical number: it drives the badge and the @@ -101,10 +315,24 @@ def from_payload(cls, payload: object) -> "CountResult": raise InvalidCountPayload(f"'count' is not an integer: {count!r}") if count < 0: raise InvalidCountPayload(f"'count' is negative: {count}") + credential = CredentialStatus.unknown() + credential_error = None + if "credential" not in payload: + credential_error = "credential block is absent" + else: + try: + credential = parse_credential_status( + payload["credential"], headers=headers + ) + except InvalidCredentialPayload as e: + credential_error = str(e) return cls( count=count, halts=_optional_int(payload, "halts"), uncertain_dispatches=_optional_int(payload, "uncertain_dispatches"), + credential=credential, + credential_error=credential_error, + credential_identity=credential_id, ) @@ -128,8 +356,11 @@ def __init__( on_count: Callable[[CountResult], None], notifier: object | None = None, on_break_clicked: Callable[[], None] | None = None, + on_credential: Callable[[CredentialStatus], None] | None = None, + on_credential_clicked: Callable[[], None] | None = None, token_provider: Callable[[], str | None] | None = None, set_offline: Callable[[bool], None] | None = None, + warning_store: CredentialWarningStore | None = None, ): """Initialize the poller. @@ -141,19 +372,28 @@ def __init__( method (a :class:`NotificationManager`). on_break_clicked: Optional lane-aware click handler for the notification. Defaults to :func:`route_break_click` behaviour. + on_credential: Receives each validated credential status, including + the fail-visible unknown and signed-out states. + on_credential_clicked: Opens the hosted credential renewal page. token_provider: Returns the current bearer token. Defaults to ``config.get_ingest_token``. set_offline: Optional callback invoked with the offline boolean each cycle (used to drive the tray sync channel). + warning_store: Persistent delivered-warning store. Tests can supply + an isolated store. """ self.config = config self._on_count = on_count self._notifier = notifier self._on_break_clicked = on_break_clicked + self._on_credential = on_credential + self._on_credential_clicked = on_credential_clicked self._token_provider = token_provider or config.get_ingest_token self._set_offline = set_offline + self._warning_store = warning_store or CredentialWarningStore() self._last_count = 0 + self._last_had_token = False self._current_interval = config.effective_poll_interval_s() self._thread: threading.Thread | None = None self._stop = threading.Event() @@ -169,8 +409,13 @@ def poll_once(self) -> CountResult | None: """ token = self._token_provider() if not token: + self._last_had_token = False # Not logged in yet — treat as offline for badge purposes. return None + self._last_had_token = True + if not is_valid_ingest_token(token): + print("needs-attention poll refused an invalid credential format") + return None url = count_url(self.config.hosted_url) headers = {"Authorization": f"Bearer {token}"} @@ -181,7 +426,18 @@ def poll_once(self) -> CountResult | None: print(f"needs-attention count returned {resp.status_code}") return None try: - return CountResult.from_payload(resp.json()) + identity = credential_identity(self.config.hosted_url, token) + result = CountResult.from_payload( + resp.json(), + headers=getattr(resp, "headers", None), + credential_id=identity, + ) + if result.credential_error: + print( + "needs-attention credential status unusable: " + f"{result.credential_error}" + ) + return result except InvalidCountPayload as e: # A body we cannot read is not a count of zero. Report it as a # failed poll so the badge keeps its last known value instead of @@ -202,6 +458,13 @@ def _handle_result(self, result: CountResult | None) -> None: self._current_interval = OFFLINE_POLL_INTERVAL_S if self._set_offline: self._set_offline(True) + if self._on_credential: + credential = ( + CredentialStatus.unknown() + if self._last_had_token + else CredentialStatus.signed_out() + ) + self._on_credential(credential) return # Online: restore configured interval (respecting the floor). @@ -213,6 +476,10 @@ def _handle_result(self, result: CountResult | None) -> None: # Drive the badge/state. self._on_count(result) + if self._on_credential: + self._on_credential(result.credential) + + self._maybe_warn_credential(result) # Notify only when the count RISES (0→N or N→N+1), never on a decrease. if result.count > self._last_count and result.count > 0: @@ -225,6 +492,74 @@ def _handle_result(self, result: CountResult | None) -> None: else: self._last_count = result.count + def _maybe_warn_credential(self, result: CountResult) -> None: + """Show one persistent warning for an expiring credential version.""" + credential = result.credential + if credential.state != CredentialState.EXPIRING: + return + if not result.credential_identity or credential.expires_at is None: + print("credential warning not sent: credential identity is unavailable") + return + + warning_key = credential_warning_key( + result.credential_identity, + credential, + ) + try: + if self._warning_store.was_delivered(warning_key): + return + except CredentialWarningStateError as e: + print(f"credential warning state failure: {e}") + + if not self._fire_credential_notification(credential): + return + try: + self._warning_store.mark_delivered(warning_key) + except CredentialWarningStateError as e: + # Do not mark a warning as durable when the local record failed. + # The next poll retries the user-visible warning. + print(f"credential warning state failure: {e}") + + def _fire_credential_notification(self, credential: CredentialStatus) -> bool: + """Show a PHI-free actionable warning for an expiring credential.""" + if not self._notifier: + return False + days = credential.expires_in_days + if days is None or credential.expires_at is None: + return False + deadline = datetime.fromisoformat( + credential.expires_at.replace("Z", "+00:00") + ).strftime("%Y-%m-%d") + day_text = "day" if days == 1 else "days" + try: + delivered = self._notifier.show( + "OpenAdapt sign-in expires soon", + f"Your local connection expires in {days} {day_text} " + f"({deadline}). Renew it now.", + urgency="critical", + on_clicked=self._on_credential_clicked + or self._default_credential_click, + ) + except Exception as e: + print(f"Failed to show credential warning: {e}") + return False + if not delivered: + print("credential expiry warning was not delivered; retrying later") + return False + return True + + def _default_credential_click(self) -> None: + """Open the account credential settings page.""" + try: + opened = webbrowser.open( + f"{self.config.hosted_url.rstrip('/')}/dashboard/settings/ingest" + ) + except Exception as e: + print(f"Could not open credential settings: {e}") + return + if not opened: + print("Could not open credential settings: no usable browser") + def _fire_notification(self, count: int) -> bool: """Fire the 'N automations need attention' notification. diff --git a/src/openadapt_tray/menu.py b/src/openadapt_tray/menu.py index 34defc7..26bb7ac 100644 --- a/src/openadapt_tray/menu.py +++ b/src/openadapt_tray/menu.py @@ -14,7 +14,7 @@ from openadapt_tray.app import TrayApplication from openadapt_tray.platform.base import DialogUnavailableError -from openadapt_tray.state import TrayState +from openadapt_tray.state import CredentialState, TrayState @dataclass @@ -60,8 +60,8 @@ def build(self) -> Menu: Menu.SEPARATOR, Item("Open Desktop App", self._open_desktop_app), Item("Open Cloud Dashboard", self._open_cloud_dashboard), + self._build_account_item(state), self._build_sync_item(state), - Item("Login...", self._login), Item("Settings...", self._open_settings), Menu.SEPARATOR, Item("Quit", self._quit), @@ -87,6 +87,31 @@ def _build_break_item(self, state) -> Item | None: lambda: self.app.open_needs_attention(), ) + def _build_account_item(self, state) -> Item: + """Build the hosted account status and renewal action.""" + credential = state.credential + if credential.state == CredentialState.NOT_CHECKED: + return Item("Account: checking status...", None, enabled=False) + if credential.state == CredentialState.SIGNED_OUT: + return Item("Account: sign in", self._login) + if credential.state == CredentialState.UNKNOWN: + return Item("Account: status unavailable", self._login) + if credential.state == CredentialState.LEGACY: + return Item("Account: connected · renew credential", self._login) + if credential.state == CredentialState.EXPIRING: + days = credential.expires_in_days + if days == 0: + label = "Account: sign-in expires today" + else: + noun = "day" if days == 1 else "days" + label = f"Account: sign-in expires in {days} {noun}" + return Item(label, self._login) + days = credential.expires_in_days + if days is None: + return Item("Account: connected", self._login) + noun = "day" if days == 1 else "days" + return Item(f"Account: connected · {days} {noun} left", self._login) + def _build_sync_item(self, state) -> Item: """Build the pause/resume-sync toggle. diff --git a/src/openadapt_tray/state.py b/src/openadapt_tray/state.py index 6516b63..e77caf1 100644 --- a/src/openadapt_tray/state.py +++ b/src/openadapt_tray/state.py @@ -43,6 +43,37 @@ def PUSHING(cls) -> "SyncState": return cls.SYNCING +class CredentialState(Enum): + """Status of the hosted credential reported by the control plane.""" + + NOT_CHECKED = auto() + ACTIVE = auto() + EXPIRING = auto() + LEGACY = auto() + SIGNED_OUT = auto() + UNKNOWN = auto() + + +@dataclass(frozen=True) +class CredentialStatus: + """Privacy-safe credential status for the tray UI.""" + + state: CredentialState = CredentialState.NOT_CHECKED + expires_at: str | None = None + expires_in_days: int | None = None + warning_days: int | None = None + + @classmethod + def signed_out(cls) -> "CredentialStatus": + """Return the status used when no hosted credential is available.""" + return cls(state=CredentialState.SIGNED_OUT) + + @classmethod + def unknown(cls) -> "CredentialStatus": + """Return the fail-visible status for unreadable or unreachable data.""" + return cls(state=CredentialState.UNKNOWN) + + # Deployment lanes (drives break-click routing; learned from desktop/config). LANE_CLOUD = "cloud" LANE_BYOC = "byoc" @@ -63,6 +94,9 @@ class AppState: # Break / needs-attention badge (sourced from the cloud count endpoint). break_count: int = 0 + # Hosted credential state. This never contains the credential value. + credential: CredentialStatus = CredentialStatus() + # Deployment lane — drives lane-aware break-click routing. deployment_lane: str = LANE_CLOUD @@ -160,6 +194,7 @@ def transition(self, new_state: TrayState, **kwargs) -> None: # Preserve the orthogonal channels unless explicitly overridden. kwargs.setdefault("sync_state", self._state.sync_state) kwargs.setdefault("break_count", self._state.break_count) + kwargs.setdefault("credential", self._state.credential) kwargs.setdefault("deployment_lane", self._state.deployment_lane) self._state = AppState(state=new_state, **kwargs) @@ -184,6 +219,15 @@ def set_break_count(self, count: int) -> None: self._state = replace(self._state, break_count=count) self._emit() + def set_credential_status(self, credential: CredentialStatus) -> None: + """Update only the hosted credential status.""" + if self._state.credential == credential: + return + from dataclasses import replace + + self._state = replace(self._state, credential=credential) + self._emit() + def set_deployment_lane(self, lane: str) -> None: """Update ONLY the deployment lane (cloud|byoc).""" if self._state.deployment_lane == lane: diff --git a/tests/test_hosted.py b/tests/test_hosted.py index f79b94e..75c77ca 100644 --- a/tests/test_hosted.py +++ b/tests/test_hosted.py @@ -11,11 +11,18 @@ ) from openadapt_tray.hosted import ( CountResult, + CredentialWarningStore, HostedPoller, InvalidCountPayload, + InvalidCredentialPayload, count_url, + credential_identity, + parse_credential_status, route_break_click, ) +from openadapt_tray.state import CredentialState + +VALID_INGEST_TOKEN = "oai_ingest_" + ("A" * 43) def make_config(**kw): @@ -24,9 +31,10 @@ def make_config(**kw): class _FakeResponse: - def __init__(self, status_code=200, payload=None): + def __init__(self, status_code=200, payload=None, headers=None): self.status_code = status_code self._payload = payload or {} + self.headers = headers or {} def json(self): return self._payload @@ -83,7 +91,7 @@ class TestPollOnce: def test_poll_once_success(self): cfg = make_config() poller = HostedPoller( - cfg, on_count=lambda r: None, token_provider=lambda: "oai_ingest_x" + cfg, on_count=lambda r: None, token_provider=lambda: VALID_INGEST_TOKEN ) fake = _FakeClient( _FakeResponse(200, {"count": 4, "halts": 3, "uncertain_dispatches": 1}) @@ -97,7 +105,80 @@ def test_poll_once_success(self): assert result.uncertain_dispatches == 1 # Verify the exact request contract (endpoint + bearer auth). assert fake.last_url == "https://example.test/api/needs-attention/count" - assert fake.last_headers["Authorization"] == "Bearer oai_ingest_x" + assert fake.last_headers["Authorization"] == f"Bearer {VALID_INGEST_TOKEN}" + + def test_poll_once_parses_closed_credential_contract(self): + cfg = make_config() + poller = HostedPoller( + cfg, on_count=lambda r: None, token_provider=lambda: VALID_INGEST_TOKEN + ) + payload = { + "count": 0, + "credential": { + "expires_at": "2026-08-05T12:00:00Z", + "expires_in_days": 8, + "expiring_soon": True, + "legacy_non_expiring": False, + "warning_days": 14, + }, + } + fake = _FakeClient( + _FakeResponse( + 200, + payload, + { + "Cache-Control": "no-store", + "X-OpenAdapt-Credential-Warning-Days": "14", + "X-OpenAdapt-Credential-Expires-In-Days": "8", + }, + ) + ) + with patch("openadapt_tray.hosted.httpx.Client", return_value=fake): + result = poller.poll_once() + + assert result is not None + assert result.credential.state == CredentialState.EXPIRING + assert result.credential_identity == credential_identity( + cfg.hosted_url, VALID_INGEST_TOKEN + ) + + def test_missing_no_store_header_keeps_valid_attention_count(self): + result = CountResult.from_payload( + { + "count": 4, + "credential": { + "expires_at": "2026-08-05T12:00:00Z", + "expires_in_days": 8, + "expiring_soon": True, + "legacy_non_expiring": False, + "warning_days": 14, + }, + }, + headers={ + "X-OpenAdapt-Credential-Warning-Days": "14", + "X-OpenAdapt-Credential-Expires-In-Days": "8", + }, + ) + + assert result.count == 4 + assert result.credential.state == CredentialState.UNKNOWN + assert result.credential_error is not None + + @pytest.mark.parametrize( + ("days", "expiring"), + [(13, False), (15, True)], + ) + def test_impossible_expiry_combinations_are_rejected(self, days, expiring): + with pytest.raises(InvalidCredentialPayload): + parse_credential_status( + { + "expires_at": "2026-08-05T12:00:00Z", + "expires_in_days": days, + "expiring_soon": expiring, + "legacy_non_expiring": False, + "warning_days": 14, + } + ) def test_poll_once_no_token_returns_none(self): cfg = make_config() @@ -112,7 +193,7 @@ def test_poll_once_no_token_returns_none(self): def test_poll_once_non_200_returns_none(self): cfg = make_config() poller = HostedPoller( - cfg, on_count=lambda r: None, token_provider=lambda: "t" + cfg, on_count=lambda r: None, token_provider=lambda: VALID_INGEST_TOKEN ) fake = _FakeClient(_FakeResponse(401, {})) with patch("openadapt_tray.hosted.httpx.Client", return_value=fake): @@ -121,12 +202,23 @@ def test_poll_once_non_200_returns_none(self): def test_poll_once_network_error_returns_none(self): cfg = make_config() poller = HostedPoller( - cfg, on_count=lambda r: None, token_provider=lambda: "t" + cfg, on_count=lambda r: None, token_provider=lambda: VALID_INGEST_TOKEN ) fake = _FakeClient(raise_exc=OSError("no route to host")) with patch("openadapt_tray.hosted.httpx.Client", return_value=fake): assert poller.poll_once() is None + def test_poll_once_rejects_wrong_token_type_before_http(self): + poller = HostedPoller( + make_config(), + on_count=lambda r: None, + token_provider=lambda: "oap_pairing_secret", + ) + + with patch("openadapt_tray.hosted.httpx.Client") as mock_client: + assert poller.poll_once() is None + mock_client.assert_not_called() + class TestHandleResult: """Tests for badge updates, notifications, and back-off.""" @@ -220,6 +312,20 @@ def test_interval_never_below_floor(self): poller._handle_result(CountResult(count=0)) assert poller.current_interval == MIN_POLL_INTERVAL_S + def test_unreachable_status_is_unknown_not_healthy(self): + statuses = [] + poller = HostedPoller( + make_config(), + on_count=lambda r: None, + on_credential=statuses.append, + token_provider=lambda: "t", + ) + poller._last_had_token = True + + poller._handle_result(None) + + assert statuses[-1].state == CredentialState.UNKNOWN + class TestRouteBreakClick: """Tests for lane-aware click routing (spec §3c).""" @@ -308,7 +414,7 @@ def test_poll_once_reports_failure_not_zero(self): """The poller must return None, never ``CountResult(count=0)``.""" cfg = make_config() poller = HostedPoller( - cfg, on_count=lambda r: None, token_provider=lambda: "t" + cfg, on_count=lambda r: None, token_provider=lambda: VALID_INGEST_TOKEN ) fake = _FakeClient(_FakeResponse(200, {"total": 7})) # no "count" with patch("openadapt_tray.hosted.httpx.Client", return_value=fake): @@ -320,7 +426,7 @@ def test_unreadable_body_never_clears_the_badge(self): counts = [] poller = HostedPoller( cfg, on_count=lambda r: counts.append(r.count), - token_provider=lambda: "t", + token_provider=lambda: VALID_INGEST_TOKEN, ) good = _FakeClient(_FakeResponse(200, {"count": 4})) with patch("openadapt_tray.hosted.httpx.Client", return_value=good): @@ -393,6 +499,119 @@ def test_no_notifier_is_not_a_delivery(self): assert poller._fire_notification(1) is False +class TestCredentialExpiryWarning: + def _credential( + self, + *, + expiring_soon=True, + expires_in_days=8, + expires_at="2026-08-05T12:00:00Z", + ): + return parse_credential_status( + { + "expires_at": expires_at, + "expires_in_days": expires_in_days, + "expiring_soon": expiring_soon, + "legacy_non_expiring": False, + "warning_days": 14, + } + ) + + def test_server_decision_controls_warning_at_day_fourteen(self, tmp_path): + notifier = MagicMock() + notifier.show.return_value = True + poller = HostedPoller( + make_config(), + on_count=lambda r: None, + notifier=notifier, + token_provider=lambda: "t", + warning_store=CredentialWarningStore(tmp_path / "warnings.json"), + ) + poller._handle_result( + CountResult( + count=0, + credential=self._credential( + expiring_soon=False, expires_in_days=14 + ), + credential_identity="credential-a", + ) + ) + + notifier.show.assert_not_called() + + def test_delivered_warning_survives_poller_restart(self, tmp_path): + path = tmp_path / "warnings.json" + notifier = MagicMock() + notifier.show.return_value = True + raw_token = "oai_ingest_secret-value" + result = CountResult( + count=0, + credential=self._credential(), + credential_identity=credential_identity( + "https://example.test", raw_token + ), + ) + + first = HostedPoller( + make_config(), + on_count=lambda r: None, + notifier=notifier, + token_provider=lambda: "t", + warning_store=CredentialWarningStore(path), + ) + first._handle_result(result) + second = HostedPoller( + make_config(), + on_count=lambda r: None, + notifier=notifier, + token_provider=lambda: "t", + warning_store=CredentialWarningStore(path), + ) + second._handle_result(result) + + notifier.show.assert_called_once() + assert raw_token not in path.read_text() + + def test_new_identity_or_expiry_can_warn_again(self, tmp_path): + notifier = MagicMock() + notifier.show.return_value = True + store = CredentialWarningStore(tmp_path / "warnings.json") + poller = HostedPoller( + make_config(), + on_count=lambda r: None, + notifier=notifier, + token_provider=lambda: "t", + warning_store=store, + ) + credential = self._credential() + + poller._handle_result( + CountResult( + count=0, + credential=credential, + credential_identity="credential-a", + ) + ) + poller._handle_result( + CountResult( + count=0, + credential=credential, + credential_identity="credential-b", + ) + ) + poller._handle_result( + CountResult( + count=0, + credential=self._credential( + expires_at="2026-08-06T12:00:00Z" + ), + credential_identity="credential-b", + ) + ) + + assert notifier.show.call_count == 3 + + class TestBreakClickReportsFailure: """A click that opened nothing must not report itself as routed.""" diff --git a/tests/test_menu.py b/tests/test_menu.py index 8ed5fc0..ae95b16 100644 --- a/tests/test_menu.py +++ b/tests/test_menu.py @@ -7,7 +7,12 @@ from openadapt_tray.menu import CaptureInfo, MenuBuilder from openadapt_tray.platform.base import DialogUnavailableError -from openadapt_tray.state import AppState, TrayState +from openadapt_tray.state import ( + AppState, + CredentialState, + CredentialStatus, + TrayState, +) class TestCaptureInfo: @@ -128,6 +133,28 @@ def test_sync_item_pause_when_online(self): item = builder._build_sync_item(app.state.current) assert "Pause Sync" in str(item.text) + @pytest.mark.parametrize( + ("credential", "label"), + [ + (CredentialStatus.signed_out(), "sign in"), + (CredentialStatus.unknown(), "status unavailable"), + ( + CredentialStatus( + state=CredentialState.EXPIRING, + expires_at="2026-08-05T12:00:00Z", + expires_in_days=8, + warning_days=14, + ), + "expires in 8 days", + ), + ], + ) + def test_account_item_exposes_actionable_status(self, credential, label): + app = self.create_mock_app(AppState(credential=credential)) + item = MenuBuilder(app)._build_account_item(app.state.current) + + assert label in str(item.text).lower() + def test_get_recent_captures_empty(self): """Test get_recent_captures returns empty list when no captures.""" app = self.create_mock_app() diff --git a/tests/test_state.py b/tests/test_state.py index 4351a66..9145c26 100644 --- a/tests/test_state.py +++ b/tests/test_state.py @@ -5,6 +5,8 @@ LANE_BYOC, LANE_CLOUD, AppState, + CredentialState, + CredentialStatus, StateManager, SyncState, TrayState, @@ -230,6 +232,21 @@ def test_set_break_count_clamps_and_notifies(self): assert manager.current.break_count == 0 assert received == [3, 0] + def test_credential_update_preserves_attention_count(self): + manager = StateManager() + manager.set_break_count(3) + manager.set_credential_status( + CredentialStatus( + state=CredentialState.EXPIRING, + expires_at="2026-08-05T12:00:00Z", + expires_in_days=8, + warning_days=14, + ) + ) + + assert manager.current.break_count == 3 + assert manager.current.credential.state == CredentialState.EXPIRING + def test_reset_preserves_sync_channel(self): """reset() returns recording to IDLE but keeps the sync channel.""" manager = StateManager() From 9c970ce5105885ed82a3b7a8f20eb40d80bcf3c5 Mon Sep 17 00:00:00 2001 From: abrichr Date: Tue, 28 Jul 2026 02:28:01 -0400 Subject: [PATCH 2/2] fix: derive credential identity with HMAC --- src/openadapt_tray/hosted.py | 10 ++++++++-- tests/test_hosted.py | 10 ++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/openadapt_tray/hosted.py b/src/openadapt_tray/hosted.py index b8b9973..10abda6 100644 --- a/src/openadapt_tray/hosted.py +++ b/src/openadapt_tray/hosted.py @@ -16,6 +16,7 @@ """ import hashlib +import hmac import json import os import re @@ -48,6 +49,7 @@ CREDENTIAL_WARNING_STATE_VERSION = 1 MAX_DELIVERED_CREDENTIAL_WARNINGS = 32 INGEST_TOKEN_PATTERN = re.compile(r"^oai_ingest_[A-Za-z0-9_-]{43}$") +CREDENTIAL_IDENTITY_DOMAIN = b"openadapt-tray/credential-identity/v1" class InvalidCountPayload(ValueError): @@ -166,8 +168,12 @@ def parse_credential_status( def credential_identity(hosted_url: str, token: str) -> str: """Return a non-secret identity digest without storing or logging a token.""" - material = f"{hosted_url.rstrip('/')}\0{token}".encode() - return hashlib.sha256(material).hexdigest() + message = CREDENTIAL_IDENTITY_DOMAIN + b"\0" + hosted_url.rstrip("/").encode() + return hmac.new( + token.encode(), + message, + hashlib.sha256, + ).hexdigest() def credential_warning_key(identity: str, credential: CredentialStatus) -> str: diff --git a/tests/test_hosted.py b/tests/test_hosted.py index 75c77ca..2f030a3 100644 --- a/tests/test_hosted.py +++ b/tests/test_hosted.py @@ -84,6 +84,16 @@ def test_count_url(self): == "https://example.test/api/needs-attention/count" ) + def test_credential_identity_is_stable_and_host_bound(self): + first = credential_identity("https://example.test", VALID_INGEST_TOKEN) + + assert first == credential_identity( + "https://example.test/", VALID_INGEST_TOKEN + ) + assert first != credential_identity( + "https://other.test", VALID_INGEST_TOKEN + ) + class TestPollOnce: """Tests for the single authenticated request."""