feat(credentials): agent-driven OAuth 2.0 connect flows (device_authorization + authorization_code) - #1371
Open
DavidAtJentic wants to merge 51 commits into
Open
feat(credentials): agent-driven OAuth 2.0 connect flows (device_authorization + authorization_code)#1371DavidAtJentic wants to merge 51 commits into
DavidAtJentic wants to merge 51 commits into
Conversation
This was referenced Sep 15, 2026
Open
phase 1: verified-vendor SSO through the connect-session state
machine + broker-ready credential row + best-effort catalog import.
New surface
- POST /integrations:connect starts a session (agent- or user-initiated).
- GET /connect-sessions/{id} returns review data.
- POST /connect-sessions/{id}:confirm confirms scopes+rules, kicks off
the device-flow round-trip at the vendor.
- GET /connect-sessions/{id}/status polls until connected/failed/expired
(poll_token capability).
- GET /vendors + GET /vendors/{vendor}/auth-capabilities expose the
config-seeded vendor registry.
State
- connect_sessions, device_flow_credentials, agent_credential_permissions
under a new migration. credentials.state adds pending/connected/failed
with backfill to connected.
- Vendor config carries the catalog api_id (e.g. github.com/api.github.com);
create_session decomposes it via canonical_credential_scope so the
credential row's (api_vendor, api_name, catalog_api_id) matches what
a normal catalog import puts on a registered Api row.
Cross-surface wiring
- CatalogAutoImportProtocol in shared/catalog exposes a DI seam for the
connect flow to trigger a catalog import after a credential connects
(broker requires a registered API before it can route).
wiring.InProcessCatalogAutoImporter is the registry-backed impl;
install_control_catalog_auto_importer threads it onto app.state when
the process serves both control and registry.
- ConnectSessionService._finalise_connected fires ensure_imported
best-effort - idempotent (skips already-registered entries), swallows
all errors so credential validity is unaffected.
Approval URL now points at /app/credentials?approve=<sid>&poll_token=<tok>
so the agent-initiated approval landing rides the credentials-dialog
overhaul (the standalone /app/integrations page is gone).
Permissions
- New credentials:connect scope narrower than credentials:write (begin +
poll only). credentials:write implies it; org:admin includes it;
DEFAULT_AGENT_SCOPES ships it so agents can initiate the flow out of
the box.
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
The standalone /app/integrations page overexposed developer-facing concepts (raw agent IDs, "reason" free-text, permission-rule preview) and duplicated a create-credential entry point. This collapses it into the existing CreateCredentialDialog on the Credentials page — one entry point, two variants of the same primitive. Verified-vendor tiles on top of the API picker - ApiPicker gains a "One-click sign-in" section that lists vendors from the config-seeded registry, with VendorIcon avatars. Picking a vendor bypasses the OpenAPI-spec form and routes to VendorConnectFlow. VendorConnectFlow (self mode) - Inline agent picker (name only, no exposed IDs) + scope toggles with read/write/admin badges. No "reason" field, no permission-rule preview. - Single Continue button; a combined start+confirm mutation opens the session and returns the vendor challenge in one shot. - Awaiting: big device code with a copy button, "Open <Vendor>" primary action, live status line while polling. - Terminal: success / failure card with "Signed in as @user" or retry. Agent-initiated approval landing (approve mode) - Same component; when CredentialsPage sees ?approve=<sid>&poll_token=<tok> it auto-opens the dialog with an approvalSession prop. The flow fetches the existing session, shows "Requested by <agent>", flags agent-requested scopes with a subtle badge, and goes straight to confirm → device code → poll. Cleanup - Deleted the /app/integrations route, nav entry, and its two pages. - Vendor connect infrastructure moved into shared/credentials/api/vendors-* so it's reachable from the shared credentials dialog without breaching the shared → modules boundary. - Mock handler for GET /vendors added so ApiPicker tests don't spam MSW warnings. Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
… router Extends the connect-session flow to authorization_code. The state machine + finalise / identity-echo / catalog-auto-import loop stay flow-agnostic; each auth flow's specifics live behind a small handler Protocol. AuthFlowHandler Protocol (flow_handlers/base.py) - prepare(txn, ...) - set up flow-specific storage in-txn - begin(row, flow, scopes) - vendor conversation; returns BeginResult - status(row) - "how's it going?" (poll or return pending) - on_finalise(txn, ...) - flow-specific cleanup inside finalise txn `complete_from_callback` lives only on the concrete AuthCodeFlowHandler — the base Protocol has no dummy stubs. The callback router calls it directly because callback landing is auth-code-specific by construction (only sid-bearing state JWTs reach the connect-session branch). BeginResult is a discriminated union (DeviceFlowChallenge | AuthCodeChallenge) — same shape on the wire (ConfirmSessionResponse.kind) so clients branch on a tag, not on which optional field showed up. DeviceFlowHandler (moved existing logic) - prepare seeds device_flow_credentials - begin runs the RFC 8628 device_authorization request + persists transient state - status lazy-polls the vendor token endpoint - on_finalise clears the transient device_code / user_code columns AuthCodeFlowHandler (new) - prepare seeds oauth_client_credentials from the vendor config (same table a normal DirectOAuth2 credential lives in — refresh path stays on the existing broker code, no changes needed) - begin signs a state JWT with a `sid` claim, builds authorize_url - status returns pending (completion is server-driven via the callback) - complete_from_callback does the RFC 6749 §4.1.3 code exchange Callback routing - state JWT grows an optional `session_id` claim (`sid`) — old credential-connect states keep working (session_id=None) - `GET /credentials/oauth/callback` peeks the state; sid present ⇒ route to ConnectSessionService.complete_from_callback. One callback URL, two consumers. Session-row cleanups (drop cross-flow leaks) - `requested_scopes` moves to connect_sessions (new migration r9f0a1b2c3d4). get_review_data no longer reaches into device_flow_credentials for a flow-agnostic piece of state. - `oauth_token.scope` is the single terminal-readback source for bound_scopes. Handler writes granted_scopes via SuccessTokens; service persists uniformly. - poll_status renamed to get_status: the callable models intent, not implementation. Device flow polls upstream, auth-code returns pending; both come out the same call. Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
Adds the client side of the auth-code flow to the credentials-dialog vendor path. When confirm returns an authorization_code challenge the dialog auto-opens the vendor's authorize_url in a popup; the client observes completion through the existing status polling — no new routes on the client, no separate landing page. - ConfirmResponse becomes a discriminated union on `kind`; the UI branches on the tag rather than sniffing which optional field is set. - AwaitingStep splits into DeviceCodeAwaitingStep and RedirectAwaitingStep — the two shapes really are different, so a single "if user_code then..." component was pretending they weren't. - Small shared bits (PollingStatusLine, CancelBar) live alongside so the two branches don't drift. Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
Vendor-side polling for device-flow sessions moves off the request thread into a background scanner. GET /status becomes a pure stored-state read; the scanner is the sole upstream trigger. An agent that calls :confirm and walks away now sees progress land — no client polling required. - New shared/jobs/connect_poll_scanner.py mirrors the credential-expiry scanner shape: tick loop, per-tick error containment, wired into app_factory alongside the other two scanners. Runs only when the control surface is enabled. - ConnectSessionService.advance_polling_session(session_id) is the scanner entrypoint: owns the outer session-TTL guard + state-machine transitions, delegates the vendor conversation to DeviceFlowHandler.advance. - DeviceFlowHandler.status → DeviceFlowHandler.advance (concrete, not on the AuthFlowHandler Protocol). Scanner is the only caller. Fail-fast on non-retryable vendor errors: any HTTP status the RFC 8628 mapper can't recognise (403, 401, 5xx that doesn't clear, malformed body) surfaces as terminal-failed with vendor_forbidden / vendor_error — no exponential backoff, no time-wasting up to session TTL. - ConnectSessionService.get_status is stripped to a stored-state read: terminal → return persisted state (bound_scopes from oauth_token.scope, uniform across flows); anything else → pending. No handler call, no vendor call, no state transitions. - AuthFlowHandler.status drops off the Protocol entirely — the base contract now covers only lifecycle points that apply to every flow (prepare / begin / on_finalise). Progress observation is service-side (stored-state read); vendor advancement is out-of-band (scanner for polling flows, callback for redirect flows). Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
Users can now manually create OAUTH2_DEVICE_CODE credentials via the same create-credential dialog they use for authorization_code and client_credentials — the type stops being invisible-outside-the-connect- session-flow. If a picked API's OpenAPI spec declares a ``deviceAuthorization`` flow (OpenAPI 3.2), the spec parser now surfaces it in the grant-type selector alongside the other flows. Backend - mapping.to_stored: grant_type=device_code → OAUTH2_DEVICE_CODE. - credentials/service.create: OAUTH2 branch now writes to device_flow_credentials (not oauth_client_credentials) when grant_type=device_code. Public-client flow — no client_secret required and the `authorize_url` field carries the vendor's device_authorization endpoint (OpenAPI 3.2's `deviceAuthorizationUrl`). - credentials/service._to_redacted: detects OAUTH2_DEVICE_CODE and reads from device_flow_credential instead of oauth_client_credential (fixes handover follow-up #5 — the row no longer renders with empty client_id and a misleading grant_type=client_credentials default). - _validate_create_fields: skip client_secret requirement for device_code. UI - schemes.ts: FLOW_TYPE_LABELS / FLOW_TYPE_TO_GRANT_TYPE / FLOW_TYPE_ORDER learn `deviceAuthorization`; the flow parser reads ``deviceAuthorizationUrl`` from the flow object shape (OpenAPI 3.2) and maps it onto the shared `authorizationUrl` slot for uniform UI consumption. - CredentialTypeFields: when grant_type=device_code, hide the Client Secret field, relabel Authorize URL → Device authorization URL, and drop the Callback URL hint (device flow doesn't use a browser redirect). - formBody.validateCreate: no client_secret required for device_code; requires an authorize URL just like authorization_code does. Not touched - "Connect" action for a manually-created device_code credential (running the actual RFC 8628 round-trip against the vendor from the standalone credential path). Existing DirectOAuth2Provider assumes a redirect flow; a DeviceFlowConnectProvider — either standalone or one that wraps the new create-in-a-session — is the next step. Documented in the phase-2 plan doc alongside the other deferred items (device-flow badge polish, UI agent-scope catalogue for credentials:write). Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
… wire device_code connect UI
Backend
- ConnectChallenge, ConfirmResult, BeginResult replaced with proper
discriminated-union subclasses (AuthCodeChallenge / DeviceCodeChallenge
etc.) rather than single classes with all-optional fields; dispatch on
isinstance instead of null-branching on optional attrs.
- New DeviceFlowConnectProvider handles POST /credentials/{id}/connect
for manually-created OAUTH2 device_code credentials — begin_connect
seeds the aux-row transient state and returns the RFC 8628 challenge.
Registered in ProviderRegistry alongside StaticProvider.
- Option C for the poll scanner: ConnectPollScanner targets
device_flow_credentials rows and dispatches to session-mode vs
credential-mode via ConnectSessionService.advance_polling_target,
which reads live-session presence via get_live_by_credential. No
session-wrapping for standalone credentials; both paths share one
scanner target and one write-finalise sink.
- Credential-mode finalise threads the real created_by identity, no
"system" fallback (enforced by tests/arch/test_no_system_actor).
UI
- Hand-authored ConnectChallengeResponse union in
shared/credentials/api/types.ts (regenerated codegen collapses it to
any). runConnectFlow branches on kind — device-code invokes a
caller-supplied onDeviceCodeChallenge render hook and polls
GET /credentials/{id} for completion (server-driven; no popup).
- DeviceCodeConnectDialog renders user_code + verification_uri;
CredentialsPage mounts it and passes onDeviceCodeChallenge into
runConnectFlow for both connect-after-create and the standalone
Connect action.
- CredentialTypeBadge disambiguates OAuth 2.0 grant variants
(Device Code / Authorization Code / Client Credentials).
- CreateCredentialDialog echoes needsConnect for device_code so the
connect flow auto-fires after create like it does for
authorization_code.
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
…e + device flow DirectOAuth2Provider and DeviceFlowConnectProvider now inherit from a new OAuth2Provider abstract base (sibling relation, not parent/child — neither is "a kind of" the other; both are peer variants of RFC 6749). Moved onto the base: - ``supported_types = [OAUTH2]`` + ``supports`` (identical in both) - ``_post_token`` HTTP + JSON-parse helper — device flow's ad-hoc ``httpx.AsyncClient`` + inline error strings are gone; both flows now map non-200s onto the shared TokenExchangeError, and an ``invalid_grant`` body onto InvalidGrantError so callers can distinguish revoked refresh tokens from transient upstream faults. InvalidGrantError and TokenExchangeError moved from ``direct_oauth2`` onto ``oauth2`` (the family module); ``direct_oauth2`` re-exports them so existing importers stay unbroken. PipedreamProvider stays outside this hierarchy — it doesn't call an IdP's token endpoint directly, so the shared _post_token scaffolding wouldn't apply. Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
…handlers + poll scanner
Fills the coverage gap on this branch's new modules:
* DeviceFlowConnectProvider (11 tests) — begin_connect aux-row seed
+ DeviceCodeChallenge shape, complete_connect refusal (server-
driven completion only), refresh public-client invariant (no
client_secret in the token payload), and the inherited
InvalidGrantError / TokenExchangeError mapping via the shared
OAuth2Provider base.
* DeviceFlowHandler.advance (9 tests) — every branch of the RFC
8628 state machine: rate-limit skip, device_code TTL, pending,
slow_down interval widening, denied, expired, success (with
granted_scopes from the aux row, not the vendor's scope field),
and non-retryable 403 / other 5xx terminal mapping.
* AuthCodeFlowHandler (9 tests) — state-JWT round-trip (sid +
credential_id + actor_type all recoverable), scope inclusion /
omission, redirect_uri config guard, on_finalise no-op, and
complete_from_callback token exchange + error paths.
* ConnectPollScanner._tick (4 tests) — empty-batch no-op, per-
candidate dispatch through advance_polling_target, and per-row
fault isolation so one bad row can't strand the batch.
* runConnectFlow device-code branch, UI (3 tests) — no render hook
returns unsupported_challenge with no popup opened; with hook,
polls to connected (still no popup); cleanup fires on both
connected + timeout terminals.
+33 backend tests, +3 UI tests.
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
…ation, integrations router, UI components
Closes the coverage gaps flagged in the previous test commit's summary:
Integration — real control DB, faked vendor HTTP
* tests/integration/control/test_connect_session_service.py (11 tests)
covers create_session (device + auth-code paths seed credential +
aux row + session in the states the scanner/confirm both rely on),
confirm's polling transition, get_status pending / wrong-token
gating, mark_terminal_from_callback flipping both session AND
pending credential to failed (auth-code has no aux row for the
scanner to catch dangling rows otherwise), advance_polling_target
dispatch on live-session presence, complete_from_callback
end-to-end (token vault + identity echo + credential flip), and
SessionNotFoundError surfacing.
Router — FastAPI TestClient + dependency_overrides
* tests/unit/control/web/test_integrations_router.py (16 tests) pins
the four endpoints' HTTP contract: the anti-spoof identity override
on POST /integrations:connect (agent caller's agent_id in the
payload is ignored), user-caller agent_id validation, the
discriminated-union confirm response (kind: device_flow vs
authorization_code — the UI branches on it), and the full error →
status mapping (UnknownVendor → 400, InvalidState → 409,
ScopeValidation → 400 with unknown_scopes echo, SelfConfirm → 403,
NotFound → 404, InvalidPollToken → 403).
UI components — RTL + MSW
* ui/src/shared/credentials/components/__tests__/DeviceCodeConnectDialog.test.tsx
(5 tests): user_code + credential name render, null-challenge
unmount, verification_uri opens in a new tab with
noopener/noreferrer (opening in-place would kill the polling SPA),
verification_uri_complete → verification_uri fallback, Cancel
invokes onCancel.
* ui/src/shared/credentials/components/__tests__/VendorConnectFlow.test.tsx
(4 tests): self-mode configure step renders vendor + agent picker +
scope catalog; a start-and-confirm cycle transitions to the
awaiting step with the user_code visible; approve-mode fetches
session data and surfaces the "requested" tag on agent-requested
scopes; a 404 on the approval link renders an error alert with a
Close button (never crashes the dialog).
+27 backend tests (11 integration + 16 router), +9 UI tests.
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
…e_authorization
The RFC 8628 flow was leaking two names across the stack. This branch
introduced ``"device_flow"`` (config kind, provider name, aux table,
DeviceFlowHandler) and ``"device_code"`` (transient challenge/confirm
response kinds on the two new endpoints) as two discriminator strings
for the same concept — and repeatedly the two AuthCodeChallenge classes
in different layers.
Standardises on ``device_authorization`` — the OpenAPI 3.2 flow name
(sibling of ``authorization_code``, ``client_credentials`` etc.) —
matching what the codebase already reads from OpenAPI specs
(``deviceAuthorizationUrl`` / ``deviceAuthorization`` flow object).
* Discriminator strings on transient response types
(``BeginResult``, ``ConfirmResult``, ``ConnectChallengeResponse``,
``ConfirmSessionResponse``) → ``"device_authorization"``.
* Persisted discriminators (``VendorFlowConfig.kind``,
``credential.provider``, ``connect_sessions.resolved_flow``,
``DeviceAuthorizationHandler.kind`` / ``.provider_id``) →
``"device_authorization"``. Migrated because nothing is shipped —
the branch introduced these values, so no on-disk data yet.
* Class + file renames: ``VendorDeviceFlowConfig`` →
``VendorDeviceAuthorizationFlowConfig``; ``DeviceFlowHandler`` →
``DeviceAuthorizationHandler``; ``DeviceFlowConnectProvider`` →
``DeviceAuthorizationConnectProvider``; ``DeviceFlowCredential`` /
``Repository`` → ``DeviceAuthorization*``; ``DeviceFlowError`` /
``UpstreamError`` → ``DeviceAuthorization*Error``.
* Function renames: ``begin_device_flow`` / ``poll_device_flow`` →
``begin_device_authorization`` / ``poll_device_authorization``.
* Module files renamed via ``git mv``:
``services/integrations/device_flow.py`` → ``device_authorization.py``;
``services/credentials/providers/device_flow.py`` → ``device_authorization.py``;
``core/schema/device_flow_credentials.py`` → ``device_authorization_credentials.py``;
``repos/device_flow_credential_repo.py`` → ``device_authorization_credential_repo.py``;
``flow_handlers/device_code.py`` → ``device_authorization.py``;
the alembic migration + two test files.
* Table + relationship: ``device_flow_credentials`` →
``device_authorization_credentials``;
``Credential.device_flow_credential`` → ``.device_authorization_credential``.
* OpenAPI spec regenerated (``make openapi``).
* ``config/local.yaml`` vendor entry updated.
Handler-layer dataclasses in ``flow_handlers/base.py`` are renamed
``*BeginResult`` (from ``*Challenge``) to disambiguate from the
provider-layer pydantic ``AuthCodeChallenge`` / ``DeviceAuthorizationChallenge``
in ``schemas/connect.py`` — different shapes at different layers, same
concept.
Also lands (from the earlier round):
* Router uses the ``ConnectChallengeResponse`` union alias instead of
inlining the two subclasses in its return-type annotation.
* ``CredentialTypeBadge`` — ``credential`` prop is now required and
the redundant ``type`` prop is removed (derived from
``credential.type``).
The RFC 8628 grant_type wire value ``"device_code"`` stays on
``credential.grant_type`` (that's the OAuth 2.0 grant_type parameter
name at the token endpoint, not the flow discriminator). DB columns
``encrypted_device_code`` / ``device_code_expires_at`` also keep the
RFC field names.
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
…minal, strip agent_id from credential name
Fixes four issues surfaced testing the connect-session flow end-to-end:
1. Agent picker was gated behind ``!agentId`` in the UI, but
credentials still bind through toolkits — the choice was purely
cosmetic + confusing. Removed the picker from ``VendorConnectFlow``
self-mode; ``agent_id`` on the API is optional now
(``ConnectSession.agent_id`` and the migration column both flip
to nullable). Agent callers get their identity injected as before
but are now REFUSED with 403 if they pass ``agent_id`` in the
payload (permission-boundary violation — the caller *is* the
agent). The permission-rule upsert in ``confirm`` is skipped when
``agent_id`` is None. Reinstated as required once agent-credential
bindings replace toolkit membership.
2. Unhappy terminal outcomes (failed / expired / cancelled /
token-exchange error / callback error / identity-echo failure)
used to leave the pending credential as ``state=failed`` — a
dangling row the user then had to hand-delete. ``_mark_terminal``
now logs the outcome then deletes the credential in a single
transaction; the ``connect_sessions`` row cascades away
(FK ``ondelete=CASCADE``) along with every flow-specific aux row
(``device_authorization_credentials``, ``oauth_client_credentials``,
``oauth_tokens`` via ``all, delete-orphan``). SPA polling
``/status`` handles the resulting 404 as terminal-failed in both
self- and approve-mode effects.
3. Credential name was ``f"{entry.display_name} ({agent_id})"`` —
collapses to ``(None)`` under (1) and duplicates what an eventual
agent-credential binding row will express anyway. Now just
``entry.display_name``.
4. (context) The device-authorization flow the user reported broken
was actually working end-to-end on the backend — the built SPA
bundle under ``ui/dist/`` was stale (baked at the pre-rename
``device_flow`` discriminator). Rebuilding fixes it; the SPA now
branches on ``kind === 'device_authorization'`` consistently.
Tests updated: router self-confirm test split into
``refuses_agent_caller_passing_agent_id`` (403) +
``allows_user_caller_without_agent_id`` (201 with agent_id=None);
integration test renamed to
``mark_terminal_from_callback_deletes_credential_and_cascades_session``
and asserts both rows are gone; ``VendorConnectFlow`` self-mode UI
tests assert the picker is NOT rendered and no longer interact with
an agent select.
OpenAPI regenerated. ``agent_id`` column nullability change edits the
in-branch migration directly since nothing on-disk is affected —
migration + schema were introduced on this branch.
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
… connect session on user dismiss
Two UX fixes surfaced by testing:
1. ``CredentialCard`` visual hierarchy — the OAuth grant-variant
chip (``OAuth 2.0 · Authorization Code``) and the ``Connected``
badge sat inline with the name in a ``truncate`` + ``shrink-0``
row, so the pills won the horizontal fight and the user's
credential name was heavily ellipsised. Bumped the title to
``text-base`` and moved the chips to their own line under the
API tuple. The name is now the visual anchor of the card.
2. Dismissing an in-flight connect session (Cancel button, dialog
close, ESC, unmount) left the pending credential lingering in
the credentials list indefinitely — no signal reached the
backend that the flow was abandoned. Added:
* ``ConnectSessionService.cancel_session(session_id, poll_token)``
— verifies the token, no-ops on already-terminal sessions,
otherwise routes through the shared ``_mark_terminal`` (which
deletes the credential + cascades the session + aux rows).
* ``POST /connect-sessions/{id}:cancel?poll_token=...`` — same
``poll_token``-as-capability posture as ``/status``, so the SPA
doesn't need a heavier scope than the poller it already uses.
Returns 204; idempotent for the race where the poll scanner
terminates the session concurrently with a Cancel click.
* ``useCancelConnectSession`` hook + ``cancelConnectSession``
client. VendorConnectFlow wires the explicit Cancel button
through ``handleCancel`` (fires the mutation) and adds an
unmount cleanup effect that fires the raw client (not the
mutation — TanStack Query aborts pending mutations on
unmount, and this call MUST reach the server). Both self- and
approve-mode paths carry the same treatment; the phase is
tracked in a ref so the cleanup can check it without a
closure over stale state. Cancellation invalidates the
credentials list so the dangling pending row drops.
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
… is already connected
Two coupled fixes so a user clicking Connect on an already-connected
device-flow credential actually completes:
1. ``advance_polling_credential`` used to early-return whenever
``credential.state != "pending"`` — a defensive guard that
accidentally stranded the re-connect case. A user starting a
fresh device-flow round on a ``connected`` credential (new
grant, rotated scopes) seeds a new ``encrypted_device_code``
on the aux row, and the scanner correctly picks it up — but
this gate then dropped it on the floor, so the SPA's
``runConnectFlow`` poll loop never observed a transition.
The "flow in flight" signal was always the aux row's
``encrypted_device_code`` being non-NULL and within its TTL
(which is what the scanner query already filters on); the
``state != "pending"`` check duplicated it and got it wrong.
Dropped the gate.
2. ``_write_finalise`` unconditionally called
``OAuthTokenRepository.create`` — but ``oauth_tokens.credential_id``
is uniquely indexed, and a re-connect over an existing token
row trips the constraint on the second successful poll. Now
upserts: ``get_by_credential`` → ``update_tokens`` (which also
clears ``revoked_at`` — a re-connect over a revoked row yields
a live token) or ``create`` if no row exists.
Also: force an ``updated_at`` bump on the credential row on
finalise so SPA pollers that key off ``updated_at`` (e.g.
``runConnectFlow``'s ``isConnected``) detect the transition
even when the ``state`` field is unchanged (already-connected
→ connected). SQLAlchemy may short-circuit the UPDATE when
every assigned column matches its stored value, leaving
``updated_at`` unchanged — hence the explicit assignment.
+1 integration test:
``test_advance_polling_credential_advances_a_re_connect_of_a_connected_credential``
seeds a ``state="connected"`` credential and asserts
``DeviceAuthorizationHandler.advance`` gets invoked, catching a
regression to the ``state != "pending"`` gate.
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
…r so device-flow finalises trigger imports Symptom: after a successful device-flow connect (GitHub via the connect-session flow) the credential lands in the workspace but the vendor's OpenAPI is NOT auto-imported into the catalog — no ``catalog_auto_import.enqueued`` log line fires, and the ``/apis`` list stays without the newly-connected vendor. Cause: ``get_connect_session_service`` reads ``app.state.catalog_auto_importer`` (installed by ``install_control_catalog_auto_importer``) and passes it to ``ConnectSessionService``. That's fine for the auth-code path, which finalises inside the OAuth callback route (has ``request.app.state``). The device-flow path finalises inside the ``ConnectPollScanner`` background task — which lives OUTSIDE the request scope and used to construct its own ``ConnectSessionService(self._ctx)`` with no importer. Result: ``_maybe_import_catalog`` early-returned on the ``is None`` guard, and no import ever fired for scanner-driven finalises. Fix: plumb the importer through the scanner's constructor and reuse it on every tick. Both ``_start_connect_poll_scanner`` call sites (combined-app lifespan + single-surface lifespan) now read the same ``getattr(app.state, "catalog_auto_importer", None)`` the worker path already uses for its own request-agnostic resources. +1 unit test: ``test_tick_threads_catalog_auto_importer_into_session_service`` constructs the scanner with an opaque importer and asserts the same object identity flows into every ``ConnectSessionService`` the tick builds. Catches a regression where the scanner or its call sites silently stop passing the importer through. Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
…otection covers both entrypoints
The connect-session callback path
(``ConnectSessionService.complete_from_callback``) duplicated the
state-JWT decode/verify prologue but skipped the
``ConnectNonceRepository.consume`` step that
``ConnectService.complete`` has always run. A replayed callback URL
inside the state-JWT TTL could double-fire the token exchange
against the vendor until the session state machine flipped to
``connected``. Branch-introduced parity gap — not a pre-existing
issue.
Fix: extract the shared prologue into
``state.consume_callback_state(ctx, raw_state)`` (decode → verify
``actor_id`` → atomic nonce consume) and call it from BOTH callback
paths. Introducing a third entrypoint in the future can't silently
skip the guard — the type-checked ``StateReplayedError`` propagates
uniformly.
* ``state.py`` — new helper + typed errors
(``StateMissingActorError``, ``StateReplayedError``).
* ``ConnectService.complete`` — inline decode+consume replaced by
``consume_callback_state``; ``StateError`` → ``ConnectFlowError``
mapping preserved.
* ``ConnectSessionService.complete_from_callback`` — signature
changes from ``(session_id, code)`` to ``(raw_state, code)`` so
the service owns the consume gate. Router passes raw ``state``
through; the peeked ``sid`` is now only a routing hint, never a
replay-protection bypass.
* Callback router logs ``StateError`` subclass separately from
other exchange failures for ops triage.
+1 integration test:
``test_complete_from_callback_refuses_state_replay`` — drives the
happy path once, then replays the same ``raw_state`` and asserts
(a) ``StateReplayedError`` raised, (b) vendor token endpoint hit
EXACTLY once (a regression that lets the second call through would
show 2). The existing happy-path test switched to the new signature
via a ``_state_from_authorize_url`` helper that extracts the real
signed state from the confirm response — no manufactured JWTs.
Standalone-path regression coverage: existing
``tests/integration/control/test_connect_flow.py`` (6 tests)
continues to pass, confirming the refactor preserves the pre-existing
one-shot semantics for the standalone credential-connect flow.
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
Makes feat/oauth2-connect-flows pass `make test-arch` on top of origin/main (0 failures, was 8): - flow_handlers: drop `sqlalchemy.ext.asyncio` imports; type `db_session` as `Any` (matches AccessRequestService precedent — the concrete session is passed straight to a repository). - credentials router callback: replace `getattr(request.app.state, "ctx", None)` peek with a proper `Depends(get_ctx)` param. - integrations + vendors routers: use Title Case tag names to match `OPENAPI_TAGS` convention. - openapi_meta: declare Vendors + Integrations tags with descriptions, add them to the Core / Access group, map their URL prefixes. - device_authorization_credentials: add to `ksuid_exempt_tables` (1:1 aux table keyed by parent `credentials.id` — same pattern as `oauth_client_credentials`). - test_secrets_are_secretstr: exempt URL-suffixed field names (`_url`, `_endpoint`, `_uri`) from the secret-field heuristic — the arch heuristic was false-positive on OAuth token endpoint URLs. - test_connect_session_service: hoist 5 inline imports. - test_oauth_callback_router: add `get_ctx` dep override. - Regenerated: openapi spec, endpoint reference, config schema, UI client schema. No runtime behaviour change. Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
…eam_url Defense-in-depth SSRF guard at every raw ``httpx`` call the connect flow makes on behalf of the platform's own OAuth credentials. Each URL is either operator-supplied config (vendor registry) or DB-persisted user-supplied config (``oauth_client_credentials.token_url``); a misconfigured / tampered value must not be able to aim a platform-issued request at a private, loopback, or cloud-metadata target. Five call sites now front their POST/GET with ``validate_upstream_url(...)`` (strict-default policy: OAuth token endpoints + identity probes are always public HTTPS in production; no control-plane egress opt-in is threaded): - ``device_authorization.begin_device_authorization`` (POST to ``authorization_endpoint``). - ``device_authorization.poll_device_authorization`` (POST to ``token_endpoint``). - ``providers.oauth2.OAuth2Provider._post_token`` (POST to ``token_url`` — shared refresh + code-exchange path for every non-managed OAuth provider). - ``flow_handlers.auth_code.AuthCodeFlowHandler.complete_from_callback`` (POST to ``occ.token_url``; the call carries the client_secret, so the guard has extra teeth here). - ``integrations.identity_echo.echo_identity`` (GET to ``probe.endpoint`` with the freshly minted bearer token). Each ``ValueError`` from the validator is wrapped into the call site's existing typed error taxonomy so the handler layer's error routing is preserved: * ``DeviceAuthorizationUpstreamError(status=0, ...)`` * ``TokenExchangeError(status=0, ...)`` * ``AuthCodeExchangeError(...)`` * ``IdentityEchoError(...)`` A new unit test (``test_ssrf_guards.py``) pins all four module-function sites: an unsafe (loopback) URL raises the site's typed error AND ``httpx.AsyncClient`` is never constructed — the guard fires pre-network. Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
Every URL the connect flow relays into the user's browser
(``verification_uri`` / ``verification_uri_complete`` for device flow;
``authorize_url`` for auth-code) arrives as free-form text in the
vendor's OAuth JSON response. A compromised or misconfigured vendor
could return ``javascript:...`` (XSS on our origin) or
``data:text/html,...`` (data-URI phishing) and we would previously
have called ``window.open`` on it.
New helper module ``ui/src/shared/credentials/lib/safe-navigation.ts``
exports the rule: HTTPS only. HTTP is refused too — real OAuth vendors
never redirect over plaintext, and treating http as unsafe closes a
downgrade-attack window with zero legitimate cost.
* ``UnsafeVendorUrlError`` — typed error.
* ``isHttpsVendorUrl(raw)`` — predicate for conditional render.
* ``assertHttpsVendorUrl(raw)`` — throws.
* ``openVendorUrl(raw, target?, features?)`` — validates then
delegates to ``window.open``.
* ``assignVendorUrl(raw)`` — validates then delegates to
``window.location.assign``.
Six call sites converted (zero raw ``window.open`` on vendor-supplied
URLs remain in the credentials module):
- ``DeviceCodeConnectDialog``: renders the open button OR an inline
``ShieldAlert`` notice depending on ``isHttpsVendorUrl``.
- ``DeviceCodeAwaitingStep`` and ``RedirectAwaitingStep`` in
``VendorConnectFlow``: same conditional-render pattern via a shared
``UnsafeVendorUrlNotice`` sub-component.
- ``VendorSelfConnectFlow.startFlow`` and
``VendorApproveFlow.approve`` (the two eager auto-open paths that
fire once the auth-code confirm returns): now gated by
``isHttpsVendorUrl`` — unsafe URLs are simply not opened, and the
matching awaiting-step renders the notice.
- ``runConnectFlow`` in ``api/index.ts`` (three navigation paths —
redirect mode, popup, popup-blocked fallback to full redirect):
gated once up front; unsafe URLs return a new
``{ status: 'unsafe_challenge_url' }`` outcome (added to
``ConnectOutcome``).
Regression test ``safe-navigation.test.ts`` (15 cases) pins the
predicate table (``https`` accepted; ``http``, ``javascript:``,
``data:``, ``file:``, unparsable, empty, null, undefined all
rejected) and confirms ``openVendorUrl`` never invokes
``window.open`` when the URL is unsafe.
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
…ty items Three narrow tweaks pulled from the review pass that all sit on the ConnectSessionService surface: - (session-id enumeration oracle). ``get_status`` and ``cancel_session`` used to surface a missing session as ``SessionNotFoundError`` → 404 while a bad ``poll_token`` on an existing session returned 403. The split let an unauth'd caller enumerate session ids by observing status codes. Both branches now raise ``InvalidPollTokenError`` → 403 — the ``poll_token`` is the real capability, and returning 403 uniformly gives no signal about session existence. Loses nothing legitimate (a caller with a valid poll_token still succeeds; unmount cancel is fire-and-forget). - (typed error for missing credential creator). ``_finalise_credential_connected`` raised a bare ``RuntimeError`` when ``credential.created_by`` was ``None`` — bypasses the router's ``ConnectSessionServiceError`` handler and surfaces as an opaque 500. Replaced with a new ``CredentialMissingCreatorError`` subclass so the router renders a structured response with a stable ``error_code``. - (observability for dropped permission_rules). ``confirm`` skipped ``AgentCredentialPermissionRepository.upsert`` when ``row.agent_id`` was ``None`` — legitimate today (rules bind through toolkits until agent-credential bindings replace toolkit membership) but silent. Now logs ``connect_session.permission_rules_dropped`` at ``info`` with ``rules_count`` when the drop happens, so operators can spot pre-migration configs that assumed the rules landed. Test: the integration ``test_get_status_raises_when_session_missing`` is renamed to ``test_get_status_refuses_missing_session_as_403`` and now asserts ``InvalidPollTokenError``. Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
Every ``:connect`` POST fires the vendor's device-authorization or authorization endpoint. A ``credentials:write`` caller that spams the endpoint can get the platform IP throttled by GitHub / Google / etc., which would fail-fast every legitimate user on the same install. Adds a per-actor token-bucket limiter lazily attached to ``app.state.integrations_connect_limiter`` — 30 rpm sustained, burst 10. Keyed by ``identity.sub``. Overflow returns 429 with the standard ``RateLimit-*`` headers plus ``Retry-After``. In-memory only (per-worker) — sufficient for the abuse case (one actor spamming), not a coordinated-cluster limit. Comment in the module points at ``build_state_backend`` for a Redis upgrade if a multi-worker cluster-wide cap is ever needed. Test: ``test_connect_rate_limit_returns_429_with_retry_after`` overrides ``app.state.integrations_connect_limiter`` with tight caps (rpm=1, burst=2) and asserts the third burst request is refused with a ``Retry-After`` header. Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
``VendorAuthConfig.vendor`` is decomposed at credential-create time via
``entry.vendor.split("/", 1)[0]``. A bare string with no ``/`` silently
produced a slugged credential where ``api_vendor`` mismatched the
broker's per-operation identity check — the mismatch only surfaced on
the first connect and was hard to trace back to a config typo.
Adds a Pydantic ``@field_validator("vendor")`` that requires an
interior ``/`` (and rejects leading / trailing slashes). Malformed
values now fail loud at ``AppConfig`` load — the process refuses to
start rather than shipping a broken vendor registry that would only
break at the first attempted connect.
Test: ``tests/unit/test_vendor_auth_config.py`` — happy path plus a
parametrised battery of bad shapes (no slash, leading slash, trailing
slash, empty).
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
The existing ``useEffect`` cleanup that calls ``cancelConnectSession`` via ``fetch`` is reliable for in-page dismiss (dialog close, tree teardown) — the caller is still around to observe the response — but the browser aborts pending ``fetch`` requests when the tab itself is closing. That left pending credential + session rows dangling until the ``ConnectPollScanner`` reaped them at TTL. ``navigator.sendBeacon`` is guaranteed to deliver on unload. Adds: - ``cancelConnectSessionBeacon`` in ``api/vendors-client.ts``: same URL/verb as the fetch client, POSTs an empty ``Blob`` (the poll_token rides on the query string). Falls back to ``false`` in environments without ``navigator.sendBeacon`` (SSR, older browsers). Kept next to the fetch client so both variants stay in sync. - ``beforeunload`` effects on both ``VendorSelfConnectFlow`` and ``VendorApproveFlow``. Each fires the beacon iff ``phaseRef.current === 'awaiting'`` — post-terminal sessions have already been cleaned up server-side, so a beacon there would be a harmless no-op we may as well skip. The existing ``fetch``-based unmount cleanup stays for in-page dismiss (``sendBeacon`` can't set custom headers or observe responses, so the regular client wins whenever the caller is still alive to read them). Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
…he SPA The connect-flow hardening made ``get_status`` and ``cancel_session`` raise ``InvalidPollTokenError`` uniformly whether the session was missing or the ``poll_token`` was wrong — the enumeration oracle guard. That change stopped short of the callers: - Router (``poll_connect_session_status``) still caught a ``SessionNotFoundError`` → 404 branch the service no longer raises; the router test parametrised both raisers, locking the dead branch in place. Removed the handler and its test row. - Router (``cancel_connect_session``) docstring claimed "204 on already-cleaned-up session" but the service now raises ``InvalidPollTokenError`` when the row is gone. Updated the doc to match the actual behaviour (403 on gone; 204 only on still-existing-but-terminal). - SPA (``VendorConnectFlow``) had two useEffect polling handlers that treated ``err.status === 404`` as terminal — after the hardening, ``_mark_terminal`` cascade-deletes the session and the SPA started polling forever against a 403. The "Waiting for you to approve…" spinner ran until the user hit Cancel. Both handlers now branch on 403 (safe: at this point in the SPA lifecycle the poll_token is known-correct, so a 403 is unambiguously "session gone"). Also removes an approve-flow UX lie noticed while touching the file: ``TerminalStep``'s ``onRetry`` was wired to ``onDone`` in ``VendorApproveFlow``, so clicking "Try again" just closed the dialog without actually retrying. Once a session is terminal the agent must initiate a new one — the human can't retry from the approve link. Made ``onRetry`` optional and dropped the caller; the button now hides in approve mode. Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
Backend ``IntegrationsConnectRequest.reason`` was captured on ``connect_sessions.reason`` and echoed back on ``ReviewSessionResponse``, but the SPA's ``ReviewSession`` type omitted the field and ``VendorApproveFlow`` never rendered it. The whole point of ``reason`` is to give the approving human context on an agent-initiated request; the flow silently dropped it end-to-end. Wire it through: - ``ReviewSession`` type gains ``reason: string | null``. - ``AgentRequestCard`` in ``VendorConnectFlow.tsx`` renders a dedicated "Reason" block under the agent identity when the field is populated. Preserves whitespace (``whitespace-pre-wrap``) so an agent that supplies a short paragraph doesn't get collapsed onto one line. Test update pins the render: the approve-mode fixture now supplies a ``reason`` and asserts the exact string is on the page. A regression that dropped ``reason`` again would flip this test. Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
… flow Small, isolated fixes surfaced by the full-branch review pass. Each is independent; grouped in one commit because none is load-bearing enough to warrant its own history entry. - ``VendorsService.resolve_flow`` was skipping the ``_require_flow_ready`` guard on the default (``preferred is None``) path — only the explicit ``preferred_flow`` branch ran it. An operator who shipped a ``device_authorization`` flow with an empty ``client_id`` got a clean config load and a cryptic vendor 400 at first use. The whole point of the guard is fail-loud-at-config-load; asymmetry defeated it for the common no-preferred case. Now runs on both branches. - ``DeviceAuthorizationHandler._poll_vendor`` had no exhaustive check on ``result.status``. A new or unexpected status from ``poll_device_authorization`` fell through to the success branch and blew up on ``assert result.access_token is not None`` instead of surfacing as a typed terminal error. Now an unknown status returns ``StatusReport(kind="failed", error_code="vendor_error")``. - Same handler: RFC 8628 §3.5 ``slow_down`` widened the poll interval by 5s per event with no cap. Repeated slow_downs from a jittery vendor could compound the interval to the session TTL. Capped at ``_MAX_POLL_INTERVAL_SECONDS = 60`` — well above any real vendor's happy-path interval, below the default session TTL. - ``AuthCodeFlowHandler.complete_from_callback`` embedded up to 200 bytes of vendor response body into the ``AuthCodeExchangeError`` message, which landed verbatim in ``terminal_detail`` on the session row (persisted) AND in the structured log. Not a secret leak (the body doesn't echo our client_secret) but arbitrary vendor JSON that hadn't been reviewed against the redaction rules. Now logs the snippet at ``warning`` for debugging and carries only the HTTP status forward in the exception message. - ``VendorApproveFlow.handleCancel`` only cancelled the server-side session when ``phase === 'awaiting'``. If the human landed on the approval URL, decided not to approve, and clicked *Cancel* during the configure step, ``onBack()`` fired but the ``state="created"`` session sat pending until the 30-min TTL scanner reaped it — and the initiating agent's ``/status`` poll kept reporting ``pending`` on an explicit human refusal. Cancel now fires for both configure and awaiting phases. Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
…rics The connect-session subsystem shipped with rich structured logs but no aggregate signal — on-call had no way to answer "is device-flow failing across the fleet?" or "what fraction of confirms actually connect?" without spelunking through per-session log lines (from the review pass). Adds three OpenTelemetry instruments via the sanctioned ``shared/metrics.get_meter`` facade — no direct ``prometheus_client`` or ``opentelemetry.exporter`` imports (which would fail ``tests/arch/test_metrics_facade.py``): - ``control.integrations.sessions_created_total`` — counter, incremented from ``create_session``. Labels: ``vendor``, ``flow``. - ``control.integrations.sessions_terminal_total`` — counter, incremented from ``_finalise_connected`` (outcome=connected) and ``_mark_terminal`` (outcome=failed|expired|cancelled). The per-outcome label is what gives on-call the vendor-vs-us-vs-user breakdown. - ``control.integrations.time_to_connected_seconds`` — histogram, recorded from ``_finalise_connected`` as ``now - connect_session.created_at``. Labels: ``vendor``, ``flow``. This is the load-bearing SLO surface: fraction of sessions reaching ``connected`` under N seconds, per vendor. No new config, no exporter changes — the surface's existing observability pipeline (see ``AppConfig.observability.metrics``) picks these up automatically. Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
…string update
The ``cancel_connect_session`` docstring update (fixing
the 204-vs-403 contract lie flagged by the review pass) leaks into the
committed OpenAPI spec, since operation descriptions are pulled from
the router docstring. Regenerated ``openapi/control/control.openapi.yaml``
and ``ui/openapi.json`` via ``make openapi`` so
``test_openapi_conformance::test_{control,ui_client}_spec_matches_generated``
stops failing.
No API surface change.
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
…ential bindings Rebase adaptation onto theme 5 (#1370): the branch's AgentCredentialPermission stand-in table anticipated direct bindings before they landed; main now has the real model (control-side agent_permission_rules + admin-side agent_credential_bindings). Write to that instead of a parallel table. - confirm(): replace_user_rules captures the approved rules (translated from the approve-page {method, path-glob, effect} shape to the canonical methods/path/match_mode shape), then the admin-DB binding row is created idempotently via EffectsRepository.bind_agent_to_credential. - _mark_terminal(): sweep the cross-DB binding row when the pending credential is deleted (new EffectsRepository.unbind_agent_from_credential mirror). - Drop the AgentCredentialPermission model, repo, and its DDL. - Re-parent the branch migrations onto main's control head and re-id them (q8e9f0a1b2c3->v3d4e5f6a7b8, r9f0a1b2c3d4->w4e5f6a7b8c9) — main took both original ids for theme-5 migrations. - Refresh comments that described the toolkit-era interim model. Co-authored-by: Cursor <cursoragent@cursor.com>
make openapi + make endpoints + ui npm run codegen + cli make generate-api, plus a detect-secrets baseline refresh for the moved spec line numbers. Restores the UI generated models the rebase's generated-file conflict resolution dropped (Integrations/Vendors services and their request models). Co-authored-by: Cursor <cursoragent@cursor.com>
…n with main Rebase follow-ups: the admin permissions shim and two enumerating tests now include credentials:connect; the requested_scopes migration gets a SQLite JSON variant (main's migration-status check applies the control chain to SQLite); regenerate config schema/reference + CLI vendored copies for the branch's new config surface. Co-authored-by: Cursor <cursoragent@cursor.com>
Manuel-Jentic
force-pushed
the
feat/oauth2-connect-flows
branch
from
September 16, 2026 10:06
8644c0f to
1ab323d
Compare
Collaborator
|
Rebased this branch onto
Verification post-rebase: 4427 unit/web/arch passed (82.7% cov), 179 control integration, lint/mypy clean, CLI Two notes:
Context for the urgency: this PR is the head of the theme-7 chain (epic #1374, Friday target) — phase 1b ( |
… dialog
The pick step's dialog title changed from "Choose an API" to "Add
credential" in this PR, but the four Playwright assertions (mocked and
docker) still waited for the old heading — both UI e2e CI jobs failed
deterministically. Heading-vs-button role keeps the locator unambiguous;
the suffix form ("Add credential — <api>") only appears after picking.
Co-authored-by: Cursor <cursoragent@cursor.com>
Collaborator
|
FYI for prioritization: theme 7's remaining work is staged behind this PR — Phase 1b ( |
Step 4 of the credential-binding-flow plan. New ``phase = 'rules'`` sits between ``configure`` (scopes) and ``awaiting`` (vendor round-trip) in both self and approve modes. Self mode: - Continue on the scopes page transitions to the rules page — no backend call yet. Seeds the rules list from the current scope classifications via the existing ``derivePermissionRules`` helper. - Continue on the rules page fires the combined ``:connect`` + ``:confirm`` mutation with the user's finalised rules and transitions to ``awaiting``. Approve mode: - Continue on the review page (scopes) transitions to the rules page. Rules pre-populate from the session's ``requested_permission_rules`` (from step 2's ``:connect`` capture) if the agent supplied any, otherwise from scope classifications. - Continue on the rules page fires ``:confirm``. Rules page (``RulesStep``): - Empty state: renders a greyed-out default ``Allow: GET /`` preview row with a "Skip & continue" button that persists the preset. Not condition-less (path constrained) so the backend model-validator accepts it. - Non-empty: renders each rule as a read-only row (effect pill + method list + path). Agent-requested rows carry a "requested by agent" badge so the human owner sees what came from the agent vs what they've since edited. - Add / Edit / Remove and the operation-impact preview land in the follow-up steps (5 + 6). Types: ``ReviewSession.requested_permission_rules`` added on the SPA side (matches the field surfaced by the router at step 2). Tests: existing self-mode flow test updated to click through both Continues; new approve-mode test pins the agent-requested rows rendering with the tag. Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
Step 5 of the credential-binding-flow plan. Turns the read-only rules
list from step 4 into a working editor.
- Each user-authored row now carries delete (X) + move-up / move-down
buttons. The greyed-out default preset stays uneditable — the
"Skip & continue" button is its only affordance.
- New inline ``AddRuleForm`` panel. Fields mirror
``PermissionRuleSchema``:
* effect toggle (allow / deny)
* methods multi-select (GET / POST / PUT / PATCH / DELETE)
* path input + match_mode select (prefix / exact / regex)
- Client-side validator reimplements
``PermissionRuleSchema._reject_condition_less_allow`` — an ``allow``
rule with no methods AND no path is refused inline with a red
message rather than 422'd by the server on Continue.
Test: new browser test drives the add flow, asserts the condition-less
allow is refused, and asserts the constrained rule renders as a row.
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
Step 2 of the credential-binding-flow plan. Lets an initiating agent hand its desired permission rules through at session creation so the human owner sees them pre-filled on the review page. Backend: - ``IntegrationsConnectRequest`` gains ``requested_permission_rules: list[PermissionRuleSchema]``. - New migration ``x5f6a7b8c9d0`` adds ``connect_sessions.requested_permission_rules jsonb NOT NULL DEFAULT '[]'::jsonb`` (chained after ``w4e5f6a7b8c9``). - ``ConnectSession`` ORM + ``ConnectSessionRepository.create`` gain the column. - ``ConnectSessionService.create_session`` persists; ``get_review_data`` + ``ReviewData`` + ``ReviewSessionResponse`` surface. Router re-validates on the way out through ``PermissionRuleSchema`` so the response contract stays honest. Store on the session — NOT on ``agent_permission_rules`` — because the rules are the initiator's *request*, only persisted onto the binding once the human confirms or edits them at ``:confirm``. Test: new integration test pins the round-trip (``:connect`` writes, ``get_review_data`` reads back) with a non-trivial two-rule payload. Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
Step 6 of the credential-binding-flow plan. Turns the rules-page from
step 5 into a live preview: the human sees, per operation, whether the
current rule set would allow or deny it, computed instantly in the
browser as rules are edited.
Backend:
- ``ReviewSessionResponse.api_reference`` (vendor + name + version)
added so the SPA can hit ``/apis/{vendor}/{name}/{version}/operations``
without duplicating ``canonical_credential_scope`` client-side.
``version`` is nullable — the catalog import runs async.
UI:
- ``lib/rule-matcher.ts`` — TS port of ``shared/permissions/matching.py``
+ ``broker/repos/rule_evaluator.evaluate_rules``. First-match-wins,
default-deny, fail-closed on invalid regex, condition-less-``allow``
skip. No per-op HTTP call.
- ``useVendorOperations`` hook fetches
``GET /apis/{v}/{n}/{ver}/operations``; ``listVendorOperations`` in
the client returns ``null`` on 404 so the "still importing"
skeleton renders instead of throwing. Polls every 2s while ``null``.
- ``OperationImpactPreview`` renders each op with a green (allow) /
red (deny) pill. Empty state + skeleton state distinct + friendly.
- Approve mode threads ``session.api_reference`` in; self mode passes
``null`` (session doesn't exist yet — preview skeletons until the
Continue click fires ``:connect`` + ``:confirm`` + import; UX
trade-off documented in the ``apiReference`` prop docstring).
Parity:
- Shared JSON fixture at ``tests/fixtures/rule-matcher-parity.json``
(15 cases) consumed by both Python and TS matcher tests. Any
divergence fails CI on both sides. Do NOT edit the fixture to make a
red test go green — fix the divergence at the source.
- Python side: ``tests/unit/shared/test_rule_matcher_parity.py``
(15/15).
- TS side: ``ui/src/shared/credentials/lib/__tests__/rule-matcher.test.ts``
(15/15).
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
Aligns the self-mode flow with the approve-mode shape from the moment
the dialog appears — both now have a session, credential, and enqueued
catalog import in place before the user reaches the rules page. Solves
the "operations still importing…" skeleton the previous self-flow got
stuck on because ``:connect`` didn't fire until rules-Continue.
Changes:
- ``VendorSelfConnectFlow`` fires ``:connect`` in a mount ``useEffect``
(guarded by a ref against React StrictMode's double-mount).
``:confirm`` fires at rules-Continue with the user's finalised
``{confirmed_scopes, permission_rules}``. Swaps
``useStartAndConfirmVendorConnect`` (combined) for the pre-existing
``useStartIntegrationConnect`` + ``useConfirmConnectSession`` pair.
- Scopes-page Continue is gated on the session id existing (so the
rules page has something to attach to) and shows a loading state
while ``:connect`` is still in-flight.
- Rules-page ``apiReference`` now comes from
``useConnectSession(session.id)`` — the operation-impact preview
works in the self flow as well as approve.
- Cancel-on-unmount + ``beforeunload`` widened to fire from any
pre-terminal phase (not just ``awaiting``). Same for the
scopes-page Back button, which now routes through ``handleCancel``
so a "opened dialog, changed mind, closed" leaves no orphan
session/credential/import.
- Errors from either ``:connect`` (mount) or ``:confirm`` (rules-Continue)
surface via a single ``flowError`` prop.
Test: ``start-and-confirm transitions…`` renamed to
``connect-on-mount, then confirm on rules-continue`` and updated to
reflect the new flow — ``GET /connect-sessions/{id}`` MSW handler
added, Continue-gating is asserted against the session id return.
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
…firm Follow-up to the mount-time ``:connect`` refactor. The ``_maybe_import_catalog`` call was still gated on ``:confirm`` running, so a user who opened the connect dialog and cancelled before reaching the rules-page Continue never enqueued an import. The operation-impact preview then sat on "importing…" indefinitely on subsequent runs for the same vendor because no import job had ever been created. Move the import to fire at ``:connect`` time, right after the session row is persisted. Idempotent so the ``:confirm`` call still runs defensively for edge cases (scanner-driven paths, races). Aligns with the plan §2 recommendation now that ``:connect`` is the earliest commitment point in both flows. Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
The rules-page operation-impact preview stayed on "importing…" even after the catalog import completed. Two gaps: 1. ``get_review_data`` was reading ``api_version`` off the credential row, which is created with ``version=None`` at ``:connect`` time and never gets updated — the imported OpenAPI decides its own version (e.g. ``1.1.4``) in the registry DB. 2. Even if the version updates, the SPA only fetched the review session once, so it never learned the new version. Fixes: - ``CatalogAutoImportProtocol`` gains ``current_version(api_id)`` — the same DI seam ``ensure_imported`` uses. ``InProcessCatalogAutoImporter`` looks up ``Api.version`` on the registry ``apis`` table by ``catalog_api_id`` gated on ``current_revision_id`` being set, so the value only appears once the import has finalised. - ``ConnectSessionService.get_review_data`` calls the new seam and returns the live version on the response instead of the always-null credential column. - ``useConnectSession`` polls every 2s while ``api_reference.version`` is null, and stops polling once it becomes a version string. The ops query is already gated on ``version`` so it enables the moment the poll surfaces the new value. Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
Introduced by 1b6abfee, the ``current_version`` seam's ``sqlalchemy`` and ``Api`` imports lived inside the function body — tripping ``tests/arch/test_no_inline_imports``. Moved them alongside the other top-level imports; no behavioural change. Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
… flow
Delivers a batch of UX and binding-model changes to ``VendorConnectFlow``
so a credential can actually reach an agent through the dialog, and so
the rules editor is safe to author against.
Rules editor:
- Drop the client-side derivation that seeded rules from scope
classifications. The list starts empty; a placeholder explains that
Continue-with-empty falls back to ``Allow GET /``. The default is
injected only when ``:confirm`` fires, so the user always sees exactly
what they authored.
- Fix invisible text in the Add form's raw ``<input>``/``<select>`` by
adding ``text-foreground`` and ``placeholder:text-input-placeholder``.
- Flag malformed / empty regex per row (``ruleValidityIssue``): the
row surfaces a "never matches" badge with a tooltip so the user
isn't puzzled by a red ops-preview grid.
- Add an Edit dialog on each row (Pencil icon). Shares ``RuleFormBody``
and ``validateDraft`` with the Add form so both paths agree on what
a valid rule looks like.
- Sort the operation-impact preview allowed first, denied last (with a
comment noting where a ``require-approval`` bucket lands when the
broker gains a third effect).
- Path autofill: a new ``path-completion.ts`` computes the next segment
from the loaded operations list; the input renders a
``<datalist>`` for browser autocomplete AND Tab-steps through
``/re → /repos/ → /repos/{owner}/`` boundaries.
Agent binding:
- Reintroduce the agent picker on the self-flow scopes page, labelled
by ``agent.name``. Continue is gated on an agent being chosen.
- ``VendorConnectFlow`` accepts ``preselectedAgentId``; when set the
picker renders disabled and locked to that id.
- ``BoundCredentialsCard`` on the agent detail page adds a
"Connect new integration" button that mounts ``CreateCredentialDialog``
with the current agent pre-selected (the existing "Bind existing"
path keeps its behaviour).
- Backend: ``ConfirmSessionRequest.agent_id`` is now accepted; the
service late-binds it onto the session row when the row had none,
and downstream binder/rules-write reads use ``effective_agent_id``.
Sessions opened without a target agent (self-flow, connect on
vendor click) can now be bound to a target chosen at
rules-page Continue-click.
Post-connect:
- New ``PostConnectBindMore`` renders on the terminal-success step:
ticks-and-binds the newly-created credential to additional agents.
Threaded via a ``renderPostConnect`` prop on ``VendorConnectFlow``
(module-level callers supply it — ``shared`` can't import from
``modules``). Additional bindings start in "start blocked" mode
(no rules); the user grants access per agent from each agent's page.
- New ``bindCredentialToAgentBlocked`` client + ``useBindCredentialToAgents``
hook in ``shared/credentials`` — loops the existing per-agent bind
endpoint (no bulk backend endpoint).
Tests:
- New unit tests for ``nextPathCompletion``.
- Existing ``VendorConnectFlow`` tests updated: agent picker is now
expected, ``:connect`` test selects the agent before Continue, edit
dialog test verifies pre-fill + write-back, ``preselectedAgentId``
test asserts the picker renders disabled and locked.
- ``BoundCredentialsCard`` tests updated for the "Bind existing"
rename + the new "Connect new integration" button.
OpenAPI + UI codegen regenerated for the ``agent_id`` addition.
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
…preview
Follow-up to 49ef1f97, addressing the round of feedback on the connect-flow
rules page.
Rules editor:
- Revert the text-only "no rules yet" placeholder. The empty state
once again shows a single greyed-out ``Allow GET /`` preview row
with a ``default`` tag so the user sees the exact rule that will
fire on Continue. Confirm-time injection semantics unchanged.
- Unify Add and Edit into the same shape: clicking Edit on a row
swaps it for an ``InlineEditRuleForm`` (same fields as the Add
form) rather than opening a modal Dialog. Save writes back at that
index; Cancel restores the row.
Path autocomplete:
- Replace the browser-native ``<datalist>`` with a compact custom
dropdown anchored directly under the input, capped to 5 rows and
styled with app design tokens. Filters as the user types
(previous datalist behaviour varied by browser). ArrowUp/Down
navigate, Enter selects, Escape closes; Tab still runs the
segment-stepper.
Ops preview:
- Fetch every page of ``GET /apis/.../operations`` via a new
``listAllVendorOperations`` client that follows ``next_cursor``
until exhaustion (was capped at the first 50-op page — most of a
vendor's surface was hidden).
- Group ops by first path segment (``/repos/``, ``/users/``, …)
with collapsible headers. Each header shows aggregate
``X allowed / Y denied`` counts; groups with any allowed ops
open by default, all-denied groups collapse.
- Template-aware evaluation: new ``lib/template-matcher.ts``
converts op templates like ``/repos/{owner}/{repo}`` to a regex
(``[^/]+`` per placeholder) and asks "does any concrete instance
satisfy the rule?". Backend enforcement is unchanged — it sees
concrete inbound paths and doesn't need template awareness — but
the UI was giving false-deny for exact/regex rules against
templated ops. Comment in the module explains the divergence.
- Example paths: allowed ops whose matching rule NARROWS the
template render a muted ``e.g. /repos/octocat/hello-world`` line
under the op. For regex rules, ``randexp`` samples concrete
strings that satisfy both the rule regex AND the template regex
(so a rule like ``/repos/octocat/[a-z-]+`` generates strings
starting with ``/repos/octocat/``, not gibberish).
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
…e ops preview
Backend rule matcher now honours ``{name}`` placeholders in exact / prefix
paths so users can author rules against the same template shape they see
on op definitions. UI ops preview goes hierarchical + partial-aware.
Backend + parity:
- ``PathMatcher`` gains a placeholder-aware compile branch:
``compile_matcher`` detects ``{name}`` in an exact/prefix path and
emits a regex where each placeholder is ``[^/]+``. ``exact`` uses
``fullmatch`` at enforce time; ``prefix`` uses ``re.match`` so a
rule like ``/repos/{owner}/{repo}`` accepts every deeper request
path with the same segment shape. Paths without placeholders keep
the original literal comparison path unchanged.
- TS ``rule-matcher.ts`` mirrors the same compile behaviour (parity
is enforced by ``tests/fixtures/rule-matcher-parity.json`` — five
new cases pin placeholder semantics for exact + prefix on both
sides).
- No new failure mode for legacy rows: paths that don't contain
``{`` fall through to the pure-literal branches on both sides.
Ops preview UX:
- Rewrite the flat "one group per first path segment" preview as a
recursive tree keyed on each successive path segment
(``/repos/`` contains ``/repos/{owner}/`` contains
``/repos/{owner}/{repo}/`` …). Every level starts COLLAPSED — the
aggregate ``X allowed / Y denied`` chips on the header let users
skim the surface without expanding anything, and expand is opt-in
per layer.
- Replace the single-example "e.g." line with a sample-and-classify
verdict per op. ``classifyOpCoverage`` generates concrete sample
paths from the op template (biased with rule-derived candidates so
narrow rules produce realistic hits) and runs the enforce-time
matcher to bucket allowed vs denied. Verdict is one of ``allow`` /
``partial`` / ``deny``. Partial rows are expandable: they show 1-2
concrete allowed samples AND 1-2 concrete denied samples so the
user sees the actual slice a narrow rule covers rather than a
single misleading example.
- Rule rows that touch zero imported operations render muted with
a "no ops affected" warning badge, matching the existing "never
matches" pattern for malformed regex — silent-fail rules were
invisible before.
Tests:
- Extend ``template-matcher.test.ts`` with cases for
``sampleTemplatePaths`` (rule-derived candidates) and
``classifyOpCoverage`` (allow / partial / deny). 179 UI tests
pass; 20 Python parity tests pass (including the new
placeholder cases).
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
The deep hierarchy from ``b2cdeb74`` was accurate but painful to navigate on real APIs — GitHub-sized surfaces produced 4-5 layers of click-throughs for a single op. Roll back to the flat one-level grouping by first path segment (``/repos/``, ``/users/`` …) that we had in ``ce6bedc8`` and rely on the partial verdict on each leaf row for the fine-grained nuance. Partial-op display tightened: the expandable-list layout is replaced with two inline lines directly under the op path — ``ALLOW e.g. <one sample>`` and ``DENY e.g. <one sample>``. One example per bucket keeps large groups scannable while still surfacing the "narrow slice of a broader op" case the sample-and-classify pass was built for. No changes to backend, template matcher, or sample-and-classify — just the presentation of the results. Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
The always-inline ``ALLOW e.g.`` / ``DENY e.g.`` lines under every partial op made large groups too noisy — each partial row grew to three lines and dominated the surrounding surface. Move them behind a click: partial rows now carry a caret and start collapsed, expand on click to show the same one-per-bucket sample pair. Fully-allowed and fully-denied rows keep their single-line shape (no caret, no expand — nothing more to reveal). Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
…review
Three-part follow-up on the agent-detail bindings UX.
1. Combine agent-page bind buttons into one.
``BoundCredentialsCard`` previously exposed both a "Connect new
integration" button (OAuth flow with the agent pre-selected) and a
"Bind existing" button (opened the legacy ``BindAgentCredentialDialog``
picker). Fold them into a single ``Connect integration`` header
action that runs the connect wizard. The empty-state inline CTA is
updated to match ("Connect an integration to grant it API access").
``BindAgentCredentialDialog`` is left defined but no longer wired
from this page.
2. Reuse the ops-preview visualisation on the agent page.
Extract ``OperationImpactPreview`` from ``VendorConnectFlow.tsx``
into its own module under ``shared/credentials/components/`` so
both the connect-flow rules page and the agent-detail per-binding
editor render the exact same grouped, allow/partial/deny preview.
``AgentBindingPermissionsEditor`` accepts an ``apiReference`` prop
(derived from the binding's ``serves`` entry) and renders the
preview under the rule editor with a scoped ``label`` prop
("Effective access for this binding"). Users editing binding rules
now see draft-rule impact against real ops as they type — same
surface they saw during connect.
3. Add tests covering the recently-introduced codepaths.
* ``VendorConnectFlow.test.tsx``: partial-op row expands to show
``ALLOW e.g.`` + ``DENY e.g.`` samples; rule that touches no
imported op renders the "no ops affected" warning; path
autocomplete supports keyboard nav (ArrowDown + Enter commits
the highlighted suggestion).
* ``BoundCredentialsCard.test.tsx``: single ``Connect integration``
button opens the shared ``CreateCredentialDialog``; ops preview
renders inside the binding rule editor when the served API is
fully resolved.
308 UI tests pass; existing arch / parity tests unchanged.
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
… on binding rows
Two-part follow-up on the agent-detail bindings surface.
1. Extract the compact rule editor to a shared component.
The connect-flow's rules page authored rules with a compact single-
line row + expandable inline form (``AddRuleForm`` +
``InlineEditRuleForm`` + ``RuleFormBody``), while the agent-detail
editor used the older ``PermissionRuleEditor`` with per-row fields
always visible. Two different aesthetics for the same conceptual
task. Fold both into a new
``shared/credentials/components/RuleListEditor.tsx`` — the
connect-flow's compact form promoted to a first-class shared
component with autocomplete + no-ops-affected warnings baked in.
Callers pass ``rules`` + ``onChange`` and (optionally) op paths /
templates; internal state (draft form, dropdown open/close,
highlight) stays private. ``AgentBindingPermissionsEditor`` swaps
its ``PermissionRuleEditor`` usage for ``RuleListEditor`` via
``PermissionRuleInput ↔ PermissionRule`` adapters.
2. Ops preview visible on the binding row (not just when editing).
Move ``OperationImpactPreview`` out of the collapsible rule editor
and render it on the binding row itself so users see the
effective-access surface at a glance. Version resolution changes
too: the binding's ``serves`` entry rarely carries a version, so we
now fetch the credential via ``useCredential(binding.credentialId)``
and use its always-populated ``api.{vendor, name, version}``. The
editor still receives an ``apiReference`` for its own autocomplete /
warnings needs — sourced from the same resolved reference.
Empty-state row: ``DefaultRulePreviewRow`` (a read-only greyed-out
``Allow GET /`` renderer, wired through ``RuleListEditor``'s new
``emptyStateContent`` prop) replaces the inline block the connect-flow
used to duplicate. One canonical greyed-default row across surfaces.
Tests: the pending-changes-diff test is rewritten around the new
compact-editor flow (open outer editor → click row Pencil → toggle
method → inline Save → outer Save). The binding-row preview test now
mocks the credential row and asserts the "Effective access for this
binding" label without opening the editor. 308 UI tests pass.
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
Post-rebase repair — my earlier commits ``8f377599`` (accept requested_permission_rules at :connect) and later variants referenced ``PermissionRuleSchema`` in ``control/web/schemas/integrations.py``, but Manuel's rebase-onto-main didn't carry the import over because the pre-rebase origin file didn't need it. Add the import in isolation here rather than force-amending upstream commits. Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
Post-rebase repair — Manuel's rebase-onto-main kept the pre-existing
narrow ``{method, path, effect}`` shape in ``vendors-types.ts`` because
the rebase base predates my expansion to the theme-5 shape. Update the
type to match ``PermissionRuleSchema`` on the backend
(``{effect, methods?, path?, match_mode?, operations?, comment?}``) so
TypeScript accepts the rule-editor + matcher references introduced by
the later cherry-picked commits.
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
Manuel's rebase kept a ``_to_binding_rules`` helper that translated the
old approve-page rule shape (``{method, path, effect}`` with ``/**``
glob paths) into the canonical ``AgentPermissionRule`` shape. My schema
commit further up the stack (``8f377599`` / ``2e12290d`` /
``38343fe6``) already replaced the wire type with
``PermissionRuleSchema`` — the canonical shape reaches ``confirm()``
directly. The stale translation was silently mangling the new-shape
input (reading ``rule["method"]`` on a dict that only carries
``rule["methods"]``) and would have stored empty ``methods=None``
rules at runtime.
Delete the helper, pass ``permission_rules`` through to
``AgentPermissionRuleRepository.replace_user_rules`` unchanged, and
drop the now-unused ``import re``. Update the confirm signature's
type hint from ``list[dict[str, str]]`` to ``list[dict[str, object]]``
to match the actual shape. Update the one integration test that
still used the legacy dict shape to the canonical form.
Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
Regen after the schema changes cherry-picked onto the rebased origin (``requested_permission_rules`` on connect + review responses, ``agent_id`` on confirm, ``PermissionRuleSchema`` replacing the legacy ``PermissionRuleModel``). Deletes the obsolete generated ``PermissionRuleModel`` client type; ``ConfirmSessionRequest`` gains the ``agent_id`` field. Runs ``make openapi`` + ``cd ui && npm run codegen`` — no hand edits. Signed-off-by: DavidAtJentic <212357562+DavidAtJentic@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Introduces agent-driven OAuth 2.0 connect flows. Agents (or users) call
POST /integrations:connectagainst a config-seeded vendor registry, the platform runs a state machine through eitherRFC 8628device authorization orRFC 6749authorization code, and the associated API is auto-imported into the catalog. This change would support a curated set of public API clients shipping with the service as well as eventually integrations that support Dynamic Client Registration or any other dynamic/procedural client format.Changes
/integrations:connect,/connect-sessions/{id}[:confirm|:cancel|/status],/vendors,/vendors/{key}/auth-capabilitiesendpoints. NewConnectSessionServicestate machine +AuthFlowHandlerprotocol withDeviceAuthorizationHandlerandAuthCodeFlowHandlerimplementations. Server-driven polling via a newConnectPollScannerbackground job. Shared/credentials/oauth/callbackroute routes to the standalone-credential or connect-session path via asidstate-JWT claim; replay protection lives in a shared prologue used by both entrypoints.vendors.entries.<slug>registry surface — per-vendor flows (device_authorization / authorization_code), scope catalog with read/write classification, identity probe.connect_sessions,device_authorization_credentials,agent_credential_permissionstables;credentials.statecolumn added with backfill.VendorConnectFlow(self + approve modes) +DeviceCodeConnectDialog, wired into the credentials dialog. Agent'sreasonfree-text surfaces on the approve page.validate_upstream_urlon five vendor-facing httpx calls, https-only guard on six vendor-URL navigation sites, session-id enumeration oracle closed on/status+:cancel, per-actor rate limit on:connect,navigator.sendBeaconfor tab-close cancel, boot-time vendor-shape validator.sessions_created_total,sessions_terminal_total(vendor + flow + outcome labels),time_to_connected_seconds.Risk & rollback
credentials. Reversible viaalembic downgrade; empty tables + column drop.vendors.entries) is off-by-default — an empty registry disables the flow entirely. No changes required to existing deployments.credentials:connect— narrower thancredentials:write, additive to the permission catalog.Test plan
make checkpasses (lint, mypy, arch tests, secrets audit).make test-arch: 150/152 pass; the two remaining failures are the vendored-orm-facts drift pending on a companion rules-repo PR (chore/orm-facts-add-device-auth-credentials).Review guide
Start with
src/jentic_one/control/services/integrations/connect_session_service.py(state machine + finalise sequence) andflow_handlers/base.py(AuthFlowHandlerprotocol). Then the two handlers, thenweb/routers/integrations.py. UI review starts atui/src/shared/credentials/components/VendorConnectFlow.tsx. Security and quality fixes surfaced during local agent reviews are landed as their own commits.Related issue
Checklist
git commit -s, DCO)make checkpasses (lint, type check, secrets audit, arch tests)