From 83c79d1b3c7a58ecdf51cf7938e6694a80aafd98 Mon Sep 17 00:00:00 2001 From: viettranx Date: Mon, 23 Mar 2026 22:57:39 +0700 Subject: [PATCH] feat(mcp): allow admin to set user credentials on behalf of any user Add ?user_id query param to MCP credential endpoints so system admins and tenant admin/owner can manage credentials for other users. Backend: - Add resolveTargetUserID() with permission checks (system admin, tenant admin/owner) and cross-tenant defense-in-depth - Add TenantStore dependency to MCPUserCredentialsHandler Frontend: - Add searchable Combobox user selector for admin users in the credentials dialog (visible only when canManageUsers) - Update credential API calls to pass optional userId param - Add i18n keys for admin mode (en/vi/zh) --- internal/http/mcp_user_credentials.go | 96 ++++++++++++++++--- ui/web/src/i18n/locales/en/mcp.json | 3 + ui/web/src/i18n/locales/vi/mcp.json | 3 + ui/web/src/i18n/locales/zh/mcp.json | 3 + ui/web/src/pages/mcp/hooks/use-mcp.ts | 15 +-- .../pages/mcp/mcp-user-credentials-dialog.tsx | 68 +++++++++++-- 6 files changed, 160 insertions(+), 28 deletions(-) diff --git a/internal/http/mcp_user_credentials.go b/internal/http/mcp_user_credentials.go index 3e2ddd1d..71c23edf 100644 --- a/internal/http/mcp_user_credentials.go +++ b/internal/http/mcp_user_credentials.go @@ -2,22 +2,24 @@ package http import ( "encoding/json" + "log/slog" "net/http" "github.com/google/uuid" + "github.com/nextlevelbuilder/goclaw/internal/permissions" "github.com/nextlevelbuilder/goclaw/internal/store" ) // MCPUserCredentialsHandler handles per-user MCP credential endpoints. type MCPUserCredentialsHandler struct { - store store.MCPServerStore - token string + store store.MCPServerStore + tenantStore store.TenantStore } // NewMCPUserCredentialsHandler creates a handler for MCP user credential endpoints. -func NewMCPUserCredentialsHandler(s store.MCPServerStore, token string) *MCPUserCredentialsHandler { - return &MCPUserCredentialsHandler{store: s, token: token} +func NewMCPUserCredentialsHandler(s store.MCPServerStore, ts store.TenantStore) *MCPUserCredentialsHandler { + return &MCPUserCredentialsHandler{store: s, tenantStore: ts} } // RegisterRoutes registers MCP user credential routes. @@ -28,7 +30,48 @@ func (h *MCPUserCredentialsHandler) RegisterRoutes(mux *http.ServeMux) { } func (h *MCPUserCredentialsHandler) auth(next http.HandlerFunc) http.HandlerFunc { - return requireAuth(h.token, "", next) + return requireAuth("", next) +} + +// resolveTargetUserID returns the effective user ID for credential operations. +// If ?user_id is absent or same as caller, returns callerID (self-service). +// If ?user_id targets another user, checks that caller is system admin or tenant admin/owner. +func (h *MCPUserCredentialsHandler) resolveTargetUserID(r *http.Request, callerID string) (string, int) { + targetID := r.URL.Query().Get("user_id") + if targetID == "" || targetID == callerID { + return callerID, 0 + } + + if err := store.ValidateUserID(targetID); err != nil { + return "", http.StatusBadRequest + } + + // System admin can always target another user. + role := permissions.Role(store.RoleFromContext(r.Context())) + if permissions.HasMinRole(role, permissions.RoleAdmin) { + return targetID, 0 + } + + // Tenant admin/owner can target users within their tenant. + if h.tenantStore != nil { + tid := store.TenantIDFromContext(r.Context()) + if tid != uuid.Nil { + callerTenantRole, err := h.tenantStore.GetUserRole(r.Context(), tid, callerID) + if err == nil && (callerTenantRole == store.TenantRoleOwner || callerTenantRole == store.TenantRoleAdmin) { + // Verify target user belongs to the same tenant. + if _, err := h.tenantStore.GetUserRole(r.Context(), tid, targetID); err == nil { + return targetID, 0 + } + } + } + } + + slog.Warn("security.mcp_credentials_forbidden", + "caller", callerID, + "target", targetID, + "role", string(role), + ) + return "", http.StatusForbidden } func (h *MCPUserCredentialsHandler) handleSet(w http.ResponseWriter, r *http.Request) { @@ -38,12 +81,18 @@ func (h *MCPUserCredentialsHandler) handleSet(w http.ResponseWriter, r *http.Req return } - userID := store.UserIDFromContext(r.Context()) - if userID == "" { + callerID := store.UserIDFromContext(r.Context()) + if callerID == "" { writeJSON(w, http.StatusBadRequest, map[string]string{"error": "user context required"}) return } + userID, errCode := h.resolveTargetUserID(r, callerID) + if errCode != 0 { + writeJSON(w, errCode, map[string]string{"error": httpStatusText(errCode)}) + return + } + var creds store.MCPUserCredentials if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, 1<<16)).Decode(&creds); err != nil { writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid request body"}) @@ -65,19 +114,24 @@ func (h *MCPUserCredentialsHandler) handleGet(w http.ResponseWriter, r *http.Req return } - userID := store.UserIDFromContext(r.Context()) - if userID == "" { + callerID := store.UserIDFromContext(r.Context()) + if callerID == "" { writeJSON(w, http.StatusBadRequest, map[string]string{"error": "user context required"}) return } + userID, errCode := h.resolveTargetUserID(r, callerID) + if errCode != 0 { + writeJSON(w, errCode, map[string]string{"error": httpStatusText(errCode)}) + return + } + creds, err := h.store.GetUserCredentials(r.Context(), serverID, userID) if err != nil { writeJSON(w, http.StatusOK, map[string]any{"has_credentials": false}) return } - // Return masked status (never expose actual credentials) writeJSON(w, http.StatusOK, map[string]any{ "has_credentials": true, "has_api_key": creds.APIKey != "", @@ -93,12 +147,18 @@ func (h *MCPUserCredentialsHandler) handleDelete(w http.ResponseWriter, r *http. return } - userID := store.UserIDFromContext(r.Context()) - if userID == "" { + callerID := store.UserIDFromContext(r.Context()) + if callerID == "" { writeJSON(w, http.StatusBadRequest, map[string]string{"error": "user context required"}) return } + userID, errCode := h.resolveTargetUserID(r, callerID) + if errCode != 0 { + writeJSON(w, errCode, map[string]string{"error": httpStatusText(errCode)}) + return + } + if err := h.store.DeleteUserCredentials(r.Context(), serverID, userID); err != nil { writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) return @@ -106,3 +166,15 @@ func (h *MCPUserCredentialsHandler) handleDelete(w http.ResponseWriter, r *http. writeJSON(w, http.StatusOK, map[string]string{"status": "deleted"}) } + +// httpStatusText returns a short error message for common HTTP status codes. +func httpStatusText(code int) string { + switch code { + case http.StatusBadRequest: + return "invalid user_id" + case http.StatusForbidden: + return "permission denied: admin or tenant admin required" + default: + return http.StatusText(code) + } +} diff --git a/ui/web/src/i18n/locales/en/mcp.json b/ui/web/src/i18n/locales/en/mcp.json index 695e3970..edf932c1 100644 --- a/ui/web/src/i18n/locales/en/mcp.json +++ b/ui/web/src/i18n/locales/en/mcp.json @@ -103,6 +103,9 @@ "userCredentials": { "title": "My Credentials", "description": "Set your personal credentials for this MCP server. These override the server defaults for your account only.", + "titleAdmin": "User Credentials", + "descriptionAdmin": "Set credentials for a specific user on this MCP server. These override the server defaults for the selected user only.", + "selectUser": "Select user", "apiKey": "API Key", "apiKeyPlaceholder": "Leave empty to keep current", "headers": "Headers", diff --git a/ui/web/src/i18n/locales/vi/mcp.json b/ui/web/src/i18n/locales/vi/mcp.json index 328702f1..0d7724ff 100644 --- a/ui/web/src/i18n/locales/vi/mcp.json +++ b/ui/web/src/i18n/locales/vi/mcp.json @@ -103,6 +103,9 @@ "userCredentials": { "title": "Thông tin xác thực", "description": "Đặt thông tin xác thực cá nhân của bạn cho máy chủ MCP này. Chỉ áp dụng cho tài khoản của bạn.", + "titleAdmin": "Thông tin xác thực", + "descriptionAdmin": "Đặt thông tin xác thực cho người dùng cụ thể trên máy chủ MCP này. Chúng ghi đè cài đặt mặc định của máy chủ chỉ cho người dùng được chọn.", + "selectUser": "Chọn người dùng", "apiKey": "API Key", "apiKeyPlaceholder": "Để trống để giữ nguyên", "headers": "Headers", diff --git a/ui/web/src/i18n/locales/zh/mcp.json b/ui/web/src/i18n/locales/zh/mcp.json index 8048935c..5d7d7906 100644 --- a/ui/web/src/i18n/locales/zh/mcp.json +++ b/ui/web/src/i18n/locales/zh/mcp.json @@ -103,6 +103,9 @@ "userCredentials": { "title": "我的凭证", "description": "为此 MCP 服务器设置您的个人凭证，仅对您的账户生效。", + "titleAdmin": "用户凭证", + "descriptionAdmin": "为特定用户设置此 MCP 服务器的凭证。这些凭证仅覆盖所选用户的服务器默认值。", + "selectUser": "选择用户", "apiKey": "API 密钥", "apiKeyPlaceholder": "留空以保持当前值", "headers": "请求头", diff --git a/ui/web/src/pages/mcp/hooks/use-mcp.ts b/ui/web/src/pages/mcp/hooks/use-mcp.ts index 67baa911..70ca55ad 100644 --- a/ui/web/src/pages/mcp/hooks/use-mcp.ts +++ b/ui/web/src/pages/mcp/hooks/use-mcp.ts @@ -118,22 +118,25 @@ export function useMCP() { ); const getUserCredentials = useCallback( - async (serverId: string) => { - return http.get(`/v1/mcp/servers/${serverId}/user-credentials`); + async (serverId: string, userId?: string) => { + const qs = userId ? `?user_id=${encodeURIComponent(userId)}` : ""; + return http.get(`/v1/mcp/servers/${serverId}/user-credentials${qs}`); }, [http], ); const setUserCredentials = useCallback( - async (serverId: string, creds: MCPUserCredentialInput) => { - await http.put(`/v1/mcp/servers/${serverId}/user-credentials`, creds); + async (serverId: string, creds: MCPUserCredentialInput, userId?: string) => { + const qs = userId ? `?user_id=${encodeURIComponent(userId)}` : ""; + await http.put(`/v1/mcp/servers/${serverId}/user-credentials${qs}`, creds); }, [http], ); const deleteUserCredentials = useCallback( - async (serverId: string) => { - await http.delete(`/v1/mcp/servers/${serverId}/user-credentials`); + async (serverId: string, userId?: string) => { + const qs = userId ? `?user_id=${encodeURIComponent(userId)}` : ""; + await http.delete(`/v1/mcp/servers/${serverId}/user-credentials${qs}`); }, [http], ); diff --git a/ui/web/src/pages/mcp/mcp-user-credentials-dialog.tsx b/ui/web/src/pages/mcp/mcp-user-credentials-dialog.tsx index 8fb903dd..22b88473 100644 --- a/ui/web/src/pages/mcp/mcp-user-credentials-dialog.tsx +++ b/ui/web/src/pages/mcp/mcp-user-credentials-dialog.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect } from "react"; +import { useState, useEffect, useMemo } from "react"; import { useTranslation } from "react-i18next"; import { KeyRound, Loader2 } from "lucide-react"; import { @@ -14,7 +14,11 @@ import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; import { Badge } from "@/components/ui/badge"; import { KeyValueEditor } from "@/components/shared/key-value-editor"; +import { Combobox } from "@/components/ui/combobox"; import { toast } from "@/stores/use-toast-store"; +import { useAuthStore } from "@/stores/use-auth-store"; +import { useTenants } from "@/hooks/use-tenants"; +import { useTenantUsersList } from "@/pages/contacts/hooks/use-tenant-users-list"; import i18next from "i18next"; import type { MCPServerData, MCPUserCredentialStatus, MCPUserCredentialInput } from "./hooks/use-mcp"; @@ -30,9 +34,9 @@ interface MCPUserCredentialsDialogProps { open: boolean; onOpenChange: (open: boolean) => void; server: MCPServerData; - onGetCredentials: (serverId: string) => Promise; - onSetCredentials: (serverId: string, creds: MCPUserCredentialInput) => Promise; - onDeleteCredentials: (serverId: string) => Promise; + onGetCredentials: (serverId: string, userId?: string) => Promise; + onSetCredentials: (serverId: string, creds: MCPUserCredentialInput, userId?: string) => Promise; + onDeleteCredentials: (serverId: string, userId?: string) => Promise; } export function MCPUserCredentialsDialog({ @@ -44,6 +48,18 @@ export function MCPUserCredentialsDialog({ onDeleteCredentials, }: MCPUserCredentialsDialogProps) { const { t } = useTranslation("mcp"); + const role = useAuthStore((s) => s.role); + const currentUserId = useAuthStore((s) => s.userId); + const { currentTenant } = useTenants(); + const { users } = useTenantUsersList(); + + const canManageUsers = + role === "admin" || + currentTenant?.role === "owner" || + currentTenant?.role === "admin"; + + const [selectedUserId, setSelectedUserId] = useState(currentUserId); + const [status, setStatus] = useState(null); const [loadingStatus, setLoadingStatus] = useState(false); const [saving, setSaving] = useState(false); @@ -53,6 +69,20 @@ export function MCPUserCredentialsDialog({ const [headers, setHeaders] = useState>({}); const [env, setEnv] = useState>({}); + const userOptions = useMemo( + () => + users.map((u) => ({ + value: u.user_id, + label: u.display_name || u.user_id, + })), + [users], + ); + + // Reset selected user when dialog opens + useEffect(() => { + if (open) setSelectedUserId(currentUserId); + }, [open, currentUserId]); + useEffect(() => { if (!open) return; setApiKey(""); @@ -60,11 +90,12 @@ export function MCPUserCredentialsDialog({ setEnv({}); setStatus(null); setLoadingStatus(true); - onGetCredentials(server.id) + const targetUser = canManageUsers ? selectedUserId : undefined; + onGetCredentials(server.id, targetUser) .then(setStatus) .catch(() => {}) .finally(() => setLoadingStatus(false)); - }, [open, server.id, onGetCredentials]); + }, [open, server.id, onGetCredentials, canManageUsers, selectedUserId]); const handleSave = async () => { setSaving(true); @@ -73,7 +104,8 @@ export function MCPUserCredentialsDialog({ if (apiKey.trim()) creds.api_key = apiKey.trim(); if (Object.keys(headers).length > 0) creds.headers = headers; if (Object.keys(env).length > 0) creds.env = env; - await onSetCredentials(server.id, creds); + const targetUser = canManageUsers ? selectedUserId : undefined; + await onSetCredentials(server.id, creds, targetUser); toast.success(i18next.t("mcp:userCredentials.saved")); onOpenChange(false); } catch (err) { @@ -86,7 +118,8 @@ export function MCPUserCredentialsDialog({ const handleDelete = async () => { setDeleting(true); try { - await onDeleteCredentials(server.id); + const targetUser = canManageUsers ? selectedUserId : undefined; + await onDeleteCredentials(server.id, targetUser); toast.success(i18next.t("mcp:userCredentials.deleted")); onOpenChange(false); } catch (err) { @@ -102,9 +135,11 @@ export function MCPUserCredentialsDialog({ - {t("userCredentials.title")} + {canManageUsers ? t("userCredentials.titleAdmin") : t("userCredentials.title")} - {t("userCredentials.description")} + + {canManageUsers ? t("userCredentials.descriptionAdmin") : t("userCredentials.description")} + {loadingStatus ? ( @@ -113,6 +148,19 @@ export function MCPUserCredentialsDialog({ ) : (
+ {/* User selector — admin only */} + {canManageUsers && ( +
+ + setSelectedUserId(val)} + options={userOptions} + placeholder={t("userCredentials.selectUser")} + /> +
+ )} + {/* Current status badges */} {status && (