mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-10 23:15:40 +00:00
905baaa010
* feat(oauth): backfill session.Extra into oauth_connections + switch read path (TASK-1522) Phase C1 for PLAN-1519. Seeds existing OAuth grant chains into the new connection tables (Phase A) and switches /console/connected-apps to read from them, retiring the session.Extra parse on the read path. Backfill (internal/store/oauth_connections_backfill.go) - Walks oauth_access_tokens + oauth_refresh_tokens to find every distinct request_id chain (including refresh-only chains). - Picks the newest token row per chain — its session.Extra drives the seeded shape, so a chain whose user re-scoped recently reflects the latest decision. - Maps session.Extra shapes to the new tables per IDEA-1517 §2: no key → all_current=1; ["*"] → all_current=1; explicit slugs → all_current=0 + one join row per slug (added_by='user'). - Resolves slugs → workspace IDs; unresolved slugs (deleted / renamed workspace) are counted + logged at WARN, not fatal. - Idempotent on every INSERT (OR IGNORE / ON CONFLICT DO NOTHING) so re-running on every startup is a cheap no-op once stable. - Returns a BackfillOAuthConnectionsResult so the startup log reports chains_seen / connections_created / workspaces_added / unresolved_slugs — operators see fresh work and notice drift. Read-path rewrite (internal/store/connected_apps.go) - ListUserOAuthConnections projects AllowedWorkspaces from GetOAuthConnectionAccess (oauth_connection_workspaces JOIN workspaces) instead of parsing session.Extra strings. - Hydrates Name + MayCreate + AllCurrent + IncludeFuture from oauth_connections so Phase D's mutation UI has them. - Defensive fallback for chains without an oauth_connections row (any leftover the backfill missed): treats as legacy "any workspace, default-on flags" so the connection still renders. Backfill at startup keeps this branch unreachable in production. - Retires parseAllowedWorkspacesFromSession; the new extractAllowedWorkspacesFromSessionExtra helper in oauth_connections_backfill.go is the only consumer of the session.Extra shape on the store side. Model (internal/models/connected_apps.go) - Adds Name / MayCreateWorkspaces / AllCurrentWorkspaces / IncludeFutureWorkspaces. AllowedWorkspaces semantics stay stable (nil = "any"; explicit slugs = chip list) so the existing DTO + frontend continue working unchanged. Phase D exposes the new fields on the wire. Startup wiring (cmd/pad/main.go) - After srv.SetOAuthServer / SetClaimSecret, run the backfill once. Non-fatal on error (partial state is consistent and the next run completes). Quiet at the Debug level on steady-state re-runs; INFO when fresh work landed. Tests - 8 BackfillOAuthConnections cases: empty DB, pre-TASK-952 (no key), wildcard, explicit list, mixed resolvable/unresolved slugs, multi-row chain newest-row-wins, refresh-only chain, idempotent re-run (verified via post-run row count). - TestExtractAllowedWorkspacesFromSessionExtra replaces the retired parseAllowedWorkspacesFromSession test — covers all three IDEA-1517 §2 input shapes + malformed/non-array defensive cases. - TestListUserOAuthConnections_DeduplicatesChain + TestHandleListConnectedApps_DTOShapeAndAuditEnrichment updated to call BackfillOAuthConnections (the production startup hook) before asserting on AllowedWorkspaces — mirrors the real-world flow now that the read path no longer parses session.Extra inline. Parent: PLAN-1519. * fix(oauth): backfill counters reflect actual new rows per Codex review (round 1) PR #583 Codex review round 1 flagged that the backfill counters over-report on steady-state restarts: - wasFreshlyInserted compared updated_at vs created_at — true for every untouched existing row, so every restart counted every pre-existing connection as "created." - slugsAdded++ ran after AddConnectionWorkspace regardless of whether the INSERT OR IGNORE / ON CONFLICT DO NOTHING hit an existing row. Net effect: startup logs "backfill complete" with non-zero counts on every restart instead of the intended quiet "no-op" path — making real fresh work indistinguishable from steady-state. Fix: probe existence BEFORE the insert on both sides. - backfillOneChain reads GetOAuthConnection first; only sets created=true and runs insertOAuthConnectionIfAbsent on a miss. - Per-slug: IsConnectionWorkspaceAllowed pre-check; skip + don't increment when the row already exists. Two cheap PK / indexed lookups per chain. Pre-Phase-C deployments have small chain counts so the added cost is well below the scan already running. Removed the now-unused wasFreshlyInserted helper. Added an assertion in TestBackfillOAuthConnections_Idempotent that both ConnectionsCreated and WorkspacesAdded report 0 on the second run — the regression guard for this exact finding. Parent: PLAN-1519. * fix(oauth): backfill skips slug re-seed on existing rows per Codex review (round 2) PR #583 round 2 caught that the round-1 fix protected the parent oauth_connections row from re-seed but left the join table mutable from stale session.Extra: When a user removes a workspace from their connection's allow-list via Phase D's mutation UI (RemoveConnectionWorkspace), the next server restart would re-run the backfill, find the parent row intact, and re-INSERT the removed slug from the original session.Extra. The user's removal would silently revert every restart. Fix: backfill is a one-shot seed. Once the parent row exists, the new tables are authoritative — legacy session.Extra is frozen reference data, not a reconciliation source. The slug loop only runs when we just inserted a fresh parent row. Added TestBackfillOAuthConnections_DoesNotResurrectRemovedWorkspace as the regression guard: seeds two slugs, removes one, runs backfill again, asserts the removed slug stays gone and the kept slug is untouched. Parent: PLAN-1519. * fix(oauth): atomic per-chain backfill transaction per Codex review (round 3) PR #583 round 3 caught that round 2's "only seed slugs on fresh parent" gate introduced a permanent-partial-state risk: if the process crashes (or AddConnectionWorkspace errors) between inserting the parent row and finishing the slug loop, the next backfill sees created=false, short-circuits the slug seeding, and leaves the connection permanently scoped to a partial allow-list. Fix: per-chain transaction. Parent insert + every slug insert land in one BEGIN/COMMIT pair; any mid-loop failure rolls everything back. The next backfill then sees the chain as un-seeded and retries from scratch — preserving both round 2's "no-resurrection of user-removed slugs" (existence probe inside the tx) and round 3's "no permanent partial seed" (atomic commit). Scope: per-chain (small tx), not whole-backfill. The original no-transaction rationale was about lock-hold duration across thousands of chains; that doesn't apply at chain granularity (one parent + a handful of join rows = sub-millisecond hold). Removed the now-unused insertOAuthConnectionIfAbsent helper; the INSERTs live inline within the transaction. Added TestBackfillOAuthConnections_AtomicOnMidLoopFailure as the regression guard: forces a mid-loop INSERT failure via a duplicate slug in session.Extra (which violates the join table's PK on the second insert), asserts the parent row rolled back, then runs a clean retry and verifies full seed completion. Parent: PLAN-1519. * fix(oauth): surface store errors from backfill + list path per Codex review (round 4) PR #583 round 4 caught two silent-fallthrough paths that could leak partial/incorrect state instead of failing loudly: 1. Backfill slug loop: GetWorkspaceBySlug errors were treated the same as "workspace not found" — both incremented slugsMissed and continued. A real I/O error mid-loop would commit a partial allow-list, and the next backfill's parent-exists short-circuit would make that partial scope permanent. Fix: distinguish (nil, nil) "not found" from (nil, err) "real failure" — return the error so the per-chain transaction rolls back and the next run retries cleanly. 2. ListUserOAuthConnections hydration: GetOAuthConnectionAccess and GetOAuthConnection errors collapsed into the "no oauth_connections row" defensive-fallback branch, returning the legacy "any workspace, default-on flags" shape. On a real store failure that silently broadens a user's scope — e.g. a connection the user explicitly removed a slug from would render as "Any workspace" until the store recovered. Fix: surface store errors from both calls; the defensive fallback path is now exclusively for HasConnection=false, not for error masking. Both findings tighten the failure mode from "silently emit broadened/partial state" to "surface the error so retries happen against accurate data." Existing tests cover the happy paths; the failure paths are exercised by I/O errors against the same store interfaces (no new test added — the change is "return err instead of swallow it" and the assertion of NOT swallowing is the diff itself). Parent: PLAN-1519.
102 lines
4.3 KiB
Go
102 lines
4.3 KiB
Go
package models
|
|
|
|
import "time"
|
|
|
|
// Connected-apps models (PLAN-943 TASK-954).
|
|
//
|
|
// A "connected app" is one OAuth client (Claude Desktop, Cursor, …)
|
|
// that the user has authorized to act on their behalf via the MCP
|
|
// surface. Each user-authorized OAuth flow produces one connection,
|
|
// identified by the OAuth grant chain's request_id (preserved across
|
|
// refresh-token rotations — see internal/store/oauth.go for the
|
|
// chain semantics). Revoking a connection invalidates every token in
|
|
// that chain so the next /mcp call from that client gets 401.
|
|
|
|
// CapabilityTier classifies the granted scopes into a coarse,
|
|
// user-readable bucket. Used by the connected-apps page so users
|
|
// don't have to read raw OAuth scope strings to understand what a
|
|
// given client can do.
|
|
type CapabilityTier string
|
|
|
|
const (
|
|
CapabilityTierReadOnly CapabilityTier = "read_only"
|
|
CapabilityTierReadWrite CapabilityTier = "read_write"
|
|
CapabilityTierFullAccess CapabilityTier = "full_access" // includes pad:admin
|
|
CapabilityTierUnknown CapabilityTier = "unknown" // empty / unparseable scopes
|
|
)
|
|
|
|
// OAuthConnection is one row of the connected-apps page. Joins
|
|
// oauth_access_tokens (the active grant) → oauth_clients (the DCR
|
|
// metadata). The audit-log enrichments (LastUsedAt + Calls30d) come
|
|
// from a separate aggregate query against mcp_audit_log so the page
|
|
// loads in two queries instead of N+1.
|
|
//
|
|
// One value per OAuth grant chain — the connections list deduplicates
|
|
// across refresh-token rotations because every chain member shares
|
|
// the same RequestID.
|
|
type OAuthConnection struct {
|
|
// RequestID is the OAuth grant chain identifier — preserved
|
|
// across refresh-token rotations and used as the public ID for
|
|
// revoke + audit drilldown ("connection_id" on the wire).
|
|
RequestID string
|
|
|
|
// ClientID / ClientName / LogoURL / RedirectURIs come from the
|
|
// DCR registration metadata (oauth_clients table). ClientName is
|
|
// what shows on the consent screen + connected-apps cards
|
|
// (e.g. "Claude Desktop").
|
|
ClientID string
|
|
ClientName string
|
|
LogoURL string
|
|
RedirectURIs []string
|
|
|
|
// AllowedWorkspaces is the workspace allow-list. Post-TASK-1522
|
|
// it's the projection of oauth_connection_workspaces rows joined
|
|
// to workspaces by ID (slug list, sorted). Two shapes the UI
|
|
// renders against:
|
|
// - nil — the connection's all_current_workspaces flag is on
|
|
// (covers every workspace the user is a member of). UI shows
|
|
// an "Any workspace" badge.
|
|
// - explicit slugs — all_current_workspaces=false; UI shows the
|
|
// slug list as chips.
|
|
// (Pre-TASK-1522 the same field also held ["*"] for wildcard
|
|
// tokens; the backfill normalizes those to the flag and the field
|
|
// is nil for them now. The wire DTO's nullable shape is unchanged
|
|
// so frontend code keeps working.)
|
|
AllowedWorkspaces []string
|
|
|
|
// Connection-level scope flags + name from oauth_connections
|
|
// (PLAN-1519 / TASK-1520 / IDEA-1517 §2). Pre-TASK-1522 these
|
|
// were unsourced (the field didn't exist); post-backfill every
|
|
// active connection has a row and the values mean what the page
|
|
// should render. Phase D's mutation UI reads + writes these.
|
|
//
|
|
// Name is empty string for backfilled rows that haven't been
|
|
// renamed yet — UI prompts on first connections-page visit.
|
|
Name string
|
|
MayCreateWorkspaces bool
|
|
AllCurrentWorkspaces bool
|
|
IncludeFutureWorkspaces bool
|
|
|
|
// GrantedScopes is the raw space-separated scope string fosite
|
|
// recorded at grant time. Surfaced in the per-connection expand
|
|
// drilldown alongside CapabilityTier so power users can see the
|
|
// exact policy.
|
|
GrantedScopes string
|
|
|
|
// CapabilityTier is the human-readable bucket derived from
|
|
// GrantedScopes (read_only / read_write / full_access / unknown).
|
|
CapabilityTier CapabilityTier
|
|
|
|
// ConnectedAt is the earliest requested_at across the chain —
|
|
// when the user originally authorized this connection. Survives
|
|
// refresh-token rotation because every chain member shares the
|
|
// same request_id and the MIN over the chain stays anchored.
|
|
ConnectedAt time.Time
|
|
|
|
// LastUsedAt + Calls30d come from MCPConnectionStatsForUser
|
|
// (TASK-960's audit-log aggregate). Nil LastUsedAt means "no
|
|
// audit entries yet" — the page renders "—" instead of a date.
|
|
LastUsedAt *time.Time
|
|
Calls30d int
|
|
}
|