diff --git a/docs/dev/creating-a-service.md b/docs/dev/creating-a-service.md index 0dda56ac..76b15543 100644 --- a/docs/dev/creating-a-service.md +++ b/docs/dev/creating-a-service.md @@ -256,6 +256,24 @@ REST download job shows it as `input_prompt`. Locally it routes through the shared Rich console (`prompt_user`) so the prompt renders correctly alongside progress and log output. +For a login that the user approves on another device (a pair code, a QR code), +use `self.wait_for_approval(message, check, timeout)`, never a hand-written +loop. It shows the message, then calls `check` at each interval until `check` +returns a value. `check` returns `None` while the approval is pending. + +```python +token = self.wait_for_approval( + f"Open {self.config['endpoints']['pair']} and enter code {code}", + check=lambda: self.poll_pair_token(code), + timeout=expires_in, +) +``` + +It raises `TimeoutError` after `timeout` seconds. Under `serve` mode it also stops +when the client deletes the remote session, so the loop does not continue on the +server with no client. The message goes out as a log line, so a remote +session shows it, but a REST download job does not relay it as `input_prompt`. + !!! warning "Make each token cache entry unique" The cache above uses `tokens_{device}_{profile}` on purpose. A token cache must include every dimension that changes the token (device, profile, or diff --git a/docs/dev/rest-api/endpoints.md b/docs/dev/rest-api/endpoints.md index a4e117c1..1b4f5038 100644 --- a/docs/dev/rest-api/endpoints.md +++ b/docs/dev/rest-api/endpoints.md @@ -957,7 +957,7 @@ The `proxy` value must be a full proxy URI, unless the operator gives your API k } ``` -`server_account` is `true` when the server authenticated with one of its own accounts (`serve.server_accounts`). Otherwise the server uses only what the client sent, and never falls back to its own credentials. Only a client that sent its own cookies, credentials, or cache files gets cookies, auth headers, and the remote session cache back. A client that answered a login prompt (a device code or an OTP) gets them too. This applies only when the login then succeeds and the remote session does not use a server account. For any other remote session (a server account, or an anonymous login through the server's proxy) the tracks and segments responses carry no cookies and no auth headers. The server also drops secret-looking keys from track data, and the cache stays on the server. A server-account service also refuses a client proxy URI, because the server's login would pass through it. +`server_account` is `true` when the server authenticated with one of its own accounts (`serve.server_accounts`). Otherwise the server uses only what the client sent, and never falls back to its own credentials. When the login succeeds and the remote session does not use a server account, the login belongs to the client, and the client gets cookies, auth headers, and the remote session cache back. A remote session on the server's device is the exception: the client gets its cookies back, but no auth headers and no cache. For a server-account remote session the tracks and segments responses carry no cookies and no auth headers. The server also drops secret-looking keys from track data, and the cache stays on the server. A server-account service also refuses a client proxy URI, because the server's login would pass through it. | Status | Error code | Meaning | | --- | --- | --- | diff --git a/docs/dev/rest-api/remote-sessions.md b/docs/dev/rest-api/remote-sessions.md index 6ef54c39..355006f8 100644 --- a/docs/dev/rest-api/remote-sessions.md +++ b/docs/dev/rest-api/remote-sessions.md @@ -213,9 +213,9 @@ hood the client walks a remote session through its lifecycle. deletes the remote session. If the login belongs to the client, the `DELETE` request returns the updated cache files (for example a refreshed token) and the client saves them locally, so the **next** remote session can skip interactive - authentication. The login belongs to the client when the client sent - cookies, credentials, or cache files, or answered a login prompt (a device code - or an OTP) that led to a successful login. A server-account login never does, + authentication. The login belongs to the client when it succeeds and the + remote session does not use a server account. This includes a pair code that + the user approves on another device. A server-account login never does, and neither does a remote session on the server's device (see below). The client saves only the returned files that pass the same profile check it applies before it sends them, so a file for another profile cannot overwrite diff --git a/docs/dev/service-architecture.md b/docs/dev/service-architecture.md index 3e60f10c..f07ac168 100644 --- a/docs/dev/service-architecture.md +++ b/docs/dev/service-architecture.md @@ -89,6 +89,9 @@ These are abstract. A service will not load unless it defines all three: `get_clearkey_license` (returns `None` by default). See [DRM hooks](#drm-hooks) below. - **`request_input(prompt)`**: prompts the user. It goes through the remote-dl input bridge, the REST download job relay, or the rich console. +- **`wait_for_approval(message, check, timeout, interval=3.0)`**: shows a message, then + calls `check` until it returns a value. Use it for a login that the user approves on + another device, such as a pair code. - **Event callbacks**: `on_segment_downloaded`, `on_track_downloaded`, `on_track_decrypted`, `on_track_repacked`, `on_track_multiplex` let a service react to pipeline events (for example, MonaLisa decrypts each segment in `on_segment_downloaded`). diff --git a/tests/core/test_service_wait_for_approval.py b/tests/core/test_service_wait_for_approval.py new file mode 100644 index 00000000..e0df490f --- /dev/null +++ b/tests/core/test_service_wait_for_approval.py @@ -0,0 +1,34 @@ +import logging +from types import SimpleNamespace + +import pytest + +from unshackle.core.api.input_bridge import InputBridge +from unshackle.core.service import Service + + +def _service(bridge=None): + return SimpleNamespace(log=logging.getLogger("test"), _input_bridge=bridge) + + +def test_returns_the_first_value_check_gives(): + answers = iter([None, None, "token"]) + assert Service.wait_for_approval(_service(), "enter code", lambda: next(answers), timeout=5, interval=0) == "token" + + +def test_times_out_when_check_never_gives_a_value(): + with pytest.raises(TimeoutError): + Service.wait_for_approval(_service(), "enter code", lambda: None, timeout=0.05, interval=0.01) + + +def test_stops_when_the_remote_session_is_cancelled(): + bridge = InputBridge() + calls = [] + + def check(): + calls.append(1) + bridge.cancel() + + with pytest.raises(RuntimeError): + Service.wait_for_approval(_service(bridge), "enter code", check, timeout=5, interval=0) + assert len(calls) == 1 diff --git a/tests/remote/unit/test_client_device_cdm.py b/tests/remote/unit/test_client_device_cdm.py index e2edf655..b671c90e 100644 --- a/tests/remote/unit/test_client_device_cdm.py +++ b/tests/remote/unit/test_client_device_cdm.py @@ -101,7 +101,7 @@ def test_the_client_refuses_a_challenge_for_the_other_drm_system() -> None: assert "not a playready device" in answer["error"] -def test_a_cdm_call_after_authentication_leaves_the_status_and_client_auth_alone() -> None: +def test_a_cdm_call_after_authentication_leaves_the_status_alone() -> None: bridge = InputBridge() bridge.status = AuthStatus.AUTHENTICATED result: list[dict] = [] @@ -118,7 +118,6 @@ def test_a_cdm_call_after_authentication_leaves_the_status_and_client_auth_alone assert result == [{"session": "s", "challenge": "Yw=="}] assert bridge.status is AuthStatus.AUTHENTICATED - assert bridge.answered is False assert bridge.get_pending_cdm_call() is None diff --git a/tests/remote/unit/test_input_bridge.py b/tests/remote/unit/test_input_bridge.py index bcbac4fa..198fcae9 100644 --- a/tests/remote/unit/test_input_bridge.py +++ b/tests/remote/unit/test_input_bridge.py @@ -96,19 +96,6 @@ def test_status_setter() -> None: assert bridge.status is AuthStatus.AUTHENTICATED -def test_answered_only_after_an_accepted_response() -> None: - bridge = InputBridge() - assert bridge.submit_response("early") is False - assert bridge.answered is False - worker = threading.Thread(target=bridge.request_input, args=("code?",)) - worker.start() - while bridge.status != AuthStatus.PENDING_INPUT: - time.sleep(0.01) - assert bridge.submit_response("CODE") is True - worker.join(timeout=2) - assert bridge.answered is True - - def test_request_input_after_authentication_fails_fast() -> None: """The client stops polling for prompts once authenticated, so a later prompt must not wait for an answer.""" bridge = InputBridge() diff --git a/tests/test_client_session_auth.py b/tests/test_client_session_auth.py index 0993c506..1b160694 100644 --- a/tests/test_client_session_auth.py +++ b/tests/test_client_session_auth.py @@ -221,11 +221,12 @@ def _run_session(monkeypatch, tmp_path, data, answer, server_account=False, fail @pytest.mark.parametrize( "data, answer, server_account, fail, expected", [ - ({}, None, False, False, False), # anonymous login through the server: stays the server's + ({}, None, False, False, True), # no prompt: a pair code shown in the log, approved on another device ({}, "CODE", False, False, True), # device code the client approved ({}, "CODE", False, True, False), # answered, but the login failed ({}, "CODE", True, False, False), # server account OTP answered by the client ({"cache": "tokens"}, None, False, False, True), # client uploaded its own earlier login + ({}, None, True, False, False), # server account ], ) def test_client_auth_follows_login_provenance(monkeypatch, tmp_path, data, answer, server_account, fail, expected): diff --git a/unshackle/core/api/handlers.py b/unshackle/core/api/handlers.py index c3ac3938..d9584826 100644 --- a/unshackle/core/api/handlers.py +++ b/unshackle/core/api/handlers.py @@ -356,7 +356,7 @@ class RequestCache: Such a server keeps nothing that a client sends, so the request runs on a cache directory of its own that the handler removes when the request ends, on success and on error. Before that, a client that logged in with its own cookies, credentials or cache gets the updated - files back, the same as when a remote session ends. + files back. """ def __init__(self) -> None: @@ -3559,7 +3559,7 @@ async def session_create_handler(data: Dict[str, Any], request: Optional[web.Req async def run_auth() -> None: try: await asyncio.to_thread(service_instance.authenticate, cookies, credential) - if bridge.answered and not server_account: + if not server_account: session.client_auth = True session.auth_status = AuthStatus.AUTHENTICATED bridge.status = AuthStatus.AUTHENTICATED @@ -3647,10 +3647,9 @@ async def session_titles_handler(session_id: str, request: Optional[web.Request] def client_session_auth(session: Any) -> tuple[Dict[str, str], Dict[str, str]]: """The service session headers and cookies a remote client may receive. - Only a client that sent its own cookies or credentials gets them back. Otherwise (a server - account, or an anonymous login through the server's proxy) the cookie jar stays on the - server and the server drops the auth-bearing headers, so the client cannot reuse a login - the server paid for. A remote session on the server's lent device keeps its cookies (the client's account) + A client whose own login succeeded gets them back. For a server account the cookie jar + stays on the server and the server drops the auth-bearing headers, so the client cannot + reuse a login the server paid for. A remote session on the server's lent device keeps its cookies (the client's account) but drops those headers too. """ svc_session = session.service_instance.session diff --git a/unshackle/core/api/input_bridge.py b/unshackle/core/api/input_bridge.py index 186efaf7..1ba334f9 100644 --- a/unshackle/core/api/input_bridge.py +++ b/unshackle/core/api/input_bridge.py @@ -49,7 +49,6 @@ class InputBridge: _response: Optional[str] = field(default=None, init=False, repr=False) _status: AuthStatus = field(default=AuthStatus.AUTHENTICATING, init=False) _cancelled: bool = field(default=False, init=False, repr=False) - _answered: bool = field(default=False, init=False, repr=False) _cdm_call: Optional[dict[str, Any]] = field(default=None, init=False, repr=False) _response_ready: threading.Event = field(default_factory=threading.Event, init=False, repr=False) _picked_up: threading.Event = field(default_factory=threading.Event, init=False, repr=False) @@ -166,8 +165,6 @@ class InputBridge: if not self._awaiting_answer(): return False self._response = response - if self._cdm_call is None: - self._answered = True self._picked_up.set() self._response_ready.set() return True @@ -189,10 +186,10 @@ class InputBridge: self._response_ready.set() @property - def answered(self) -> bool: - """``True`` once the bridge has accepted a response from the client.""" + def cancelled(self) -> bool: + """``True`` once the server cancelled the bridge, so a login loop must stop.""" with self._lock: - return self._answered + return self._cancelled @property def status(self) -> AuthStatus: diff --git a/unshackle/core/api/session_store.py b/unshackle/core/api/session_store.py index 82f1d7f8..abfa865a 100644 --- a/unshackle/core/api/session_store.py +++ b/unshackle/core/api/session_store.py @@ -50,9 +50,7 @@ class SessionEntry: server_cdm_max_height: Optional[int] = None server_vault: bool = False # a session the client's device licenses may still take keys from the server vault server_device: bool = False # its service runs on the server's lent device, so keys come only from a vault - client_auth: bool = ( - False # the client sent its own cookies, credentials, or cache files, or answered a login prompt - ) + client_auth: bool = False # the login is the client's: it succeeded without a server account input_bridge: Optional[InputBridge] = None log_buffer: Optional[Any] = None # SessionLogBuffer mirroring the service's self.log auth_status: AuthStatus = AuthStatus.AUTHENTICATED diff --git a/unshackle/core/service.py b/unshackle/core/service.py index 658eaa8c..8c4d8660 100644 --- a/unshackle/core/service.py +++ b/unshackle/core/service.py @@ -1,10 +1,11 @@ import logging +import time from abc import ABCMeta, abstractmethod from collections.abc import Callable, Generator from dataclasses import dataclass, field from http.cookiejar import CookieJar from pathlib import Path -from typing import TYPE_CHECKING, Any, Optional, Union +from typing import TYPE_CHECKING, Any, Optional, TypeVar, Union if TYPE_CHECKING: from unshackle.core.api.input_bridge import InputBridge @@ -140,6 +141,9 @@ def sanitize_proxy_for_log(uri: Optional[str], mask_host: bool = False) -> Optio return mask_proxy(uri, mask_host=mask_host, allow_debug=False) +T = TypeVar("T") + + class Service(metaclass=ABCMeta): """The Service Base Class. @@ -479,6 +483,27 @@ class Service(metaclass=ABCMeta): return self._input_bridge.request_input(prompt) return prompt_user(prompt) + def wait_for_approval( + self, message: str, check: Callable[[], Optional[T]], timeout: float, interval: float = 3.0 + ) -> T: + """Show ``message``, then call ``check`` until it returns a value. + + Use it for a login that the user approves on another device, such as a pair code. + ``check`` returns None while the approval is pending. Raises TimeoutError after + ``timeout`` seconds, and RuntimeError when the remote session no longer exists. + """ + self.log.info(message) + deadline = time.monotonic() + timeout + while True: + if self._input_bridge is not None and self._input_bridge.cancelled: + raise RuntimeError("Session was cancelled") + result = check() + if result is not None: + return result + if time.monotonic() + interval >= deadline: + raise TimeoutError(f"No approval within {timeout:.0f}s") + time.sleep(interval) + def search(self) -> Generator[SearchResult, None, None]: """ Find titles from the Service by query.