diff --git a/backend/modules/config/config_manager.py b/backend/modules/config/config_manager.py index dce452b..f4202aa 100644 --- a/backend/modules/config/config_manager.py +++ b/backend/modules/config/config_manager.py @@ -99,6 +99,7 @@ class MCPServerConfig(BaseModel): help_email: Optional[str] = None # Contact email for help/support groups: List[str] = Field(default_factory=list) enabled: bool = True + allow_discovery: bool = False # Allow users to discover server even without access (shows overview and contact info only) command: Optional[List[str]] = None # Command to run server (for stdio servers) cwd: Optional[str] = None # Working directory for command env: Optional[Dict[str, str]] = None # Environment variables for stdio servers diff --git a/backend/modules/mcp_tools/client.py b/backend/modules/mcp_tools/client.py index 08e1c27..07747dd 100644 --- a/backend/modules/mcp_tools/client.py +++ b/backend/modules/mcp_tools/client.py @@ -8,7 +8,6 @@ from fastmcp import Client from modules.config import config_manager -from core.auth_utils import create_authorization_manager from core.utils import sanitize_for_logging from modules.config.config_manager import resolve_env_var from domain.messages.models import ToolCall, ToolResult @@ -628,6 +627,46 @@ async def get_authorized_servers(self, user_email: str, auth_check_func) -> List authorized_servers.append(server_name) return authorized_servers + async def get_discoverable_servers(self, user_email: str, auth_check_func) -> Dict[str, Dict[str, Any]]: + """Get servers that are discoverable but not authorized for the user. + + Returns a dict mapping server names to their basic info (description, author, help_email, groups). + Only includes servers with allow_discovery=true where the user lacks access. + """ + discoverable_servers = {} + for server_name, server_config in self.servers_config.items(): + if not server_config.get("enabled", True): + continue + + # Skip if discovery is not allowed + if not server_config.get("allow_discovery", False): + continue + + required_groups = server_config.get("groups", []) + + # Skip servers with no groups (they're accessible to everyone) + if not required_groups: + continue + + # Check if user is in any of the required groups + group_checks = [await auth_check_func(user_email, group) for group in required_groups] + + # Only include if user does NOT have access + if not any(group_checks): + discoverable_servers[server_name] = { + 'server': server_name, + 'description': server_config.get('description', ''), + 'author': server_config.get('author', ''), + 'short_description': server_config.get('short_description', ''), + 'help_email': server_config.get('help_email', ''), + 'groups': required_groups, + 'compliance_level': server_config.get('compliance_level'), + 'is_discoverable': True, + 'has_access': False + } + + return discoverable_servers + def get_available_tools(self) -> List[str]: """Get list of available tool names.""" available_tools = [] diff --git a/backend/routes/config_routes.py b/backend/routes/config_routes.py index ecabd6c..8a4f96c 100644 --- a/backend/routes/config_routes.py +++ b/backend/routes/config_routes.py @@ -115,6 +115,7 @@ async def get_config( tools_info = [] prompts_info = [] authorized_servers = [] + discoverable_servers_info = [] if app_settings.feature_tools_enabled: # Get MCP manager @@ -123,6 +124,9 @@ async def get_config( # Get authorized servers for the user - this filters out unauthorized servers completely authorized_servers = await mcp_manager.get_authorized_servers(current_user, is_user_in_group) + # Get discoverable servers (servers user can see but not access) + discoverable_servers = await mcp_manager.get_discoverable_servers(current_user, is_user_in_group) + # Add canvas pseudo-tool to authorized servers (available to all users) authorized_servers.append("canvas") @@ -197,6 +201,9 @@ async def get_config( 'help_email': server_config.get('help_email', ''), 'compliance_level': server_config.get('compliance_level') }) + + # Build discoverable servers info (limited information for servers user can't access) + discoverable_servers_info = list(discoverable_servers.values()) # Read help page configuration (supports new config directory layout + legacy paths) help_config = {} @@ -243,6 +250,7 @@ async def get_config( # Log what the user can see for debugging logger.info( f"User {sanitize_for_logging(current_user)} has access to {len(authorized_servers)} servers: {authorized_servers}\n" + f"User can discover {len(discoverable_servers_info)} additional servers: {[s['server'] for s in discoverable_servers_info]}\n" f"Returning {len(tools_info)} server tool groups to frontend for user {sanitize_for_logging(current_user)}" ) # Build models list with compliance levels @@ -284,6 +292,7 @@ async def get_config( "models": models_list, "tools": tools_info, # Only authorized servers are included "prompts": prompts_info, # Available prompts from authorized servers + "discoverable_servers": discoverable_servers_info, # Servers user can see but not access "data_sources": rag_data_sources, # RAG data sources for the user "rag_servers": rag_servers, # Optional richer structure for RAG UI "user": current_user, diff --git a/backend/tests/test_mcp_discoverable_servers.py b/backend/tests/test_mcp_discoverable_servers.py new file mode 100644 index 0000000..0825410 --- /dev/null +++ b/backend/tests/test_mcp_discoverable_servers.py @@ -0,0 +1,221 @@ +"""Test get_discoverable_servers functionality.""" + +import pytest +from modules.mcp_tools.client import MCPToolManager + + +@pytest.mark.asyncio +async def test_get_discoverable_servers_basic(): + """Test that get_discoverable_servers returns servers user can discover but not access.""" + + # Create a mock MCPToolManager with test server config + mcp_manager = MCPToolManager(None) + mcp_manager.servers_config = { + "public_server": { + "enabled": True, + "groups": [], + "allow_discovery": True, + "description": "Public server", + "author": "Test Team", + "help_email": "help@test.com" + }, + "admin_server": { + "enabled": True, + "groups": ["admin"], + "allow_discovery": True, + "description": "Admin only server", + "author": "Test Team", + "short_description": "Admin tools", + "help_email": "admin@test.com", + "compliance_level": "SOC2" + }, + "hidden_server": { + "enabled": True, + "groups": ["superusers"], + "allow_discovery": False, # Not discoverable + "description": "Hidden server", + "author": "Test Team", + "help_email": "hidden@test.com" + }, + "user_server": { + "enabled": True, + "groups": ["users"], + "allow_discovery": True, + "description": "User server", + "author": "Test Team", + "help_email": "users@test.com" + } + } + + # Mock async auth function - user has no group access + async def mock_auth_check(user_email: str, group: str) -> bool: + """Mock auth check that returns False for all groups.""" + return False + + # Test with user who has no access + discoverable = await mcp_manager.get_discoverable_servers("user@test.com", mock_auth_check) + + # Should include admin_server and user_server (discoverable, user lacks access) + # Should NOT include public_server (no groups required, so user has access) + # Should NOT include hidden_server (allow_discovery is False) + assert "admin_server" in discoverable + assert "user_server" in discoverable + assert "public_server" not in discoverable + assert "hidden_server" not in discoverable + + # Check that discoverable servers have the right structure + assert discoverable["admin_server"]["server"] == "admin_server" + assert discoverable["admin_server"]["description"] == "Admin only server" + assert discoverable["admin_server"]["author"] == "Test Team" + assert discoverable["admin_server"]["help_email"] == "admin@test.com" + assert discoverable["admin_server"]["groups"] == ["admin"] + assert discoverable["admin_server"]["compliance_level"] == "SOC2" + assert discoverable["admin_server"]["is_discoverable"] is True + assert discoverable["admin_server"]["has_access"] is False + + +@pytest.mark.asyncio +async def test_get_discoverable_servers_with_partial_access(): + """Test discoverable servers when user has access to some servers.""" + + mcp_manager = MCPToolManager(None) + mcp_manager.servers_config = { + "server1": { + "enabled": True, + "groups": ["users"], + "allow_discovery": True, + "description": "Server 1", + "author": "Test Team", + "help_email": "help1@test.com" + }, + "server2": { + "enabled": True, + "groups": ["admin"], + "allow_discovery": True, + "description": "Server 2", + "author": "Test Team", + "help_email": "help2@test.com" + }, + "server3": { + "enabled": True, + "groups": ["superusers"], + "allow_discovery": True, + "description": "Server 3", + "author": "Test Team", + "help_email": "help3@test.com" + } + } + + # User has access to 'users' group only + async def mock_auth_check(user_email: str, group: str) -> bool: + return group == "users" + + discoverable = await mcp_manager.get_discoverable_servers("user@test.com", mock_auth_check) + + # Should include server2 and server3 (discoverable, user lacks access) + # Should NOT include server1 (user has access via 'users' group) + assert "server1" not in discoverable + assert "server2" in discoverable + assert "server3" in discoverable + + +@pytest.mark.asyncio +async def test_get_discoverable_servers_disabled_servers(): + """Test that disabled servers are not discoverable.""" + + mcp_manager = MCPToolManager(None) + mcp_manager.servers_config = { + "enabled_server": { + "enabled": True, + "groups": ["admin"], + "allow_discovery": True, + "description": "Enabled server", + "author": "Test Team", + "help_email": "help@test.com" + }, + "disabled_server": { + "enabled": False, + "groups": ["admin"], + "allow_discovery": True, + "description": "Disabled server", + "author": "Test Team", + "help_email": "disabled@test.com" + } + } + + async def mock_auth_check(user_email: str, group: str) -> bool: + return False + + discoverable = await mcp_manager.get_discoverable_servers("user@test.com", mock_auth_check) + + # Should include only enabled_server + # Should NOT include disabled_server + assert "enabled_server" in discoverable + assert "disabled_server" not in discoverable + + +@pytest.mark.asyncio +async def test_get_discoverable_servers_empty(): + """Test when no servers are discoverable.""" + + mcp_manager = MCPToolManager(None) + mcp_manager.servers_config = { + "server1": { + "enabled": True, + "groups": ["admin"], + "allow_discovery": False, # Not discoverable + "description": "Server 1", + "author": "Test Team", + "help_email": "help@test.com" + }, + "server2": { + "enabled": True, + "groups": [], # No groups required + "allow_discovery": True, + "description": "Server 2", + "author": "Test Team", + "help_email": "help2@test.com" + } + } + + async def mock_auth_check(user_email: str, group: str) -> bool: + return False + + discoverable = await mcp_manager.get_discoverable_servers("user@test.com", mock_auth_check) + + # Should be empty - server1 is not discoverable, server2 has no groups + assert discoverable == {} + + +@pytest.mark.asyncio +async def test_get_discoverable_servers_all_access(): + """Test when user has access to all servers.""" + + mcp_manager = MCPToolManager(None) + mcp_manager.servers_config = { + "server1": { + "enabled": True, + "groups": ["admin"], + "allow_discovery": True, + "description": "Server 1", + "author": "Test Team", + "help_email": "help@test.com" + }, + "server2": { + "enabled": True, + "groups": ["users"], + "allow_discovery": True, + "description": "Server 2", + "author": "Test Team", + "help_email": "help2@test.com" + } + } + + # User has access to all groups + async def mock_auth_check(user_email: str, group: str) -> bool: + return True + + discoverable = await mcp_manager.get_discoverable_servers("user@test.com", mock_auth_check) + + # Should be empty - user has access to all servers + assert discoverable == {} diff --git a/docs/IMPLEMENTATION_SUMMARY.md b/docs/IMPLEMENTATION_SUMMARY.md new file mode 100644 index 0000000..0c02348 --- /dev/null +++ b/docs/IMPLEMENTATION_SUMMARY.md @@ -0,0 +1,151 @@ +# MCP Server Discoverability - Implementation Summary + +## Feature Overview +Implemented the ability for MCP servers to be discoverable by users even when they lack access permissions. This allows users to see available capabilities and request access through proper channels. + +## Test Results + +### Backend Tests +All 5 new tests pass successfully: +- ✅ `test_get_discoverable_servers_basic` - Basic discoverability logic +- ✅ `test_get_discoverable_servers_with_partial_access` - Partial access scenarios +- ✅ `test_get_discoverable_servers_disabled_servers` - Disabled server handling +- ✅ `test_get_discoverable_servers_empty` - Empty result scenarios +- ✅ `test_get_discoverable_servers_all_access` - Full access scenarios + +All existing authorization tests continue to pass: +- ✅ `test_get_authorized_servers_with_async_auth` +- ✅ `test_get_authorized_servers_with_multiple_groups` +- ✅ `test_get_authorized_servers_no_access` + +### Code Quality +- ✅ Backend linting: All checks passed (ruff) +- ✅ Security scan: 0 vulnerabilities (CodeQL) +- ✅ Code review: Feedback addressed + +### Manual Testing +Example scenario with test configuration: +- User in "users" group can access: calculator +- User can discover but not access: restricted_server (requires "admin" group) +- User cannot see: hidden_server (allow_discovery=false) + +## Example Configuration + +```json +{ + "restricted_server": { + "command": ["python", "mcp/calculator/main.py"], + "cwd": "backend", + "groups": ["admin"], + "allow_discovery": true, + "description": "Admin-only calculator with advanced features", + "author": "Admin Team", + "short_description": "Advanced calculator", + "help_email": "admin-help@chatui.example.com", + "compliance_level": "SOC2" + } +} +``` + +## API Response Structure + +```json +{ + "tools": [...], + "discoverable_servers": [ + { + "server": "restricted_server", + "description": "Admin-only calculator with advanced features", + "author": "Admin Team", + "short_description": "Advanced calculator", + "help_email": "admin-help@chatui.example.com", + "groups": ["admin"], + "compliance_level": "SOC2", + "is_discoverable": true, + "has_access": false + } + ] +} +``` + +## UI Behavior + +### Accessible Servers +- Normal appearance +- Clickable/selectable +- Shows all tools and prompts +- Checkbox for selection + +### Discoverable Servers (User Lacks Access) +- Grayed out (opacity: 75%) +- Lock icon instead of checkbox +- "No Access" badge (orange) +- Shows: name, description, author, required groups +- Hides: tools, prompts, counts +- "Request Access" link (if help_email provided) +- Non-clickable +- Tooltip explains how to request access + +### Hidden Servers +- Completely invisible +- Not in marketplace at all + +## Security Analysis + +### What's Protected +✅ Tools and prompts remain hidden +✅ Server functionality is inaccessible +✅ Actual capabilities are not exposed +✅ Opt-in design (default: false) +✅ No vulnerabilities detected by CodeQL + +### What's Exposed (Intentionally) +- Server name +- Description and short description +- Author information +- Help/contact email +- Required groups for access +- Compliance level (if configured) + +This metadata exposure is intentional to: +1. Enable self-service access requests +2. Increase capability awareness +3. Reduce admin support burden +4. Maintain security through proper access control + +## Files Changed + +### Backend +- `backend/modules/config/config_manager.py` - Added allow_discovery field +- `backend/modules/mcp_tools/client.py` - Added get_discoverable_servers method +- `backend/routes/config_routes.py` - Updated /api/config endpoint +- `backend/tests/test_mcp_discoverable_servers.py` - New test file (221 lines) + +### Frontend +- `frontend/src/hooks/chat/useChatConfig.js` - Added discoverableServers state +- `frontend/src/contexts/ChatContext.jsx` - Exposed discoverableServers +- `frontend/src/components/MarketplacePanel.jsx` - Updated UI for discoverable servers + +### Documentation +- `docs/mcp-server-discoverability.md` - Complete feature documentation + +## Deployment Notes + +1. Feature is **opt-in** - servers must explicitly set `allow_discovery: true` +2. Default behavior unchanged - servers without this field remain hidden to unauthorized users +3. No breaking changes - backward compatible with existing configurations +4. No database migrations needed +5. Configuration takes effect on server restart/config reload + +## Future Enhancements (Not in Scope) + +Potential improvements for future consideration: +- Admin UI for managing discoverable status +- Analytics on access request patterns +- Automated access request workflow +- User notification when access is granted +- Temporary access/trial periods + +## Conclusion + +The MCP server discoverability feature is fully implemented, tested, and documented. It provides a secure, user-friendly way for users to discover available capabilities and request access through proper channels, while maintaining strict security controls on actual functionality. diff --git a/docs/mcp-server-discoverability.md b/docs/mcp-server-discoverability.md new file mode 100644 index 0000000..160c3c5 --- /dev/null +++ b/docs/mcp-server-discoverability.md @@ -0,0 +1,107 @@ +# MCP Server Discoverability Feature + +## Overview + +The `allow_discovery` feature enables MCP servers to be visible in the marketplace to users even if they don't have access. This allows users to: +- See that a capability exists +- View server overview information (description, author, short description) +- Access contact information (help_email) to request access +- Know which groups are required for access + +## Configuration + +To make an MCP server discoverable, add `"allow_discovery": true` to its configuration in `mcp.json`: + +```json +{ + "restricted_server": { + "command": ["python", "mcp/restricted_server/main.py"], + "cwd": "backend", + "groups": ["admin", "privileged_users"], + "allow_discovery": true, + "description": "Advanced analytics server with restricted access", + "author": "Analytics Team", + "short_description": "Advanced data analysis tools", + "help_email": "analytics-access@example.com", + "compliance_level": "SOC2" + } +} +``` + +### Configuration Fields + +- **allow_discovery** (boolean, default: `false`): When `true`, users without access can see the server in the marketplace with limited information +- **description** (string): Full description shown to all users who can discover the server +- **author** (string): Author/team name shown to discoverable users +- **short_description** (string): Brief description shown in marketplace cards +- **help_email** (string): Contact email for requesting access (link text changes to "Request Access" for discoverable servers) +- **groups** (array): Required groups for access (shown to users who can discover but not access the server) + +## Behavior + +### For Users with Access +- Server appears normally in the marketplace +- Can select/enable the server +- See all tools and prompts +- Can use the server's functionality + +### For Users without Access (when allow_discovery=true) +- Server appears in the marketplace with: + - Lock icon and "No Access" badge + - Server name, description, author + - Required groups list + - "Request Access" link (if help_email is provided) +- Cannot select or enable the server +- Tools and prompts are hidden +- Server is grayed out and non-clickable + +### For Users without Access (when allow_discovery=false or not set) +- Server is completely hidden from the marketplace +- User has no indication the server exists + +## Use Cases + +1. **Access Request Workflow**: Users can discover available capabilities and contact the appropriate team to request access +2. **Capability Awareness**: Organizations can make users aware of available tools without granting blanket access +3. **Self-Service Discovery**: Users can explore what's available and understand requirements for access + +## Security Considerations + +- Only server metadata is exposed to unauthorized users (description, author, contact) +- Tools, prompts, and actual functionality remain hidden +- Server remains completely inaccessible without proper group membership +- The feature is opt-in (default: false) to maintain security by default + +## API Response + +The `/api/config` endpoint returns discoverable servers in a separate `discoverable_servers` array: + +```json +{ + "tools": [...], + "prompts": [...], + "discoverable_servers": [ + { + "server": "restricted_server", + "description": "Advanced analytics server with restricted access", + "author": "Analytics Team", + "short_description": "Advanced data analysis tools", + "help_email": "analytics-access@example.com", + "groups": ["admin", "privileged_users"], + "compliance_level": "SOC2", + "is_discoverable": true, + "has_access": false + } + ] +} +``` + +## Frontend Display + +In the marketplace panel, discoverable servers are displayed with: +- Grayed out appearance (reduced opacity) +- Lock icon instead of selection checkbox +- "No Access" badge in orange +- Required groups information +- "Request Access" mailto link (if help_email provided) +- No tool/prompt preview diff --git a/frontend/src/components/MarketplacePanel.jsx b/frontend/src/components/MarketplacePanel.jsx index 903e615..47076b4 100644 --- a/frontend/src/components/MarketplacePanel.jsx +++ b/frontend/src/components/MarketplacePanel.jsx @@ -1,5 +1,5 @@ import { useNavigate } from 'react-router-dom' -import { ArrowLeft, Check, X, Search } from 'lucide-react' +import { ArrowLeft, Check, X, Search, Lock } from 'lucide-react' import { useState } from 'react' import { useChat } from '../contexts/ChatContext' import { useMarketplace } from '../contexts/MarketplaceContext' @@ -7,7 +7,7 @@ import { useMarketplace } from '../contexts/MarketplaceContext' const MarketplacePanel = () => { const [searchTerm, setSearchTerm] = useState('') const navigate = useNavigate() - const { tools, prompts } = useChat() + const { tools, prompts, discoverableServers } = useChat() const { selectedServers, toggleServer, @@ -31,7 +31,9 @@ const MarketplacePanel = () => { tools: toolServer.tools || [], tool_count: toolServer.tool_count || 0, prompts: [], - prompt_count: 0 + prompt_count: 0, + has_access: true, + is_discoverable: false } } }) @@ -49,7 +51,9 @@ const MarketplacePanel = () => { tools: [], tool_count: 0, prompts: promptServer.prompts || [], - prompt_count: promptServer.prompt_count || 0 + prompt_count: promptServer.prompt_count || 0, + has_access: true, + is_discoverable: false } } else { allServers[promptServer.server].prompts = promptServer.prompts || [] @@ -57,6 +61,30 @@ const MarketplacePanel = () => { } }) + // Add discoverable servers (servers user can see but not access) + if (discoverableServers && Array.isArray(discoverableServers)) { + discoverableServers.forEach(discoverableServer => { + if (!allServers[discoverableServer.server]) { + allServers[discoverableServer.server] = { + server: discoverableServer.server, + description: discoverableServer.description, + is_exclusive: false, + author: discoverableServer.author, + short_description: discoverableServer.short_description, + help_email: discoverableServer.help_email, + groups: discoverableServer.groups, + compliance_level: discoverableServer.compliance_level, + tools: [], + tool_count: 0, + prompts: [], + prompt_count: 0, + has_access: false, + is_discoverable: true + } + } + }) + } + const serverList = Object.values(allServers) // Filter servers based on search term @@ -168,34 +196,49 @@ const MarketplacePanel = () => {