diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cb69849b7..906921a59 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1304,10 +1304,10 @@ replaced; do not carry obsolete compatibility code forward to satisfy this secti attached Vault or anonymous MCP. Already-resolved tokens and running Sessions are not revoked. Exact hosted archive, visibility, overlapping-mutation and error semantics remain unverified; row removal does not prove physical storage erasure. -- Static-bearer Credentials are children of tenant-owned Vaults in the execution +- Static-bearer and OAuth Credentials are children of tenant-owned Vaults in the execution database. Creation admits the owner in the same SQL statement as the insert; - retrieval joins the owning Vault and selects public metadata only. No public - operation decrypts or returns a token. Encrypt before passing secret values to + retrieval joins the owning Vault and selects public metadata only. Public reads do not decrypt or return a token. OAuth replacement authenticates + the stored grant before applying a partial update. Encrypt before passing secret values to SQL, using the execution service's separately configured random 32-byte key and the standard library's random-nonce AES-GCM. The versioned authenticated binding includes tenant, Vault, Credential, authentication purpose and exact destination. @@ -1315,8 +1315,8 @@ replaced; do not carry obsolete compatibility code forward to satisfy this secti this storage boundary. Missing key configuration disables credential writes; malformed explicit configuration fails startup. See [`services/agents-api/credentials.md`](services/agents-api/credentials.md) for - key persistence and current limits. OAuth and storage-key rotation remain - separate gaps; resource creation never contacts the destination. + key persistence and current limits. Storage-key rotation remains + separate work; resource creation never contacts the destination. - `GET /v1/vaults/{vault_id}/credentials` lists safe metadata only, with both project and Vault ownership enforced on the parent, cursor and row query. An inaccessible parent returns not-found, even when the collection would be empty. @@ -1327,10 +1327,10 @@ replaced; do not carry obsolete compatibility code forward to satisfy this secti Synthetic archived fixtures prove filtering only. There is no public archive writer, timestamp or delete-to-archive inference; existing create/retrieve/token replacement, Session bindings and dispatch keep their rules. Migration rollback refuses to lose - archived classification. Archive/revocation lifecycle, OAuth and hosted - query/concurrency semantics remain gaps. -- Credential `POST /v1/vaults/{vault_id}/credentials/{credential_id}` replaces only - the static-bearer token and update time. Require `auth.type=static_bearer` and a + archived classification. Archive lifecycle and hosted query/concurrency + semantics remain gaps. +- For static auth, Credential `POST /v1/vaults/{vault_id}/credentials/{credential_id}` + replaces only the token and update time. Require `auth.type=static_bearer` and a string `auth.token`, preserving opaque bytes; reject extra mutation fields before writing. Reuse safe metadata for the immutable encryption binding, then scope the atomic SQL mutation independently by tenant, Vault, Credential, static auth type @@ -1340,7 +1340,29 @@ replaced; do not carry obsolete compatibility code forward to satisfy this secti and Session snapshots stay unchanged. Subsequent dispatch reads use the committed replacement through existing scoped lookup; already-resolved requests may retain the old token. This is not storage-key rotation, in-flight revocation or hot reload. - OAuth and exact hosted concurrent-update/retry/timestamp semantics remain gaps. + Exact hosted concurrent-update/retry/timestamp semantics remain gaps. +- OAuth grant ownership stays in Core. The application performs authorization and + provider revocation; do not add public login/callback/refresh/revoke routes. + Store access/refresh/client secrets together under existing authenticated tenant, + Vault, Credential, auth-type and destination encryption. Authenticate refresh + metadata against its encrypted copy before using an endpoint or grant. Read/list + queries still select safe metadata only. Shared MCP selection admits both auth + types and freezes one identity without changing native adapter contracts. + At dispatch, a known-expired grant is refreshed through the declared endpoint + auth method, with stored scope/resource, then persisted before returning access. + Serialize refresh and replacement with the same PostgreSQL Credential row lock; + deletion and Vault cascade cannot be undone by a stale refresh. Network exchanges + are bounded and fail closed; never return provider error bodies or claim an + uncertain grant exchange was committed. No background scheduler, 401 retry, + hot replacement, output repair or harness-specific OAuth path is introduced. + Refresh uses verified HTTPS, rejects redirects, and checks resolved addresses + before dialing them. Private issuer origins need explicit operator configuration + in `AGENTS_API_OAUTH_TRUSTED_ORIGINS`; tenants cannot relax that boundary and TLS + verification remains mandatory. Keycloak is acceptance infrastructure only. + Preserve the pinned update omission/null and immutable-field rules described in + [OAuth credentials](services/agents-api/oauth-credentials.md); record unspecified + hosted semantics. Native processes receive only access tokens. Provider revocation, + withdrawal of already-dispatched tokens and Session cancellation remain distinct. - Credential `DELETE /v1/vaults/{vault_id}/credentials/{credential_id}` removes one owned row, including ciphertext, with tenant/Vault/ID checked in the same SQL mutation. It needs no encryption key, secret read or network call. Local reads, @@ -1387,7 +1409,7 @@ replaced; do not carry obsolete compatibility code forward to satisfy this secti pinned resource types. Saved-Agent updates never change existing Session snapshots; per-Session tools replace the whole field. The initial profile admits HTTP(S), boolean `required` (default false), empty/null metadata and empty/null headers. - Static bearer authentication requires HTTPS and the attached-Vault rules below. + Static and OAuth bearer authentication require HTTPS and the attached-Vault rules below. Inline authorization, URL userinfo/query/fragment, other origins and stdio remain explicitly unsupported. - Codex required MCP initialization additionally needs `mcp_http_required`, advertised @@ -1426,19 +1448,19 @@ replaced; do not carry obsolete compatibility code forward to satisfy this secti with TLS verification. This execution profile rejects empty values and bytes outside RFC 6750 b64token syntax with generic errors; it never trims tokens or narrows opaque Credential - storage. OAuth and hosted redirect/error equivalence - remain separate work. + storage. Core-managed OAuth uses this same access-token path; native OAuth + login/refresh and hosted redirect/error equivalence remain separate work. - Session `vault_ids` omission/null/empty means `[]`; nonempty attachments must all belong to the authenticated tenant. Preserve caller order and public MCP `credential_id`. Saved Agents may store a nullable/nonempty credential reference without authorizing its use. Session admission resolves an explicit credential only inside attached Vaults for the exact declared URL, or selects the unique - matching static credential when the ID is omitted/null. No match remains + matching static or OAuth credential when the ID is omitted/null. No match remains anonymous; ambiguity is a local 400 and unavailable references use the same 404. Resolve before any Session, initial input or event write. Freeze safe bindings, including anonymous decisions, in private Session configuration; never populate the public credential field from implicit resolution. At actual dispatch, recheck - tenant, attached Vault, selected ID, static auth type and exact URL before scoped + tenant, attached Vault, selected ID, frozen auth type and exact URL before scoped decryption. Metadata queries select no ciphertext; tokens enter only the existing transient daemon request. Selected authentication requires `mcp_http_bearer_auth` during device selection and the final preclaim check. Missing/wrong keys or diff --git a/apps/web/e2e/vault-oauth-metadata.spec.ts b/apps/web/e2e/vault-oauth-metadata.spec.ts new file mode 100644 index 000000000..e65de9430 --- /dev/null +++ b/apps/web/e2e/vault-oauth-metadata.spec.ts @@ -0,0 +1,40 @@ +import { expect, test } from "@playwright/test"; + +const fixtureBaseUrl = `http://127.0.0.1:${process.env.AGENTS_FIXTURE_PORT ?? 18092}`; + +test("loads mixed OAuth and static credentials without offering OAuth static replacement", async ({ page, request }) => { + expect((await request.post(`${fixtureBaseUrl}/__fixture/reset`)).ok()).toBe(true); + const vaultId = "11111111-1111-4111-8111-111111111111"; + const vault = { id: vaultId, object: "vault", created_at: 1, name: "Mixed credentials", metadata: {} }; + const oauth = { + id: "22222222-2222-4222-8222-222222222222", vault_id: vaultId, + object: "vault.credential", name: "OAuth MCP", created_at: 2, updated_at: 2, + auth: { + type: "mcp_oauth", mcp_server_url: "https://oauth.example/tools", expires_at: null, + refresh: { + client_id: "public-client-id", token_endpoint: "https://issuer.example/token", + token_endpoint_auth: { type: "client_secret_basic" }, resource: null, scope: null, + }, + }, + }; + const staticCredential = { + id: "33333333-3333-4333-8333-333333333333", vault_id: vaultId, + object: "vault.credential", name: "Static MCP", created_at: 2, updated_at: 2, + auth: { type: "static_bearer", mcp_server_url: "https://static.example/tools" }, + }; + await page.route("**/v1/vaults?*", (route) => route.fulfill({ json: { + object: "list", data: [vault], has_more: false, first_id: vaultId, last_id: vaultId, + } })); + await page.route(`**/v1/vaults/${vaultId}/credentials?*`, (route) => route.fulfill({ json: { + object: "list", data: [oauth, staticCredential], has_more: false, first_id: oauth.id, last_id: staticCredential.id, + } })); + await page.goto("/"); + await page.getByRole("button", { name: "Vaults", exact: true }).click(); + await expect(page.getByRole("heading", { name: "Mixed credentials" })).toBeVisible(); + await expect(page.getByText("OAuth MCP", { exact: true })).toBeVisible(); + await expect(page.getByText("OAuth · Manage authorization and token replacement in your application.")).toBeVisible(); + await expect(page.getByRole("button", { name: "Replace token for OAuth MCP" })).toHaveCount(0); + await expect(page.getByRole("button", { name: "Delete OAuth MCP" })).toBeVisible(); + await page.getByRole("button", { name: "Replace token for Static MCP" }).click(); + await expect(page.getByRole("dialog", { name: "Replace token · Static MCP" })).toBeVisible(); +}); diff --git a/apps/web/src/features/vaults/VaultsView.test.tsx b/apps/web/src/features/vaults/VaultsView.test.tsx index a9e5d318a..c5f95fc29 100644 --- a/apps/web/src/features/vaults/VaultsView.test.tsx +++ b/apps/web/src/features/vaults/VaultsView.test.tsx @@ -41,6 +41,30 @@ describe("Vaults view", () => { expect(html).not.toContain("archived"); }); + it("shows OAuth metadata with application guidance and only offers static token replacement", () => { + const vaultId = "11111111-1111-4111-8111-111111111111"; + const html = renderToStaticMarkup(); + expect(html).toContain("OAuth MCP"); + expect(html).toContain("Manage authorization and token replacement in your application."); + expect(html).not.toContain('aria-label="Replace token for OAuth MCP"'); + expect(html).toContain('aria-label="Replace token for Static MCP"'); + expect(html).toContain('aria-label="Delete OAuth MCP"'); + }); + it("keeps an incomplete catalog in a durable failed state", () => { const html = renderToStaticMarkup(); expect(html).toContain("Couldn’t load Vaults"); diff --git a/apps/web/src/features/vaults/VaultsView.tsx b/apps/web/src/features/vaults/VaultsView.tsx index 7c00dafab..b6f5ead78 100644 --- a/apps/web/src/features/vaults/VaultsView.tsx +++ b/apps/web/src/features/vaults/VaultsView.tsx @@ -149,9 +149,9 @@ export function VaultsView({ busy, catalog, coreError, coreState, operations }: {credentials.length ? credentials.map((credential) => (
diff --git a/contracts/agents-api/README.md b/contracts/agents-api/README.md index 4b5eccc27..05fe8d34f 100644 --- a/contracts/agents-api/README.md +++ b/contracts/agents-api/README.md @@ -100,7 +100,7 @@ paths start at `/vaults`, not `/agents/vaults`. | environments.files | create, list | [Bounded live listing and inline/source-file creation](environment-files.md) on qualified Docker workspaces; [user-managed enrollment](user-managed-runtime-v1.md) reuses the local implementation with separate real public acceptance. Full listing, overwrite and error semantics remain partial | | environments.templates | create, retrieve, update, list, delete | [Reusable network, files, env/setup/packages, inline/referenced Skills and Session snapshots](environment-templates.md); other initialization and full semantics remain gaps | | vaults | create, retrieve, list, delete | Create/retrieve/list/delete with independent tenant persistence, stored status filtering, atomic Credential cascade and frozen Session attachments; archive semantics and full hosted lifecycle parity remain missing | -| vaults.credentials | create, retrieve, update, list, delete | Static-bearer create/retrieve/list/token replacement/deletion with scoped encrypted storage; Session attachment and exact-URL HTTPS MCP binding; OAuth, archive semantics and full hosted lifecycle parity remain missing | +| vaults.credentials | create, retrieve, update, list, delete | Static-bearer and OAuth create/retrieve/list/replacement/deletion with scoped encrypted storage and dispatch-time refresh; Session attachment and exact-URL HTTPS MCP binding; archive semantics and full hosted lifecycle parity remain missing | For each resource, verify the referenced request/response unions and observable behavior, not just the route. Non-text initial input, configuration @@ -170,7 +170,7 @@ user-managed enrollment remain outside this qualification. | Environment Templates | Unsupported restricted hostname forms, unqualified installation overrides/null network and exact hosted errors remain gaps. CRUD/list, files, env/setup/system/npm/Python, inline/referenced Skills, Plugins, workspace capability directories and Session references have recorded coverage. Environment Plugin MCP transport and placement limits are [listed separately](environment-templates.md#environment-origin-mcp-plugins) | | Input and configuration | Non-text initial input, broader content/configuration unions and reasoning/verbosity combinations; [structured output](structured-output.md) has qualified Claude function profiles on none and Core-managed Docker openai_hosted, with other combinations remaining gaps | | Tools and interactions | [Deferred discovery qualification](tool-search.md), other tool types, effective tool-set enforcement and result/cancel publication ordering; MiniMax public functions and service-origin MCP remain unsupported | -| Vault and Credentials | OAuth/refresh, archive semantics, revocation/concurrent mutation and exact hosted selection/error behavior; static bearer CRUD/token replacement is already present | +| Vault and Credentials | Archive semantics, in-flight token withdrawal and exact hosted selection/error behavior; static/OAuth CRUD, replacement and scoped dispatch-time refresh are implemented (see credential guide for qualification) | | Existing resources | Full Item/SSE/Usage variants, omitted/null/default/error semantics, pagination and overlapping lifecycle behavior beyond recorded cases | An implementation gap and an unknown upstream behavior require different follow-up @@ -181,8 +181,8 @@ including further deployment qualification; this inventory describes merged beha ## Public semantics - Credentials use `POST /vaults/{vault_id}/credentials` and - `GET /vaults/{vault_id}/credentials/{credential_id}`. The initial profile accepts - only `static_bearer` with required string token and HTTPS destination, plus a + `GET /vaults/{vault_id}/credentials/{credential_id}`. The static profile accepts + `static_bearer` with required string token and HTTPS destination, plus a required name trimmed to 1–256 UTF-8 bytes. Tokens remain opaque, including empty strings; exact hosted token validation is unverified. The local URL profile excludes userinfo/fragments and preserves queries without normalization or network @@ -190,7 +190,9 @@ including further deployment qualification; this inventory describes merged beha auth type/destination; it never returns tokens or ciphertext and can be read without the encryption key. Missing encryption configuration locally rejects creation/replacement with 503. Attached Sessions can use static credentials for - exact-URL HTTPS MCP; OAuth, storage-key rotation, archive behavior and key scopes remain gaps. See the [credential storage guide](../../services/agents-api/credentials.md) + exact-URL HTTPS MCP. OAuth grants use the same binding plus + [scoped refresh and replacement](../../services/agents-api/oauth-credentials.md); + storage-key rotation, archive behavior and key scopes remain gaps. See the [credential storage guide](../../services/agents-api/credentials.md) for encryption and operational limits; this does not establish complete Credential or hosted error/retry compatibility. - `GET /vaults/{vault_id}/credentials` lists only safe metadata, with parent and @@ -209,8 +211,9 @@ including further deployment qualification; this inventory describes merged beha Ciphertext/update time change atomically within the same tenant/Vault/ID/type/URL; name, destination, identity, creation time and Session bindings are unchanged. No old-token decryption or MCP call occurs. Subsequent dispatch reads use the committed - replacement; already-resolved requests may retain the old token. OAuth replacement, - storage-key rotation, hot reload/revocation and exact hosted concurrent-update, + replacement; already-resolved requests may retain the old token. OAuth partial + replacement follows its pinned union and authenticates the stored grant. + Storage-key rotation, hot reload/revocation and exact hosted concurrent-update, timestamp and retry semantics remain gaps. - `DELETE /vaults/{vault_id}/credentials/{credential_id}` returns only `id`, `deleted: true` and `object: vault.credential.deleted`. It removes one owned row @@ -317,7 +320,7 @@ including further deployment qualification; this inventory describes merged beha headers. Omitted/null `allowed_tools` is unrestricted; `[]` denies all tools. Session `vault_ids` attaches tenant-owned Vaults. Explicit `credential_id` must belong to an attached Vault and match the exact HTTPS URL; omission/null selects - one matching static credential, zero stays anonymous and ambiguity fails. Private + one matching static or OAuth credential, zero stays anonymous and ambiguity fails. Private immutable selections do not populate the public credential field. Scope is rechecked before dispatch-only decryption; authenticated execution requires the separate bearer capability and never downgrades on failure. Exact URL/selection diff --git a/contracts/agents-api/openapi.yaml b/contracts/agents-api/openapi.yaml index 83fe97568..c3a8fcfef 100644 --- a/contracts/agents-api/openapi.yaml +++ b/contracts/agents-api/openapi.yaml @@ -148,7 +148,7 @@ definitions: v1.CreateCredentialRequest: properties: auth: - $ref: '#/definitions/v1.StaticBearerCredentialInput' + $ref: '#/definitions/v1.CredentialAuthInput' name: type: string required: @@ -207,7 +207,7 @@ definitions: v1.Credential: properties: auth: - $ref: '#/definitions/v1.StaticBearerCredentialAuth' + $ref: '#/definitions/v1.CredentialAuth' created_at: type: integer id: @@ -231,6 +231,72 @@ definitions: - updated_at - vault_id type: object + v1.CredentialAuth: + properties: + expires_at: + type: string + x-nullable: true + mcp_server_url: + type: string + refresh: + allOf: + - $ref: '#/definitions/v1.OAuthCredentialRefresh' + x-nullable: true + type: + enum: + - static_bearer + - mcp_oauth + type: string + required: + - mcp_server_url + - type + type: object + v1.CredentialAuthInput: + properties: + access_token: + type: string + expires_at: + type: string + x-nullable: true + mcp_server_url: + type: string + refresh: + allOf: + - $ref: '#/definitions/v1.OAuthCredentialRefreshInput' + x-nullable: true + token: + type: string + type: + enum: + - static_bearer + - mcp_oauth + type: string + required: + - mcp_server_url + - type + type: object + v1.CredentialAuthReplacement: + properties: + access_token: + type: string + x-nullable: true + expires_at: + type: string + x-nullable: true + refresh: + allOf: + - $ref: '#/definitions/v1.OAuthCredentialRefreshReplacement' + x-nullable: true + token: + type: string + type: + enum: + - static_bearer + - mcp_oauth + type: string + required: + - type + type: object v1.CredentialDeleted: properties: deleted: @@ -835,6 +901,97 @@ definitions: required: - enabled type: object + v1.OAuthCredentialRefresh: + properties: + client_id: + type: string + resource: + type: string + x-nullable: true + scope: + type: string + x-nullable: true + token_endpoint: + type: string + token_endpoint_auth: + $ref: '#/definitions/v1.OAuthEndpointAuth' + required: + - client_id + - token_endpoint + - token_endpoint_auth + type: object + v1.OAuthCredentialRefreshInput: + properties: + client_id: + type: string + refresh_token: + type: string + resource: + type: string + x-nullable: true + scope: + type: string + x-nullable: true + token_endpoint: + type: string + token_endpoint_auth: + $ref: '#/definitions/v1.OAuthEndpointAuthInput' + required: + - client_id + - refresh_token + - token_endpoint + - token_endpoint_auth + type: object + v1.OAuthCredentialRefreshReplacement: + properties: + refresh_token: + type: string + x-nullable: true + scope: + type: string + x-nullable: true + token_endpoint_auth: + allOf: + - $ref: '#/definitions/v1.OAuthEndpointAuthReplacement' + x-nullable: true + type: object + v1.OAuthEndpointAuth: + properties: + type: + enum: + - none + - client_secret_basic + - client_secret_post + type: string + required: + - type + type: object + v1.OAuthEndpointAuthInput: + properties: + client_secret: + type: string + type: + enum: + - none + - client_secret_basic + - client_secret_post + type: string + required: + - type + type: object + v1.OAuthEndpointAuthReplacement: + properties: + client_secret: + type: string + x-nullable: true + type: + enum: + - client_secret_basic + - client_secret_post + type: string + required: + - type + type: object v1.OutputTokenDetails: properties: reasoning_tokens: @@ -1474,45 +1631,6 @@ definitions: - has_more - object type: object - v1.StaticBearerCredentialAuth: - properties: - mcp_server_url: - type: string - type: - enum: - - static_bearer - type: string - required: - - mcp_server_url - - type - type: object - v1.StaticBearerCredentialInput: - properties: - mcp_server_url: - type: string - token: - type: string - type: - enum: - - static_bearer - type: string - required: - - mcp_server_url - - token - - type - type: object - v1.StaticBearerCredentialReplacement: - properties: - token: - type: string - type: - enum: - - static_bearer - type: string - required: - - token - - type - type: object v1.StreamError: properties: code: @@ -1765,7 +1883,7 @@ definitions: v1.UpdateCredentialRequest: properties: auth: - $ref: '#/definitions/v1.StaticBearerCredentialReplacement' + $ref: '#/definitions/v1.CredentialAuthReplacement' required: - auth type: object @@ -4721,16 +4839,16 @@ paths: post: consumes: - application/json - description: Stores the write-only token as execution-owned authenticated ciphertext. - Required name is trimmed to 1–256 UTF-8 bytes; auth requires static_bearer, - an HTTPS mcp_server_url and a string token. Token bytes are preserved, including - empty strings; hosted token edge validation is unverified. The initial URL - profile excludes userinfo and fragments, preserves queries and makes no network - request. Public responses contain only safe metadata. Missing encryption configuration - returns local 503. Session admission can bind static credentials from attached - Vaults to exact HTTPS MCP destinations; secret decryption occurs only at dispatch. - OAuth, storage-key rotation and exact hosted error/retry semantics remain - gaps. + description: Stores static_bearer or mcp_oauth secrets as execution-owned authenticated + ciphertext without contacting any endpoint. OAuth accepts a required access + token, nullable RFC3339 expiry and optional refresh configuration with none, + client_secret_basic or client_secret_post authentication. Required name is + trimmed to 1–256 UTF-8 bytes. Credential and token endpoints require HTTPS + without userinfo or fragments. Responses contain safe metadata only, including + explicit nullable OAuth expiry, refresh, resource and scope. Missing encryption + configuration returns local 503. External authorization and provider revocation + remain caller responsibilities; exact hosted error/default semantics remain + unverified. parameters: - description: agents=v1 in: header @@ -4742,7 +4860,7 @@ paths: name: vault_id required: true type: string - - description: Write-only static bearer credential + - description: Write-only credential authentication union in: body name: body required: true @@ -4781,7 +4899,7 @@ paths: $ref: '#/definitions/v1.ErrorResponse' security: - BearerAuth: [] - summary: Create a static-bearer Vault Credential + summary: Create a Vault Credential tags: - Credentials /vaults/{vault_id}/credentials/{credential_id}: @@ -4895,16 +5013,17 @@ paths: post: consumes: - application/json - description: Requires auth with type=static_bearer and a string token; empty - and opaque token bytes are preserved. Only the write-only secret and updated_at - change, atomically within the authenticated project and owning Vault. Identity, - name, auth type, exact destination, created_at and Session bindings remain - unchanged. Responses contain only safe metadata; no old-token decryption or - network call occurs. Missing encryption configuration returns local 503 without - modifying the credential. Subsequent dispatch reads use the committed replacement; - already-resolved requests may retain the old token. OAuth, storage-key rotation, - hot reload/revocation and exact hosted concurrent-update/retry/timestamp semantics - remain gaps. + description: Updates the existing static_bearer or mcp_oauth authentication + method without network requests. OAuth access_token omission/null retains + the token; a new token clears omitted expiry, explicit null clears expiry, + and other omitted fields remain unchanged. OAuth refresh patches cannot add + configuration or change client, endpoint, resource or authentication method; + nullable token/client-secret values retain stored secrets while explicit null + scope clears scope. Whole-null refresh and token_endpoint_auth retain existing + configuration under local policy. Identity, destination, creation time and + Session bindings remain unchanged. Responses expose safe metadata only. Already-dispatched + work is not revoked; provider revocation, storage-key rotation and exact hosted + concurrent-update/error semantics remain separate. parameters: - description: agents=v1 in: header @@ -4921,7 +5040,7 @@ paths: name: credential_id required: true type: string - - description: Write-only static bearer replacement + - description: Write-only credential authentication replacement union in: body name: body required: true @@ -4960,7 +5079,7 @@ paths: $ref: '#/definitions/v1.ErrorResponse' security: - BearerAuth: [] - summary: Replace a static-bearer Vault Credential token + summary: Replace Vault Credential authentication secrets tags: - Credentials schemes: diff --git a/contracts/agents-api/v1/credentials.go b/contracts/agents-api/v1/credentials.go index b8050f78d..4ef51b135 100644 --- a/contracts/agents-api/v1/credentials.go +++ b/contracts/agents-api/v1/credentials.go @@ -1,42 +1,101 @@ package v1 -// CreateCredentialRequest is the initial static-bearer resource profile. OAuth -// remains a separate missing union member, not a change to the upstream target. +import "encoding/json" + +// CredentialAuthInput represents the pinned create union. The API validates each +// discriminator separately so fields cannot cross authentication variants. +type CredentialAuthInput struct { + Type string `json:"type" binding:"required" enums:"static_bearer,mcp_oauth"` + MCPServerURL *string `json:"mcp_server_url" binding:"required"` + Token *string `json:"token,omitempty"` + AccessToken *string `json:"access_token,omitempty"` + ExpiresAt *string `json:"expires_at,omitempty" extensions:"x-nullable"` + Refresh *OAuthCredentialRefreshInput `json:"refresh,omitempty" extensions:"x-nullable"` +} + type CreateCredentialRequest struct { - Name *string `json:"name" binding:"required"` - Auth *StaticBearerCredentialInput `json:"auth" binding:"required"` + Name *string `json:"name" binding:"required"` + Auth *CredentialAuthInput `json:"auth" binding:"required"` +} + +type OAuthCredentialRefreshInput struct { + ClientID *string `json:"client_id" binding:"required"` + RefreshToken *string `json:"refresh_token" binding:"required"` + TokenEndpoint *string `json:"token_endpoint" binding:"required"` + TokenEndpointAuth *OAuthEndpointAuthInput `json:"token_endpoint_auth" binding:"required"` + Resource *string `json:"resource,omitempty" extensions:"x-nullable"` + Scope *string `json:"scope,omitempty" extensions:"x-nullable"` } -type StaticBearerCredentialInput struct { - Type string `json:"type" binding:"required" enums:"static_bearer"` - MCPServerURL *string `json:"mcp_server_url" binding:"required"` - Token *string `json:"token" binding:"required"` +type OAuthEndpointAuthInput struct { + Type string `json:"type" binding:"required" enums:"none,client_secret_basic,client_secret_post"` + ClientSecret *string `json:"client_secret,omitempty"` } -// UpdateCredentialRequest implements static token replacement. The pinned OAuth -// replacement variant remains a separate missing union member. type UpdateCredentialRequest struct { - Auth *StaticBearerCredentialReplacement `json:"auth" binding:"required"` + Auth *CredentialAuthReplacement `json:"auth" binding:"required"` +} + +// Raw nullable fields retain omission separately from explicit null. +type CredentialAuthReplacement struct { + Type string `json:"type" binding:"required" enums:"static_bearer,mcp_oauth"` + Token *string `json:"token,omitempty"` + AccessToken *string `json:"access_token,omitempty" extensions:"x-nullable"` + ExpiresAt json.RawMessage `json:"expires_at,omitempty" swaggertype:"string" extensions:"x-nullable"` + Refresh *OAuthCredentialRefreshReplacement `json:"refresh,omitempty" extensions:"x-nullable"` +} + +type OAuthCredentialRefreshReplacement struct { + RefreshToken *string `json:"refresh_token,omitempty" extensions:"x-nullable"` + Scope json.RawMessage `json:"scope,omitempty" swaggertype:"string" extensions:"x-nullable"` + TokenEndpointAuth *OAuthEndpointAuthReplacement `json:"token_endpoint_auth,omitempty" extensions:"x-nullable"` +} + +type OAuthEndpointAuthReplacement struct { + Type string `json:"type" binding:"required" enums:"client_secret_basic,client_secret_post"` + ClientSecret *string `json:"client_secret,omitempty" extensions:"x-nullable"` +} + +// CredentialAuth contains safe metadata only. OAuth nullable fields are emitted +// for OAuth credentials and excluded entirely from static bearer resources. +type CredentialAuth struct { + Type string `json:"type" binding:"required" enums:"static_bearer,mcp_oauth"` + MCPServerURL string `json:"mcp_server_url" binding:"required"` + ExpiresAt *string `json:"expires_at" extensions:"x-nullable"` + Refresh *OAuthCredentialRefresh `json:"refresh" extensions:"x-nullable"` +} + +type OAuthCredentialRefresh struct { + ClientID string `json:"client_id" binding:"required"` + TokenEndpoint string `json:"token_endpoint" binding:"required"` + TokenEndpointAuth OAuthEndpointAuth `json:"token_endpoint_auth" binding:"required"` + Resource *string `json:"resource" extensions:"x-nullable"` + Scope *string `json:"scope" extensions:"x-nullable"` } -type StaticBearerCredentialReplacement struct { - Type string `json:"type" binding:"required" enums:"static_bearer"` - Token *string `json:"token" binding:"required"` +type OAuthEndpointAuth struct { + Type string `json:"type" binding:"required" enums:"none,client_secret_basic,client_secret_post"` } -type StaticBearerCredentialAuth struct { - Type string `json:"type" binding:"required" enums:"static_bearer"` - MCPServerURL string `json:"mcp_server_url" binding:"required"` +func (a CredentialAuth) MarshalJSON() ([]byte, error) { + if a.Type == "static_bearer" { + return json.Marshal(struct { + Type string `json:"type"` + MCPServerURL string `json:"mcp_server_url"` + }{a.Type, a.MCPServerURL}) + } + type resource CredentialAuth + return json.Marshal(resource(a)) } type Credential struct { - ID string `json:"id" binding:"required"` - VaultID string `json:"vault_id" binding:"required"` - Name string `json:"name" binding:"required"` - Object string `json:"object" binding:"required" enums:"vault.credential"` - Auth StaticBearerCredentialAuth `json:"auth" binding:"required"` - CreatedAt int64 `json:"created_at" binding:"required"` - UpdatedAt int64 `json:"updated_at" binding:"required"` + ID string `json:"id" binding:"required"` + VaultID string `json:"vault_id" binding:"required"` + Name string `json:"name" binding:"required"` + Object string `json:"object" binding:"required" enums:"vault.credential"` + Auth CredentialAuth `json:"auth" binding:"required"` + CreatedAt int64 `json:"created_at" binding:"required"` + UpdatedAt int64 `json:"updated_at" binding:"required"` } type CredentialList struct { diff --git a/contracts/agents-api/v1/credentials_test.go b/contracts/agents-api/v1/credentials_test.go new file mode 100644 index 000000000..747ee5339 --- /dev/null +++ b/contracts/agents-api/v1/credentials_test.go @@ -0,0 +1,27 @@ +package v1 + +import ( + "encoding/json" + "reflect" + "testing" +) + +func TestCredentialAuthUnionNullableProjection(t *testing.T) { + for _, tc := range []struct { + auth CredentialAuth + want string + }{ + {CredentialAuth{Type: "static_bearer", MCPServerURL: "https://mcp.example", Refresh: &OAuthCredentialRefresh{ClientID: "must-not-cross-variants"}}, `{"type":"static_bearer","mcp_server_url":"https://mcp.example"}`}, + {CredentialAuth{Type: "mcp_oauth", MCPServerURL: "https://mcp.example"}, `{"type":"mcp_oauth","mcp_server_url":"https://mcp.example","expires_at":null,"refresh":null}`}, + {CredentialAuth{Type: "mcp_oauth", MCPServerURL: "https://mcp.example", Refresh: &OAuthCredentialRefresh{ClientID: "client", TokenEndpoint: "https://issuer.example/token", TokenEndpointAuth: OAuthEndpointAuth{Type: "none"}}}, `{"type":"mcp_oauth","mcp_server_url":"https://mcp.example","expires_at":null,"refresh":{"client_id":"client","token_endpoint":"https://issuer.example/token","token_endpoint_auth":{"type":"none"},"resource":null,"scope":null}}`}, + } { + raw, err := json.Marshal(tc.auth) + if err != nil { + t.Fatal(err) + } + var got, want map[string]any + if json.Unmarshal(raw, &got) != nil || json.Unmarshal([]byte(tc.want), &want) != nil || !reflect.DeepEqual(got, want) { + t.Fatal("credential discriminator or nullable metadata changed", string(raw)) + } + } +} diff --git a/go.mod b/go.mod index 2e4a5dfb4..68c42d207 100644 --- a/go.mod +++ b/go.mod @@ -49,7 +49,7 @@ require ( github.com/segmentio/asm v1.2.1 // indirect github.com/segmentio/encoding v0.5.4 // indirect github.com/yosida95/uritemplate/v3 v3.0.2 // indirect - golang.org/x/oauth2 v0.36.0 // indirect + golang.org/x/oauth2 v0.36.0 ) require ( diff --git a/packages/agents-client/src/client.ts b/packages/agents-client/src/client.ts index c2f32630b..d82108782 100644 --- a/packages/agents-client/src/client.ts +++ b/packages/agents-client/src/client.ts @@ -1,4 +1,5 @@ import { createSSEDecoder } from "./sse"; +import { projectVaultCredentialAuth, validCredentialURL } from "./vault-credential-auth"; import type { AgentCore, AgentDeleted, @@ -164,7 +165,6 @@ const vaultFields = new Set(["id", "object", "created_at", "name", "metadata"]); const vaultListFields = new Set(["object", "data", "has_more", "first_id", "last_id"]); const vaultDeletedFields = new Set(["id", "object", "deleted"]); const vaultCredentialFields = new Set(["id", "vault_id", "name", "object", "auth", "created_at", "updated_at"]); -const vaultCredentialAuthFields = new Set(["type", "mcp_server_url"]); const vaultCredentialDeletedFields = new Set(["id", "object", "deleted"]); const sessionFields = new Set([ "id", "object", "agent", "environment", "status", "error", "metadata", @@ -417,25 +417,12 @@ function projectVault(value: unknown, expectedId?: string): Vault { }; } -function validCredentialURL(value: unknown): value is string { - if ( - typeof value !== "string" || value !== value.trim() || - /[\u0000-\u0020\u007f\\]/u.test(value) - ) return false; - try { - const url = new URL(value); - return url.protocol === "https:" && Boolean(url.hostname) && !url.username && !url.password && !url.hash; - } catch { - return false; - } -} - function projectVaultCredential( value: unknown, expectedVaultId: string, expectedCredentialId?: string, ): VaultCredential { - if (!isRecord(value) || !exactFields(value, vaultCredentialFields) || !isRecord(value.auth) || !exactFields(value.auth, vaultCredentialAuthFields)) { + if (!isRecord(value) || !exactFields(value, vaultCredentialFields) || !isRecord(value.auth)) { return invalidVaultResponse("invalid_vault_credential", "Agent Core returned invalid Credential metadata."); } if ( @@ -443,18 +430,19 @@ function projectVaultCredential( (expectedCredentialId !== undefined && !sameUuid(value.id, expectedCredentialId)) || !sameUuid(value.vault_id, expectedVaultId) || value.object !== "vault.credential" || !isTrimmedName(value.name) || - value.auth.type !== "static_bearer" || !validCredentialURL(value.auth.mcp_server_url) || !Number.isSafeInteger(value.created_at) || Number(value.created_at) < 0 || !Number.isSafeInteger(value.updated_at) || Number(value.updated_at) < Number(value.created_at) ) { return invalidVaultResponse("invalid_vault_credential", "Agent Core returned invalid Credential metadata."); } + const auth = projectVaultCredentialAuth(value.auth); + if (!auth) return invalidVaultResponse("invalid_vault_credential", "Agent Core returned invalid Credential metadata."); return { id: value.id, vault_id: value.vault_id as string, name: value.name, object: "vault.credential", - auth: { type: "static_bearer", mcp_server_url: value.auth.mcp_server_url }, + auth, created_at: Number(value.created_at), updated_at: Number(value.updated_at), }; @@ -1939,7 +1927,7 @@ export class OpenAIAgentsClient implements AgentCore { "Agent Core Credential creation failed.", ); const credential = projectVaultCredential(value, vaultId); - if (credential.name !== input.name || credential.auth.mcp_server_url !== input.auth.mcp_server_url) { + if (credential.auth.type !== "static_bearer" || credential.name !== input.name || credential.auth.mcp_server_url !== input.auth.mcp_server_url) { return invalidVaultResponse("invalid_vault_credential", "Agent Core returned mismatched Credential metadata."); } return credential; @@ -1973,6 +1961,9 @@ export class OpenAIAgentsClient implements AgentCore { throw new TypeError("Credential replacement accepts one write-only static bearer token."); } const baseline = await this.retrieveVaultCredential(vaultId, credentialId); + if (baseline.auth.type !== "static_bearer") { + throw new TypeError("Static bearer token replacement is unavailable for OAuth credentials."); + } const value = await this.requestCredentialWrite( `/vaults/${encodeURIComponent(vaultId)}/credentials/${encodeURIComponent(credentialId)}`, JSON.stringify(input), @@ -1980,7 +1971,7 @@ export class OpenAIAgentsClient implements AgentCore { ); const credential = projectVaultCredential(value, vaultId, credentialId); if ( - credential.name !== baseline.name || + credential.auth.type !== "static_bearer" || credential.name !== baseline.name || credential.auth.mcp_server_url !== baseline.auth.mcp_server_url || credential.created_at !== baseline.created_at || credential.updated_at < baseline.updated_at diff --git a/packages/agents-client/src/types.ts b/packages/agents-client/src/types.ts index 74a156353..6026b7e29 100644 --- a/packages/agents-client/src/types.ts +++ b/packages/agents-client/src/types.ts @@ -124,12 +124,27 @@ export interface StaticBearerCredentialAuth { mcp_server_url: string; } +export interface McpOAuthRefreshMetadata { + client_id: string; + token_endpoint: string; + token_endpoint_auth: { type: "none" | "client_secret_basic" | "client_secret_post" }; + resource: string | null; + scope: string | null; +} + +export interface McpOAuthCredentialAuth { + type: "mcp_oauth"; + mcp_server_url: string; + expires_at: string | null; + refresh: McpOAuthRefreshMetadata | null; +} + export interface VaultCredential { id: string; vault_id: string; name: string; object: "vault.credential"; - auth: StaticBearerCredentialAuth; + auth: StaticBearerCredentialAuth | McpOAuthCredentialAuth; created_at: number; updated_at: number; } diff --git a/packages/agents-client/src/vault-credential-auth.ts b/packages/agents-client/src/vault-credential-auth.ts new file mode 100644 index 000000000..62ec5da3d --- /dev/null +++ b/packages/agents-client/src/vault-credential-auth.ts @@ -0,0 +1,63 @@ +import type { McpOAuthRefreshMetadata, VaultCredential } from "./types"; + +const oauthFields = new Set(["type", "mcp_server_url", "expires_at", "refresh"]); +const refreshFields = new Set(["client_id", "token_endpoint", "token_endpoint_auth", "resource", "scope"]); + +export function validCredentialURL(value: unknown): value is string { + if ( + typeof value !== "string" || value !== value.trim() || + /[\u0000-\u0020\u007f\\]/u.test(value) + ) return false; + try { + const url = new URL(value); + return url.protocol === "https:" && Boolean(url.hostname) && !url.username && !url.password && !url.hash; + } catch { + return false; + } +} + +// Each level is projected explicitly; unexpected fields may contain secrets. +export function projectVaultCredentialAuth(value: Record): VaultCredential["auth"] | null { + if (!validCredentialURL(value.mcp_server_url)) return null; + const fields = Object.keys(value); + if (value.type === "static_bearer" && fields.length === 2) { + return { type: "static_bearer", mcp_server_url: value.mcp_server_url }; + } + if ( + value.type !== "mcp_oauth" || fields.length !== oauthFields.size || !fields.every((field) => oauthFields.has(field)) || + !(value.expires_at === null || typeof value.expires_at === "string") + ) return null; + const refresh = value.refresh === null ? null : projectOAuthRefresh(value.refresh); + if (value.refresh !== null && refresh === null) return null; + return { + type: "mcp_oauth", + mcp_server_url: value.mcp_server_url, + expires_at: value.expires_at, + refresh, + }; +} + +function projectOAuthRefresh(value: unknown): McpOAuthRefreshMetadata | null { + if (value === null || typeof value !== "object" || Array.isArray(value)) return null; + const refresh = value as Record; + const fields = Object.keys(refresh); + if ( + fields.length !== refreshFields.size || !fields.every((field) => refreshFields.has(field)) || + typeof refresh.client_id !== "string" || !validCredentialURL(refresh.token_endpoint) || + !(refresh.resource === null || typeof refresh.resource === "string") || + !(refresh.scope === null || typeof refresh.scope === "string") || + refresh.token_endpoint_auth === null || typeof refresh.token_endpoint_auth !== "object" || Array.isArray(refresh.token_endpoint_auth) + ) return null; + const auth = refresh.token_endpoint_auth as Record; + if ( + Object.keys(auth).length !== 1 || + (auth.type !== "none" && auth.type !== "client_secret_basic" && auth.type !== "client_secret_post") + ) return null; + return { + client_id: refresh.client_id, + token_endpoint: refresh.token_endpoint, + token_endpoint_auth: { type: auth.type }, + resource: refresh.resource, + scope: refresh.scope, + }; +} diff --git a/packages/agents-client/src/vault-credentials-oauth.test.ts b/packages/agents-client/src/vault-credentials-oauth.test.ts new file mode 100644 index 000000000..5a01cb8e0 --- /dev/null +++ b/packages/agents-client/src/vault-credentials-oauth.test.ts @@ -0,0 +1,110 @@ +import { describe, expect, it } from "vitest"; + +import { OpenAIAgentsClient } from "./client"; +import type { McpOAuthCredentialAuth, VaultCredential } from "./types"; + +const vaultId = "11111111-1111-4111-8111-111111111111"; +const credentialId = "22222222-2222-4222-8222-222222222222"; +const staticCredential: VaultCredential = { + id: credentialId, + vault_id: vaultId, + object: "vault.credential", + name: "Internal MCP", + auth: { type: "static_bearer", mcp_server_url: "https://mcp.example/tools" }, + created_at: 1, + updated_at: 2, +}; +const oauthAuth: McpOAuthCredentialAuth = { + type: "mcp_oauth", + mcp_server_url: "https://mcp.example/tools", + expires_at: "2030-01-01T00:00:00Z", + refresh: { + client_id: "public-client-id", + token_endpoint: "https://issuer.example/token", + token_endpoint_auth: { type: "none" }, + resource: "https://mcp.example/tools", + scope: "read", + }, +}; +const oauthCredential: VaultCredential = { ...staticCredential, auth: oauthAuth }; +const jsonResponse = (body: unknown) => new Response(JSON.stringify(body), { + status: 200, + headers: { "content-type": "application/json" }, +}); + +describe("OAuth Credential metadata", () => { + it.each(["none", "client_secret_basic", "client_secret_post"] as const)("reads safe %s metadata", async (type) => { + const credential = { ...oauthCredential, auth: { + ...oauthAuth, refresh: { ...oauthAuth.refresh!, token_endpoint_auth: { type } }, + } }; + const client = new OpenAIAgentsClient({ fetch: (async () => jsonResponse(credential)) as typeof fetch }); + await expect(client.retrieveVaultCredential(vaultId, credentialId)).resolves.toEqual(credential); + }); + + it("reads a mixed list with nullable OAuth fields", async () => { + const oauth = { ...oauthCredential, id: "33333333-3333-4333-8333-333333333333", auth: { + ...oauthAuth, expires_at: null, refresh: null, + } }; + const page = { object: "list", data: [oauth, staticCredential], has_more: false, first_id: oauth.id, last_id: staticCredential.id }; + const client = new OpenAIAgentsClient({ fetch: (async () => jsonResponse(page)) as typeof fetch }); + await expect(client.listVaultCredentials(vaultId)).resolves.toEqual(page); + }); + + it("preserves nullable scope and resource", async () => { + const credential = { ...oauthCredential, auth: { + ...oauthAuth, refresh: { ...oauthAuth.refresh!, resource: null, scope: null }, + } }; + const client = new OpenAIAgentsClient({ fetch: (async () => jsonResponse(credential)) as typeof fetch }); + await expect(client.retrieveVaultCredential(vaultId, credentialId)).resolves.toEqual(credential); + }); + + it.each([ + { ...oauthAuth, access_token: "unexpected-secret" }, + { ...oauthAuth, refresh: { ...oauthAuth.refresh, refresh_token: "unexpected-secret" } }, + { ...oauthAuth, refresh: { ...oauthAuth.refresh, token_endpoint_auth: { type: "client_secret_basic", client_secret: "unexpected-secret" } } }, + { ...oauthAuth, refresh: { ...oauthAuth.refresh, extra: "unexpected-secret" } }, + { ...oauthAuth, refresh: { ...oauthAuth.refresh, token_endpoint_auth: { type: "unsupported" } } }, + { ...oauthAuth, refresh: { ...oauthAuth.refresh, token_endpoint: "http://issuer.example/token" } }, + { ...oauthAuth, refresh: { ...oauthAuth.refresh, token_endpoint: "https://user:pass@issuer.example/token" } }, + { ...oauthAuth, refresh: { ...oauthAuth.refresh, scope: 1 } }, + { ...oauthAuth, expires_at: 1 }, + { type: "mcp_oauth", mcp_server_url: oauthAuth.mcp_server_url }, + ])("rejects secret-bearing or malformed OAuth metadata case %#", async (auth) => { + const client = new OpenAIAgentsClient({ fetch: (async () => jsonResponse({ ...oauthCredential, auth })) as typeof fetch }); + await expect(client.retrieveVaultCredential(vaultId, credentialId)).rejects.toMatchObject({ + status: 502, + code: "invalid_vault_credential", + message: "Agent Core returned invalid Credential metadata.", + }); + }); + + it("blocks static replacement of OAuth before sending a write", async () => { + const calls: Array = []; + const client = new OpenAIAgentsClient({ fetch: (async (_input, init) => { + calls.push(init); + return jsonResponse(oauthCredential); + }) as typeof fetch }); + await expect(client.replaceVaultCredentialToken(vaultId, credentialId, { + auth: { type: "static_bearer", token: "replacement" }, + })).rejects.toThrow("Static bearer token replacement is unavailable for OAuth credentials."); + expect(calls).toHaveLength(1); + expect(calls.some((call) => call?.method === "POST")).toBe(false); + }); + + it("rejects an OAuth response to static creation", async () => { + const client = new OpenAIAgentsClient({ fetch: (async () => jsonResponse(oauthCredential)) as typeof fetch }); + await expect(client.createVaultCredential(vaultId, { + name: staticCredential.name, + auth: { type: "static_bearer", mcp_server_url: staticCredential.auth.mcp_server_url, token: "creation" }, + })).rejects.toMatchObject({ status: 502, code: "invalid_vault_credential" }); + }); + + it("rejects an auth type change after static replacement", async () => { + const client = new OpenAIAgentsClient({ fetch: (async (_input, init) => jsonResponse( + init?.method === "POST" ? oauthCredential : staticCredential, + )) as typeof fetch }); + await expect(client.replaceVaultCredentialToken(vaultId, credentialId, { + auth: { type: "static_bearer", token: "replacement" }, + })).rejects.toMatchObject({ status: 502, code: "invalid_vault_credential" }); + }); +}); diff --git a/services/agents-api/README.md b/services/agents-api/README.md index 96da2c5cb..08842a859 100644 --- a/services/agents-api/README.md +++ b/services/agents-api/README.md @@ -19,11 +19,11 @@ Parsar product execution and its eventual public-client cutover are separate. ## Reusable Agents -Static-bearer Vault Credentials support creation, token replacement, deletion and -safe metadata retrieval/listing. Vault deletion atomically removes its Credentials. -Configure their independent encryption key and authenticated -Session use through the [credential guide](credentials.md). OAuth remains a -separate implementation gap. +Static-bearer and OAuth Vault Credentials support creation, replacement, deletion +and safe metadata retrieval/listing. Vault deletion atomically removes its +Credentials. Configure their independent encryption key and authenticated Session +use through the [credential guide](credentials.md); see [OAuth credentials](oauth-credentials.md) +for application authorization, dispatch-time refresh and revocation boundaries. The pinned Python client can save configuration independently of execution: @@ -214,8 +214,8 @@ resources); general Files routes do not. Supported operations include: - Project-owned `user_data` source file upload/list, metadata/content retrieval and deletion; see [source Files](../../contracts/agents-api/source-files.md). - Vault create/retrieve/list/delete, project-scoped pagination and stored status - filtering; static-bearer Credential create/retrieve/list/token replacement/delete. - Public archive semantics and OAuth remain gaps. Already-delivered credentials + filtering; static-bearer and OAuth Credential create/retrieve/list/replacement/delete, + plus [dispatch-time OAuth refresh](oauth-credentials.md). Public archive semantics remain gaps. Already-delivered credentials are not withdrawn by local deletion. Session attachments support [authenticated HTTPS MCP](credentials.md#use-a-credential-in-a-session). @@ -604,7 +604,7 @@ name collisions, changing inventories and original MCP metadata fidelity remain gaps. Items retain the observed native JSON, which may differ from the original MCP envelope. See the [Claude SDK profile](../../CONTRIBUTING.md#claude-sdk-adapter-foundation). -The current subset rejects OAuth, inline authorization, nonempty headers or +The current subset rejects native OAuth login, inline authorization, nonempty headers or request metadata, URL userinfo/query/fragment, implicit/other origins, stdio and engines other than Codex/Claude SDK. The Codex adapter also rejects reserved native labels and stored native MCP credentials. It verifies diff --git a/services/agents-api/cmd/server/main.go b/services/agents-api/cmd/server/main.go index 607b5c01f..e1fc46947 100644 --- a/services/agents-api/cmd/server/main.go +++ b/services/agents-api/cmd/server/main.go @@ -85,7 +85,11 @@ func run() error { if err != nil { return err } - executionStore := store.NewWithCredentialCipher(pool, credentialKey) + oauthClient, err := oauthRefreshClient() + if err != nil { + return err + } + executionStore := store.NewWithCredentialCipherAndOAuthRefresh(pool, credentialKey, oauthClient) if err := executionStore.EnsureProjectScopes(ready, auth.ProjectScopes()); err != nil { return err } diff --git a/services/agents-api/cmd/server/oauth_refresh.go b/services/agents-api/cmd/server/oauth_refresh.go new file mode 100644 index 000000000..30698760a --- /dev/null +++ b/services/agents-api/cmd/server/oauth_refresh.go @@ -0,0 +1,18 @@ +package main + +import ( + "os" + "strings" + + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/oauthrefresh" +) + +func oauthRefreshClient() (*oauthrefresh.Client, error) { + var origins []string + if raw := os.Getenv("AGENTS_API_OAUTH_TRUSTED_ORIGINS"); raw != "" { + for _, origin := range strings.Split(raw, ",") { + origins = append(origins, strings.TrimSpace(origin)) + } + } + return oauthrefresh.NewClient(origins) +} diff --git a/services/agents-api/cmd/server/oauth_refresh_test.go b/services/agents-api/cmd/server/oauth_refresh_test.go new file mode 100644 index 000000000..93d4e39cb --- /dev/null +++ b/services/agents-api/cmd/server/oauth_refresh_test.go @@ -0,0 +1,18 @@ +package main + +import "testing" + +func TestOAuthRefreshOperatorPolicy(t *testing.T) { + for _, raw := range []string{"", "https://issuer.example", "https://issuer.example, https://10.0.0.1:9443"} { + t.Setenv("AGENTS_API_OAUTH_TRUSTED_ORIGINS", raw) + if _, err := oauthRefreshClient(); err != nil { + t.Fatal(err) + } + } + for _, raw := range []string{"http://issuer.example", "https://issuer.example/token", "https://issuer.example,", "https://user:secret@issuer.example"} { + t.Setenv("AGENTS_API_OAUTH_TRUSTED_ORIGINS", raw) + if _, err := oauthRefreshClient(); err == nil { + t.Fatal("invalid issuer policy accepted") + } + } +} diff --git a/services/agents-api/credentials.md b/services/agents-api/credentials.md index 58d5d4cfb..8a2292389 100644 --- a/services/agents-api/credentials.md +++ b/services/agents-api/credentials.md @@ -1,6 +1,6 @@ # Vault credential storage -The standalone service supports static-bearer Credential creation, token replacement, +The standalone service supports static-bearer and OAuth Credential creation, token replacement, deletion and safe metadata retrieval through the pinned official SDK. It stores tokens as authenticated ciphertext in its own PostgreSQL database. There is no product-service dependency, public secret-read endpoint. Resource creation, replacement and retrieval do not contact the @@ -72,7 +72,7 @@ archived fixtures verify filtering, not a public archive operation. Archive beha and exact hosted query/concurrent-page semantics remain separate gaps. Required name is trimmed to 1–256 UTF-8 bytes. Required `auth` accepts -`static_bearer`, an HTTPS `mcp_server_url` and a string `token`. The token is +`static_bearer` or the [OAuth variant](oauth-credentials.md). Static auth requires an HTTPS `mcp_server_url` and a string `token`. The token is preserved as opaque data, including whitespace or an empty string. This does not verify that it will authenticate to a destination. The local URL profile excludes userinfo and fragments, preserves queries and performs no DNS or HTTP request. @@ -95,8 +95,9 @@ authentication. Names are public mutable metadata and are not part of this bindi Resource SQL reads select no secret ciphertext. The key and request token exist in trusted service memory; this protects stored secrets, not a compromised service host. -Credential archive behavior, OAuth refresh, restricted-key scopes and revocation -semantics remain separate gaps. The foreign key preserves Vault ownership and +See [OAuth credentials](oauth-credentials.md) for grant storage, dispatch-time refresh, +replacement and provider-revocation boundaries. Credential archive behavior, +restricted-key scopes and storage-key rotation remain separate gaps. The foreign key preserves Vault ownership and atomically removes all dependent Credentials when their Vault is deleted. The full protocol target is unchanged. @@ -125,7 +126,7 @@ Trusted service-side Codex and Claude SDK support `environment:none`; Codex also supports the documented `self_hosted` combination. The daemon must advertise both `mcp_http_tools` and `mcp_http_bearer_auth`. The usual [MCP profile limits](README.md#http-mcp-execution) still apply. Without an explicit -`credential_id`, one exact-URL static credential among attached Vaults is selected; +`credential_id`, one exact-URL static or OAuth credential among attached Vaults is selected; zero matches remains anonymous and multiple matches fail. A foreign, missing, unattached or wrong-destination reference returns the same local 404 before Session creation. Saving a reference on an Agent does not authorize it for a Session. @@ -168,8 +169,9 @@ an already-resolved or running request may still hold the previous token. Updati this resource performs no MCP call, changes no server-side token independently, and provides no in-flight revocation, hot reload or cancellation. Coordinate the destination's token change operationally. Replacing this token does not rotate the -storage encryption key or reset its encryption budget. OAuth replacement and exact -hosted overlapping-update, replay and timestamp semantics remain unverified. +storage encryption key or reset its encryption budget. OAuth partial replacement +is described in [OAuth credentials](oauth-credentials.md). Exact hosted +overlapping-update, replay and timestamp semantics remain unverified. ## Delete a stored credential diff --git a/services/agents-api/internal/api/credentials.go b/services/agents-api/internal/api/credentials.go index 0ed264ba3..7fca2843c 100644 --- a/services/agents-api/internal/api/credentials.go +++ b/services/agents-api/internal/api/credentials.go @@ -2,8 +2,8 @@ package api import ( "context" + "encoding/json" "net/http" - "net/url" v1 "github.com/MiniMax-AI-Dev/parsar/contracts/agents-api/v1" "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/store" @@ -12,6 +12,8 @@ import ( ) type CredentialStore interface { + CreateOAuthCredential(context.Context, string, string, store.CreateOAuthCredentialInput) (store.Credential, error) + UpdateOAuthCredential(context.Context, string, string, string, store.UpdateOAuthCredentialInput) (store.Credential, error) CreateStaticCredential(context.Context, string, string, store.CreateStaticCredentialInput) (store.Credential, error) UpdateStaticCredential(context.Context, string, string, string, store.UpdateStaticCredentialInput) (store.Credential, error) GetCredential(context.Context, string, string, string) (store.Credential, error) @@ -19,15 +21,15 @@ type CredentialStore interface { ListCredentials(context.Context, string, string, string, int, bool, []string) (store.CredentialPage, error) } -// @Summary Create a static-bearer Vault Credential -// @Description Stores the write-only token as execution-owned authenticated ciphertext. Required name is trimmed to 1–256 UTF-8 bytes; auth requires static_bearer, an HTTPS mcp_server_url and a string token. Token bytes are preserved, including empty strings; hosted token edge validation is unverified. The initial URL profile excludes userinfo and fragments, preserves queries and makes no network request. Public responses contain only safe metadata. Missing encryption configuration returns local 503. Session admission can bind static credentials from attached Vaults to exact HTTPS MCP destinations; secret decryption occurs only at dispatch. OAuth, storage-key rotation and exact hosted error/retry semantics remain gaps. +// @Summary Create a Vault Credential +// @Description Stores static_bearer or mcp_oauth secrets as execution-owned authenticated ciphertext without contacting any endpoint. OAuth accepts a required access token, nullable RFC3339 expiry and optional refresh configuration with none, client_secret_basic or client_secret_post authentication. Required name is trimmed to 1–256 UTF-8 bytes. Credential and token endpoints require HTTPS without userinfo or fragments. Responses contain safe metadata only, including explicit nullable OAuth expiry, refresh, resource and scope. Missing encryption configuration returns local 503. External authorization and provider revocation remain caller responsibilities; exact hosted error/default semantics remain unverified. // @Tags Credentials // @Accept json // @Produce json // @Security BearerAuth // @Param OpenAI-Beta header string true "agents=v1" // @Param vault_id path string true "Vault ID" -// @Param body body v1.CreateCredentialRequest true "Write-only static bearer credential" +// @Param body body v1.CreateCredentialRequest true "Write-only credential authentication union" // @Success 200 {object} v1.Credential // @Failure 400,401,404,413,500,503 {object} v1.ErrorResponse // @Router /vaults/{vault_id}/credentials [post] @@ -44,9 +46,12 @@ func (h *Handler) createCredential(w http.ResponseWriter, r *http.Request) { if !ok { return } - var request v1.CreateCredentialRequest - if decodeInputObject(raw, &request, "name", "auth") != nil || request.Name == nil || request.Auth == nil || request.Auth.Type != "static_bearer" || request.Auth.Token == nil || request.Auth.MCPServerURL == nil { - writeError(w, http.StatusBadRequest, "invalid_request", "name and static_bearer auth with string mcp_server_url and token are required.") + var request struct { + Name *string `json:"name"` + Auth json.RawMessage `json:"auth"` + } + if decodeInputObject(raw, &request, "name", "auth") != nil || request.Name == nil { + writeError(w, http.StatusBadRequest, "invalid_request", "name and supported auth are required.") return } name, err := normalizedVaultName(*request.Name) @@ -54,12 +59,26 @@ func (h *Handler) createCredential(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "invalid_request", err.Error()) return } - u, err := url.Parse(*request.Auth.MCPServerURL) - if err != nil || u.Scheme != "https" || u.Hostname() == "" || u.User != nil || u.Fragment != "" { - writeError(w, http.StatusBadRequest, "invalid_request", "mcp_server_url must be an absolute HTTPS URL without userinfo or a fragment.") + var credential store.Credential + switch credentialAuthType(request.Auth) { + case "static_bearer": + var auth v1.CredentialAuthInput + if decodeInputObject(request.Auth, &auth, "type", "mcp_server_url", "token") != nil || auth.Token == nil || !credentialHTTPSURL(auth.MCPServerURL) { + writeError(w, http.StatusBadRequest, "invalid_request", "static_bearer requires string token and an absolute HTTPS mcp_server_url without userinfo or a fragment.") + return + } + credential, err = h.store.CreateStaticCredential(r.Context(), tenantID(r), vaultID, store.CreateStaticCredentialInput{Name: name, MCPServerURL: *auth.MCPServerURL, Token: *auth.Token}) + case "mcp_oauth": + input, parseErr := oauthCredentialCreate(request.Auth, name) + if parseErr != nil { + writeStoreError(w, r, parseErr) + return + } + credential, err = h.store.CreateOAuthCredential(r.Context(), tenantID(r), vaultID, input) + default: + writeError(w, http.StatusBadRequest, "invalid_request", "auth requires type static_bearer or mcp_oauth.") return } - credential, err := h.store.CreateStaticCredential(r.Context(), tenantID(r), vaultID, store.CreateStaticCredentialInput{Name: name, MCPServerURL: *request.Auth.MCPServerURL, Token: *request.Auth.Token}) if err != nil { writeStoreError(w, r, err) return @@ -109,5 +128,12 @@ func credentialResourceID(w http.ResponseWriter, r *http.Request, param string) } func credentialResponse(c store.Credential) v1.Credential { - return v1.Credential{ID: c.ID, VaultID: c.VaultID, Name: c.Name, Object: "vault.credential", Auth: v1.StaticBearerCredentialAuth{Type: c.AuthType, MCPServerURL: c.MCPServerURL}, CreatedAt: c.CreatedAt.Unix(), UpdatedAt: c.UpdatedAt.Unix()} + auth := v1.CredentialAuth{Type: c.AuthType, MCPServerURL: c.MCPServerURL} + if c.OAuth != nil { + auth.ExpiresAt = c.OAuth.ExpiresAt + if r := c.OAuth.Refresh; r != nil { + auth.Refresh = &v1.OAuthCredentialRefresh{ClientID: r.ClientID, TokenEndpoint: r.TokenEndpoint, TokenEndpointAuth: v1.OAuthEndpointAuth{Type: r.TokenEndpointAuth}, Resource: r.Resource, Scope: r.Scope} + } + } + return v1.Credential{ID: c.ID, VaultID: c.VaultID, Name: c.Name, Object: "vault.credential", Auth: auth, CreatedAt: c.CreatedAt.Unix(), UpdatedAt: c.UpdatedAt.Unix()} } diff --git a/services/agents-api/internal/api/credentials_oauth.go b/services/agents-api/internal/api/credentials_oauth.go new file mode 100644 index 000000000..d377a39ac --- /dev/null +++ b/services/agents-api/internal/api/credentials_oauth.go @@ -0,0 +1,113 @@ +package api + +import ( + "encoding/json" + "net/url" + "regexp" + "time" + + v1 "github.com/MiniMax-AI-Dev/parsar/contracts/agents-api/v1" + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/store" +) + +func credentialHTTPSURL(value *string) bool { + if value == nil { + return false + } + u, err := url.Parse(*value) + return err == nil && u.Scheme == "https" && u.Hostname() != "" && u.User == nil && u.Fragment == "" +} + +var credentialTimestamp = regexp.MustCompile(`^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)?(?:Z|[+-](?:[01]\d|2[0-3]):[0-5]\d)$`) + +func credentialExpiry(value *string) bool { + if value == nil { + return true + } + if !credentialTimestamp.MatchString(*value) { + return false + } + _, err := time.Parse(time.RFC3339Nano, *value) + return err == nil +} + +func credentialAuthType(raw json.RawMessage) string { + var value struct { + Type string `json:"type"` + } + if json.Unmarshal(raw, &value) != nil { + return "" + } + return value.Type +} + +func oauthCredentialCreate(raw json.RawMessage, name string) (store.CreateOAuthCredentialInput, error) { + var auth v1.CredentialAuthInput + input := store.CreateOAuthCredentialInput{Name: name} + if decodeInputObject(raw, &auth, "type", "mcp_server_url", "access_token", "expires_at", "refresh") != nil || auth.Type != "mcp_oauth" || auth.AccessToken == nil || !credentialHTTPSURL(auth.MCPServerURL) || !credentialExpiry(auth.ExpiresAt) { + return input, store.ErrInvalidInput + } + input.MCPServerURL, input.AccessToken = *auth.MCPServerURL, *auth.AccessToken + input.OAuth.ExpiresAt = auth.ExpiresAt + if r := auth.Refresh; r != nil { + if r.ClientID == nil || r.RefreshToken == nil || !credentialHTTPSURL(r.TokenEndpoint) || r.TokenEndpointAuth == nil { + return input, store.ErrInvalidInput + } + a := r.TokenEndpointAuth + switch a.Type { + case "none": + // Presence of client_secret is invalid even when explicitly null. + var fields struct { + Refresh struct { + Auth json.RawMessage `json:"token_endpoint_auth"` + } `json:"refresh"` + } + if json.Unmarshal(raw, &fields) != nil || decodeInputObject(fields.Refresh.Auth, &struct { + Type string `json:"type"` + }{}, "type") != nil { + return input, store.ErrInvalidInput + } + case "client_secret_basic", "client_secret_post": + if a.ClientSecret == nil { + return input, store.ErrInvalidInput + } + input.ClientSecret = *a.ClientSecret + default: + return input, store.ErrInvalidInput + } + input.RefreshToken = *r.RefreshToken + input.OAuth.Refresh = &store.OAuthRefreshMetadata{ClientID: *r.ClientID, TokenEndpoint: *r.TokenEndpoint, TokenEndpointAuth: a.Type, Resource: r.Resource, Scope: r.Scope} + } + return input, nil +} + +func oauthCredentialUpdate(raw json.RawMessage) (store.UpdateOAuthCredentialInput, error) { + var auth v1.CredentialAuthReplacement + var input store.UpdateOAuthCredentialInput + if decodeInputObject(raw, &auth, "type", "access_token", "expires_at", "refresh") != nil || auth.Type != "mcp_oauth" { + return input, store.ErrInvalidInput + } + input.AccessToken = auth.AccessToken + if len(auth.ExpiresAt) > 0 { + input.ExpiresAtSet = true + if json.Unmarshal(auth.ExpiresAt, &input.ExpiresAt) != nil || !credentialExpiry(input.ExpiresAt) { + return input, store.ErrInvalidInput + } + } + if r := auth.Refresh; r != nil { + input.Refresh = &store.OAuthRefreshUpdate{RefreshToken: r.RefreshToken} + if len(r.Scope) > 0 { + input.Refresh.ScopeSet = true + if json.Unmarshal(r.Scope, &input.Refresh.Scope) != nil { + return input, store.ErrInvalidInput + } + } + if a := r.TokenEndpointAuth; a != nil { + if a.Type != "client_secret_basic" && a.Type != "client_secret_post" { + return input, store.ErrInvalidInput + } + input.Refresh.TokenEndpointAuthType, input.Refresh.ClientSecret = a.Type, a.ClientSecret + } + } + return input, nil +} diff --git a/services/agents-api/internal/api/credentials_oauth_test.go b/services/agents-api/internal/api/credentials_oauth_test.go new file mode 100644 index 000000000..444025aeb --- /dev/null +++ b/services/agents-api/internal/api/credentials_oauth_test.go @@ -0,0 +1,166 @@ +package api + +import ( + "context" + "encoding/json" + "errors" + "reflect" + "strings" + "testing" + + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/store" +) + +func (f *credentialFixture) CreateOAuthCredential(_ context.Context, tenant, vault string, input store.CreateOAuthCredentialInput) (store.Credential, error) { + f.tenant, f.vault, f.oauthInput, f.calls = tenant, vault, input, f.calls+1 + f.credential.Name, f.credential.MCPServerURL, f.credential.AuthType, f.credential.OAuth = input.Name, input.MCPServerURL, "mcp_oauth", &input.OAuth + return f.credential, f.err +} + +func (f *credentialFixture) UpdateOAuthCredential(_ context.Context, tenant, vault, id string, input store.UpdateOAuthCredentialInput) (store.Credential, error) { + f.tenant, f.vault, f.id, f.oauthUpdate, f.calls = tenant, vault, id, input, f.calls+1 + return f.credential, f.err +} + +func oauthCreateBody(t *testing.T, auth map[string]any) string { + t.Helper() + raw, err := json.Marshal(map[string]any{"name": " OAuth credential \n", "auth": auth}) + if err != nil { + t.Fatal(err) + } + return string(raw) +} + +func TestOAuthCredentialVariantsAndSafeResourceReads(t *testing.T) { + for _, method := range []string{"", "none", "client_secret_basic", "client_secret_post"} { + t.Run(method, func(t *testing.T) { + h, f, tenant := credentialHandler(t) + auth := map[string]any{"type": "mcp_oauth", "mcp_server_url": "https://mcp.example/tools", "access_token": "access-canary", "expires_at": nil, "refresh": nil} + wantAuth := map[string]any{"type": "mcp_oauth", "mcp_server_url": "https://mcp.example/tools", "expires_at": nil, "refresh": nil} + if method != "" { + endpointAuth := map[string]any{"type": method} + if method != "none" { + endpointAuth["client_secret"] = "client-canary" + } + auth["refresh"] = map[string]any{"client_id": "test-client", "refresh_token": "refresh-canary", "token_endpoint": "https://issuer.example/token", "token_endpoint_auth": endpointAuth, "resource": nil, "scope": "read write"} + wantAuth["refresh"] = map[string]any{"client_id": "test-client", "token_endpoint": "https://issuer.example/token", "token_endpoint_auth": map[string]any{"type": method}, "resource": nil, "scope": "read write"} + } + path := "/v1/vaults/" + f.credential.VaultID + "/credentials" + w := credentialRequest(h, "POST", path, oauthCreateBody(t, auth)) + var body map[string]any + if w.Code != 200 || json.Unmarshal(w.Body.Bytes(), &body) != nil || !reflect.DeepEqual(body["auth"], wantAuth) || strings.Contains(w.Body.String(), "canary") { + t.Fatal("OAuth resource must expose only exact safe metadata", w.Code) + } + if f.tenant != tenant || f.vault != f.credential.VaultID || f.oauthInput.Name != "OAuth credential" || f.oauthInput.AccessToken != "access-canary" { + t.Fatal("OAuth creation lost authenticated scope or secret input") + } + if method != "" && (f.oauthInput.RefreshToken != "refresh-canary" || f.oauthInput.OAuth.Refresh.TokenEndpointAuth != method) { + t.Fatal("OAuth refresh input changed") + } + if method != "" && method != "none" && f.oauthInput.ClientSecret != "client-canary" { + t.Fatal("OAuth client secret changed") + } + read := credentialRequest(h, "GET", path+"/"+f.credential.ID, "") + if read.Code != 200 || read.Body.String() != w.Body.String() { + t.Fatal("OAuth retrieval changed safe metadata") + } + f.page = store.CredentialPage{Credentials: []store.Credential{f.credential}} + list := credentialRequest(h, "GET", path, "") + var page struct { + Data []map[string]any `json:"data"` + } + if list.Code != 200 || json.Unmarshal(list.Body.Bytes(), &page) != nil || len(page.Data) != 1 || !reflect.DeepEqual(page.Data[0], body) { + t.Fatal("OAuth listing changed safe metadata") + } + }) + } +} + +func TestOAuthCreateRejectsInvalidFieldsBeforeStorage(t *testing.T) { + const base = `{"type":"mcp_oauth","mcp_server_url":"https://mcp.example","access_token":"access-canary"` + const refresh = `,"refresh":{"client_id":"client","refresh_token":"refresh-canary","token_endpoint":"https://issuer.example/token","token_endpoint_auth":` + for _, auth := range []string{ + `{"type":"mcp_oauth","mcp_server_url":"https://mcp.example"}`, + `{"type":"mcp_oauth","mcp_server_url":"https://mcp.example","access_token":null}`, + `{"type":"mcp_oauth","mcp_server_url":"http://mcp.example","access_token":"access-canary"}`, + base + `,"token":"cross-variant"}`, base + `,"expires_at":3}`, base + `,"expires_at":"tomorrow"}`, + base + `,"refresh":{}}`, base + `,"refresh":[]}`, base + `,"refresh":{"client_id":null}}`, + base + refresh + `{"type":"none","client_secret":null}}}`, + base + refresh + `{"type":"none","client_secret":"client-canary"}}}`, + base + refresh + `{"type":"client_secret_basic"}}}`, + base + refresh + `{"type":"client_secret_post","client_secret":null}}}`, + base + refresh + `{"type":"unknown"}}}`, + base + refresh + `{"type":"none"},"unknown":"refresh-canary"}}`, + strings.Replace(base+refresh+`{"type":"none"}}}`, "https://issuer.example/token", "https://client-canary@issuer.example/token", 1), + } { + h, f, _ := credentialHandler(t) + w := credentialRequest(h, "POST", "/v1/vaults/"+f.credential.VaultID+"/credentials", `{"name":"OAuth","auth":`+auth+`}`) + if w.Code != 400 || f.calls != 0 || strings.Contains(w.Body.String(), "canary") { + t.Fatal("invalid OAuth creation reached storage or exposed secrets", w.Code) + } + } +} + +func TestOAuthUpdateRetainsPresenceAndSecretPointers(t *testing.T) { + text := func(s string) *string { return &s } + for _, tc := range []struct { + auth string + want store.UpdateOAuthCredentialInput + }{ + {`{"type":"mcp_oauth"}`, store.UpdateOAuthCredentialInput{}}, + {`{"type":"mcp_oauth","access_token":null,"refresh":null}`, store.UpdateOAuthCredentialInput{}}, + {`{"type":"mcp_oauth","access_token":""}`, store.UpdateOAuthCredentialInput{AccessToken: text("")}}, + {`{"type":"mcp_oauth","expires_at":null}`, store.UpdateOAuthCredentialInput{ExpiresAtSet: true}}, + {`{"type":"mcp_oauth","expires_at":"2026-09-22T12:30:00.123+08:00"}`, store.UpdateOAuthCredentialInput{ExpiresAtSet: true, ExpiresAt: text("2026-09-22T12:30:00.123+08:00")}}, + {`{"type":"mcp_oauth","refresh":{}}`, store.UpdateOAuthCredentialInput{Refresh: &store.OAuthRefreshUpdate{}}}, + {`{"type":"mcp_oauth","refresh":{"refresh_token":null,"token_endpoint_auth":null}}`, store.UpdateOAuthCredentialInput{Refresh: &store.OAuthRefreshUpdate{}}}, + {`{"type":"mcp_oauth","refresh":{"scope":null}}`, store.UpdateOAuthCredentialInput{Refresh: &store.OAuthRefreshUpdate{ScopeSet: true}}}, + {`{"type":"mcp_oauth","refresh":{"scope":"","refresh_token":"refresh-canary","token_endpoint_auth":{"type":"client_secret_post","client_secret":"client-canary"}}}`, store.UpdateOAuthCredentialInput{Refresh: &store.OAuthRefreshUpdate{Scope: text(""), ScopeSet: true, RefreshToken: text("refresh-canary"), TokenEndpointAuthType: "client_secret_post", ClientSecret: text("client-canary")}}}, + {`{"type":"mcp_oauth","refresh":{"token_endpoint_auth":{"type":"client_secret_basic","client_secret":null}}}`, store.UpdateOAuthCredentialInput{Refresh: &store.OAuthRefreshUpdate{TokenEndpointAuthType: "client_secret_basic"}}}, + } { + h, f, tenant := credentialHandler(t) + f.credential.AuthType, f.credential.OAuth = "mcp_oauth", &store.OAuthMetadata{} + w := credentialRequest(h, "POST", "/v1/vaults/"+f.credential.VaultID+"/credentials/"+f.credential.ID, `{"auth":`+tc.auth+`}`) + if w.Code != 200 || f.tenant != tenant || f.vault != f.credential.VaultID || f.id != f.credential.ID || !reflect.DeepEqual(f.oauthUpdate, tc.want) || strings.Contains(w.Body.String(), "canary") { + t.Fatal("OAuth update lost scope or nullable field intent", tc.auth, w.Code) + } + } +} + +func TestOAuthUpdateRejectsInvalidPatchesBeforeStorage(t *testing.T) { + for _, patch := range []string{ + `"token":"cross-variant"`, `"access_token":3`, `"expires_at":[]`, `"expires_at":"not a timestamp"`, `"expires_at":"2026-09-22T1:02:03Z"`, `"expires_at":"2026-09-22T01:02:03+24:00"`, + `"refresh":{"client_id":"other"}`, `"refresh":{"token_endpoint":"https://other.example"}`, + `"refresh":{"resource":null}`, `"refresh":{"scope":3}`, `"refresh":{"refresh_token":false}`, + `"refresh":{"token_endpoint_auth":{}}`, `"refresh":{"token_endpoint_auth":{"type":"none"}}`, + `"refresh":{"token_endpoint_auth":{"type":"client_secret_basic","client_secret":3}}`, + `"refresh":{"token_endpoint_auth":{"type":"client_secret_post","client_id":"other"}}`, + } { + h, f, _ := credentialHandler(t) + w := credentialRequest(h, "POST", "/v1/vaults/"+f.credential.VaultID+"/credentials/"+f.credential.ID, `{"auth":{"type":"mcp_oauth",`+patch+`}}`) + if w.Code != 400 || f.calls != 0 { + t.Fatal("invalid OAuth patch reached storage", patch, w.Code) + } + } +} + +func TestOAuthCredentialStoreFailuresUseSafeExistingErrors(t *testing.T) { + for _, tc := range []struct { + err error + code int + }{{store.ErrNotFound, 404}, {store.ErrInvalidInput, 400}, {store.ErrCredentialStorageUnavailable, 503}, {errors.New("access-canary"), 500}} { + for _, update := range []bool{false, true} { + h, f, _ := credentialHandler(t) + f.err = tc.err + path, body := "/v1/vaults/"+f.credential.VaultID+"/credentials", `{"name":"OAuth","auth":{"type":"mcp_oauth","mcp_server_url":"https://mcp.example","access_token":"access-canary"}}` + if update { + path += "/" + f.credential.ID + body = `{"auth":{"type":"mcp_oauth","access_token":"access-canary"}}` + } + w := credentialRequest(h, "POST", path, body) + if w.Code != tc.code || strings.Contains(w.Body.String(), "canary") { + t.Fatal("unsafe OAuth store error", w.Code) + } + } + } +} diff --git a/services/agents-api/internal/api/credentials_test.go b/services/agents-api/internal/api/credentials_test.go index 7eb9eb747..39eb0dd09 100644 --- a/services/agents-api/internal/api/credentials_test.go +++ b/services/agents-api/internal/api/credentials_test.go @@ -20,6 +20,8 @@ type credentialFixture struct { credential store.Credential input store.CreateStaticCredentialInput replacement store.UpdateStaticCredentialInput + oauthInput store.CreateOAuthCredentialInput + oauthUpdate store.UpdateOAuthCredentialInput tenant, vault, id string calls int err error diff --git a/services/agents-api/internal/api/credentials_update.go b/services/agents-api/internal/api/credentials_update.go index 7e8f6d763..b89bc8f91 100644 --- a/services/agents-api/internal/api/credentials_update.go +++ b/services/agents-api/internal/api/credentials_update.go @@ -1,14 +1,15 @@ package api import ( + "encoding/json" "net/http" v1 "github.com/MiniMax-AI-Dev/parsar/contracts/agents-api/v1" "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/store" ) -// @Summary Replace a static-bearer Vault Credential token -// @Description Requires auth with type=static_bearer and a string token; empty and opaque token bytes are preserved. Only the write-only secret and updated_at change, atomically within the authenticated project and owning Vault. Identity, name, auth type, exact destination, created_at and Session bindings remain unchanged. Responses contain only safe metadata; no old-token decryption or network call occurs. Missing encryption configuration returns local 503 without modifying the credential. Subsequent dispatch reads use the committed replacement; already-resolved requests may retain the old token. OAuth, storage-key rotation, hot reload/revocation and exact hosted concurrent-update/retry/timestamp semantics remain gaps. +// @Summary Replace Vault Credential authentication secrets +// @Description Updates the existing static_bearer or mcp_oauth authentication method without network requests. OAuth access_token omission/null retains the token; a new token clears omitted expiry, explicit null clears expiry, and other omitted fields remain unchanged. OAuth refresh patches cannot add configuration or change client, endpoint, resource or authentication method; nullable token/client-secret values retain stored secrets while explicit null scope clears scope. Whole-null refresh and token_endpoint_auth retain existing configuration under local policy. Identity, destination, creation time and Session bindings remain unchanged. Responses expose safe metadata only. Already-dispatched work is not revoked; provider revocation, storage-key rotation and exact hosted concurrent-update/error semantics remain separate. // @Tags Credentials // @Accept json // @Produce json @@ -16,7 +17,7 @@ import ( // @Param OpenAI-Beta header string true "agents=v1" // @Param vault_id path string true "Vault ID" // @Param credential_id path string true "Credential ID" -// @Param body body v1.UpdateCredentialRequest true "Write-only static bearer replacement" +// @Param body body v1.UpdateCredentialRequest true "Write-only credential authentication replacement union" // @Success 200 {object} v1.Credential // @Failure 400,401,404,413,500,503 {object} v1.ErrorResponse // @Router /vaults/{vault_id}/credentials/{credential_id} [post] @@ -37,12 +38,34 @@ func (h *Handler) updateCredential(w http.ResponseWriter, r *http.Request) { if !ok { return } - var request v1.UpdateCredentialRequest - if decodeInputObject(raw, &request, "auth") != nil || request.Auth == nil || request.Auth.Type != "static_bearer" || request.Auth.Token == nil { - writeError(w, http.StatusBadRequest, "invalid_request", "auth with type=static_bearer and a string token is required.") + var request struct { + Auth json.RawMessage `json:"auth"` + } + if decodeInputObject(raw, &request, "auth") != nil { + writeError(w, http.StatusBadRequest, "invalid_request", "auth with a supported type is required.") + return + } + var credential store.Credential + var err error + switch credentialAuthType(request.Auth) { + case "static_bearer": + var auth v1.CredentialAuthReplacement + if decodeInputObject(request.Auth, &auth, "type", "token") != nil || auth.Token == nil { + writeError(w, http.StatusBadRequest, "invalid_request", "static_bearer auth requires a string token.") + return + } + credential, err = h.store.UpdateStaticCredential(r.Context(), tenantID(r), vaultID, id, store.UpdateStaticCredentialInput{Token: *auth.Token}) + case "mcp_oauth": + input, parseErr := oauthCredentialUpdate(request.Auth) + if parseErr != nil { + writeStoreError(w, r, parseErr) + return + } + credential, err = h.store.UpdateOAuthCredential(r.Context(), tenantID(r), vaultID, id, input) + default: + writeError(w, http.StatusBadRequest, "invalid_request", "auth requires type static_bearer or mcp_oauth.") return } - credential, err := h.store.UpdateStaticCredential(r.Context(), tenantID(r), vaultID, id, store.UpdateStaticCredentialInput{Token: *request.Auth.Token}) if err != nil { writeStoreError(w, r, err) return diff --git a/services/agents-api/internal/api/credentials_update_test.go b/services/agents-api/internal/api/credentials_update_test.go index 82fdc7f2d..87a947da5 100644 --- a/services/agents-api/internal/api/credentials_update_test.go +++ b/services/agents-api/internal/api/credentials_update_test.go @@ -45,7 +45,7 @@ func TestCredentialUpdateRejectsInvalidBodiesBeforeStorage(t *testing.T) { `{"auth":{"type":null,"token":"credential-canary"}}`, `{"auth":{"type":3,"token":"credential-canary"}}`, `{"auth":{"type":"static_bearer","token":null}}`, `{"auth":{"type":"static_bearer","token":3}}`, `{"auth":{"type":"static_bearer","token":{}}}`, `{"auth":{"type":"static_bearer","token":[]}}`, - `{"auth":{"type":"mcp_oauth","access_token":"credential-canary"}}`, + `{"auth":{"type":"mcp_oauth","access_token":3}}`, `{"auth":{"type":"static_bearer","token":"credential-canary","mcp_server_url":"https://other.invalid"}}`, `{"auth":{"type":"static_bearer","token":"credential-canary"},"name":"other"}`, `{"auth":{"type":"static_bearer","token":"credential-canary"},"vault_id":"other"}`, diff --git a/services/agents-api/internal/db/queries/mcp_credentials.sql b/services/agents-api/internal/db/queries/mcp_credentials.sql index fc337e000..fb82e3804 100644 --- a/services/agents-api/internal/db/queries/mcp_credentials.sql +++ b/services/agents-api/internal/db/queries/mcp_credentials.sql @@ -2,13 +2,13 @@ SELECT id FROM vaults WHERE tenant_id = sqlc.arg(tenant_id) AND id = ANY(sqlc.arg(vault_ids)::uuid[]); --- name: FindMCPStaticCredentials :many +-- name: FindMCPCredentials :many SELECT c.id, c.vault_id, c.name, c.auth_type, c.mcp_server_url, c.created_at, c.updated_at FROM vault_credentials c JOIN vaults v ON v.id = c.vault_id WHERE v.tenant_id = sqlc.arg(tenant_id) AND v.id = ANY(sqlc.arg(vault_ids)::uuid[]) - AND c.auth_type = 'static_bearer' + AND c.auth_type IN ('static_bearer', 'mcp_oauth') AND c.mcp_server_url = sqlc.arg(mcp_server_url) AND (sqlc.narg(credential_id)::uuid IS NULL OR c.id = sqlc.narg(credential_id)::uuid) ORDER BY c.id diff --git a/services/agents-api/internal/db/queries/oauth_credentials.sql b/services/agents-api/internal/db/queries/oauth_credentials.sql new file mode 100644 index 000000000..44ff44c11 --- /dev/null +++ b/services/agents-api/internal/db/queries/oauth_credentials.sql @@ -0,0 +1,25 @@ +-- name: CreateOAuthCredential :one +INSERT INTO vault_credentials (id, vault_id, name, auth_type, mcp_server_url, token_ciphertext, oauth_metadata) +SELECT sqlc.arg(id), v.id, sqlc.arg(name), 'mcp_oauth', sqlc.arg(mcp_server_url), sqlc.arg(token_ciphertext), sqlc.arg(oauth_metadata) +FROM vaults v +WHERE v.tenant_id = sqlc.arg(tenant_id) AND v.id = sqlc.arg(vault_id) +RETURNING id, vault_id, name, auth_type, mcp_server_url, created_at, updated_at, oauth_metadata; + +-- name: GetOAuthCredentialForUpdate :one +SELECT c.id, c.vault_id, c.name, c.auth_type, c.mcp_server_url, c.created_at, c.updated_at, + c.oauth_metadata, c.token_ciphertext +FROM vault_credentials c +JOIN vaults v ON v.id = c.vault_id +WHERE v.tenant_id = sqlc.arg(tenant_id) AND v.id = sqlc.arg(vault_id) + AND c.id = sqlc.arg(id) AND c.auth_type = 'mcp_oauth' +FOR UPDATE OF c; + +-- name: UpdateOAuthCredential :one +UPDATE vault_credentials c +SET token_ciphertext = sqlc.arg(token_ciphertext), oauth_metadata = sqlc.arg(oauth_metadata), + updated_at = statement_timestamp() +FROM vaults v +WHERE v.id = c.vault_id AND v.tenant_id = sqlc.arg(tenant_id) + AND v.id = sqlc.arg(vault_id) AND c.id = sqlc.arg(id) + AND c.auth_type = 'mcp_oauth' AND c.mcp_server_url = sqlc.arg(mcp_server_url) +RETURNING c.id, c.vault_id, c.name, c.auth_type, c.mcp_server_url, c.created_at, c.updated_at, c.oauth_metadata; diff --git a/services/agents-api/internal/db/queries/vault_credentials.sql b/services/agents-api/internal/db/queries/vault_credentials.sql index 488a81459..735f0a621 100644 --- a/services/agents-api/internal/db/queries/vault_credentials.sql +++ b/services/agents-api/internal/db/queries/vault_credentials.sql @@ -3,10 +3,10 @@ INSERT INTO vault_credentials (id, vault_id, name, auth_type, mcp_server_url, to SELECT sqlc.arg(id), v.id, sqlc.arg(name), 'static_bearer', sqlc.arg(mcp_server_url), sqlc.arg(token_ciphertext) FROM vaults v WHERE v.tenant_id = sqlc.arg(tenant_id) AND v.id = sqlc.arg(vault_id) -RETURNING id, vault_id, name, auth_type, mcp_server_url, created_at, updated_at; +RETURNING id, vault_id, name, auth_type, mcp_server_url, created_at, updated_at, oauth_metadata; -- name: GetCredential :one -SELECT c.id, c.vault_id, c.name, c.auth_type, c.mcp_server_url, c.created_at, c.updated_at +SELECT c.id, c.vault_id, c.name, c.auth_type, c.mcp_server_url, c.created_at, c.updated_at, c.oauth_metadata FROM vault_credentials c JOIN vaults v ON v.id = c.vault_id WHERE v.tenant_id = sqlc.arg(tenant_id) AND v.id = sqlc.arg(vault_id) AND c.id = sqlc.arg(id); @@ -18,10 +18,10 @@ FROM vaults v WHERE v.id = c.vault_id AND v.tenant_id = sqlc.arg(tenant_id) AND v.id = sqlc.arg(vault_id) AND c.id = sqlc.arg(id) AND c.auth_type = 'static_bearer' AND c.mcp_server_url = sqlc.arg(mcp_server_url) -RETURNING c.id, c.vault_id, c.name, c.auth_type, c.mcp_server_url, c.created_at, c.updated_at; +RETURNING c.id, c.vault_id, c.name, c.auth_type, c.mcp_server_url, c.created_at, c.updated_at, c.oauth_metadata; -- name: ListCredentials :many -SELECT c.id, c.vault_id, c.name, c.auth_type, c.mcp_server_url, c.created_at, c.updated_at +SELECT c.id, c.vault_id, c.name, c.auth_type, c.mcp_server_url, c.created_at, c.updated_at, c.oauth_metadata FROM vault_credentials c JOIN vaults v ON v.id = c.vault_id WHERE v.tenant_id = sqlc.arg(tenant_id) AND v.id = sqlc.arg(vault_id) diff --git a/services/agents-api/internal/db/sqlc/mcp_credentials.sql.go b/services/agents-api/internal/db/sqlc/mcp_credentials.sql.go index 584f40b8c..5f2322efc 100644 --- a/services/agents-api/internal/db/sqlc/mcp_credentials.sql.go +++ b/services/agents-api/internal/db/sqlc/mcp_credentials.sql.go @@ -11,27 +11,27 @@ import ( "github.com/jackc/pgx/v5/pgtype" ) -const findMCPStaticCredentials = `-- name: FindMCPStaticCredentials :many +const findMCPCredentials = `-- name: FindMCPCredentials :many SELECT c.id, c.vault_id, c.name, c.auth_type, c.mcp_server_url, c.created_at, c.updated_at FROM vault_credentials c JOIN vaults v ON v.id = c.vault_id WHERE v.tenant_id = $1 AND v.id = ANY($2::uuid[]) - AND c.auth_type = 'static_bearer' + AND c.auth_type IN ('static_bearer', 'mcp_oauth') AND c.mcp_server_url = $3 AND ($4::uuid IS NULL OR c.id = $4::uuid) ORDER BY c.id LIMIT 2 ` -type FindMCPStaticCredentialsParams struct { +type FindMCPCredentialsParams struct { TenantID pgtype.UUID `json:"tenant_id"` VaultIds []pgtype.UUID `json:"vault_ids"` McpServerUrl string `json:"mcp_server_url"` CredentialID pgtype.UUID `json:"credential_id"` } -type FindMCPStaticCredentialsRow struct { +type FindMCPCredentialsRow struct { ID pgtype.UUID `json:"id"` VaultID pgtype.UUID `json:"vault_id"` Name string `json:"name"` @@ -41,8 +41,8 @@ type FindMCPStaticCredentialsRow struct { UpdatedAt pgtype.Timestamptz `json:"updated_at"` } -func (q *Queries) FindMCPStaticCredentials(ctx context.Context, arg FindMCPStaticCredentialsParams) ([]FindMCPStaticCredentialsRow, error) { - rows, err := q.db.Query(ctx, findMCPStaticCredentials, +func (q *Queries) FindMCPCredentials(ctx context.Context, arg FindMCPCredentialsParams) ([]FindMCPCredentialsRow, error) { + rows, err := q.db.Query(ctx, findMCPCredentials, arg.TenantID, arg.VaultIds, arg.McpServerUrl, @@ -52,9 +52,9 @@ func (q *Queries) FindMCPStaticCredentials(ctx context.Context, arg FindMCPStati return nil, err } defer rows.Close() - items := []FindMCPStaticCredentialsRow{} + items := []FindMCPCredentialsRow{} for rows.Next() { - var i FindMCPStaticCredentialsRow + var i FindMCPCredentialsRow if err := rows.Scan( &i.ID, &i.VaultID, diff --git a/services/agents-api/internal/db/sqlc/models.go b/services/agents-api/internal/db/sqlc/models.go index 2a3811d95..7b5a834e0 100644 --- a/services/agents-api/internal/db/sqlc/models.go +++ b/services/agents-api/internal/db/sqlc/models.go @@ -366,4 +366,5 @@ type VaultCredential struct { CreatedAt pgtype.Timestamptz `json:"created_at"` UpdatedAt pgtype.Timestamptz `json:"updated_at"` Status string `json:"status"` + OauthMetadata []byte `json:"oauth_metadata"` } diff --git a/services/agents-api/internal/db/sqlc/oauth_credentials.sql.go b/services/agents-api/internal/db/sqlc/oauth_credentials.sql.go new file mode 100644 index 000000000..f6e69f97e --- /dev/null +++ b/services/agents-api/internal/db/sqlc/oauth_credentials.sql.go @@ -0,0 +1,164 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.29.0 +// source: oauth_credentials.sql + +package sqlc + +import ( + "context" + + "github.com/jackc/pgx/v5/pgtype" +) + +const createOAuthCredential = `-- name: CreateOAuthCredential :one +INSERT INTO vault_credentials (id, vault_id, name, auth_type, mcp_server_url, token_ciphertext, oauth_metadata) +SELECT $1, v.id, $2, 'mcp_oauth', $3, $4, $5 +FROM vaults v +WHERE v.tenant_id = $6 AND v.id = $7 +RETURNING id, vault_id, name, auth_type, mcp_server_url, created_at, updated_at, oauth_metadata +` + +type CreateOAuthCredentialParams struct { + ID pgtype.UUID `json:"id"` + Name string `json:"name"` + McpServerUrl string `json:"mcp_server_url"` + TokenCiphertext []byte `json:"token_ciphertext"` + OauthMetadata []byte `json:"oauth_metadata"` + TenantID pgtype.UUID `json:"tenant_id"` + VaultID pgtype.UUID `json:"vault_id"` +} + +type CreateOAuthCredentialRow struct { + ID pgtype.UUID `json:"id"` + VaultID pgtype.UUID `json:"vault_id"` + Name string `json:"name"` + AuthType string `json:"auth_type"` + McpServerUrl string `json:"mcp_server_url"` + CreatedAt pgtype.Timestamptz `json:"created_at"` + UpdatedAt pgtype.Timestamptz `json:"updated_at"` + OauthMetadata []byte `json:"oauth_metadata"` +} + +func (q *Queries) CreateOAuthCredential(ctx context.Context, arg CreateOAuthCredentialParams) (CreateOAuthCredentialRow, error) { + row := q.db.QueryRow(ctx, createOAuthCredential, + arg.ID, + arg.Name, + arg.McpServerUrl, + arg.TokenCiphertext, + arg.OauthMetadata, + arg.TenantID, + arg.VaultID, + ) + var i CreateOAuthCredentialRow + err := row.Scan( + &i.ID, + &i.VaultID, + &i.Name, + &i.AuthType, + &i.McpServerUrl, + &i.CreatedAt, + &i.UpdatedAt, + &i.OauthMetadata, + ) + return i, err +} + +const getOAuthCredentialForUpdate = `-- name: GetOAuthCredentialForUpdate :one +SELECT c.id, c.vault_id, c.name, c.auth_type, c.mcp_server_url, c.created_at, c.updated_at, + c.oauth_metadata, c.token_ciphertext +FROM vault_credentials c +JOIN vaults v ON v.id = c.vault_id +WHERE v.tenant_id = $1 AND v.id = $2 + AND c.id = $3 AND c.auth_type = 'mcp_oauth' +FOR UPDATE OF c +` + +type GetOAuthCredentialForUpdateParams struct { + TenantID pgtype.UUID `json:"tenant_id"` + VaultID pgtype.UUID `json:"vault_id"` + ID pgtype.UUID `json:"id"` +} + +type GetOAuthCredentialForUpdateRow struct { + ID pgtype.UUID `json:"id"` + VaultID pgtype.UUID `json:"vault_id"` + Name string `json:"name"` + AuthType string `json:"auth_type"` + McpServerUrl string `json:"mcp_server_url"` + CreatedAt pgtype.Timestamptz `json:"created_at"` + UpdatedAt pgtype.Timestamptz `json:"updated_at"` + OauthMetadata []byte `json:"oauth_metadata"` + TokenCiphertext []byte `json:"token_ciphertext"` +} + +func (q *Queries) GetOAuthCredentialForUpdate(ctx context.Context, arg GetOAuthCredentialForUpdateParams) (GetOAuthCredentialForUpdateRow, error) { + row := q.db.QueryRow(ctx, getOAuthCredentialForUpdate, arg.TenantID, arg.VaultID, arg.ID) + var i GetOAuthCredentialForUpdateRow + err := row.Scan( + &i.ID, + &i.VaultID, + &i.Name, + &i.AuthType, + &i.McpServerUrl, + &i.CreatedAt, + &i.UpdatedAt, + &i.OauthMetadata, + &i.TokenCiphertext, + ) + return i, err +} + +const updateOAuthCredential = `-- name: UpdateOAuthCredential :one +UPDATE vault_credentials c +SET token_ciphertext = $1, oauth_metadata = $2, + updated_at = statement_timestamp() +FROM vaults v +WHERE v.id = c.vault_id AND v.tenant_id = $3 + AND v.id = $4 AND c.id = $5 + AND c.auth_type = 'mcp_oauth' AND c.mcp_server_url = $6 +RETURNING c.id, c.vault_id, c.name, c.auth_type, c.mcp_server_url, c.created_at, c.updated_at, c.oauth_metadata +` + +type UpdateOAuthCredentialParams struct { + TokenCiphertext []byte `json:"token_ciphertext"` + OauthMetadata []byte `json:"oauth_metadata"` + TenantID pgtype.UUID `json:"tenant_id"` + VaultID pgtype.UUID `json:"vault_id"` + ID pgtype.UUID `json:"id"` + McpServerUrl string `json:"mcp_server_url"` +} + +type UpdateOAuthCredentialRow struct { + ID pgtype.UUID `json:"id"` + VaultID pgtype.UUID `json:"vault_id"` + Name string `json:"name"` + AuthType string `json:"auth_type"` + McpServerUrl string `json:"mcp_server_url"` + CreatedAt pgtype.Timestamptz `json:"created_at"` + UpdatedAt pgtype.Timestamptz `json:"updated_at"` + OauthMetadata []byte `json:"oauth_metadata"` +} + +func (q *Queries) UpdateOAuthCredential(ctx context.Context, arg UpdateOAuthCredentialParams) (UpdateOAuthCredentialRow, error) { + row := q.db.QueryRow(ctx, updateOAuthCredential, + arg.TokenCiphertext, + arg.OauthMetadata, + arg.TenantID, + arg.VaultID, + arg.ID, + arg.McpServerUrl, + ) + var i UpdateOAuthCredentialRow + err := row.Scan( + &i.ID, + &i.VaultID, + &i.Name, + &i.AuthType, + &i.McpServerUrl, + &i.CreatedAt, + &i.UpdatedAt, + &i.OauthMetadata, + ) + return i, err +} diff --git a/services/agents-api/internal/db/sqlc/vault_credentials.sql.go b/services/agents-api/internal/db/sqlc/vault_credentials.sql.go index bdfd9d8bb..f20245175 100644 --- a/services/agents-api/internal/db/sqlc/vault_credentials.sql.go +++ b/services/agents-api/internal/db/sqlc/vault_credentials.sql.go @@ -16,7 +16,7 @@ INSERT INTO vault_credentials (id, vault_id, name, auth_type, mcp_server_url, to SELECT $1, v.id, $2, 'static_bearer', $3, $4 FROM vaults v WHERE v.tenant_id = $5 AND v.id = $6 -RETURNING id, vault_id, name, auth_type, mcp_server_url, created_at, updated_at +RETURNING id, vault_id, name, auth_type, mcp_server_url, created_at, updated_at, oauth_metadata ` type CreateStaticCredentialParams struct { @@ -29,13 +29,14 @@ type CreateStaticCredentialParams struct { } type CreateStaticCredentialRow struct { - ID pgtype.UUID `json:"id"` - VaultID pgtype.UUID `json:"vault_id"` - Name string `json:"name"` - AuthType string `json:"auth_type"` - McpServerUrl string `json:"mcp_server_url"` - CreatedAt pgtype.Timestamptz `json:"created_at"` - UpdatedAt pgtype.Timestamptz `json:"updated_at"` + ID pgtype.UUID `json:"id"` + VaultID pgtype.UUID `json:"vault_id"` + Name string `json:"name"` + AuthType string `json:"auth_type"` + McpServerUrl string `json:"mcp_server_url"` + CreatedAt pgtype.Timestamptz `json:"created_at"` + UpdatedAt pgtype.Timestamptz `json:"updated_at"` + OauthMetadata []byte `json:"oauth_metadata"` } func (q *Queries) CreateStaticCredential(ctx context.Context, arg CreateStaticCredentialParams) (CreateStaticCredentialRow, error) { @@ -56,6 +57,7 @@ func (q *Queries) CreateStaticCredential(ctx context.Context, arg CreateStaticCr &i.McpServerUrl, &i.CreatedAt, &i.UpdatedAt, + &i.OauthMetadata, ) return i, err } @@ -82,7 +84,7 @@ func (q *Queries) DeleteCredential(ctx context.Context, arg DeleteCredentialPara } const getCredential = `-- name: GetCredential :one -SELECT c.id, c.vault_id, c.name, c.auth_type, c.mcp_server_url, c.created_at, c.updated_at +SELECT c.id, c.vault_id, c.name, c.auth_type, c.mcp_server_url, c.created_at, c.updated_at, c.oauth_metadata FROM vault_credentials c JOIN vaults v ON v.id = c.vault_id WHERE v.tenant_id = $1 AND v.id = $2 AND c.id = $3 @@ -95,13 +97,14 @@ type GetCredentialParams struct { } type GetCredentialRow struct { - ID pgtype.UUID `json:"id"` - VaultID pgtype.UUID `json:"vault_id"` - Name string `json:"name"` - AuthType string `json:"auth_type"` - McpServerUrl string `json:"mcp_server_url"` - CreatedAt pgtype.Timestamptz `json:"created_at"` - UpdatedAt pgtype.Timestamptz `json:"updated_at"` + ID pgtype.UUID `json:"id"` + VaultID pgtype.UUID `json:"vault_id"` + Name string `json:"name"` + AuthType string `json:"auth_type"` + McpServerUrl string `json:"mcp_server_url"` + CreatedAt pgtype.Timestamptz `json:"created_at"` + UpdatedAt pgtype.Timestamptz `json:"updated_at"` + OauthMetadata []byte `json:"oauth_metadata"` } func (q *Queries) GetCredential(ctx context.Context, arg GetCredentialParams) (GetCredentialRow, error) { @@ -115,12 +118,13 @@ func (q *Queries) GetCredential(ctx context.Context, arg GetCredentialParams) (G &i.McpServerUrl, &i.CreatedAt, &i.UpdatedAt, + &i.OauthMetadata, ) return i, err } const listCredentials = `-- name: ListCredentials :many -SELECT c.id, c.vault_id, c.name, c.auth_type, c.mcp_server_url, c.created_at, c.updated_at +SELECT c.id, c.vault_id, c.name, c.auth_type, c.mcp_server_url, c.created_at, c.updated_at, c.oauth_metadata FROM vault_credentials c JOIN vaults v ON v.id = c.vault_id WHERE v.tenant_id = $1 AND v.id = $2 @@ -147,13 +151,14 @@ type ListCredentialsParams struct { } type ListCredentialsRow struct { - ID pgtype.UUID `json:"id"` - VaultID pgtype.UUID `json:"vault_id"` - Name string `json:"name"` - AuthType string `json:"auth_type"` - McpServerUrl string `json:"mcp_server_url"` - CreatedAt pgtype.Timestamptz `json:"created_at"` - UpdatedAt pgtype.Timestamptz `json:"updated_at"` + ID pgtype.UUID `json:"id"` + VaultID pgtype.UUID `json:"vault_id"` + Name string `json:"name"` + AuthType string `json:"auth_type"` + McpServerUrl string `json:"mcp_server_url"` + CreatedAt pgtype.Timestamptz `json:"created_at"` + UpdatedAt pgtype.Timestamptz `json:"updated_at"` + OauthMetadata []byte `json:"oauth_metadata"` } func (q *Queries) ListCredentials(ctx context.Context, arg ListCredentialsParams) ([]ListCredentialsRow, error) { @@ -181,6 +186,7 @@ func (q *Queries) ListCredentials(ctx context.Context, arg ListCredentialsParams &i.McpServerUrl, &i.CreatedAt, &i.UpdatedAt, + &i.OauthMetadata, ); err != nil { return nil, err } @@ -199,7 +205,7 @@ FROM vaults v WHERE v.id = c.vault_id AND v.tenant_id = $2 AND v.id = $3 AND c.id = $4 AND c.auth_type = 'static_bearer' AND c.mcp_server_url = $5 -RETURNING c.id, c.vault_id, c.name, c.auth_type, c.mcp_server_url, c.created_at, c.updated_at +RETURNING c.id, c.vault_id, c.name, c.auth_type, c.mcp_server_url, c.created_at, c.updated_at, c.oauth_metadata ` type UpdateStaticCredentialParams struct { @@ -211,13 +217,14 @@ type UpdateStaticCredentialParams struct { } type UpdateStaticCredentialRow struct { - ID pgtype.UUID `json:"id"` - VaultID pgtype.UUID `json:"vault_id"` - Name string `json:"name"` - AuthType string `json:"auth_type"` - McpServerUrl string `json:"mcp_server_url"` - CreatedAt pgtype.Timestamptz `json:"created_at"` - UpdatedAt pgtype.Timestamptz `json:"updated_at"` + ID pgtype.UUID `json:"id"` + VaultID pgtype.UUID `json:"vault_id"` + Name string `json:"name"` + AuthType string `json:"auth_type"` + McpServerUrl string `json:"mcp_server_url"` + CreatedAt pgtype.Timestamptz `json:"created_at"` + UpdatedAt pgtype.Timestamptz `json:"updated_at"` + OauthMetadata []byte `json:"oauth_metadata"` } func (q *Queries) UpdateStaticCredential(ctx context.Context, arg UpdateStaticCredentialParams) (UpdateStaticCredentialRow, error) { @@ -237,6 +244,7 @@ func (q *Queries) UpdateStaticCredential(ctx context.Context, arg UpdateStaticCr &i.McpServerUrl, &i.CreatedAt, &i.UpdatedAt, + &i.OauthMetadata, ) return i, err } diff --git a/services/agents-api/internal/execution/mcp_credentials.go b/services/agents-api/internal/execution/mcp_credentials.go index 9aad2c6cf..0611e40f4 100644 --- a/services/agents-api/internal/execution/mcp_credentials.go +++ b/services/agents-api/internal/execution/mcp_credentials.go @@ -47,7 +47,7 @@ func selectedMCPCredentials(snapshot Snapshot) (map[string]store.MCPCredentialBi continue } vaultID, err := uuid.Parse(binding.VaultID) - if err != nil || !attached[vaultID] || binding.AuthType != "static_bearer" { + if err != nil || !attached[vaultID] || (binding.AuthType != "static_bearer" && binding.AuthType != "mcp_oauth") { return nil, invalid } id, err := uuid.Parse(binding.CredentialID) diff --git a/services/agents-api/internal/execution/mcp_credentials_test.go b/services/agents-api/internal/execution/mcp_credentials_test.go index 1216278b1..1711b72dc 100644 --- a/services/agents-api/internal/execution/mcp_credentials_test.go +++ b/services/agents-api/internal/execution/mcp_credentials_test.go @@ -12,13 +12,13 @@ import ( func TestMCPFrozenCredentialAdmission(t *testing.T) { vault, credential := uuid.NewString(), uuid.NewString() - for _, mode := range []string{"implicit", "explicit", "anonymous", "missing", "unattached", "wrong URL", "wrong auth", "changed selection", "HTTP", "remote", "self-hosted explicit", "self-hosted implicit", "self-hosted anonymous"} { + for _, mode := range []string{"implicit", "explicit", "oauth implicit", "oauth explicit", "anonymous", "missing", "unattached", "wrong URL", "wrong auth", "changed selection", "HTTP", "remote", "self-hosted explicit", "self-hosted implicit", "self-hosted anonymous"} { t.Run(mode, func(t *testing.T) { tool := v1.MCPTool{Type: "mcp", ServerLabel: "tools", ConnectionOrigin: "service", Transport: v1.MCPHTTPTransport{Type: "http", ServerURL: "https://mcp.example/tools"}} binding := store.MCPCredentialBinding{ServerLabel: "tools", ServerURL: tool.Transport.ServerURL, VaultID: vault, CredentialID: credential, AuthType: "static_bearer"} snapshot := Snapshot{Agent: v1.Agent{Model: "model"}, Environment: &v1.Environment{Type: "none"}, VaultIDs: []string{vault}} switch mode { - case "explicit", "missing", "changed selection", "self-hosted explicit": + case "explicit", "oauth explicit", "missing", "changed selection", "self-hosted explicit": tool.CredentialID = &credential case "anonymous", "self-hosted anonymous": binding.VaultID, binding.CredentialID, binding.AuthType = "", "", "" @@ -33,6 +33,9 @@ func TestMCPFrozenCredentialAdmission(t *testing.T) { case "remote": snapshot.Daemon = &DaemonConfig{WorkDir: "/tmp"} } + if strings.HasPrefix(mode, "oauth ") { + binding.AuthType = "mcp_oauth" + } if strings.HasPrefix(mode, "self-hosted") { snapshot.Environment = &v1.Environment{Type: "self_hosted", WorkspaceDirectory: "/workspace"} } @@ -46,7 +49,7 @@ func TestMCPFrozenCredentialAdmission(t *testing.T) { rawTool, _ := json.Marshal(tool) snapshot.Agent.Tools = []json.RawMessage{rawTool} raw, _ := json.Marshal(snapshot) - valid := mode == "implicit" || mode == "explicit" || mode == "anonymous" + valid := mode == "implicit" || mode == "explicit" || mode == "anonymous" || strings.HasPrefix(mode, "oauth ") for _, engine := range []string{"codex", "claude_sdk"} { supported := valid && (engine == "codex" || !strings.HasPrefix(mode, "self-hosted")) if err := (Policy{}).ValidateSessionConfiguration(engine, raw); (err == nil) != supported { diff --git a/services/agents-api/internal/oauthrefresh/client.go b/services/agents-api/internal/oauthrefresh/client.go new file mode 100644 index 000000000..3e082768c --- /dev/null +++ b/services/agents-api/internal/oauthrefresh/client.go @@ -0,0 +1,123 @@ +package oauthrefresh + +import ( + "context" + "errors" + "io" + "net/http" + "net/url" + "strings" + "time" + + "golang.org/x/oauth2" +) + +var ErrRefresh = errors.New("OAuth credential refresh failed") + +// Client has immutable network policy. Grant state belongs to PostgreSQL, not a +// process-local TokenSource cache, so replacement and deletion stay authoritative. +type Client struct { + transport *http.Transport +} + +// NewClient permits public HTTPS endpoints by default. Operators may trust exact +// HTTPS origins for private issuers; tenants cannot relax this network policy. +func NewClient(privateOrigins []string) (*Client, error) { + policy, err := newNetworkPolicy(privateOrigins) + if err != nil { + return nil, err + } + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.Proxy = nil + transport.DialContext = policy.dialContext + transport.TLSHandshakeTimeout = 5 * time.Second + transport.ResponseHeaderTimeout = 5 * time.Second + return &Client{transport: transport}, nil +} + +func (c *Client) Refresh(ctx context.Context, input Request) (Token, error) { + if c == nil || c.transport == nil { + return Token{}, ErrRefresh + } + endpoint, err := parseEndpoint(input.TokenEndpoint) + if err != nil { + return Token{}, ErrRefresh + } + style := oauth2.AuthStyleInParams + switch input.AuthMethod { + case "none": + if input.ClientSecret != "" { + return Token{}, ErrRefresh + } + case "client_secret_post": + case "client_secret_basic": + style = oauth2.AuthStyleInHeader + default: + return Token{}, ErrRefresh + } + client := &http.Client{ + Transport: refreshFormTransport{base: c.transport, endpoint: endpoint.String(), input: input}, + Timeout: 10 * time.Second, + CheckRedirect: func(*http.Request, []*http.Request) error { return ErrRefresh }, + } + ctx = context.WithValue(ctx, oauth2.HTTPClient, client) + config := oauth2.Config{ClientID: input.ClientID, ClientSecret: input.ClientSecret, + Endpoint: oauth2.Endpoint{TokenURL: endpoint.String(), AuthStyle: style}} + token, err := config.TokenSource(ctx, &oauth2.Token{RefreshToken: input.RefreshToken}).Token() + // RetrieveError contains provider response bodies and must never escape. + if err != nil || token.AccessToken == "" || !strings.EqualFold(token.TokenType, "Bearer") { + return Token{}, ErrRefresh + } + result := Token{AccessToken: token.AccessToken, RefreshToken: token.RefreshToken} + if !token.Expiry.IsZero() { + if !token.Expiry.After(time.Now()) { + return Token{}, ErrRefresh + } + expiry := token.Expiry.UTC() + result.ExpiresAt = &expiry + } + return result, nil +} + +// oauth2's refresh TokenSource omits scope/resource. Add only these pinned grant +// fields to its one token request, retaining the library's auth and response logic. +type refreshFormTransport struct { + base http.RoundTripper + endpoint string + input Request +} + +func (t refreshFormTransport) RoundTrip(request *http.Request) (*http.Response, error) { + if request.Method != http.MethodPost || request.URL.String() != t.endpoint { + return nil, ErrRefresh + } + body, err := io.ReadAll(request.Body) + request.Body.Close() + if err != nil { + return nil, ErrRefresh + } + values, err := url.ParseQuery(string(body)) + if err != nil { + return nil, ErrRefresh + } + // Explicit client authentication also preserves empty strings from the stored + // grant; the library omits them when using AuthStyleInParams. + if t.input.AuthMethod != "client_secret_basic" { + values.Set("client_id", t.input.ClientID) + if t.input.AuthMethod == "client_secret_post" { + values.Set("client_secret", t.input.ClientSecret) + } + } + if t.input.Scope != nil { + values.Set("scope", *t.input.Scope) + } + if t.input.Resource != nil { + values.Set("resource", *t.input.Resource) + } + clone := request.Clone(request.Context()) + encoded := values.Encode() + clone.Body = io.NopCloser(strings.NewReader(encoded)) + clone.ContentLength = int64(len(encoded)) + clone.GetBody = nil + return t.base.RoundTrip(clone) +} diff --git a/services/agents-api/internal/oauthrefresh/client_test.go b/services/agents-api/internal/oauthrefresh/client_test.go new file mode 100644 index 000000000..a9683a1a9 --- /dev/null +++ b/services/agents-api/internal/oauthrefresh/client_test.go @@ -0,0 +1,151 @@ +package oauthrefresh + +import ( + "context" + "crypto/tls" + "crypto/x509" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync/atomic" + "testing" + "time" +) + +func TestRefreshGrantAndAuthentication(t *testing.T) { + for _, method := range []string{"none", "client_secret_basic", "client_secret_post"} { + t.Run(method, func(t *testing.T) { + var calls atomic.Int32 + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls.Add(1) + if r.Method != "POST" || r.ParseForm() != nil { + t.Error("invalid token request") + } + if r.Form.Get("grant_type") != "refresh_token" || r.Form.Get("refresh_token") != "refresh-old" || r.Form.Get("scope") != "read write" || r.Form.Get("resource") != "https://mcp.example/tool?a=1&b=2" { + t.Error("grant fields missing or changed") + } + if method == "client_secret_basic" { + id, secret, ok := r.BasicAuth() + if !ok || id != url.QueryEscape("client +id") || secret != url.QueryEscape("secret &+=") || r.Form.Has("client_secret") || r.Form.Has("client_id") { + t.Error("incorrect basic authentication") + } + } else { + if r.Header.Get("Authorization") != "" || r.Form.Get("client_id") != "client +id" { + t.Error("unexpected header or client") + } + if method == "none" && r.Form.Has("client_secret") { + t.Error("public client sent a secret") + } + if method == "client_secret_post" && r.Form.Get("client_secret") != "secret &+=" { + t.Error("missing client secret") + } + } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]any{"access_token": "access-new", "refresh_token": "refresh-new", "token_type": "Bearer", "expires_in": 300}) + })) + defer server.Close() + client := trustedClient(t, server) + scope, resource := "read write", "https://mcp.example/tool?a=1&b=2" + input := Request{TokenEndpoint: server.URL, ClientID: "client +id", AuthMethod: method, RefreshToken: "refresh-old", Scope: &scope, Resource: &resource} + if method != "none" { + input.ClientSecret = "secret &+=" + } + token, err := client.Refresh(context.Background(), input) + if err != nil || token.AccessToken != "access-new" || token.RefreshToken != "refresh-new" || token.ExpiresAt == nil || !token.ExpiresAt.After(time.Now().Add(250*time.Second)) || calls.Load() != 1 { + t.Fatal("refresh did not preserve the complete grant", token.ExpiresAt, calls.Load(), err) + } + }) + } +} + +func trustedClient(t *testing.T, server *httptest.Server) *Client { + t.Helper() + client, err := NewClient([]string{server.URL}) + if err != nil { + t.Fatal(err) + } + roots := x509.NewCertPool() + roots.AddCert(server.Certificate()) + client.transport.TLSClientConfig = &tls.Config{RootCAs: roots} + t.Cleanup(client.transport.CloseIdleConnections) + return client +} + +func TestRefreshFailuresAreSecretSafeAndDoNotRetry(t *testing.T) { + for _, mode := range []string{"invalid_grant", "missing_token", "wrong_type", "malformed", "oversized", "redirect"} { + t.Run(mode, func(t *testing.T) { + var calls atomic.Int32 + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls.Add(1) + w.Header().Set("Content-Type", "application/json") + switch mode { + case "invalid_grant": + w.WriteHeader(400) + w.Write([]byte(`{"error":"invalid_grant","error_description":"DO-NOT-LEAK-SECRET"}`)) + case "missing_token": + w.Write([]byte(`{"token_type":"Bearer"}`)) + case "wrong_type": + w.Write([]byte(`{"access_token":"DO-NOT-LEAK-SECRET","token_type":"mac"}`)) + case "malformed": + w.Write([]byte("DO-NOT-LEAK-SECRET")) + case "oversized": + w.Write([]byte(`{"access_token":"` + strings.Repeat("x", 1<<20) + `","token_type":"Bearer"}`)) + case "redirect": + w.Header().Set("Location", "/stolen") + w.WriteHeader(307) + } + })) + defer server.Close() + client := trustedClient(t, server) + _, err := client.Refresh(context.Background(), Request{TokenEndpoint: server.URL, ClientID: "client", AuthMethod: "client_secret_basic", ClientSecret: "DO-NOT-LEAK-SECRET", RefreshToken: "refresh"}) + if err != ErrRefresh || calls.Load() != 1 { + t.Fatal("failure leaked details or retried", err, calls.Load()) + } + }) + } +} + +func TestRefreshRetainsTokenAndHonorsCancellation(t *testing.T) { + var block atomic.Bool + release := make(chan struct{}) + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if block.Load() { + select { + case <-r.Context().Done(): + case <-release: + } + return + } + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"access_token":"next","token_type":"bearer"}`)) + })) + defer server.Close() + defer close(release) + client := trustedClient(t, server) + input := Request{TokenEndpoint: server.URL, AuthMethod: "none", ClientID: "public", RefreshToken: "retain"} + token, err := client.Refresh(context.Background(), input) + if err != nil || token.RefreshToken != "retain" || token.ExpiresAt != nil { + t.Fatal("omitted grant fields changed", err) + } + block.Store(true) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Millisecond) + defer cancel() + if _, err := client.Refresh(ctx, input); err != ErrRefresh { + t.Fatal(err) + } +} + +func TestPrivateOriginDoesNotDisableTLSVerification(t *testing.T) { + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { t.Error("untrusted certificate reached provider") })) + defer server.Close() + client, err := NewClient([]string{server.URL}) + if err != nil { + t.Fatal(err) + } + defer client.transport.CloseIdleConnections() + if _, err := client.Refresh(context.Background(), Request{TokenEndpoint: server.URL, AuthMethod: "none", RefreshToken: "secret"}); err != ErrRefresh { + t.Fatal(err) + } +} diff --git a/services/agents-api/internal/oauthrefresh/network.go b/services/agents-api/internal/oauthrefresh/network.go new file mode 100644 index 000000000..27fd6c59c --- /dev/null +++ b/services/agents-api/internal/oauthrefresh/network.go @@ -0,0 +1,84 @@ +package oauthrefresh + +import ( + "context" + "errors" + "net" + "net/netip" + "net/url" + "strings" + "time" +) + +func parseEndpoint(raw string) (*url.URL, error) { + u, err := url.Parse(raw) + if err != nil || u.Scheme != "https" || u.Hostname() == "" || u.User != nil || u.Fragment != "" || u.Opaque != "" { + return nil, ErrRefresh + } + return u, nil +} + +func endpointAddress(u *url.URL) string { + port := u.Port() + if port == "" { + port = "443" + } + return net.JoinHostPort(strings.ToLower(u.Hostname()), port) +} + +type networkPolicy struct { + privateAddresses map[string]bool + lookup func(context.Context, string) ([]net.IPAddr, error) + dial func(context.Context, string, string) (net.Conn, error) +} + +func newNetworkPolicy(origins []string) (*networkPolicy, error) { + p := &networkPolicy{privateAddresses: map[string]bool{}, lookup: net.DefaultResolver.LookupIPAddr, + dial: (&net.Dialer{Timeout: 5 * time.Second, KeepAlive: 30 * time.Second}).DialContext} + for _, raw := range origins { + u, err := parseEndpoint(raw) + if err != nil || (u.Path != "" && u.Path != "/") || u.RawQuery != "" || u.ForceQuery { + return nil, errors.New("invalid OAuth trusted origin configuration") + } + p.privateAddresses[endpointAddress(u)] = true + } + return p, nil +} + +func (p *networkPolicy) dialContext(ctx context.Context, network, address string) (net.Conn, error) { + host, port, err := net.SplitHostPort(address) + if err != nil { + return nil, ErrRefresh + } + addresses, err := p.lookup(ctx, host) + if err != nil || len(addresses) == 0 { + return nil, ErrRefresh + } + trusted := p.privateAddresses[net.JoinHostPort(strings.ToLower(host), port)] + // Validate the complete answer before dialing any address. Use the resolved IP + // directly so a second DNS lookup cannot switch a public hostname to an internal + // service after validation. HTTPS still verifies the original hostname. + for _, item := range addresses { + ip, ok := netip.AddrFromSlice(item.IP) + if !ok || item.Zone != "" || (!trusted && !publicAddress(ip.Unmap())) { + return nil, ErrRefresh + } + } + for _, item := range addresses { + conn, err := p.dial(ctx, network, net.JoinHostPort(item.IP.String(), port)) + if err == nil { + return conn, nil + } + if ctx.Err() != nil { + break + } + } + return nil, ErrRefresh +} + +var sharedAddressSpace = netip.MustParsePrefix("100.64.0.0/10") + +func publicAddress(ip netip.Addr) bool { + return ip.IsGlobalUnicast() && !ip.IsPrivate() && !ip.IsLoopback() && + !ip.IsLinkLocalUnicast() && !sharedAddressSpace.Contains(ip) +} diff --git a/services/agents-api/internal/oauthrefresh/network_test.go b/services/agents-api/internal/oauthrefresh/network_test.go new file mode 100644 index 000000000..0a1cd54e8 --- /dev/null +++ b/services/agents-api/internal/oauthrefresh/network_test.go @@ -0,0 +1,82 @@ +package oauthrefresh + +import ( + "context" + "errors" + "net" + "strings" + "testing" +) + +func TestRefreshDialChecksEveryDNSAddressAndPinsResolution(t *testing.T) { + for _, tc := range []struct { + name string + addresses []string + trusted bool + dial bool + }{ + {"public", []string{"8.8.8.8"}, false, true}, + {"loopback", []string{"127.0.0.1"}, false, false}, + {"private", []string{"10.0.0.1"}, false, false}, + {"metadata", []string{"169.254.169.254"}, false, false}, + {"shared", []string{"100.100.100.200"}, false, false}, + {"mapped", []string{"::ffff:127.0.0.1"}, false, false}, + {"mixed", []string{"8.8.8.8", "10.0.0.1"}, false, false}, + {"ipv6-private", []string{"fd00::1"}, false, false}, + {"operator-trusted", []string{"10.0.0.1"}, true, true}, + } { + t.Run(tc.name, func(t *testing.T) { + var origins []string + if tc.trusted { + origins = []string{"https://issuer.example:9443"} + } + p, err := newNetworkPolicy(origins) + if err != nil { + t.Fatal(err) + } + lookups, calls := 0, 0 + p.lookup = func(_ context.Context, host string) ([]net.IPAddr, error) { + lookups++ + if host != "issuer.example" { + t.Fatal(host) + } + var out []net.IPAddr + for _, s := range tc.addresses { + out = append(out, net.IPAddr{IP: net.ParseIP(s)}) + } + return out, nil + } + p.dial = func(_ context.Context, _, address string) (net.Conn, error) { + calls++ + if strings.Contains(address, "issuer.example") { + t.Fatal("second DNS lookup allowed") + } + return nil, errors.New("test dial") + } + _, err = p.dialContext(context.Background(), "tcp", "issuer.example:9443") + if err != ErrRefresh || lookups != 1 || (calls > 0) != tc.dial { + t.Fatal("unexpected policy decision", lookups, calls, err) + } + }) + } +} + +func TestOAuthTrustedOriginsAreExactAndOperatorOnly(t *testing.T) { + for _, origin := range []string{"http://issuer.example", "https://user:secret@issuer.example", "https://issuer.example/token", "https://issuer.example?x=1", "https://issuer.example#fragment", ""} { + if _, err := NewClient([]string{origin}); err == nil { + t.Fatalf("accepted invalid trusted origin %q", origin) + } + } + p, err := newNetworkPolicy([]string{"https://issuer.example"}) + if err != nil { + t.Fatal(err) + } + if !p.privateAddresses["issuer.example:443"] || p.privateAddresses["issuer.example:9443"] || p.privateAddresses["other.example:443"] { + t.Fatal("trust expanded beyond exact origin") + } + for _, endpoint := range []string{"http://issuer.example/token", "https://x:y@issuer.example/token", "https://issuer.example/token#fragment"} { + if _, err := parseEndpoint(endpoint); err == nil { + t.Fatal("invalid endpoint") + } + } +} diff --git a/services/agents-api/internal/oauthrefresh/types.go b/services/agents-api/internal/oauthrefresh/types.go new file mode 100644 index 000000000..2079a40ce --- /dev/null +++ b/services/agents-api/internal/oauthrefresh/types.go @@ -0,0 +1,25 @@ +// Package oauthrefresh exchanges a stored OAuth refresh grant for an access token. +package oauthrefresh + +import ( + "context" + "time" +) + +// Request is private execution input. Never log it or return it as metadata. +type Request struct { + TokenEndpoint, ClientID, AuthMethod, ClientSecret, RefreshToken string + Resource, Scope *string +} + +// Token contains only the new grant material returned by the token endpoint. +// An omitted RefreshToken leaves the stored refresh token unchanged. +type Token struct { + AccessToken, RefreshToken string + ExpiresAt *time.Time +} + +// Refresher is the narrow network boundary used by credential persistence. +type Refresher interface { + Refresh(context.Context, Request) (Token, error) +} diff --git a/services/agents-api/internal/store/credential_oauth_migration_test.go b/services/agents-api/internal/store/credential_oauth_migration_test.go new file mode 100644 index 000000000..87515e9ed --- /dev/null +++ b/services/agents-api/internal/store/credential_oauth_migration_test.go @@ -0,0 +1,79 @@ +package store + +import ( + "context" + "database/sql" + "os" + "testing" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/stdlib" + "github.com/pressly/goose/v3" +) + +func TestOAuthMigrationPreservesStaticGrantsAndRefusesLossyRollback(t *testing.T) { + _, pool := testStore(t) + schema := "oauth_migration_" + uuid.NewString()[:8] + quoted := pgx.Identifier{schema}.Sanitize() + if _, err := pool.Exec(t.Context(), "CREATE SCHEMA "+quoted); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if _, err := pool.Exec(context.Background(), "DROP SCHEMA "+quoted+" CASCADE"); err != nil { + t.Error(err) + } + }) + cfg := pool.Config().ConnConfig.Copy() + cfg.RuntimeParams["search_path"] = schema + db := sql.OpenDB(stdlib.GetConnector(*cfg)) + t.Cleanup(func() { _ = db.Close() }) + provider, err := goose.NewProvider(goose.DialectPostgres, db, os.DirFS("../../migrations"), goose.WithTableName("agents_api_schema_version")) + if err != nil { + t.Fatal(err) + } + if _, err := provider.UpTo(t.Context(), 53); err != nil { + t.Fatal(err) + } + vault, id := uuid.NewString(), uuid.NewString() + if _, err := db.ExecContext(t.Context(), "INSERT INTO vaults(id,tenant_id,metadata) VALUES ($1,$2,'{}')", vault, uuid.NewString()); err != nil { + t.Fatal(err) + } + if _, err := db.ExecContext(t.Context(), "INSERT INTO vault_credentials(id,vault_id,name,auth_type,mcp_server_url,token_ciphertext) VALUES ($1,$2,'Existing','static_bearer','https://mcp.example/tools',$3)", id, vault, []byte{1, 2, 3}); err != nil { + t.Fatal(err) + } + snapshot := func() string { + t.Helper() + var value string + if err := db.QueryRowContext(t.Context(), "SELECT (to_jsonb(c)-'oauth_metadata')::text FROM vault_credentials c WHERE id=$1", id).Scan(&value); err != nil { + t.Fatal(err) + } + return value + } + before := snapshot() + if _, err := provider.UpTo(t.Context(), 54); err != nil || snapshot() != before { + t.Fatal("OAuth migration changed static grant", err) + } + if _, err := provider.DownTo(t.Context(), 53); err != nil || snapshot() != before { + t.Fatal("static-only rollback failed", err) + } + if _, err := provider.UpTo(t.Context(), 54); err != nil { + t.Fatal(err) + } + oauthID := uuid.NewString() + if _, err := db.ExecContext(t.Context(), "INSERT INTO vault_credentials(id,vault_id,name,auth_type,mcp_server_url,token_ciphertext,oauth_metadata) VALUES ($1,$2,'OAuth','mcp_oauth','https://mcp.example/tools',$3,'{}')", oauthID, vault, []byte{4, 5, 6}); err != nil { + t.Fatal(err) + } + if _, err := provider.DownTo(t.Context(), 53); err == nil { + t.Fatal("rollback discarded OAuth support with existing grants") + } + var count int + if err := db.QueryRowContext(t.Context(), "SELECT count(*) FROM vault_credentials WHERE id=$1 AND auth_type='mcp_oauth' AND oauth_metadata='{}'::jsonb", oauthID).Scan(&count); err != nil || count != 1 || snapshot() != before { + t.Fatal("failed rollback altered grants", err) + } + for _, mutation := range []string{"oauth_metadata=NULL", "oauth_metadata='[]'::jsonb", "auth_type='static_bearer'"} { + if _, err := db.ExecContext(t.Context(), "UPDATE vault_credentials SET "+mutation+" WHERE id=$1", oauthID); err == nil { + t.Fatal("invalid OAuth metadata constraint admitted") + } + } +} diff --git a/services/agents-api/internal/store/credential_oauth_secret.go b/services/agents-api/internal/store/credential_oauth_secret.go new file mode 100644 index 000000000..667c9fa62 --- /dev/null +++ b/services/agents-api/internal/store/credential_oauth_secret.go @@ -0,0 +1,82 @@ +package store + +import ( + "encoding/json" + "errors" + "reflect" + "time" + + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/credentialcrypto" +) + +// The encrypted copy authenticates every public setting used for refresh, including +// the token endpoint. Substituting database metadata must never redirect a grant. +type oauthSecret struct { + Version int `json:"version"` + Metadata OAuthMetadata `json:"metadata"` + AccessToken string `json:"access_token"` + RefreshToken string `json:"refresh_token"` + ClientSecret string `json:"client_secret"` +} + +func validOAuthMetadata(metadata OAuthMetadata) bool { + if metadata.ExpiresAt != nil { + if _, err := time.Parse(time.RFC3339Nano, *metadata.ExpiresAt); err != nil { + return false + } + } + if refresh := metadata.Refresh; refresh != nil { + if refresh.ClientID == "" || refresh.TokenEndpoint == "" { + return false + } + switch refresh.TokenEndpointAuth { + case "none", "client_secret_basic", "client_secret_post": + default: + return false + } + } + return true +} + +func oauthBinding(tenantID string, credential Credential) credentialcrypto.Binding { + return credentialcrypto.Binding{TenantID: tenantID, VaultID: credential.VaultID, + CredentialID: credential.ID, AuthType: "mcp_oauth", Destination: credential.MCPServerURL} +} + +func (s *Store) sealOAuth(tenantID string, credential Credential, secret oauthSecret) ([]byte, []byte, error) { + if s.credentialCipher == nil { + return nil, nil, ErrCredentialStorageUnavailable + } + if !validOAuthMetadata(secret.Metadata) { + return nil, nil, ErrInvalidInput + } + metadata, err := json.Marshal(secret.Metadata) + if err != nil { + return nil, nil, errors.New("credential encoding failed") + } + plaintext, err := json.Marshal(secret) + if err != nil { + return nil, nil, errors.New("credential encoding failed") + } + ciphertext, err := s.credentialCipher.Seal(plaintext, oauthBinding(tenantID, credential)) + if err != nil { + return nil, nil, errors.New("credential encryption failed") + } + return metadata, ciphertext, nil +} + +func (s *Store) openOAuth(tenantID string, credential Credential, ciphertext []byte) (oauthSecret, error) { + if s.credentialCipher == nil { + return oauthSecret{}, ErrCredentialStorageUnavailable + } + plaintext, err := s.credentialCipher.Open(ciphertext, oauthBinding(tenantID, credential)) + if err != nil { + return oauthSecret{}, errors.New("OAuth credential decryption failed") + } + var secret oauthSecret + if json.Unmarshal(plaintext, &secret) != nil || secret.Version != 1 || credential.OAuth == nil || + !reflect.DeepEqual(secret.Metadata, *credential.OAuth) || !validOAuthMetadata(secret.Metadata) { + return oauthSecret{}, errors.New("OAuth credential authentication failed") + } + return secret, nil +} diff --git a/services/agents-api/internal/store/credential_oauth_types.go b/services/agents-api/internal/store/credential_oauth_types.go new file mode 100644 index 000000000..1eb674f0b --- /dev/null +++ b/services/agents-api/internal/store/credential_oauth_types.go @@ -0,0 +1,38 @@ +package store + +// OAuthMetadata is the safe projection of a stored OAuth grant. +type OAuthMetadata struct { + ExpiresAt *string `json:"expires_at"` + Refresh *OAuthRefreshMetadata `json:"refresh"` +} + +type OAuthRefreshMetadata struct { + ClientID string `json:"client_id"` + TokenEndpoint string `json:"token_endpoint"` + TokenEndpointAuth string `json:"token_endpoint_auth"` + Resource *string `json:"resource"` + Scope *string `json:"scope"` +} + +// CreateOAuthCredentialInput keeps write-only secrets separate from metadata. +type CreateOAuthCredentialInput struct { + Name, MCPServerURL, AccessToken string + OAuth OAuthMetadata + RefreshToken, ClientSecret string +} + +// ExpiresAtSet and ScopeSet preserve omitted-versus-null update semantics. +type UpdateOAuthCredentialInput struct { + AccessToken *string + ExpiresAt *string + ExpiresAtSet bool + Refresh *OAuthRefreshUpdate +} + +type OAuthRefreshUpdate struct { + RefreshToken *string + Scope *string + ScopeSet bool + TokenEndpointAuthType string + ClientSecret *string +} diff --git a/services/agents-api/internal/store/mcp_credentials.go b/services/agents-api/internal/store/mcp_credentials.go index 93e9a7762..d5e425396 100644 --- a/services/agents-api/internal/store/mcp_credentials.go +++ b/services/agents-api/internal/store/mcp_credentials.go @@ -72,7 +72,7 @@ func (s *Store) ResolveMCPCredentials(ctx context.Context, tenantID string, vaul return nil, ErrNotFound } } - rows, err := s.queries.FindMCPStaticCredentials(ctx, sqlc.FindMCPStaticCredentialsParams{ + rows, err := s.queries.FindMCPCredentials(ctx, sqlc.FindMCPCredentialsParams{ TenantID: tenant, VaultIds: vaults, McpServerUrl: request.ServerURL, CredentialID: id, }) if err != nil { @@ -111,9 +111,21 @@ func (s *Store) MCPBearerToken(ctx context.Context, tenantID string, vaultIDs [] return "", ErrNotFound } id, err := parseID(binding.CredentialID) - if err != nil || binding.AuthType != "static_bearer" || binding.ServerURL == "" { + if err != nil || (binding.AuthType != "static_bearer" && binding.AuthType != "mcp_oauth") || binding.ServerURL == "" { return "", ErrNotFound } + if binding.AuthType == "mcp_oauth" { + attached := false + for _, candidate := range vaults { + if candidate == vault { + attached = true + } + } + if !attached { + return "", ErrNotFound + } + return s.oauthBearerToken(ctx, tenantID, binding) + } ciphertext, err := s.queries.GetMCPStaticCredentialCiphertext(ctx, sqlc.GetMCPStaticCredentialCiphertextParams{ TenantID: tenant, VaultIds: vaults, VaultID: vault, CredentialID: id, McpServerUrl: binding.ServerURL, }) diff --git a/services/agents-api/internal/store/mcp_credentials_oauth.go b/services/agents-api/internal/store/mcp_credentials_oauth.go new file mode 100644 index 000000000..3d8b9a9c2 --- /dev/null +++ b/services/agents-api/internal/store/mcp_credentials_oauth.go @@ -0,0 +1,74 @@ +package store + +import ( + "context" + "errors" + "time" + + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/oauthrefresh" +) + +const oauthRefreshTimeout = 20 * time.Second + +func (s *Store) oauthBearerToken(ctx context.Context, tenantID string, binding MCPCredentialBinding) (string, error) { + // Bound both lock contention and the external exchange, so a slow provider cannot + // hold a credential indefinitely. The HTTP client supplies its own tighter bound. + ctx, cancel := context.WithTimeout(ctx, oauthRefreshTimeout) + defer cancel() + tx, credential, secret, err := s.lockOAuth(ctx, tenantID, binding.VaultID, binding.CredentialID, binding.ServerURL) + if err != nil { + return "", err + } + defer tx.Rollback(context.Background()) + if credential.MCPServerURL != binding.ServerURL { + return "", ErrNotFound + } + if secret.Metadata.ExpiresAt == nil { + return oauthAccessToken(secret.AccessToken) + } + expiry, err := time.Parse(time.RFC3339Nano, *secret.Metadata.ExpiresAt) + if err != nil { + return "", errors.New("invalid OAuth token expiry") + } + if time.Now().Before(expiry) { + return oauthAccessToken(secret.AccessToken) + } + refresh := secret.Metadata.Refresh + if refresh == nil || secret.RefreshToken == "" || s.oauthRefresher == nil { + return "", errors.New("expired OAuth credential cannot be refreshed") + } + token, err := s.oauthRefresher.Refresh(ctx, oauthrefresh.Request{ + TokenEndpoint: refresh.TokenEndpoint, ClientID: refresh.ClientID, + AuthMethod: refresh.TokenEndpointAuth, ClientSecret: secret.ClientSecret, + RefreshToken: secret.RefreshToken, Resource: refresh.Resource, Scope: refresh.Scope, + }) + if err != nil { + return "", errors.New("OAuth credential refresh failed") + } + if token.AccessToken == "" || token.ExpiresAt != nil && !time.Now().Before(*token.ExpiresAt) { + return "", errors.New("OAuth refresh returned an unusable token") + } + secret.AccessToken = token.AccessToken + if token.RefreshToken != "" { + secret.RefreshToken = token.RefreshToken + } + secret.Metadata.ExpiresAt = nil + if token.ExpiresAt != nil { + value := token.ExpiresAt.UTC().Format(time.RFC3339Nano) + secret.Metadata.ExpiresAt = &value + } + if _, err := s.saveOAuth(ctx, tx, tenantID, credential, secret); err != nil { + return "", err + } + if err := tx.Commit(ctx); err != nil { + return "", errors.New("OAuth credential refresh commit failed") + } + return token.AccessToken, nil +} + +func oauthAccessToken(token string) (string, error) { + if token == "" { + return "", errors.New("OAuth access token is missing") + } + return token, nil +} diff --git a/services/agents-api/internal/store/mcp_credentials_oauth_test.go b/services/agents-api/internal/store/mcp_credentials_oauth_test.go new file mode 100644 index 000000000..cc0c3b2b4 --- /dev/null +++ b/services/agents-api/internal/store/mcp_credentials_oauth_test.go @@ -0,0 +1,293 @@ +package store + +import ( + "context" + "errors" + "reflect" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/oauthrefresh" + "github.com/google/uuid" +) + +func TestOAuthRefreshPersistsRotatedGrantAndRequest(t *testing.T) { + for _, method := range []string{"none", "client_secret_basic", "client_secret_post"} { + t.Run(method, func(t *testing.T) { + var request oauthrefresh.Request + expiry := time.Now().Add(time.Hour).UTC() + s, pool, tenant, vault, input := oauthFixture(t, oauthRefreshFunc(func(_ context.Context, r oauthrefresh.Request) (oauthrefresh.Token, error) { + request = r + return oauthrefresh.Token{AccessToken: "renewed-access", RefreshToken: "rotated-refresh", ExpiresAt: &expiry}, nil + })) + input.OAuth.Refresh.TokenEndpointAuth = method + if method == "none" { + input.ClientSecret = "" + } + credential := createOAuthFixture(t, s, tenant, vault, input) + binding := oauthFixtureBinding(credential) + got, err := s.MCPBearerToken(t.Context(), tenant, []string{vault.ID}, binding) + if err != nil || got != "renewed-access" { + t.Fatal("refresh did not return committed access", err) + } + want := oauthrefresh.Request{TokenEndpoint: input.OAuth.Refresh.TokenEndpoint, ClientID: input.OAuth.Refresh.ClientID, AuthMethod: method, ClientSecret: input.ClientSecret, RefreshToken: input.RefreshToken, Resource: input.OAuth.Refresh.Resource, Scope: input.OAuth.Refresh.Scope} + if !reflect.DeepEqual(request, want) { + t.Fatal("refresh request lost grant fields") + } + restarted := NewWithCredentialCipherAndOAuthRefresh(pool, s.credentialCipher, nil) + after := storedOAuthSecret(t, restarted, tenant, credential) + if after.AccessToken != "renewed-access" || after.RefreshToken != "rotated-refresh" || after.Metadata.ExpiresAt == nil || *after.Metadata.ExpiresAt != expiry.Format(time.RFC3339Nano) { + t.Fatal("refreshed grant not durable") + } + got, err = restarted.MCPBearerToken(t.Context(), tenant, []string{vault.ID}, binding) + if err != nil || got != "renewed-access" { + t.Fatal("fresh grant needed another refresh after restart", err) + } + metadata, err := New(pool).GetCredential(t.Context(), tenant, vault.ID, credential.ID) + if err != nil || !reflect.DeepEqual(metadata.OAuth, &after.Metadata) { + t.Fatal("safe expiry metadata did not follow refresh", err) + } + }) + } +} + +func TestOAuthRefreshPreservesRefreshTokenWhenOmitted(t *testing.T) { + s, _, tenant, vault, input := oauthFixture(t, oauthRefreshFunc(func(context.Context, oauthrefresh.Request) (oauthrefresh.Token, error) { + return oauthrefresh.Token{AccessToken: "renewed-access"}, nil + })) + credential := createOAuthFixture(t, s, tenant, vault, input) + if _, err := s.MCPBearerToken(t.Context(), tenant, []string{vault.ID}, oauthFixtureBinding(credential)); err != nil { + t.Fatal(err) + } + secret := storedOAuthSecret(t, s, tenant, credential) + if secret.RefreshToken != input.RefreshToken || secret.Metadata.ExpiresAt != nil { + t.Fatal("omitted refresh token or unknown expiry changed incorrectly") + } +} + +func TestOAuthFreshAndUnknownExpiryDoNotRefresh(t *testing.T) { + var requests atomic.Int32 + s, _, tenant, vault, input := oauthFixture(t, oauthRefreshFunc(func(context.Context, oauthrefresh.Request) (oauthrefresh.Token, error) { + requests.Add(1) + return oauthrefresh.Token{}, errors.New("unexpected refresh") + })) + for _, expiry := range []*string{nil, oauthString(time.Now().Add(time.Hour).UTC().Format(time.RFC3339Nano))} { + input.OAuth.ExpiresAt = expiry + credential := createOAuthFixture(t, s, tenant, vault, input) + if token, err := s.MCPBearerToken(t.Context(), tenant, []string{vault.ID}, oauthFixtureBinding(credential)); err != nil || token != input.AccessToken { + t.Fatal("usable token was not returned", err) + } + } + if requests.Load() != 0 { + t.Fatal("non-expired grant refreshed") + } + input.AccessToken = "" + input.OAuth.ExpiresAt = nil + credential := createOAuthFixture(t, s, tenant, vault, input) + if token, err := s.MCPBearerToken(t.Context(), tenant, []string{vault.ID}, oauthFixtureBinding(credential)); err == nil || token != "" { + t.Fatal("empty access token admitted") + } +} + +func TestOAuthConcurrentRefreshUsesOneCommittedGrant(t *testing.T) { + var requests atomic.Int32 + expiry := time.Now().Add(time.Hour) + s, pool, tenant, vault, input := oauthFixture(t, oauthRefreshFunc(func(context.Context, oauthrefresh.Request) (oauthrefresh.Token, error) { + requests.Add(1) + return oauthrefresh.Token{AccessToken: "concurrent-access", RefreshToken: "single-use-next", ExpiresAt: &expiry}, nil + })) + credential := createOAuthFixture(t, s, tenant, vault, input) + var workers sync.WaitGroup + errorsFound := make(chan error, 12) + start := make(chan struct{}) + for i := 0; i < 12; i++ { + workers.Add(1) + go func() { + defer workers.Done() + <-start + // Separate Store instances exercise PostgreSQL serialization, not a local lock. + reader := NewWithCredentialCipherAndOAuthRefresh(pool, s.credentialCipher, s.oauthRefresher) + token, err := reader.MCPBearerToken(t.Context(), tenant, []string{vault.ID}, oauthFixtureBinding(credential)) + if err == nil && token != "concurrent-access" { + err = errors.New("concurrent lookup returned stale token") + } + errorsFound <- err + }() + } + close(start) + workers.Wait() + close(errorsFound) + for err := range errorsFound { + if err != nil { + t.Fatal(err) + } + } + if requests.Load() != 1 { + t.Fatal("one expiring grant triggered duplicate provider exchanges") + } +} + +func TestOAuthRefreshSerializesReplacementAndDeletion(t *testing.T) { + for _, mutation := range []string{"replacement", "credential-delete", "vault-delete"} { + t.Run(mutation, func(t *testing.T) { + entered := make(chan struct{}) + release := make(chan struct{}) + var releaseOnce sync.Once + unblock := func() { releaseOnce.Do(func() { close(release) }) } + defer unblock() + expiry := time.Now().Add(time.Hour) + s, pool, tenant, vault, input := oauthFixture(t, oauthRefreshFunc(func(ctx context.Context, _ oauthrefresh.Request) (oauthrefresh.Token, error) { + close(entered) + select { + case <-release: + return oauthrefresh.Token{AccessToken: "refreshed-access", RefreshToken: "rotated-refresh", ExpiresAt: &expiry}, nil + case <-ctx.Done(): + return oauthrefresh.Token{}, ctx.Err() + } + })) + credential := createOAuthFixture(t, s, tenant, vault, input) + refreshed := make(chan error, 1) + go func() { + _, err := s.MCPBearerToken(t.Context(), tenant, []string{vault.ID}, oauthFixtureBinding(credential)) + refreshed <- err + }() + select { + case <-entered: + case <-time.After(3 * time.Second): + t.Fatal("refresh never reached provider") + } + mutated := make(chan error, 1) + go func() { + var err error + switch mutation { + case "replacement": + _, err = s.UpdateOAuthCredential(t.Context(), tenant, vault.ID, credential.ID, UpdateOAuthCredentialInput{AccessToken: oauthString("manual-access"), Refresh: &OAuthRefreshUpdate{RefreshToken: oauthString("manual-refresh")}}) + case "credential-delete": + _, err = New(pool).DeleteCredential(t.Context(), tenant, vault.ID, credential.ID) + case "vault-delete": + _, err = New(pool).DeleteVault(t.Context(), tenant, vault.ID) + } + mutated <- err + }() + select { + case <-mutated: + t.Fatal("mutation bypassed pending refresh ownership") + case <-time.After(75 * time.Millisecond): + } + unblock() + for _, completed := range []chan error{refreshed, mutated} { + select { + case err := <-completed: + if err != nil { + t.Fatal("serialized operation failed", err) + } + case <-time.After(3 * time.Second): + t.Fatal("serialized operation deadlocked") + } + } + if mutation == "replacement" { + secret := storedOAuthSecret(t, s, tenant, credential) + if secret.AccessToken != "manual-access" || secret.RefreshToken != "manual-refresh" || secret.Metadata.ExpiresAt != nil { + t.Fatal("late refresh overwrote manual replacement") + } + } else { + if _, err := s.GetCredential(t.Context(), tenant, vault.ID, credential.ID); !errors.Is(err, ErrNotFound) { + t.Fatal("deleted credential resurrected") + } + if token, err := s.MCPBearerToken(t.Context(), tenant, []string{vault.ID}, oauthFixtureBinding(credential)); !errors.Is(err, ErrNotFound) || token != "" { + t.Fatal("deleted grant remained usable") + } + } + }) + } +} + +func TestOAuthCancelledRefreshRollsBackAndAllowsReplacement(t *testing.T) { + entered := make(chan struct{}) + s, _, tenant, vault, input := oauthFixture(t, oauthRefreshFunc(func(ctx context.Context, _ oauthrefresh.Request) (oauthrefresh.Token, error) { + close(entered) + <-ctx.Done() + return oauthrefresh.Token{}, ctx.Err() + })) + credential := createOAuthFixture(t, s, tenant, vault, input) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + done := make(chan error, 1) + go func() { + _, err := s.MCPBearerToken(ctx, tenant, []string{vault.ID}, oauthFixtureBinding(credential)) + done <- err + }() + select { + case <-entered: + case <-time.After(3 * time.Second): + t.Fatal("refresh not started") + } + cancel() + select { + case err := <-done: + if err == nil { + t.Fatal("cancelled refresh succeeded") + } + case <-time.After(3 * time.Second): + t.Fatal("cancel did not release refresh") + } + if _, err := s.UpdateOAuthCredential(t.Context(), tenant, vault.ID, credential.ID, UpdateOAuthCredentialInput{AccessToken: oauthString("manual-after-cancel")}); err != nil { + t.Fatal("cancelled refresh retained ownership", err) + } + if token, err := s.MCPBearerToken(t.Context(), tenant, []string{vault.ID}, oauthFixtureBinding(credential)); err != nil || token != "manual-after-cancel" { + t.Fatal("replacement after cancellation unusable") + } +} + +func TestOAuthDeletionCannotReselectAnotherCredential(t *testing.T) { + s, _, tenant, vault, input := oauthFixture(t, nil) + input.OAuth.ExpiresAt = nil + credential := createOAuthFixture(t, s, tenant, vault, input) + bindings, err := s.ResolveMCPCredentials(t.Context(), tenant, []string{vault.ID}, []MCPCredentialRequest{{ServerLabel: "test", ServerURL: input.MCPServerURL}}) + if err != nil { + t.Fatal(err) + } + if _, err := s.DeleteCredential(t.Context(), tenant, vault.ID, credential.ID); err != nil { + t.Fatal(err) + } + input.AccessToken = uuid.NewString() + createOAuthFixture(t, s, tenant, vault, input) + if token, err := s.MCPBearerToken(t.Context(), tenant, []string{vault.ID}, bindings[0]); !errors.Is(err, ErrNotFound) || token != "" { + t.Fatal("frozen identity fell back after deletion") + } +} + +func TestOAuthRefreshCommitFailureDoesNotReturnUncommittedToken(t *testing.T) { + expiry := time.Now().Add(time.Hour) + s, pool, tenant, vault, input := oauthFixture(t, oauthRefreshFunc(func(context.Context, oauthrefresh.Request) (oauthrefresh.Token, error) { + return oauthrefresh.Token{AccessToken: "uncommitted-access", RefreshToken: "uncommitted-refresh", ExpiresAt: &expiry}, nil + })) + credential := createOAuthFixture(t, s, tenant, vault, input) + before := storedOAuthSecret(t, s, tenant, credential) + suffix := strings.ReplaceAll(uuid.NewString(), "-", "") + function, trigger := "oauth_commit_fail_"+suffix, "oauth_commit_fail_"+suffix + // A deferred trigger exercises failure after the UPDATE has returned metadata. + // The condition confines the synthetic failure to this test's unique grant. + if _, err := pool.Exec(t.Context(), "CREATE FUNCTION "+function+"() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN RAISE EXCEPTION 'private-refresh-canary'; END $$"); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + _, err := pool.Exec(context.Background(), "DROP FUNCTION "+function+"() CASCADE") + if err != nil { + t.Error(err) + } + }) + if _, err := pool.Exec(t.Context(), "CREATE CONSTRAINT TRIGGER "+trigger+" AFTER UPDATE ON vault_credentials DEFERRABLE INITIALLY DEFERRED FOR EACH ROW WHEN (NEW.id='"+credential.ID+"'::uuid) EXECUTE FUNCTION "+function+"()"); err != nil { + t.Fatal(err) + } + token, err := s.MCPBearerToken(t.Context(), tenant, []string{vault.ID}, oauthFixtureBinding(credential)) + if err == nil || token != "" || strings.Contains(err.Error(), "private-refresh-canary") { + t.Fatal("commit failure returned an uncommitted grant or unsafe error") + } + if after := storedOAuthSecret(t, s, tenant, credential); !reflect.DeepEqual(before, after) { + t.Fatal("failed commit modified the grant") + } +} diff --git a/services/agents-api/internal/store/sessions.go b/services/agents-api/internal/store/sessions.go index 0cf31049e..19882d41c 100644 --- a/services/agents-api/internal/store/sessions.go +++ b/services/agents-api/internal/store/sessions.go @@ -21,6 +21,7 @@ import ( "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/credentialcrypto" "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/db/sqlc" "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/identity" + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/oauthrefresh" ) var ( @@ -71,6 +72,7 @@ type Store struct { pool *pgxpool.Pool executionLease *ExecutionLease credentialCipher *credentialcrypto.Cipher + oauthRefresher oauthrefresh.Refresher } func New(pool *pgxpool.Pool) *Store { return &Store{queries: sqlc.New(pool), pool: pool} } diff --git a/services/agents-api/internal/store/vault_credentials.go b/services/agents-api/internal/store/vault_credentials.go index 6b46ce024..05793e51a 100644 --- a/services/agents-api/internal/store/vault_credentials.go +++ b/services/agents-api/internal/store/vault_credentials.go @@ -2,12 +2,14 @@ package store import ( "context" + "encoding/json" "errors" "fmt" "time" "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/credentialcrypto" "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/db/sqlc" + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/oauthrefresh" "github.com/google/uuid" "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgtype" @@ -21,6 +23,7 @@ var ErrCredentialStorageUnavailable = errors.New("credential encryption is not c type Credential struct { ID, VaultID, Name, AuthType, MCPServerURL string CreatedAt, UpdatedAt time.Time + OAuth *OAuthMetadata } type CreateStaticCredentialInput struct { @@ -30,9 +33,8 @@ type CreateStaticCredentialInput struct { // NewWithCredentialCipher configures immutable credential encryption before the // Store is published. A nil cipher leaves non-secret resource operations available. func NewWithCredentialCipher(pool *pgxpool.Pool, cipher *credentialcrypto.Cipher) *Store { - s := New(pool) - s.credentialCipher = cipher - return s + refresher, _ := oauthrefresh.NewClient(nil) + return NewWithCredentialCipherAndOAuthRefresh(pool, cipher, refresher) } func (s *Store) CreateStaticCredential(ctx context.Context, tenantID, vaultID string, input CreateStaticCredentialInput) (Credential, error) { @@ -66,7 +68,7 @@ func (s *Store) CreateStaticCredential(ctx context.Context, tenantID, vaultID st if err != nil { return Credential{}, fmt.Errorf("create credential: %w", err) } - return credentialFromRow(sqlc.GetCredentialRow(row)), nil + return credentialFromRow(sqlc.GetCredentialRow(row)) } func (s *Store) GetCredential(ctx context.Context, tenantID, vaultID, credentialID string) (Credential, error) { @@ -89,13 +91,27 @@ func (s *Store) GetCredential(ctx context.Context, tenantID, vaultID, credential if err != nil { return Credential{}, fmt.Errorf("get credential: %w", err) } - return credentialFromRow(row), nil + return credentialFromRow(row) } -func credentialFromRow(row sqlc.GetCredentialRow) Credential { - return Credential{ +func credentialFromRow(row sqlc.GetCredentialRow) (Credential, error) { + result := Credential{ ID: uuid.UUID(row.ID.Bytes).String(), VaultID: uuid.UUID(row.VaultID.Bytes).String(), Name: row.Name, AuthType: row.AuthType, MCPServerURL: row.McpServerUrl, CreatedAt: row.CreatedAt.Time, UpdatedAt: row.UpdatedAt.Time, } + if row.AuthType == "mcp_oauth" { + result.OAuth = &OAuthMetadata{} + if err := json.Unmarshal(row.OauthMetadata, result.OAuth); err != nil { + return Credential{}, errors.New("invalid stored OAuth metadata") + } + } + return result, nil +} + +// NewWithCredentialCipherAndOAuthRefresh configures the execution-only refresh boundary. +func NewWithCredentialCipherAndOAuthRefresh(pool *pgxpool.Pool, cipher *credentialcrypto.Cipher, refresher oauthrefresh.Refresher) *Store { + s := New(pool) + s.credentialCipher, s.oauthRefresher = cipher, refresher + return s } diff --git a/services/agents-api/internal/store/vault_credentials_list.go b/services/agents-api/internal/store/vault_credentials_list.go index 07b79322e..d63439fff 100644 --- a/services/agents-api/internal/store/vault_credentials_list.go +++ b/services/agents-api/internal/store/vault_credentials_list.go @@ -52,7 +52,11 @@ func (s *Store) ListCredentials(ctx context.Context, tenantID, vaultID, cursor s rows = rows[:limit] } for _, row := range rows { - page.Credentials = append(page.Credentials, credentialFromRow(sqlc.GetCredentialRow(row))) + credential, err := credentialFromRow(sqlc.GetCredentialRow(row)) + if err != nil { + return CredentialPage{}, err + } + page.Credentials = append(page.Credentials, credential) } return page, nil } diff --git a/services/agents-api/internal/store/vault_credentials_oauth.go b/services/agents-api/internal/store/vault_credentials_oauth.go new file mode 100644 index 000000000..fa1f9149f --- /dev/null +++ b/services/agents-api/internal/store/vault_credentials_oauth.go @@ -0,0 +1,162 @@ +package store + +import ( + "context" + "errors" + + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/db/sqlc" + "github.com/google/uuid" + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" +) + +func (s *Store) CreateOAuthCredential(ctx context.Context, tenantID, vaultID string, input CreateOAuthCredentialInput) (Credential, error) { + tenant, err := parseID(tenantID) + if err != nil { + return Credential{}, ErrNotFound + } + vault, err := parseID(vaultID) + if err != nil { + return Credential{}, ErrNotFound + } + if !validVaultName(input.Name) || input.MCPServerURL == "" || !validOAuthMetadata(input.OAuth) { + return Credential{}, ErrInvalidInput + } + if input.OAuth.Refresh == nil && (input.RefreshToken != "" || input.ClientSecret != "") || + input.OAuth.Refresh != nil && input.OAuth.Refresh.TokenEndpointAuth == "none" && input.ClientSecret != "" { + return Credential{}, ErrInvalidInput + } + credential := Credential{ID: uuid.NewString(), VaultID: uuid.UUID(vault.Bytes).String(), + Name: input.Name, AuthType: "mcp_oauth", MCPServerURL: input.MCPServerURL} + secret := oauthSecret{Version: 1, Metadata: input.OAuth, AccessToken: input.AccessToken, + RefreshToken: input.RefreshToken, ClientSecret: input.ClientSecret} + metadata, ciphertext, err := s.sealOAuth(uuid.UUID(tenant.Bytes).String(), credential, secret) + if err != nil { + return Credential{}, err + } + row, err := s.queries.CreateOAuthCredential(ctx, sqlc.CreateOAuthCredentialParams{ + ID: pgtype.UUID{Bytes: uuid.MustParse(credential.ID), Valid: true}, TenantID: tenant, VaultID: vault, + Name: input.Name, McpServerUrl: input.MCPServerURL, OauthMetadata: metadata, TokenCiphertext: ciphertext, + }) + if errors.Is(err, pgx.ErrNoRows) { + return Credential{}, ErrNotFound + } + if err != nil { + return Credential{}, errors.New("credential creation failed") + } + return credentialFromRow(sqlc.GetCredentialRow(row)) +} + +func (s *Store) UpdateOAuthCredential(ctx context.Context, tenantID, vaultID, credentialID string, input UpdateOAuthCredentialInput) (Credential, error) { + current, err := s.GetCredential(ctx, tenantID, vaultID, credentialID) + if err != nil { + return Credential{}, err + } + if current.AuthType != "mcp_oauth" { + return Credential{}, ErrInvalidInput + } + tx, credential, secret, err := s.lockOAuth(ctx, tenantID, vaultID, credentialID, "") + if err != nil { + return Credential{}, err + } + defer tx.Rollback(context.Background()) + if input.AccessToken != nil { + secret.AccessToken = *input.AccessToken + secret.Metadata.ExpiresAt = nil + } + if input.ExpiresAtSet { + secret.Metadata.ExpiresAt = input.ExpiresAt + } + if update := input.Refresh; update != nil { + refresh := secret.Metadata.Refresh + if refresh == nil { + return Credential{}, ErrInvalidInput + } + if update.TokenEndpointAuthType != "" && update.TokenEndpointAuthType != refresh.TokenEndpointAuth { + return Credential{}, ErrInvalidInput + } + if update.ClientSecret != nil { + if refresh.TokenEndpointAuth == "none" { + return Credential{}, ErrInvalidInput + } + secret.ClientSecret = *update.ClientSecret + } + if update.RefreshToken != nil { + secret.RefreshToken = *update.RefreshToken + } + if update.ScopeSet { + refresh.Scope = update.Scope + } + } + updated, err := s.saveOAuth(ctx, tx, tenantID, credential, secret) + if err != nil { + return Credential{}, err + } + if err := tx.Commit(ctx); err != nil { + return Credential{}, errors.New("credential update failed") + } + return updated, nil +} + +// Row ownership lasts through refresh or manual replacement. PostgreSQL serializes +// competing updates and deletes, including a parent Vault's cascading deletion. +func (s *Store) lockOAuth(ctx context.Context, tenantID, vaultID, credentialID, destination string) (pgx.Tx, Credential, oauthSecret, error) { + tenant, e1 := parseID(tenantID) + vault, e2 := parseID(vaultID) + id, e3 := parseID(credentialID) + if e1 != nil || e2 != nil || e3 != nil { + return nil, Credential{}, oauthSecret{}, ErrNotFound + } + tx, err := s.pool.Begin(ctx) + if err != nil { + return nil, Credential{}, oauthSecret{}, errors.New("credential transaction failed") + } + fail := func(err error) (pgx.Tx, Credential, oauthSecret, error) { + _ = tx.Rollback(context.Background()) + return nil, Credential{}, oauthSecret{}, err + } + row, err := sqlc.New(tx).GetOAuthCredentialForUpdate(ctx, sqlc.GetOAuthCredentialForUpdateParams{ + TenantID: tenant, VaultID: vault, ID: id, + }) + if errors.Is(err, pgx.ErrNoRows) { + return fail(ErrNotFound) + } + if err != nil { + return fail(errors.New("credential lookup failed")) + } + credential, err := credentialFromRow(sqlc.GetCredentialRow{ID: row.ID, VaultID: row.VaultID, + Name: row.Name, AuthType: row.AuthType, McpServerUrl: row.McpServerUrl, + CreatedAt: row.CreatedAt, UpdatedAt: row.UpdatedAt, OauthMetadata: row.OauthMetadata}) + if err != nil { + return fail(err) + } + if destination != "" && credential.MCPServerURL != destination { + return fail(ErrNotFound) + } + secret, err := s.openOAuth(uuid.UUID(tenant.Bytes).String(), credential, row.TokenCiphertext) + if err != nil { + return fail(err) + } + return tx, credential, secret, nil +} + +func (s *Store) saveOAuth(ctx context.Context, tx pgx.Tx, tenantID string, credential Credential, secret oauthSecret) (Credential, error) { + tenant, _ := parseID(tenantID) + metadata, ciphertext, err := s.sealOAuth(uuid.UUID(tenant.Bytes).String(), credential, secret) + if err != nil { + return Credential{}, err + } + vault, _ := parseID(credential.VaultID) + id, _ := parseID(credential.ID) + row, err := sqlc.New(tx).UpdateOAuthCredential(ctx, sqlc.UpdateOAuthCredentialParams{ + TenantID: tenant, VaultID: vault, ID: id, McpServerUrl: credential.MCPServerURL, + OauthMetadata: metadata, TokenCiphertext: ciphertext, + }) + if errors.Is(err, pgx.ErrNoRows) { + return Credential{}, ErrNotFound + } + if err != nil { + return Credential{}, errors.New("credential update failed") + } + return credentialFromRow(sqlc.GetCredentialRow(row)) +} diff --git a/services/agents-api/internal/store/vault_credentials_oauth_test.go b/services/agents-api/internal/store/vault_credentials_oauth_test.go new file mode 100644 index 000000000..7c2d2b172 --- /dev/null +++ b/services/agents-api/internal/store/vault_credentials_oauth_test.go @@ -0,0 +1,278 @@ +package store + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "reflect" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/credentialcrypto" + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/oauthrefresh" + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgxpool" +) + +type oauthRefreshFunc func(context.Context, oauthrefresh.Request) (oauthrefresh.Token, error) + +func (f oauthRefreshFunc) Refresh(ctx context.Context, request oauthrefresh.Request) (oauthrefresh.Token, error) { + return f(ctx, request) +} +func oauthString(value string) *string { return &value } + +func oauthFixture(t *testing.T, refresher oauthrefresh.Refresher) (*Store, *pgxpool.Pool, string, Vault, CreateOAuthCredentialInput) { + t.Helper() + _, pool := testStore(t) + cipher, err := credentialcrypto.New(bytes.Repeat([]byte{17}, 32)) + if err != nil { + t.Fatal(err) + } + s := NewWithCredentialCipherAndOAuthRefresh(pool, cipher, refresher) + tenant := uuid.NewString() + vault, err := s.CreateVault(t.Context(), tenant, CreateVaultInput{}) + if err != nil { + t.Fatal(err) + } + input := CreateOAuthCredentialInput{Name: "OAuth fixture", MCPServerURL: "https://mcp.example/tools", + AccessToken: "private-access-canary", RefreshToken: "private-refresh-canary", ClientSecret: "private-client-canary", + OAuth: OAuthMetadata{ExpiresAt: oauthString(time.Now().Add(-time.Hour).UTC().Format(time.RFC3339Nano)), + Refresh: &OAuthRefreshMetadata{ClientID: "test-client", TokenEndpoint: "https://issuer.example/token", + TokenEndpointAuth: "client_secret_basic", Resource: oauthString("https://mcp.example/tools"), Scope: oauthString("read write")}}} + return s, pool, tenant, vault, input +} + +func createOAuthFixture(t *testing.T, s *Store, tenant string, vault Vault, input CreateOAuthCredentialInput) Credential { + t.Helper() + credential, err := s.CreateOAuthCredential(t.Context(), tenant, vault.ID, input) + if err != nil { + t.Fatal("create OAuth fixture", err) + } + return credential +} + +func oauthFixtureBinding(credential Credential) MCPCredentialBinding { + return MCPCredentialBinding{ServerLabel: "test", ServerURL: credential.MCPServerURL, + VaultID: credential.VaultID, CredentialID: credential.ID, AuthType: credential.AuthType} +} + +func storedOAuthSecret(t *testing.T, s *Store, tenant string, credential Credential) oauthSecret { + t.Helper() + tx, _, secret, err := s.lockOAuth(t.Context(), tenant, credential.VaultID, credential.ID, "") + if err != nil { + t.Fatal("read private test grant", err) + } + if err := tx.Rollback(t.Context()); err != nil { + t.Fatal(err) + } + return secret +} + +func TestOAuthCredentialMetadataEncryptionAndScope(t *testing.T) { + var exchanges atomic.Int32 + s, pool, tenant, vault, input := oauthFixture(t, oauthRefreshFunc(func(context.Context, oauthrefresh.Request) (oauthrefresh.Token, error) { + exchanges.Add(1) + return oauthrefresh.Token{}, errors.New("unexpected exchange") + })) + credential := createOAuthFixture(t, s, tenant, vault, input) + for _, reader := range []*Store{New(pool), s} { + got, err := reader.GetCredential(t.Context(), tenant, vault.ID, credential.ID) + if err != nil || !reflect.DeepEqual(got, credential) { + t.Fatal("keyless safe metadata changed", err) + } + page, err := reader.ListCredentials(t.Context(), tenant, vault.ID, "", 20, true, nil) + if err != nil || len(page.Credentials) != 1 || !reflect.DeepEqual(page.Credentials[0], credential) { + t.Fatal("keyless listing failed", err) + } + encoded, _ := json.Marshal(page) + for _, secret := range []string{input.AccessToken, input.RefreshToken, input.ClientSecret} { + if bytes.Contains(encoded, []byte(secret)) { + t.Fatal("metadata disclosed a secret") + } + } + } + var ciphertext []byte + if err := pool.QueryRow(t.Context(), "SELECT token_ciphertext FROM vault_credentials WHERE id=$1", credential.ID).Scan(&ciphertext); err != nil { + t.Fatal(err) + } + for _, secret := range []string{input.AccessToken, input.RefreshToken, input.ClientSecret} { + if bytes.Contains(ciphertext, []byte(secret)) { + t.Fatal("plaintext grant persisted") + } + } + restored := NewWithCredentialCipherAndOAuthRefresh(pool, s.credentialCipher, s.oauthRefresher) + if secret := storedOAuthSecret(t, restored, tenant, credential); secret.AccessToken != input.AccessToken || secret.RefreshToken != input.RefreshToken || secret.ClientSecret != input.ClientSecret { + t.Fatal("restart lost grant material") + } + binding := oauthFixtureBinding(credential) + for _, target := range []struct { + tenant string + vaults []string + binding MCPCredentialBinding + }{ + {uuid.NewString(), []string{vault.ID}, binding}, {tenant, nil, binding}, + {tenant, []string{vault.ID}, MCPCredentialBinding{ServerLabel: "test", ServerURL: input.MCPServerURL + "/other", VaultID: vault.ID, CredentialID: credential.ID, AuthType: "mcp_oauth"}}, + } { + if token, err := s.MCPBearerToken(t.Context(), target.tenant, target.vaults, target.binding); !errors.Is(err, ErrNotFound) || token != "" { + t.Fatal("foreign or mismatched scope admitted", err) + } + } + if _, err := s.UpdateOAuthCredential(t.Context(), uuid.NewString(), vault.ID, credential.ID, UpdateOAuthCredentialInput{AccessToken: oauthString("replacement")}); !errors.Is(err, ErrNotFound) { + t.Fatal("foreign update admitted") + } + if _, err := s.CreateOAuthCredential(t.Context(), uuid.NewString(), vault.ID, input); !errors.Is(err, ErrNotFound) { + t.Fatal("foreign creation admitted") + } + if _, err := New(pool).CreateOAuthCredential(t.Context(), tenant, vault.ID, input); !errors.Is(err, ErrCredentialStorageUnavailable) { + t.Fatal("keyless creation admitted") + } + if token, err := New(pool).MCPBearerToken(t.Context(), tenant, []string{vault.ID}, binding); !errors.Is(err, ErrCredentialStorageUnavailable) || token != "" { + t.Fatal("keyless execution admitted") + } + wrongCipher, _ := credentialcrypto.New(bytes.Repeat([]byte{18}, 32)) + wrong := NewWithCredentialCipherAndOAuthRefresh(pool, wrongCipher, s.oauthRefresher) + if token, err := wrong.MCPBearerToken(t.Context(), tenant, []string{vault.ID}, binding); err == nil || token != "" { + t.Fatal("wrong key executed") + } + if exchanges.Load() != 0 { + t.Fatal("resource operations or invalid scopes contacted provider") + } +} + +func TestOAuthCredentialUpdatesPreservePinnedSemantics(t *testing.T) { + s, _, tenant, vault, input := oauthFixture(t, nil) + credential := createOAuthFixture(t, s, tenant, vault, input) + update := func(patch UpdateOAuthCredentialInput) Credential { + t.Helper() + got, err := s.UpdateOAuthCredential(t.Context(), tenant, vault.ID, credential.ID, patch) + if err != nil { + t.Fatal(err) + } + if got.ID != credential.ID || got.Name != credential.Name || got.AuthType != credential.AuthType || got.MCPServerURL != credential.MCPServerURL || !got.CreatedAt.Equal(credential.CreatedAt) { + t.Fatal("immutable metadata changed") + } + return got + } + unchanged := update(UpdateOAuthCredentialInput{}) + if !reflect.DeepEqual(unchanged.OAuth, credential.OAuth) { + t.Fatal("omitted values changed") + } + replacement := update(UpdateOAuthCredentialInput{AccessToken: oauthString("new-access")}) + if replacement.OAuth.ExpiresAt != nil { + t.Fatal("new access token retained old expiry") + } + expiry := time.Now().Add(time.Hour).UTC().Format(time.RFC3339Nano) + replacement = update(UpdateOAuthCredentialInput{ExpiresAtSet: true, ExpiresAt: &expiry, Refresh: &OAuthRefreshUpdate{ScopeSet: true, Scope: nil}}) + if replacement.OAuth.ExpiresAt == nil || *replacement.OAuth.ExpiresAt != expiry || replacement.OAuth.Refresh.Scope != nil { + t.Fatal("expiry or null scope semantics failed") + } + secret := storedOAuthSecret(t, s, tenant, replacement) + if secret.AccessToken != "new-access" || secret.RefreshToken != input.RefreshToken || secret.ClientSecret != input.ClientSecret { + t.Fatal("omitted secrets were replaced") + } + replacement = update(UpdateOAuthCredentialInput{ExpiresAtSet: true, Refresh: &OAuthRefreshUpdate{RefreshToken: oauthString("new-refresh"), TokenEndpointAuthType: "client_secret_basic", ClientSecret: oauthString("new-secret"), ScopeSet: true, Scope: oauthString("read")}}) + secret = storedOAuthSecret(t, s, tenant, replacement) + if secret.Metadata.ExpiresAt != nil || secret.RefreshToken != "new-refresh" || secret.ClientSecret != "new-secret" || *secret.Metadata.Refresh.Scope != "read" { + t.Fatal("replacement fields not persisted") + } + for _, patch := range []UpdateOAuthCredentialInput{ + {ExpiresAtSet: true, ExpiresAt: oauthString("not-a-date")}, + {Refresh: &OAuthRefreshUpdate{TokenEndpointAuthType: "client_secret_post"}}, + } { + if _, err := s.UpdateOAuthCredential(t.Context(), tenant, vault.ID, credential.ID, patch); !errors.Is(err, ErrInvalidInput) { + t.Fatal("invalid mutation admitted") + } + } + if got := storedOAuthSecret(t, s, tenant, replacement); !reflect.DeepEqual(got, secret) { + t.Fatal("rejected update changed the grant") + } + input.OAuth.Refresh = nil + input.RefreshToken = "" + input.ClientSecret = "" + noRefresh := createOAuthFixture(t, s, tenant, vault, input) + if _, err := s.UpdateOAuthCredential(t.Context(), tenant, vault.ID, noRefresh.ID, UpdateOAuthCredentialInput{Refresh: &OAuthRefreshUpdate{RefreshToken: oauthString("cannot-add")}}); !errors.Is(err, ErrInvalidInput) { + t.Fatal("added missing refresh configuration") + } +} + +func TestOAuthMetadataTamperingNeverReachesProvider(t *testing.T) { + var exchanges atomic.Int32 + s, pool, tenant, vault, input := oauthFixture(t, oauthRefreshFunc(func(context.Context, oauthrefresh.Request) (oauthrefresh.Token, error) { + exchanges.Add(1) + return oauthrefresh.Token{}, errors.New("unexpected refresh") + })) + for _, mutation := range []string{ + `jsonb_set(oauth_metadata,'{refresh,token_endpoint}','"https://attacker.example/token"')`, + `jsonb_set(oauth_metadata,'{refresh,client_id}','"other-client"')`, + `jsonb_set(oauth_metadata,'{refresh,scope}','"all"')`, + `jsonb_set(oauth_metadata,'{expires_at}','null')`, + } { + credential := createOAuthFixture(t, s, tenant, vault, input) + if _, err := pool.Exec(t.Context(), "UPDATE vault_credentials SET oauth_metadata="+mutation+" WHERE id=$1", credential.ID); err != nil { + t.Fatal(err) + } + if token, err := s.MCPBearerToken(t.Context(), tenant, []string{vault.ID}, oauthFixtureBinding(credential)); err == nil || token != "" { + t.Fatal("metadata substitution executed") + } + if _, err := s.UpdateOAuthCredential(t.Context(), tenant, vault.ID, credential.ID, UpdateOAuthCredentialInput{AccessToken: oauthString("new")}); err == nil { + t.Fatal("update authenticated substituted metadata") + } + } + if exchanges.Load() != 0 { + t.Fatal("tampered metadata reached token endpoint") + } +} + +func TestOAuthCredentialSelectionIncludesBothAuthTypes(t *testing.T) { + s, _, tenant, vault, input := oauthFixture(t, nil) + credential := createOAuthFixture(t, s, tenant, vault, input) + requests := []MCPCredentialRequest{{ServerLabel: "test", ServerURL: input.MCPServerURL}} + bindings, err := s.ResolveMCPCredentials(t.Context(), tenant, []string{vault.ID}, requests) + if err != nil || len(bindings) != 1 || bindings[0].AuthType != "mcp_oauth" || bindings[0].CredentialID != credential.ID { + t.Fatal("OAuth was not selected", err) + } + if _, err := s.CreateStaticCredential(t.Context(), tenant, vault.ID, CreateStaticCredentialInput{Name: "Static", MCPServerURL: input.MCPServerURL, Token: "static"}); err != nil { + t.Fatal(err) + } + if _, err := s.ResolveMCPCredentials(t.Context(), tenant, []string{vault.ID}, requests); !errors.Is(err, ErrInvalidInput) { + t.Fatal("ambiguous mixed credentials selected") + } + requests[0].CredentialID = &credential.ID + if selected, err := s.ResolveMCPCredentials(t.Context(), tenant, []string{vault.ID}, requests); err != nil || selected[0] != bindings[0] { + t.Fatal("explicit OAuth identity changed") + } +} + +func TestOAuthRefreshErrorsAreSafeAndPreserveGrant(t *testing.T) { + for _, mode := range []string{"provider_error", "empty_access", "expired_response", "no_refresh"} { + t.Run(mode, func(t *testing.T) { + s, _, tenant, vault, input := oauthFixture(t, oauthRefreshFunc(func(context.Context, oauthrefresh.Request) (oauthrefresh.Token, error) { + switch mode { + case "provider_error": + return oauthrefresh.Token{}, errors.New("private-refresh-canary: provider body") + case "expired_response": + past := time.Now().Add(-time.Second) + return oauthrefresh.Token{AccessToken: "new", ExpiresAt: &past}, nil + } + return oauthrefresh.Token{}, nil + })) + if mode == "no_refresh" { + input.OAuth.Refresh = nil + input.RefreshToken = "" + input.ClientSecret = "" + } + credential := createOAuthFixture(t, s, tenant, vault, input) + before := storedOAuthSecret(t, s, tenant, credential) + token, err := s.MCPBearerToken(t.Context(), tenant, []string{vault.ID}, oauthFixtureBinding(credential)) + if err == nil || token != "" || strings.Contains(err.Error(), "private-refresh-canary") { + t.Fatal("refresh failed unsafely") + } + if after := storedOAuthSecret(t, s, tenant, credential); !reflect.DeepEqual(before, after) { + t.Fatal("failed refresh changed grant") + } + }) + } +} diff --git a/services/agents-api/internal/store/vault_credentials_update.go b/services/agents-api/internal/store/vault_credentials_update.go index a85d75126..731cd75d2 100644 --- a/services/agents-api/internal/store/vault_credentials_update.go +++ b/services/agents-api/internal/store/vault_credentials_update.go @@ -56,5 +56,5 @@ func (s *Store) UpdateStaticCredential(ctx context.Context, tenantID, vaultID, c if err != nil { return Credential{}, errors.New("credential update failed") } - return credentialFromRow(sqlc.GetCredentialRow(row)), nil + return credentialFromRow(sqlc.GetCredentialRow(row)) } diff --git a/services/agents-api/migrations/000054_oauth_credentials.sql b/services/agents-api/migrations/000054_oauth_credentials.sql new file mode 100644 index 000000000..37dd72330 --- /dev/null +++ b/services/agents-api/migrations/000054_oauth_credentials.sql @@ -0,0 +1,17 @@ +-- +goose Up +ALTER TABLE vault_credentials DROP CONSTRAINT vault_credentials_auth_type_check; +ALTER TABLE vault_credentials ADD COLUMN oauth_metadata jsonb; +ALTER TABLE vault_credentials ADD CONSTRAINT vault_credentials_auth_type_check + CHECK (auth_type IN ('static_bearer', 'mcp_oauth')); +ALTER TABLE vault_credentials ADD CONSTRAINT vault_credentials_oauth_metadata_check + CHECK ((auth_type = 'static_bearer' AND oauth_metadata IS NULL) + OR (auth_type = 'mcp_oauth' AND oauth_metadata IS NOT NULL + AND jsonb_typeof(oauth_metadata) = 'object')); + +-- +goose Down +-- Refuse to discard grants when rolling back to static-only storage. +ALTER TABLE vault_credentials DROP CONSTRAINT vault_credentials_auth_type_check; +ALTER TABLE vault_credentials ADD CONSTRAINT vault_credentials_auth_type_check + CHECK (auth_type = 'static_bearer'); +ALTER TABLE vault_credentials DROP CONSTRAINT vault_credentials_oauth_metadata_check; +ALTER TABLE vault_credentials DROP COLUMN oauth_metadata; diff --git a/services/agents-api/oauth-credentials.md b/services/agents-api/oauth-credentials.md new file mode 100644 index 000000000..755540ac9 --- /dev/null +++ b/services/agents-api/oauth-credentials.md @@ -0,0 +1,165 @@ +# OAuth MCP credentials + +Core implements the pinned `mcp_oauth` Credential variant alongside +`static_bearer`. The application obtains authorization and consent from its OAuth +provider, then stores the resulting grant through the ordinary Vault API. Core +has no authorization redirect, callback, provider-revocation or public refresh +endpoint. These responsibilities follow the [official Vault guide](https://developers.openai.com/api/docs/guides/agents-api/tools/vaults). +The protocol pin in `contracts/agents-api/upstream.json` remains authoritative. + +## Store and use an authorized grant + +```python +credential = client.beta.agents.vaults.credentials.create( + vault.id, + name="Authorized MCP account", + auth={ + "type": "mcp_oauth", + "mcp_server_url": mcp_url, + "access_token": grant.access_token, + "expires_at": grant.expires_at, + "refresh": { + "client_id": oauth_client_id, + "refresh_token": grant.refresh_token, + "token_endpoint": issuer_token_endpoint, + "token_endpoint_auth": { + "type": "client_secret_basic", + "client_secret": oauth_client_secret, + }, + }, + }, +) +``` + +`token_endpoint_auth` accepts `none`, `client_secret_basic`, or +`client_secret_post`. The latter two require a write-only client secret. +`refresh.resource` and `refresh.scope` are optional nullable strings. Expiry is an +optional nullable RFC 3339 timestamp. Without refresh configuration Core can use a +known-valid or unknown-expiry access token, but rejects an expired token. + +Attach the Vault and reference the exact HTTPS MCP destination as described in +[credential storage](credentials.md#use-a-credential-in-a-session). Static and +OAuth credentials share tenant checks, frozen selection and the existing bearer +Runtime contract. A unique implicit selection searches both authentication types; +ambiguity fails rather than preferring either type. Only existing qualified MCP +harness/placement profiles are supported. OAuth is not a new harness capability: +adapters receive the access token, never refresh tokens or client secrets. + +Resource reads and lists select safe metadata without decryption. OAuth metadata +contains expiry and refresh client ID, endpoint, authentication method, resource +and scope. Access tokens, refresh tokens and client secrets never appear there. +Creation and manual replacement do not contact the OAuth provider or MCP server. + +## Refresh and replace + +At execution dispatch, Core rechecks the selected tenant, attached Vault, +Credential identity, auth type and exact MCP URL. A known-expired grant is refreshed +before native execution. Core uses the existing OAuth library with the declared +endpoint authentication method and stored scope/resource. Refresh is a single +bounded exchange, without authentication-method probing, redirects or automatic +401 retries. Provider error bodies are not returned or logged. + +A PostgreSQL row lock serializes refresh and manual replacement with Credential +and Vault deletion. The complete grant is authenticated before use. Refreshed +access token, expiry and an optional replacement refresh token are committed +atomically before an access token is returned. An omitted refresh token retains +the previous value. A failed exchange or commit never returns the new token or +falls back to anonymous access. A provider may rotate its grant before a local +commit fails; that uncertain outcome can require application reauthorization. +Core does not retry an uncertain external grant exchange to hide it. + +Use the pinned Credential update operation to replace grant material: + +```python +client.beta.agents.vaults.credentials.update( + credential.id, + vault_id=vault.id, + auth={"type": "mcp_oauth", "access_token": new_token}, +) +``` + +Identity, name, auth type, destination, refresh endpoint/client ID/resource and +endpoint authentication method remain unchanged. A refresh configuration cannot +be added to a Credential that was created without one. + +| Update field | Omitted | Explicit null | +| --- | --- | --- | +| `access_token` | Keep | Keep (local interpretation) | +| `expires_at` | Keep, or clear when a new access token is supplied | Clear | +| `refresh` | Keep | Keep (local interpretation) | +| `refresh.refresh_token` | Keep | Keep | +| `refresh.scope` | Keep | Stop sending scope | +| `refresh.token_endpoint_auth` | Keep | Keep (local interpretation) | +| `refresh.token_endpoint_auth.client_secret` | Keep | Keep | + +Supplying `token_endpoint_auth` during replacement requires the existing +`client_secret_basic` or `client_secret_post` method. Read the Credential again to +observe updated safe metadata. Whole-object null rules marked above are not proven +hosted behavior. Exact refresh timing, retry/error equivalence and unspecified +field-edge behavior are not complete protocol qualification. + +## Provider network policy + +Refresh endpoints must be HTTPS. By default Core rejects nonpublic destinations, +including loopback, private, link-local and shared-address ranges. Resolution is +checked before dialing the resolved IP, retaining TLS hostname verification and +preventing a second DNS lookup from changing the destination. Ambient HTTP proxies +are not used for grant exchange. + +For an operator-controlled private issuer, configure exact HTTPS origins: + +```sh +export AGENTS_API_OAUTH_TRUSTED_ORIGINS='https://issuer.internal:8443' +``` + +The comma-separated list is server configuration, not a tenant parameter. It +allows private addresses only for those exact host/port origins; it does not +allow HTTP, redirects or invalid certificates. Install the issuer CA in the +service trust store (for example with Go's `SSL_CERT_FILE`). An invalid explicit +origin fails service startup. Trust only destinations authorized to receive tenant +grants; this setting is not an unrestricted network bypass. + +## Revocation and limits + +Deleting the stored Credential or Vault blocks later Core lookups while preserving +Session history and frozen selection. It does not revoke the provider grant, +withdraw an access token already delivered to a native process, or cancel running +Sessions. The application performs provider revocation and cancellation when +required. Revoked or invalid refresh grants fail closed until replaced; they do +not cause automatic resource deletion or credential reselection. + +This dispatch-time refresh does not promise mid-turn hot replacement, immediate +provider revocation detection, native 401 recovery or transparent replay. Unknown +expiry does not trigger proactive refresh. Storage-key rotation, archive lifecycle +and arbitrary provider/harness combinations remain separate work. + +Keycloak is used only by private real-acceptance infrastructure. No production +code depends on its realm, endpoints, token format or administrative APIs. + +## Acceptance scope + +The 2026-09-22 credential lifecycle batch uses a standalone Core database, genuine +Keycloak 26.7.4 authorization-code grants with S256 PKCE, a TLS MCP server checking +issuer/audience/expiry through provider introspection, and real Kimi model calls. +Keycloak's public, Basic and POST client authentication flows exercise consent, +refresh-token rotation, rejected reuse, revocation and code/PKCE rejection. All +three grant variants also exercise fixed-SDK/raw-HTTP resource operations. + +Codex with Basic client authentication exercises initial MCP access, dispatch +refresh, another refresh after Core restart, manual replacement, refusal after +refresh-grant revocation, reauthorization and refusal after Credential deletion. +Claude with POST client authentication exercises initial access and refresh +through the same Core path. Revocation/deletion failures are checked before new +native or MCP work. These are the existing trusted `environment:none` public +MCP profiles; no new hosted/user-managed/MiniMax profile is qualified. + +Refresh acceptance explicitly moves the stored declared expiry into the past; +it does not claim waiting for the provider JWT to expire. Reads, public histories +and owned logs are checked for known grant/model secrets. Controlled PostgreSQL +concurrency and failed-commit tests supplement these live checks. The browser +console reads mixed static/OAuth metadata and permits deletion; application-owned +OAuth authorization/replacement is performed through the public API, while its +existing static-token replacement UI remains static-only. + +This evidence does not qualify Google, GitHub or arbitrary OAuth services, +provider-independent error equivalence, or the complete Agents API protocol. diff --git a/services/agents-api/tests/official_credentials.py b/services/agents-api/tests/official_credentials.py index aae9d08c4..6d8779514 100644 --- a/services/agents-api/tests/official_credentials.py +++ b/services/agents-api/tests/official_credentials.py @@ -70,7 +70,7 @@ def safe_body(response, status): {**request, "name": " " + "🧪" * 64 + "a "}, {**request, "auth": None}, {**request, "auth": []}, {**request, "auth": {**auth, "token": 3}}, - {**request, "auth": {"type": "mcp_oauth", "mcp_server_url": destination, "access_token": canary}}, + {**request, "auth": {"type": "mcp_oauth", "mcp_server_url": destination, "access_token": None}}, {**request, "metadata": {"unexpected": "field"}}, ] for field in ("type", "mcp_server_url", "token"): diff --git a/services/agents-api/tests/official_oauth_credentials.py b/services/agents-api/tests/official_oauth_credentials.py new file mode 100644 index 000000000..593d053fc --- /dev/null +++ b/services/agents-api/tests/official_oauth_credentials.py @@ -0,0 +1,214 @@ +"""Pinned SDK/raw HTTP OAuth resource verification, with no provider requests. + +The caller supplies an isolated Core/PostgreSQL service, strict official clients +for an owner, foreign tenant and same-tenant peer, and an HTTP transport. This +verifier creates and deletes only its own Vaults. External authorization, refresh +execution and provider revocation need separate real-provider acceptance. +""" + +from copy import deepcopy +import importlib.metadata +import json +from pathlib import Path +import uuid + + +def verify_oauth_credentials(client, other, peer, raw, *, evidence_path=None): + pin = json.loads((Path(__file__).resolve().parents[3] / "contracts/agents-api/upstream.json").read_text()) + distribution = importlib.metadata.distribution("openai") + source = json.loads(distribution.read_text("direct_url.json") or "{}") + assert distribution.version == pin["sdk_version"] and source["vcs_info"]["commit_id"] == pin["commit"] + assert all(api._strict_response_validation for api in (client, other, peer)) + credentials = client.beta.agents.vaults.credentials + base = str(client.base_url).rstrip("/") + prefix = "oauth-resource-" + uuid.uuid4().hex + secrets = [prefix + suffix for suffix in ("-access", "-refresh", "-client", "-replacement")] + proof = {"sdk": pin, "variants": [], "checks": [], "passed": False} + owned = [] + expiry = "2030-01-02T03:04:05.123Z" + destination = "https://mcp.example.invalid/tools" + + def headers(api=client): + return {"Authorization": "Bearer " + api.api_key, "OpenAI-Beta": "agents=v1"} + + def safe(response, status=200): + assert response.status_code == status, "Unexpected OAuth resource HTTP status" + assert not any(secret in response.text for secret in secrets), "OAuth secret appeared in a public response" + assert response.headers["cache-control"] == "no-store" + return response.json() + + def endpoint(vault, credential=None): + return base + "/vaults/" + vault + "/credentials" + ("/" + credential if credential else "") + + def check_resource(body, vault, expected_auth): + assert set(body) == {"id", "vault_id", "name", "object", "auth", "created_at", "updated_at"} + assert body["object"] == "vault.credential" and body["vault_id"] == vault + assert uuid.UUID(body["id"]).int != 0 + assert type(body["created_at"]) is int and type(body["updated_at"]) is int + assert body["auth"] == expected_auth, "OAuth safe auth metadata or nullable fields changed" + return body + + def retrieve(vault, credential, expected_auth): + response = credentials.with_raw_response.retrieve(credential, vault_id=vault) + body = check_resource(safe(response.http_response), vault, expected_auth) + assert response.parse().to_dict() == body + assert safe(raw.get(endpoint(vault, credential), headers=headers())) == body + assert peer.beta.agents.vaults.credentials.retrieve(credential, vault_id=vault).to_dict() == body + return body + + def update(vault, previous, patch, expected_auth, sdk=True): + if sdk: + response = credentials.with_raw_response.update(previous["id"], vault_id=vault, auth={"type": "mcp_oauth", **patch}) + body = safe(response.http_response) + assert response.parse().to_dict() == body + else: + body = safe(raw.post(endpoint(vault, previous["id"]), headers=headers(), json={"auth": {"type": "mcp_oauth", **patch}})) + check_resource(body, vault, expected_auth) + assert {k: v for k, v in body.items() if k not in ("auth", "updated_at")} == { + k: v for k, v in previous.items() if k not in ("auth", "updated_at")} + assert body["updated_at"] >= previous["updated_at"] + assert retrieve(vault, body["id"], expected_auth) == body + return body + + try: + vault = client.beta.agents.vaults.create(name=prefix) + owned.append((client, vault.id)) + wrong = client.beta.agents.vaults.create(name=prefix + "-wrong") + owned.append((client, wrong.id)) + foreign = other.beta.agents.vaults.create(name=prefix + "-foreign") + owned.append((other, foreign.id)) + expected = {} + for index, method in enumerate((None, "none", "client_secret_basic", "client_secret_post")): + auth = {"type": "mcp_oauth", "mcp_server_url": destination, "access_token": secrets[0]} + expected_auth = {"type": "mcp_oauth", "mcp_server_url": destination, "expires_at": None, "refresh": None} + if method is not None: + auth.update(expires_at=expiry, refresh={ + "client_id": "public-test-client", "refresh_token": secrets[1], + "token_endpoint": "https://issuer.example.invalid/token", "token_endpoint_auth": {"type": method}}) + if method != "none": + auth["refresh"]["token_endpoint_auth"]["client_secret"] = secrets[2] + auth["refresh"].update(resource=destination, scope="read write") + expected_auth.update(expires_at=expiry, refresh={ + "client_id": "public-test-client", "token_endpoint": "https://issuer.example.invalid/token", + "token_endpoint_auth": {"type": method}, "resource": auth["refresh"].get("resource"), + "scope": auth["refresh"].get("scope")}) + response = credentials.with_raw_response.create(vault.id, name=" OAuth " + str(index) + " ", auth=auth) + body = check_resource(safe(response.http_response), vault.id, expected_auth) + assert response.parse().to_dict() == body and body["name"] == "OAuth " + str(index) + current = retrieve(vault.id, body["id"], expected_auth) + current = update(vault.id, current, {}, expected_auth) + current = update(vault.id, current, {"access_token": None, "refresh": None}, expected_auth, sdk=False) + changed = deepcopy(expected_auth) + changed["expires_at"] = None + current = update(vault.id, current, {"access_token": secrets[3]}, changed) + expected_auth = deepcopy(expected_auth) + expected_auth["expires_at"] = expiry + current = update(vault.id, current, {"expires_at": expiry}, expected_auth, sdk=False) + current = update(vault.id, current, {"expires_at": None}, changed) + current = update(vault.id, current, {"access_token": secrets[0], "expires_at": expiry}, expected_auth) + if method is not None: + current = update(vault.id, current, {"refresh": {"refresh_token": None, "token_endpoint_auth": None}}, expected_auth) + current = update(vault.id, current, {"refresh": {"refresh_token": secrets[3]}}, expected_auth, sdk=False) + changed = deepcopy(expected_auth) + changed["refresh"]["scope"] = None + current = update(vault.id, current, {"refresh": {"scope": None}}, changed) + expected_auth = changed + changed = deepcopy(expected_auth) + changed["refresh"]["scope"] = "" + current = update(vault.id, current, {"refresh": {"scope": ""}}, changed, sdk=False) + expected_auth = changed + if method != "none": + for secret_patch in ({}, {"client_secret": None}, {"client_secret": secrets[3]}): + current = update(vault.id, current, {"refresh": {"token_endpoint_auth": {"type": method, **secret_patch}}}, expected_auth) + invalid_patches = [ + {"refresh": {"token_endpoint_auth": {"type": "client_secret_post" if method != "client_secret_post" else "client_secret_basic"}}}, + {"refresh": {"client_id": "changed"}}, {"refresh": {"token_endpoint": "https://other.example/token"}}, + {"refresh": {"resource": None}}, {"refresh": {"token_endpoint_auth": {"type": "none"}}}] + else: + invalid_patches = [{"refresh": {}}, {"refresh": {"scope": "added"}}] + invalid_patches += [{"access_token": 3}, {"expires_at": "tomorrow"}, {"token": secrets[0]}, + {"refresh": {"scope": 3}}, {"refresh": {"refresh_token": 3}}] + for patch in invalid_patches: + safe(raw.post(endpoint(vault.id, current["id"]), headers=headers(), json={"auth": {"type": "mcp_oauth", **patch}}), 400) + assert retrieve(vault.id, current["id"], expected_auth) == current + expected[current["id"]] = current + proof["variants"].append({"method": method, "credential": current}) + + # Explicit null creation must have the same public nullable fields as omission. + null_auth = {"type": "mcp_oauth", "mcp_server_url": destination, "expires_at": None, "refresh": None} + null_body = safe(raw.post(endpoint(vault.id), headers=headers(), json={ + "name": "Raw nulls", "auth": {**null_auth, "access_token": secrets[0]}})) + check_resource(null_body, vault.id, null_auth) + assert retrieve(vault.id, null_body["id"], null_auth) == null_body + expected[null_body["id"]] = null_body + static = credentials.create(vault.id, name="Static regression", auth={"type": "static_bearer", "mcp_server_url": destination, "token": secrets[0]}) + expected[static.id] = static.to_dict() + assert expected[static.id]["auth"] == {"type": "static_bearer", "mcp_server_url": destination} + safe(raw.post(endpoint(vault.id, static.id), headers=headers(), json={"auth": {"type": "mcp_oauth", "access_token": secrets[3]}}), 400) + + invalid_create = [ + {"type": "mcp_oauth", "mcp_server_url": destination}, + {"type": "mcp_oauth", "mcp_server_url": destination, "access_token": None}, + {"type": "mcp_oauth", "mcp_server_url": "http://issuer.example", "access_token": secrets[0]}, + {"type": "mcp_oauth", "mcp_server_url": destination, "access_token": secrets[0], "refresh": {}}, + {"type": "mcp_oauth", "mcp_server_url": destination, "access_token": secrets[0], "expires_at": 3}, + ] + for auth in invalid_create: + safe(raw.post(endpoint(vault.id), headers=headers(), json={"name": "Invalid", "auth": auth}), 400) + full = {"type": "mcp_oauth", "mcp_server_url": destination, "access_token": secrets[0], "refresh": { + "client_id": "client", "refresh_token": secrets[1], "token_endpoint": "https://issuer.example/token", + "token_endpoint_auth": {"type": "none"}}} + for invalid in (None, {}, {"type": "client_secret_basic"}, {"type": "none", "client_secret": None}, + {"type": "client_secret_post", "client_secret": 3}): + auth = deepcopy(full) + auth["refresh"]["token_endpoint_auth"] = invalid + safe(raw.post(endpoint(vault.id), headers=headers(), json={"name": "Invalid auth", "auth": auth}), 400) + + response = credentials.with_raw_response.list(vault.id, order="asc", limit=100) + listed = safe(response.http_response) + assert response.parse().to_dict() == listed + assert {row["id"]: row for row in listed["data"]} == expected and not listed["has_more"] + assert safe(raw.get(endpoint(vault.id), headers=headers(), params={"order": "asc", "limit": 100})) == listed + selected = null_body["id"] + for target, api in ((endpoint(wrong.id, selected), client), (endpoint(foreign.id, selected), client), + (endpoint(vault.id, selected), other), (endpoint(vault.id, str(uuid.uuid4())), client)): + for method in ("GET", "POST", "DELETE"): + kwargs = {"json": {"auth": {"type": "mcp_oauth", "access_token": secrets[3]}}} if method == "POST" else {} + safe(raw.request(method, target, headers=headers(api), **kwargs), 404) + safe(raw.get(endpoint(vault.id), headers=headers(other)), 404) + safe(raw.post(endpoint(vault.id), headers=headers(other), json={"name": "Foreign", "auth": full}), 404) + safe(raw.get(endpoint(vault.id, selected), headers={"Authorization": "Bearer " + client.api_key}), 400) + safe(raw.get(endpoint(vault.id, selected), headers={"OpenAI-Beta": "agents=v1"}), 401) + assert retrieve(vault.id, selected, null_auth) == null_body + proof["checks"].extend(["strict_sdk_raw_metadata", "all_endpoint_auth_variants", "omitted_null_patches", + "immutable_method_and_configuration", "tenant_and_vault_scope", "static_regression"]) + for index, (credential, previous) in enumerate(expected.items()): + if index % 2: + deleted = safe(raw.delete(endpoint(vault.id, credential), headers=headers())) + else: + response = credentials.with_raw_response.delete(credential, vault_id=vault.id) + deleted = safe(response.http_response) + assert response.parse().to_dict() == deleted + assert deleted == {"id": credential, "deleted": True, "object": "vault.credential.deleted"} + safe(raw.get(endpoint(vault.id, credential), headers=headers()), 404) + safe(raw.delete(endpoint(vault.id, credential), headers=headers()), 404) + safe(raw.post(endpoint(vault.id, credential), headers=headers(), json={"auth": {"type": "mcp_oauth", "access_token": secrets[3]}}), 404) + assert list(credentials.list(vault.id)) == [] + proof["checks"].append("sdk_raw_deletion_and_no_resurrection") + proof["passed"] = True + return proof + finally: + cleanup_errors = [] + for api, vault_id in reversed(owned): + try: + api.beta.agents.vaults.delete(vault_id) + except Exception as error: + cleanup_errors.append(type(error).__name__) + proof["cleanup_errors"] = cleanup_errors + if cleanup_errors: + proof["passed"] = False + if evidence_path is not None: + serialized = json.dumps(proof, indent=2) + assert not any(secret in serialized for secret in secrets) + Path(evidence_path).write_text(serialized + "\n") + assert not cleanup_errors, "OAuth verifier could not remove owned Vaults"