diff --git a/README.md b/README.md index efad2267..773f52f4 100644 --- a/README.md +++ b/README.md @@ -75,6 +75,22 @@ client = ThingsboardClient( ) ``` +**No authentication:** All auth arguments are optional. Omit them to get a client that +sends no `X-Authorization` header, for use with the `/api/noauth` endpoints. + +```python +client = ThingsboardClient("http://localhost:9090") +``` + +The three authenticated modes are mutually exclusive — passing more than one raises +`ValueError`, as does passing `username=` without `password=` or vice versa, or +`refresh_token=` without `token=`. `token=` on its own is valid; it simply means the +token is never refreshed. + +An empty string is also rejected, so `api_key=os.environ.get("TB_API_KEY", "")` raises +rather than building a client that silently sends no credentials. Omit the auth +arguments entirely for an unauthenticated client. + ## Resource cleanup Use the client as a context manager so `close()` is called automatically on exit: diff --git a/ce/docs/tb-examples.md b/ce/docs/tb-examples.md index 0af4f168..c1dafa05 100644 --- a/ce/docs/tb-examples.md +++ b/ce/docs/tb-examples.md @@ -23,6 +23,40 @@ from tb_ce_client import ThingsboardClient client = ThingsboardClient("http://localhost:9090", api_key="your-api-key") ``` +## Pre-existing Token + +Injects an externally obtained JWT; no login call is made. + +```python +from tb_ce_client import ThingsboardClient + +client = ThingsboardClient( + "http://localhost:9090", + token="eyJhbGciOi...", + refresh_token="eyJhbGciOi...", +) +``` + +## No Authentication + +All auth arguments are optional. Omit them for a client that sends no +`X-Authorization` header, for use with the `/api/noauth` endpoints. + +```python +from tb_ce_client import ThingsboardClient + +client = ThingsboardClient("http://localhost:9090") +``` + +The three authenticated modes above are mutually exclusive — passing more than one +raises `ValueError`, as does passing `username=` without `password=` or vice versa, or +`refresh_token=` without `token=`. `token=` on its own is valid; it simply means the +token is never refreshed. + +An empty string is also rejected, so `api_key=os.environ.get("TB_API_KEY", "")` raises +rather than building a client that silently sends no credentials. Omit the auth +arguments entirely for an unauthenticated client. + ## Context Manager ```python diff --git a/ce/tb_ce_client/_auth.py b/ce/tb_ce_client/_auth.py index 1c793d11..eb193221 100644 --- a/ce/tb_ce_client/_auth.py +++ b/ce/tb_ce_client/_auth.py @@ -34,6 +34,60 @@ # Matches Java's AuthManager.AVG_REQUEST_TIMEOUT (30 seconds in ms) AVG_REQUEST_TIMEOUT_MS = 30_000 +# Wall-clock ceiling for a single raw auth call. urllib3 defaults to no timeout at all, +# and every API thread now blocks behind an in-flight refresh, so an unresponsive auth +# endpoint would otherwise hang the whole process rather than one thread. +# +# Applied as Timeout(total=...) rather than a bare float, which would set connect and +# read separately and leave total unbounded. Because _AUTH_RETRIES makes exactly one +# request — no retry, no redirect — total is the per-call ceiling; urllib3 gives each +# attempt its own budget, so any allowance there would multiply this number. +# +# A blocked thread can wait twice this: _do_refresh_token falls back to _do_login, which +# is a second call. That is the worst case behind one refresh, and the fallback is what +# recovers an expired refresh token, so the 2x is deliberate rather than a leak. +DEFAULT_AUTH_TIMEOUT_MS = 30_000 + +# Retry policy for the raw auth calls, spelled out rather than left to urllib3, so that +# exactly one request goes out and DEFAULT_AUTH_TIMEOUT_MS means what it says. +# +# No retries: Retry.DEFAULT is total=3, and its connection-error branch never consults +# allowed_methods, so this POST would retry and cost 4x the ceiling above. Auth POSTs are +# not idempotent, and the timeout exists precisely to bound how long every other thread +# sits blocked, so a single attempt is the deliberate trade — a caller wanting tolerance +# should retry ThingsboardClient(...) itself, since urllib3 has no global deadline that +# would let us have both. +# +# No redirects either, which is a departure from the generated RESTClientObject: +# - urllib3 clones the timeout per hop instead of drawing down a shared budget, so +# following N redirects costs (1 + N) x auth_timeout_ms on the one round-trip every +# other API thread is blocked behind. +# - the body is re-sent to whatever Location names, with no same-origin restriction, +# so a redirect out of the configured server hands username/password to a third host +# and _do_login would install the token it returns. +# The case this gives up is a deployment that redirects auth (a proxy forcing https, +# path normalisation). Failing loudly is the better answer there: on an http -> https +# redirect the credentials have already gone out in cleartext, so the fix is to point url= +# at the redirect target's base URL, which _raw_post's error tells the caller to do. +# +# total is set explicitly: it defaults to 10, and leaving it there would contradict the +# "exactly one request" this whole block is for, even though the per-class zeros already +# exhaust first. +# +# Spelled redirect=False rather than redirect=0, which is not the same thing: Retry +# normalises False to 0 *and* clears raise_on_redirect, and that is what lets a 3xx come +# back as a response for the status check below to report. With redirect=0 the same reply +# raises MaxRetryError("too many redirects") instead and the remedy never reaches the +# caller. Both spellings leave .redirect == 0, so only raise_on_redirect tells them apart. +_AUTH_RETRIES = urllib3.Retry(total=0, connect=0, read=0, status=0, other=0, redirect=False) + +# Security scheme name and prefixes dictated by the generated configuration.py. +# Keep them in one place so a spec regeneration that renames the scheme has a +# single owner instead of literals scattered across client.py and _auth.py. +_SECURITY_SCHEME = "ApiKeyForm" +_JWT_PREFIX = "Bearer" +_API_KEY_PREFIX = "ApiKey" + # --------------------------------------------------------------------------- # _TokenInfo @@ -51,8 +105,8 @@ class _TokenInfo: def __init__( self, - token, - refresh_token, + token: "str | None", + refresh_token: "str | None", token_exp_ts: int, refresh_exp_ts: int, clock_diff: int, @@ -101,27 +155,46 @@ class _AuthManager: Mirrors the Java ThingsboardClient.java inner AuthManager class. - Thread safety: a threading.Lock protects the _refreshing flag so that only - one concurrent API thread triggers a refresh call. Other threads wait at the - lock and skip the refresh once the first thread completes. + Thread safety: a threading.Condition guards the _refreshing flag so that only + one concurrent API thread performs a refresh. The others block until that + refresh finishes and then use its result — they must not proceed meanwhile, + since the token they would send is the expired one being replaced. + + The auth mode is decided once in __init__ and never re-derived per request. """ - def __init__(self, base_url: str, auth_type: str, api_key=None): + def __init__( + self, + base_url: str, + api_key: "str | None" = None, + auth_timeout_ms: int = DEFAULT_AUTH_TIMEOUT_MS, + ): """ Args: - base_url: ThingsBoard server URL (e.g. "http://tb-server:9090"). - Trailing slashes are stripped. - auth_type: Either 'jwt' (username/password or token) or 'api_key'. - api_key: The API key string when auth_type='api_key', else None. + base_url: ThingsBoard server URL (e.g. "http://tb-server:9090"). + Trailing slashes are stripped. + api_key: The API key string for API key auth, or None for JWT auth + (username/password or an externally supplied token). + auth_timeout_ms: Ceiling for a single /api/auth call, in milliseconds. + Bounds how long every other thread can be blocked behind a + refresh, so it is a knob a slow on-prem server or a + latency-sensitive caller will want to change. """ + # urllib3.Timeout rejects a non-positive total, but it would only raise inside + # _raw_post — where _do_refresh_token and _do_login catch Exception and log — + # so a bad value would construct fine and then silently never refresh. + if auth_timeout_ms <= 0: + raise ValueError(f"auth_timeout_ms must be positive; got {auth_timeout_ms}") + self._auth_timeout_s = auth_timeout_ms / 1000 self._base_url = base_url.rstrip("/") - self._auth_type = auth_type - self._lock = threading.Lock() + self._is_api_key = api_key is not None + self._header_prefix = _API_KEY_PREFIX if self._is_api_key else _JWT_PREFIX + self._refresh_state = threading.Condition() self._refreshing = False self._username = None self._password = None - if auth_type == "api_key": + if self._is_api_key: self._token_info = _TokenInfo(api_key, None, -1, -1, 0) else: self._token_info = _TokenInfo.EMPTY # type: ignore[attr-defined] @@ -136,18 +209,37 @@ def on_login(self, username: str, password: str, token: str, refresh_token: str) self._password = password self._token_info = self._build_token_info(token, refresh_token) - def set_external_token(self, token: str, refresh_token=None) -> None: - """Set a pre-existing token without storing login credentials.""" - self._token_info = self._build_token_info(token, refresh_token or "") + def set_external_token(self, token: str, refresh_token: "str | None" = None) -> None: + """Set a pre-existing token without storing login credentials. + + refresh_token is passed through as-is so that omitting it leaves + get_refresh_token() returning None, as its docstring promises. + """ + self._token_info = self._build_token_info(token, refresh_token) - def get_token(self): + def get_token(self) -> "str | None": """Return the current access token, or None if not yet set.""" return self._token_info.token - def get_refresh_token(self): + def get_refresh_token(self) -> "str | None": """Return the current refresh token, or None if not available.""" return self._token_info.refresh_token + def install_header(self, configuration) -> None: + """Write the current token into configuration's X-Authorization slots. + + Configuration.auth_settings() emits the header only when the security + scheme is already present in configuration.api_key, so the slot has to be + seeded at construction time before the hook can ever take over. + """ + token = self._token_info.token + if not token: + # No auth configured (e.g. /api/noauth usage) — leave the slot absent + # so auth_settings() emits no header at all. + return + configuration.api_key[_SECURITY_SCHEME] = token + configuration.api_key_prefix[_SECURITY_SCHEME] = self._header_prefix + def hook(self, configuration) -> None: """refresh_api_key_hook implementation. @@ -155,14 +247,11 @@ def hook(self, configuration) -> None: every API request assembles its X-Authorization header. Checks token expiry and refreshes if needed, then updates configuration.api_key. """ - if self._auth_type != "jwt": + if self._is_api_key: # API key auth — hook is a no-op; the key is set at construction time return self._refresh_if_needed() - token = self._token_info.token - if token: - configuration.api_key["ApiKeyForm"] = token - configuration.api_key_prefix["ApiKeyForm"] = "Bearer" + self.install_header(configuration) # ------------------------------------------------------------------ # Internal refresh logic @@ -171,9 +260,14 @@ def hook(self, configuration) -> None: def _refresh_if_needed(self) -> None: """Check token expiry and trigger refresh if the estimated server time exceeds the token expiry (with AVG_REQUEST_TIMEOUT buffer).""" - with self._lock: + with self._refresh_state: if self._refreshing: - # Another thread is already refreshing — skip + # Another thread is already refreshing. Block rather than return: + # returning here would send the expired token that thread is busy + # replacing, and nothing retries the resulting 401. + self._refresh_state.wait_for(lambda: not self._refreshing) + # Its outcome is ours. Refreshing again on failure would multiply one + # failed round-trip by however many threads were waiting. return info = self._token_info if info.token is None or info.token_exp_ts < 0: @@ -196,8 +290,9 @@ def _refresh_if_needed(self) -> None: elif self._username: self._do_login() finally: - with self._lock: + with self._refresh_state: self._refreshing = False + self._refresh_state.notify_all() def _do_refresh_token(self, info: "_TokenInfo") -> None: """POST to /api/auth/token with the refresh token. Falls back to login on error.""" @@ -226,6 +321,10 @@ def _raw_post(self, path: str, body: bytes) -> dict: if we used ApiClient here, the hook would fire again while already inside the hook, causing infinite recursion (mirrors Java's pattern of using a separate raw HttpClient for AuthManager calls). + + Bounded by auth_timeout_ms and _AUTH_RETRIES: this call is on the critical path + for every thread waiting on a refresh, so it makes exactly one request — no + retry, no redirect. See _AUTH_RETRIES for why both trades are deliberate. """ http = urllib3.PoolManager() response = http.request( @@ -233,12 +332,25 @@ def _raw_post(self, path: str, body: bytes) -> dict: self._base_url + path, body=body, headers={"Content-Type": "application/json"}, + timeout=urllib3.Timeout(total=self._auth_timeout_s), + retries=_AUTH_RETRIES, ) if response.status != 200: - raise RuntimeError(f"Auth request to {path} returned HTTP {response.status}") + # A redirect arrives here rather than being followed — say so, since the + # remedy is specific and not guessable from the status alone. Gated on + # get_redirect_location() rather than the 3xx range because that is the + # predicate urllib3 itself would have followed: it covers 301/302/303/307/308 + # and excludes 300 and 304, which carry no Location worth chasing. + hint = ( + "; auth requests do not follow redirects, so set url= to the redirect " + "target's base URL instead." + if response.get_redirect_location() + else "" + ) + raise RuntimeError(f"Auth request to {path} returned HTTP {response.status}{hint}") return json.loads(response.data) - def _build_token_info(self, token: str, refresh_token: str) -> "_TokenInfo": + def _build_token_info(self, token: str, refresh_token: "str | None") -> "_TokenInfo": """Parse JWT claims from token and refresh_token; compute clock_diff.""" now_ms = int(time.time() * 1000) token_exp = _parse_jwt_claim_ms(token, "exp") diff --git a/ce/tb_ce_client/client.py b/ce/tb_ce_client/client.py index 27502cc0..5947faa4 100644 --- a/ce/tb_ce_client/client.py +++ b/ce/tb_ce_client/client.py @@ -27,7 +27,7 @@ import importlib -from ._auth import _AuthManager +from ._auth import DEFAULT_AUTH_TIMEOUT_MS, _AuthManager from ._controller_map import _CONTROLLER_ATTR_MAP, _CONTROLLER_MAP from ._retry import _RetryingRESTClient from .api_client import ApiClient @@ -35,11 +35,74 @@ from .models.login_request import LoginRequest +def _validate_auth_args( + username: "str | None", + password: "str | None", + api_key: "str | None", + token: "str | None", + refresh_token: "str | None", +) -> None: + """Reject auth argument combinations that cannot be honoured. + + Raises ValueError describing the offending arguments; returns None otherwise. + """ + # An empty string passes every "is not None" check below but installs no header, + # so the client would silently send no credentials at all — the failure mode this + # validation exists to prevent. Easy to reach via os.environ.get("TB_API_KEY", ""). + # Checked before the rules below so an empty value reports itself rather than the + # companion-argument error it would also trip. + for name, value in ( + ("username", username), + ("password", password), + ("api_key", api_key), + ("token", token), + ("refresh_token", refresh_token), + ): + if value is not None and not value: + raise ValueError( + f"{name}= must not be empty; pass a value, or omit all auth " + "arguments for an unauthenticated client." + ) + + # The three auth modes share a single X-Authorization slot, so combining them is + # ambiguous: whichever ran last would win, and under api_key auth the refresh hook + # is a no-op, so a JWT installed alongside a key would be frozen at its initial + # value and never refreshed. + modes = [ + f"{name}=" + for name, value in (("username", username), ("api_key", api_key), ("token", token)) + if value is not None + ] + if len(modes) > 1: + raise ValueError( + "ThingsboardClient authentication modes are mutually exclusive; got " + f"{', '.join(modes)}. Pass exactly one of username=, api_key= or token=." + ) + + # password= is only read by the username branch, so on its own it would be silently + # dropped and surface later as a 401. username= alone is rejected here rather than + # left to LoginRequest, whose password is a required StrictStr — otherwise the + # caller gets a pydantic ValidationError from inside the generated model. + if password is not None and username is None: + raise ValueError( + "password= requires username=; pass both, or omit both for an unauthenticated client." + ) + if username is not None and password is None: + raise ValueError( + "username= requires password=; pass both, or omit both for an unauthenticated client." + ) + # refresh_token= is likewise read only by the token branch. + if refresh_token is not None and token is None: + raise ValueError( + "refresh_token= requires token=; pass both, or omit both for an unauthenticated client." + ) + + class ThingsboardClient: """User-facing ThingsBoard client. Wraps the generated per-controller APIs with authentication management and - transparent 429 retry. Supports three authentication modes: + transparent 429 retry. Supports three authentication modes, plus unauthenticated: 1. Username + password (JWT): ThingsboardClient(url, username, password) @@ -52,6 +115,11 @@ class ThingsboardClient: 3. Pre-existing token: ThingsboardClient(url, token="jwt", refresh_token="jwt") Injects an externally obtained JWT; no login call made. + refresh_token is optional — omit it for a token that is never refreshed. + + The three modes are mutually exclusive — passing more than one raises ValueError. + All auth arguments are optional: omitting them yields an unauthenticated client + that sends no X-Authorization header, which is what the /api/noauth endpoints want. Context manager usage: with ThingsboardClient(url, api_key="key") as client: @@ -61,15 +129,16 @@ class ThingsboardClient: def __init__( self, url: str, - username: str = None, - password: str = None, - api_key: str = None, - token: str = None, - refresh_token: str = None, + username: "str | None" = None, + password: "str | None" = None, + api_key: "str | None" = None, + token: "str | None" = None, + refresh_token: "str | None" = None, max_retries: int = 3, initial_retry_delay_ms: int = 1_000, max_retry_delay_ms: int = 30_000, retry_on_rate_limit: bool = True, + auth_timeout_ms: int = DEFAULT_AUTH_TIMEOUT_MS, ): """Construct ThingsboardClient and authenticate. @@ -79,31 +148,41 @@ def __init__( password: Password for JWT authentication. api_key: API key for X-Authorization: ApiKey authentication. token: Pre-existing JWT access token. - refresh_token: Pre-existing JWT refresh token (used with token=). + refresh_token: Pre-existing JWT refresh token (used with token=). Omit it + to install a token that is never refreshed. max_retries: Maximum retry attempts on HTTP 429 (default 3). initial_retry_delay_ms: Base backoff delay in milliseconds (default 1000). max_retry_delay_ms: Maximum backoff cap in milliseconds (default 30000). retry_on_rate_limit: If True (default), wraps rest_client with _RetryingRESTClient. If False, uses plain RESTClientObject. + auth_timeout_ms: Ceiling for a single /api/auth call (default 30000). + Bounds how long other threads block behind a token refresh. + + Raises: + ValueError: If more than one of username=, api_key= or token= is given; + if password= is given without username= or vice versa; if + refresh_token= is given without token=; if any auth argument is + an empty string; or if auth_timeout_ms is not positive. """ # Must be the very first assignment — prevents __getattr__ infinite recursion # if __init__ raises partway through (before self.api_client is set). self._controllers: dict = {} + _validate_auth_args( + username=username, + password=password, + api_key=api_key, + token=token, + refresh_token=refresh_token, + ) + configuration = Configuration(host=url) - # Determine auth type - auth_type = "api_key" if api_key is not None else "jwt" - auth_manager = _AuthManager(url, auth_type, api_key) + auth_manager = _AuthManager(url, api_key, auth_timeout_ms) # Install the refresh hook so the hook fires before every API request configuration.refresh_api_key_hook = auth_manager.hook - # API key auth: set header at construction time - if api_key is not None: - configuration.api_key["ApiKeyForm"] = api_key - configuration.api_key_prefix["ApiKeyForm"] = "ApiKey" - # Build the ApiClient api_client = ApiClient(configuration=configuration) @@ -130,8 +209,9 @@ def __init__( # Pre-existing token if token is not None: auth_manager.set_external_token(token, refresh_token) - configuration.api_key["ApiKeyForm"] = token - configuration.api_key_prefix["ApiKeyForm"] = "Bearer" + + # Seed the header slot for whichever mode ran — see _AuthManager.install_header. + auth_manager.install_header(configuration) # ------------------------------------------------------------------ # Controller delegation diff --git a/common/_auth.py b/common/_auth.py index 1c793d11..eb193221 100644 --- a/common/_auth.py +++ b/common/_auth.py @@ -34,6 +34,60 @@ # Matches Java's AuthManager.AVG_REQUEST_TIMEOUT (30 seconds in ms) AVG_REQUEST_TIMEOUT_MS = 30_000 +# Wall-clock ceiling for a single raw auth call. urllib3 defaults to no timeout at all, +# and every API thread now blocks behind an in-flight refresh, so an unresponsive auth +# endpoint would otherwise hang the whole process rather than one thread. +# +# Applied as Timeout(total=...) rather than a bare float, which would set connect and +# read separately and leave total unbounded. Because _AUTH_RETRIES makes exactly one +# request — no retry, no redirect — total is the per-call ceiling; urllib3 gives each +# attempt its own budget, so any allowance there would multiply this number. +# +# A blocked thread can wait twice this: _do_refresh_token falls back to _do_login, which +# is a second call. That is the worst case behind one refresh, and the fallback is what +# recovers an expired refresh token, so the 2x is deliberate rather than a leak. +DEFAULT_AUTH_TIMEOUT_MS = 30_000 + +# Retry policy for the raw auth calls, spelled out rather than left to urllib3, so that +# exactly one request goes out and DEFAULT_AUTH_TIMEOUT_MS means what it says. +# +# No retries: Retry.DEFAULT is total=3, and its connection-error branch never consults +# allowed_methods, so this POST would retry and cost 4x the ceiling above. Auth POSTs are +# not idempotent, and the timeout exists precisely to bound how long every other thread +# sits blocked, so a single attempt is the deliberate trade — a caller wanting tolerance +# should retry ThingsboardClient(...) itself, since urllib3 has no global deadline that +# would let us have both. +# +# No redirects either, which is a departure from the generated RESTClientObject: +# - urllib3 clones the timeout per hop instead of drawing down a shared budget, so +# following N redirects costs (1 + N) x auth_timeout_ms on the one round-trip every +# other API thread is blocked behind. +# - the body is re-sent to whatever Location names, with no same-origin restriction, +# so a redirect out of the configured server hands username/password to a third host +# and _do_login would install the token it returns. +# The case this gives up is a deployment that redirects auth (a proxy forcing https, +# path normalisation). Failing loudly is the better answer there: on an http -> https +# redirect the credentials have already gone out in cleartext, so the fix is to point url= +# at the redirect target's base URL, which _raw_post's error tells the caller to do. +# +# total is set explicitly: it defaults to 10, and leaving it there would contradict the +# "exactly one request" this whole block is for, even though the per-class zeros already +# exhaust first. +# +# Spelled redirect=False rather than redirect=0, which is not the same thing: Retry +# normalises False to 0 *and* clears raise_on_redirect, and that is what lets a 3xx come +# back as a response for the status check below to report. With redirect=0 the same reply +# raises MaxRetryError("too many redirects") instead and the remedy never reaches the +# caller. Both spellings leave .redirect == 0, so only raise_on_redirect tells them apart. +_AUTH_RETRIES = urllib3.Retry(total=0, connect=0, read=0, status=0, other=0, redirect=False) + +# Security scheme name and prefixes dictated by the generated configuration.py. +# Keep them in one place so a spec regeneration that renames the scheme has a +# single owner instead of literals scattered across client.py and _auth.py. +_SECURITY_SCHEME = "ApiKeyForm" +_JWT_PREFIX = "Bearer" +_API_KEY_PREFIX = "ApiKey" + # --------------------------------------------------------------------------- # _TokenInfo @@ -51,8 +105,8 @@ class _TokenInfo: def __init__( self, - token, - refresh_token, + token: "str | None", + refresh_token: "str | None", token_exp_ts: int, refresh_exp_ts: int, clock_diff: int, @@ -101,27 +155,46 @@ class _AuthManager: Mirrors the Java ThingsboardClient.java inner AuthManager class. - Thread safety: a threading.Lock protects the _refreshing flag so that only - one concurrent API thread triggers a refresh call. Other threads wait at the - lock and skip the refresh once the first thread completes. + Thread safety: a threading.Condition guards the _refreshing flag so that only + one concurrent API thread performs a refresh. The others block until that + refresh finishes and then use its result — they must not proceed meanwhile, + since the token they would send is the expired one being replaced. + + The auth mode is decided once in __init__ and never re-derived per request. """ - def __init__(self, base_url: str, auth_type: str, api_key=None): + def __init__( + self, + base_url: str, + api_key: "str | None" = None, + auth_timeout_ms: int = DEFAULT_AUTH_TIMEOUT_MS, + ): """ Args: - base_url: ThingsBoard server URL (e.g. "http://tb-server:9090"). - Trailing slashes are stripped. - auth_type: Either 'jwt' (username/password or token) or 'api_key'. - api_key: The API key string when auth_type='api_key', else None. + base_url: ThingsBoard server URL (e.g. "http://tb-server:9090"). + Trailing slashes are stripped. + api_key: The API key string for API key auth, or None for JWT auth + (username/password or an externally supplied token). + auth_timeout_ms: Ceiling for a single /api/auth call, in milliseconds. + Bounds how long every other thread can be blocked behind a + refresh, so it is a knob a slow on-prem server or a + latency-sensitive caller will want to change. """ + # urllib3.Timeout rejects a non-positive total, but it would only raise inside + # _raw_post — where _do_refresh_token and _do_login catch Exception and log — + # so a bad value would construct fine and then silently never refresh. + if auth_timeout_ms <= 0: + raise ValueError(f"auth_timeout_ms must be positive; got {auth_timeout_ms}") + self._auth_timeout_s = auth_timeout_ms / 1000 self._base_url = base_url.rstrip("/") - self._auth_type = auth_type - self._lock = threading.Lock() + self._is_api_key = api_key is not None + self._header_prefix = _API_KEY_PREFIX if self._is_api_key else _JWT_PREFIX + self._refresh_state = threading.Condition() self._refreshing = False self._username = None self._password = None - if auth_type == "api_key": + if self._is_api_key: self._token_info = _TokenInfo(api_key, None, -1, -1, 0) else: self._token_info = _TokenInfo.EMPTY # type: ignore[attr-defined] @@ -136,18 +209,37 @@ def on_login(self, username: str, password: str, token: str, refresh_token: str) self._password = password self._token_info = self._build_token_info(token, refresh_token) - def set_external_token(self, token: str, refresh_token=None) -> None: - """Set a pre-existing token without storing login credentials.""" - self._token_info = self._build_token_info(token, refresh_token or "") + def set_external_token(self, token: str, refresh_token: "str | None" = None) -> None: + """Set a pre-existing token without storing login credentials. + + refresh_token is passed through as-is so that omitting it leaves + get_refresh_token() returning None, as its docstring promises. + """ + self._token_info = self._build_token_info(token, refresh_token) - def get_token(self): + def get_token(self) -> "str | None": """Return the current access token, or None if not yet set.""" return self._token_info.token - def get_refresh_token(self): + def get_refresh_token(self) -> "str | None": """Return the current refresh token, or None if not available.""" return self._token_info.refresh_token + def install_header(self, configuration) -> None: + """Write the current token into configuration's X-Authorization slots. + + Configuration.auth_settings() emits the header only when the security + scheme is already present in configuration.api_key, so the slot has to be + seeded at construction time before the hook can ever take over. + """ + token = self._token_info.token + if not token: + # No auth configured (e.g. /api/noauth usage) — leave the slot absent + # so auth_settings() emits no header at all. + return + configuration.api_key[_SECURITY_SCHEME] = token + configuration.api_key_prefix[_SECURITY_SCHEME] = self._header_prefix + def hook(self, configuration) -> None: """refresh_api_key_hook implementation. @@ -155,14 +247,11 @@ def hook(self, configuration) -> None: every API request assembles its X-Authorization header. Checks token expiry and refreshes if needed, then updates configuration.api_key. """ - if self._auth_type != "jwt": + if self._is_api_key: # API key auth — hook is a no-op; the key is set at construction time return self._refresh_if_needed() - token = self._token_info.token - if token: - configuration.api_key["ApiKeyForm"] = token - configuration.api_key_prefix["ApiKeyForm"] = "Bearer" + self.install_header(configuration) # ------------------------------------------------------------------ # Internal refresh logic @@ -171,9 +260,14 @@ def hook(self, configuration) -> None: def _refresh_if_needed(self) -> None: """Check token expiry and trigger refresh if the estimated server time exceeds the token expiry (with AVG_REQUEST_TIMEOUT buffer).""" - with self._lock: + with self._refresh_state: if self._refreshing: - # Another thread is already refreshing — skip + # Another thread is already refreshing. Block rather than return: + # returning here would send the expired token that thread is busy + # replacing, and nothing retries the resulting 401. + self._refresh_state.wait_for(lambda: not self._refreshing) + # Its outcome is ours. Refreshing again on failure would multiply one + # failed round-trip by however many threads were waiting. return info = self._token_info if info.token is None or info.token_exp_ts < 0: @@ -196,8 +290,9 @@ def _refresh_if_needed(self) -> None: elif self._username: self._do_login() finally: - with self._lock: + with self._refresh_state: self._refreshing = False + self._refresh_state.notify_all() def _do_refresh_token(self, info: "_TokenInfo") -> None: """POST to /api/auth/token with the refresh token. Falls back to login on error.""" @@ -226,6 +321,10 @@ def _raw_post(self, path: str, body: bytes) -> dict: if we used ApiClient here, the hook would fire again while already inside the hook, causing infinite recursion (mirrors Java's pattern of using a separate raw HttpClient for AuthManager calls). + + Bounded by auth_timeout_ms and _AUTH_RETRIES: this call is on the critical path + for every thread waiting on a refresh, so it makes exactly one request — no + retry, no redirect. See _AUTH_RETRIES for why both trades are deliberate. """ http = urllib3.PoolManager() response = http.request( @@ -233,12 +332,25 @@ def _raw_post(self, path: str, body: bytes) -> dict: self._base_url + path, body=body, headers={"Content-Type": "application/json"}, + timeout=urllib3.Timeout(total=self._auth_timeout_s), + retries=_AUTH_RETRIES, ) if response.status != 200: - raise RuntimeError(f"Auth request to {path} returned HTTP {response.status}") + # A redirect arrives here rather than being followed — say so, since the + # remedy is specific and not guessable from the status alone. Gated on + # get_redirect_location() rather than the 3xx range because that is the + # predicate urllib3 itself would have followed: it covers 301/302/303/307/308 + # and excludes 300 and 304, which carry no Location worth chasing. + hint = ( + "; auth requests do not follow redirects, so set url= to the redirect " + "target's base URL instead." + if response.get_redirect_location() + else "" + ) + raise RuntimeError(f"Auth request to {path} returned HTTP {response.status}{hint}") return json.loads(response.data) - def _build_token_info(self, token: str, refresh_token: str) -> "_TokenInfo": + def _build_token_info(self, token: str, refresh_token: "str | None") -> "_TokenInfo": """Parse JWT claims from token and refresh_token; compute clock_diff.""" now_ms = int(time.time() * 1000) token_exp = _parse_jwt_claim_ms(token, "exp") diff --git a/common/client.py b/common/client.py index 27502cc0..5947faa4 100644 --- a/common/client.py +++ b/common/client.py @@ -27,7 +27,7 @@ import importlib -from ._auth import _AuthManager +from ._auth import DEFAULT_AUTH_TIMEOUT_MS, _AuthManager from ._controller_map import _CONTROLLER_ATTR_MAP, _CONTROLLER_MAP from ._retry import _RetryingRESTClient from .api_client import ApiClient @@ -35,11 +35,74 @@ from .models.login_request import LoginRequest +def _validate_auth_args( + username: "str | None", + password: "str | None", + api_key: "str | None", + token: "str | None", + refresh_token: "str | None", +) -> None: + """Reject auth argument combinations that cannot be honoured. + + Raises ValueError describing the offending arguments; returns None otherwise. + """ + # An empty string passes every "is not None" check below but installs no header, + # so the client would silently send no credentials at all — the failure mode this + # validation exists to prevent. Easy to reach via os.environ.get("TB_API_KEY", ""). + # Checked before the rules below so an empty value reports itself rather than the + # companion-argument error it would also trip. + for name, value in ( + ("username", username), + ("password", password), + ("api_key", api_key), + ("token", token), + ("refresh_token", refresh_token), + ): + if value is not None and not value: + raise ValueError( + f"{name}= must not be empty; pass a value, or omit all auth " + "arguments for an unauthenticated client." + ) + + # The three auth modes share a single X-Authorization slot, so combining them is + # ambiguous: whichever ran last would win, and under api_key auth the refresh hook + # is a no-op, so a JWT installed alongside a key would be frozen at its initial + # value and never refreshed. + modes = [ + f"{name}=" + for name, value in (("username", username), ("api_key", api_key), ("token", token)) + if value is not None + ] + if len(modes) > 1: + raise ValueError( + "ThingsboardClient authentication modes are mutually exclusive; got " + f"{', '.join(modes)}. Pass exactly one of username=, api_key= or token=." + ) + + # password= is only read by the username branch, so on its own it would be silently + # dropped and surface later as a 401. username= alone is rejected here rather than + # left to LoginRequest, whose password is a required StrictStr — otherwise the + # caller gets a pydantic ValidationError from inside the generated model. + if password is not None and username is None: + raise ValueError( + "password= requires username=; pass both, or omit both for an unauthenticated client." + ) + if username is not None and password is None: + raise ValueError( + "username= requires password=; pass both, or omit both for an unauthenticated client." + ) + # refresh_token= is likewise read only by the token branch. + if refresh_token is not None and token is None: + raise ValueError( + "refresh_token= requires token=; pass both, or omit both for an unauthenticated client." + ) + + class ThingsboardClient: """User-facing ThingsBoard client. Wraps the generated per-controller APIs with authentication management and - transparent 429 retry. Supports three authentication modes: + transparent 429 retry. Supports three authentication modes, plus unauthenticated: 1. Username + password (JWT): ThingsboardClient(url, username, password) @@ -52,6 +115,11 @@ class ThingsboardClient: 3. Pre-existing token: ThingsboardClient(url, token="jwt", refresh_token="jwt") Injects an externally obtained JWT; no login call made. + refresh_token is optional — omit it for a token that is never refreshed. + + The three modes are mutually exclusive — passing more than one raises ValueError. + All auth arguments are optional: omitting them yields an unauthenticated client + that sends no X-Authorization header, which is what the /api/noauth endpoints want. Context manager usage: with ThingsboardClient(url, api_key="key") as client: @@ -61,15 +129,16 @@ class ThingsboardClient: def __init__( self, url: str, - username: str = None, - password: str = None, - api_key: str = None, - token: str = None, - refresh_token: str = None, + username: "str | None" = None, + password: "str | None" = None, + api_key: "str | None" = None, + token: "str | None" = None, + refresh_token: "str | None" = None, max_retries: int = 3, initial_retry_delay_ms: int = 1_000, max_retry_delay_ms: int = 30_000, retry_on_rate_limit: bool = True, + auth_timeout_ms: int = DEFAULT_AUTH_TIMEOUT_MS, ): """Construct ThingsboardClient and authenticate. @@ -79,31 +148,41 @@ def __init__( password: Password for JWT authentication. api_key: API key for X-Authorization: ApiKey authentication. token: Pre-existing JWT access token. - refresh_token: Pre-existing JWT refresh token (used with token=). + refresh_token: Pre-existing JWT refresh token (used with token=). Omit it + to install a token that is never refreshed. max_retries: Maximum retry attempts on HTTP 429 (default 3). initial_retry_delay_ms: Base backoff delay in milliseconds (default 1000). max_retry_delay_ms: Maximum backoff cap in milliseconds (default 30000). retry_on_rate_limit: If True (default), wraps rest_client with _RetryingRESTClient. If False, uses plain RESTClientObject. + auth_timeout_ms: Ceiling for a single /api/auth call (default 30000). + Bounds how long other threads block behind a token refresh. + + Raises: + ValueError: If more than one of username=, api_key= or token= is given; + if password= is given without username= or vice versa; if + refresh_token= is given without token=; if any auth argument is + an empty string; or if auth_timeout_ms is not positive. """ # Must be the very first assignment — prevents __getattr__ infinite recursion # if __init__ raises partway through (before self.api_client is set). self._controllers: dict = {} + _validate_auth_args( + username=username, + password=password, + api_key=api_key, + token=token, + refresh_token=refresh_token, + ) + configuration = Configuration(host=url) - # Determine auth type - auth_type = "api_key" if api_key is not None else "jwt" - auth_manager = _AuthManager(url, auth_type, api_key) + auth_manager = _AuthManager(url, api_key, auth_timeout_ms) # Install the refresh hook so the hook fires before every API request configuration.refresh_api_key_hook = auth_manager.hook - # API key auth: set header at construction time - if api_key is not None: - configuration.api_key["ApiKeyForm"] = api_key - configuration.api_key_prefix["ApiKeyForm"] = "ApiKey" - # Build the ApiClient api_client = ApiClient(configuration=configuration) @@ -130,8 +209,9 @@ def __init__( # Pre-existing token if token is not None: auth_manager.set_external_token(token, refresh_token) - configuration.api_key["ApiKeyForm"] = token - configuration.api_key_prefix["ApiKeyForm"] = "Bearer" + + # Seed the header slot for whichever mode ran — see _AuthManager.install_header. + auth_manager.install_header(configuration) # ------------------------------------------------------------------ # Controller delegation diff --git a/common/docs/tb-examples.md b/common/docs/tb-examples.md index 0af4f168..c1dafa05 100644 --- a/common/docs/tb-examples.md +++ b/common/docs/tb-examples.md @@ -23,6 +23,40 @@ from tb_ce_client import ThingsboardClient client = ThingsboardClient("http://localhost:9090", api_key="your-api-key") ``` +## Pre-existing Token + +Injects an externally obtained JWT; no login call is made. + +```python +from tb_ce_client import ThingsboardClient + +client = ThingsboardClient( + "http://localhost:9090", + token="eyJhbGciOi...", + refresh_token="eyJhbGciOi...", +) +``` + +## No Authentication + +All auth arguments are optional. Omit them for a client that sends no +`X-Authorization` header, for use with the `/api/noauth` endpoints. + +```python +from tb_ce_client import ThingsboardClient + +client = ThingsboardClient("http://localhost:9090") +``` + +The three authenticated modes above are mutually exclusive — passing more than one +raises `ValueError`, as does passing `username=` without `password=` or vice versa, or +`refresh_token=` without `token=`. `token=` on its own is valid; it simply means the +token is never refreshed. + +An empty string is also rejected, so `api_key=os.environ.get("TB_API_KEY", "")` raises +rather than building a client that silently sends no credentials. Omit the auth +arguments entirely for an unauthenticated client. + ## Context Manager ```python diff --git a/editions.txt b/editions.txt new file mode 100644 index 00000000..12342af7 --- /dev/null +++ b/editions.txt @@ -0,0 +1,3 @@ +ce +pe +paas diff --git a/generate-client.sh b/generate-client.sh index f8ad4596..ea4742fe 100755 --- a/generate-client.sh +++ b/generate-client.sh @@ -22,7 +22,7 @@ # ./generate-client.sh [options] [base-url] # # Arguments: -# edition ce | pe | paas | all +# edition one of the names in editions.txt, or "all" # base-url Optional. Fetches spec from /v3/api-docs/thingsboard # and updates the local spec file before generation. # Not supported with "all". @@ -61,7 +61,26 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" -EDITIONS=("ce" "pe" "paas") +# Which editions exist. editions.txt is the shared source for this script, +# scripts/build-packages.sh and tests/test_common_overlay.py, so none of them has to +# parse another's formatting. It governs that list only — the per-edition controller +# thresholds below, and the spec/ directories, still need their own edits. +# +# Format: one name per line; blank lines and # comments ignored, surrounding whitespace +# trimmed. `read -r line` with the default IFS does that trimming, which is exactly +# str.strip() in the Python mirror — do not add `tr -d [:space:]`, which would also +# delete whitespace *inside* a line and silently disagree with it. +EDITIONS=() +# `|| [ -n "$line" ]` so a final line with no trailing newline is not dropped. +while read -r line || [ -n "$line" ]; do + case "$line" in ''|'#'*) continue ;; esac + EDITIONS+=("$line") +done < "$SCRIPT_DIR/editions.txt" +# `${EDITIONS[*]:-}` rather than ${#EDITIONS[@]}: the latter is unbound under set -u on +# bash < 4.4 when the array is empty, which is exactly the case being tested for. +if [ -z "${EDITIONS[*]:-}" ]; then + echo "Error: no editions listed in $SCRIPT_DIR/editions.txt"; exit 1 +fi VERBOSE=false DRY_RUN=false @@ -69,14 +88,17 @@ while [ $# -gt 0 ]; do case "$1" in --verbose) VERBOSE=true; shift ;; --dry-run) DRY_RUN=true; shift ;; + # Lets tests assert against this script's own parse of editions.txt rather than a + # reimplementation of it. Must stay ahead of the JAR download below. + --list-editions) printf '%s\n' "${EDITIONS[@]}"; exit 0 ;; -*) echo "Unknown option: $1"; exit 1 ;; *) break ;; esac done if [ $# -eq 0 ]; then - echo "Usage: $0 [--verbose] [--dry-run] [base-url]" - echo " edition: ce | pe | paas | all" + echo "Usage: $0 [--verbose] [--dry-run] [--list-editions] [base-url]" + echo " edition: ${EDITIONS[*]} | all" echo " base-url: optional, fetches spec from /v3/api-docs/thingsboard" exit 1 fi diff --git a/paas/docs/tb-examples.md b/paas/docs/tb-examples.md index 0af4f168..c1dafa05 100644 --- a/paas/docs/tb-examples.md +++ b/paas/docs/tb-examples.md @@ -23,6 +23,40 @@ from tb_ce_client import ThingsboardClient client = ThingsboardClient("http://localhost:9090", api_key="your-api-key") ``` +## Pre-existing Token + +Injects an externally obtained JWT; no login call is made. + +```python +from tb_ce_client import ThingsboardClient + +client = ThingsboardClient( + "http://localhost:9090", + token="eyJhbGciOi...", + refresh_token="eyJhbGciOi...", +) +``` + +## No Authentication + +All auth arguments are optional. Omit them for a client that sends no +`X-Authorization` header, for use with the `/api/noauth` endpoints. + +```python +from tb_ce_client import ThingsboardClient + +client = ThingsboardClient("http://localhost:9090") +``` + +The three authenticated modes above are mutually exclusive — passing more than one +raises `ValueError`, as does passing `username=` without `password=` or vice versa, or +`refresh_token=` without `token=`. `token=` on its own is valid; it simply means the +token is never refreshed. + +An empty string is also rejected, so `api_key=os.environ.get("TB_API_KEY", "")` raises +rather than building a client that silently sends no credentials. Omit the auth +arguments entirely for an unauthenticated client. + ## Context Manager ```python diff --git a/paas/tb_paas_client/_auth.py b/paas/tb_paas_client/_auth.py index 1c793d11..eb193221 100644 --- a/paas/tb_paas_client/_auth.py +++ b/paas/tb_paas_client/_auth.py @@ -34,6 +34,60 @@ # Matches Java's AuthManager.AVG_REQUEST_TIMEOUT (30 seconds in ms) AVG_REQUEST_TIMEOUT_MS = 30_000 +# Wall-clock ceiling for a single raw auth call. urllib3 defaults to no timeout at all, +# and every API thread now blocks behind an in-flight refresh, so an unresponsive auth +# endpoint would otherwise hang the whole process rather than one thread. +# +# Applied as Timeout(total=...) rather than a bare float, which would set connect and +# read separately and leave total unbounded. Because _AUTH_RETRIES makes exactly one +# request — no retry, no redirect — total is the per-call ceiling; urllib3 gives each +# attempt its own budget, so any allowance there would multiply this number. +# +# A blocked thread can wait twice this: _do_refresh_token falls back to _do_login, which +# is a second call. That is the worst case behind one refresh, and the fallback is what +# recovers an expired refresh token, so the 2x is deliberate rather than a leak. +DEFAULT_AUTH_TIMEOUT_MS = 30_000 + +# Retry policy for the raw auth calls, spelled out rather than left to urllib3, so that +# exactly one request goes out and DEFAULT_AUTH_TIMEOUT_MS means what it says. +# +# No retries: Retry.DEFAULT is total=3, and its connection-error branch never consults +# allowed_methods, so this POST would retry and cost 4x the ceiling above. Auth POSTs are +# not idempotent, and the timeout exists precisely to bound how long every other thread +# sits blocked, so a single attempt is the deliberate trade — a caller wanting tolerance +# should retry ThingsboardClient(...) itself, since urllib3 has no global deadline that +# would let us have both. +# +# No redirects either, which is a departure from the generated RESTClientObject: +# - urllib3 clones the timeout per hop instead of drawing down a shared budget, so +# following N redirects costs (1 + N) x auth_timeout_ms on the one round-trip every +# other API thread is blocked behind. +# - the body is re-sent to whatever Location names, with no same-origin restriction, +# so a redirect out of the configured server hands username/password to a third host +# and _do_login would install the token it returns. +# The case this gives up is a deployment that redirects auth (a proxy forcing https, +# path normalisation). Failing loudly is the better answer there: on an http -> https +# redirect the credentials have already gone out in cleartext, so the fix is to point url= +# at the redirect target's base URL, which _raw_post's error tells the caller to do. +# +# total is set explicitly: it defaults to 10, and leaving it there would contradict the +# "exactly one request" this whole block is for, even though the per-class zeros already +# exhaust first. +# +# Spelled redirect=False rather than redirect=0, which is not the same thing: Retry +# normalises False to 0 *and* clears raise_on_redirect, and that is what lets a 3xx come +# back as a response for the status check below to report. With redirect=0 the same reply +# raises MaxRetryError("too many redirects") instead and the remedy never reaches the +# caller. Both spellings leave .redirect == 0, so only raise_on_redirect tells them apart. +_AUTH_RETRIES = urllib3.Retry(total=0, connect=0, read=0, status=0, other=0, redirect=False) + +# Security scheme name and prefixes dictated by the generated configuration.py. +# Keep them in one place so a spec regeneration that renames the scheme has a +# single owner instead of literals scattered across client.py and _auth.py. +_SECURITY_SCHEME = "ApiKeyForm" +_JWT_PREFIX = "Bearer" +_API_KEY_PREFIX = "ApiKey" + # --------------------------------------------------------------------------- # _TokenInfo @@ -51,8 +105,8 @@ class _TokenInfo: def __init__( self, - token, - refresh_token, + token: "str | None", + refresh_token: "str | None", token_exp_ts: int, refresh_exp_ts: int, clock_diff: int, @@ -101,27 +155,46 @@ class _AuthManager: Mirrors the Java ThingsboardClient.java inner AuthManager class. - Thread safety: a threading.Lock protects the _refreshing flag so that only - one concurrent API thread triggers a refresh call. Other threads wait at the - lock and skip the refresh once the first thread completes. + Thread safety: a threading.Condition guards the _refreshing flag so that only + one concurrent API thread performs a refresh. The others block until that + refresh finishes and then use its result — they must not proceed meanwhile, + since the token they would send is the expired one being replaced. + + The auth mode is decided once in __init__ and never re-derived per request. """ - def __init__(self, base_url: str, auth_type: str, api_key=None): + def __init__( + self, + base_url: str, + api_key: "str | None" = None, + auth_timeout_ms: int = DEFAULT_AUTH_TIMEOUT_MS, + ): """ Args: - base_url: ThingsBoard server URL (e.g. "http://tb-server:9090"). - Trailing slashes are stripped. - auth_type: Either 'jwt' (username/password or token) or 'api_key'. - api_key: The API key string when auth_type='api_key', else None. + base_url: ThingsBoard server URL (e.g. "http://tb-server:9090"). + Trailing slashes are stripped. + api_key: The API key string for API key auth, or None for JWT auth + (username/password or an externally supplied token). + auth_timeout_ms: Ceiling for a single /api/auth call, in milliseconds. + Bounds how long every other thread can be blocked behind a + refresh, so it is a knob a slow on-prem server or a + latency-sensitive caller will want to change. """ + # urllib3.Timeout rejects a non-positive total, but it would only raise inside + # _raw_post — where _do_refresh_token and _do_login catch Exception and log — + # so a bad value would construct fine and then silently never refresh. + if auth_timeout_ms <= 0: + raise ValueError(f"auth_timeout_ms must be positive; got {auth_timeout_ms}") + self._auth_timeout_s = auth_timeout_ms / 1000 self._base_url = base_url.rstrip("/") - self._auth_type = auth_type - self._lock = threading.Lock() + self._is_api_key = api_key is not None + self._header_prefix = _API_KEY_PREFIX if self._is_api_key else _JWT_PREFIX + self._refresh_state = threading.Condition() self._refreshing = False self._username = None self._password = None - if auth_type == "api_key": + if self._is_api_key: self._token_info = _TokenInfo(api_key, None, -1, -1, 0) else: self._token_info = _TokenInfo.EMPTY # type: ignore[attr-defined] @@ -136,18 +209,37 @@ def on_login(self, username: str, password: str, token: str, refresh_token: str) self._password = password self._token_info = self._build_token_info(token, refresh_token) - def set_external_token(self, token: str, refresh_token=None) -> None: - """Set a pre-existing token without storing login credentials.""" - self._token_info = self._build_token_info(token, refresh_token or "") + def set_external_token(self, token: str, refresh_token: "str | None" = None) -> None: + """Set a pre-existing token without storing login credentials. + + refresh_token is passed through as-is so that omitting it leaves + get_refresh_token() returning None, as its docstring promises. + """ + self._token_info = self._build_token_info(token, refresh_token) - def get_token(self): + def get_token(self) -> "str | None": """Return the current access token, or None if not yet set.""" return self._token_info.token - def get_refresh_token(self): + def get_refresh_token(self) -> "str | None": """Return the current refresh token, or None if not available.""" return self._token_info.refresh_token + def install_header(self, configuration) -> None: + """Write the current token into configuration's X-Authorization slots. + + Configuration.auth_settings() emits the header only when the security + scheme is already present in configuration.api_key, so the slot has to be + seeded at construction time before the hook can ever take over. + """ + token = self._token_info.token + if not token: + # No auth configured (e.g. /api/noauth usage) — leave the slot absent + # so auth_settings() emits no header at all. + return + configuration.api_key[_SECURITY_SCHEME] = token + configuration.api_key_prefix[_SECURITY_SCHEME] = self._header_prefix + def hook(self, configuration) -> None: """refresh_api_key_hook implementation. @@ -155,14 +247,11 @@ def hook(self, configuration) -> None: every API request assembles its X-Authorization header. Checks token expiry and refreshes if needed, then updates configuration.api_key. """ - if self._auth_type != "jwt": + if self._is_api_key: # API key auth — hook is a no-op; the key is set at construction time return self._refresh_if_needed() - token = self._token_info.token - if token: - configuration.api_key["ApiKeyForm"] = token - configuration.api_key_prefix["ApiKeyForm"] = "Bearer" + self.install_header(configuration) # ------------------------------------------------------------------ # Internal refresh logic @@ -171,9 +260,14 @@ def hook(self, configuration) -> None: def _refresh_if_needed(self) -> None: """Check token expiry and trigger refresh if the estimated server time exceeds the token expiry (with AVG_REQUEST_TIMEOUT buffer).""" - with self._lock: + with self._refresh_state: if self._refreshing: - # Another thread is already refreshing — skip + # Another thread is already refreshing. Block rather than return: + # returning here would send the expired token that thread is busy + # replacing, and nothing retries the resulting 401. + self._refresh_state.wait_for(lambda: not self._refreshing) + # Its outcome is ours. Refreshing again on failure would multiply one + # failed round-trip by however many threads were waiting. return info = self._token_info if info.token is None or info.token_exp_ts < 0: @@ -196,8 +290,9 @@ def _refresh_if_needed(self) -> None: elif self._username: self._do_login() finally: - with self._lock: + with self._refresh_state: self._refreshing = False + self._refresh_state.notify_all() def _do_refresh_token(self, info: "_TokenInfo") -> None: """POST to /api/auth/token with the refresh token. Falls back to login on error.""" @@ -226,6 +321,10 @@ def _raw_post(self, path: str, body: bytes) -> dict: if we used ApiClient here, the hook would fire again while already inside the hook, causing infinite recursion (mirrors Java's pattern of using a separate raw HttpClient for AuthManager calls). + + Bounded by auth_timeout_ms and _AUTH_RETRIES: this call is on the critical path + for every thread waiting on a refresh, so it makes exactly one request — no + retry, no redirect. See _AUTH_RETRIES for why both trades are deliberate. """ http = urllib3.PoolManager() response = http.request( @@ -233,12 +332,25 @@ def _raw_post(self, path: str, body: bytes) -> dict: self._base_url + path, body=body, headers={"Content-Type": "application/json"}, + timeout=urllib3.Timeout(total=self._auth_timeout_s), + retries=_AUTH_RETRIES, ) if response.status != 200: - raise RuntimeError(f"Auth request to {path} returned HTTP {response.status}") + # A redirect arrives here rather than being followed — say so, since the + # remedy is specific and not guessable from the status alone. Gated on + # get_redirect_location() rather than the 3xx range because that is the + # predicate urllib3 itself would have followed: it covers 301/302/303/307/308 + # and excludes 300 and 304, which carry no Location worth chasing. + hint = ( + "; auth requests do not follow redirects, so set url= to the redirect " + "target's base URL instead." + if response.get_redirect_location() + else "" + ) + raise RuntimeError(f"Auth request to {path} returned HTTP {response.status}{hint}") return json.loads(response.data) - def _build_token_info(self, token: str, refresh_token: str) -> "_TokenInfo": + def _build_token_info(self, token: str, refresh_token: "str | None") -> "_TokenInfo": """Parse JWT claims from token and refresh_token; compute clock_diff.""" now_ms = int(time.time() * 1000) token_exp = _parse_jwt_claim_ms(token, "exp") diff --git a/paas/tb_paas_client/client.py b/paas/tb_paas_client/client.py index 27502cc0..5947faa4 100644 --- a/paas/tb_paas_client/client.py +++ b/paas/tb_paas_client/client.py @@ -27,7 +27,7 @@ import importlib -from ._auth import _AuthManager +from ._auth import DEFAULT_AUTH_TIMEOUT_MS, _AuthManager from ._controller_map import _CONTROLLER_ATTR_MAP, _CONTROLLER_MAP from ._retry import _RetryingRESTClient from .api_client import ApiClient @@ -35,11 +35,74 @@ from .models.login_request import LoginRequest +def _validate_auth_args( + username: "str | None", + password: "str | None", + api_key: "str | None", + token: "str | None", + refresh_token: "str | None", +) -> None: + """Reject auth argument combinations that cannot be honoured. + + Raises ValueError describing the offending arguments; returns None otherwise. + """ + # An empty string passes every "is not None" check below but installs no header, + # so the client would silently send no credentials at all — the failure mode this + # validation exists to prevent. Easy to reach via os.environ.get("TB_API_KEY", ""). + # Checked before the rules below so an empty value reports itself rather than the + # companion-argument error it would also trip. + for name, value in ( + ("username", username), + ("password", password), + ("api_key", api_key), + ("token", token), + ("refresh_token", refresh_token), + ): + if value is not None and not value: + raise ValueError( + f"{name}= must not be empty; pass a value, or omit all auth " + "arguments for an unauthenticated client." + ) + + # The three auth modes share a single X-Authorization slot, so combining them is + # ambiguous: whichever ran last would win, and under api_key auth the refresh hook + # is a no-op, so a JWT installed alongside a key would be frozen at its initial + # value and never refreshed. + modes = [ + f"{name}=" + for name, value in (("username", username), ("api_key", api_key), ("token", token)) + if value is not None + ] + if len(modes) > 1: + raise ValueError( + "ThingsboardClient authentication modes are mutually exclusive; got " + f"{', '.join(modes)}. Pass exactly one of username=, api_key= or token=." + ) + + # password= is only read by the username branch, so on its own it would be silently + # dropped and surface later as a 401. username= alone is rejected here rather than + # left to LoginRequest, whose password is a required StrictStr — otherwise the + # caller gets a pydantic ValidationError from inside the generated model. + if password is not None and username is None: + raise ValueError( + "password= requires username=; pass both, or omit both for an unauthenticated client." + ) + if username is not None and password is None: + raise ValueError( + "username= requires password=; pass both, or omit both for an unauthenticated client." + ) + # refresh_token= is likewise read only by the token branch. + if refresh_token is not None and token is None: + raise ValueError( + "refresh_token= requires token=; pass both, or omit both for an unauthenticated client." + ) + + class ThingsboardClient: """User-facing ThingsBoard client. Wraps the generated per-controller APIs with authentication management and - transparent 429 retry. Supports three authentication modes: + transparent 429 retry. Supports three authentication modes, plus unauthenticated: 1. Username + password (JWT): ThingsboardClient(url, username, password) @@ -52,6 +115,11 @@ class ThingsboardClient: 3. Pre-existing token: ThingsboardClient(url, token="jwt", refresh_token="jwt") Injects an externally obtained JWT; no login call made. + refresh_token is optional — omit it for a token that is never refreshed. + + The three modes are mutually exclusive — passing more than one raises ValueError. + All auth arguments are optional: omitting them yields an unauthenticated client + that sends no X-Authorization header, which is what the /api/noauth endpoints want. Context manager usage: with ThingsboardClient(url, api_key="key") as client: @@ -61,15 +129,16 @@ class ThingsboardClient: def __init__( self, url: str, - username: str = None, - password: str = None, - api_key: str = None, - token: str = None, - refresh_token: str = None, + username: "str | None" = None, + password: "str | None" = None, + api_key: "str | None" = None, + token: "str | None" = None, + refresh_token: "str | None" = None, max_retries: int = 3, initial_retry_delay_ms: int = 1_000, max_retry_delay_ms: int = 30_000, retry_on_rate_limit: bool = True, + auth_timeout_ms: int = DEFAULT_AUTH_TIMEOUT_MS, ): """Construct ThingsboardClient and authenticate. @@ -79,31 +148,41 @@ def __init__( password: Password for JWT authentication. api_key: API key for X-Authorization: ApiKey authentication. token: Pre-existing JWT access token. - refresh_token: Pre-existing JWT refresh token (used with token=). + refresh_token: Pre-existing JWT refresh token (used with token=). Omit it + to install a token that is never refreshed. max_retries: Maximum retry attempts on HTTP 429 (default 3). initial_retry_delay_ms: Base backoff delay in milliseconds (default 1000). max_retry_delay_ms: Maximum backoff cap in milliseconds (default 30000). retry_on_rate_limit: If True (default), wraps rest_client with _RetryingRESTClient. If False, uses plain RESTClientObject. + auth_timeout_ms: Ceiling for a single /api/auth call (default 30000). + Bounds how long other threads block behind a token refresh. + + Raises: + ValueError: If more than one of username=, api_key= or token= is given; + if password= is given without username= or vice versa; if + refresh_token= is given without token=; if any auth argument is + an empty string; or if auth_timeout_ms is not positive. """ # Must be the very first assignment — prevents __getattr__ infinite recursion # if __init__ raises partway through (before self.api_client is set). self._controllers: dict = {} + _validate_auth_args( + username=username, + password=password, + api_key=api_key, + token=token, + refresh_token=refresh_token, + ) + configuration = Configuration(host=url) - # Determine auth type - auth_type = "api_key" if api_key is not None else "jwt" - auth_manager = _AuthManager(url, auth_type, api_key) + auth_manager = _AuthManager(url, api_key, auth_timeout_ms) # Install the refresh hook so the hook fires before every API request configuration.refresh_api_key_hook = auth_manager.hook - # API key auth: set header at construction time - if api_key is not None: - configuration.api_key["ApiKeyForm"] = api_key - configuration.api_key_prefix["ApiKeyForm"] = "ApiKey" - # Build the ApiClient api_client = ApiClient(configuration=configuration) @@ -130,8 +209,9 @@ def __init__( # Pre-existing token if token is not None: auth_manager.set_external_token(token, refresh_token) - configuration.api_key["ApiKeyForm"] = token - configuration.api_key_prefix["ApiKeyForm"] = "Bearer" + + # Seed the header slot for whichever mode ran — see _AuthManager.install_header. + auth_manager.install_header(configuration) # ------------------------------------------------------------------ # Controller delegation diff --git a/pe/docs/tb-examples.md b/pe/docs/tb-examples.md index 0af4f168..c1dafa05 100644 --- a/pe/docs/tb-examples.md +++ b/pe/docs/tb-examples.md @@ -23,6 +23,40 @@ from tb_ce_client import ThingsboardClient client = ThingsboardClient("http://localhost:9090", api_key="your-api-key") ``` +## Pre-existing Token + +Injects an externally obtained JWT; no login call is made. + +```python +from tb_ce_client import ThingsboardClient + +client = ThingsboardClient( + "http://localhost:9090", + token="eyJhbGciOi...", + refresh_token="eyJhbGciOi...", +) +``` + +## No Authentication + +All auth arguments are optional. Omit them for a client that sends no +`X-Authorization` header, for use with the `/api/noauth` endpoints. + +```python +from tb_ce_client import ThingsboardClient + +client = ThingsboardClient("http://localhost:9090") +``` + +The three authenticated modes above are mutually exclusive — passing more than one +raises `ValueError`, as does passing `username=` without `password=` or vice versa, or +`refresh_token=` without `token=`. `token=` on its own is valid; it simply means the +token is never refreshed. + +An empty string is also rejected, so `api_key=os.environ.get("TB_API_KEY", "")` raises +rather than building a client that silently sends no credentials. Omit the auth +arguments entirely for an unauthenticated client. + ## Context Manager ```python diff --git a/pe/tb_pe_client/_auth.py b/pe/tb_pe_client/_auth.py index 1c793d11..eb193221 100644 --- a/pe/tb_pe_client/_auth.py +++ b/pe/tb_pe_client/_auth.py @@ -34,6 +34,60 @@ # Matches Java's AuthManager.AVG_REQUEST_TIMEOUT (30 seconds in ms) AVG_REQUEST_TIMEOUT_MS = 30_000 +# Wall-clock ceiling for a single raw auth call. urllib3 defaults to no timeout at all, +# and every API thread now blocks behind an in-flight refresh, so an unresponsive auth +# endpoint would otherwise hang the whole process rather than one thread. +# +# Applied as Timeout(total=...) rather than a bare float, which would set connect and +# read separately and leave total unbounded. Because _AUTH_RETRIES makes exactly one +# request — no retry, no redirect — total is the per-call ceiling; urllib3 gives each +# attempt its own budget, so any allowance there would multiply this number. +# +# A blocked thread can wait twice this: _do_refresh_token falls back to _do_login, which +# is a second call. That is the worst case behind one refresh, and the fallback is what +# recovers an expired refresh token, so the 2x is deliberate rather than a leak. +DEFAULT_AUTH_TIMEOUT_MS = 30_000 + +# Retry policy for the raw auth calls, spelled out rather than left to urllib3, so that +# exactly one request goes out and DEFAULT_AUTH_TIMEOUT_MS means what it says. +# +# No retries: Retry.DEFAULT is total=3, and its connection-error branch never consults +# allowed_methods, so this POST would retry and cost 4x the ceiling above. Auth POSTs are +# not idempotent, and the timeout exists precisely to bound how long every other thread +# sits blocked, so a single attempt is the deliberate trade — a caller wanting tolerance +# should retry ThingsboardClient(...) itself, since urllib3 has no global deadline that +# would let us have both. +# +# No redirects either, which is a departure from the generated RESTClientObject: +# - urllib3 clones the timeout per hop instead of drawing down a shared budget, so +# following N redirects costs (1 + N) x auth_timeout_ms on the one round-trip every +# other API thread is blocked behind. +# - the body is re-sent to whatever Location names, with no same-origin restriction, +# so a redirect out of the configured server hands username/password to a third host +# and _do_login would install the token it returns. +# The case this gives up is a deployment that redirects auth (a proxy forcing https, +# path normalisation). Failing loudly is the better answer there: on an http -> https +# redirect the credentials have already gone out in cleartext, so the fix is to point url= +# at the redirect target's base URL, which _raw_post's error tells the caller to do. +# +# total is set explicitly: it defaults to 10, and leaving it there would contradict the +# "exactly one request" this whole block is for, even though the per-class zeros already +# exhaust first. +# +# Spelled redirect=False rather than redirect=0, which is not the same thing: Retry +# normalises False to 0 *and* clears raise_on_redirect, and that is what lets a 3xx come +# back as a response for the status check below to report. With redirect=0 the same reply +# raises MaxRetryError("too many redirects") instead and the remedy never reaches the +# caller. Both spellings leave .redirect == 0, so only raise_on_redirect tells them apart. +_AUTH_RETRIES = urllib3.Retry(total=0, connect=0, read=0, status=0, other=0, redirect=False) + +# Security scheme name and prefixes dictated by the generated configuration.py. +# Keep them in one place so a spec regeneration that renames the scheme has a +# single owner instead of literals scattered across client.py and _auth.py. +_SECURITY_SCHEME = "ApiKeyForm" +_JWT_PREFIX = "Bearer" +_API_KEY_PREFIX = "ApiKey" + # --------------------------------------------------------------------------- # _TokenInfo @@ -51,8 +105,8 @@ class _TokenInfo: def __init__( self, - token, - refresh_token, + token: "str | None", + refresh_token: "str | None", token_exp_ts: int, refresh_exp_ts: int, clock_diff: int, @@ -101,27 +155,46 @@ class _AuthManager: Mirrors the Java ThingsboardClient.java inner AuthManager class. - Thread safety: a threading.Lock protects the _refreshing flag so that only - one concurrent API thread triggers a refresh call. Other threads wait at the - lock and skip the refresh once the first thread completes. + Thread safety: a threading.Condition guards the _refreshing flag so that only + one concurrent API thread performs a refresh. The others block until that + refresh finishes and then use its result — they must not proceed meanwhile, + since the token they would send is the expired one being replaced. + + The auth mode is decided once in __init__ and never re-derived per request. """ - def __init__(self, base_url: str, auth_type: str, api_key=None): + def __init__( + self, + base_url: str, + api_key: "str | None" = None, + auth_timeout_ms: int = DEFAULT_AUTH_TIMEOUT_MS, + ): """ Args: - base_url: ThingsBoard server URL (e.g. "http://tb-server:9090"). - Trailing slashes are stripped. - auth_type: Either 'jwt' (username/password or token) or 'api_key'. - api_key: The API key string when auth_type='api_key', else None. + base_url: ThingsBoard server URL (e.g. "http://tb-server:9090"). + Trailing slashes are stripped. + api_key: The API key string for API key auth, or None for JWT auth + (username/password or an externally supplied token). + auth_timeout_ms: Ceiling for a single /api/auth call, in milliseconds. + Bounds how long every other thread can be blocked behind a + refresh, so it is a knob a slow on-prem server or a + latency-sensitive caller will want to change. """ + # urllib3.Timeout rejects a non-positive total, but it would only raise inside + # _raw_post — where _do_refresh_token and _do_login catch Exception and log — + # so a bad value would construct fine and then silently never refresh. + if auth_timeout_ms <= 0: + raise ValueError(f"auth_timeout_ms must be positive; got {auth_timeout_ms}") + self._auth_timeout_s = auth_timeout_ms / 1000 self._base_url = base_url.rstrip("/") - self._auth_type = auth_type - self._lock = threading.Lock() + self._is_api_key = api_key is not None + self._header_prefix = _API_KEY_PREFIX if self._is_api_key else _JWT_PREFIX + self._refresh_state = threading.Condition() self._refreshing = False self._username = None self._password = None - if auth_type == "api_key": + if self._is_api_key: self._token_info = _TokenInfo(api_key, None, -1, -1, 0) else: self._token_info = _TokenInfo.EMPTY # type: ignore[attr-defined] @@ -136,18 +209,37 @@ def on_login(self, username: str, password: str, token: str, refresh_token: str) self._password = password self._token_info = self._build_token_info(token, refresh_token) - def set_external_token(self, token: str, refresh_token=None) -> None: - """Set a pre-existing token without storing login credentials.""" - self._token_info = self._build_token_info(token, refresh_token or "") + def set_external_token(self, token: str, refresh_token: "str | None" = None) -> None: + """Set a pre-existing token without storing login credentials. + + refresh_token is passed through as-is so that omitting it leaves + get_refresh_token() returning None, as its docstring promises. + """ + self._token_info = self._build_token_info(token, refresh_token) - def get_token(self): + def get_token(self) -> "str | None": """Return the current access token, or None if not yet set.""" return self._token_info.token - def get_refresh_token(self): + def get_refresh_token(self) -> "str | None": """Return the current refresh token, or None if not available.""" return self._token_info.refresh_token + def install_header(self, configuration) -> None: + """Write the current token into configuration's X-Authorization slots. + + Configuration.auth_settings() emits the header only when the security + scheme is already present in configuration.api_key, so the slot has to be + seeded at construction time before the hook can ever take over. + """ + token = self._token_info.token + if not token: + # No auth configured (e.g. /api/noauth usage) — leave the slot absent + # so auth_settings() emits no header at all. + return + configuration.api_key[_SECURITY_SCHEME] = token + configuration.api_key_prefix[_SECURITY_SCHEME] = self._header_prefix + def hook(self, configuration) -> None: """refresh_api_key_hook implementation. @@ -155,14 +247,11 @@ def hook(self, configuration) -> None: every API request assembles its X-Authorization header. Checks token expiry and refreshes if needed, then updates configuration.api_key. """ - if self._auth_type != "jwt": + if self._is_api_key: # API key auth — hook is a no-op; the key is set at construction time return self._refresh_if_needed() - token = self._token_info.token - if token: - configuration.api_key["ApiKeyForm"] = token - configuration.api_key_prefix["ApiKeyForm"] = "Bearer" + self.install_header(configuration) # ------------------------------------------------------------------ # Internal refresh logic @@ -171,9 +260,14 @@ def hook(self, configuration) -> None: def _refresh_if_needed(self) -> None: """Check token expiry and trigger refresh if the estimated server time exceeds the token expiry (with AVG_REQUEST_TIMEOUT buffer).""" - with self._lock: + with self._refresh_state: if self._refreshing: - # Another thread is already refreshing — skip + # Another thread is already refreshing. Block rather than return: + # returning here would send the expired token that thread is busy + # replacing, and nothing retries the resulting 401. + self._refresh_state.wait_for(lambda: not self._refreshing) + # Its outcome is ours. Refreshing again on failure would multiply one + # failed round-trip by however many threads were waiting. return info = self._token_info if info.token is None or info.token_exp_ts < 0: @@ -196,8 +290,9 @@ def _refresh_if_needed(self) -> None: elif self._username: self._do_login() finally: - with self._lock: + with self._refresh_state: self._refreshing = False + self._refresh_state.notify_all() def _do_refresh_token(self, info: "_TokenInfo") -> None: """POST to /api/auth/token with the refresh token. Falls back to login on error.""" @@ -226,6 +321,10 @@ def _raw_post(self, path: str, body: bytes) -> dict: if we used ApiClient here, the hook would fire again while already inside the hook, causing infinite recursion (mirrors Java's pattern of using a separate raw HttpClient for AuthManager calls). + + Bounded by auth_timeout_ms and _AUTH_RETRIES: this call is on the critical path + for every thread waiting on a refresh, so it makes exactly one request — no + retry, no redirect. See _AUTH_RETRIES for why both trades are deliberate. """ http = urllib3.PoolManager() response = http.request( @@ -233,12 +332,25 @@ def _raw_post(self, path: str, body: bytes) -> dict: self._base_url + path, body=body, headers={"Content-Type": "application/json"}, + timeout=urllib3.Timeout(total=self._auth_timeout_s), + retries=_AUTH_RETRIES, ) if response.status != 200: - raise RuntimeError(f"Auth request to {path} returned HTTP {response.status}") + # A redirect arrives here rather than being followed — say so, since the + # remedy is specific and not guessable from the status alone. Gated on + # get_redirect_location() rather than the 3xx range because that is the + # predicate urllib3 itself would have followed: it covers 301/302/303/307/308 + # and excludes 300 and 304, which carry no Location worth chasing. + hint = ( + "; auth requests do not follow redirects, so set url= to the redirect " + "target's base URL instead." + if response.get_redirect_location() + else "" + ) + raise RuntimeError(f"Auth request to {path} returned HTTP {response.status}{hint}") return json.loads(response.data) - def _build_token_info(self, token: str, refresh_token: str) -> "_TokenInfo": + def _build_token_info(self, token: str, refresh_token: "str | None") -> "_TokenInfo": """Parse JWT claims from token and refresh_token; compute clock_diff.""" now_ms = int(time.time() * 1000) token_exp = _parse_jwt_claim_ms(token, "exp") diff --git a/pe/tb_pe_client/client.py b/pe/tb_pe_client/client.py index 27502cc0..5947faa4 100644 --- a/pe/tb_pe_client/client.py +++ b/pe/tb_pe_client/client.py @@ -27,7 +27,7 @@ import importlib -from ._auth import _AuthManager +from ._auth import DEFAULT_AUTH_TIMEOUT_MS, _AuthManager from ._controller_map import _CONTROLLER_ATTR_MAP, _CONTROLLER_MAP from ._retry import _RetryingRESTClient from .api_client import ApiClient @@ -35,11 +35,74 @@ from .models.login_request import LoginRequest +def _validate_auth_args( + username: "str | None", + password: "str | None", + api_key: "str | None", + token: "str | None", + refresh_token: "str | None", +) -> None: + """Reject auth argument combinations that cannot be honoured. + + Raises ValueError describing the offending arguments; returns None otherwise. + """ + # An empty string passes every "is not None" check below but installs no header, + # so the client would silently send no credentials at all — the failure mode this + # validation exists to prevent. Easy to reach via os.environ.get("TB_API_KEY", ""). + # Checked before the rules below so an empty value reports itself rather than the + # companion-argument error it would also trip. + for name, value in ( + ("username", username), + ("password", password), + ("api_key", api_key), + ("token", token), + ("refresh_token", refresh_token), + ): + if value is not None and not value: + raise ValueError( + f"{name}= must not be empty; pass a value, or omit all auth " + "arguments for an unauthenticated client." + ) + + # The three auth modes share a single X-Authorization slot, so combining them is + # ambiguous: whichever ran last would win, and under api_key auth the refresh hook + # is a no-op, so a JWT installed alongside a key would be frozen at its initial + # value and never refreshed. + modes = [ + f"{name}=" + for name, value in (("username", username), ("api_key", api_key), ("token", token)) + if value is not None + ] + if len(modes) > 1: + raise ValueError( + "ThingsboardClient authentication modes are mutually exclusive; got " + f"{', '.join(modes)}. Pass exactly one of username=, api_key= or token=." + ) + + # password= is only read by the username branch, so on its own it would be silently + # dropped and surface later as a 401. username= alone is rejected here rather than + # left to LoginRequest, whose password is a required StrictStr — otherwise the + # caller gets a pydantic ValidationError from inside the generated model. + if password is not None and username is None: + raise ValueError( + "password= requires username=; pass both, or omit both for an unauthenticated client." + ) + if username is not None and password is None: + raise ValueError( + "username= requires password=; pass both, or omit both for an unauthenticated client." + ) + # refresh_token= is likewise read only by the token branch. + if refresh_token is not None and token is None: + raise ValueError( + "refresh_token= requires token=; pass both, or omit both for an unauthenticated client." + ) + + class ThingsboardClient: """User-facing ThingsBoard client. Wraps the generated per-controller APIs with authentication management and - transparent 429 retry. Supports three authentication modes: + transparent 429 retry. Supports three authentication modes, plus unauthenticated: 1. Username + password (JWT): ThingsboardClient(url, username, password) @@ -52,6 +115,11 @@ class ThingsboardClient: 3. Pre-existing token: ThingsboardClient(url, token="jwt", refresh_token="jwt") Injects an externally obtained JWT; no login call made. + refresh_token is optional — omit it for a token that is never refreshed. + + The three modes are mutually exclusive — passing more than one raises ValueError. + All auth arguments are optional: omitting them yields an unauthenticated client + that sends no X-Authorization header, which is what the /api/noauth endpoints want. Context manager usage: with ThingsboardClient(url, api_key="key") as client: @@ -61,15 +129,16 @@ class ThingsboardClient: def __init__( self, url: str, - username: str = None, - password: str = None, - api_key: str = None, - token: str = None, - refresh_token: str = None, + username: "str | None" = None, + password: "str | None" = None, + api_key: "str | None" = None, + token: "str | None" = None, + refresh_token: "str | None" = None, max_retries: int = 3, initial_retry_delay_ms: int = 1_000, max_retry_delay_ms: int = 30_000, retry_on_rate_limit: bool = True, + auth_timeout_ms: int = DEFAULT_AUTH_TIMEOUT_MS, ): """Construct ThingsboardClient and authenticate. @@ -79,31 +148,41 @@ def __init__( password: Password for JWT authentication. api_key: API key for X-Authorization: ApiKey authentication. token: Pre-existing JWT access token. - refresh_token: Pre-existing JWT refresh token (used with token=). + refresh_token: Pre-existing JWT refresh token (used with token=). Omit it + to install a token that is never refreshed. max_retries: Maximum retry attempts on HTTP 429 (default 3). initial_retry_delay_ms: Base backoff delay in milliseconds (default 1000). max_retry_delay_ms: Maximum backoff cap in milliseconds (default 30000). retry_on_rate_limit: If True (default), wraps rest_client with _RetryingRESTClient. If False, uses plain RESTClientObject. + auth_timeout_ms: Ceiling for a single /api/auth call (default 30000). + Bounds how long other threads block behind a token refresh. + + Raises: + ValueError: If more than one of username=, api_key= or token= is given; + if password= is given without username= or vice versa; if + refresh_token= is given without token=; if any auth argument is + an empty string; or if auth_timeout_ms is not positive. """ # Must be the very first assignment — prevents __getattr__ infinite recursion # if __init__ raises partway through (before self.api_client is set). self._controllers: dict = {} + _validate_auth_args( + username=username, + password=password, + api_key=api_key, + token=token, + refresh_token=refresh_token, + ) + configuration = Configuration(host=url) - # Determine auth type - auth_type = "api_key" if api_key is not None else "jwt" - auth_manager = _AuthManager(url, auth_type, api_key) + auth_manager = _AuthManager(url, api_key, auth_timeout_ms) # Install the refresh hook so the hook fires before every API request configuration.refresh_api_key_hook = auth_manager.hook - # API key auth: set header at construction time - if api_key is not None: - configuration.api_key["ApiKeyForm"] = api_key - configuration.api_key_prefix["ApiKeyForm"] = "ApiKey" - # Build the ApiClient api_client = ApiClient(configuration=configuration) @@ -130,8 +209,9 @@ def __init__( # Pre-existing token if token is not None: auth_manager.set_external_token(token, refresh_token) - configuration.api_key["ApiKeyForm"] = token - configuration.api_key_prefix["ApiKeyForm"] = "Bearer" + + # Seed the header slot for whichever mode ran — see _AuthManager.install_header. + auth_manager.install_header(configuration) # ------------------------------------------------------------------ # Controller delegation diff --git a/scripts/build-packages.sh b/scripts/build-packages.sh index af8d3773..5573396f 100755 --- a/scripts/build-packages.sh +++ b/scripts/build-packages.sh @@ -16,8 +16,8 @@ # # -# Builds Python wheel and sdist packages for all three ThingsBoard client editions -# (CE, PE, PaaS), including version stamping, generation, and smoke testing. +# Builds Python wheel and sdist packages for every ThingsBoard client edition listed in +# editions.txt, including version stamping, generation, and smoke testing. # # Usage: # ./scripts/build-packages.sh @@ -25,9 +25,9 @@ # What it does: # 1. Reads version from root pyproject.toml # 2. Cleans dist/ -# 3. Generates all 3 editions via generate-client.sh all +# 3. Generates each edition in editions.txt via generate-client.sh # 4. For each edition: stamps version, builds wheel + sdist, smoke-tests in clean venv -# 5. Asserts all 3 wheels have the same version in their filenames +# 5. Asserts one wheel per edition, all with the same version in their filenames # 6. Prints summary of built artifacts # # Prerequisites: @@ -41,7 +41,20 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" ROOT_DIR="$SCRIPT_DIR/.." DIST_DIR="$ROOT_DIR/dist" -EDITIONS=("ce" "pe" "paas") +# Editions come from generate-client.sh's own parse of editions.txt, so this script +# never reimplements that format — see the comment on the EDITIONS block there. +# Captured into a variable first: process substitution discards the child's exit status, +# so `set -e` would not see --list-editions fail and we would build nothing, silently. +editions_output="$("$ROOT_DIR/generate-client.sh" --list-editions)" +EDITIONS=() +while read -r line; do + [ -n "$line" ] && EDITIONS+=("$line") +done <<< "$editions_output" +if [ -z "${EDITIONS[*]:-}" ]; then + echo "Error: generate-client.sh --list-editions returned no editions" >&2 + exit 1 +fi +EDITION_COUNT=${#EDITIONS[@]} # Add project venv to PATH so that tools installed via pip install (e.g. poetry) # are accessible without requiring a manual `source .venv/bin/activate`. @@ -187,7 +200,7 @@ for edition in "${EDITIONS[@]}"; do done # --------------------------------------------------------------------------- -# 6. Final verification: assert all 3 wheels have the same version +# 6. Final verification: one wheel per edition, all at the same version # --------------------------------------------------------------------------- info "Final verification" @@ -202,11 +215,11 @@ for edition in "${EDITIONS[@]}"; do ok " ${edition}: $(basename "$wheel")" done -if [ "$WHEEL_COUNT" -ne 3 ]; then - fail "Expected 3 wheels, found $WHEEL_COUNT" +if [ "$WHEEL_COUNT" -ne "$EDITION_COUNT" ]; then + fail "Expected $EDITION_COUNT wheels, found $WHEEL_COUNT" fi -ok "All 3 editions built with version ${VERSION}" +ok "All $EDITION_COUNT editions built with version ${VERSION}" # --------------------------------------------------------------------------- # 7. Print summary diff --git a/tests/_jwt.py b/tests/_jwt.py new file mode 100644 index 00000000..f2168ec4 --- /dev/null +++ b/tests/_jwt.py @@ -0,0 +1,52 @@ +""" +JWT factories shared by the auth and client tests. + +Underscore-prefixed so pytest does not collect it as a test module. Stdlib only — +it must not import common/ or any tb_*_client package, so either side can use it. +""" + +import base64 +import json +import time + + +def make_jwt(claims: dict) -> str: + """Create a minimal 3-part JWT (header.payload.signature) for testing. + + The header and signature are stubs — only the payload is meaningful. + """ + header = ( + base64.urlsafe_b64encode(json.dumps({"alg": "HS256", "typ": "JWT"}).encode()) + .rstrip(b"=") + .decode() + ) + payload = base64.urlsafe_b64encode(json.dumps(claims).encode()).rstrip(b"=").decode() + signature = "fakesig" + return f"{header}.{payload}.{signature}" + + +def make_token(exp_offset_s: int, iat_offset_s: int = 0) -> str: + """Create a JWT with exp = now + exp_offset_s and iat = now + iat_offset_s.""" + now = int(time.time()) + return make_jwt( + { + "exp": now + exp_offset_s, + "iat": now + iat_offset_s, + "sub": "user@example.com", + } + ) + + +def make_refresh_token(exp_offset_s: int) -> str: + """Create a refresh JWT with exp = now + exp_offset_s. + + Kept separate from make_token because ThingsBoard refresh tokens carry no iat + claim — _build_token_info() reads iat only from the access token. + """ + now = int(time.time()) + return make_jwt( + { + "exp": now + exp_offset_s, + "sub": "user@example.com", + } + ) diff --git a/tests/test_auth.py b/tests/test_auth.py index 34c6debe..418afaa2 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -3,57 +3,22 @@ Covers AUTH-01 through AUTH-06 requirements. """ -import base64 -import json +import http.server +import threading import time import unittest from unittest.mock import MagicMock, patch -from common._auth import _AuthManager, _parse_jwt_claim_ms +import urllib3 + +from common._auth import DEFAULT_AUTH_TIMEOUT_MS, _AuthManager, _parse_jwt_claim_ms +from tests._jwt import make_jwt, make_refresh_token, make_token # --------------------------------------------------------------------------- # Test helpers # --------------------------------------------------------------------------- -def _make_jwt(claims: dict) -> str: - """Create a minimal 3-part JWT (header.payload.signature) for testing. - - The header and signature are stubs — only the payload is meaningful. - """ - header = ( - base64.urlsafe_b64encode(json.dumps({"alg": "HS256", "typ": "JWT"}).encode()) - .rstrip(b"=") - .decode() - ) - payload = base64.urlsafe_b64encode(json.dumps(claims).encode()).rstrip(b"=").decode() - signature = "fakesig" - return f"{header}.{payload}.{signature}" - - -def _make_token(exp_offset_s: int, iat_offset_s: int = 0) -> str: - """Create a JWT with exp = now + exp_offset_s and iat = now + iat_offset_s.""" - now = int(time.time()) - return _make_jwt( - { - "exp": now + exp_offset_s, - "iat": now + iat_offset_s, - "sub": "user@example.com", - } - ) - - -def _make_refresh_token(exp_offset_s: int) -> str: - """Create a JWT with exp = now + exp_offset_s (for refresh tokens).""" - now = int(time.time()) - return _make_jwt( - { - "exp": now + exp_offset_s, - "sub": "user@example.com", - } - ) - - def _mock_configuration(): """Return a minimal mock Configuration object.""" config = MagicMock() @@ -71,14 +36,14 @@ class TestParseJwtClaimMs(unittest.TestCase): def test_parse_exp_claim(self): """_parse_jwt_claim_ms returns exp * 1000 for a valid JWT.""" exp_seconds = int(time.time()) + 3600 - jwt = _make_jwt({"exp": exp_seconds, "iat": int(time.time())}) + jwt = make_jwt({"exp": exp_seconds, "iat": int(time.time())}) result = _parse_jwt_claim_ms(jwt, "exp") self.assertEqual(result, exp_seconds * 1000) def test_parse_iat_claim(self): """_parse_jwt_claim_ms returns iat * 1000 for the iat claim.""" iat_seconds = int(time.time()) - jwt = _make_jwt({"exp": iat_seconds + 3600, "iat": iat_seconds}) + jwt = make_jwt({"exp": iat_seconds + 3600, "iat": iat_seconds}) result = _parse_jwt_claim_ms(jwt, "iat") self.assertEqual(result, iat_seconds * 1000) @@ -90,7 +55,7 @@ def test_invalid_jwt_returns_minus_one(self): def test_missing_claim_returns_minus_one(self): """_parse_jwt_claim_ms returns -1 when the claim key is absent.""" - jwt = _make_jwt({"sub": "user@example.com"}) + jwt = make_jwt({"sub": "user@example.com"}) self.assertEqual(_parse_jwt_claim_ms(jwt, "exp"), -1) def test_malformed_base64_returns_minus_one(self): @@ -106,9 +71,9 @@ def test_malformed_base64_returns_minus_one(self): class TestJwtLogin(unittest.TestCase): def test_jwt_login(self): """on_login stores credentials and builds _TokenInfo from provided JWTs.""" - auth = _AuthManager("http://tb:9090", "jwt", None) - token = _make_token(exp_offset_s=3600, iat_offset_s=0) - refresh = _make_refresh_token(exp_offset_s=86400) + auth = _AuthManager("http://tb:9090") + token = make_token(exp_offset_s=3600, iat_offset_s=0) + refresh = make_refresh_token(exp_offset_s=86400) auth.on_login("user@tb.io", "password", token, refresh) @@ -130,15 +95,15 @@ def test_jwt_login(self): class TestHookRefreshesExpiredToken(unittest.TestCase): def test_hook_refreshes_expired_token(self): """hook() calls /api/auth/token when access token is expired but refresh is valid.""" - auth = _AuthManager("http://tb:9090", "jwt", None) + auth = _AuthManager("http://tb:9090") # expired access token (exp in the past) - old_token = _make_token(exp_offset_s=-3600, iat_offset_s=0) + old_token = make_token(exp_offset_s=-3600, iat_offset_s=0) # valid refresh token - refresh = _make_refresh_token(exp_offset_s=86400) + refresh = make_refresh_token(exp_offset_s=86400) auth.on_login("user@tb.io", "password", old_token, refresh) - new_token = _make_token(exp_offset_s=7200, iat_offset_s=0) - new_refresh = _make_refresh_token(exp_offset_s=172800) + new_token = make_token(exp_offset_s=7200, iat_offset_s=0) + new_refresh = make_refresh_token(exp_offset_s=172800) new_response_data = {"token": new_token, "refreshToken": new_refresh} with patch.object(auth, "_raw_post", return_value=new_response_data) as mock_post: @@ -151,10 +116,10 @@ def test_hook_refreshes_expired_token(self): def test_hook_skips_when_token_valid(self): """hook() does not call HTTP when access token is still valid.""" - auth = _AuthManager("http://tb:9090", "jwt", None) + auth = _AuthManager("http://tb:9090") # token valid for an hour - token = _make_token(exp_offset_s=3600, iat_offset_s=0) - refresh = _make_refresh_token(exp_offset_s=86400) + token = make_token(exp_offset_s=3600, iat_offset_s=0) + refresh = make_refresh_token(exp_offset_s=86400) auth.on_login("user@tb.io", "password", token, refresh) with patch.object(auth, "_raw_post") as mock_post: @@ -174,11 +139,11 @@ def test_hook_skips_when_token_valid(self): class TestClockSkewCompensation(unittest.TestCase): def test_clock_skew_compensation(self): """clock_diff is computed from iat; a 5s server-ahead skew is absorbed into estimates.""" - auth = _AuthManager("http://tb:9090", "jwt", None) + auth = _AuthManager("http://tb:9090") # iat is 5 seconds ahead of "now" (simulates server clock being 5s ahead) skew_s = 5 - token = _make_token(exp_offset_s=skew_s + 35, iat_offset_s=skew_s) - refresh = _make_refresh_token(exp_offset_s=86400) + token = make_token(exp_offset_s=skew_s + 35, iat_offset_s=skew_s) + refresh = make_refresh_token(exp_offset_s=86400) auth.on_login("user@tb.io", "password", token, refresh) @@ -203,13 +168,13 @@ def test_clock_skew_compensation(self): class TestReloginOnRefreshExpiry(unittest.TestCase): def test_relogin_on_refresh_expiry(self): """hook() calls /api/auth/login when both access and refresh tokens are expired.""" - auth = _AuthManager("http://tb:9090", "jwt", None) - expired_token = _make_token(exp_offset_s=-7200, iat_offset_s=0) - expired_refresh = _make_refresh_token(exp_offset_s=-3600) + auth = _AuthManager("http://tb:9090") + expired_token = make_token(exp_offset_s=-7200, iat_offset_s=0) + expired_refresh = make_refresh_token(exp_offset_s=-3600) auth.on_login("user@tb.io", "password", expired_token, expired_refresh) - new_token = _make_token(exp_offset_s=3600, iat_offset_s=0) - new_refresh = _make_refresh_token(exp_offset_s=86400) + new_token = make_token(exp_offset_s=3600, iat_offset_s=0) + new_refresh = make_refresh_token(exp_offset_s=86400) new_response_data = {"token": new_token, "refreshToken": new_refresh} with patch.object(auth, "_raw_post", return_value=new_response_data) as mock_post: @@ -221,14 +186,14 @@ def test_relogin_on_refresh_expiry(self): def test_refresh_failure_falls_back_to_relogin(self): """When refresh fails, _do_login is called as fallback (AUTH-04).""" - auth = _AuthManager("http://tb:9090", "jwt", None) + auth = _AuthManager("http://tb:9090") # expired access token, but valid refresh token so _do_refresh_token will be tried first - expired_token = _make_token(exp_offset_s=-7200, iat_offset_s=0) - valid_refresh = _make_refresh_token(exp_offset_s=86400) + expired_token = make_token(exp_offset_s=-7200, iat_offset_s=0) + valid_refresh = make_refresh_token(exp_offset_s=86400) auth.on_login("user@tb.io", "password", expired_token, valid_refresh) - new_token = _make_token(exp_offset_s=3600, iat_offset_s=0) - new_refresh = _make_refresh_token(exp_offset_s=86400) + new_token = make_token(exp_offset_s=3600, iat_offset_s=0) + new_refresh = make_refresh_token(exp_offset_s=86400) new_response_data = {"token": new_token, "refreshToken": new_refresh} call_count = [0] @@ -251,6 +216,216 @@ def side_effect(path, body): self.assertEqual(config.api_key["ApiKeyForm"], new_token) +# --------------------------------------------------------------------------- +# Concurrent refresh +# --------------------------------------------------------------------------- + + +def _raw_post_kwargs(auth): + """Run _raw_post against a mocked PoolManager and return the request kwargs.""" + response = MagicMock() + response.status = 200 + response.data = b'{"token": "t", "refreshToken": "r"}' + with patch("common._auth.urllib3.PoolManager") as mock_pool: + mock_pool.return_value.request.return_value = response + auth._raw_post("/api/auth/login", b"{}") + return mock_pool.return_value.request.call_args.kwargs + + +class TestRawPostBounds(unittest.TestCase): + """The auth round-trip is the one call every other thread waits on.""" + + def test_timeout_is_a_total_and_uses_the_configured_value(self): + """The ceiling is a Timeout(total=...), not a bare float. + + A bare float sets connect and read separately and leaves total unbounded, so a + slow-drip response would never hit the limit the constant advertises. + """ + kwargs = _raw_post_kwargs(_AuthManager("http://tb:9090")) + + timeout = kwargs.get("timeout") + self.assertIsInstance(timeout, urllib3.Timeout) + self.assertEqual(timeout.total, DEFAULT_AUTH_TIMEOUT_MS / 1000) + + def test_does_not_retry(self): + """urllib3 would otherwise apply Retry.DEFAULT (total=3). + + Its connection-error branch never consults allowed_methods, so this POST would + retry too and cost 4x the advertised ceiling before _do_login tries again. + """ + retries = _raw_post_kwargs(_AuthManager("http://tb:9090")).get("retries") + + self.assertIsInstance(retries, urllib3.Retry) + self.assertEqual( + (retries.total, retries.connect, retries.read, retries.status, retries.other), + (0, 0, 0, 0, 0), + ) + + def test_does_not_follow_redirects(self): + """The hop count is load-bearing, so pin the exact value like its neighbour. + + urllib3 gives each hop a fresh timeout budget, so any allowance multiplies the + ceiling; and a followed redirect re-sends username/password to whatever Location + names. See _AUTH_RETRIES. + """ + retries = _raw_post_kwargs(_AuthManager("http://tb:9090")).get("retries") + + # urllib3 normalises redirect=False to 0. + self.assertEqual(retries.redirect, 0) + # The half that redirect=0 would not give: without raise_on_redirect cleared, a + # 3xx raises MaxRetryError instead of reaching _raw_post's status check, and the + # remedy in that message never reaches the caller. .redirect alone cannot tell + # the two spellings apart, so this is the assertion that pins redirect=False. + self.assertIs(retries.raise_on_redirect, False) + + def test_timeout_is_configurable(self): + """auth_timeout_ms reaches the request, in seconds.""" + kwargs = _raw_post_kwargs(_AuthManager("http://tb:9090", auth_timeout_ms=1_500)) + + self.assertEqual(kwargs["timeout"].total, 1.5) + + def test_non_positive_timeout_rejected(self): + """Rejected at construction, not on the first refresh. + + urllib3.Timeout raises for a non-positive total, but only inside _raw_post, + where _do_refresh_token and _do_login catch Exception and log — so the client + would build fine and then silently stop refreshing. + """ + for bad in (0, -1): + with self.subTest(auth_timeout_ms=bad): + with self.assertRaisesRegex(ValueError, "auth_timeout_ms must be positive"): + _AuthManager("http://tb:9090", auth_timeout_ms=bad) + + +class _StubAuthServer: + """Local HTTP server answering one canned response, for real _raw_post calls. + + The kwargs tests above prove a setting reaches urllib3; these prove the behaviour + that setting exists for. + """ + + def __init__(self, status, headers=(), body=b""): + # Every path the server is asked for, so a test can assert on what was *not* + # requested — a followed redirect shows up here as a second entry. + self.requests = [] + requests = self.requests + + class Handler(http.server.BaseHTTPRequestHandler): + def do_POST(handler): + requests.append(handler.path) + handler.send_response(status) + for name, value in headers: + handler.send_header(name, value) + handler.send_header("Content-Length", str(len(body))) + handler.end_headers() + handler.wfile.write(body) + + def log_message(handler, *args): + pass + + self._server = http.server.HTTPServer(("127.0.0.1", 0), Handler) + self.url = f"http://127.0.0.1:{self._server.server_port}" + + def __enter__(self): + threading.Thread(target=self._server.serve_forever, daemon=True).start() + return self + + def __exit__(self, *exc): + self._server.shutdown() + self._server.server_close() + + +class TestRawPostAgainstAServer(unittest.TestCase): + def test_successful_login_is_parsed(self): + """The happy path works against a real socket, not just a mock.""" + with _StubAuthServer(200, body=b'{"token": "t", "refreshToken": "r"}') as server: + result = _AuthManager(server.url)._raw_post("/api/auth/login", b"{}") + + self.assertEqual(result, {"token": "t", "refreshToken": "r"}) + + def test_redirect_is_not_followed_and_says_why(self): + """A redirect surfaces as an error naming the remedy, rather than being followed. + + Following it would forward username/password to the redirect target, so this is + the deliberate behaviour rather than a gap — the message has to explain that. + + The request count is the load-bearing assertion: the message alone would still + match if a later change forwarded the body once and reported the second reply. + """ + with _StubAuthServer(307, headers=[("Location", "/elsewhere")]) as server: + auth = _AuthManager(server.url) + with self.assertRaisesRegex(RuntimeError, r"HTTP 307.*set url= to the redirect"): + auth._raw_post("/api/auth/login", b"{}") + + self.assertEqual(server.requests, ["/api/auth/login"], "the redirect was followed") + + +class TestConcurrentRefresh(unittest.TestCase): + def test_second_thread_waits_for_in_flight_refresh(self): + """A thread arriving mid-refresh blocks, then sends the refreshed token. + + Returning early instead would install the expired token the in-flight refresh + is replacing, and nothing retries the resulting 401 — _RetryingRESTClient only + handles 429 — so it would surface to the caller as a spurious ApiException. + """ + auth = _AuthManager("http://tb:9090") + auth.on_login( + "user@tb.io", + "password", + make_token(exp_offset_s=-3600), + make_refresh_token(exp_offset_s=86400), + ) + new_token = make_token(exp_offset_s=7200) + + refresh_started = threading.Event() + release_refresh = threading.Event() + posts = [] + + def blocking_post(path, _body): + posts.append(path) + refresh_started.set() + release_refresh.wait(timeout=5) + return {"token": new_token, "refreshToken": make_refresh_token(exp_offset_s=172800)} + + configs = {} + errors = [] + + def run_hook(name): + # An exception in a thread target only reaches stderr, which would surface + # here as a misleading "did not wait" — collect it and re-raise in the main + # thread instead. + try: + configs[name] = _mock_configuration() + auth.hook(configs[name]) + except BaseException as exc: # noqa: BLE001 - re-raised below + errors.append(exc) + + with patch.object(auth, "_raw_post", side_effect=blocking_post): + first = threading.Thread(target=run_hook, args=("first",)) + first.start() + self.assertTrue(refresh_started.wait(timeout=5), "first thread never refreshed") + + second = threading.Thread(target=run_hook, args=("second",)) + second.start() + second.join(timeout=0.2) + if errors: + raise errors[0] + self.assertTrue(second.is_alive(), "second thread did not wait for the refresh") + + release_refresh.set() + first.join(timeout=5) + second.join(timeout=5) + + if errors: + raise errors[0] + self.assertFalse(first.is_alive(), "refreshing thread never finished") + self.assertFalse(second.is_alive(), "waiting thread was never released") + # One round-trip, not one per waiting thread, and both threads send its result. + self.assertEqual(posts, ["/api/auth/token"]) + self.assertEqual(configs["first"].api_key["ApiKeyForm"], new_token) + self.assertEqual(configs["second"].api_key["ApiKeyForm"], new_token) + + # --------------------------------------------------------------------------- # AUTH-05: API key passthrough # --------------------------------------------------------------------------- @@ -259,7 +434,7 @@ def side_effect(path, body): class TestApiKeyAuthNoRefresh(unittest.TestCase): def test_api_key_auth_no_refresh(self): """hook() returns immediately for api_key auth without making HTTP calls.""" - auth = _AuthManager("http://tb:9090", "api_key", "test-key-12345") + auth = _AuthManager("http://tb:9090", "test-key-12345") with patch.object(auth, "_raw_post") as mock_post: config = _mock_configuration() @@ -278,10 +453,10 @@ def test_api_key_auth_no_refresh(self): class TestPreexistingToken(unittest.TestCase): def test_preexisting_token(self): """set_external_token parses exp times correctly from provided JWTs.""" - auth = _AuthManager("http://tb:9090", "jwt", None) + auth = _AuthManager("http://tb:9090") now_s = int(time.time()) - token = _make_jwt({"exp": now_s + 3600, "iat": now_s}) - refresh = _make_jwt({"exp": now_s + 86400}) + token = make_jwt({"exp": now_s + 3600, "iat": now_s}) + refresh = make_jwt({"exp": now_s + 86400}) auth.set_external_token(token, refresh) diff --git a/tests/test_client.py b/tests/test_client.py index 87b0a049..ad3f36f6 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1,21 +1,25 @@ """ Unit tests for ThingsboardClient — WRAP-01 through WRAP-04. -All tests import from tb_ce_client.client (after common/ is copied to ce/). -conftest.py adds ce/ to sys.path so this works without packaging. +All tests import from tb_ce_client.client — the committed overlay copy of +common/client.py. conftest.py adds ce/ to sys.path so this works without packaging. """ import unittest -from unittest.mock import MagicMock, patch +from unittest.mock import ANY, MagicMock, patch from tb_ce_client._retry import _RetryingRESTClient - -# conftest.py handles sys.path; this import will work once client.py is copied from tb_ce_client.client import ThingsboardClient from tb_ce_client.rest import RESTClientObject +from tests._jwt import make_refresh_token, make_token + URL = "http://tb-server:9090" +# Patch the login endpoint at its module path so the mock survives module eviction +# and re-import by test_split.py's lazy-load tests. +_LOGIN_PATCH_TARGET = "tb_ce_client.api.login_endpoint_api.LoginEndpointApi.login" + def _mock_login_response(token="test.jwt.token", refresh_token="test.jwt.refresh"): """Build a mock LoginResponse object.""" @@ -25,40 +29,214 @@ def _mock_login_response(token="test.jwt.token", refresh_token="test.jwt.refresh return resp -class TestThingsboardClientJWTLogin(unittest.TestCase): - """WRAP-01, AUTH-01 integration: username/password login flow.""" +def _logged_in_client(token="test.jwt.token", refresh_token="test.jwt.refresh"): + """Construct a ThingsboardClient through a mocked username/password login.""" + resp = _mock_login_response(token, refresh_token) + with patch(_LOGIN_PATCH_TARGET, return_value=resp): + return ThingsboardClient(URL, "user@tb.io", "pass123") + + +def _assert_header_slot(case, client, token, prefix): + """The X-Authorization slot holds this token, and emits it under this prefix. + + Reading auth_settings() runs the refresh hook, so these are the header name and + value an API request actually sends rather than a pure state inspection. + """ + cfg = client.api_client.configuration + case.assertEqual(cfg.api_key.get("ApiKeyForm"), token) + case.assertEqual(cfg.api_key_prefix.get("ApiKeyForm"), prefix) + emitted = cfg.auth_settings()["ApiKeyForm"] + case.assertEqual(emitted["key"], "X-Authorization") + case.assertEqual(emitted["value"], f"{prefix} {token}") + + +class TestThingsboardClientAuthModes(unittest.TestCase): + """WRAP-01, AUTH-01/05/06 integration: every auth mode, plus unauthenticated.""" def test_jwt_login(self): """ThingsboardClient(url, username, password) calls login() and stores tokens.""" mock_resp = _mock_login_response() - # Use module-path patch so the mock works even if the module was - # evicted and re-imported by test_split.py lazy-load tests. - with patch( - "tb_ce_client.api.login_endpoint_api.LoginEndpointApi.login", return_value=mock_resp - ) as mock_login: + with patch(_LOGIN_PATCH_TARGET, return_value=mock_resp) as mock_login: client = ThingsboardClient(URL, "user@tb.io", "pass123") mock_login.assert_called_once() # Token stored in auth manager self.assertEqual(client.get_token(), mock_resp.token) + def test_jwt_login_emits_x_authorization_header(self): + """AUTH-01: auth_settings() yields the header an API request actually sends. + + Covers the username/password mode, the one path that calls /api/auth/login. + """ + _assert_header_slot(self, _logged_in_client(), "test.jwt.token", "Bearer") + + def test_jwt_header_follows_token_rotation(self): + """AUTH-02: seeding at login does not freeze the first token. + + Covers the whole seed -> hook -> refresh chain: the client logs in with an + already-expired access token, and reading auth_settings() drives the hook + through a real /api/auth/token refresh before the header is assembled. + """ + client = _logged_in_client( + token=make_token(exp_offset_s=-3600), + refresh_token=make_refresh_token(exp_offset_s=86400), + ) + rotated = make_token(exp_offset_s=3600) + refreshed = {"token": rotated, "refreshToken": make_refresh_token(exp_offset_s=172800)} + with patch.object(client._auth_manager, "_raw_post", return_value=refreshed) as mock_post: + auth = client.api_client.configuration.auth_settings() + mock_post.assert_called_once_with("/api/auth/token", ANY) + self.assertEqual(auth["ApiKeyForm"]["value"], f"Bearer {rotated}") + def test_api_key_auth(self): """WRAP-01, AUTH-05: api_key sets header without calling login().""" - with patch("tb_ce_client.api.login_endpoint_api.LoginEndpointApi.login") as mock_login: + with patch(_LOGIN_PATCH_TARGET) as mock_login: client = ThingsboardClient(URL, api_key="test-key") mock_login.assert_not_called() - cfg = client.api_client.configuration - self.assertEqual(cfg.api_key.get("ApiKeyForm"), "test-key") - self.assertEqual(cfg.api_key_prefix.get("ApiKeyForm"), "ApiKey") + _assert_header_slot(self, client, "test-key", "ApiKey") def test_preexisting_token(self): """WRAP-01, AUTH-06: pre-existing token sets header without login().""" - with patch("tb_ce_client.api.login_endpoint_api.LoginEndpointApi.login") as mock_login: + with patch(_LOGIN_PATCH_TARGET) as mock_login: client = ThingsboardClient( - URL, token="jwt.payload.sig", refresh_token="jwt.payload.sig" + URL, token="jwt.payload.sig", refresh_token="jwt.refresh.sig" ) mock_login.assert_not_called() + _assert_header_slot(self, client, "jwt.payload.sig", "Bearer") + self.assertEqual(client.get_refresh_token(), "jwt.refresh.sig") + + def test_preexisting_token_without_refresh_token(self): + """token= alone is valid — the token is simply never refreshed. + + The mutual-exclusion and companion checks deliberately do not pair token= with + refresh_token=, so this pins the asymmetry the docstring describes. The refresh + token is None rather than "", matching get_refresh_token()'s documented contract. + """ + with patch(_LOGIN_PATCH_TARGET) as mock_login: + client = ThingsboardClient(URL, token="jwt.payload.sig") + mock_login.assert_not_called() + _assert_header_slot(self, client, "jwt.payload.sig", "Bearer") + self.assertIsNone(client.get_refresh_token()) + + def test_no_auth_leaves_header_slot_absent(self): + """A client built without auth kwargs creates no ApiKeyForm slot. + + Legitimate for the /api/noauth endpoints: construction must not raise, and + auth_settings() must stay empty so no X-Authorization header is sent. + """ + with patch(_LOGIN_PATCH_TARGET) as mock_login: + client = ThingsboardClient(URL) + mock_login.assert_not_called() cfg = client.api_client.configuration - self.assertEqual(cfg.api_key.get("ApiKeyForm"), "jwt.payload.sig") + self.assertNotIn("ApiKeyForm", cfg.api_key) + self.assertEqual(cfg.auth_settings(), {}) + + +class TestThingsboardClientAuthArgValidation(unittest.TestCase): + """Auth argument combinations that cannot be honoured are rejected in __init__. + + Cases that pass username= patch the login endpoint and assert it was not called, + which is what proves validation runs before the network. The rest cannot reach + login at any point, so the raise is the whole assertion. + """ + + def test_api_key_with_username_rejected(self): + """api_key= plus username= raises before any login call is made. + + Allowing both would install a JWT under api_key auth, where the refresh + hook is a no-op — the token would be frozen and every request would start + failing with 401 once it expired. + """ + with patch(_LOGIN_PATCH_TARGET) as mock_login: + with self.assertRaisesRegex(ValueError, "username=, api_key="): + ThingsboardClient(URL, "user@tb.io", "pass123", api_key="test-key") + mock_login.assert_not_called() + + def test_api_key_with_token_rejected(self): + """api_key= plus token= raises.""" + with self.assertRaisesRegex(ValueError, "api_key=, token="): + ThingsboardClient(URL, api_key="test-key", token="jwt.payload.sig") + + def test_username_with_token_rejected(self): + """username= plus token= raises.""" + with patch(_LOGIN_PATCH_TARGET) as mock_login: + with self.assertRaisesRegex(ValueError, "username=, token="): + ThingsboardClient(URL, "user@tb.io", "pass123", token="jwt.payload.sig") + mock_login.assert_not_called() + + def test_all_three_modes_rejected(self): + """All three at once raises and the message names every colliding mode.""" + with patch(_LOGIN_PATCH_TARGET) as mock_login: + with self.assertRaisesRegex(ValueError, "username=, api_key=, token="): + ThingsboardClient( + URL, "user@tb.io", "pass123", api_key="test-key", token="jwt.payload.sig" + ) + mock_login.assert_not_called() + + def test_mutual_exclusion_message_says_what_to_do(self): + """The message names the fix, not only the collision.""" + with self.assertRaisesRegex( + ValueError, r"Pass exactly one of username=, api_key= or token=\." + ): + ThingsboardClient(URL, api_key="test-key", token="jwt.payload.sig") + + def test_password_without_username_rejected(self): + """password= alone would be silently dropped, so it raises instead.""" + with self.assertRaisesRegex(ValueError, "password= requires username="): + ThingsboardClient(URL, password="pass123") + + def test_refresh_token_without_token_rejected(self): + """refresh_token= alone would be silently dropped, so it raises instead.""" + with self.assertRaisesRegex(ValueError, "refresh_token= requires token="): + ThingsboardClient(URL, refresh_token="jwt.payload.sig") + + def test_username_without_password_rejected(self): + """username= alone raises here rather than as a pydantic error from LoginRequest.""" + with patch(_LOGIN_PATCH_TARGET) as mock_login: + with self.assertRaisesRegex(ValueError, "username= requires password="): + ThingsboardClient(URL, "user@tb.io") + mock_login.assert_not_called() + + def test_empty_auth_arguments_rejected(self): + """Every auth argument rejects "", which would otherwise install no header. + + The empty check runs ahead of the companion rules, so username="" reports the + empty argument rather than falling through to "username= requires password=". + Covering all five means reordering the checks fails here instead of silently + changing which message a caller sees. + """ + cases = ( + ("username", {"username": "", "password": "pass123"}), + ("password", {"username": "user@tb.io", "password": ""}), + ("api_key", {"api_key": ""}), + ("token", {"token": ""}), + ("refresh_token", {"token": "jwt.payload.sig", "refresh_token": ""}), + ) + for name, kwargs in cases: + with self.subTest(argument=name): + with patch(_LOGIN_PATCH_TARGET) as mock_login: + with self.assertRaisesRegex(ValueError, f"{name}= must not be empty"): + ThingsboardClient(URL, **kwargs) + mock_login.assert_not_called() + + def test_non_positive_auth_timeout_rejected(self): + """auth_timeout_ms is validated through the public constructor too. + + Rejection happens before the eager login, so a bad value cannot reach the + network — the same guarantee the other cases in this class pin. + """ + for bad in (0, -1): + with self.subTest(auth_timeout_ms=bad): + with patch(_LOGIN_PATCH_TARGET) as mock_login: + with self.assertRaisesRegex(ValueError, "auth_timeout_ms must be positive"): + ThingsboardClient(URL, "user@tb.io", "pass123", auth_timeout_ms=bad) + mock_login.assert_not_called() + + def test_empty_argument_message_says_what_to_do(self): + """The empty-argument message carries a remedy, like the mutual-exclusion one.""" + with self.assertRaisesRegex( + ValueError, r"pass a value, or omit all auth arguments for an unauthenticated" + ): + ThingsboardClient(URL, api_key="") class TestThingsboardClientStructure(unittest.TestCase): @@ -143,20 +321,12 @@ def test_get_token_api_key(self): def test_get_token_jwt(self): """get_token() returns the JWT after successful login.""" - mock_resp = _mock_login_response(token="access.jwt.here") - with patch( - "tb_ce_client.api.login_endpoint_api.LoginEndpointApi.login", return_value=mock_resp - ): - client = ThingsboardClient(URL, "u", "p") + client = _logged_in_client(token="access.jwt.here") self.assertEqual(client.get_token(), "access.jwt.here") def test_get_refresh_token_jwt(self): """get_refresh_token() returns the refresh JWT after login.""" - mock_resp = _mock_login_response(refresh_token="refresh.jwt.here") - with patch( - "tb_ce_client.api.login_endpoint_api.LoginEndpointApi.login", return_value=mock_resp - ): - client = ThingsboardClient(URL, "u", "p") + client = _logged_in_client(refresh_token="refresh.jwt.here") self.assertEqual(client.get_refresh_token(), "refresh.jwt.here") diff --git a/tests/test_common_overlay.py b/tests/test_common_overlay.py new file mode 100644 index 00000000..1d742aec --- /dev/null +++ b/tests/test_common_overlay.py @@ -0,0 +1,235 @@ +""" +Guards the common/ -> edition overlays performed by generate-client.sh. + +generate-client.sh copies common/ verbatim into every tb__client/ package and +common/docs/ into every /docs/, and those copies are committed. Nothing else in +CI compares them, so a fix landed in common/ but overlaid into only some editions would +ship stale code — or stale documentation — to the rest. + +Every list is taken from the thing that defines it rather than hardcoded here: +filenames from common/ itself, editions from editions.txt — the same file +generate-client.sh reads. Adding either a file or an edition extends the check with no +test edit — and, because the editions come from that shared list rather than from +whichever directories happen to exist, an edition whose package directory is missing +fails instead of quietly dropping out. +""" + +import subprocess +from pathlib import Path + +import pytest + +_REPO_ROOT = Path(__file__).parent.parent +_COMMON_DIR = _REPO_ROOT / "common" +_EDITIONS_FILE = _REPO_ROOT / "editions.txt" + +# generate-client.sh overlays common/docs into /docs rather than into the +# package, so the package check excludes it and the docs check reads from it. +_DOCS_DIRNAME = "docs" + +# Excluded only where they sit at the top level of common/ — a nested file of the +# same name would still be overlaid verbatim and must stay checked: +# docs — generate-client.sh overlays it into /docs, not the package +# __init__.py — post_process.py merges it with the generated package __init__ +_EXCLUDED_TOP_LEVEL = {_DOCS_DIRNAME, "__init__.py"} + +# Excluded at any depth, because they are build output that is never committed: +_EXCLUDED_DIRS_ANY_DEPTH = {"__pycache__"} + + +def _overlaid_filenames(root: Path = _COMMON_DIR) -> list[str]: + """Paths under root that must appear verbatim in every edition package. + + Walks recursively and returns forward-slash paths relative to root, because + generate-client.sh `cp -r`s every entry — subdirectories included. + """ + names = [] + for path in root.rglob("*"): + if not path.is_file(): + continue + rel = path.relative_to(root) + if rel.parts[0] in _EXCLUDED_TOP_LEVEL: + continue + if _EXCLUDED_DIRS_ANY_DEPTH.intersection(rel.parts): + continue + names.append(rel.as_posix()) + return sorted(names) + + +def _overlaid_doc_filenames(root: Path = _COMMON_DIR / _DOCS_DIRNAME) -> list[str]: + """Names directly under root that must appear verbatim in every /docs/. + + Flat rather than recursive: the script overlays these with `cp common/docs/* ...`, + which copies top-level entries only — and would abort on a subdirectory, since it + passes no -r. + + A missing directory yields an empty list rather than raising, matching what rglob + does for _overlaid_filenames — see test_missing_directory_yields_empty_list. + """ + if not root.is_dir(): + return [] + return sorted(p.name for p in root.iterdir() if p.is_file()) + + +def _editions(path: Path = _EDITIONS_FILE) -> list[str]: + """Edition names read from editions.txt — the same file generate-client.sh reads. + + One name per line; blank lines and # comments ignored, matching the read loop in + generate-client.sh. That script's --list-editions is the canonical parser; this is a + mirror of it, kept rather than shelled out to because it runs at collection time from + the parametrize decorators below — where a subprocess would be paid on every run, and + would make collection depend on bash and on the script succeeding. + test_editions_parsing_matches_the_script holds the two together. + + A missing file yields an empty list rather than raising, matching the two walk + helpers: this runs at collection time from the parametrize decorators below, so + raising here would take the unrelated package-sync cases down with it. + """ + if not path.is_file(): + return [] + lines = path.read_text(encoding="utf-8").splitlines() + return sorted(s for line in lines if (s := line.strip()) and not s.startswith("#")) + + +def test_discovery_finds_filenames_and_editions(): + """Every derived list is non-empty. + + Without this, a glob or regex that silently matched nothing would collect zero + parametrized cases and the sync check would vacuously pass. + """ + assert _overlaid_filenames(), "no overlaid files discovered in common/" + assert _overlaid_doc_filenames(), "no overlaid docs discovered in common/docs/" + assert _editions(), f"no editions listed in {_EDITIONS_FILE.name}" + + +def _assert_identical(source: Path, copy: Path, destinations: str) -> None: + """Assert copy exists and is byte-identical to source, or explain how to fix it. + + destinations names where the source has to be copied to, e.g. "every tb_*_client/ + package" — the rest of the remediation is the same for both callers. + """ + source_rel = source.relative_to(_REPO_ROOT).as_posix() + copy_rel = copy.relative_to(_REPO_ROOT).as_posix() + # Shared with the missing-copy branch, where the source is fine and re-running the + # script is the only step needed — hence the single action and no "after editing it". + remediation = f"Run generate-client.sh (or copy {source_rel} into {destinations})." + + assert copy.is_file(), f"{copy_rel} is missing. {remediation}" + assert copy.read_bytes() == source.read_bytes(), ( + f"{copy_rel} is out of sync with {source_rel}. {remediation}" + ) + + +def test_walk_exclusion_semantics(tmp_path): + """The two exclusion sets are anchored differently — pin that against a fixture. + + common/ is flat today apart from docs/, so nothing real exercises the recursion + or the any-depth filter; this checks them now rather than the first time someone + adds a subdirectory. + """ + (tmp_path / "client.py").write_text("x") + (tmp_path / "__init__.py").write_text("x") # excluded: top level + (tmp_path / "docs").mkdir() + (tmp_path / "docs" / "guide.md").write_text("x") # excluded: under top-level docs + (tmp_path / "sub").mkdir() + (tmp_path / "sub" / "mod.py").write_text("x") # kept: nested file + (tmp_path / "sub" / "__init__.py").write_text("x") # kept: not at the top level + (tmp_path / "sub" / "docs").mkdir() + (tmp_path / "sub" / "docs" / "guide.md").write_text("x") # kept: not at the top level + (tmp_path / "sub" / "__pycache__").mkdir() + (tmp_path / "sub" / "__pycache__" / "mod.pyc").write_bytes(b"x") # excluded: any depth + + assert _overlaid_filenames(tmp_path) == [ + "client.py", + "sub/__init__.py", + "sub/docs/guide.md", + "sub/mod.py", + ] + + +# Lines chosen so that a parser disagreeing with the script's fails: interior +# whitespace separates `tr -d [:space:]` from str.strip(), a trailing comment separates +# a naive '#' strip from a whole-line one, and the file ends without a newline. +_EDITIONS_FIXTURE = "# a comment\n\nce\n pe \n\n# not shipped yet\npa as\nce # note" + + +def test_editions_parsing_matches_the_script(tmp_path): + """The Python parser agrees with generate-client.sh on the same file. + + Runs the script rather than a restatement of its rules — the two implementations + are the thing at risk of drifting, so pinning only the Python side would let a + divergence sit here undetected. + """ + listing = tmp_path / "editions.txt" + listing.write_text(_EDITIONS_FIXTURE, encoding="utf-8") + script = tmp_path / "generate-client.sh" + script.write_bytes((_REPO_ROOT / "generate-client.sh").read_bytes()) + script.chmod(0o755) + + result = subprocess.run( + ["bash", str(script), "--list-editions"], + capture_output=True, + text=True, + check=True, + ) + from_script = sorted(result.stdout.splitlines()) + + assert from_script == _editions(listing) + # Spelled out too, so a change that broke both sides identically still fails. + assert from_script == ["ce", "ce # note", "pa as", "pe"] + + +def test_editions_missing_file_yields_empty_list(tmp_path): + """A lost editions.txt is reported by the discovery test, not by a collection error.""" + assert _editions(tmp_path / "editions.txt") == [] + + +def test_doc_walk_is_flat(tmp_path): + """The docs helper takes top-level files only, matching `cp common/docs/*`. + + Kept deliberately flat because the script passes no -r; a subdirectory would abort + it, so silently skipping one here is the right behaviour rather than an oversight. + """ + (tmp_path / "tb-examples.md").write_text("x") + (tmp_path / "sub").mkdir() + (tmp_path / "sub" / "nested.md").write_text("x") # skipped: not a top-level file + + assert _overlaid_doc_filenames(tmp_path) == ["tb-examples.md"] + + +@pytest.mark.parametrize("walk", (_overlaid_filenames, _overlaid_doc_filenames)) +def test_missing_directory_yields_empty_list(walk, tmp_path): + """Both helpers degrade to [] rather than raising when their root is absent. + + An absent root is then reported by test_discovery_finds_filenames_and_editions as + a plain failure, instead of a collection-time error that would take the unrelated + package-sync cases down with it. + """ + assert walk(tmp_path / "missing") == [] + + +@pytest.mark.parametrize("edition", _editions()) +@pytest.mark.parametrize("filename", _overlaid_filenames()) +def test_edition_copy_matches_common(edition, filename): + """Each committed edition copy is byte-identical to its common/ source.""" + _assert_identical( + _COMMON_DIR / filename, + _REPO_ROOT / edition / f"tb_{edition}_client" / filename, + "every tb_*_client/ package", + ) + + +@pytest.mark.parametrize("edition", _editions()) +@pytest.mark.parametrize("filename", _overlaid_doc_filenames()) +def test_edition_doc_copy_matches_common(edition, filename): + """Each committed /docs/ copy is byte-identical to its common/docs/ source. + + The package overlay above skips common/docs because the script sends it to + /docs instead; without this, a hand-edit to one edition's copy of the + shared documentation would pass CI unnoticed. + """ + _assert_identical( + _COMMON_DIR / _DOCS_DIRNAME / filename, + _REPO_ROOT / edition / _DOCS_DIRNAME / filename, + f"every /{_DOCS_DIRNAME}/ directory", + ) diff --git a/tests/test_facade.py b/tests/test_facade.py index 8a739fd4..bc53abb0 100644 --- a/tests/test_facade.py +++ b/tests/test_facade.py @@ -38,16 +38,10 @@ if _CE_DIR not in sys.path: sys.path.insert(0, _CE_DIR) -# Also copy common/client.py to ce/tb_ce_client/client.py if it's missing __getattr__ -# (after implementation the copy happens at test time to avoid stale state) -import shutil +# Do not overlay common/client.py onto the ce copy here: the committed copy is what +# users install, and repairing it at import time would defeat tests/test_common_overlay.py. -_COMMON_CLIENT = _REPO_ROOT / "common" / "client.py" -_CE_CLIENT = _CE_PKG_DIR / "client.py" -if _COMMON_CLIENT.exists(): - shutil.copy2(str(_COMMON_CLIENT), str(_CE_CLIENT)) - -# Evict any stale tb_ce_client imports so the updated client.py is picked up +# Evict any stale tb_ce_client imports so the committed client.py is picked up for mod_name in list(sys.modules.keys()): if mod_name == "tb_ce_client" or mod_name.startswith("tb_ce_client."): del sys.modules[mod_name] diff --git a/tests/test_readme.py b/tests/test_readme.py index 8cec78de..0bf1084f 100644 --- a/tests/test_readme.py +++ b/tests/test_readme.py @@ -1,18 +1,31 @@ """ -Tests validating README.md and ce/docs/tb-examples.md content. +Tests validating README.md and common/docs/tb-examples.md content. Validates: - README.md existence, quickstart section, code block syntax (DOC-01) - README.md uses keyword constructor form: username=... (DOC-01) -- ce/docs/tb-examples.md existence, required sections, code block syntax (DOC-04) -- ce/docs/tb-examples.md uses keyword constructor form: username=... (DOC-04) +- common/docs/tb-examples.md existence, required sections, code block syntax (DOC-04) +- common/docs/tb-examples.md uses keyword constructor form: username=... (DOC-04) + +The examples are checked at their source in common/docs/ rather than in one edition's +copy: that is the file people edit, and test_common_overlay.py already proves every +/docs/ copy is byte-identical to it, so every edition is covered here. + +Checks that apply to both documents are parametrized over DOCUMENTS rather than written +twice, so a rule added for one cannot silently miss the other. """ import ast import re from pathlib import Path +import pytest + REPO_ROOT = Path(__file__).parent.parent +README = REPO_ROOT / "README.md" +TB_EXAMPLES = REPO_ROOT / "common" / "docs" / "tb-examples.md" + +DOCUMENTS = (README, TB_EXAMPLES) # --------------------------------------------------------------------------- @@ -20,16 +33,48 @@ # --------------------------------------------------------------------------- -def _extract_python_blocks(text: str) -> list: - """Return list of Python code block contents from a markdown string. +def _label(path: Path) -> str: + """Repo-relative name for a document, used for both test ids and messages. + + Falls back to the full path for the tmp_path fixtures the helper tests use. + """ + if path.is_relative_to(REPO_ROOT): + return path.relative_to(REPO_ROOT).as_posix() + return str(path) + + +def _read(path: Path) -> str: + return path.read_text(encoding="utf-8") + + +def _has_heading(content: str, heading: str) -> bool: + """Whether content contains heading as a whole line of its own. + + Anchored and case-sensitive, so the match means what the caller spells: a + demotion to '### ...', a change of capitalization, or the heading appearing + mid-line all count as absent. A bare substring would survive deleting the very + section it is meant to guard, since these terms recur in prose and code samples. + """ + return re.search(rf"^{re.escape(heading)}[ \t]*$", content, re.MULTILINE) is not None + + +def _assert_heading(path: Path, heading: str) -> None: + """Assert the document contains heading as a whole line.""" + assert _has_heading(_read(path), heading), f"{_label(path)} missing '{heading}' section" + - Finds all fenced code blocks marked with ```python ... ``` and returns - the text between the fences (excluding the fence lines themselves). +def _require_python_blocks(path: Path) -> list[str]: + """Return the document's ```python blocks, failing if it has none. + + The non-empty guard keeps a document that lost all its code samples from + vacuously satisfying the rules applied to those samples. """ - return re.findall(r"```python\n(.*?)```", text, re.DOTALL) + blocks = re.findall(r"```python\n(.*?)```", _read(path), re.DOTALL) + assert blocks, f"{_label(path)} has no Python code blocks" + return blocks -def _validate_python_syntax(blocks: list) -> list: +def _validate_python_syntax(blocks: list[str]) -> list: """Validate Python syntax for each block using ast.parse. Returns a list of (block_index, error_message) tuples for any blocks @@ -45,120 +90,95 @@ def _validate_python_syntax(blocks: list) -> list: # --------------------------------------------------------------------------- -# DOC-01: README.md tests +# The heading matcher both documents' section checks rely on # --------------------------------------------------------------------------- -def test_readme_exists(): - """README.md exists at the repository root.""" - readme = REPO_ROOT / "README.md" - assert readme.is_file(), f"README.md does not exist at {readme}" +@pytest.mark.parametrize( + "line, expected", + ( + ("## JWT Login", True), + ("### JWT Login", False), # demoted + ("## jwt login", False), # recased + ("x ## JWT Login", False), # not at the start of a line + ("## JWT Login extra", False), # not the whole line + ), +) +def test_has_heading_is_anchored_and_case_sensitive(line, expected): + """Pin the matcher's strictness, which the section checks assert nothing about.""" + assert _has_heading(f"intro\n{line}\nbody\n", "## JWT Login") is expected -def test_readme_has_quickstart(): - """README.md contains quickstart section with install, client, and error handling.""" - readme = REPO_ROOT / "README.md" - assert readme.is_file(), "README.md does not exist" - content = readme.read_text(encoding="utf-8") +def test_require_python_blocks_fails_without_any(tmp_path): + """A document that lost all its ```python fences fails instead of passing vacuously.""" + doc = tmp_path / "no-blocks.md" + doc.write_text("# Title\n\nProse only.\n", encoding="utf-8") + with pytest.raises(AssertionError, match="no Python code blocks"): + _require_python_blocks(doc) - assert "## Quickstart" in content, "README.md missing '## Quickstart' section heading" - assert "pip install" in content, "README.md missing 'pip install' instruction" - assert "ThingsboardClient" in content, "README.md missing 'ThingsboardClient' class name" - assert "ApiException" in content, "README.md missing 'ApiException' error handling" + +# --------------------------------------------------------------------------- +# DOC-01 / DOC-04: checks that apply to both documents +# --------------------------------------------------------------------------- -def test_readme_code_blocks_valid_python(): - """All Python code blocks in README.md are syntactically valid.""" - readme = REPO_ROOT / "README.md" - assert readme.is_file(), "README.md does not exist" - content = readme.read_text(encoding="utf-8") +@pytest.mark.parametrize("path", DOCUMENTS, ids=_label) +def test_document_exists(path): + """The document exists where the other tests expect to find it.""" + assert path.is_file(), f"{_label(path)} does not exist at {path}" - blocks = _extract_python_blocks(content) - assert blocks, "README.md has no Python code blocks" - errors = _validate_python_syntax(blocks) - assert not errors, "README.md has Python code blocks with syntax errors:\n" + "\n".join( +@pytest.mark.parametrize("path", DOCUMENTS, ids=_label) +def test_document_code_blocks_valid_python(path): + """All Python code blocks in the document are syntactically valid.""" + errors = _validate_python_syntax(_require_python_blocks(path)) + assert not errors, f"{_label(path)} has Python code blocks with syntax errors:\n" + "\n".join( f" Block {i}: {msg}" for i, msg in errors ) -def test_readme_uses_keyword_constructor(): - """README.md Python code blocks use keyword argument form (username=...).""" - readme = REPO_ROOT / "README.md" - assert readme.is_file(), "README.md does not exist" - content = readme.read_text(encoding="utf-8") - - blocks = _extract_python_blocks(content) - assert blocks, "README.md has no Python code blocks" - - has_keyword_form = any("username=" in block for block in blocks) +@pytest.mark.parametrize("path", DOCUMENTS, ids=_label) +def test_document_uses_keyword_constructor(path): + """The document's Python code blocks use keyword argument form (username=...).""" + has_keyword_form = any("username=" in block for block in _require_python_blocks(path)) assert has_keyword_form, ( - "README.md has no Python code block containing 'username=' " + f"{_label(path)} has no Python code block containing 'username=' " "(must use keyword argument form, not positional)" ) # --------------------------------------------------------------------------- -# DOC-04: ce/docs/tb-examples.md tests +# DOC-01: README.md only # --------------------------------------------------------------------------- -def test_tb_examples_exists(): - """ce/docs/tb-examples.md exists.""" - examples = REPO_ROOT / "ce" / "docs" / "tb-examples.md" - assert examples.is_file(), f"ce/docs/tb-examples.md does not exist at {examples}" - - -def test_tb_examples_required_sections(): - """ce/docs/tb-examples.md contains all required operation sections.""" - examples = REPO_ROOT / "ce" / "docs" / "tb-examples.md" - assert examples.is_file(), "ce/docs/tb-examples.md does not exist" - content = examples.read_text(encoding="utf-8") - lower = content.lower() - - assert "jwt" in lower and "login" in lower, ( - "tb-examples.md missing JWT login section (must contain 'jwt' and 'login')" - ) - assert "api key" in lower or "api_key" in lower, ( - "tb-examples.md missing API key login section (must contain 'api key' or 'api_key')" - ) - assert "device" in lower, "tb-examples.md missing device section (must contain 'device')" - assert "telemetry" in lower, ( - "tb-examples.md missing telemetry section (must contain 'telemetry')" - ) - assert "alarm" in lower, "tb-examples.md missing alarm section (must contain 'alarm')" - assert "with " in lower or "context manager" in lower, ( - "tb-examples.md missing with-statement section (must contain 'with ' or 'context manager')" - ) - - -def test_tb_examples_code_blocks_valid_python(): - """All Python code blocks in ce/docs/tb-examples.md are syntactically valid.""" - examples = REPO_ROOT / "ce" / "docs" / "tb-examples.md" - assert examples.is_file(), "ce/docs/tb-examples.md does not exist" - content = examples.read_text(encoding="utf-8") - - blocks = _extract_python_blocks(content) - assert blocks, "ce/docs/tb-examples.md has no Python code blocks" - - errors = _validate_python_syntax(blocks) - assert not errors, ( - "ce/docs/tb-examples.md has Python code blocks with syntax errors:\n" - + "\n".join(f" Block {i}: {msg}" for i, msg in errors) - ) +def test_readme_has_quickstart(): + """README.md contains quickstart section with install, client, and error handling.""" + _assert_heading(README, "## Quickstart") + content = _read(README) + assert "pip install" in content, "README.md missing 'pip install' instruction" + assert "ThingsboardClient" in content, "README.md missing 'ThingsboardClient' class name" + assert "ApiException" in content, "README.md missing 'ApiException' error handling" -def test_tb_examples_uses_keyword_constructor(): - """ce/docs/tb-examples.md Python code blocks use keyword argument form (username=...).""" - examples = REPO_ROOT / "ce" / "docs" / "tb-examples.md" - assert examples.is_file(), "ce/docs/tb-examples.md does not exist" - content = examples.read_text(encoding="utf-8") - blocks = _extract_python_blocks(content) - assert blocks, "ce/docs/tb-examples.md has no Python code blocks" +# --------------------------------------------------------------------------- +# DOC-04: common/docs/tb-examples.md only +# --------------------------------------------------------------------------- - has_keyword_form = any("username=" in block for block in blocks) - assert has_keyword_form, ( - "ce/docs/tb-examples.md has no Python code block containing 'username=' " - "(must use keyword argument form, not positional)" - ) +_REQUIRED_HEADINGS = ( + "## JWT Login", + "## API Key Login", + "## Pre-existing Token", + "## No Authentication", + "## Context Manager", + "## List Devices", + "## Push Telemetry", + "## List Alarms", +) + + +@pytest.mark.parametrize("heading", _REQUIRED_HEADINGS) +def test_tb_examples_required_sections(heading): + """common/docs/tb-examples.md contains each required section heading.""" + _assert_heading(TB_EXAMPLES, heading)