mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-12 14:15:00 +00:00
* feat(mcp): MCP OAuth 2.1 client — full implementation with tests
Implements a complete MCP OAuth 2.1 authorization flow for tool servers that
require user-delegated access, covering all layers from DB to UI.
- discovery.go: RFC 9728 protected-resource → RFC 8414 AS metadata → OIDC
fallback chain with 5-min in-memory cache and InvalidateCache()
- dcr.go: RFC 7591 Dynamic Client Registration with response size guard
- flow.go: PKCE (S256) authorization code flow — StartFlow(), ExchangeCode(),
ClientCredentials(), auto-cleanup of expired flows; carries AS issuer through
PendingFlow for status display
- refresher.go: OAuthTokenProvider with in-memory token cache, automatic refresh
on expiry, per-user vs global slot isolation, InvalidateCache/InvalidateServer
- migrations/000074 + SQLite schema: mcp_oauth_tokens with AES-256-GCM encrypted
access/refresh tokens, partial unique index for global vs per-user rows,
ON DELETE CASCADE from mcp_servers
- store.MCPOAuthTokenStore: Upsert, Get/GetUser, Delete/DeleteUser, and
DeleteServerOAuthTokens (purge all rows for a server)
- PostgreSQL + SQLite implementations
- POST /v1/mcp/oauth/start — discovery + optional DCR + PKCE redirect URL;
client_credentials completes server-side (no redirect) and returns completed=true
- GET /v1/mcp/oauth/callback — exchange code, persist token, publish WS event;
payload built via json.Marshal (no reflected XSS via error_description)
- GET /v1/mcp/oauth/status/{id}, DELETE /v1/mcp/oauth/token/{id} — admin-gated
- POST /v1/mcp/oauth/discover/{id} — on-demand discovery probe
- All outbound calls go through the SSRF-safe client with pinned IPs
- pkg/protocol/mcp_events.go: EventMCPOAuthComplete routed only to the initiating
user (admins in-tenant included); fail-closed across tenants
- getUserMCPTools() injects Authorization: Bearer from OAuthTokenProvider; on a
401 for OAuth servers it purges the cached token so the next turn re-resolves
- handleUpdateServer purges all OAuth tokens (global + per-user), drops the
refresher cache, and evicts the pool when a server's URL or OAuth config
(client_id / endpoints / grant_type / scope / auth_type) changes — so the
status UI and agent never use a token minted for the old resource/AS
- MCPOAuthDialog (WS-driven), unified user-credentials dialog, OAuth settings
fields; handles the no-redirect client_credentials completion
- internal/mcp/oauth/*_test.go: discovery cache, PKCE, DCR, refresher
- internal/http/mcp_oauth_test.go + mcp_update_oauth_purge_test.go: routes, auth
gating, WS event, purge-on-URL/OAuth-config-change
- tests/integration: store + encryption + tenant isolation, E2E start→callback,
DeleteServerOAuthTokens
- internal/gateway/event_filter_test.go, internal/agent/loop_mcp_user_test.go
* fix(mcp): return 400 on OAuth callback with code but missing state
The callback handler rendered a 200 HTML page whenever code or state was
absent. An auth code WITH a missing state is a malformed / CSRF-risk
callback (state is the CSRF token), so reject that case with HTTP 400.
A bare hit with neither code nor state (user opening the URL directly),
provider errors, and exchange failures keep their 200 HTML popup page.
Adds a status code parameter to writeCallbackHTML. Fixes the
TestOAuthCallbackMissingState integration regression while keeping
TestHandleCallbackMissingCodeAndState (no params -> 200) green.
* fix(mcp): scope-based OAuth auth + honor manual OAuth endpoints
Addresses the two MCP/OAuth security-review findings.
Finding 1 — authorization. mcp_oauth_tokens is tenant-scoped, but
start/status/revoke were gated only by requireAuth(RoleAdmin), an RBAC
role check, not tenant membership, so a RoleAdmin caller could act on a
tenant they don't administer. A blanket requireTenantAdmin would have
broken per-user self-service, which the UI exposes (the per-user
MCPUserCredentialsDialog shows an "Authorize" button to regular users for
their own credentials). Instead mirror the existing per-user MCP
credentials model (resolveTargetUserID in mcp_user_credentials.go):
- start/status/revoke accept any authenticated user; each handler calls
authorizeOAuthScope.
- a caller may manage their OWN per-user token (self-service); the
global/server token (user_id="") and other users' tokens require
tenant-admin (owner bypass), so a RoleAdmin that is not a tenant admin
is rejected.
- discover stays admin-only (it only previews AS metadata for a server).
Add a TenantStore dependency. Tests cover self-service, on-behalf-of-
another (403), and global-by-non-tenant-admin (403).
Finding 2 — honor manual OAuth config end-to-end. The UI sent use_dcr /
auth_endpoint / token_endpoint and the update path fingerprinted them for
purge, but handleStart always discovered + DCR'd and ignored them. Now:
- use_dcr=false (a *bool, so legacy/absent stays discover+DCR) skips
discovery/registration and uses the operator endpoints, SSRF-validated.
- token_endpoint is always required; auth_endpoint only for auth-code
grants — client_credentials needs no authorization URL, matching the UI
which hides that field for that grant.
- the refresher already refreshes against the stored token_endpoint and
the callback persists it, so manual-mode tokens refresh correctly.
- oauthFingerprint includes use_dcr (nil normalized to true) so toggling
DCR mode purges stale tokens.
- the web form only serializes manual endpoints when use_dcr is off.
Audited all MCP dialogs (form, global OAuth, per-user credentials, grants,
tools): OAuth dialogs handle completed/auth_url identically and read
config from stored server settings; runtime connect uses the stored token
via the refresher (no re-discovery).
Tests: manual auth-code + client_credentials endpoints, missing/SSRF
endpoints, and the full self/global/on-behalf authorization matrix.
236 lines
7.3 KiB
Go
236 lines
7.3 KiB
Go
//go:build integration
|
|
|
|
package integration
|
|
|
|
import (
|
|
"context"
|
|
"crypto/sha256"
|
|
"database/sql"
|
|
"encoding/hex"
|
|
"fmt"
|
|
"testing"
|
|
|
|
"github.com/google/uuid"
|
|
"github.com/nextlevelbuilder/goclaw/internal/store"
|
|
pgstore "github.com/nextlevelbuilder/goclaw/internal/store/pg"
|
|
)
|
|
|
|
// seedTwoTenants creates 2 independent tenants with agents for isolation testing.
|
|
func seedTwoTenants(t *testing.T, db *sql.DB) (tenantA, tenantB, agentA, agentB uuid.UUID) {
|
|
t.Helper()
|
|
tenantA, agentA = seedTenantAgent(t, db)
|
|
tenantB, agentB = seedTenantAgent(t, db)
|
|
return
|
|
}
|
|
|
|
// seedTeam creates an active team with v2 settings (required for recovery queries).
|
|
// The ownerAgentID is set as lead. A second agent is created and added as a member.
|
|
// Returns teamID and the second agent's ID.
|
|
func seedTeam(t *testing.T, db *sql.DB, tenantID, ownerAgentID uuid.UUID) (teamID, memberAgentID uuid.UUID) {
|
|
t.Helper()
|
|
|
|
teamID = uuid.New()
|
|
memberAgentID = uuid.New()
|
|
memberKey := "member-" + memberAgentID.String()[:8]
|
|
|
|
// Create second agent as team member.
|
|
_, err := db.Exec(
|
|
`INSERT INTO agents (id, tenant_id, agent_key, agent_type, status, provider, model, owner_id)
|
|
VALUES ($1, $2, $3, 'predefined', 'active', 'test', 'test-model', 'test-owner')
|
|
ON CONFLICT DO NOTHING`,
|
|
memberAgentID, tenantID, memberKey)
|
|
if err != nil {
|
|
t.Fatalf("seed member agent: %v", err)
|
|
}
|
|
|
|
// Create team with v2 settings — CRITICAL for recovery/lifecycle queries.
|
|
_, err = db.Exec(
|
|
`INSERT INTO agent_teams (id, tenant_id, name, lead_agent_id, status, settings, created_by)
|
|
VALUES ($1, $2, $3, $4, 'active', '{"version": 2}', 'test')`,
|
|
teamID, tenantID, "test-team-"+teamID.String()[:8], ownerAgentID)
|
|
if err != nil {
|
|
t.Fatalf("seed team: %v", err)
|
|
}
|
|
|
|
// Add both agents as members.
|
|
for _, m := range []struct {
|
|
agentID uuid.UUID
|
|
role string
|
|
}{
|
|
{ownerAgentID, "lead"},
|
|
{memberAgentID, "member"},
|
|
} {
|
|
_, err = db.Exec(
|
|
`INSERT INTO agent_team_members (team_id, agent_id, tenant_id, role)
|
|
VALUES ($1, $2, $3, $4) ON CONFLICT DO NOTHING`,
|
|
teamID, m.agentID, tenantID, m.role)
|
|
if err != nil {
|
|
t.Fatalf("seed team member: %v", err)
|
|
}
|
|
}
|
|
|
|
t.Cleanup(func() {
|
|
db.Exec("DELETE FROM agent_team_members WHERE team_id = $1", teamID)
|
|
db.Exec("DELETE FROM agent_teams WHERE id = $1", teamID)
|
|
db.Exec("DELETE FROM agents WHERE id = $1", memberAgentID)
|
|
})
|
|
|
|
return teamID, memberAgentID
|
|
}
|
|
|
|
// seedSession creates a minimal session record.
|
|
func seedSession(t *testing.T, db *sql.DB, tenantID, agentID uuid.UUID) string {
|
|
t.Helper()
|
|
|
|
sessionKey := "sess-" + uuid.New().String()[:8]
|
|
_, err := db.Exec(
|
|
`INSERT INTO sessions (session_key, tenant_id, agent_id, user_id, messages, summary)
|
|
VALUES ($1, $2, $3, 'test-user', '[]', '')`,
|
|
sessionKey, tenantID, agentID)
|
|
if err != nil {
|
|
t.Fatalf("seed session: %v", err)
|
|
}
|
|
|
|
t.Cleanup(func() {
|
|
db.Exec("DELETE FROM sessions WHERE session_key = $1 AND tenant_id = $2", sessionKey, tenantID)
|
|
})
|
|
|
|
return sessionKey
|
|
}
|
|
|
|
// seedMCPServer creates a minimal MCP server record.
|
|
func seedMCPServer(t *testing.T, db *sql.DB, tenantID uuid.UUID) uuid.UUID {
|
|
t.Helper()
|
|
|
|
serverID := uuid.New()
|
|
name := "test-mcp-" + serverID.String()[:8]
|
|
_, err := db.Exec(
|
|
`INSERT INTO mcp_servers (id, tenant_id, name, display_name, transport, enabled, created_by)
|
|
VALUES ($1, $2, $3, $3, 'stdio', true, 'test-user')`,
|
|
serverID, tenantID, name)
|
|
if err != nil {
|
|
t.Fatalf("seed mcp server: %v", err)
|
|
}
|
|
|
|
t.Cleanup(func() {
|
|
db.Exec("DELETE FROM mcp_user_credentials WHERE server_id = $1", serverID)
|
|
db.Exec("DELETE FROM mcp_access_requests WHERE server_id = $1", serverID)
|
|
db.Exec("DELETE FROM mcp_user_grants WHERE server_id = $1", serverID)
|
|
db.Exec("DELETE FROM mcp_agent_grants WHERE server_id = $1", serverID)
|
|
db.Exec("DELETE FROM mcp_servers WHERE id = $1", serverID)
|
|
})
|
|
|
|
return serverID
|
|
}
|
|
|
|
// seedSecureCLI creates a minimal secure CLI binary record.
|
|
// Uses testEncryptionKey for the encrypted_env field.
|
|
func seedSecureCLI(t *testing.T, db *sql.DB, tenantID uuid.UUID) uuid.UUID {
|
|
t.Helper()
|
|
|
|
binaryID := uuid.New()
|
|
name := "test-cli-" + binaryID.String()[:8]
|
|
// encrypted_env is BYTEA NOT NULL — use a dummy encrypted value.
|
|
dummyEnv := []byte(`{"TEST_KEY": "test_value"}`)
|
|
|
|
_, err := db.Exec(
|
|
`INSERT INTO secure_cli_binaries (id, tenant_id, binary_name, encrypted_env, description, enabled)
|
|
VALUES ($1, $2, $3, $4, 'test CLI', true)`,
|
|
binaryID, tenantID, name, dummyEnv)
|
|
if err != nil {
|
|
t.Fatalf("seed secure cli: %v", err)
|
|
}
|
|
|
|
t.Cleanup(func() {
|
|
db.Exec("DELETE FROM secure_cli_user_credentials WHERE binary_id = $1", binaryID)
|
|
db.Exec("DELETE FROM secure_cli_agent_grants WHERE binary_id = $1", binaryID)
|
|
db.Exec("DELETE FROM secure_cli_binaries WHERE id = $1", binaryID)
|
|
})
|
|
|
|
return binaryID
|
|
}
|
|
|
|
// seedAPIKey creates a minimal API key record. Returns key ID and the raw key hash.
|
|
func seedAPIKey(t *testing.T, db *sql.DB, tenantID uuid.UUID) (uuid.UUID, string) {
|
|
t.Helper()
|
|
|
|
keyID := uuid.New()
|
|
rawKey := "gclw_test_" + keyID.String()[:16]
|
|
hash := sha256.Sum256([]byte(rawKey))
|
|
keyHash := hex.EncodeToString(hash[:])
|
|
|
|
_, err := db.Exec(
|
|
`INSERT INTO api_keys (id, tenant_id, name, prefix, key_hash, scopes, created_by)
|
|
VALUES ($1, $2, 'test-key', $3, $4, '{}', 'test-user')`,
|
|
keyID, tenantID, rawKey[:8], keyHash)
|
|
if err != nil {
|
|
t.Fatalf("seed api key: %v", err)
|
|
}
|
|
|
|
t.Cleanup(func() {
|
|
db.Exec("DELETE FROM api_keys WHERE id = $1", keyID)
|
|
})
|
|
|
|
return keyID, keyHash
|
|
}
|
|
|
|
// seedContact creates a minimal channel contact record.
|
|
func seedContact(t *testing.T, db *sql.DB, tenantID uuid.UUID) uuid.UUID {
|
|
t.Helper()
|
|
|
|
contactID := uuid.New()
|
|
senderID := fmt.Sprintf("sender-%s", contactID.String()[:8])
|
|
_, err := db.Exec(
|
|
`INSERT INTO channel_contacts (id, tenant_id, channel_type, sender_id, peer_kind)
|
|
VALUES ($1, $2, 'telegram', $3, 'private')`,
|
|
contactID, tenantID, senderID)
|
|
if err != nil {
|
|
t.Fatalf("seed contact: %v", err)
|
|
}
|
|
|
|
t.Cleanup(func() {
|
|
db.Exec("DELETE FROM channel_contacts WHERE id = $1", contactID)
|
|
})
|
|
|
|
return contactID
|
|
}
|
|
|
|
// oauthStore returns a PGMCPOAuthTokenStore backed by the shared test DB.
|
|
func oauthStore(t *testing.T) *pgstore.PGMCPOAuthTokenStore {
|
|
t.Helper()
|
|
return pgstore.NewPGMCPOAuthTokenStore(testDB(t), testEncryptionKey)
|
|
}
|
|
|
|
// seedMCPOAuthToken inserts an OAuth token for testing and registers cleanup.
|
|
// userID="" inserts a global (tenant-level) token; non-empty inserts per-user.
|
|
func seedMCPOAuthToken(t *testing.T, db *sql.DB, tenantID, serverID uuid.UUID, userID string) *store.MCPOAuthToken {
|
|
t.Helper()
|
|
tok := &store.MCPOAuthToken{
|
|
ID: uuid.New(),
|
|
ServerID: serverID,
|
|
TenantID: tenantID,
|
|
UserID: userID,
|
|
AccessToken: "seed-access-token",
|
|
RefreshToken: "seed-refresh-token",
|
|
TokenType: "Bearer",
|
|
Scopes: "read write",
|
|
DCRClientID: "seed-client-id",
|
|
DCRIssuer: "https://auth.example.com",
|
|
TokenEndpoint: "https://auth.example.com/token",
|
|
}
|
|
st := pgstore.NewPGMCPOAuthTokenStore(db, testEncryptionKey)
|
|
if err := st.UpsertOAuthToken(context.Background(), tok); err != nil {
|
|
t.Fatalf("seedMCPOAuthToken: %v", err)
|
|
}
|
|
t.Cleanup(func() {
|
|
if userID == "" {
|
|
db.Exec("DELETE FROM mcp_oauth_tokens WHERE server_id = $1 AND tenant_id = $2 AND user_id IS NULL",
|
|
serverID, tenantID)
|
|
} else {
|
|
db.Exec("DELETE FROM mcp_oauth_tokens WHERE server_id = $1 AND tenant_id = $2 AND user_id = $3",
|
|
serverID, tenantID, userID)
|
|
}
|
|
})
|
|
return tok
|
|
}
|