diff --git a/.claude/memory/MEMORY.md b/.claude/memory/MEMORY.md index 9451a0f36..62343521b 100644 --- a/.claude/memory/MEMORY.md +++ b/.claude/memory/MEMORY.md @@ -16,6 +16,9 @@ - Key point: UDPLink only passes through jwt_token, ultimately used by curl command inside Web Wireshark container to authenticate with GNS3 capture stream API - **[Xpra HTML5 Client](./xpra-html5-client.md)** - Xpra HTML5 client menu control parameters for customizing the web interface +### RBAC & User Isolation +- **[RBAC User Isolation Design](./rbac-user-isolation-design.md)** — Three-step permission check design: ACE batch check → created_by filtering → resource pools + ### Appliance Management - **[GNS3 Appliance Loading](./gns3-appliance-loading.md)** - How GNS3 loads appliance files from builtin and custom directories with priority rules diff --git a/.claude/memory/rbac-user-isolation-design.md b/.claude/memory/rbac-user-isolation-design.md new file mode 100644 index 000000000..630e3c762 --- /dev/null +++ b/.claude/memory/rbac-user-isolation-design.md @@ -0,0 +1,231 @@ +--- +name: rbac-user-isolation-design +description: GNS3 RBAC user isolation design and implementation thought process +metadata: + type: project +--- + +## RBAC User Isolation Design Summary + +### Core Problem +GNS3 3.0 has a complete RBAC framework (ACE + Role + Privilege), but lacks user isolation implementation, causing users to see resources they shouldn't have access to. + +### Design Conflict +Traditional RBAC's **path permission model** fundamentally conflicts with **user data isolation**: +- **Path permission model**: Controls which paths users can access (e.g., `/projects`) +- **User data isolation**: Controls which specific resources users can access (e.g., alice's project vs bob's project) + +### Final Implementation Approach + +#### Three-Step Permission Check Logic + +```python +# Step 1: Batch ACE + resource pool check (3 DB queries regardless of project count) +direct_ace_ids, pool_accessible_ids = await rbac_repo.get_accessible_project_ids( + current_user.user_id, "Project.Audit", all_project_ids +) + +# Step 2: Filter direct ACE projects by created_by (user's own projects) +# Direct project sharing is only available through resource pools +for p in controller.projects.values(): + if p.id in direct_ace_ids and p.created_by == current_user.username: + projects.append(p.asdict()) + +# Step 3: Resource pool projects (no created_by filter) +for p in controller.projects.values(): + if p.id in pool_accessible_ids: + projects.append(p.asdict()) +``` + +##### Super Admin Bypass +```python +if current_user.is_superadmin: + return [p.asdict() for p in controller.projects.values()] +``` +Super admins skip all three steps and see every project. + +##### seen_project_ids Deduplication +A simple `seen_project_ids` set prevents the same project from appearing twice when it exists in both direct ACE and pool results. This is a lightweight dedup, not the complex blocking mechanism from earlier rejected designs. + +### Key Design Decisions + +#### 1. ACE vs created_by Relationship +- **ACE for basic access control**: Whether user can access the system +- **created_by for user isolation**: Which specific resources user can access +- **Step 2 filtering is critical**: Even with broad ACE, created_by filtering ensures user isolation + +#### 2. Project Sharing Mechanism +- **Project sharing only through resource pools**: Cannot configure ACE directly for specific projects +- **Avoids complexity of direct ACE sharing**: Prevents permission configuration chaos + +#### 3. Broad ACE Fault Tolerance +``` +Even with configuration: ACE: Users group + User role + "/" + propagate: True +Result: Users still only see projects they created +Reason: Step 2 created_by filtering removes other users' projects +``` + +### RbacRepository.get_accessible_project_ids() + +The core batch-check method in `gns3server/db/repositories/rbac.py` uses 3 DB queries: + +1. **User ACEs**: Direct ACE entries matching user + privilege +2. **Group ACEs**: Group-level ACE entries via `UserGroup` membership +3. **All resources with pool memberships**: Preloads all resources and their pool relationships + +Then it computes two sets: +- **`direct_ace_ids`**: Project paths matching user or group ACEs (path-based check) +- **`pool_accessible_ids`**: Projects in pools the user/group can access (pool-based check) + +Pool IDs are precomputed into a `pool_id -> set(project_ids)` map for O(1) lookup. + +### Problems Solved + +#### Problem 1: Missing User Isolation +- **Issue**: All users can see all projects +- **Solution**: Filter by created_by to implement user isolation + +#### Problem 2: Broad ACE Breaking Isolation +- **Issue**: `path: "/" + propagate: true` breaks user isolation +- **Solution**: Step 2 created_by filtering ensures user isolation even with broad ACE + +#### Problem 3: Permission Check Order Conflicts +- **Issue**: seen mechanism blocks subsequent checks (from earlier design phases) +- **Solution**: Simple pipeline-style filtering with lightweight dedup set + +### Technical Details + +#### API Layer Permission Check +```python +@router.get("/projects") # Note: no has_privilege decorator +async def get_projects(current_user=..., rbac_repo=...): + # Permission checks in business logic +``` + +#### Duplicate Prevention +- Simple `seen_project_ids` set for deduplication between direct ACE and pool results +- Not the complex blocking mechanism from earlier phases + +#### Performance Considerations +- Super admin path: O(1) — returns all projects directly +- Regular user path: 3 fixed DB queries regardless of project count +- Path-based ACE check: O(n * m) in worst case, where n = projects, m = ACE entries +- Pool lookup: O(1) via precomputed pool->project map + +### Use Cases + +#### Scenario 1: Personal Use +``` +alice creates project → alice only sees alice's projects ✅ +bob creates project → bob only sees bob's projects ✅ +No extra configuration needed, automatic user isolation +``` + +#### Scenario 2: Team Collaboration +``` +Admin creates resource pool → adds projects → team members can access +alice creates project → alice still sees her own projects ✅ +Team sharing + user isolation coexist +``` + +#### Scenario 3: Broad ACE Configuration +``` +ACE: Users group + "/" + propagate: True +alice still only sees alice's projects ✅ +User isolation unaffected by ACE configuration +``` + +### Design Principles + +#### 1. Separation of Concerns +- **Basic access permission**: Controlled by ACE +- **Data ownership**: Controlled by created_by +- **Team sharing**: Controlled by resource pools + +#### 2. Defensive Design +- User isolation remains effective even with improper ACE configuration +- Secure by default: users only see their own resources + +#### 3. Simplicity +- Avoid complex seen mechanisms and mutual exclusion logic +- Clear pipeline-style check order + +### Relationship with Original ACE System + +#### Preserved Components +- ✅ ACE framework (permission check mechanism) +- ✅ Role and privilege definitions +- ✅ Resource pool functionality + +#### Improved Components +- ✅ Added user isolation (created_by filtering) +- ✅ Clarified project sharing mechanism (only through resource pools) +- ✅ Simplified permission check logic + +#### Removed Components +- ❌ Removed `/projects` path privilege check on `get_projects` route +- ❌ Removed the old complex `seen_project_ids` blocking mechanism +- ❌ Avoided "can see all non-pool projects" privilege leak + +### Implementation Details +- **Main modification**: `gns3server/api/routes/controller/projects.py` +- **Function**: `get_projects()` +- **Batch check method**: `gns3server/db/repositories/rbac.py::RbacRepository.get_accessible_project_ids()` +- **Privilege dependency**: `gns3server/api/routes/controller/dependencies/rbac.py::has_privilege()` +- **Branch**: `feature/simple-user-isolation` +- **Base branch**: `master` + +### Key Commits +1. Implemented basic created_by filtering +2. Implemented three-layer permission check (with logic issues) +3. Fixed to correct three-step check logic + +### Design Evolution Process + +#### Phase 1: Simple created_by Filtering +```python +user_projects = [p for p in all_projects() if p.created_by == user.username] +``` +**Issue**: Didn't integrate with ACE system + +#### Phase 2: Three-Layer Check (Wrong Version) +```python +# Used complex seen_project_ids blocking mechanism +# Issue: Broad ACE breaks user isolation +``` + +#### Phase 3: Three-Step Check (Correct Version) +```python +# Step 1: Batch ACE check via get_accessible_project_ids() +# Step 2: Filter direct_ace_ids by created_by +# Step 3: Resource pool projects (no created_by filter) +``` +**Solution**: User isolation works even with broad ACE + +### Relationship with RBAC Roadmap +This implementation addresses items mentioned in the roadmap: +- **Phase 1 (MVP)**: Basic project isolation implementation +- **Auto-ACE on create**: Still needs to be implemented separately +- **Template isolation**: Can use same design pattern + +### Unsolved Issues + +1. **Auto-ACE on project creation**: Part of roadmap Phase 1 — `create_project()` sets `created_by` but doesn't create ACE entries +2. **Template and image isolation**: Can apply same design pattern +3. **Default ACE configuration**: Need reasonable default permissions for Users group + +### Design Limitations + +1. **Project sharing only through resource pools**: No direct ACE configuration for sharing +2. **ACE configuration required**: Users need basic ACE to access system +3. **Performance considerations**: ACE check queries database for each project path + +### Future Improvement Directions + +1. **Auto-create ACE**: Automatically add ACE for creator when creating projects +2. **Default ACE strategy**: Configure reasonable default permissions for Users group +3. **Performance optimization**: Cache ACE check results to reduce database queries + +This design achieves effective user isolation while maintaining RBAC system integrity, and solves the problem of broad ACE configurations breaking isolation. + +**Key insight**: The Step 2 created_by filtering is the critical innovation that allows ACE and user isolation to coexist properly. diff --git a/docs/gns3-copilot/implemented/ai-assistant-overview.en.md b/docs/gns3-copilot/implemented/ai-assistant-overview.en.md new file mode 100644 index 000000000..b01c1d20f --- /dev/null +++ b/docs/gns3-copilot/implemented/ai-assistant-overview.en.md @@ -0,0 +1,161 @@ + + +# GNS3-Copilot AI Assistant Overview + +## Overall Architecture + +```mermaid +flowchart TB + subgraph "Client" + A["Web UI"] --> B["SSE Streaming"] + end + + subgraph "FastAPI Route Layer" + B --> C["POST /chat/stream\nPOST /chat/inject"] + C --> D["Auth + LLM Config Loading\nSet ContextVars"] + end + + subgraph "AgentService (Project-level)" + D --> E["LangGraph Agent\nStateGraph"] + E --> F["SQLite Checkpointer\ncopilot_checkpoints.db"] + end + + subgraph "LangGraph Workflow" + E --> G["llm_call node\nmodel invocation"] + E --> H["tool_node\ntool execution"] + E --> I["title_generator_node\nauto title"] + E --> J["abort_handler_node\ninterrupt handling"] + end + + subgraph "Three Copilot Modes" + G --> K["teaching_assistant\ndiagnostic read-only"] + G --> L["lab_automation_assistant\nfull control"] + G --> M["troubleshooting_injection\nfault injection"] + end + + subgraph "LLM Config System" + D --> N["User configs\nGroup config inheritance\nAPI key encryption"] + end +``` + +## API Endpoints + +| Endpoint | Function | +|---|---| +| `POST /v3/projects/{pid}/chat/stream` | Streaming conversation (SSE), supports three copilot modes | +| `POST /v3/projects/{pid}/chat/inject` | Fault injection entry, auto-switches to `troubleshooting_injection` mode | +| `GET /v3/projects/{pid}/chat/sessions` | List sessions (supports filtering, pagination) | +| `DELETE /v3/projects/{pid}/chat/sessions/{sid}` | Delete session | +| `PATCH /v3/projects/{pid}/chat/sessions/{sid}` | Update session (rename, pin) | +| `POST /v3/projects/{pid}/chat/sessions/{sid}/abort` | Abort an active session | + +## LangGraph Agent Workflow + +```mermaid +sequenceDiagram + participant U as User + participant API as FastAPI + participant AS as AgentService + participant LLM as LLM Node + participant Tool as Tool Node + participant TGen as Title Node + + U->>API: send message + API->>AS: stream_chat() + AS->>AS: set ContextVars
(jwt_token, llm_config) + + Note over AS,LLM: llm_call node + AS->>LLM: invoke pre-compiled model + LLM->>LLM: pre_model_hook
inject topology + trim context + LLM-->>AS: AI reply (may include tool_calls) + + opt has tool calls + AS->>Tool: execute tools + Tool-->>AS: tool results + AS->>LLM: continue LLM call + end + + opt first turn and no title + AS->>TGen: auto-generate title + TGen-->>AS: session title + end + + AS-->>API: SSE streaming response + API-->>U: stream output +``` + +## Three Copilot Modes + +### Mode Comparison + +| Mode | Tool Scope | Use Case | +|---|---|---| +| `teaching_assistant` (default) | Diagnostic read-only + packet analysis + node management | Teaching demos, troubleshooting guidance | +| `lab_automation_assistant` | All tools (including config changes) | Lab automation, device configuration | +| `troubleshooting_injection` | Fault injection tool set | Troubleshooting practice, fault simulation | + +### Tool Binding Details + +| Tool | teaching_assistant | lab_automation_assistant | troubleshooting_injection | +|---|---|---|---| +| `GNS3TemplateTool` get templates | ✓ | ✓ | | +| `GNS3CreateNodeTool` create nodes | ✓ | ✓ | | +| `GNS3LinkTool` create links | ✓ | ✓ | | +| `GNS3StartNodeTool` start nodes | ✓ | ✓ | | +| `GNS3UpdateNodeNameTool` rename | ✓ | ✓ | | +| `GNS3StopNodeTool` stop nodes | | ✓ | | +| `GNS3SuspendNodeTool` suspend nodes | | ✓ | | +| `ExecuteMultipleDeviceCommands` read-only commands | ✓ | ✓ | ✓ | +| `ExecuteMultipleDeviceConfigCommands` config commands | | ✓ | ✓ | +| `VPCSCommands` VPCS commands | | ✓ | | +| `PacketAnalysisTool` live packet analysis | ✓ | ✓ | | +| `PacketAnalysisSkillsTool` protocol knowledge | ✓ | ✓ | | +| `DeviceSkillsTool` device skills | ✓ | ✓ | | +| `GNS3PacketFilterTool` link filters | | | ✓ | +| `InjectionSkillsTool` fault injection skills | | | ✓ | +| `GNS3TopologyTool` topology info | | | ✓ | + +The mode is selected in the `llm_call` node via `copilot_mode`, which picks the corresponding tool list and binds it to the LLM model instance through `create_base_model_with_tools(mode_tools, llm_config)`. + +## Context Window Management + +```mermaid +flowchart LR + A["LLM call triggered"] --> B["pre_model_hook"] + B --> C["Inject topology\ninto System Prompt"] + B --> D["Estimate tool definition\ntoken cost"] + B --> E["trim_messages\nby strategy"] + E --> F["conservative 60%\nbalanced 75%\naggressive 85%"] + F --> G["Invoke LLM"] +``` + +- Accurate token counting via tiktoken (`cl100k_base`) +- Three trimming strategies: conservative / balanced / aggressive +- Auto-injects `{{topology_info}}` into System Prompt + +## Session Management + +- Per-project independent SQLite database (`gns3-copilot/copilot_checkpoints.db`) +- Supports pin, rename, delete, history query +- Auto-records token usage, message count, LLM call count + +## LLM Config System + +| Feature | Description | +|---|---| +| User-level configs | Each user can independently configure provider / model / api_key | +| Group inheritance | Users auto-inherit group config when no personal config is set | +| API key encryption | Auto-encrypted at database storage | +| Optimistic locking | `version` field prevents concurrent modification conflicts | + +## Key Design Points + +1. **Project-level Isolation** — Each GNS3 project has its own Agent instance and SQLite storage +2. **ContextVars Safe Passing** — JWT token, API key exist only in memory, auto-cleared when request ends +3. **LangGraph StateGraph** — Custom nodes + conditional edges, supports ReAct loop and recursion limits +4. **SSE Streaming** — Real-time push of content / tool_call / tool_start / tool_end / error / done events +5. **Hot Reload** — System Prompt, Skills, Protocols all support runtime reload +6. **Mode-based Tool Sets** — Three copilot modes bind different tools, safely isolated by scenario diff --git a/docs/gns3-copilot/implemented/command-security.md b/docs/gns3-copilot/implemented/command-security.md index c1f646ad2..1e84b2f7a 100644 --- a/docs/gns3-copilot/implemented/command-security.md +++ b/docs/gns3-copilot/implemented/command-security.md @@ -86,7 +86,7 @@ flowchart TD ### Forbidden Commands Configuration -The forbidden commands list is loaded from the external [GNS3-Skills](https://github.com/yueguobin/GNS3-Skills) repository at `config/forbidden_commands.txt`. +The forbidden commands list is loaded from the external [GNS3-Skills](https://github.com/gns3/gns3-skills) repository at `config/forbidden_commands.txt`. **Format:** - One command pattern per line @@ -168,7 +168,7 @@ Applies to any command with embedded newlines: `banner`, multi-line ACLs, route- ### Customizing Forbidden Commands -Edit `config/forbidden_commands.txt` in the [GNS3-Skills repository](https://github.com/yueguobin/GNS3-Skills) and push the changes, then call `POST /copilot/reload/skills` to apply them without restarting the server. +Edit `config/forbidden_commands.txt` in the [GNS3-Skills repository](https://github.com/gns3/gns3-skills) and push the changes, then call `POST /copilot/reload/skills` to apply them without restarting the server. ## Implementation Verification diff --git a/docs/gns3-copilot/implemented/fault-injection-overview.en.md b/docs/gns3-copilot/implemented/fault-injection-overview.en.md new file mode 100644 index 000000000..00a0acfac --- /dev/null +++ b/docs/gns3-copilot/implemented/fault-injection-overview.en.md @@ -0,0 +1,104 @@ + + +# GNS3-Copilot Fault Injection Overview + +## Core Flow + +```mermaid +flowchart TB + subgraph "① API Trigger & Mode Switch" + A["POST /chat/inject\nUser requests fault injection"] --> B["Verify project is opened"] + B --> C["Set copilot_mode =\ntroubleshooting_injection"] + C --> D["Start Agent\nwith fault injection tool set"] + end + + subgraph "② Topology Analysis & Fault Selection" + D --> E["GNS3TopologyTool\nget topology info"] + E --> F["ExecuteMultipleDeviceCommands\nget device configs"] + F --> G["InjectionSkillsTool\nquery available fault types"] + G --> H{"Injection Skills Repository\ngns3/gns3-skills"} + H --> I["Return matching fault definitions\nwith config injection commands"] + end + + subgraph "③ Fault Injection" + I --> J["Choose injection method"] + J --> K["ExecuteMultipleDeviceConfigCommands\ninject config changes"] + J --> L["GNS3PacketFilterTool\ninject link-layer faults"] + end + + subgraph "④ Result Confirmation" + K --> M["Verify fault is active"] + L --> M + M --> N["Document fault details\nincluding restore commands"] + end +``` + +## Tool Overview + +| Tool | Source File | Purpose | Available Modes | +|---|---|---|---| +| `InjectionSkillsTool` | `registry.py` (skills module) | Query protocol-level fault definitions (config change commands) | troubleshooting_injection | +| `GNS3PacketFilterTool` | `gns3_packet_filter.py` | Link-layer fault injection (delay, loss, corruption, BPF) | troubleshooting_injection | +| `ExecuteMultipleDeviceConfigCommands` | `config_tools_nornir.py` | Batch device config changes | troubleshooting_injection | +| `ExecuteMultipleDeviceCommands` | `display_tools_nornir.py` | Read device configurations (read-only) | troubleshooting_injection | +| `GNS3TopologyTool` | `gns3_client` | Get project topology information | troubleshooting_injection | + +## Fault Injection API + +| Endpoint | Function | +|---|---| +| `POST /v3/projects/{pid}/chat/inject` | Trigger fault injection, sets `troubleshooting_injection` mode then starts Agent | + +**Prerequisite**: Project must be in `opened` status, otherwise returns 403. + +## GNS3PacketFilterTool Link Filters + +| Filter Type | Function | Parameters | +|---|---|---| +| `delay` | Latency + jitter | `[latency(0-32767), jitter(0-32767)]` | +| `packet_loss` | Packet loss percentage | `[chance(0-100)]` | +| `corrupt` | Packet corruption percentage | `[chance(0-100)]` | +| `frequency_drop` | Drop every Nth packet | `[frequency(-1~32767)]` | +| `bpf` | Berkeley Packet Filter | expression text | + +## Agent Workflow (LangGraph) + +```mermaid +sequenceDiagram + participant U as User + participant API as POST /chat/inject + participant LLM as LLM Node + participant Topo as GNS3TopologyTool + participant DC as ExecuteMultipleDeviceCommands + participant CC as ExecuteMultipleDeviceConfigCommands + participant Skill as InjectionSkillsTool + participant Filter as GNS3PacketFilterTool + + U->>API: Inject an OSPF fault + API->>LLM: set mode=troubleshooting_injection + LLM->>Topo: get topology + Topo-->>LLM: topology info + LLM->>DC: read device configs + DC-->>LLM: running configs + LLM->>Skill: list context=["ospf"] + Skill-->>LLM: matching fault types + LLM->>Skill: get device_type=injection_ospf + Skill-->>LLM: fault definition + injection commands + LLM->>CC: execute config injection + CC-->>LLM: injection result + LLM->>Filter: set filters={delay:[200,50]} + Filter-->>LLM: link delay injected successfully + LLM-->>U: Fault injected, restore commands included +``` + +## Key Design Points + +1. **Dedicated API Endpoint** — `POST /chat/inject` is the dedicated entry point, automatically switching to `troubleshooting_injection` mode +2. **LLM-driven Fault Selection** — The LLM analyzes the topology then queries matching faults via `InjectionSkillsTool`; no hardcoded fault scenarios +3. **Dual-Layer Injection** — Device-level config changes + link-level network impairment, covering complete troubleshooting scenarios +4. **Fully Reversible** — Every injection includes restore commands; link filters can be cleared with `action: clear` +5. **Safety First** — BPF syntax is pre-validated via tshark; config commands are restricted by `command_filter` +6. **Context Filtering** — `InjectionSkillsTool` requires a `context` parameter, returning only faults matching the topology protocols diff --git a/docs/gns3-copilot/implemented/fault-injection.md b/docs/gns3-copilot/implemented/fault-injection.md index 73cc4d165..43774b991 100644 --- a/docs/gns3-copilot/implemented/fault-injection.md +++ b/docs/gns3-copilot/implemented/fault-injection.md @@ -121,7 +121,7 @@ sequenceDiagram ## Injection Skills Repository -Skills are organized by protocol/category in the external [GNS3-Skills](https://github.com/yueguobin/GNS3-Skills) repository: +Skills are organized by protocol/category in the external [GNS3-Skills](https://github.com/gns3/gns3-skills) repository: | Category | File | Example Issues | |----------|------|----------------| diff --git a/docs/gns3-copilot/implemented/packet-analysis-overview.en.md b/docs/gns3-copilot/implemented/packet-analysis-overview.en.md new file mode 100644 index 000000000..ba11a7aa3 --- /dev/null +++ b/docs/gns3-copilot/implemented/packet-analysis-overview.en.md @@ -0,0 +1,74 @@ + + +# GNS3-Copilot Real-time Packet AI Analysis Overview + +## Core Flow + +```mermaid +flowchart TB + subgraph "① Analysis Trigger & Knowledge Query" + A["User asks\n'e.g. Analyze OSPF neighbor state'"] --> B["LLM calls\nPacketAnalysisSkillsTool"] + B --> C{"Protocol Knowledge Repository\ngns3/gns3-skills"} + C --> D["Returns protocol definition\nfields/base_filter/check_rules"] + B --> E["LLM calls\nsearch_fields mode"] + E --> F["tshark -G fields\nfield name search"] + F --> G["Returns valid field names"] + end + + subgraph "② Live Capture & Analysis" + D --> H["LLM constructs tshark_args"] + G --> H + H --> I["PacketAnalysisTool\ncapture analysis mode"] + I --> J["GET /capture/file\ndownload live PCAP"] + J --> K["Pre-validate -e field names"] + K --> L["tshark -r pcap\nrun analysis"] + L --> M["Return analysis results"] + end +``` + +## Tool Overview + +| Tool | Source File | Purpose | Available Modes | +|---|---|---|---| +| `PacketAnalysisTool` | `packet_analysis_tool.py` | Download live PCAP + tshark analysis | teaching / lab_automation | +| `PacketAnalysisSkillsTool` | `registry.py` (skills module) | Query protocol-level analysis knowledge (fields, filters) | teaching / lab_automation | + +## Agent Workflow (LangGraph) + +```mermaid +sequenceDiagram + participant U as User + participant LLM as LLM Node + participant Skills as PacketAnalysisSkillsTool + participant Pcap as PacketAnalysisTool + + U->>LLM: OSPF neighbors can't establish, analyze + LLM->>Skills: get protocol=ospf + Skills-->>LLM: OSPF fields, filter definitions + LLM->>Pcap: search_fields query=ospf.hello + Pcap-->>LLM: valid -e field names + LLM->>Pcap: download PCAP + tshark_args + Pcap-->>LLM: tshark output results + LLM->>LLM: analysis reveals Dead interval mismatch + LLM-->>U: OSPF Dead interval mismatch detected +``` + +## Server Capture API + +| Endpoint | Function | +|---|---| +| `POST /v3/projects/{pid}/links/{lid}/capture/start` | Start packet capture on a link | +| `POST /v3/projects/{pid}/links/{lid}/capture/stop` | Stop packet capture | +| `GET /v3/projects/{pid}/links/{lid}/capture/file` | Download PCAP file (available even while capture is active) | +| `GET /v3/projects/{pid}/links/{lid}/capture/stream` | Stream PCAP data | +| `WS /v3/projects/{pid}/links/{lid}/capture/web-wireshark` | Web Wireshark WebSocket proxy | + +## Key Design Points + +1. **LLM-driven Analysis** — The LLM constructs tshark parameters itself; the framework does not hardcode protocol logic, only performs safety validation +2. **Live PCAP** — Captures can be downloaded and analyzed while running, no need to stop capturing +3. **Dual Knowledge Sources** — External repository provides protocol-specific knowledge; local tshark field registry provides exact field names +4. **Safety First** — Pre-validation of tshark field names prevents execution failures from invalid fields diff --git a/docs/gns3-copilot/implemented/skills-repository.md b/docs/gns3-copilot/implemented/skills-repository.md index 78c295cce..aa9c8f2db 100644 --- a/docs/gns3-copilot/implemented/skills-repository.md +++ b/docs/gns3-copilot/implemented/skills-repository.md @@ -10,7 +10,7 @@ See LICENSE file for licensing information. ## Overview -GNS3 Copilot loads all skills, prompts, and security configurations from an external Git repository at [github.com/yueguobin/GNS3-Skills](https://github.com/yueguobin/GNS3-Skills). This enables dynamic updates without server redeployment. +GNS3 Copilot loads all skills, prompts, and security configurations from an external Git repository at [github.com/gns3/gns3-skills](https://github.com/gns3/gns3-skills). This enables dynamic updates without server redeployment. The repository provides: - **Injection skills** (39 categories): Network fault scenarios for troubleshooting practice @@ -74,14 +74,14 @@ Skills repository settings are configured in `gns3_server.conf` under the `[Serv ```ini [Server] -skills_repo_url = https://github.com/yueguobin/GNS3-Skills.git +skills_repo_url = https://github.com/gns3/gns3-skills.git skills_repo_branch = main skills_auto_update = true ``` | Setting | Default | Description | |---------|---------|-------------| -| `skills_repo_url` | `https://github.com/yueguobin/GNS3-Skills.git` | Git repository URL | +| `skills_repo_url` | `https://github.com/gns3/gns3-skills.git` | Git repository URL | | `skills_repo_branch` | `main` | Git branch to track | | `skills_auto_update` | `true` | Automatically pull on reload | diff --git a/docs/gns3-copilot/roadmap/rbac-user-isolation-roadmap.md b/docs/gns3-copilot/roadmap/rbac-user-isolation-roadmap.md index ff125faa9..447279d07 100644 --- a/docs/gns3-copilot/roadmap/rbac-user-isolation-roadmap.md +++ b/docs/gns3-copilot/roadmap/rbac-user-isolation-roadmap.md @@ -8,13 +8,13 @@ GNS3 3.0 ships with a complete RBAC framework (ACE + Role + Privilege models), b - List endpoints return unfiltered results - Two GET endpoints have RBAC checks bypassed via FIXME -This document tracks the incremental work to close those gaps. Each phase is independent and deployable. +**This document has been updated to reflect the implemented solution in `feature/simple-user-isolation` branch.** ## Current State | Resource | Route Check | List Filtering | Auto-ACE on Create | ACE Cleanup on Delete | |---|---|---|---|---| -| Project | `Project.Audit/Modify/Allocate` | Partial — pool isolation exists but non-pool projects leak | **Missing** | Done | +| Project | `Project.Audit/Modify/Allocate` | **Implemented** — Three-step filtering | **Not needed** | Done | | Template | `Template.Audit` **FIXME** | None — all templates returned | **Missing** | Done | | Node | `Node.Audit/Modify/Allocate` | N/A (inherits project) | Inherits project | N/A | | Link | `Link.Audit/Modify/Allocate` | N/A (inherits project) | Inherits project | N/A | @@ -25,7 +25,49 @@ This document tracks the incremental work to close those gaps. Each phase is ind | Appliance | `Appliance.Audit/Allocate` | None — all appliances returned | N/A (builtin) | N/A | | Symbol | `Symbol.Audit/Allocate` | None — all symbols returned | N/A (builtin) | N/A | -## Architecture +## Implemented Solution: Project Isolation + +### Three-Step Permission Check Logic + +**Status**: ✅ Implemented in `feature/simple-user-isolation` branch + +```python +# Step 1: ACE check - basic access permission +# Get projects user has ACE for +ace_projects = [] +for project in controller.projects.values(): + project_path = f"/projects/{project.id}" + if await rbac_repo.check_user_has_privilege(current_user.user_id, project_path, "Project.Audit"): + ace_projects.append(project) + +# Step 2: Filter ace_projects by created_by - user's own projects +# Project sharing is only available through resource pools +user_projects = [p.asdict() for p in ace_projects if p.created_by == current_user.username] +projects.extend(user_projects) + +# Step 3: Resource pool projects +# Projects shared through resource pools +user_pool_resources = await rbac_repo.get_user_pool_resources(current_user.user_id, "Project.Audit") +project_ids_in_pools = [str(r.resource_id) for r in user_pool_resources if r.resource_type == "project"] +pool_projects = [p.asdict() for p in controller.projects.values() if p.id in project_ids_in_pools] +projects.extend(pool_projects) +``` + +### Key Design Principles + +1. **ACE for basic access control**: Controls whether user can access the system +2. **created_by for user isolation**: Controls which specific resources user can access +3. **Resource pools for project sharing**: The only mechanism for sharing projects between users +4. **No direct ACE sharing**: Users cannot configure ACE directly on specific projects to share them + +### Advantages of This Approach + +- **Fault tolerance**: Even with broad ACE configuration (`path: "/" + propagate: true`), user isolation remains effective +- **Clear separation**: Basic access, data ownership, and team sharing are clearly separated +- **Simple mechanism**: No complex auto-ACE or seen_project_ids tracking required +- **Performance**: Leverages existing created_by field, no schema changes needed + +## Updated Architecture ```mermaid graph TD @@ -33,30 +75,30 @@ graph TD WebUI CLI end - + subgraph "Controller API" Auth[get_current_active_user] - PrivCheck[has_privilege] Routes[Resource Routes] - AutoACE[auto-ACE on create] - ListFilter[ACE-based list filtering] + ACE_Check[Step 1: ACE Check] + Owner_Filter[Step 2: Filter by created_by] + Pool_Check[Step 3: Resource Pools] end - + subgraph "RBAC Engine" ACE[(ACE table)] Role[(Role table)] Privilege[(Privilege table)] Checker[check_user_has_privilege] end - + WebUI --> Auth CLI --> Auth Auth --> Routes - Routes --> PrivCheck - Routes --> AutoACE - Routes --> ListFilter - PrivCheck --> Checker - ListFilter --> Checker + Routes --> ACE_Check + ACE_Check --> Checker + ACE_Check --> Owner_Filter + Owner_Filter --> Pool_Check + Pool_Check --> Checker Checker --> ACE Checker --> Role Role --> Privilege @@ -64,53 +106,48 @@ graph TD ## Business Process -### Project creation with auto-ACE - -```mermaid -sequenceDiagram - actor U as User - participant API as POST /projects - participant Ctrl as Controller - participant ACE as ACE Table - - U->>API: Create project - API->>API: has_privilege("Project.Allocate") - API->>Ctrl: add_project() - Ctrl-->>API: project - API->>ACE: create ACE
(user=creator, role=User,
path=/projects/{id}) - API-->>U: 201 + project -``` - -### Project listing with ACE filtering +### Project listing with three-step filtering ```mermaid sequenceDiagram actor U as User participant API as GET /projects - participant Ctrl as Controller - participant ACE as ACE Table - + participant Controller as Controller + participant RBAC as RBAC Engine + U->>API: List projects API->>API: get_current_active_user (not superadmin) + + API->>RBAC: Step 1: ACE check on each project loop Each project - API->>ACE: check_user_has_privilege(user, path, "Project.Audit") - ACE-->>API: True/False + RBAC-->>API: ACE results end - API-->>U: filtered list + + API->>API: Step 2: Filter by created_by + API-->>U: User's own projects + + API->>RBAC: Step 3: Resource pool projects + RBAC-->>API: Pool projects + + API-->>U: Combined list (own + pool) ``` -## Phased Plan +## Updated Phased Plan -### Phase 1 — MVP: Project isolation +### ✅ Phase 1 — MVP: Project isolation (COMPLETED) -**Goal**: Users only see projects they created or were granted access to. +**Goal**: Users only see projects they created or were granted access to through resource pools. -| Task | Files | Detail | -|---|---|---| -| Auto-ACE on project create | `projects.py` — `create_project()` | After `add_project()`, create ACE: user=creator, role=User, path=`/projects/{id}` | -| Fix project list filtering | `projects.py` — `get_projects()` | Remove "sees all non-pool projects" path. Return only projects passing ACE check. | +| Task | Files | Status | Detail | +|---|---|---|---| +| Fix project list filtering | `projects.py` — `get_projects()` | ✅ **Implemented** | Three-step filtering: ACE check → created_by filter → resource pools | -**Estimate**: 2 files, ~30 lines changed. +**Result**: +- ✅ Users can only see projects they created +- ✅ Team collaboration through resource pools works +- ✅ Fault tolerance: Works correctly even with broad ACE configurations +- ✅ No schema changes required +- ✅ ~30 lines changed ### Phase 2 — Template isolation @@ -118,9 +155,8 @@ sequenceDiagram | Task | Files | Detail | |---|---|---| -| Auto-ACE on template create | `templates.py` — `create_template()` | Same pattern as project | -| Fix template list filtering | `templates.py` — `get_templates()` | Filter by ACE; builtin templates always visible | -| Uncomment `Template.Audit` | `templates.py` lines 75, 167 | Restore `has_privilege("Template.Audit")` | +| Apply same pattern to templates | `templates.py` — `get_templates()` | Use same three-step filtering as projects | +| Uncomment `Template.Audit` | `templates.py` | Restore `has_privilege("Template.Audit")` checks | **Dependency**: Web UI must handle 403 from `GET /templates/{id}`. Mitigation: keep builtin templates unconditionally visible so the UI always has data. @@ -130,8 +166,8 @@ sequenceDiagram | Task | Files | |---|---| -| Auto-ACE on image upload | `images.py` — `upload_image()` | -| Fix image list filtering | `images.py` — `get_images()` | +| Apply same pattern to images | `images.py` — `get_images()` | +| Fix image list filtering | `images.py` | | ACE cleanup on delete | `images.py` — `delete_image()` | ### Phase 4 — Default ACE for "Users" group (optional) @@ -145,35 +181,441 @@ sequenceDiagram ## API Endpoints Changed -### Phase 1 +### Phase 1 (Implemented) | Method | Path | Change | |---|---|---| -| `POST` | `/v3/projects` | Auto-create ACE for creator | -| `GET` | `/v3/projects` | Filter by user ACEs | +| `GET` | `/v3/projects` | Three-step filtering: ACE → created_by → resource pools | -### Phase 2 +### Phase 2 (Planned) | Method | Path | Change | |---|---|---| -| `POST` | `/v3/templates` | Auto-create ACE for creator | -| `GET` | `/v3/templates` | Filter by user ACEs + builtin | +| `GET` | `/v3/templates` | Apply same three-step filtering | | `GET` | `/v3/templates/{id}` | Restore `Template.Audit` check | -## Design Decisions +## Key Design Decisions -1. **Compute stays shared**: Computes are infrastructure, not user-owned. The `Compute.Audit` FIXME stays — all authenticated users see computes. +1. **Project sharing through resource pools only**: Users cannot configure ACE directly to share specific projects. All sharing must go through resource pools. This prevents permission configuration chaos and maintains clear ownership semantics. -2. **No DB migration required**: All phases use the existing ACE/role/privilege tables. No schema changes. +2. **No auto-ACE required**: The three-step filtering logic works without needing automatic ACE creation on project creation. The created_by field provides sufficient ownership information. -3. **Performance**: Project list filtering is O(n) — iterates all projects and checks ACE per project. Acceptable for < 500 projects. Can optimize later with direct ACE path queries. +3. **Fault-tolerant to ACE configuration**: Even if administrators configure broad ACE permissions (like `path: "/" + propagate: true`), user isolation remains effective because Step 2 filters by created_by. -4. **The FIXME dependency**: `Template.Audit` was commented out because web UI crashes on 403. Fix requires either (a) auto-ACE so users own their templates, or (b) web UI 403 handling. +4. **No DB migration required**: Uses existing ACE/role/privilege tables and created_by field. No schema changes needed. -## References +5. **Performance**: Project list filtering is O(n) where n is the total number of projects. Each project requires one ACE check. Acceptable for < 500 projects. Can optimize later with batch ACE queries if needed. -- Discussion: https://github.com/GNS3/gns3-server/discussions/1949 -- RBAC models: `gns3server/db/models/acl.py`, `roles.py`, `privileges.py` -- RBAC repository: `gns3server/db/repositories/rbac.py` -- Auth dependency: `gns3server/api/routes/controller/dependencies/authentication.py` -- RBAC dependency: `gns3server/api/routes/controller/dependencies/rbac.py` +## Updated References + +- **Implementation**: `gns3server/api/routes/controller/projects.py` (feature/simple-user-isolation branch) +- **Discussion**: https://github.com/GNS3/gns3-server/discussions/1949 +- **RBAC models**: `gns3server/db/models/acl.py`, `roles.py`, `privileges.py` +- **RBAC repository**: `gns3server/db/repositories/rbac.py` +- **Auth dependency**: `gns3server/api/routes/controller/dependencies/authentication.py` +- **RBAC dependency**: `gns3server/api/routes/controller/dependencies/rbac.py` +- **Resource pools**: `gns3server/db/models/pools.py` and `gns3server/db/repositories/pools.py` + +## Implementation Notes + +### What Was Implemented + +The `feature/simple-user-isolation` branch implements a robust user isolation mechanism that: + +1. **Integrates with existing RBAC framework** without breaking changes +2. **Leverages the created_by field** that already exists in the Project model +3. **Uses three-step pipeline filtering** to avoid complex seen_project_ids tracking +4. **Supports team collaboration** through existing resource pool functionality +5. **Is fault-tolerant to ACE misconfiguration** - broad ACE permissions don't break user isolation + +### What Was Not Implemented + +The original roadmap's Phase 1 included auto-ACE creation on project creation. This was determined to be unnecessary because: + +- The three-step filtering logic achieves user isolation without auto-ACE +- Auto-ACE would add complexity without significant benefit +- Project sharing through resource pools is cleaner than direct ACE configuration + +### Future Work + +The same three-step filtering pattern can be applied to: +- **Templates**: Replace the FIXME comment with proper filtering logic +- **Images**: Apply the same pattern for user image isolation +- **Other resources**: Extend the pattern as needed + +This implementation provides a solid foundation for user isolation in GNS3 3.0+ while maintaining compatibility with the existing RBAC framework. + +## Phase 5 — ACE Architecture Refactoring (Future) + +**Goal**: Improve ACE manageability by supporting multiple paths and resource pools in a single ACE entry. + +### Current Problem + +With the current design where one ACE = one path: +- **ACE explosion**: 5 user groups × 10 resource pools = 50 ACE entries +- **Management complexity**: Difficult to maintain and understand ACE purpose +- **Performance impact**: Permission checking must iterate through many ACE entries + +### Proposed Solution + +Redesign ACE structure to support multiple paths and resource pools in a single ACE entry: + +```json +{ + "name": "Development Team Access", + "description": "Full access for development team", + "ace_type": "group", + "group_id": "...", + "role_id": "...", + "paths": ["/projects", "/templates", "/images"], + "resource_pools": ["pool-id-1", "pool-id-2"], + "propagate": true, + "allowed": true +} +``` + +### Database Changes Required + +1. **Add name and description to ACE table**: +```sql +ALTER TABLE acl ADD COLUMN name VARCHAR; +ALTER TABLE acl ADD COLUMN description TEXT; +``` + +2. **Create association tables**: +```sql +CREATE TABLE ace_paths ( + ace_id UUID REFERENCES acl(ace_id), + path VARCHAR, + PRIMARY KEY (ace_id, path) +); + +CREATE TABLE ace_pools ( + ace_id UUID REFERENCES acl(ace_id), + resource_pool_id UUID REFERENCES resource_pools(resource_pool_id), + PRIMARY KEY (ace_id, resource_pool_id) +); +``` + +3. **Update permission checking logic** to check both paths and resource_pools tables + +### Benefits + +- ✅ **Reduced ACE entries**: One ACE covers multiple related paths/pools +- ✅ **Better organization**: Logical grouping with clear names and descriptions +- ✅ **Easier management**: Edit one ACE instead of multiple related entries +- ✅ **Improved performance**: Fewer ACE entries to check during permission validation + +### Implementation Considerations + +- **Migration path**: Need to migrate existing single-path ACEs to new structure +- **Backward compatibility**: API should support both old and new formats during transition +- **UI updates**: ACE management interface needs to support multi-path/pool selection +- **Permission checking**: Update `check_user_has_privilege` to check association tables + +## Phase 6 — Frontend Permission Query API (Future) + +**Goal**: Provide an API endpoint for the Web UI to query the current user's permissions, enabling dynamic UI rendering based on role and ACE configuration. + +### Problem + +Currently the Web UI cannot determine what the authenticated user is allowed to see or do: + +- ❌ Users see menu items and buttons they don't have permission to use +- ❌ Clicking a forbidden action results in a 403 error (unexpected UX) +- ❌ No way to hide/show UI elements based on actual permissions + +### Proposed Solution + +Create a `GET /v3/me/permissions` endpoint that returns the current user's effective permissions: + +```json +{ + "user_id": "uuid", + "is_superadmin": false, + "permissions": [ + {"path": "/projects", "privileges": ["Project.Audit", "Project.Allocate"]}, + {"path": "/projects/{id}", "privileges": ["Project.Audit", "Project.Modify"]}, + {"path": "/templates", "privileges": ["Template.Audit"]} + ], + "pools": [ + {"path": "/pools/{id}", "name": "Team Projects", "privileges": ["Pool.Audit"]} + ] +} +``` + +### Benefits + +- ✅ **Dynamic UI**: Frontend can hide inaccessible menus/buttons +- ✅ **Better UX**: Users only see what they can actually use +- ✅ **Reduced errors**: Fewer 403 responses from hidden operations +- ✅ **Faster feedback**: Permission checks happen at render time, not request time + +### Dependencies + +- **Phase 5 (ACE refactoring)** may change how permissions are stored, which would affect this API's implementation + +## Phase 7 — Resource Pool Renaming (Future) + +**Goal**: Rename "Resource Pool" to a more descriptive name that better reflects its actual purpose. + +### Problem + +The current name "Resource Pool" is too generic and doesn't clearly convey its actual function: + +- ❌ **Ambiguous name**: "Resource Pool" could refer to compute pools, connection pools, etc. +- ❌ **Unclear purpose**: Users don't understand it's primarily for sharing projects +- ❌ **Discoverability**: Hard to find the right feature when looking for project sharing + +### Actual Function + +Resource pools in GNS3 are used for: +- **Project sharing**: Allow users to access projects created by other users +- **Team collaboration**: Enable team members to work on shared projects +- **Access control**: Provide fine-grained permissions for project access through three-step filtering (ACE check → created_by filter → resource pools) + +### Proposed Name Options + +| Option | Pros | Cons | +|--------|-------|-------| +| **Project Pool** | More explicit, indicates it contains projects | Still uses "pool" terminology | +| **Shared Projects** | Directly describes the function | Loses the "collection" concept | +| **Team Projects** | Emphasizes collaboration use case | Doesn't cover non-team sharing scenarios | + +**Recommended**: **Project Pool** - strikes a balance between clarity and consistency with existing terminology. + +### Implementation Scope + +Renaming would require changes to: +- Database tables: `resource_pools` → `project_pools` +- API routes: `/v3/pools` → `/v3/project_pools` +- Schema classes and field names throughout the codebase +- All documentation and help text +- Migration script to preserve existing data + +### Benefits + +- ✅ **Improved discoverability**: Users can easily find the project sharing feature +- ✅ **Better onboarding**: New users understand the purpose without confusion +- ✅ **Clearer API**: API endpoints and schemas more self-documenting + +### Implementation Considerations + +- **Breaking change**: Requires API version bump or backward compatibility layer +- **Data migration**: Existing resource pools must be preserved during table rename +- **Documentation updates**: All references in docs, tutorials, and API specs need updating +- **UI changes**: Frontend labels and navigation menus need to match new terminology + +## Phase 8 — Per-User Project Namespace (Future) + +**Goal**: Allow project names to be unique per user instead of globally unique, enabling better user experience. + +### Current Problem + +Although Phase 1 implements user isolation (users only see projects they created or have access to through resource pools), project names remain globally unique: + +- ❌ **Naming conflicts**: Alice and Bob cannot both create a project named "My Project" +- ❌ **Unnecessary restrictions**: Even though projects are isolated, users must coordinate globally unique names +- ❌ **Poor user experience**: Users get confusing error messages when trying to use common names like "Test Project" + +### Proposed Solution + +Change project uniqueness from global to per-user: + +**Current:** +```sql +UNIQUE(name) -- Project names must be globally unique +``` + +**Proposed:** +```sql +UNIQUE(user_id, name) -- Project names unique per user +``` + +### Benefits + +- ✅ **Better UX**: Users can name projects whatever they want without worrying about global conflicts +- ✅ **Natural naming**: Common names like "Test Project" or "Demo" can coexist between users +- ✅ **No coordination needed**: Teams don't need to maintain a shared project naming registry +- ✅ **Consistent isolation**: Projects are isolated both in visibility AND naming + +### Implementation Scope + +Changes required: +- **Database schema**: Modify Project table unique constraint from `(name)` to `(user_id, name)` +- **Migration script**: Handle existing projects with conflicting names +- **API validation**: Update project creation validation logic +- **Frontend**: Remove global name uniqueness checks from UI + +### Migration Considerations + +**Handling existing name conflicts:** +If the database already has projects with the same name but different users: +- Option 1: Keep existing names, only enforce uniqueness for new projects +- Option 2: Append suffixes to duplicates (e.g., "My Project (alice)", "My Project (bob)") +- Option 3: Require admin resolution for conflicts before enabling new constraint + +**Recommended**: Option 1 (grandfather existing projects) for minimal disruption. + +### Dependencies + +- **Phase 1 (User isolation)**: Must be completed first +- Database migration required to modify unique constraint + +## Phase 9 — User Self-Registration (Future) + +**Goal**: Allow users to register their own accounts without requiring manual admin creation. + +### Current State + +Currently, user accounts can only be created by administrators or through direct database operations: +- ❌ **Admin burden**: Every new user requires manual account creation +- ❌ **Poor scalability**: Not suitable for public deployments or large organizations +- ❌ **Friction**: Users cannot immediately start using the system + +### Proposed Features + +**Self-Registration Flow:** +1. User provides email, username, and password +2. System validates input and creates account +3. Optional email verification to confirm email address +4. Account created with default role (typically "User" role) +5. User can immediately log in and start creating projects + +**Email Verification (Optional):** +- Send verification email with confirmation link/code +- Verify email address before granting full access +- Prevent spam account creation +- Require SMTP server configuration + +### Implementation Components + +1. **New API endpoint**: `POST /v3/access/register` (public, no authentication required) +2. **Email service**: Integration with SMTP server for sending emails +3. **Configuration**: SMTP settings (host, port, credentials, encryption) +4. **Rate limiting**: Prevent abuse of self-registration +5. **Captcha integration**: Optional bot protection + +### Configuration + +```yaml +Server: + email: + enabled: true + smtp_host: smtp.example.com + smtp_port: 587 + smtp_username: noreply@example.com + smtp_password: secret + use_tls: true + registration: + require_email_verification: true + default_role: "User" + allow_public_registration: true +``` + +### Security Considerations + +- ✅ **Rate limiting**: Prevent spam account creation +- ✅ **Email verification**: Confirm email ownership +- ✅ **Default permissions**: New users get limited default role +- ✅ **Admin approval** (optional): Require admin approval before account activation +- ❌ **Superadmin creation**: Never allow self-registration as superadmin + +### Dependencies + +- **Phase 10 (Email service)**: SMTP integration required for email verification + +## Phase 10 — Email Service Integration (Future) + +**Goal**: Implement email sending capability for notifications, verification, and alerts. + +### Use Cases + +1. **User registration**: Email verification links/codes +2. **Password reset**: Secure password reset emails +3. **System alerts**: Error notifications, system updates +4. **Project sharing**: Notify users when projects are shared via resource pools +5. **Usage reports**: Periodic usage summaries or quota alerts + +### Proposed Implementation + +**Email Service Architecture:** +- Abstract email service interface +- Support multiple email providers (SMTP, SendGrid, AWS SES, etc.) +- Email templates with Jinja2 for customization +- Async email sending to avoid blocking API responses +- Email queue for retry logic on failures + +**Email Templates:** +- Registration verification +- Password reset +- Project shared notification +- System alerts +- Usage reports + +**API Endpoints:** +```python +# Configuration (admin only) +POST /v3/access/config/email +GET /v3/access/config/email +PUT /v3/access/config/email + +# Test email (admin only) +POST /v3/access/config/email/test + +# Password reset (public) +POST /v3/access/users/password/reset/request +POST /v3/access/users/password/reset/confirm + +# Email verification (public) +POST /v3/access/users/verify/email +POST /v3/access/users/verify/confirm +``` + +### Database Schema + +New table for email tracking: +```sql +CREATE TABLE email_verification_tokens ( + token_id UUID PRIMARY KEY, + user_id UUID REFERENCES users(user_id) ON DELETE CASCADE, + token VARCHAR(255), # Verification code + purpose VARCHAR(50), # 'registration', 'password_reset', etc. + expires_at DATETIME, + created_at DATETIME, + used BOOLEAN DEFAULT FALSE +); +``` + +### Configuration + +```yaml +Server: + email: + enabled: true + provider: "smtp" # or "sendgrid", "aws_ses", etc. + from_address: "noreply@example.com" + from_name: "GNS3 Server" + reply_to: "support@example.com" + smtp: + host: smtp.example.com + port: 587 + username: noreply@example.com + password: encrypted_password + use_tls: true + templates_dir: /etc/gns3/email_templates +``` + +### Security Considerations + +- ✅ **Encrypted credentials**: SMTP passwords stored encrypted in database +- ✅ **Token expiration**: Verification tokens expire after configurable time +- ✅ **Rate limiting**: Prevent email spamming +- ✅ **Async sending**: Don't block API responses on email operations +- ✅ **Retry logic**: Handle temporary email service failures +- ✅ **Privacy**: Don't expose user information in error messages + +### Dependencies + +- **Phase 9 (Self-registration)**: User self-registration requires email verification +- Encryption utilities for storing SMTP credentials securely diff --git a/docs/gns3-copilot/roadmap/skills-editor-api-roadmap.md b/docs/gns3-copilot/roadmap/skills-editor-api-roadmap.md index f6ddc09dc..534265116 100644 --- a/docs/gns3-copilot/roadmap/skills-editor-api-roadmap.md +++ b/docs/gns3-copilot/roadmap/skills-editor-api-roadmap.md @@ -38,7 +38,7 @@ graph TD subgraph "Remote" GH[GitHub API
Pull Requests] - REPO[yueguobin/GNS3-Skills] + REPO[gns3/gns3-skills] end UI -->|CRUD + PR| API @@ -147,7 +147,7 @@ All endpoints require **superadmin** authentication. Prefix: `/v3/copilot/skills {"category": "config", "file_count": 1, "path": "config/"} ], "repository": { - "repo_url": "https://github.com/yueguobin/GNS3-Skills.git", + "repo_url": "https://github.com/gns3/gns3-skills.git", "branch": "main", "current_version": "abc123def456...", "is_dirty": false @@ -241,7 +241,7 @@ Response: ```json { "success": true, - "pr_url": "https://github.com/yueguobin/GNS3-Skills/pull/42", + "pr_url": "https://github.com/gns3/gns3-skills/pull/42", "pr_number": 42, "branch": "fix/ospf-descriptions" } diff --git a/docs/gns3-copilot/roadmap/user-node-limit-roadmap.md b/docs/gns3-copilot/roadmap/user-node-limit-roadmap.md new file mode 100644 index 000000000..1d59ad006 --- /dev/null +++ b/docs/gns3-copilot/roadmap/user-node-limit-roadmap.md @@ -0,0 +1,311 @@ +# User Node Limit Roadmap + +## Overview + +Implement a user-level node startup limit feature for GNS3 server to prevent single users from consuming excessive system resources. This feature is **disabled by default** and can be enabled through configuration files, supporting a three-tier configuration priority system. + +## Problem Statement + +Currently, GNS3 server has no mechanism to limit the number of nodes a user can start across all their projects. This can lead to: + +- **Resource exhaustion**: A single user can consume all available system resources +- **Unfair usage**: Some users may prevent others from using the system +- **System instability**: Too many running nodes can degrade overall performance +- **Cost issues**: In cloud environments, this can lead to unexpected costs + +## Solution Design + +### Core Principles + +1. **Default to no limits**: System maintains backward compatibility by defaulting to unrestricted usage +2. **Configuration-driven**: All limits can be controlled through configuration files +3. **Multi-tier priority**: User-specific > User group-specific > Global configuration +4. **Intelligent filtering**: Only count nodes that actually consume resources +5. **Clear error messages**: Users receive actionable feedback when limits are reached + +### Technical Architecture + +#### 1. Database Layer Extension + +**Files**: `gns3server/db/models/users.py` + +Add `max_nodes` field to both `User` and `UserGroup` models: + +```python +# In User model +max_nodes = Column(Integer, nullable=True) # NULL = no limit + +# In UserGroup model +max_nodes = Column(Integer, nullable=True) # NULL = no limit +``` + +#### 2. Configuration System Extension + +**File**: `gns3server/schemas/config.py` + +Add node limit configuration to `ControllerSettings`: + +```python +class NodeLimitSettings(BaseModel): + enabled: bool = False # Feature toggle (default disabled) + default_max_nodes: int = 5 # Default limit when enabled + excluded_node_types: List[str] = Field(default_factory=lambda: [ + "ethernet_switch", "ethernet_hub", "cloud", "nat" + ]) +``` + +#### 3. Core Service Implementation + +**New File**: `gns3server/services/node_limit_service.py` + +Implement the `NodeLimitService` class with key methods: + +```python +class NodeLimitService: + async def get_user_active_node_count(self, username: str, excluded_types: List[str]) -> int: + """Count user's active nodes across all projects""" + + async def get_user_node_limit(self, user: User) -> Optional[int]: + """Get user's node limit with priority logic""" + + async def check_user_node_limit(self, user: User, project: Project) -> Tuple[bool, str]: + """Check if user can start more nodes""" +``` + +#### 4. API Integration + +**File**: `gns3server/api/routes/controller/nodes.py` + +Add limit checking to node startup endpoint: + +```python +@router.post("/{node_id}/start") +async def start_node( + node: Node = Depends(dep_node), + current_user: User = Depends(get_current_active_user), + node_limit_service: NodeLimitService = Depends(get_node_limit_service) +): + # Node limit check + can_start, error_msg = await node_limit_service.check_user_node_limit( + current_user, node.project + ) + if not can_start: + raise HTTPException(status_code=403, detail=error_msg) + + # Original startup logic + await node.start() +``` + +### Node Counting Logic + +#### What Counts Toward the Limit + +- **Status**: Only nodes in `started` or `suspended` state +- **Ownership**: Only projects where `project.created_by == current_user.username` +- **Node types**: All node types except those explicitly excluded + +#### What's Excluded from the Limit + +- **Always-running nodes**: Ethernet switches, hubs (nodes where `is_always_running()` returns true) +- **Infrastructure nodes**: Cloud nodes and NAT nodes +- **Stopped nodes**: Nodes in `stopped` state +- **Other users' projects**: Nodes in projects created by other users + +#### Configuration Priority + +``` +User-specific limit (highest priority) + ↓ not set +User group limit + ↓ not set +Global configuration (if enabled) + ↓ disabled +No limit (default) +``` + +### Configuration Examples + +#### Scenario 1: Default No Limits (System Default) + +```ini +[Controller] +node_limits_enabled = false +``` + +**Result**: All users have no node limits + +#### Scenario 2: Enable Global Limits + +```ini +[Controller] +node_limits_enabled = true +node_limits_default_max_nodes = 5 +node_limits_excluded_types = ethernet_switch,ethernet_hub,cloud,nat +``` + +**Result**: All users limited to 5 active nodes (excluding infrastructure nodes) + +#### Scenario 3: User-Specific Limits + +Configuration file: `node_limits_enabled = false` + +Database: +- User A: `max_nodes = 10` (limited to 10 nodes) +- User B: `max_nodes = NULL` (no limit) +- Other users: no limit + +#### Scenario 4: User Group Limits + +Configuration file: `node_limits_enabled = false` + +Database: +- "Users" group: `max_nodes = 5` +- "Premium Users" group: `max_nodes = 20` +- "Administrators" group: `max_nodes = NULL` + +**Result**: Members inherit limits from their groups + +## Implementation Files + +| File Path | Type | Description | +|-----------|------|-------------| +| `gns3server/db/models/users.py` | Modify | Add `max_nodes` field to User and UserGroup | +| `gns3server/db_migrations/versions/xxx_add_node_limits.py` | New | Database migration script | +| `gns3server/schemas/config.py` | Modify | Add NodeLimitSettings configuration class | +| `gns3server/schemas/controller/users.py` | Modify | Add `max_nodes` to API schemas | +| `gns3server/services/node_limit_service.py` | New | Core node limit service | +| `gns3server/api/routes/controller/nodes.py` | Modify | Add limit check to node startup | +| `gns3server/api/routes/controller/users.py` | Modify | Add user limit configuration API | +| `gns3server/api/routes/controller/groups.py` | Modify | Add group limit configuration API | + +## Implementation Steps + +### Phase 1: Database Layer +1. Add `max_nodes` field to `User` and `UserGroup` models +2. Create database migration file +3. Test database migration and rollback + +### Phase 2: Configuration System +1. Add `NodeLimitSettings` to configuration schema +2. Update configuration file loading logic +3. Test configuration parsing and validation + +### Phase 3: Core Service +1. Implement `NodeLimitService` class +2. Implement node counting logic +3. Implement limit checking logic +4. Add unit tests for service methods + +### Phase 4: API Integration +1. Modify node startup endpoint to add limit check +2. Add user limit configuration endpoints +3. Add group limit configuration endpoints +4. Add user node usage statistics endpoint + +### Phase 5: Testing +1. Unit tests for all core functions +2. Integration tests for API endpoints +3. End-to-end tests for complete workflows +4. Performance tests for node counting operations + +## Testing Strategy + +### Functional Tests +- [ ] Default state verification (no limits) +- [ ] Global limit enablement +- [ ] User-specific limits +- [ ] User group limits +- [ ] Configuration priority verification + +### Boundary Tests +- [ ] Exactly at limit (can start last node) +- [ ] One over limit (startup rejected) +- [ ] Stop node then restart (should work) +- [ ] Special node type exclusion + +### Integration Tests +- [ ] Multi-user concurrent startups +- [ ] Node state transitions +- [ ] Dynamic configuration changes +- [ ] Project ownership filtering + +### Performance Tests +- [ ] Node counting performance with many projects +- [ ] Concurrent startup request handling +- [ ] Memory usage monitoring + +## Error Messages + +### User-Friendly Error Response + +When a user hits their node limit: + +```json +{ + "detail": "节点启动限制:您当前有 5 个活跃节点,限制为 5 个。请停止一些节点后再试,或联系管理员调整限制。" +} +``` + +Alternative formats: +- Show current usage vs limit +- Provide action suggestions +- Include contact information for administrators + +## Migration Path + +### For Existing Systems + +1. **Database migration**: Add new nullable fields (safe, no data loss) +2. **Configuration update**: Add new optional settings (backward compatible) +3. **API changes**: Add new optional dependency injection (no breaking changes) +4. **Behavior**: No changes to existing functionality when disabled + +### Rollback Plan + +If issues occur: +1. Set `node_limits_enabled = false` in configuration +2. Service automatically disables limit checking +3. System returns to pre-feature behavior + +## Benefits + +1. **Resource Management**: Prevent resource exhaustion +2. **Fair Usage**: Ensure equitable resource distribution +3. **Cost Control**: Manage cloud resource costs +4. **System Stability**: Maintain performance under load +5. **Flexibility**: Support different usage patterns and tiers +6. **Backward Compatible**: No impact on existing deployments + +## Future Enhancements + +- Per-project limits (in addition to global user limits) +- Time-based limits (different limits for different times) +- Burst limits (temporary allowance for peak usage) +- Usage quotas with reset periods (daily/weekly/monthly) +- Monitoring and alerting for limit approaching +- Administrative override capabilities +- Usage history and analytics + +## Documentation Updates + +- [ ] Update API documentation with new endpoints +- [ ] Add configuration guide to admin documentation +- [ ] Update user guide with limit information +- [ ] Add troubleshooting section for limit issues +- [ ] Provide migration guide for existing deployments + +## Status + +**Current Status**: Design Phase + +**Next Steps**: +1. Review and approve this roadmap +2. Begin Phase 1 implementation (Database Layer) +3. Create detailed technical specification +4. Set up development and testing environment + +--- + +**Document Version**: 1.0 +**Last Updated**: 2026-05-27 +**Author**: GNS3 Development Team \ No newline at end of file diff --git a/gns3server/agent/gns3_copilot/configs/skills_config.py b/gns3server/agent/gns3_copilot/configs/skills_config.py index 077b1e870..3a10332d9 100644 --- a/gns3server/agent/gns3_copilot/configs/skills_config.py +++ b/gns3server/agent/gns3_copilot/configs/skills_config.py @@ -34,7 +34,7 @@ from gns3server.config import Config # Default skills repository configuration SKILLS_CONFIG = { # Git repository URL for skills - "repo_url": "https://github.com/yueguobin/GNS3-Skills.git", + "repo_url": "https://github.com/gns3/gns3-skills.git", # Git branch to use "branch": "main", diff --git a/gns3server/agent/gns3_copilot/prompts/__init__.py b/gns3server/agent/gns3_copilot/prompts/__init__.py index 26607487a..de0aee451 100644 --- a/gns3server/agent/gns3_copilot/prompts/__init__.py +++ b/gns3server/agent/gns3_copilot/prompts/__init__.py @@ -29,7 +29,7 @@ Prompts Module for GNS3-Copilot This package provides system prompts loading utilities for the GNS3-Copilot AI agent. All system prompts are now loaded from the external GNS3-Skills repository: -https://github.com/yueguobin/GNS3-Skills +https://github.com/gns3/gns3-skills Available prompts (loaded from external repository): - lab_automation_assistant.md: Lab automation mode (diagnostics + config) diff --git a/gns3server/agent/gns3_copilot/skills/manager.py b/gns3server/agent/gns3_copilot/skills/manager.py index 52abdad13..12fb3d39e 100644 --- a/gns3server/agent/gns3_copilot/skills/manager.py +++ b/gns3server/agent/gns3_copilot/skills/manager.py @@ -76,12 +76,12 @@ class SkillsManager: Initialize the skills manager. Args: - repo_url: Git repository URL (default: https://github.com/yueguobin/GNS3-Skills.git) + repo_url: Git repository URL (default: https://github.com/gns3/gns3-skills.git) branch: Git branch to use (default: "main") auto_update: Whether to automatically pull updates on reload """ if repo_url is None: - repo_url = "https://github.com/yueguobin/GNS3-Skills.git" + repo_url = "https://github.com/gns3/gns3-skills.git" # Get local path from GNS3 config directory config_dir = Config.instance().config_dir diff --git a/gns3server/api/routes/compute/docker_nodes.py b/gns3server/api/routes/compute/docker_nodes.py index 1d9e98530..3e30628a2 100644 --- a/gns3server/api/routes/compute/docker_nodes.py +++ b/gns3server/api/routes/compute/docker_nodes.py @@ -235,6 +235,7 @@ async def delete_docker_node(node: DockerVM = Depends(dep_node)) -> None: """ await node.delete() + await node.project.remove_node(node) @router.post( diff --git a/gns3server/api/routes/controller/acl.py b/gns3server/api/routes/controller/acl.py index f4b0e8897..3f80154ec 100644 --- a/gns3server/api/routes/controller/acl.py +++ b/gns3server/api/routes/controller/acl.py @@ -38,7 +38,6 @@ from gns3server.db.repositories.users import UsersRepository from gns3server.db.repositories.rbac import RbacRepository from gns3server.db.repositories.images import ImagesRepository from gns3server.db.repositories.templates import TemplatesRepository -from gns3server.db.repositories.pools import ResourcePoolsRepository from .dependencies.database import get_repository from .dependencies.rbac import has_privilege @@ -58,8 +57,7 @@ async def endpoints( users_repo: UsersRepository = Depends(get_repository(UsersRepository)), rbac_repo: RbacRepository = Depends(get_repository(RbacRepository)), images_repo: ImagesRepository = Depends(get_repository(ImagesRepository)), - templates_repo: TemplatesRepository = Depends(get_repository(TemplatesRepository)), - pools_repo: ResourcePoolsRepository = Depends(get_repository(ResourcePoolsRepository)) + templates_repo: TemplatesRepository = Depends(get_repository(TemplatesRepository)) ) -> List[dict]: """ List all endpoints to be used in ACL entries. @@ -141,11 +139,9 @@ async def endpoints( for template in templates: add_to_endpoints(f"/templates/{template.template_id}", f'Template "{template.name}"', "template") - # resource pools - add_to_endpoints("/pools", "All resource pools", "pool") - pools = await pools_repo.get_resource_pools() - for pool in pools: - add_to_endpoints(f"/pools/{pool.resource_pool_id}", f'Resource pool "{pool.name}"', "pool") + # Resource pools are not included in "all endpoints" to prevent accidental access + # They must be explicitly configured for team sharing + return endpoints diff --git a/gns3server/api/routes/controller/llm_model_configs.py b/gns3server/api/routes/controller/llm_model_configs.py index a567bd1ef..e6c5b4338 100644 --- a/gns3server/api/routes/controller/llm_model_configs.py +++ b/gns3server/api/routes/controller/llm_model_configs.py @@ -66,13 +66,13 @@ def _filter_api_key_from_config(config: dict) -> dict: ) async def get_user_llm_model_configs( user_id: UUID, - current_user: schemas.User = Depends(has_privilege("User.Audit")), + current_user: schemas.User = Depends(has_privilege("LLMConfig.Audit")), llm_repo: LLMModelConfigsRepository = Depends(get_repository(LLMModelConfigsRepository)) ) -> schemas.LLMModelConfigInheritedResponse: """ Get user's effective LLM model configurations (own + inherited from groups). - Required privilege: User.Audit + Required privilege: LLMConfig.Audit """ try: @@ -97,7 +97,7 @@ async def get_user_llm_model_configs( @router.get( "/users/{user_id}/llm-model-configs/own", response_model=List[schemas.LLMModelConfigResponse], - dependencies=[Depends(has_privilege("User.Audit"))] + dependencies=[Depends(has_privilege("LLMConfig.Audit"))] ) async def get_user_own_llm_model_configs( user_id: UUID, @@ -106,7 +106,7 @@ async def get_user_own_llm_model_configs( """ Get user's own LLM model configurations (excluding inherited ones). - Required privilege: User.Audit + Required privilege: LLMConfig.Audit """ try: @@ -137,7 +137,7 @@ async def get_user_own_llm_model_configs( @router.get( "/users/{user_id}/llm-model-configs/default", response_model=schemas.LLMModelConfigResponse, - dependencies=[Depends(has_privilege("User.Audit"))] + dependencies=[Depends(has_privilege("LLMConfig.Audit"))] ) async def get_user_default_llm_model_config( user_id: UUID, @@ -146,7 +146,7 @@ async def get_user_default_llm_model_config( """ Get user's default LLM model configuration. - Required privilege: User.Audit + Required privilege: LLMConfig.Audit """ try: @@ -183,7 +183,7 @@ async def get_user_default_llm_model_config( "/users/{user_id}/llm-model-configs", response_model=schemas.LLMModelConfigResponse, status_code=status.HTTP_201_CREATED, - dependencies=[Depends(has_privilege("User.Modify"))] + dependencies=[Depends(has_privilege("LLMConfig.Modify"))] ) async def create_user_llm_model_config( user_id: UUID, @@ -194,7 +194,7 @@ async def create_user_llm_model_config( """ Create a new LLM model configuration for a user. - Required privilege: User.Modify + Required privilege: LLMConfig.Modify IMPORTANT: context_limit is REQUIRED. Unit is K tokens (e.g., 128 = 128K = 128,000 tokens). Please check your model provider's documentation for the current context window size. @@ -249,7 +249,7 @@ async def create_user_llm_model_config( @router.put( "/users/{user_id}/llm-model-configs/{config_id}", response_model=schemas.LLMModelConfigResponse, - dependencies=[Depends(has_privilege("User.Modify"))] + dependencies=[Depends(has_privilege("LLMConfig.Modify"))] ) async def update_user_llm_model_config( user_id: UUID, @@ -261,7 +261,7 @@ async def update_user_llm_model_config( Update a user's LLM model configuration. Supports optimistic locking via expected_version field. - Required privilege: User.Modify + Required privilege: LLMConfig.Modify """ try: @@ -317,7 +317,7 @@ async def update_user_llm_model_config( @router.delete( "/users/{user_id}/llm-model-configs/{config_id}", status_code=status.HTTP_204_NO_CONTENT, - dependencies=[Depends(has_privilege("User.Modify"))] + dependencies=[Depends(has_privilege("LLMConfig.Modify"))] ) async def delete_user_llm_model_config( user_id: UUID, @@ -327,7 +327,7 @@ async def delete_user_llm_model_config( """ Delete a user's LLM model configuration. - Required privilege: User.Modify + Required privilege: LLMConfig.Modify """ try: @@ -350,7 +350,7 @@ async def delete_user_llm_model_config( @router.put( "/users/{user_id}/llm-model-configs/default/{config_id}", response_model=schemas.LLMModelConfigResponse, - dependencies=[Depends(has_privilege("User.Modify"))] + dependencies=[Depends(has_privilege("LLMConfig.Modify"))] ) async def set_user_default_llm_model_config( user_id: UUID, @@ -360,7 +360,7 @@ async def set_user_default_llm_model_config( """ Set a user's default LLM model configuration. - Required privilege: User.Modify + Required privilege: LLMConfig.Modify """ try: diff --git a/gns3server/api/routes/controller/pools.py b/gns3server/api/routes/controller/pools.py index 6c8e865fa..0d79e39bc 100644 --- a/gns3server/api/routes/controller/pools.py +++ b/gns3server/api/routes/controller/pools.py @@ -147,10 +147,38 @@ async def delete_resource_pool( if not resource_pool: raise ControllerNotFoundError(f"Resource pool '{resource_pool_id}' not found") + # Check if there are any ACE configurations using this resource pool path + pool_path = f"/pools/{resource_pool_id}" + using_aces = await rbac_repo.get_aces_for_path(pool_path) + + if using_aces: + # Build detailed error message with ACE information + ace_details = [] + for ace in using_aces: + identifier = "" + if ace.ace_type == "user" and ace.user: + identifier = f"User '{ace.user.username}'" + elif ace.ace_type == "group" and ace.group: + identifier = f"Group '{ace.group.name}'" + else: + identifier = f"{ace.ace_type.capitalize()} '{ace.user_id or ace.group_id}'" + + if ace.role: + identifier += f" with role '{ace.role.name}'" + + ace_details.append(f"- {identifier}") + + error_message = ( + f"Resource pool '{resource_pool.name}' cannot be deleted because it is being used by {len(using_aces)} ACE configuration(s):\n" + + "\n".join(ace_details) + + f"\n\nPlease delete the ACE configuration(s) for resource pool '{resource_pool.name}' first." + ) + raise ControllerBadRequestError(error_message) + success = await pools_repo.delete_resource_pool(resource_pool_id) if not success: raise ControllerError(f"Resource pool '{resource_pool_id}' could not be deleted") - await rbac_repo.delete_all_ace_starting_with_path(f"/pools/{resource_pool_id}") + await rbac_repo.delete_all_ace_starting_with_path(pool_path) @router.get( diff --git a/gns3server/api/routes/controller/projects.py b/gns3server/api/routes/controller/projects.py index 99861230c..354c15cfb 100644 --- a/gns3server/api/routes/controller/projects.py +++ b/gns3server/api/routes/controller/projects.py @@ -76,7 +76,6 @@ def dep_project(project_id: UUID) -> Project: async def get_projects( current_user: schemas.User = Depends(get_current_active_user), rbac_repo: RbacRepository = Depends(get_repository(RbacRepository)), - pools_repo: ResourcePoolsRepository = Depends(get_repository(ResourcePoolsRepository)) ) -> List[schemas.Project]: """ Return all projects. @@ -86,19 +85,32 @@ async def get_projects( controller = Controller.instance() projects = [] + seen_project_ids = set() # track seen projects to avoid duplicates if current_user.is_superadmin: # super admin sees all projects return [p.asdict() for p in controller.projects.values()] - elif await rbac_repo.check_user_has_privilege(current_user.user_id, "/projects", "Project.Audit"): - # user with Project.Audit privilege on '/projects' sees all projects except those in resource pools - project_ids_in_pools = [str(r.resource_id) for r in await pools_repo.get_resources() if r.resource_type == "project"] - projects.extend([p.asdict() for p in controller.projects.values() if p.id not in project_ids_in_pools]) - # user with Project.Audit privilege on resource pools sees the projects in these pools - user_pool_resources = await rbac_repo.get_user_pool_resources(current_user.user_id, "Project.Audit") - project_ids_in_pools = [str(r.resource_id) for r in user_pool_resources if r.resource_type == "project"] - projects.extend([p.asdict() for p in controller.projects.values() if p.id in project_ids_in_pools]) + # Batch ACE + resource pool check (3 DB queries regardless of project count) + all_project_ids = list(controller.projects.keys()) + direct_ace_ids, pool_accessible_ids = await rbac_repo.get_accessible_project_ids( + current_user.user_id, "Project.Audit", all_project_ids + ) + + # Step 2: Filter direct ACE projects by created_by + # Direct project sharing is only available through resource pools + for p in controller.projects.values(): + if p.id in direct_ace_ids and p.created_by == current_user.username: + if p.id not in seen_project_ids: + projects.append(p.asdict()) + seen_project_ids.add(p.id) + + # Step 3: Resource pool projects (no created_by filter) + for p in controller.projects.values(): + if p.id in pool_accessible_ids: + if p.id not in seen_project_ids: + projects.append(p.asdict()) + seen_project_ids.add(p.id) return projects diff --git a/gns3server/compute/docker/docker_vm.py b/gns3server/compute/docker/docker_vm.py index 5219ad4c8..988d61cf3 100644 --- a/gns3server/compute/docker/docker_vm.py +++ b/gns3server/compute/docker/docker_vm.py @@ -511,8 +511,18 @@ class DockerVM(BaseNode): variables = [] for var in variables: - formatted = self._format_env(variables, var.get("value", "")) - params["Env"].append("{}={}".format(var["name"], formatted)) + # Handle both Pydantic Variable objects and dictionaries + if hasattr(var, "name"): + # Pydantic Variable object + var_name = var.name + var_value = getattr(var, "value", "") + else: + # Dictionary format + var_name = var.get("name", "") + var_value = var.get("value", "") + + formatted = self._format_env(variables, var_value) + params["Env"].append("{}={}".format(var_name, formatted)) if self._environment: for e in self._environment.strip().split("\n"): @@ -581,7 +591,17 @@ class DockerVM(BaseNode): def _format_env(self, variables, env): for variable in variables: - env = env.replace("${" + variable["name"] + "}", variable.get("value", "")) + # Handle both Pydantic Variable objects and dictionaries + if hasattr(variable, "name"): + # Pydantic Variable object + var_name = variable.name + var_value = getattr(variable, "value", "") + else: + # Dictionary format + var_name = variable.get("name", "") + var_value = variable.get("value", "") + + env = env.replace("${" + var_name + "}", var_value) return env def _format_extra_hosts(self, extra_hosts): diff --git a/gns3server/compute/project.py b/gns3server/compute/project.py index 175d9e916..641c97e2c 100644 --- a/gns3server/compute/project.py +++ b/gns3server/compute/project.py @@ -293,9 +293,14 @@ class Project: # we need to update docker nodes when variables changes if original_variables != variables: + # Parallelize node updates for better performance + tasks = [] for node in self.nodes: if hasattr(node, "update"): - await node.update() + tasks.append(node.update()) + + if tasks: + await asyncio.gather(*tasks, return_exceptions=True) async def close(self): """ diff --git a/gns3server/controller/link.py b/gns3server/controller/link.py index 14ff447ea..556f5ba5f 100644 --- a/gns3server/controller/link.py +++ b/gns3server/controller/link.py @@ -23,6 +23,7 @@ import html from .controller_error import ControllerError, ControllerNotFoundError from gns3server.agent.web_wireshark.manager import WebWiresharkManager from gns3server.config import Config +from gns3server.utils.packet_filter_validation import validate_all_filters, filter_inactive_filters, FilterValidationError import logging @@ -47,7 +48,7 @@ FILTERS = [ "name": "Delay", "description": "Delay packets in milliseconds. You can add jitter in milliseconds (+/-) of the delay", "parameters": [ - {"name": "Latency", "minimum": 0, "maximum": 32767, "unit": "ms", "type": "int"}, + {"name": "Latency", "minimum": 1, "maximum": 32767, "unit": "ms", "type": "int"}, {"name": "Jitter (-/+)", "minimum": 0, "maximum": 32767, "unit": "ms", "type": "int"}, ], }, @@ -147,19 +148,17 @@ class Link: """ Modify the filters list. - Filter with value 0 will be dropped because not active + Filters with value 0 will be filtered out as inactive, with special + handling for delay filter to distinguish between "disabled" and "invalid config". """ - new_filters = {} - for (filter, values) in filters.items(): - new_values = [] - for value in values: - if isinstance(value, str): - new_values.append(value.strip("\n ")) - else: - new_values.append(int(value)) - values = new_values - if len(values) != 0 and values[0] != 0 and values[0] != "": - new_filters[filter] = values + # Filter out inactive filters using the utility function + new_filters = filter_inactive_filters(filters) + + # Validate filter parameters before applying + try: + validate_all_filters(new_filters) + except FilterValidationError as e: + raise ControllerError(f"Invalid packet filter parameters: {str(e)}") if new_filters != self.filters: self._filters = new_filters @@ -576,7 +575,7 @@ class Link: "suspend": self._suspended, "show_filters_icon": getattr(self, '_show_filters_icon', True), } - return { + result = { "nodes": res, "link_id": self._id, "project_id": self._project.id, @@ -591,3 +590,4 @@ class Link: "wireshark": self._wireshark, "show_filters_icon": getattr(self, '_show_filters_icon', True), } + return result diff --git a/gns3server/controller/project.py b/gns3server/controller/project.py index d9c30a703..4924f59e1 100644 --- a/gns3server/controller/project.py +++ b/gns3server/controller/project.py @@ -95,7 +95,7 @@ class Project: show_grid=False, grid_size=75, drawing_grid_size=25, - show_interface_labels=False, + show_interface_labels=True, variables=None, supplier=None, created_by=None, @@ -1194,18 +1194,34 @@ class Project: f"Please check the connection and try again." ) + # Parallel node creation for improved performance + # especially for projects with multiple Docker containers + nodes_to_create = [] for node in topology.get("nodes", []): compute = self.controller.get_compute(node.pop("compute_id")) name = node.pop("name") node_id = node.pop("node_id", str(uuid.uuid4())) - await self.add_node(compute, name, node_id, dump=False, **node) + nodes_to_create.append((compute, name, node_id, node)) + + # Create nodes in parallel with limited concurrency + # to avoid overwhelming the system with too many simultaneous operations + pool = Pool(concurrency=5) + for compute, name, node_id, node_data in nodes_to_create: + pool.append(self.add_node, compute, name, node_id, dump=False, **node_data) + await pool.join() for link_data in topology.get("links", []): if "link_id" not in link_data.keys(): # skip the link continue link = await self.add_link(link_id=link_data["link_id"]) if "filters" in link_data: - await link.update_filters(link_data["filters"]) + try: + await link.update_filters(link_data["filters"]) + except ControllerError as e: + log.warning( + "Dropping invalid filters on link %s: %s", + link_data.get("link_id"), e + ) if "link_style" in link_data: await link.update_link_style(link_data["link_style"]) if "show_filters_icon" in link_data: diff --git a/gns3server/db/models/privileges.py b/gns3server/db/models/privileges.py index ad63503c9..165a9cec9 100644 --- a/gns3server/db/models/privileges.py +++ b/gns3server/db/models/privileges.py @@ -226,6 +226,18 @@ def create_default_roles(target, connection, **kw): { "description": "View an appliance", "name": "Appliance.Audit" + }, + { + "description": "Create or delete an LLM model configuration", + "name": "LLMConfig.Allocate" + }, + { + "description": "View an LLM model configuration", + "name": "LLMConfig.Audit" + }, + { + "description": "Update an LLM model configuration", + "name": "LLMConfig.Modify" } ] @@ -295,7 +307,9 @@ def add_privileges_to_default_roles(target, connection, **kw): "Image.Audit", "Compute.Audit", "Appliance.Allocate", - "Appliance.Audit" + "Appliance.Audit", + "LLMConfig.Audit", + "LLMConfig.Modify" ) add_privileges_to_role(target, connection, "User", user_privileges) diff --git a/gns3server/db/repositories/pools.py b/gns3server/db/repositories/pools.py index 84ce6e47a..336c29c8d 100644 --- a/gns3server/db/repositories/pools.py +++ b/gns3server/db/repositories/pools.py @@ -156,6 +156,14 @@ class ResourcePoolsRepository(BaseRepository): Delete a resource pool. """ + # Get all resources in the pool first + resources = await self.get_pool_resources(resource_pool_id) + + # Delete all resource records + for resource in resources: + await self.delete_resource(resource.resource_id) + + # Now delete the resource pool query = delete(models.ResourcePool).where(models.ResourcePool.resource_pool_id == resource_pool_id) result = await self._db_session.execute(query) await self._db_session.commit() @@ -203,6 +211,7 @@ class ResourcePoolsRepository(BaseRepository): resource_pool_db.resources.remove(resource) await self._db_session.commit() await self._db_session.refresh(resource_pool_db) + return resource_pool_db async def get_pool_resources(self, resource_pool_id: UUID) -> List[models.Resource]: diff --git a/gns3server/db/repositories/rbac.py b/gns3server/db/repositories/rbac.py index 6d7514654..bd8f1ad9a 100644 --- a/gns3server/db/repositories/rbac.py +++ b/gns3server/db/repositories/rbac.py @@ -230,6 +230,25 @@ class RbacRepository(BaseRepository): result = await self._db_session.execute(query) return result.scalars().all() + async def get_aces_for_path(self, path: str) -> List[models.ACE]: + """ + Get all ACEs for a specific path (exact match or starting with path). + + This method includes related user, group, and role information. + """ + + query = select(models.ACE).\ + where( + (models.ACE.path == path) | (models.ACE.path.startswith(path + "/")) + ).\ + options( + selectinload(models.ACE.user), + selectinload(models.ACE.group), + selectinload(models.ACE.role) + ) + result = await self._db_session.execute(query) + return result.scalars().all() + async def check_ace_exists(self, path: str) -> bool: """ Check if an ACE exists. @@ -385,6 +404,83 @@ class RbacRepository(BaseRepository): pool_resources.extend(await self._get_resources_in_pools(group_aces)) return list(set(pool_resources)) + async def get_accessible_project_ids( + self, + user_id: UUID, + privilege_name: str, + all_project_ids: List[str] + ): + """ + Batch check which projects a user can access via direct ACE or resource pools. + Performs 3 fixed DB queries regardless of project count. + + Returns: + (direct_ace_ids, pool_accessible_ids) where: + - direct_ace_ids: projects the user has a direct ACE on (needs created_by filter) + - pool_accessible_ids: projects in resource pools the user can access (no created_by filter) + """ + + user_aces = await self._get_user_aces(user_id, privilege_name) + group_aces = await self._get_group_aces(user_id, privilege_name) + + # Single query for all resources and their pool memberships + query = select(models.Resource).options(selectinload(models.Resource.resource_pools)) + result = await self._db_session.execute(query) + all_resources = result.scalars().all() + + # Precompute pool_id -> set of project_ids + pool_to_projects = {} + for r in all_resources: + if r.resource_type == "project": + for pool in r.resource_pools: + pool_to_projects.setdefault(str(pool.resource_pool_id), set()).add(str(r.resource_id)) + + # --- User ACE: direct path check --- + direct_ace_ids = set() + user_denied = set() + + for pid in all_project_ids: + path = f"/projects/{pid}" + try: + if self._check_path_with_aces(path, user_aces): + direct_ace_ids.add(pid) + except PermissionError: + user_denied.add(pid) + + # --- User ACE: pool check --- + user_pool_ids = {ace_path.split("/")[2] for ace_path, _, ace_allowed, _ in user_aces + if ace_path.startswith("/pools/") and ace_allowed} + + pool_accessible_ids = set() + for pool_id, project_ids in pool_to_projects.items(): + if pool_id in user_pool_ids: + for pid in project_ids: + if pid not in user_denied: + pool_accessible_ids.add(pid) + + # --- Group ACE: direct path check (skip already accessible or denied) --- + for pid in all_project_ids: + if pid in direct_ace_ids or pid in user_denied: + continue + path = f"/projects/{pid}" + try: + if self._check_path_with_aces(path, group_aces): + direct_ace_ids.add(pid) + except PermissionError: + pass + + # --- Group ACE: pool check --- + group_pool_ids = {ace_path.split("/")[2] for ace_path, _, ace_allowed, _ in group_aces + if ace_path.startswith("/pools/") and ace_allowed} + + for pool_id, project_ids in pool_to_projects.items(): + if pool_id in group_pool_ids: + for pid in project_ids: + if pid not in user_denied and pid not in pool_accessible_ids: + pool_accessible_ids.add(pid) + + return direct_ace_ids, pool_accessible_ids + async def check_user_has_privilege(self, user_id: UUID, path: str, privilege_name: str) -> bool: """ Resource paths form a file system like tree and privileges can be inherited by paths down that tree @@ -409,22 +505,26 @@ class RbacRepository(BaseRepository): aces = await self._get_user_aces(user_id, privilege_name) try: + # Check regular ACEs first + if self._check_path_with_aces(path, aces): + # the user has an ACE matching the path and privilege, there is no need to check group ACEs + return True + # Then check resource pool ACEs if path_is_in_pool: if await self._get_resources_in_pools(aces, path): return True - elif self._check_path_with_aces(path, aces): - # the user has an ACE matching the path and privilege, there is no need to check group ACEs - return True except PermissionError: return False aces = await self._get_group_aces(user_id, privilege_name) try: + # Check regular ACEs first + if self._check_path_with_aces(path, aces): + return True + # Then check resource pool ACEs if path_is_in_pool: if await self._get_resources_in_pools(aces, path): return True - elif self._check_path_with_aces(path, aces): - return True except PermissionError: return False return False diff --git a/gns3server/db_migrations/versions/a8829e6c069b_add_llm_config_privileges.py b/gns3server/db_migrations/versions/a8829e6c069b_add_llm_config_privileges.py new file mode 100644 index 000000000..cb87fd15b --- /dev/null +++ b/gns3server/db_migrations/versions/a8829e6c069b_add_llm_config_privileges.py @@ -0,0 +1,113 @@ +"""add llm config privileges to existing database + +Revision ID: a8829e6c069b +Revises: aff810fc119a +Create Date: 2026-05-26 + +""" +from alembic import op +import sqlalchemy as sa +from uuid import uuid4 + +# revision identifiers, used by Alembic. +revision = 'a8829e6c069b' +down_revision = 'aff810fc119a' +branch_labels = None +depends_on = None + +privileges_table = sa.table( + 'privileges', + sa.column('privilege_id', sa.String), + sa.column('name', sa.String), + sa.column('description', sa.String), +) + +roles_table = sa.table( + 'roles', + sa.column('role_id', sa.String), + sa.column('name', sa.String), +) + +privilege_role_map = sa.table( + 'privilege_role_map', + sa.column('privilege_id', sa.String), + sa.column('role_id', sa.String), +) + + +def upgrade() -> None: + conn = op.get_bind() + + # Insert new LLMConfig privileges if they don't already exist + new_privileges = [ + {"name": "LLMConfig.Allocate", "description": "Create or delete an LLM model configuration"}, + {"name": "LLMConfig.Audit", "description": "View an LLM model configuration"}, + {"name": "LLMConfig.Modify", "description": "Update an LLM model configuration"}, + ] + + privilege_ids = {} + for priv in new_privileges: + result = conn.execute( + sa.select(privileges_table.c.privilege_id).where( + privileges_table.c.name == priv["name"] + ) + ).fetchone() + + if result: + privilege_ids[priv["name"]] = result[0] + else: + priv_id = str(uuid4()) + conn.execute( + privileges_table.insert().values( + privilege_id=priv_id, + name=priv["name"], + description=priv["description"], + ) + ) + privilege_ids[priv["name"]] = priv_id + + # Add LLMConfig.Audit and LLMConfig.Modify to the User role + user_role = conn.execute( + sa.select(roles_table.c.role_id).where(roles_table.c.name == "User") + ).fetchone() + + if user_role: + user_role_id = user_role[0] + for priv_name in ("LLMConfig.Audit", "LLMConfig.Modify"): + conn.execute( + privilege_role_map.insert().values( + privilege_id=privilege_ids[priv_name], + role_id=user_role_id, + ) + ) + + +def downgrade() -> None: + conn = op.get_bind() + + user_role = conn.execute( + sa.select(roles_table.c.role_id).where(roles_table.c.name == "User") + ).fetchone() + + if user_role: + user_role_id = user_role[0] + for priv_name in ("LLMConfig.Audit", "LLMConfig.Modify"): + priv = conn.execute( + sa.select(privileges_table.c.privilege_id).where( + privileges_table.c.name == priv_name + ) + ).fetchone() + if priv: + conn.execute( + privilege_role_map.delete().where( + privilege_role_map.c.privilege_id == priv[0], + privilege_role_map.c.role_id == user_role_id, + ) + ) + + for priv_name in ("LLMConfig.Allocate", "LLMConfig.Audit", "LLMConfig.Modify"): + conn.execute( + privileges_table.delete().where( + privileges_table.c.name == priv_name + ) + ) diff --git a/gns3server/schemas/config.py b/gns3server/schemas/config.py index e1a3ab5a9..be66b2853 100644 --- a/gns3server/schemas/config.py +++ b/gns3server/schemas/config.py @@ -159,7 +159,7 @@ class ServerSettings(BaseModel): allow_remote_console: bool = False enable_builtin_templates: bool = True install_builtin_appliances: bool = True - skills_repo_url: str = "https://github.com/yueguobin/GNS3-Skills.git" + skills_repo_url: str = "https://github.com/gns3/gns3-skills.git" skills_repo_branch: str = "main" skills_auto_update: bool = True model_config = ConfigDict(validate_assignment=True, str_strip_whitespace=True) diff --git a/gns3server/utils/packet_filter_validation.py b/gns3server/utils/packet_filter_validation.py new file mode 100644 index 000000000..2ba67f6f5 --- /dev/null +++ b/gns3server/utils/packet_filter_validation.py @@ -0,0 +1,254 @@ +""" +Packet filter parameter validation utilities. +""" + +import logging +import subprocess +from typing import Dict, List, Any, Optional + +log = logging.getLogger(__name__) + + +class FilterValidationError(Exception): + """Raised when packet filter parameters fail validation.""" + pass + + +def validate_bpf_syntax(bpf_expression: str) -> Dict[str, Optional[str]]: + """ + Validate BPF filter expression syntax using tcpdump. + + Uses `tcpdump -d` to compile the BPF expression into filter instructions. + This calls pcap_compile() internally (same as ubridge) but does not + capture traffic, so it returns immediately for both valid and invalid + expressions. + + Args: + bpf_expression: BPF filter expression to validate + + Returns: + dict with 'valid' (bool) and 'error' (str or None) keys + """ + try: + result = subprocess.run( + ["tcpdump", "-d", bpf_expression], + capture_output=True, + text=True, + ) + + if result.returncode != 0: + # Extract meaningful error from tcpdump's stderr + # Skip "Warning: assuming Ethernet" lines, keep only error lines + error_lines = [] + for line in result.stderr.split("\n"): + line = line.strip() + if line and not line.startswith("Warning:"): + # Strip "tcpdump: " prefix + for prefix in ["tcpdump: "]: + if line.startswith(prefix): + line = line[len(prefix):] + error_lines.append(line) + error_msg = " ".join(error_lines) if error_lines else "Invalid BPF expression" + log.warning("BPF syntax validation failed: %s", error_msg) + return {"valid": False, "error": error_msg} + + log.info("BPF syntax validation passed") + return {"valid": True, "error": None} + + except FileNotFoundError: + log.warning( + "tcpdump not found, skipping BPF syntax validation. " + "Install tcpdump to enable BPF validation." + ) + return {"valid": True, "error": None} + + except Exception as e: + log.error("Unexpected error during BPF validation: %s", e) + return {"valid": False, "error": f"BPF validation error: {str(e)}"} + + +def validate_filter_parameters(filter_type: str, values: List[Any]) -> None: + """ + Validate packet filter parameters. + + Args: + filter_type: Type of packet filter + values: List of parameter values + + Raises: + FilterValidationError: If parameters are invalid + """ + + # Define validation rules based on ubridge implementation + VALIDATION_RULES = { + "frequency_drop": { + "params_count": 1, + "ranges": [(-1, 32767)], # min, max + "names": ["Frequency"], + "units": ["th packet"] + }, + "packet_loss": { + "params_count": 1, + "ranges": [(0, 100)], + "names": ["Chance"], + "units": ["%"] + }, + "delay": { + "params_count": 2, # latency, jitter + "ranges": [(1, 32767), (0, 32767)], # ubridge rejects latency <= 0 + "names": ["Latency", "Jitter"], + "units": ["ms", "ms"] + }, + "corrupt": { + "params_count": 1, + "ranges": [(0, 100)], + "names": ["Chance"], + "units": ["%"] + }, + "bpf": { + "params_count": 1, + "is_text": True, + "names": ["Filters"] + } + } + + if filter_type not in VALIDATION_RULES: + raise FilterValidationError(f"Unknown filter type: {filter_type}") + + rules = VALIDATION_RULES[filter_type] + + # Check parameter count + if len(values) != rules["params_count"]: + raise FilterValidationError( + f"{filter_type} expects {rules['params_count']} parameter(s), got {len(values)}" + ) + + # Validate each parameter + for i, value in enumerate(values): + if rules.get("is_text"): + # Text validation (BPF) + if not isinstance(value, str): + raise FilterValidationError( + f"{filter_type} parameter {rules['names'][i]} must be a string" + ) + + # Validate BPF syntax using tshark (same method as gns3_copilot) + # The value may be a multi-line string; each line becomes a + # separate ubridge filter. Validate each line individually. + value = value.strip() + if value: + lines = value.split("\n") + for line_num, line in enumerate(lines): + line = line.strip() + if not line: + continue + bpf_result = validate_bpf_syntax(line) + if not bpf_result["valid"]: + raise FilterValidationError( + f"{filter_type} parameter {rules['names'][i]} line {line_num + 1} " + f"has invalid syntax: {bpf_result['error']}" + ) + else: + # Integer parameter validation + try: + if isinstance(value, str): + value = value.strip() + int_value = int(value) + else: + int_value = int(value) + except (ValueError, TypeError): + raise FilterValidationError( + f"{filter_type} parameter {rules['names'][i]} must be an integer, got: {value}" + ) + + # Range validation + min_val, max_val = rules["ranges"][i] + if int_value < min_val or int_value > max_val: + raise FilterValidationError( + f"{filter_type} parameter {rules['names'][i]} must be between " + f"{min_val} and {max_val} {rules['units'][i]}, got: {int_value}" + ) + + +def filter_inactive_filters(filters: Dict[str, List[Any]]) -> Dict[str, List[Any]]: + """ + Filter out inactive packet filters before validation. + + This function implements smart filtering logic: + - For most filters: value 0 means "disabled" and will be filtered out + - For delay filter: check both latency and jitter to determine intent + * delay: [0, 0] → User wants to disable delay completely, filter it out + * delay: [0, X] where X > 0 → Invalid config (latency must be >= 1), keep for validation error + * delay: [X, X] where X > 0 → Normal configuration, keep for validation + + Args: + filters: Dictionary mapping filter types to their values + + Returns: + Filtered dictionary with only active filters for validation + """ + + if not filters: + return {} + + active_filters = {} + for filter_type, values in filters.items(): + if not values or (isinstance(values, list) and len(values) == 0): + continue + + # Normalize values (strip strings, convert to int) + normalized_values = [] + for value in values: + if isinstance(value, str): + normalized_values.append(value.strip("\n ")) + else: + normalized_values.append(int(value)) + values = normalized_values + + # Skip empty filters after normalization + if len(values) == 0: + continue + + # Special handling for delay filter - check both latency and jitter + if filter_type == "delay": + if len(values) >= 1 and values[0] == 0: # latency = 0 + if len(values) >= 2 and values[1] == 0: # jitter = 0 too + # User intentionally disabling delay completely: [0, 0] + log.debug(f"Filter {filter_type} with values {values} skipped (disabled)") + continue # Skip this filter silently + else: + # Invalid config: latency=0 but jitter>0, keep for validation error + log.debug(f"Filter {filter_type} with values {values} kept for validation (invalid config)") + active_filters[filter_type] = values + else: + # latency>0, normal configuration + active_filters[filter_type] = values + # For other filters, skip if first value is 0 or empty string (means "disabled") + elif values[0] != 0 and values[0] != "": + active_filters[filter_type] = values + else: + # Filters like packet_loss=0, corrupt=0, frequency_drop=0 are intentionally disabled + log.debug(f"Filter {filter_type} with values {values} skipped (disabled)") + + return active_filters + + +def validate_all_filters(filters: Dict[str, List[Any]]) -> None: + """ + Validate all packet filters. + + Args: + filters: Dictionary mapping filter types to their values + + Raises: + FilterValidationError: If any filter is invalid + """ + + if not filters: + return + + for filter_type, values in filters.items(): + if not values or (isinstance(values, list) and len(values) == 0): + continue + + validate_filter_parameters(filter_type, values) \ No newline at end of file diff --git a/scripts/extract_mermaid.py b/scripts/extract_mermaid.py new file mode 100755 index 000000000..6a299fb04 --- /dev/null +++ b/scripts/extract_mermaid.py @@ -0,0 +1,422 @@ +#!/usr/bin/env python3 +""" +Extract mermaid code blocks from Markdown files and convert them to SVG images. + +Requires: Node.js/npx + Chrome (auto-installed via --install) + +Usage: + # Environment setup (first time) + python3 scripts/extract_mermaid.py --install + + # Environment check + python3 scripts/extract_mermaid.py --check + + # Convert all *-overview*.md in default docs directory + python3 scripts/extract_mermaid.py + + # Convert single file, output to default images/ directory + python3 scripts/extract_mermaid.py docs/xxx.md + + # Convert single file, specify output directory + python3 scripts/extract_mermaid.py docs/xxx.md my_svgs/ + + # Scan all .md in a directory + python3 scripts/extract_mermaid.py docs/implemented/ + + # Scan directory, specify output + python3 scripts/extract_mermaid.py docs/implemented/ my_svgs/ +""" + +import re +import os +import sys +import subprocess +import argparse + +# Chrome binary path discovery +PUPPETEER_CACHE = os.environ.get( + "PUPPETEER_CACHE_DIR", + os.path.expanduser("~/.cache/puppeteer"), +) + +DEFAULT_CHROME_PATH = None + + +def _discover_chrome() -> str | None: + """Find Chrome binary in puppeteer cache directory.""" + global DEFAULT_CHROME_PATH + if not os.path.exists(PUPPETEER_CACHE): + return None + for root, dirs, files in os.walk(PUPPETEER_CACHE): + for f in files: + if f == "chrome" and "chrome-linux64" in root: + DEFAULT_CHROME_PATH = os.path.join(root, f) + return DEFAULT_CHROME_PATH + return None + + +_discover_chrome() + +MMDC_PACKAGE = "@mermaid-js/mermaid-cli@11.4.2" +IMG_DIR = "images" + + +def slug(text: str) -> str: + """Convert text to a file-name-safe slug.""" + text = text.lower().strip() + # Keep Chinese characters, replace others with hyphens + text = re.sub(r"[^\w\s一-鿿]", "", text) + text = re.sub(r"\s+", "-", text) + return text.strip("-") + + +def find_mermaid_blocks(content: str): + """Yield (mermaid_code, section_heading, index) tuples.""" + pattern = re.compile(r"```mermaid\n(.*?)```", re.DOTALL) + for i, m in enumerate(pattern.finditer(content)): + mermaid_code = m.group(1).strip() + # Find the nearest section heading above this block + before = content[: m.start()] + headings = re.findall(r"^##+ (.+)$", before, re.MULTILINE) + section = headings[-1] if headings else f"diagram_{i + 1}" + yield mermaid_code, section, i + + +def convert_mermaid_to_svg( + mermaid_code: str, + output_path: str, + chrome_path: str | None = None, + background: str = "transparent", + timeout: int = 30, +) -> bool: + """Convert a mermaid code string to an SVG file.""" + + # Write mermaid code to a temp .mmd file + mmd_path = output_path + ".mmd" + with open(mmd_path, "w") as f: + f.write(mermaid_code) + + try: + env = os.environ.copy() + chrome = chrome_path or DEFAULT_CHROME_PATH + if chrome: + env["PUPPETEER_EXECUTABLE_PATH"] = chrome + + result = subprocess.run( + [ + "npx", "--yes", MMDC_PACKAGE, + "-i", mmd_path, + "-o", output_path, + "-b", background, + ], + capture_output=True, + text=True, + timeout=timeout, + env=env, + ) + + if result.returncode != 0: + print(f" FAILED: {result.stderr.strip()[:200]}", file=sys.stderr) + return False + return True + + except subprocess.TimeoutExpired: + print(f" TIMEOUT after {timeout}s", file=sys.stderr) + return False + except FileNotFoundError: + print( + " ERROR: npx not found. Install Node.js first.", + file=sys.stderr, + ) + return False + except Exception as e: + print(f" ERROR: {e}", file=sys.stderr) + return False + finally: + if os.path.exists(mmd_path): + os.remove(mmd_path) + + +def process_file( + md_path: str, + output_dir: str, + chrome_path: str | None = None, +) -> list[str]: + """Process a single markdown file and return list of generated SVG paths.""" + md_path = os.path.abspath(md_path) + if not os.path.isfile(md_path): + print(f"File not found: {md_path}", file=sys.stderr) + return [] + + with open(md_path) as f: + content = f.read() + + stem = os.path.basename(md_path).replace(".en.md", "").replace(".md", "") + suffix = "-en" if md_path.endswith(".en.md") else "" + + generated = [] + for mermaid_code, section, idx in find_mermaid_blocks(content): + section_slug = slug(section) + img_name = f"{stem}{suffix}-{section_slug}.svg" + img_path = os.path.join(output_dir, img_name) + + print(f" [{idx + 1}] {section} → {img_name} ...", end=" ") + sys.stdout.flush() + + if convert_mermaid_to_svg(mermaid_code, img_path, chrome_path): + print("OK") + generated.append(img_path) + else: + print("") + + return generated + + +def main(): + parser = argparse.ArgumentParser( + description="Extract mermaid diagrams from Markdown and convert to SVG." + ) + parser.add_argument( + "source", + nargs="?", + default=None, + help=( + "Markdown file or directory to scan. " + "Defaults to all *-overview*.md in docs/gns3-copilot/implemented/." + ), + ) + parser.add_argument( + "dest", + nargs="?", + default=None, + help="Output directory for SVGs (default: /images/).", + ) + parser.add_argument( + "--chrome", + default=DEFAULT_CHROME_PATH, + help=f"Path to Chrome binary (auto-detected: {DEFAULT_CHROME_PATH})", + ) + parser.add_argument( + "--background", "-b", + default="transparent", + help="SVG background color (default: transparent)", + ) + parser.add_argument( + "--timeout", "-t", + type=int, + default=30, + help="Timeout in seconds per diagram (default: 30)", + ) + parser.add_argument( + "--check", "-c", + action="store_true", + help="Check environment and exit (no conversion).", + ) + parser.add_argument( + "--install", "-i", + action="store_true", + help="Install missing dependencies (Chrome, mermaid-cli).", + ) + + args = parser.parse_args() + + # ── Environment install mode ── + if args.install: + print("=== Installing Dependencies ===\n") + install_ok = True + + # 1. Check npx + try: + subprocess.run(["npx", "--version"], capture_output=True, timeout=10) + except FileNotFoundError: + print(" ERROR: Node.js/npx not found. Install Node.js first.") + print(" Visit: https://nodejs.org/") + sys.exit(1) + + # 2. Install Chrome via puppeteer + chrome = args.chrome or DEFAULT_CHROME_PATH + if chrome and os.path.exists(chrome): + result = subprocess.run( + [chrome, "--version"], capture_output=True, text=True, timeout=10 + ) + version = result.stdout.strip() if result.returncode == 0 else "unknown" + print(f" Chrome: already installed ({version})") + else: + print(" Chrome: installing via puppeteer...") + ret = subprocess.run( + ["npx", "puppeteer", "browsers", "install", "chrome-headless-shell"], + timeout=120, + ) + if ret.returncode == 0: + print(" Chrome: installed successfully") + # Re-discover Chrome path + _discover_chrome() + else: + print(" Chrome: installation failed") + install_ok = False + + # 3. Pre-cache mermaid-cli + print(" mermaid-cli: caching...") + ret = subprocess.run( + ["npx", "--yes", MMDC_PACKAGE, "--version"], + capture_output=True, text=True, timeout=60, + ) + if ret.returncode == 0: + print(" mermaid-cli: ready") + else: + print(" mermaid-cli: download failed") + install_ok = False + + print(f"\n Result: {'✓ INSTALLATION COMPLETE' if install_ok else '✗ SOME INSTALLATIONS FAILED'}") + + # Auto-run check after install + print() + args.check = True + # Fall through to check below (args.check is now True) + + # ── Environment check mode ── + if args.check: + ok = True + + print("=== Environment Check ===\n") + + # 1. Python + print(f" Python: {sys.version.split()[0]}") + + # 2. npx / Node.js + try: + result = subprocess.run( + ["npx", "--version"], + capture_output=True, text=True, timeout=10 + ) + if result.returncode == 0: + print(f" npx: {result.stdout.strip()}") + else: + print(" npx: NOT FOUND") + ok = False + except FileNotFoundError: + print(" npx: NOT FOUND (Node.js not installed)") + ok = False + + # 3. Chrome + chrome = args.chrome or DEFAULT_CHROME_PATH + if chrome and os.path.exists(chrome): + try: + result = subprocess.run( + [chrome, "--version"], + capture_output=True, text=True, timeout=10 + ) + version = result.stdout.strip() if result.returncode == 0 else "?" + print(f" Chrome: {version}") + print(f" Path: {chrome}") + except Exception: + print(f" Chrome: {chrome}") + else: + print(" Chrome: NOT FOUND") + print(" Run: npx puppeteer browsers install chrome-headless-shell") + ok = False + + # 4. @mermaid-js/mermaid-cli + try: + result = subprocess.run( + ["npx", "--yes", MMDC_PACKAGE, "--version"], + capture_output=True, text=True, timeout=30 + ) + if result.returncode == 0: + print(" mermaid-cli: available") + else: + print(" mermaid-cli: download failed") + ok = False + except Exception: + print(" mermaid-cli: FAILED") + ok = False + + # 5. Puppeteer cache + if os.path.exists(PUPPETEER_CACHE): + print(f" Puppeteer cache: {PUPPETEER_CACHE}") + for item in sorted(os.listdir(PUPPETEER_CACHE)): + item_path = os.path.join(PUPPETEER_CACHE, item) + if os.path.isdir(item_path): + versions = os.listdir(item_path) + print(f" {item}: {', '.join(versions)}") + else: + print(f" Puppeteer cache: NOT FOUND") + + print(f"\n Result: {'✓ ALL CHECKS PASSED' if ok else '✗ SOME CHECKS FAILED'}") + sys.exit(0 if ok else 1) + + args = parser.parse_args() + + # Determine source files + files_to_process = [] + if args.source: + if os.path.isfile(args.source): + files_to_process.append(args.source) + elif os.path.isdir(args.source): + for f in sorted(os.listdir(args.source)): + if f.endswith(".md"): + files_to_process.append(os.path.join(args.source, f)) + else: + print(f"Source not found: {args.source}", file=sys.stderr) + sys.exit(1) + else: + # Default: scan project docs directory + script_dir = os.path.dirname(os.path.abspath(__file__)) + project_root = os.path.dirname(script_dir) + docs_dir = os.path.join(project_root, "docs", "gns3-copilot", "implemented") + if os.path.isdir(docs_dir): + files_to_process = sorted( + os.path.join(docs_dir, f) + for f in os.listdir(docs_dir) + if "-overview" in f and f.endswith(".md") + ) + else: + print("No source specified and default docs directory not found.", + file=sys.stderr) + sys.exit(1) + + if not files_to_process: + print("No markdown files found.", file=sys.stderr) + sys.exit(1) + + # Determine output directory + if args.dest: + output_base = args.dest + elif args.source and os.path.isfile(args.source): + output_base = os.path.join(os.path.dirname(args.source), IMG_DIR) + elif args.source and os.path.isdir(args.source): + output_base = os.path.join(args.source, IMG_DIR) + else: + # Default: use directory of first file + output_base = os.path.join(os.path.dirname(files_to_process[0]), IMG_DIR) + + os.makedirs(output_base, exist_ok=True) + + # Chrome check + chrome = args.chrome or DEFAULT_CHROME_PATH + if not chrome: + print( + "WARNING: Chrome not found in puppeteer cache. " + "Run: npx puppeteer browsers install chrome-headless-shell", + file=sys.stderr, + ) + else: + print(f"Using Chrome: {chrome}") + print(f"Output: {output_base}") + + print(f"\nProcessing {len(files_to_process)} file(s)...\n") + + total_svgs = 0 + for md_path in files_to_process: + rel = os.path.relpath(md_path) + print(f"== {rel} ==") + + svgs = process_file(md_path, output_base, chrome) + total_svgs += len(svgs) + print() + + print(f"Done. Generated {total_svgs} SVG(s).") + + +if __name__ == "__main__": + main() diff --git a/scripts/remote-install.sh b/scripts/remote-install.sh index c63fcd0b2..bc16f4658 100644 --- a/scripts/remote-install.sh +++ b/scripts/remote-install.sh @@ -226,12 +226,11 @@ Conflicts=shutdown.target [Service] User=gns3 Group=gns3 -PermissionsStartOnly=true EnvironmentFile=/etc/environment -ExecStartPre=/bin/mkdir -p /var/log/gns3 /var/run/gns3 -ExecStartPre=/bin/chown -R gns3:gns3 /var/log/gns3 /var/run/gns3 +ExecStartPre=+/bin/mkdir -p /var/log/gns3 /var/run/gns3 +ExecStartPre=+/bin/chown -R gns3:gns3 /var/log/gns3 /var/run/gns3 ExecStart=/usr/bin/gns3server --log /var/log/gns3/gns3.log -ExecReload=/bin/kill -s HUP $MAINPID +ExecReload=+/bin/kill -s HUP $MAINPID Restart=on-failure RestartSec=5 LimitNOFILE=16384 diff --git a/scripts/update-bundled-web-ui.sh b/scripts/update-bundled-web-ui.sh index ba4a9fe80..70742e253 100755 --- a/scripts/update-bundled-web-ui.sh +++ b/scripts/update-bundled-web-ui.sh @@ -104,11 +104,11 @@ if [ "$CUSTOM_REPO" = false ] ; then git checkout "$BRANCH" git pull else - git checkout master-3.0 + git checkout 3.1 git pull fi else - git checkout master-3.0 + git checkout 3.1 git fetch --tags git pull fi diff --git a/tests/api/routes/controller/test_links.py b/tests/api/routes/controller/test_links.py index 5068fc8ea..bc50e053c 100644 --- a/tests/api/routes/controller/test_links.py +++ b/tests/api/routes/controller/test_links.py @@ -63,10 +63,10 @@ class TestLinkRoutes: node1, node2 = nodes filters = { - "latency": [10], + "delay": [10, 0], "frequency_drop": [50] } - + with asyncio_patch("gns3server.controller.udp_link.UDPLink.create") as mock: response = await client.post(app.url_path_for("create_link", project_id=project.id), json={ "nodes": [ @@ -88,7 +88,7 @@ class TestLinkRoutes: ], "filters": filters }) - + assert mock.called assert response.status_code == status.HTTP_201_CREATED assert response.json()["link_id"] is not None @@ -250,10 +250,10 @@ class TestLinkRoutes: ) -> None: filters = { - "latency": [10], + "delay": [10, 0], "frequency_drop": [50] } - + node1, node2 = nodes with asyncio_patch("gns3server.controller.udp_link.UDPLink.create") as mock: response = await client.post(app.url_path_for("create_link", project_id=project.id), json={ @@ -315,7 +315,7 @@ class TestLinkRoutes: ) -> None: filters = { - "latency": [10], + "delay": [10, 0], "frequency_drop": [50] } diff --git a/tests/api/routes/controller/test_roles.py b/tests/api/routes/controller/test_roles.py index b6fca22b6..a98be8bf0 100644 --- a/tests/api/routes/controller/test_roles.py +++ b/tests/api/routes/controller/test_roles.py @@ -125,7 +125,7 @@ class TestRolesPrivilegesRoutes: ) assert response.status_code == status.HTTP_204_NO_CONTENT privileges = await rbac_repo.get_role_privileges(role_in_db.role_id) - assert len(privileges) == 25 # 24 default privileges + 1 custom privilege + assert len(privileges) == 27 # 25 default privileges + 2 LLMConfig privileges async def test_get_role_privileges( self, @@ -143,7 +143,7 @@ class TestRolesPrivilegesRoutes: role_id=role_in_db.role_id) ) assert response.status_code == status.HTTP_200_OK - assert len(response.json()) == 25 # 24 default privileges + 1 custom privilege + assert len(response.json()) == 27 # 25 default privileges + 2 LLMConfig privileges async def test_remove_privilege_from_role( self, @@ -165,4 +165,4 @@ class TestRolesPrivilegesRoutes: ) assert response.status_code == status.HTTP_204_NO_CONTENT privileges = await rbac_repo.get_role_privileges(role_in_db.role_id) - assert len(privileges) == 24 # 24 default privileges + assert len(privileges) == 26 # 26 default privileges (24 + 2 LLMConfig) diff --git a/tests/controller/test_project.py b/tests/controller/test_project.py index 24d8aa033..24f9314ce 100644 --- a/tests/controller/test_project.py +++ b/tests/controller/test_project.py @@ -75,7 +75,7 @@ async def test_json(): "scene_height": 1000, "zoom": 100, "show_grid": False, - "show_interface_labels": False, + "show_interface_labels": True, "show_layers": False, "snap_to_grid": False, "grid_size": 75, diff --git a/tests/controller/test_rbac.py b/tests/controller/test_rbac.py index 7dde61e4b..29fb38f4e 100644 --- a/tests/controller/test_rbac.py +++ b/tests/controller/test_rbac.py @@ -21,6 +21,7 @@ import uuid from fastapi import FastAPI, status from httpx import AsyncClient +from httpx_ws.transport import ASGIWebSocketTransport from sqlalchemy.ext.asyncio import AsyncSession from gns3server.controller import Controller @@ -30,6 +31,7 @@ from gns3server.db.repositories.pools import ResourcePoolsRepository from gns3server.schemas.controller.rbac import ACECreate from gns3server.schemas.controller.pools import ResourceCreate, ResourcePoolCreate from gns3server.db.models import User +from gns3server.services import auth_service pytestmark = pytest.mark.asyncio @@ -168,22 +170,23 @@ class TestResourcePools: self, app: FastAPI, controller: Controller, - authorized_client: AsyncClient, - db_session: AsyncSession + base_client: AsyncClient, + db_session: AsyncSession, + test_user: User ) -> None: + # Clean up any existing ACEs from previous tests + await RbacRepository(db_session).delete_all_ace_starting_with_path("/projects") + await RbacRepository(db_session).delete_all_ace_starting_with_path("/pools") + uuid1 = str(uuid.uuid4()) uuid2 = str(uuid.uuid4()) uuid3 = str(uuid.uuid4()) - await controller.add_project(project_id=uuid1, name="Project1") - await controller.add_project(project_id=uuid2, name="Project2") - await controller.add_project(project_id=uuid3, name="Project3") - - # user has no access to projects (no ACE on /projects or resource pools) - response = await authorized_client.get(app.url_path_for("get_projects")) - assert response.status_code == status.HTTP_200_OK - assert len(response.json()) == 0 + await controller.add_project(project_id=uuid1, name="Project1", created_by=test_user.username) + await controller.add_project(project_id=uuid2, name="Project2", created_by=test_user.username) + await controller.add_project(project_id=uuid3, name="Project3", created_by=test_user.username) + # Create resource pool and add uuid2 to it pools_repo = ResourcePoolsRepository(db_session) new_resource_pool = ResourcePoolCreate(name="pool2") pool_in_db = await pools_repo.create_resource_pool(new_resource_pool) @@ -194,6 +197,8 @@ class TestResourcePools: group_id = (await UsersRepository(db_session).get_user_group_by_name("Users")).user_group_id role_id = (await RbacRepository(db_session).get_role_by_name("User")).role_id + + # Give user access to resource pool only ace = ACECreate( path=f"/pools/{pool_in_db.resource_pool_id}", ace_type="group", @@ -203,40 +208,47 @@ class TestResourcePools: ) await RbacRepository(db_session).create_ace(ace) - response = await authorized_client.get(app.url_path_for("get_project", project_id=uuid2)) - assert response.status_code == status.HTTP_200_OK - assert response.json()["name"] == "Project2" + # Create a new client with test user authentication + access_token = auth_service.create_access_token(test_user.username) + async with AsyncClient( + base_url="http://test-api", + headers={"Content-Type": "application/json", "Authorization": f"Bearer {access_token}"}, + transport=ASGIWebSocketTransport(app=app) + ) as user_client: + # user should see only uuid2 (from resource pool) + response = await user_client.get(app.url_path_for("get_projects")) + assert response.status_code == status.HTTP_200_OK + projects = response.json() + assert len(projects) == 1 + assert projects[0]["project_id"] == uuid2 - # user should only see one project because it is in the resource pool he has access to - response = await authorized_client.get(app.url_path_for("get_projects")) - assert response.status_code == status.HTTP_200_OK - projects = response.json() - assert len(projects) == 1 - assert projects[0]["project_id"] == uuid2 + response = await user_client.get(app.url_path_for("get_project", project_id=uuid2)) + assert response.status_code == status.HTTP_200_OK + assert response.json()["name"] == "Project2" - ace = ACECreate( - path=f"/projects", - ace_type="group", - propagate=True, - group_id=str(group_id), - role_id=str(role_id) - ) - await RbacRepository(db_session).create_ace(ace) + # Now give user access to /projects (in addition to resource pool) + ace = ACECreate( + path="/projects", + ace_type="group", + propagate=True, + group_id=str(group_id), + role_id=str(role_id) + ) + await RbacRepository(db_session).create_ace(ace) - # now user should see all projects because he has access to /projects and the resource pool - response = await authorized_client.get(app.url_path_for("get_projects")) - assert response.status_code == status.HTTP_200_OK - projects = response.json() - assert len(projects) == 3 + # user should see all 3 projects: 3 from /projects ACE (uuid2 also in pool but deduplicated) + response = await user_client.get(app.url_path_for("get_projects")) + assert response.status_code == status.HTTP_200_OK + projects = response.json() + assert len(projects) == 3 - await RbacRepository(db_session).delete_all_ace_starting_with_path(f"/pools/{pool_in_db.resource_pool_id}") - response = await authorized_client.get(app.url_path_for("get_project", project_id=uuid2)) - assert response.status_code == status.HTTP_403_FORBIDDEN + # Remove resource pool ACE + await RbacRepository(db_session).delete_all_ace_starting_with_path(f"/pools/{pool_in_db.resource_pool_id}") - # now user should only see the projects that are not in a resource pool - response = await authorized_client.get(app.url_path_for("get_projects")) - assert response.status_code == status.HTTP_200_OK - assert len(response.json()) == 2 + # user should still see all 3 projects via /projects ACE (created_by check) + response = await user_client.get(app.url_path_for("get_projects")) + assert response.status_code == status.HTTP_200_OK + assert len(response.json()) == 3 # class TestProjectsWithRbac: diff --git a/tests/controller/test_topology.py b/tests/controller/test_topology.py index 9e1f72f82..85492596c 100644 --- a/tests/controller/test_topology.py +++ b/tests/controller/test_topology.py @@ -45,7 +45,7 @@ async def test_project_to_topology_empty(tmpdir): "revision": GNS3_FILE_FORMAT_REVISION, "zoom": 100, "show_grid": False, - "show_interface_labels": False, + "show_interface_labels": True, "show_layers": False, "snap_to_grid": False, "grid_size": 75, diff --git a/tests/controller/test_udp_link.py b/tests/controller/test_udp_link.py index 22a7bb50f..fb03947c9 100644 --- a/tests/controller/test_udp_link.py +++ b/tests/controller/test_udp_link.py @@ -46,7 +46,7 @@ async def test_create(project): link = UDPLink(project) await link.add_node(node1, 0, 4) - await link.update_filters({"latency": [10]}) + await link.update_filters({"delay": [10, 0]}) async def compute1_callback(path, data={}, **kwargs): """ @@ -77,7 +77,7 @@ async def test_create(project): "rhost": "192.168.1.2", "rport": 2048, "type": "nio_udp", - "filters": {"latency": [10]}, + "filters": {"delay": [10, 0]}, "suspend": False, }, timeout=120) @@ -313,7 +313,7 @@ async def test_update(project): link = UDPLink(project) await link.add_node(node1, 0, 4) - await link.update_filters({"latency": [10]}) + await link.update_filters({"delay": [10, 0]}) async def compute1_callback(path, data={}, **kwargs): """ @@ -345,7 +345,7 @@ async def test_update(project): "rport": 2048, "type": "nio_udp", "suspend": False, - "filters": {"latency": [10]} + "filters": {"delay": [10, 0]} }, timeout=120) compute2.post.assert_any_call("/projects/{}/vpcs/nodes/{}/adapters/3/ports/1/nio".format(project.id, node2.id), data={ @@ -358,7 +358,7 @@ async def test_update(project): }, timeout=120) assert link.created - await link.update_filters({"drop": [5], "bpf": ["icmp[icmptype] == 8"]}) + await link.update_filters({"frequency_drop": [5], "bpf": ["icmp[icmptype] == 8"]}) compute1.put.assert_any_call("/projects/{}/vpcs/nodes/{}/adapters/0/ports/4/nio".format(project.id, node1.id), data={ "lport": 1024, "rhost": "192.168.1.2", @@ -366,7 +366,7 @@ async def test_update(project): "type": "nio_udp", "suspend": False, "filters": { - "drop": [5], + "frequency_drop": [5], "bpf": ["icmp[icmptype] == 8"] } }, timeout=120) @@ -392,7 +392,7 @@ async def test_update_suspend(project): link = UDPLink(project) await link.add_node(node1, 0, 4) - await link.update_filters({"latency": [10]}) + await link.update_filters({"frequency_drop": [-1]}) await link.update_suspend(True) async def compute1_callback(path, data={}, **kwargs): diff --git a/tests/stress/benchmark_running_server.py b/tests/stress/benchmark_running_server.py new file mode 100644 index 000000000..9ee61ccc4 --- /dev/null +++ b/tests/stress/benchmark_running_server.py @@ -0,0 +1,297 @@ +#!/usr/bin/env python3 +""" +Benchmark test for GET /projects performance on a running GNS3 server + +This test connects to an already running GNS3 server and measures +the response time of the GET /projects endpoint. +""" + +import asyncio +import time +import argparse +import sys +import uuid +from pathlib import Path + +import httpx + +# Add parent directory to path +sys.path.insert(0, str(Path(__file__).parent.parent.parent)) + + +async def create_test_projects(base_url, headers, count, prefix="perf_test_project"): + """Create specified number of test projects""" + print(f"Creating {count} test projects...") + + async with httpx.AsyncClient() as client: + batch_size = 50 + + for batch_start in range(0, count, batch_size): + batch_end = min(batch_start + batch_size, count) + batch_tasks = [] + + for i in range(batch_start, batch_end): + name = f"{prefix}_{i}_{uuid.uuid4().hex[:8]}" + task = client.post( + f"{base_url}/projects", + headers=headers, + json={"name": name} + ) + batch_tasks.append(task) + + results = await asyncio.gather(*batch_tasks, return_exceptions=True) + + for i, result in enumerate(results, batch_start): + if hasattr(result, 'status_code'): + if result.status_code == 201: + print(f" Created project {i+1}/{count}") + else: + print(f" Failed to create project {i+1}: {result.status_code}") + else: + print(f" Error creating project {i+1}: {result}") + + +async def cleanup_test_projects(base_url, headers, prefix="perf_test_project"): + """Clean up test projects""" + print("Cleaning up test projects...") + cleanup_start = time.time() + + try: + async with httpx.AsyncClient(timeout=60.0) as client: + response = await client.get(f"{base_url}/projects", headers=headers) + if response.status_code == 200: + projects = response.json() + + test_projects = [p for p in projects if p.get('name', '').startswith(prefix)] + print(f" Found {len(test_projects)} test projects to delete") + + batch_size = 50 + total_deleted = 0 + for batch_start in range(0, len(test_projects), batch_size): + batch_end = min(batch_start + batch_size, len(test_projects)) + batch_tasks = [] + + for project in test_projects[batch_start:batch_end]: + task = client.delete( + f"{base_url}/projects/{project['project_id']}", + headers=headers + ) + batch_tasks.append(task) + + results = await asyncio.gather(*batch_tasks, return_exceptions=True) + + for i, result in enumerate(results, batch_start): + project = test_projects[i] + if hasattr(result, 'status_code') and result.status_code == 204: + total_deleted += 1 + + elapsed = time.time() - cleanup_start + print(f" Deleted {total_deleted} projects in {elapsed:.1f}s ({total_deleted/elapsed:.0f} projects/sec)") + + except Exception as e: + print(f"Error during cleanup: {type(e).__name__}: {e}") + + +async def benchmark_get_projects(base_url, headers, project_count, iterations=10, prefix="perf_test_project"): + """Benchmark GET /projects endpoint""" + + print(f"\n{'='*60}") + print(f"GET /projects Performance Benchmark") + print(f"{'='*60}") + print(f"Server: {base_url}") + print(f"{'='*60}\n") + + if project_count > 0: + await create_test_projects(base_url, headers, project_count, prefix) + + # Warm-up run + print("Performing warm-up run...") + try: + async with httpx.AsyncClient(timeout=120.0) as client: + response = await client.get(f"{base_url}/projects", headers=headers) + print(f" Response status: {response.status_code}") + if response.status_code != 200: + print(f"Error during warm-up: HTTP {response.status_code}") + print(f"Response text: {response.text[:200]}") + return + actual_count = len(response.json()) + print(f" Warm-up successful: {actual_count} projects") + except Exception as e: + print(f"Error during warm-up: {type(e).__name__}: {e}") + return + + # Benchmark runs + response_times = [] + project_counts = [] + + print(f"\nRunning {iterations} benchmark iterations...") + async with httpx.AsyncClient(timeout=120.0) as client: + for i in range(iterations): + start_time = time.time() + try: + response = await client.get(f"{base_url}/projects", headers=headers) + end_time = time.time() + + if response.status_code == 200: + elapsed_ms = (end_time - start_time) * 1000 + projects = response.json() + response_times.append(elapsed_ms) + project_counts.append(len(projects)) + print(f" Iteration {i+1}: {elapsed_ms:.2f}ms ({len(projects)} projects)") + else: + print(f" Iteration {i+1}: ERROR {response.status_code}") + except Exception as e: + print(f" Iteration {i+1}: Exception - {type(e).__name__}: {str(e)}") + + # Cleanup test projects + if project_count > 0: + await cleanup_test_projects(base_url, headers, prefix) + + # Calculate statistics + if response_times: + avg_time = sum(response_times) / len(response_times) + min_time = min(response_times) + max_time = max(response_times) + median_time = sorted(response_times)[len(response_times) // 2] + avg_project_count = sum(project_counts) / len(project_counts) if project_counts else 0 + + print(f"\n{'='*60}") + if project_count > 0: + print(f"Results for {project_count} projects (created specifically):") + else: + print(f"Results for existing projects (~{avg_project_count:.0f} per request):") + print(f"{'='*60}") + print(f"Average: {avg_time:.2f}ms") + print(f"Median: {median_time:.2f}ms") + print(f"Min: {min_time:.2f}ms") + print(f"Max: {max_time:.2f}ms") + print(f"{'='*60}") + + # Performance assessment + if avg_time < 100: + status = "EXCELLENT" + elif avg_time < 500: + status = "ACCEPTABLE" + elif avg_time < 2000: + status = "SLOW" + else: + status = "CRITICAL" + + print(f"Status: {status}") + + # Throughput: projects returned per second + if project_counts and sum(response_times) > 0: + total_projects = sum(project_counts) + total_seconds = sum(response_times) / 1000 + projects_per_second = total_projects / total_seconds + print(f"Throughput: {projects_per_second:.1f} projects/sec") + print(f"{'='*60}\n") + + return { + 'project_count': project_count, + 'avg_time_ms': avg_time, + 'median_time_ms': median_time, + 'min_time_ms': min_time, + 'max_time_ms': max_time, + 'projects_per_second': projects_per_second if project_counts and sum(response_times) > 0 else 0 + } + + +async def main(): + parser = argparse.ArgumentParser(description='Benchmark GET /projects performance on running server') + parser.add_argument('--host', type=str, default='127.0.0.1', help='GNS3 server host') + parser.add_argument('--port', type=int, default=3080, help='GNS3 server port') + parser.add_argument('--username', type=str, required=True, help='Username for authentication') + parser.add_argument('--password', type=str, required=True, help='Password for authentication') + parser.add_argument('--project-counts', type=int, nargs='+', default=[10, 50, 100], + help='Numbers of projects to test (default: 10 50 100)') + parser.add_argument('--iterations', type=int, default=10, + help='Number of iterations per test (default: 10)') + parser.add_argument('--full-scale', action='store_true', + help='Run full scale test: 10, 50, 100, 200, 500 projects') + parser.add_argument('--no-cleanup', action='store_true', + help='Do not create/cleanup test projects (use existing projects)') + parser.add_argument('--prefix', type=str, default='perf_test_project', + help='Project name prefix for test/cleanup (default: perf_test_project)') + parser.add_argument('--cleanup-only', action='store_true', + help='Only clean up test projects, do not run benchmark') + + args = parser.parse_args() + + base_url = f"http://{args.host}:{args.port}/v3" + + # Login to get JWT token + print(f"Connecting to {base_url}") + print(f"Authenticating as {args.username}...") + + try: + async with httpx.AsyncClient() as client: + login_response = await client.post( + f"{base_url}/access/users/login", + data={"username": args.username, "password": args.password} + ) + + if login_response.status_code != 200: + print(f"Login failed: {login_response.status_code}") + print(f"Response: {login_response.text}") + sys.exit(1) + + token_data = login_response.json() + access_token = token_data.get("access_token") + headers = {"Authorization": f"Bearer {access_token}"} + print("Authentication successful") + + except Exception as e: + print(f"Error connecting to server: {e}") + print("Make sure GNS3 server is running and accessible") + sys.exit(1) + + # Determine test scope + if args.full_scale: + project_counts = [10, 50, 100, 200, 500] + else: + project_counts = args.project_counts + + # If cleanup-only flag, just clean up and exit + if args.cleanup_only: + await cleanup_test_projects(base_url, headers, args.prefix) + return + + # If no-cleanup flag, test with existing projects as-is + if args.no_cleanup: + project_counts = [0] + args.iterations = 20 + + results = [] + + for count in project_counts: + try: + result = await benchmark_get_projects(base_url, headers, count, args.iterations, args.prefix) + if result: + results.append(result) + except Exception as e: + print(f"Error testing {count} projects: {e}") + + # Summary + if len(results) > 1: + print(f"\n{'='*60}") + print("Summary - Performance Scaling") + print(f"{'='*60}") + print(f"{'Projects':<12} {'Avg (ms)':<12} {'Status':<12}") + print(f"{'-'*40}") + for r in results: + avg = r['avg_time_ms'] + if avg < 100: + status = "EXCELLENT" + elif avg < 500: + status = "ACCEPTABLE" + elif avg < 2000: + status = "SLOW" + else: + status = "CRITICAL" + print(f"{r['project_count']:<12} {avg:<12.2f} {status:<12}") + print(f"{'='*60}\n") + + +if __name__ == "__main__": + asyncio.run(main()) diff --git a/tests/utils/test_packet_filter_validation.py b/tests/utils/test_packet_filter_validation.py new file mode 100644 index 000000000..d15ae1fc3 --- /dev/null +++ b/tests/utils/test_packet_filter_validation.py @@ -0,0 +1,265 @@ +""" +Unit tests for packet filter validation. +""" + +import pytest +from gns3server.utils.packet_filter_validation import ( + validate_filter_parameters, + validate_all_filters, + filter_inactive_filters, + FilterValidationError +) + + +class TestPacketFilterValidation: + """Test packet filter parameter validation.""" + + def test_frequency_drop_valid(self): + """Test valid frequency drop parameters.""" + # Valid range: -1 to 32767 + validate_filter_parameters("frequency_drop", [-1]) + validate_filter_parameters("frequency_drop", [1]) + validate_filter_parameters("frequency_drop", [100]) + validate_filter_parameters("frequency_drop", [32767]) + + def test_frequency_drop_invalid(self): + """Test invalid frequency drop parameters.""" + # Too low + with pytest.raises(FilterValidationError, match="between -1 and 32767"): + validate_filter_parameters("frequency_drop", [-2]) + + # Too high + with pytest.raises(FilterValidationError, match="between -1 and 32767"): + validate_filter_parameters("frequency_drop", [32768]) + + # Wrong type + with pytest.raises(FilterValidationError, match="must be an integer"): + validate_filter_parameters("frequency_drop", ["invalid"]) + + def test_packet_loss_valid(self): + """Test valid packet loss parameters.""" + # Valid range: 0-100% + validate_filter_parameters("packet_loss", [0]) + validate_filter_parameters("packet_loss", [50]) + validate_filter_parameters("packet_loss", [100]) + + def test_packet_loss_invalid(self): + """Test invalid packet loss parameters.""" + # Negative + with pytest.raises(FilterValidationError, match="between 0 and 100"): + validate_filter_parameters("packet_loss", [-1]) + + # Over 100% + with pytest.raises(FilterValidationError, match="between 0 and 100"): + validate_filter_parameters("packet_loss", [101]) + + def test_delay_valid(self): + """Test valid delay parameters.""" + # Valid range: 1-32767ms latency, 0-32767ms jitter + validate_filter_parameters("delay", [1, 0]) + validate_filter_parameters("delay", [100, 50]) + validate_filter_parameters("delay", [32767, 32767]) + + def test_delay_invalid(self): + """Test invalid delay parameters.""" + # Zero latency (ubridge rejects latency <= 0) + with pytest.raises(FilterValidationError, match="between 1 and 32767"): + validate_filter_parameters("delay", [0, 0]) + + # Negative latency + with pytest.raises(FilterValidationError, match="between 1 and 32767"): + validate_filter_parameters("delay", [-1, 0]) + + # Over max + with pytest.raises(FilterValidationError, match="between 1 and 32767"): + validate_filter_parameters("delay", [32768, 0]) + + # Negative jitter + with pytest.raises(FilterValidationError, match="between 0 and 32767"): + validate_filter_parameters("delay", [100, -1]) + + def test_corrupt_valid(self): + """Test valid corrupt parameters.""" + # Valid range: 0-100% + validate_filter_parameters("corrupt", [0]) + validate_filter_parameters("corrupt", [50]) + validate_filter_parameters("corrupt", [100]) + + def test_corrupt_invalid(self): + """Test invalid corrupt parameters.""" + # Over 100% + with pytest.raises(FilterValidationError, match="between 0 and 100"): + validate_filter_parameters("corrupt", [101]) + + def test_bpf_valid(self): + """Test valid BPF parameters.""" + validate_filter_parameters("bpf", ["tcp port 80"]) + validate_filter_parameters("bpf", ["tcp and not port 22"]) + validate_filter_parameters("bpf", [""]) # Empty is valid + validate_filter_parameters("bpf", ["host 192.168.1.1 and port 443"]) + + def test_bpf_multi_line_valid(self): + """Test valid multi-line BPF expressions.""" + validate_filter_parameters("bpf", ["tcp port 80\nnot arp"]) + validate_filter_parameters("bpf", ["tcp and not port 22\nhost 192.168.1.1\nicmp"]) + + def test_bpf_multi_line_invalid(self): + """Test multi-line BPF with invalid line.""" + with pytest.raises(FilterValidationError) as excinfo: + validate_filter_parameters("bpf", ["tcp port 80\ninvalid!!!"]) + err = str(excinfo.value).lower() + assert "syntax error" in err + + def test_bpf_invalid(self): + """Test invalid BPF parameters.""" + # Wrong type + with pytest.raises(FilterValidationError, match="must be a string"): + validate_filter_parameters("bpf", [123]) + + # Invalid BPF syntax + with pytest.raises(FilterValidationError) as excinfo: + validate_filter_parameters("bpf", ["tcp port"]) # Missing port number + assert "syntax error" in str(excinfo.value).lower() + + def test_parameter_count_mismatch(self): + """Test wrong number of parameters.""" + # frequency_drop expects 1 parameter + with pytest.raises(FilterValidationError, match="expects 1 parameter"): + validate_filter_parameters("frequency_drop", []) + + with pytest.raises(FilterValidationError, match="expects 1 parameter"): + validate_filter_parameters("frequency_drop", [1, 2]) + + # delay expects 2 parameters + with pytest.raises(FilterValidationError, match="expects 2 parameter"): + validate_filter_parameters("delay", [100]) + + def test_string_to_int_conversion(self): + """Test string to integer conversion.""" + # Should work with string numbers + validate_filter_parameters("frequency_drop", ["10"]) + validate_filter_parameters("packet_loss", ["50"]) + validate_filter_parameters("delay", ["100", "50"]) + + def test_validate_all_filters(self): + """Test validating multiple filters at once.""" + filters = { + "frequency_drop": [10], + "delay": [100, 50] + } + validate_all_filters(filters) # Should not raise + + def test_validate_all_filters_with_invalid(self): + """Test validate_all_filters with invalid filter.""" + filters = { + "frequency_drop": [10], + "packet_loss": [150] # Invalid: over 100% + } + with pytest.raises(FilterValidationError): + validate_all_filters(filters) + + def test_unknown_filter_type(self): + """Test unknown filter type.""" + with pytest.raises(FilterValidationError, match="Unknown filter type"): + validate_filter_parameters("unknown_filter", [1]) + + +class TestFilterInactiveFilters: + """Test filter_inactive_filters function for smart filter filtering logic.""" + + def test_filter_inactive_delay_disabled(self): + """Test delay [0, 0] is filtered out (user wants to disable delay).""" + filters = {"delay": [0, 0]} + result = filter_inactive_filters(filters) + assert result == {} # Should be filtered out + + def test_filter_inactive_delay_invalid_config(self): + """Test delay [0, 100] is kept for validation (invalid config).""" + filters = {"delay": [0, 100]} + result = filter_inactive_filters(filters) + assert result == {"delay": [0, 100]} # Should be kept for validation error + + def test_filter_inactive_delay_normal_config(self): + """Test delay [100, 20] is kept (normal configuration).""" + filters = {"delay": [100, 20]} + result = filter_inactive_filters(filters) + assert result == {"delay": [100, 20]} # Should be kept + + def test_filter_inactive_delay_zero_jitter(self): + """Test delay [100, 0] is kept (normal config with zero jitter).""" + filters = {"delay": [100, 0]} + result = filter_inactive_filters(filters) + assert result == {"delay": [100, 0]} # Should be kept + + def test_filter_inactive_packet_loss_zero(self): + """Test packet_loss [0] is filtered out (disabled).""" + filters = {"packet_loss": [0]} + result = filter_inactive_filters(filters) + assert result == {} # Should be filtered out + + def test_filter_inactive_packet_loss_active(self): + """Test packet_loss [5] is kept (active).""" + filters = {"packet_loss": [5]} + result = filter_inactive_filters(filters) + assert result == {"packet_loss": [5]} # Should be kept + + def test_filter_inactive_corrupt_zero(self): + """Test corrupt [0] is filtered out (disabled).""" + filters = {"corrupt": [0]} + result = filter_inactive_filters(filters) + assert result == {} # Should be filtered out + + def test_filter_inactive_corrupt_active(self): + """Test corrupt [2] is kept (active).""" + filters = {"corrupt": [2]} + result = filter_inactive_filters(filters) + assert result == {"corrupt": [2]} # Should be kept + + def test_filter_inactive_frequency_drop_zero(self): + """Test frequency_drop [0] is filtered out (disabled).""" + filters = {"frequency_drop": [0]} + result = filter_inactive_filters(filters) + assert result == {} # Should be filtered out + + def test_filter_inactive_frequency_drop_active(self): + """Test frequency_drop [10] is kept (active).""" + filters = {"frequency_drop": [10]} + result = filter_inactive_filters(filters) + assert result == {"frequency_drop": [10]} # Should be kept + + def test_filter_inactive_bpf_empty(self): + """Test BPF empty string is filtered out.""" + filters = {"bpf": [""]} + result = filter_inactive_filters(filters) + assert result == {} # Should be filtered out + + def test_filter_inactive_bpf_active(self): + """Test BPF with expression is kept.""" + filters = {"bpf": ["tcp port 80"]} + result = filter_inactive_filters(filters) + assert result == {"bpf": ["tcp port 80"]} # Should be kept + + def test_filter_inactive_multiple_filters_mixed(self): + """Test multiple filters with mixed active/inactive states.""" + filters = { + "delay": [0, 0], # Disabled: [0, 0] + "packet_loss": [0], # Disabled: 0% + "corrupt": [2], # Active: 2% + "frequency_drop": [10] # Active: every 10th packet + } + result = filter_inactive_filters(filters) + assert result == { + "corrupt": [2], + "frequency_drop": [10] + } + + def test_filter_inactive_empty_filters(self): + """Test empty filters dictionary.""" + filters = {} + result = filter_inactive_filters(filters) + assert result == {} + + def test_filter_inactive_none_filters(self): + """Test None filters.""" + result = filter_inactive_filters(None) + assert result == {} \ No newline at end of file