feat: add OAuth support for cloud APIs

[zerzhang]

Human responsible: @zerzhang.

Summary

• Add helpers for building SwitchBot OAuth authorization URLs and exchanging authorization codes.
• Support access-token-based cloud device discovery and encryption-key retrieval.
• Validate provider responses while keeping authorization codes, access tokens, refresh tokens, and encryption keys out of logs.
• Preserve authentication and API error types through device and encryption-key requests.
• Document the public-client OAuth contract and caller responsibilities.

OAuth wire contract

• The caller supplies a SwitchBot-issued client ID and its registered redirect URI; pySwitchbot does not embed Home Assistant credentials.
• This is a public-client flow. Open-source consumers cannot keep a client secret, so the token exchange intentionally does not send one.
• SwitchBot's current authorization server does not support PKCE. Callers must still generate, store, and validate a single-use state; state does not replace PKCE.
• The returned access token is sent directly in the authorization header expected by the SwitchBot internal account endpoints.
• The current Home Assistant setup flow consumes the token transiently and does not persist it, so this change does not add an unverified refresh request helper.

Error semantics

HTTP 401 and 403 responses now surface as SwitchbotAuthenticationError, including on the pre-existing password flow. Other provider API failures surface as SwitchbotApiError; transport and service-availability failures use SwitchbotAccountConnectionError.

Testing

• poetry run pytest --cov=switchbot tests
• 1403 passed
• switchbot/oauth.py and the new shared request-ID helper are fully covered

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines	Coverage Δ
switchbot/__init__.py	100.00% <100.00%> (ø)
switchbot/devices/device.py	75.24% <100.00%> (+5.33%)	⬆️
switchbot/oauth.py	100.00% <100.00%> (ø)
switchbot/utils.py	100.00% <100.00%> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

Warning

Important issues found.

• Token exchange sends no client authentication and no PKCE — and the helper signature can't express either
• Unknown-model diagnostic loses the payload that exists to add new device support

[zerzhang]

Thanks for the detailed review. I addressed the diagnostic and error-handling findings in 2d7dbce and clarified the OAuth contract in the README and PR description.

On the OAuth wire contract: there is not currently a public SwitchBot OAuth document I can link. The implementation follows the SwitchBot-provisioned public-client flow used by the current consumer. client_id and the registered redirect_uri are supplied by the caller and are not embedded in pySwitchbot. Because the consumers are open source, a client_secret cannot be kept confidential and is intentionally not sent. SwitchBot's current authorization server also does not support PKCE, so I documented that limitation rather than exposing parameters the server cannot use. The caller must generate and validate a single-use state; the README explicitly notes that state does not replace PKCE.

Changes made from the review:

• Restored the original full unknown-model payload and unmasked MAC diagnostic.
• Preserved SwitchbotAuthenticationError through the later device-list and encryption-key requests.
• Preserved SwitchbotApiError from the key endpoint and classify malformed communicationKey responses as API errors instead of connection errors.
• Documented the 401/403 exception-type change for the existing password flow.
• Parse only bounded error and error_description fields from OAuth failures, redact an echoed authorization code, and never log the raw response.
• Moved the duplicated request-ID extraction into a shared helper.
• Relaxed exact debug-message assertions while retaining the negative secret-leakage assertions.
• Added tests for the new authentication, API, connection, malformed-response, and OAuth error paths.

The token returned by this flow is expected directly in the SwitchBot internal account endpoints' authorization header, without a Bearer prefix. The current Home Assistant setup flow uses the token transiently and does not persist it. I therefore did not add a refresh helper without a verified refresh wire contract.

Local result: poetry run pytest --cov=switchbot tests — 1403 passed; the OAuth module and shared request-ID helper are fully covered.

[bluetoothbot]

@zerzhang: Thanks for the detailed review. I addressed the diagnostic and error-handling findings in 2d7dbce ...

Verified 2d7dbce against head. Unknown-model payload restored, device.py:404-411. Auth passthrough now covers getdevice and key endpoint. Request-ID extraction deduped into utils.extract_request_id. Malformed communicationKey correctly classified.

PKCE point accepted as a documented server limitation, not re-raised as blocking. Residual risk stays real: an intercepted code remains redeemable by anyone. Worth documenting that redirect_uri must be an exact HTTPS endpoint, no wildcards.

Two follow-ups from this pass. README line 89 still quotes Failed to retrieve encryption key from SwitchBot Account: ...; that string no longer appears for statusCode 190 now that SwitchbotApiError propagates raw. And every non-429 4xx from the token endpoint maps to SwitchbotAuthenticationError, so a redirect_uri mismatch drives a reauth loop rather than surfacing as a config error.

[bluetoothbot]

PR Review — feat: add OAuth support for cloud APIs

All previously blocking findings are resolved or dismissed by the author; what remains is documentation drift and error-taxonomy polish. Merge-ready.

Verified fixed since the last review: the unknown-model diagnostic is back to logging the full item payload with an unmasked MAC (device.py:404-411), the SwitchbotAuthenticationError passthrough now also guards the getdevice call (device.py:371) and the key endpoint (device.py:1208), the duplicated request-ID extraction collapsed into utils.extract_request_id, and the 401/403 type change is now declared in the PR description.

Still the strongest part of this PR: _async_get_devices / _async_retrieve_encryption_key give the password and token flows one shared implementation instead of a copy-paste, and test_retrieve_encryption_key_with_password proves the legacy path delegates identically. The negative caplog assertions — no access token, refresh token, authorization code, or encryption key ever reaches a log line — are the right way to lock that property in, and _oauth_error_field bounding provider text to 256 chars with the code redacted is a careful touch.

• README troubleshooting still quotes Failed to retrieve encryption key from SwitchBot Account: ..., a message the non-owner/statusCode 190 case no longer produces now that SwitchbotApiError propagates raw
• Every non-429 4xx from the token endpoint maps to SwitchbotAuthenticationError, so a redirect_uri mismatch or 404 sends users into a reauth loop
• expires_in is validated and int()-parsed, then returned unnormalized, so callers repeat the string/int branch
• README example binds the whole token mapping to token without showing token["access_token"]
• extract_request_id has no direct test — the cf-ray and no-header-present paths are unexercised despite the "fully covered" claim
• PKCE/client-secret omission recorded as a dismissed finding per @zerzhang's explanation, with the residual interception risk noted for the consumer's owner

---

✅ Resolved since last review (3)

Previously-flagged issues verified fixed

• switchbot/oauth.py:71 Token exchange sends no client authentication and no PKCE — and the helper signature can't express either
• switchbot/devices/device.py:406 Unknown-model diagnostic loses the payload that exists to add new device support
• tests/test_oauth.py:261 Log assertions pin exact debug wording, not just the security property

---

🟢 Suggestions

1. README troubleshooting quotes an error message the key path no longer emits
README.md:89-93

The new except SwitchbotApiError: raise passthrough in switchbot/devices/device.py:1210-1211 changes what the pre-existing password flow produces for the most commonly reported failure.

Before this PR, a non-100 statusCode from wonder/keys/v1/communicate (e.g. the non-owner 190 case documented in this very bullet) was raised by api_request as SwitchbotApiError, caught by the bare except Exception, and re-raised as SwitchbotAccountConnectionError("Failed to retrieve encryption key from SwitchBot Account: <msg>, status code: 190"). Now it propagates untouched, so the user sees only <msg>, status code: 190 and the exception type differs.

Why it matters: this README bullet tells users to match on the string Failed to retrieve encryption key from SwitchBot Account: ... to diagnose the shared-account/non-owner case — that string no longer appears for that case, so the troubleshooting section silently points at nothing. Downstream consumers that branch on SwitchbotAccountConnectionError (rather than the common RuntimeError base) also see a different type on an unchanged call.

Unverified: I could not check homeassistant-core's except clauses from this worktree, so I'm not treating the type change as blocking — the PR description does declare it under "Error semantics".

Fix: update this bullet to quote the message users will actually see (and name SwitchbotApiError), and consider calling the type change out in the commit body so it lands in the changelog for downstreams.

- **`Failed to retrieve encryption key from SwitchBot Account: ...`** —
  authentication succeeded but the key could not be read. Usually the account
  is not the device **owner**: keys are only returned to the owning account,
  not to shared/family members. Retrieve the key from the owner account, or
  transfer ownership in the app.

2. Every non-429 4xx from the token endpoint becomes an authentication error
switchbot/oauth.py:131-138

The classification treats the whole 400..499 range minus 429 as SwitchbotAuthenticationError. That over-collapses two very different failures:

• 400 invalid_request / 401 invalid_client caused by an unregistered client_id or a redirect_uri mismatch — a caller configuration bug.
• 404 — wrong endpoint or a provider-side route change.

Why it matters: a consumer that maps SwitchbotAuthenticationError to "re-authorize" (which is the natural mapping, and what the README implies) will send the user back through the consent screen repeatedly for a misconfiguration that re-authorizing can never fix.

Cheap improvement: keep the auth classification for 401/403 (and 400 with error in {"invalid_grant", "invalid_client"}, which you already parse), and route the remaining 4xx to SwitchbotApiError — the taxonomy the README already documents for "other API failures".

    if 400 <= status < 500 and status != 429:
        raise SwitchbotAuthenticationError(
            f"SwitchBot OAuth token request rejected ({status}){error_suffix}"
        )

3. `expires_in` is validated and parsed, then thrown away
switchbot/oauth.py:142-166

exchange_oauth_code goes to real trouble over expires_in — rejects bool, accepts a numeric string, int()-casts it to prove it parses — and then returns token unchanged, so a caller receiving {"expires_in": "3600"} still holds a str and has to redo the same cast (and re-handle ValueError) to compute an expiry.

Why it matters: the module exists to centralize validation and error mapping; leaving the normalization out means every consumer duplicates the string/int branch, and the one that forgets gets a TypeError at expiry-math time rather than a SwitchbotApiError here.

Options: return the token with expires_in normalized to int (a small, documented deviation from "unchanged"), or state explicitly in the README that expires_in may be a numeric string.

    token: dict[str, Any] = token_data
    _LOGGER.debug("SwitchBot OAuth token response fields: %s", sorted(token))
    access_token = token.get("access_token")
    expires_in = token.get("expires_in")
    if (
        not isinstance(access_token, str)
        or not access_token
        or isinstance(expires_in, bool)
        or not isinstance(expires_in, int | str)
    ):
        raise SwitchbotApiError("Invalid token data from SwitchBot OAuth token API")
    try:
        int(expires_in)
    except ValueError as err:
        raise SwitchbotApiError(
            "Invalid token data from SwitchBot OAuth token API"
        ) from err
    _LOGGER.debug(
        "SwitchBot OAuth token response validated; expires_in=%s "
        "refresh_token_present=%s refresh_expires_in_present=%s",
        int(expires_in),
        bool(token.get("refresh_token")),
        "refresh_expires_in" in token,
    )
    return token

4. OAuth example leaves the access-token extraction implicit
README.md:47-51

The example binds the whole mapping to token, and the following prose says "The access token can then be passed to fetch_cloud_devices_by_token". A reader skimming the snippet will plausibly write fetch_cloud_devices_by_token(session, token).

Why it matters: passing the dict flows straight into {"authorization": access_token} and fails inside aiohttp's header encoding with an opaque TypeError, far from the mistake — no validation in this library catches it.

One extra line in the snippet removes the ambiguity:

devices = await fetch_cloud_devices_by_token(session, token["access_token"])

The client ID and redirect URI must be registered with SwitchBot; arbitrary
values will not work. `exchange_oauth_code` returns the provider's token
mapping unchanged after validating the access token and expiry fields. The
access token can then be passed to `fetch_cloud_devices_by_token` or
`SwitchbotEncryptedDevice.async_retrieve_encryption_key_by_token`.

---

Checklist

• No hardcoded secrets or credentials
• Secrets kept out of logs (tokens, codes, keys)
• Input validation at system boundaries
• Error handling: no swallowed exceptions, correct taxonomy — suggestion ＃1, suggestion ＃2
• No resource leaks (sessions/responses closed)
• Backward compatibility of public API and exception types — suggestion ＃1
• Documentation matches implemented behaviour — suggestion ＃1, suggestion ＃4
• Diff matches the PR description (no scope creep)

---

Silent Failure Analysis

🟠 **5. HIGH** — error downgraded to wrong type (auth failure masked as transient)
switchbot/devices/device.py:271-281

Risk: The SwitchBot app API signals auth problems in-body (HTTP 200 with statusCode != 100), which api_request turns into SwitchbotApiError; here that is reclassified as a connection error, so an expired or revoked OAuth token looks like a transient outage and callers retry forever instead of re-authenticating — note the sibling _async_retrieve_encryption_key in this same diff does add an except SwitchbotApiError: raise passthrough, so the two paths now disagree.

except SwitchbotAuthenticationError:
    raise
except Exception as err:
    raise SwitchbotAccountConnectionError(
        f"Failed to retrieve SwitchBot Account user details: {err}"
    ) from err

Fix: Add except SwitchbotApiError: raise alongside the authentication passthrough so API-level rejections keep their classification.

🟡 **6. MEDIUM** — silent default masking missing data
switchbot/devices/device.py:356-359

Risk: _extract_region silently falls back to "us" when botRegion is absent, and the OAuth path is a newly exercised caller whose userinfo payload may not carry that field — an EU/JP account would then be queried against the wrong regional host and return an empty or failing device list with no error.

userinfo = await cls._async_get_user_info(session, auth_headers)
region = _extract_region(userinfo)
_LOGGER.debug("SwitchBot account region resolved to %s", region)

Fix: Log a warning (or raise SwitchbotApiError) when botRegion is missing from the userinfo response rather than defaulting at debug level only.

🟡 **7. MEDIUM** — unvalidated response field outside error handling
switchbot/devices/device.py:377-378

Risk: This indexing sits outside the try block, so a response missing Items (or returning a non-list) escapes as a raw KeyError/TypeError instead of the SwitchbotApiError/SwitchbotAccountConnectionError contract the README documents, and callers catching only the library exceptions will crash — the diff hardens communicationKey this way but leaves Items unchecked.

items: list[dict[str, Any]] = device_info["Items"]
_LOGGER.debug("SwitchBot cloud API returned %s device records", len(items))

Fix: Validate Items is a list and raise SwitchbotApiError("Invalid device response from SwitchBot API") otherwise, mirroring the new communicationKey validation.

🟡 **8. MEDIUM** — swallowed exception with no diagnostic
switchbot/oauth.py:97-101

Risk: Redacting the provider body is intentional, but discarding the exception entirely means a malformed/HTML error page and a well-formed error body are indistinguishable in logs — both render as error=unavailable, leaving only the status code to debug a failed token exchange.

try:
    error_data = await response.json()
except (aiohttp.ClientError, ValueError, TypeError):
    error_data = None

Fix: Log the exception class (e.g. _LOGGER.debug("...error body unparsable: %s", type(err).__name__)) while still keeping the body contents out of logs and exception messages.

🟡 **9. MEDIUM** — silent None fallback from case-sensitive lookup
switchbot/utils.py:9-13

Risk: The parameter is typed as a plain Mapping, where get("x-request-id") is case-sensitive, so a provider (or any non-aiohttp caller) sending X-Request-Id silently yields request_id=unavailable, quietly defeating the correlation logging this PR adds.

def extract_request_id(headers: Mapping[str, str]) -> str | None:
    return next(
        (value for name in _REQUEST_ID_HEADERS if (value := headers.get(name))), None
    )

Fix: Normalize casing in the lookup (e.g. build a lowercased view of the headers, or headers.get(name) or headers.get(name.title())) so the ID is found regardless of header casing.

---

Automated review by Kōan (Claude) HEAD=2d7dbce 6 min 49s

[bluetoothbot]

Tip

No blocking issues found — ready to merge.