diff --git a/CHANGELOG.md b/CHANGELOG.md index f17eda0..aec9923 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## Unreleased + +- Accept refresh requests that omit `resource` only when the existing grant is bound to the unchanged connector resource and matching client ID. +- Reject renewal of stored grants after an origin change, including requests explicitly naming the new origin. +- Include `error="invalid_token"` in rejected Bearer-token challenges while preserving initial OAuth discovery metadata. +- Add private Boolean refresh diagnostics and timed renewal/security regressions. The actual client failure in issue #6 still needs logs or live confirmation. + ## 2026.10.6.1 — 2026-10-06 - Restyle the OAuth pairing page with the connector logo, a responsive card and automatic light/dark themes. diff --git a/custom_components/grok_connector/diagnostics.py b/custom_components/grok_connector/diagnostics.py index 23e8549..eec5d3a 100644 --- a/custom_components/grok_connector/diagnostics.py +++ b/custom_components/grok_connector/diagnostics.py @@ -28,7 +28,8 @@ "state_valid", "pkce_s256", "challenge_valid", "scope_matches", "response_type_code", "authorization_present", "bearer_valid", "code_present", "code_known", "code_live", "verifier_valid", "code_client_matches", "code_redirect_matches", "pkce_matches", - "refresh_present", "client_secret_supplied", "accepts_json", "protocol_supported", + "refresh_present", "refresh_known", "refresh_replayed", "refresh_client_matches", + "refresh_live", "refresh_resource_matches", "client_secret_supplied", "accepts_json", "protocol_supported", "session_present", "session_valid"} _COUNTS = {"status", "pending_count", "clients_count", "grant_count", "failed_pairings", "cookie_name_count", "redirect_count"} diff --git a/custom_components/grok_connector/http.py b/custom_components/grok_connector/http.py index e183866..e87158f 100644 --- a/custom_components/grok_connector/http.py +++ b/custom_components/grok_connector/http.py @@ -169,14 +169,20 @@ class MCPView(GatewayView): name = DOMAIN + ":mcp" url = MCP_PATH + def unauthorized(self, request): + challenge = 'Bearer resource_metadata="' + self.runtime.authority.base_url + RESOURCE_METADATA_PATH + '"' + if request.headers.get("Authorization", "").startswith("Bearer "): + # RFC 6750 distinguishes an invalid/expired token from initial discovery. + challenge += ', error="invalid_token"' + return web.HTTPUnauthorized(headers={**NO_CACHE, "WWW-Authenticate": challenge}) + def authorize(self, request): self.check_origin(request, mcp=True) principal = self.runtime.authority.authenticate(request.headers.get("Authorization")) if principal is None: self.diagnostic(request, "mcp", "bearer_rejected", authorization_present= bool(request.headers.get("Authorization")), bearer_valid=False, status=401) - raise web.HTTPUnauthorized(headers={**NO_CACHE, "WWW-Authenticate": - 'Bearer resource_metadata="' + self.runtime.authority.base_url + RESOURCE_METADATA_PATH + '"'}) + raise self.unauthorized(request) # Credentials in the URL are never accepted, including alongside a valid header. if any(key in request.query for key in ("access_token", "token", "authSig")): raise web.HTTPBadRequest(text="Credentials must be in the Authorization header") @@ -219,7 +225,7 @@ async def post(self, request): # Each initialization establishes its own request-ID namespace. Keep # stateless clients compatible when they omit the optional header. if not self.runtime.gateway.authorized(principal): - raise web.HTTPUnauthorized(headers=NO_CACHE) + raise self.unauthorized(request) session = self.runtime.gateway.open_session(principal) headers["MCP-Session-Id"] = session self.diagnostic(request, "mcp", "session_created", session_present=True, session_valid=True) @@ -404,8 +410,21 @@ async def post(self, request): "refresh_present": bool(data.get("refresh_token")), "client_secret_supplied": "client_secret" in data, "client_auth": "basic" if request.headers.get("Authorization", "").startswith("Basic ") else "other" if request.headers.get("Authorization") else "none"} - self.diagnostic(request, "token", "request", **fields) async with self.runtime.token_lock: + if data.get("grant_type") == "refresh_token": + try: + refresh_hash = digest(data.get("refresh_token", "")) + except UnicodeError: + refresh_hash = "" + grant = next((grant for grant in authority.grants.values() if + secrets.compare_digest(refresh_hash, grant["refresh_hash"]) + or refresh_hash in grant["used_refresh"]), {}) + fields.update(refresh_known=bool(grant), + refresh_replayed=bool(grant) and refresh_hash in grant["used_refresh"], + refresh_client_matches=bool(grant) and data.get("client_id") == grant["client_id"], + refresh_live=bool(grant) and grant["refresh_expires"] > authority.clock(), + refresh_resource_matches=bool(grant) and grant.get("resource") == authority.resource) + self.diagnostic(request, "token", "request", **fields) try: result = self.runtime.authority.token(data) except OAuthError as err: @@ -414,7 +433,7 @@ async def post(self, request): await self.runtime.save() return web.json_response({"error": err.error}, status=400, headers=NO_CACHE) await self.runtime.save() - self.diagnostic(request, "token", "token_issued", flow_id=fields["flow_id"], status=200, + self.diagnostic(request, "token", "token_issued", flow_id=fields["flow_id"], grant_type=fields["grant_type"], status=200, grant_count=len(authority.grants)) return web.json_response(result, headers=NO_CACHE) diff --git a/custom_components/grok_connector/security.py b/custom_components/grok_connector/security.py index d907689..072020e 100644 --- a/custom_components/grok_connector/security.py +++ b/custom_components/grok_connector/security.py @@ -186,7 +186,10 @@ def approve(self, transaction, pairing_secret): def token(self, params): self.prune() - if params.get("resource") != self.resource: + # Initial exchange remains explicit. A refresh may omit the indicator, + # but it can only renew the audience already bound to the saved grant. + if (("resource" in params and params["resource"] != self.resource) + or (params.get("grant_type") != "refresh_token" and params.get("resource") != self.resource)): raise OAuthError("invalid_target") if params.get("grant_type") == "authorization_code": try: @@ -226,6 +229,8 @@ def token(self, params): raise OAuthError("invalid_grant") if params.get("client_id") != grant["client_id"]: raise OAuthError("invalid_grant") + if grant.get("resource") != self.resource: + raise OAuthError("invalid_target") grant["used_refresh"] = [*grant["used_refresh"][-31:], refresh_hash] else: raise OAuthError("unsupported_grant_type") diff --git a/docs/TOKEN-REFRESH-TROUBLESHOOTING.md b/docs/TOKEN-REFRESH-TROUBLESHOOTING.md new file mode 100644 index 0000000..0c9ce14 --- /dev/null +++ b/docs/TOKEN-REFRESH-TROUBLESHOOTING.md @@ -0,0 +1,38 @@ +# Investigating token renewal after ten minutes + +Related report: [issue #6](https://github.com/Jenik5/GrokConnectorForHomeAssistant/issues/6), version 2026.10.6.1, HA 2026.9.4, Nabu Casa. The reporter says pairing and tool discovery work on web, iOS and Tesla, then connection fails after roughly ten minutes. Client acceptance in that report is user-provided evidence. + +## Confirmed from source and local tests + +Access tokens expire after **600 seconds**. Refresh grants last at most **30 days**. The token endpoint advertises refresh support and returns a refresh token at initial exchange. Renewal using the current refresh token, matching client ID and explicit matching resource works after access-token expiration. + +Two interoperability problems are reproduced in the released code: + +1. A refresh request that omits `resource` is rejected with `invalid_target`, even when the valid refresh token is already bound to this resource. This is a plausible explanation for the report, but the reporter's actual refresh request has not been observed. [MCP clients must send resource indicators](https://modelcontextprotocol.io/specification/2025-11-25/basic/authorization#resource-parameter-implementation); tolerating omission during refresh is an interoperability choice by this single-resource server, not evidence that Grok complies with that requirement. +2. A rejected expired Bearer token produces HTTP 401 with resource discovery metadata, but lacks `error="invalid_token"`. The patch adds the standard [RFC 6750 challenge](https://www.rfc-editor.org/rfc/rfc6750.html#section-3.1), while keeping initial unauthenticated discovery free of an error code. Whether Grok uses this field to trigger renewal has not been verified. + +HA 2026.9.4's [ban middleware](https://github.com/home-assistant/core/blob/2026.9.4/homeassistant/components/http/ban.py) processes raised HTTPUnauthorized exceptions as failed logins and can display a notification. An expired connector token can therefore produce the reported notification. The IP and reverse DNS label alone do not establish why renewal failed or who operated the caller. Failed-login and IP-ban behavior are unchanged by this patch. + +## Patch and security boundaries + +Omitted `resource` is accepted **only for a refresh** backed by a valid existing grant with its original resource equal to this connector's current resource and a matching client ID. Explicit empty, malformed or different resource values remain rejected. Initial authorization/code exchange still require the explicit matching resource and PKCE. A saved refresh grant cannot be migrated to a changed origin, even if the request names that new origin; review added this missing guard as well. + +Access-token lifetime remains 600 seconds; the refresh grant's original deadline is not extended. Hash-only credential storage, rotation, replay revocation, policy revocation and grant-bound MCP sessions remain in place. Existing schemas are unchanged. The patch adds only bounded Boolean renewal diagnostics (known token, replay, client match, deadline and saved resource match) and the grant type on successful issuance, without logging tokens, their hashes, client IDs or request bodies. + +Local validation: **100 Python tests**, **5 JavaScript tests**, package and JavaScript syntax checks passed. Seven added regressions failed on the released code before the patch. The timed HTTP regression simulates four renewal cycles over 40 minutes, alternating requests with/without the resource indicator, and continues reading selected states in the same MCP session. Separate cases cover restoration after access-token expiry, changed origins, bad clients/resources, expired grants, replay and diagnostic privacy. Tests use shipped modules with framework adapters and simulated state readers. No live HA deployment or Grok/Tesla retest was performed. + +## Logs needed to confirm this report + +1. In HA, enable debug logging for Grok Connector from its integration menu. If unavailable, use the `logger.set_level` action with `custom_components.grok_connector: debug` for the reproduction period, then restore the previous logging level. +2. Start with a working connection, note the time, wait at least 11–12 minutes, and ask Grok for a selected entity's state once. Capture the failure before using Reconnect. +3. Provide lines containing `GROK_CONNECTOR_DIAG` with stages `token` and `mcp` around that time, plus the HA failed-login log's **path** (e.g. `/api/grok_connector/mcp` or `/api/grok_connector/oauth/token`). Remove the hostname and query string. Do not send Authorization headers, access/refresh tokens, pairing codes, credential storage, client IDs or whole raw HTTP requests. +4. Stop debug logging after capture. These connector records are deliberately restricted to categories, flags, counts and unrelated diagnostic correlation IDs. + +Useful distinctions: + +- `grant_type=refresh_token`, `oauth_error=invalid_target`, `resource_supplied=false` on 2026.10.6.1 confirms the omission case reproduced here. +- `invalid_grant` requires distinguishing client mismatch, expired/revoked/unknown refresh tokens and replay. The patch's Boolean diagnostics provide those distinctions without secrets. +- No token-stage entries under debug logging does not prove the client sent no refresh. The request may have been blocked before the connector; inspect HA's requested path and any HTTP 403/IP-ban entries too. +- A successful token renewal followed by MCP 404 points to session recovery rather than token exchange. A successful renewal followed by MCP 401 needs the old/new access-token usage checked without disclosing either token. + +The user's actual failure remains unconfirmed until logs or a live retest identify which path occurred. The patch addresses reproduced server behavior; it is not yet a published release. diff --git a/tests/test_http.py b/tests/test_http.py index 52dd3e4..218a271 100644 --- a/tests/test_http.py +++ b/tests/test_http.py @@ -528,6 +528,131 @@ async def test_custom_https_port_preserves_oauth_origin_and_resource_boundary(se self.assertEqual(rotated.status, 200) self.assertIsNone(self.authority.authenticate(header)) + async def test_expired_bearer_challenge_identifies_invalid_token(self): + now = 1000 + self.authority.clock = lambda: now + transaction = await self.form() + _, code, _ = self.authority.approve(transaction, self.authority.pair()) + tokens = self.authority.token({"grant_type": "authorization_code", "code": code, + "client_id": self.client, "redirect_uri": self.params["redirect_uri"], + "resource": self.authority.resource, "code_verifier": self.verifier}) + now += 600 + mcp = namespace["MCPView"](self.hass) + with self.assertRaises(web.HTTPUnauthorized) as caught: + mcp.authorize(Request(headers={"Authorization": "Bearer " + tokens["access_token"]})) + self.assertIn('error="invalid_token"', caught.exception.headers["WWW-Authenticate"]) + self.assertIn('resource_metadata="' + self.authority.base_url + const.RESOURCE_METADATA_PATH + '"', + caught.exception.headers["WWW-Authenticate"]) + with self.assertRaises(web.HTTPUnauthorized) as caught: + mcp.authorize(Request()) + self.assertNotIn('error=', caught.exception.headers["WWW-Authenticate"]) + + async def test_mcp_session_survives_repeated_timed_http_refresh(self): + now = 1000 + self.authority.clock = lambda: now + transaction = await self.form() + _, code, _ = self.authority.approve(transaction, self.authority.pair()) + response = await self.token_view.post(Request(form={"grant_type": "authorization_code", "code": code, + "client_id": self.client, "redirect_uri": self.params["redirect_uri"], + "resource": self.authority.resource, "code_verifier": self.verifier})) + self.assertEqual(response.status, 200) + tokens = json.loads(response.text) + principal = self.authority.authenticate("Bearer " + tokens["access_token"]) + self.runtime.policy = policy.Policy.from_dict({"read_entities": ["light.test"], "actions": []}) + self.runtime.gateway = gateway.Gateway(lambda entity: "off", None, self.runtime.policy, catalogs, + authorized=lambda p: p in self.authority.grants and + self.authority.grants[p]["access_expires"] > now and + self.authority.grants[p]["refresh_expires"] > now, clock=lambda: now) + self.hass.async_create_task = lambda task, name: asyncio.create_task(task) + mcp = namespace["MCPView"](self.hass) + def request(message, session=None): + headers = {"Authorization": "Bearer " + tokens["access_token"], "Accept": "application/json"} + if session: + headers["MCP-Session-Id"] = session + return Request(body=json.dumps(message).encode(), headers=headers) + initialized = await mcp.post(request({"jsonrpc": "2.0", "method": "initialize", "id": 1})) + session = initialized.headers["MCP-Session-Id"] + for cycle in range(4): + now += 601 + old_access = tokens["access_token"] + with self.assertRaises(web.HTTPUnauthorized): + await mcp.post(request({"jsonrpc": "2.0", "method": "tools/list", "id": 2}, session)) + form = {"grant_type": "refresh_token", "client_id": self.client, + "refresh_token": tokens["refresh_token"]} + if cycle % 2 == 0: + form["resource"] = self.authority.resource + rotated = await self.token_view.post(Request(form=form)) + self.assertEqual(rotated.status, 200) + tokens = json.loads(rotated.text) + self.assertIsNone(self.authority.authenticate("Bearer " + old_access)) + self.assertEqual(self.authority.authenticate("Bearer " + tokens["access_token"]), principal) + result = await mcp.post(request({"jsonrpc": "2.0", "method": "tools/call", "id": cycle + 3, + "params": {"name": "entities_status", "arguments": {}}}, session)) + payload = json.loads(result.text) + self.assertNotIn("error", payload) + states = json.loads(payload["result"]["content"][0]["text"])["entities"] + self.assertEqual(states[0]["entity_id"], "light.test") + self.assertEqual(states[0]["state"], "off") + self.assertEqual(self.save_count, 5) + + async def test_refresh_diagnostics_explain_rejections_without_credentials(self): + transaction = await self.form() + _, code, _ = self.authority.approve(transaction, self.authority.pair()) + tokens = self.authority.token({"grant_type": "authorization_code", "code": code, + "client_id": self.client, "redirect_uri": self.params["redirect_uri"], + "resource": self.authority.resource, "code_verifier": self.verifier}) + params = {"grant_type": "refresh_token", "refresh_token": tokens["refresh_token"], + "client_id": self.client} + with self.assertLogs(diagnostics._LOGGER, level="INFO") as captured: + wrong_client = await self.token_view.post(Request(form={**params, "client_id": "unknown"})) + self.assertEqual(wrong_client.status, 400) + renewed = await self.token_view.post(Request(form=params)) + self.assertEqual(renewed.status, 200) + replay = await self.token_view.post(Request(form=params)) + self.assertEqual(replay.status, 400) + requests = [record for record in self.records(captured) + if record["stage"] == "token" and record["outcome"] == "request" + and record.get("grant_type") == "refresh_token"] + self.assertEqual(len(requests), 3) + self.assertFalse(requests[0]["refresh_client_matches"]) + self.assertTrue(all(record["refresh_known"] and record["refresh_live"] and + record["refresh_resource_matches"] for record in requests)) + self.assertTrue(requests[1]["refresh_client_matches"]) + self.assertFalse(requests[1]["resource_supplied"]) + self.assertFalse(requests[1]["refresh_replayed"]) + self.assertTrue(requests[2]["refresh_replayed"]) + issued = next(record for record in self.records(captured) if record["outcome"] == "token_issued") + self.assertEqual(issued["grant_type"], "refresh_token") + combined = "\n".join(captured.output) + renewed = json.loads(renewed.text) + for credential in (code, self.client, self.verifier, *tokens.values(), + renewed["access_token"], renewed["refresh_token"]): + if isinstance(credential, str) and len(credential) >= 20: + self.assertNotIn(credential, combined) + self.assertNotIn(security.digest(credential), combined) + self.assertEqual(self.authority.grants, {}) + self.assertEqual(self.save_count, 3) + + async def test_expired_refresh_has_safe_diagnostics_and_stays_rejected(self): + now = 1000 + self.authority.clock = lambda: now + transaction = await self.form() + _, code, _ = self.authority.approve(transaction, self.authority.pair()) + tokens = self.authority.token({"grant_type": "authorization_code", "code": code, + "client_id": self.client, "redirect_uri": self.params["redirect_uri"], + "resource": self.authority.resource, "code_verifier": self.verifier}) + now += 30 * 86400 + with self.assertLogs(diagnostics._LOGGER, level="INFO") as captured: + response = await self.token_view.post(Request(form={"grant_type": "refresh_token", + "client_id": self.client, "refresh_token": tokens["refresh_token"]})) + self.assertEqual(response.status, 400) + record = next(record for record in self.records(captured) if record["outcome"] == "token_rejected") + self.assertTrue(record["refresh_known"]) + self.assertFalse(record["refresh_live"]) + self.assertEqual(record["oauth_error"], "invalid_grant") + self.assertEqual(self.authority.grants, {}) + self.assertEqual(self.save_count, 1) + async def test_existing_nabu_grants_survive_default_port_canonicalization(self): transaction = await self.form() _, code, _ = self.authority.approve(transaction, self.authority.pair()) diff --git a/tests/test_security.py b/tests/test_security.py index 14bade1..2060b5b 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -154,6 +154,73 @@ def test_refresh_rotates_and_replay_revokes_family(self): self.auth.token(refresh) self.assertIsNone(self.auth.authenticate("Bearer " + new_tokens["access_token"])) + def test_refresh_without_resource_after_expiry_and_restart(self): + tokens, client, _ = self.exchange() + principal = self.auth.authenticate("Bearer " + tokens["access_token"]) + self.now += 601 + self.auth = security.OAuthAuthority(BASE, self.auth.snapshot(), clock=lambda: self.now) + self.assertIsNone(self.auth.authenticate("Bearer " + tokens["access_token"])) + refreshed = self.auth.token({"grant_type": "refresh_token", "client_id": client, + "refresh_token": tokens["refresh_token"]}) + self.assertEqual(refreshed["expires_in"], 600) + self.assertEqual(self.auth.authenticate("Bearer " + refreshed["access_token"]), principal) + self.assertEqual(self.auth.grants[principal]["resource"], self.auth.resource) + self.assertNotIn(refreshed["refresh_token"], json.dumps(self.auth.snapshot())) + + def test_refresh_resource_binding_cannot_migrate_to_changed_origin(self): + tokens, client, _ = self.exchange() + saved = self.auth.snapshot() + for include_resource in (False, True): + changed = security.OAuthAuthority("https://other-house.ui.nabu.casa", saved, clock=lambda: self.now) + params = {"grant_type": "refresh_token", "client_id": client, + "refresh_token": tokens["refresh_token"]} + if include_resource: + params["resource"] = changed.resource + with self.subTest(include_resource=include_resource): + with self.assertRaises(security.OAuthError) as caught: + changed.token(params) + self.assertEqual(caught.exception.error, "invalid_target") + self.assertEqual(changed.snapshot(), saved) + + def test_refresh_explicit_invalid_resource_leaves_grant_usable(self): + tokens, client, _ = self.exchange() + saved = self.auth.snapshot() + params = {"grant_type": "refresh_token", "client_id": client, + "refresh_token": tokens["refresh_token"]} + for resource in ("", None, BASE + "/api/mcp", "https://other-house.ui.nabu.casa" + const.MCP_PATH): + with self.subTest(resource=resource), self.assertRaises(security.OAuthError) as caught: + self.auth.token({**params, "resource": resource}) + self.assertEqual(caught.exception.error, "invalid_target") + self.assertEqual(self.auth.snapshot(), saved) + self.assertIsNotNone(self.auth.authenticate("Bearer " + self.auth.token(params)["access_token"])) + + def test_refresh_omitted_resource_still_requires_valid_client_and_token(self): + tokens, client, _ = self.exchange() + saved = self.auth.snapshot() + params = {"grant_type": "refresh_token", "client_id": client, + "refresh_token": tokens["refresh_token"]} + for bad in ({**params, "client_id": "other"}, {k: v for k, v in params.items() if k != "client_id"}, + {**params, "refresh_token": "unknown"}): + with self.subTest(params_keys=sorted(bad)), self.assertRaises(security.OAuthError) as caught: + self.auth.token(bad) + self.assertEqual(caught.exception.error, "invalid_grant") + self.assertEqual(self.auth.snapshot(), saved) + self.now += 30 * 86400 + with self.assertRaises(security.OAuthError) as caught: + self.auth.token(params) + self.assertEqual(caught.exception.error, "invalid_grant") + self.assertEqual(self.auth.grants, {}) + + def test_refresh_without_resource_rotates_and_replay_revokes_family(self): + tokens, client, _ = self.exchange() + params = {"grant_type": "refresh_token", "client_id": client, + "refresh_token": tokens["refresh_token"]} + refreshed = self.auth.token(params) + with self.assertRaises(security.OAuthError) as caught: + self.auth.token(params) + self.assertEqual(caught.exception.error, "invalid_grant") + self.assertIsNone(self.auth.authenticate("Bearer " + refreshed["access_token"])) + def test_refresh_client_binding_and_full_revoke(self): tokens, client, _ = self.exchange() params = {"grant_type": "refresh_token", "refresh_token": tokens["refresh_token"],