ENG-12481 feat(sdk): read third-party credentials an app is connected to - #7192
adhami3310 wants to merge 2 commits into
Conversation
 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/units/reflex_build_sdk/test_errors.py">
<violation number="1" location="tests/units/reflex_build_sdk/test_errors.py:70">
P2: Because `detail` and `x-reflex-error-code` have the same value, this test does not verify that `APIStatusError.code` comes from the header. Use a different detail value so a regression to detail-based extraction fails.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| def test_status_error_code_from_the_header(): | ||
| response = _response( | ||
| 409, | ||
| json={"detail": "not_connected"}, |
There was a problem hiding this comment.
P2: Because detail and x-reflex-error-code have the same value, this test does not verify that APIStatusError.code comes from the header. Use a different detail value so a regression to detail-based extraction fails.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/units/reflex_build_sdk/test_errors.py, line 70:
<comment>Because `detail` and `x-reflex-error-code` have the same value, this test does not verify that `APIStatusError.code` comes from the header. Use a different detail value so a regression to detail-based extraction fails.</comment>
<file context>
@@ -64,6 +64,19 @@ def test_status_error_type(status_code: int, error_type: type[APIStatusError]):
+def test_status_error_code_from_the_header():
+ response = _response(
+ 409,
+ json={"detail": "not_connected"},
+ headers={"x-reflex-error-code": "not_connected"},
+ )
</file context>
| json={"detail": "not_connected"}, | |
| json={"detail": "Connection unavailable"}, |
Merging this PR will not alter performance
Comparing 
Footnotes
|
There was a problem hiding this comment.
2 issues found across 16 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/reflex-build-sdk/src/reflex_build_sdk/_base.py">
<violation number="1" location="packages/reflex-build-sdk/src/reflex_build_sdk/_base.py:223">
P3: When an internal caller supplies `X-Request-ID`, `Accept`, or `User-Agent` through `extra_headers`, this merge replaces the SDK-owned header even though `_build_request` promises a fresh request ID. That makes `APIError.request_id` caller-controlled and can conflate requests when the same value is reused. Merge extra headers before the SDK-owned headers or reject reserved names.</violation>
</file>
<file name="tests/units/reflex_build_sdk/_async/resources/test_connections.py">
<violation number="1" location="tests/units/reflex_build_sdk/_async/resources/test_connections.py:141">
P2: The credential tests never exercise `end_user`, so they cannot catch a regression that stops forwarding `X-End-User` for user credentials. Call `credential` with `end_user=END_USER` and assert the header, while retaining the existing app-scoped coverage elsewhere.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| client: AsyncReflexCloud, mock_api: MockAPI, body: dict, credential: Credential | ||
| ): | ||
| mock_api.add("GET", f"{PROVIDER_PATH}/credential", reply(200, json=body)) | ||
| assert await client.apps.connections.credential(APP_ID, "openai") == credential |
There was a problem hiding this comment.
P2: The credential tests never exercise end_user, so they cannot catch a regression that stops forwarding X-End-User for user credentials. Call credential with end_user=END_USER and assert the header, while retaining the existing app-scoped coverage elsewhere.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/units/reflex_build_sdk/_async/resources/test_connections.py, line 141:
<comment>The credential tests never exercise `end_user`, so they cannot catch a regression that stops forwarding `X-End-User` for user credentials. Call `credential` with `end_user=END_USER` and assert the header, while retaining the existing app-scoped coverage elsewhere.</comment>
<file context>
@@ -0,0 +1,229 @@
+ client: AsyncReflexCloud, mock_api: MockAPI, body: dict, credential: Credential
+):
+ mock_api.add("GET", f"{PROVIDER_PATH}/credential", reply(200, json=body))
+ assert await client.apps.connections.credential(APP_ID, "openai") == credential
+
+
</file context>
| assert await client.apps.connections.credential(APP_ID, "openai") == credential | |
| assert await client.apps.connections.credential( | |
| APP_ID, "openai", end_user=END_USER | |
| ) == credential | |
| assert mock_api.requests[0].headers["X-End-User"] == END_USER |
| "Accept": "application/json", | ||
| "User-Agent": user_agent(), | ||
| "X-Request-ID": uuid.uuid4().hex, | ||
| **(extra_headers or {}), |
There was a problem hiding this comment.
P3: When an internal caller supplies X-Request-ID, Accept, or User-Agent through extra_headers, this merge replaces the SDK-owned header even though _build_request promises a fresh request ID. That makes APIError.request_id caller-controlled and can conflate requests when the same value is reused. Merge extra headers before the SDK-owned headers or reject reserved names.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-build-sdk/src/reflex_build_sdk/_base.py, line 223:
<comment>When an internal caller supplies `X-Request-ID`, `Accept`, or `User-Agent` through `extra_headers`, this merge replaces the SDK-owned header even though `_build_request` promises a fresh request ID. That makes `APIError.request_id` caller-controlled and can conflate requests when the same value is reused. Merge extra headers before the SDK-owned headers or reject reserved names.</comment>
<file context>
@@ -218,6 +220,7 @@ def _build_request(
"Accept": "application/json",
"User-Agent": user_agent(),
"X-Request-ID": uuid.uuid4().hex,
+ **(extra_headers or {}),
}
if authenticated:
</file context>
2 participants
The SDK half of ENG-12481. ENG-12434 shipped brokered connections — the seam, the store, the ledger and the routes — and nothing called them, because no client could. This is a client.
API
apps.connections.providers()GET /connections/providerslist(app_id)GET /apps/{id}/connectionsstatus(app_id, provider, *, end_user)GET .../statuscredential(app_id, provider, *, end_user)GET .../credentialconnect_link(app_id, provider, *, end_user, return_to)POST .../authorizeor.../sessiondisconnect(app_id, provider, *, end_user)DELETE .../{provider}orPOST .../disconnectend_userpicks whose connection a call acts on. It travels asX-End-User, and the header is absent rather than empty when it is not passed, because the API reads a missing header as the app's own connection — a helper that always sent it would have an app read a visitor's row. There are tests for both directions.Two things this adds to the client itself
extra_headerson the internal request path, which is whatX-End-Userneeded; nothing else uses it.APIStatusError.code, read fromX-Reflex-Error-Code. These routes are the first whose refusals a caller must branch on —not_connected,connection_gone,unsupported_credential,prior_revoke_owed,broker_unavailable,connections_unconfigured— and matching on prose was the only way before. flexgen emits that header on everyServiceErrorrefusal, so this pays off beyond connections. I stopped at the field rather than minting named exception classes; that can follow if callers want it.Handling the credential
Credential.access_tokenis the first live secret in an SDK model, and a frozen dataclass prints every field. It is declaredfield(repr=False)with a test thatrepr()does not contain the token, so printing the model, or an exception rendering a local, discloses nothing. The API sendsCache-Control: no-storeand the client stores nothing.Not in this PR
{app_id}, andplatform_env_for_appships only the token and the backend URL. The token resolves to an app server-side, but no route answers "which app am I", so code inside a deployed app cannot fill the path parameter. That is a flexgen change; the SDK takesapp_idfrom the caller meanwhile.app_idis NULL, so the routes refuse it; an unpublished thread has no app row either.Testing
uv run pytest tests/units/reflex_build_sdk: 728 passed, covering each method through both clients, bothX-End-Userdirections, all three credential kinds, the refusal code, the masked repr, and that a failedsessionis not retried into a second link.uv run pytest tests/units: 9579 passed, 78.8% coverage. One unrelated flake,test_state_manager_lock_expire, which passes on its own.pyright,ruffandpre-commitclean.CONNECTIONS_ENABLEDgoes on.