package agent import ( "context" "encoding/json" "sync" "testing" "time" "github.com/google/uuid" mcpbridge "github.com/nextlevelbuilder/goclaw/internal/mcp" "github.com/nextlevelbuilder/goclaw/internal/store" ) // ---- mock helpers for getUserMCPTools OAuth tests ---- // minimalMCPServerStore implements store.MCPServerStore with panics for all // methods except GetUserCredentials, which is the only method called in // getUserMCPTools. type minimalMCPServerStore struct { userCreds *store.MCPUserCredentials userCredsErr error getUserCredsCalls int } func (m *minimalMCPServerStore) GetUserCredentials(_ context.Context, _ uuid.UUID, _ string) (*store.MCPUserCredentials, error) { m.getUserCredsCalls++ return m.userCreds, m.userCredsErr } func (m *minimalMCPServerStore) CreateServer(_ context.Context, _ *store.MCPServerData) error { panic("not implemented") } func (m *minimalMCPServerStore) GetServer(_ context.Context, _ uuid.UUID) (*store.MCPServerData, error) { panic("not implemented") } func (m *minimalMCPServerStore) GetServerByName(_ context.Context, _ string) (*store.MCPServerData, error) { panic("not implemented") } func (m *minimalMCPServerStore) ListServers(_ context.Context) ([]store.MCPServerData, error) { panic("not implemented") } func (m *minimalMCPServerStore) UpdateServer(_ context.Context, _ uuid.UUID, _ map[string]any) error { panic("not implemented") } func (m *minimalMCPServerStore) DeleteServer(_ context.Context, _ uuid.UUID) error { panic("not implemented") } func (m *minimalMCPServerStore) GrantToAgent(_ context.Context, _ *store.MCPAgentGrant) error { panic("not implemented") } func (m *minimalMCPServerStore) RevokeFromAgent(_ context.Context, _, _ uuid.UUID) error { panic("not implemented") } func (m *minimalMCPServerStore) ListAgentGrants(_ context.Context, _ uuid.UUID) ([]store.MCPAgentGrant, error) { panic("not implemented") } func (m *minimalMCPServerStore) ListServerGrants(_ context.Context, _ uuid.UUID) ([]store.MCPAgentGrant, error) { panic("not implemented") } func (m *minimalMCPServerStore) GrantToUser(_ context.Context, _ *store.MCPUserGrant) error { panic("not implemented") } func (m *minimalMCPServerStore) RevokeFromUser(_ context.Context, _ uuid.UUID, _ string) error { panic("not implemented") } func (m *minimalMCPServerStore) CountAgentGrantsByServer(_ context.Context) (map[uuid.UUID]int, error) { panic("not implemented") } func (m *minimalMCPServerStore) ListAccessible(_ context.Context, _ uuid.UUID, _ string) ([]store.MCPAccessInfo, error) { panic("not implemented") } func (m *minimalMCPServerStore) CreateRequest(_ context.Context, _ *store.MCPAccessRequest) error { panic("not implemented") } func (m *minimalMCPServerStore) ListPendingRequests(_ context.Context) ([]store.MCPAccessRequest, error) { panic("not implemented") } func (m *minimalMCPServerStore) ReviewRequest(_ context.Context, _ uuid.UUID, _ bool, _, _ string) error { panic("not implemented") } func (m *minimalMCPServerStore) SetUserCredentials(_ context.Context, _ uuid.UUID, _ string, _ store.MCPUserCredentials) error { panic("not implemented") } func (m *minimalMCPServerStore) DeleteUserCredentials(_ context.Context, _ uuid.UUID, _ string) error { panic("not implemented") } func (m *minimalMCPServerStore) CacheToolDescriptions(_ context.Context, _ uuid.UUID, _ map[string]store.CachedToolInfo) error { return nil } // recordingOAuthProvider records calls to GetValidToken for assertion. type recordingOAuthProvider struct { mu sync.Mutex callCount int returnToken string returnErr error } func (r *recordingOAuthProvider) GetValidToken(_ context.Context, _ uuid.UUID, _ uuid.UUID, _ string) (string, error) { r.mu.Lock() defer r.mu.Unlock() r.callCount++ return r.returnToken, r.returnErr } // newTestPool creates a real *mcpbridge.Pool with short timeouts for unit tests. // The pool's AcquireUser will fail (no real MCP server), but token injection // happens before the pool call. func newTestPool(t *testing.T) *mcpbridge.Pool { t.Helper() pool := mcpbridge.NewPool(mcpbridge.PoolConfig{ MaxSize: 10, UserAcquireTimeout: 10 * time.Millisecond, }) t.Cleanup(func() { pool.Stop() }) return pool } // oauthSettings returns JSON settings with auth_type: oauth. func oauthSettings() json.RawMessage { return json.RawMessage(`{"oauth":{"auth_type":"oauth"}}`) } // oauthServerData returns a minimal MCPServerData for an OAuth-configured HTTP // server. Transport=http + unreachable URL causes AcquireUser to fail quickly // (connection refused) without panicking — unlike stdio with empty command // which would spawn a goroutine reading from a nil pipe. func oauthServerData() store.MCPServerData { return store.MCPServerData{ BaseModel: store.BaseModel{ID: uuid.New()}, Name: "oauth-server-" + uuid.New().String()[:8], Transport: "http", URL: "http://127.0.0.1:1", // port 1 = connection refused immediately Settings: oauthSettings(), } } // TestResolveActorUserID locks the actor-vs-context user-id resolution semantics // that gate per-user MCP credential lookup (and other per-actor resources). // // Two bugs this helper fixes: // // 1. Group chats: gateway consumer rewrites UserID to a group-scope composite // ("group::") for shared memory. The Bitrix24 provisioner // stores MCPUserCredentials keyed by the real external user id (= SenderID). // Lookup with group composite always missed → MCP tools silently absent. // // 2. DM with merged contact (C1): gateway consumer rewrites DM UserID to the // tenant_user UUID when ContactCollector.ResolveTenantUserID succeeds. // Provisioner still stores by SenderID. Lookup with UUID misses → MCP // tools fail in DMs after contact merge. // // resolveActorUserID accepts channelType so Bitrix24 always recovers SenderID // (covers both rewrite cases). Other channels retain group-only recovery. func TestResolveActorUserID(t *testing.T) { cases := []struct { name string userID string senderID string peerKind string channelType string want string }{ // DM unmerged: UserID == SenderID. No rewrite happened. { name: "dm_returns_user_id_unchanged", userID: "99", senderID: "99", peerKind: "direct", channelType: "", want: "99", }, // Group: gateway overrides UserID with group composite for shared // memory. Helper must recover SenderID for actor-scoped lookups. { name: "group_overrides_to_sender", userID: "group:bitrix-synity:chat4838", senderID: "99", peerKind: "group", channelType: "", want: "99", }, // Discord guild composite ("guild::user:") is also a // group peer — fall back to SenderID for credential lookup. { name: "discord_guild_overrides_to_sender", userID: "guild:1234:user:5678", senderID: "5678", peerKind: "group", channelType: "", want: "5678", }, // Synthetic / system senders (ticker, notification) carry empty // SenderID. No per-user credentials exist for them — fall back to // UserID so the lookup still uses a sensible key. { name: "group_with_empty_sender_falls_back_to_user_id", userID: "group:bitrix-synity:chat4838", senderID: "", peerKind: "group", channelType: "", want: "group:bitrix-synity:chat4838", }, // Empty peer_kind defaults to direct semantics. { name: "empty_peer_kind_treated_as_direct", userID: "99", senderID: "99", peerKind: "", channelType: "", want: "99", }, // Future channel using a peer_kind we don't recognize must NOT be // treated as group automatically — DM semantics are the safer // default (no override). { name: "unknown_peer_kind_does_not_override", userID: "99", senderID: "42", peerKind: "channel", channelType: "", want: "99", }, // ── Bitrix24-specific cases (C1 fix) ─────────────────────────── // Bitrix24 DM, sender NOT merged: UserID == SenderID. Helper returns // SenderID (which is identical) — same outcome either way. { name: "bitrix24_dm_unmerged_uses_sender", userID: "62", senderID: "62", peerKind: "direct", channelType: "bitrix24", want: "62", }, // Bitrix24 DM, sender MERGED to tenant_user (C1 bug): consumer // rewrites UserID to tenant_user UUID. Provisioner stored creds // by SenderID. Helper must return SenderID so lookup hits. { name: "bitrix24_dm_merged_uses_sender_not_uuid", userID: "uuid-abc-def-0123", senderID: "62", peerKind: "direct", channelType: "bitrix24", want: "62", }, // Bitrix24 group: same recovery as generic group, channelType // discriminator does no harm. { name: "bitrix24_group_uses_sender", userID: "group:bitrix-tamgiac:chat4686", senderID: "62", peerKind: "group", channelType: "bitrix24", want: "62", }, // Bitrix24 synthetic event (system/ticker) with no sender: fall // back to UserID. No creds exist anyway. { name: "bitrix24_synthetic_no_sender_falls_back", userID: "system", senderID: "", peerKind: "direct", channelType: "bitrix24", want: "system", }, // ── Other channel backward compat ────────────────────────────── // Telegram DM (no channelType match): keep original behavior. // Telegram doesn't provision per-user creds today; if it did, the // consumer's UserID rewrite for merged contacts would still apply // and Telegram support would be added here when introduced. { name: "telegram_dm_unchanged", userID: "user-456", senderID: "789", peerKind: "direct", channelType: "telegram", want: "user-456", }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { got := resolveActorUserID(tc.userID, tc.senderID, tc.peerKind, tc.channelType) if got != tc.want { t.Errorf("resolveActorUserID(%q, %q, %q, %q) = %q; want %q", tc.userID, tc.senderID, tc.peerKind, tc.channelType, got, tc.want) } }) } } // ---- getUserMCPTools OAuth token injection tests ---- // // These tests verify that mcpOAuthTokenProvider.GetValidToken is called (or // not) based on whether the MCP server settings declare auth_type=oauth. // Pool.AcquireUser fails (no real MCP server), but GetValidToken is injected // BEFORE the pool call, so we can assert on call count without needing a // working MCP connection. // TestGetUserMCPToolsOAuthTokenProviderCalled verifies that when a server's // settings declare auth_type=oauth, GetValidToken is called for the user. func TestGetUserMCPToolsOAuthTokenProviderCalled(t *testing.T) { pool := newTestPool(t) provider := &recordingOAuthProvider{returnToken: "test-bearer-token"} ctx, cancel := context.WithTimeout(context.Background(), 300*time.Millisecond) defer cancel() l := &Loop{ tenantID: uuid.New(), mcpPool: pool, mcpStore: &minimalMCPServerStore{}, mcpUserCredSrvs: []store.MCPAccessInfo{{Server: oauthServerData()}}, mcpOAuthTokenProvider: provider, } _ = l.getUserMCPTools(ctx, "user-1") provider.mu.Lock() callCount := provider.callCount provider.mu.Unlock() if callCount == 0 { t.Error("expected GetValidToken to be called for OAuth-configured server, but it was not") } } // TestGetUserMCPToolsOAuthTokenNotCalledForNonOAuth verifies that when a // server has no OAuth settings, the token provider is never invoked and the // server is skipped (neither static nor OAuth creds). func TestGetUserMCPToolsOAuthTokenNotCalledForNonOAuth(t *testing.T) { pool := newTestPool(t) provider := &recordingOAuthProvider{} l := &Loop{ tenantID: uuid.New(), mcpPool: pool, mcpStore: &minimalMCPServerStore{}, mcpUserCredSrvs: []store.MCPAccessInfo{ {Server: store.MCPServerData{ BaseModel: store.BaseModel{ID: uuid.New()}, Name: "plain-server", Transport: "http", URL: "http://127.0.0.1:1", // No settings → auth_type defaults to "" (not "oauth") }}, }, mcpOAuthTokenProvider: provider, } _ = l.getUserMCPTools(context.Background(), "user-1") provider.mu.Lock() callCount := provider.callCount provider.mu.Unlock() if callCount != 0 { t.Errorf("GetValidToken must not be called for non-OAuth server, called %d times", callCount) } } // TestGetUserMCPToolsOAuthNoTokenSkipsServer verifies that when GetValidToken // returns no valid token (expired / not authorized yet), the OAuth server is // SKIPPED entirely — it must NOT fall back to the shared server-level // headers/api_key as Authorization (which would let an unauthorized user call // tools with the common credential). GetValidToken is still consulted (proving // we entered the OAuth branch) but no tools are produced. func TestGetUserMCPToolsOAuthNoTokenSkipsServer(t *testing.T) { pool := newTestPool(t) expiredErr := errTokenExpiredForTest("oauth token expired") provider := &recordingOAuthProvider{ returnToken: "", returnErr: expiredErr, } ctx, cancel := context.WithTimeout(context.Background(), 300*time.Millisecond) defer cancel() // Server carries a server-level API key — must NOT be used as a fallback. srv := oauthServerData() srv.APIKey = "shared-server-key" l := &Loop{ tenantID: uuid.New(), mcpPool: pool, mcpStore: &minimalMCPServerStore{}, mcpUserCredSrvs: []store.MCPAccessInfo{{Server: srv}}, mcpOAuthTokenProvider: provider, } result := l.getUserMCPTools(ctx, "user-1") if result != nil { t.Errorf("expected nil tools when OAuth has no token (skip, no fallback), got %d tools", len(result)) } provider.mu.Lock() calls := provider.callCount provider.mu.Unlock() if calls == 0 { t.Error("expected GetValidToken to be consulted before skipping the OAuth server") } } // TestGetUserMCPToolsOAuthIgnoresStaticCreds verifies that for an OAuth server // the user's static per-user credentials are NOT consulted or used: even when the // user has set static creds, an OAuth server without a valid token is skipped // (the OAuth token is the sole credential). This closes the bypass where a user // with static headers could call OAuth tools without authorizing. func TestGetUserMCPToolsOAuthIgnoresStaticCreds(t *testing.T) { pool := newTestPool(t) provider := &recordingOAuthProvider{returnToken: ""} // not authorized yet ctx, cancel := context.WithTimeout(context.Background(), 300*time.Millisecond) defer cancel() // Store has valid static creds for the user — must be ignored for OAuth servers. st := &minimalMCPServerStore{ userCreds: &store.MCPUserCredentials{ APIKey: "user-static-key", Headers: map[string]string{"Authorization": "Bearer user-static-key"}, }, } l := &Loop{ tenantID: uuid.New(), mcpPool: pool, mcpStore: st, mcpUserCredSrvs: []store.MCPAccessInfo{{Server: oauthServerData()}}, mcpOAuthTokenProvider: provider, } result := l.getUserMCPTools(ctx, "user-1") if result != nil { t.Errorf("expected nil tools: OAuth server without token must be skipped even with static creds, got %d tools", len(result)) } if st.getUserCredsCalls != 0 { t.Errorf("static GetUserCredentials must NOT be consulted for OAuth servers, called %d times", st.getUserCredsCalls) } provider.mu.Lock() calls := provider.callCount provider.mu.Unlock() if calls == 0 { t.Error("expected GetValidToken to be consulted for the OAuth server") } } // TestGetUserMCPToolsNilPoolEarlyReturn verifies that a nil pool causes the // function to return immediately without calling the token provider. func TestGetUserMCPToolsNilPoolEarlyReturn(t *testing.T) { provider := &recordingOAuthProvider{} l := &Loop{ tenantID: uuid.New(), mcpPool: nil, // triggers early return mcpStore: &minimalMCPServerStore{}, mcpUserCredSrvs: []store.MCPAccessInfo{{Server: oauthServerData()}}, mcpOAuthTokenProvider: provider, } result := l.getUserMCPTools(context.Background(), "user-1") if result != nil { t.Errorf("expected nil when pool is nil, got %v", result) } if provider.callCount != 0 { t.Error("GetValidToken must not be called when pool is nil (early return path)") } } // errTokenExpiredForTest is a simple error type used to simulate ErrTokenExpired // without importing the oauth package (avoids potential import cycles in tests). type errTokenExpiredForTest string func (e errTokenExpiredForTest) Error() string { return string(e) } // TestGetUserMCPToolsGroupCredentialResolution verifies that when a Telegram user // posts in a group, the group's credentials (CredentialUserID) are used for MCP // tool lookup instead of the individual sender's credentials. // // This is the fix for the calendar event creation bug: the user's calendar email // was known but the MCP tools were being looked up with the wrong user ID, // causing the tools to appear blocked by system policy. func TestGetUserMCPToolsGroupCredentialResolution(t *testing.T) { // Simulate a group chat where the group has its own MCP credentials // (e.g., a Google Calendar MCP server configured for the group). groupID := "group:telegram:-100456" serverID := uuid.New() // Track which user ID is used for credential lookup var lookupUserID string st := &trackingMCPServerStore{ userCreds: &store.MCPUserCredentials{ APIKey: "group-calendar-api-key", Headers: map[string]string{"Authorization": "Bearer group-calendar-api-key"}, }, onGetUserCredentials: func(_ context.Context, _ uuid.UUID, userID string) { lookupUserID = userID }, } pool := newTestPool(t) l := &Loop{ tenantID: uuid.New(), mcpPool: pool, mcpStore: st, mcpUserCredSrvs: []store.MCPAccessInfo{ {Server: store.MCPServerData{ BaseModel: store.BaseModel{ID: serverID}, Name: "google-calendar", Transport: "http", URL: "http://127.0.0.1:1", }}, }, } // When CredentialUserID is set in context (resolved group identity), // it should be used for MCP credential lookup. ctx := store.WithCredentialUserID(context.Background(), groupID) _ = l.getUserMCPTools(ctx, groupID) if lookupUserID != groupID { t.Errorf("expected MCP credentials looked up with group ID %q, got %q", groupID, lookupUserID) } } // trackingMCPServerStore is a test store that tracks GetUserCredentials calls. type trackingMCPServerStore struct { userCreds *store.MCPUserCredentials onGetUserCredentials func(_ context.Context, _ uuid.UUID, userID string) } func (m *trackingMCPServerStore) GetUserCredentials(_ context.Context, _ uuid.UUID, userID string) (*store.MCPUserCredentials, error) { if m.onGetUserCredentials != nil { m.onGetUserCredentials(context.Background(), uuid.Nil, userID) } return m.userCreds, nil } func (m *trackingMCPServerStore) CreateServer(_ context.Context, _ *store.MCPServerData) error { panic("not implemented") } func (m *trackingMCPServerStore) GetServer(_ context.Context, _ uuid.UUID) (*store.MCPServerData, error) { panic("not implemented") } func (m *trackingMCPServerStore) GetServerByName(_ context.Context, _ string) (*store.MCPServerData, error) { panic("not implemented") } func (m *trackingMCPServerStore) ListServers(_ context.Context) ([]store.MCPServerData, error) { panic("not implemented") } func (m *trackingMCPServerStore) UpdateServer(_ context.Context, _ uuid.UUID, _ map[string]any) error { panic("not implemented") } func (m *trackingMCPServerStore) DeleteServer(_ context.Context, _ uuid.UUID) error { panic("not implemented") } func (m *trackingMCPServerStore) GrantToAgent(_ context.Context, _ *store.MCPAgentGrant) error { panic("not implemented") } func (m *trackingMCPServerStore) RevokeFromAgent(_ context.Context, _, _ uuid.UUID) error { panic("not implemented") } func (m *trackingMCPServerStore) ListAgentGrants(_ context.Context, _ uuid.UUID) ([]store.MCPAgentGrant, error) { panic("not implemented") } func (m *trackingMCPServerStore) ListServerGrants(_ context.Context, _ uuid.UUID) ([]store.MCPAgentGrant, error) { panic("not implemented") } func (m *trackingMCPServerStore) GrantToUser(_ context.Context, _ *store.MCPUserGrant) error { panic("not implemented") } func (m *trackingMCPServerStore) RevokeFromUser(_ context.Context, _ uuid.UUID, _ string) error { panic("not implemented") } func (m *trackingMCPServerStore) CountAgentGrantsByServer(_ context.Context) (map[uuid.UUID]int, error) { panic("not implemented") } func (m *trackingMCPServerStore) ListAccessible(_ context.Context, _ uuid.UUID, _ string) ([]store.MCPAccessInfo, error) { panic("not implemented") } func (m *trackingMCPServerStore) CreateRequest(_ context.Context, _ *store.MCPAccessRequest) error { panic("not implemented") } func (m *trackingMCPServerStore) ListPendingRequests(_ context.Context) ([]store.MCPAccessRequest, error) { panic("not implemented") } func (m *trackingMCPServerStore) ReviewRequest(_ context.Context, _ uuid.UUID, _ bool, _, _ string) error { panic("not implemented") } func (m *trackingMCPServerStore) SetUserCredentials(_ context.Context, _ uuid.UUID, _ string, _ store.MCPUserCredentials) error { panic("not implemented") } func (m *trackingMCPServerStore) DeleteUserCredentials(_ context.Context, _ uuid.UUID, _ string) error { panic("not implemented") } func (m *trackingMCPServerStore) CacheToolDescriptions(_ context.Context, _ uuid.UUID, _ map[string]store.CachedToolInfo) error { return nil }