mirror of
https://github.com/taylanbakircioglu/haproxy-openmanager.git
synced 2026-09-21 18:13:25 +00:00
fix(waf): Require frontend selection with backend + frontend validation
COMPLETE UX FIX: Backend validation + Frontend required field Changes Summary: 1. Backend API validation (waf.py) 2. Frontend API validation (frontend.py) 3. Frontend UI required field (WAFManagement.js) Problem: - User creates WAF without selecting frontends - Backend applies WAF to ALL frontends (unintentional) - No visual indication that frontend selection is required - User confused about where WAF is applied Solution - Part 1: Backend API Validation waf.py CREATE (lines 418-426): - Validate frontend_ids not empty - HTTP 400 if no frontends selected - Error: "At least one frontend must be selected" waf.py UPDATE (lines 686-693): - Validate if frontend_ids explicitly provided - HTTP 400 if trying to clear all frontends - Allow config-only updates (preserve frontends) Solution - Part 2: Frontend Validation frontend.py CREATE (lines 408-428): - Validate backend has active servers - HTTP 400 if backend has no servers - Error: "Backend has no active servers. Add servers first." frontend.py UPDATE (lines 650-670): - Same validation when changing default_backend - Prevent routing to DOWN backends Solution - Part 3: Frontend UI (User Experience) WAFManagement.js (lines 1473-1508): BEFORE: - Label: "Target Frontends" - Tooltip: "Can be left empty for globally available WAF" - Placeholder: "Select frontends" - No validation - Optional field appearance AFTER: - Label: "Target Frontends" (with red asterisk) - Required validation rules: * Antd required: true * Custom validator: at least 1 frontend - Placeholder: "Select frontends (Required *)" - Tooltip: "At least one frontend is required" - Search enabled for easy filtering - Error messages: * "Please select at least one frontend" * "At least one frontend must be selected for WAF rule" User Experience Improvements: 1. Visual indication: Red asterisk on label 2. Clear placeholder text: "(Required *)" 3. Helpful tooltip: Explains requirement 4. Client-side validation: Immediate feedback 5. Server-side validation: Safety net 6. Searchable dropdown: Easy to find frontends 7. Clear error messages: User knows what to do Test Scenarios: 1. Create WAF without selecting frontend: - UI: Red error "Please select at least one frontend" - Submit blocked (client-side) 2. Bypass client-side, try API: - API: HTTP 400 "At least one frontend must be selected" 3. Create frontend with server-less backend: - UI: Can select backend - API: HTTP 400 "Backend has no active servers" 4. Update WAF remove all frontends: - UI: Red error message - API: HTTP 400 if bypassed Related: bcb8ef0 (backend without servers) Refs: #waf-validation #frontend-validation #ux-improvement
This commit is contained in:
Taylan Bakırcıoğlu
committed by
taylanbakircioglu
taylanbakircioglu
parent
f1a3826334
commit
7d3eeebcb7
@@ -405,6 +405,28 @@ async def create_frontend(frontend: FrontendConfig, request: Request, authorizat
|
||||
await close_database_connection(conn)
|
||||
raise HTTPException(status_code=400, detail=f"Frontend '{frontend.name}' already exists")
|
||||
|
||||
# CRITICAL VALIDATION: Check if default_backend has active servers
|
||||
# Use case: Prevent frontend from routing to DOWN backend (no servers = 503 errors)
|
||||
# HAProxy allows this (syntax valid) but it's bad practice for production
|
||||
if frontend.default_backend:
|
||||
backend_has_servers = await conn.fetchval("""
|
||||
SELECT EXISTS(
|
||||
SELECT 1 FROM backend_servers bs
|
||||
JOIN backends b ON bs.backend_name = b.name AND bs.cluster_id = b.cluster_id
|
||||
WHERE b.name = $1
|
||||
AND b.cluster_id = $2
|
||||
AND b.is_active = TRUE
|
||||
AND bs.is_active = TRUE
|
||||
)
|
||||
""", frontend.default_backend, frontend.cluster_id)
|
||||
|
||||
if not backend_has_servers:
|
||||
await close_database_connection(conn)
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=f"Backend '{frontend.default_backend}' has no active servers. Please add at least one server to the backend before assigning it to a frontend."
|
||||
)
|
||||
|
||||
# ENTERPRISE DUAL-MODE: Save ssl_certificate_ids (NEW) and ssl_certificate_id (OLD - backward compat)
|
||||
# Convert ssl_certificate_ids to JSONB for database
|
||||
ssl_cert_ids_json = json.dumps(frontend.ssl_certificate_ids) if frontend.ssl_certificate_ids else '[]'
|
||||
@@ -625,6 +647,28 @@ async def update_frontend(frontend_id: int, frontend: FrontendConfig, request: R
|
||||
await close_database_connection(conn)
|
||||
raise HTTPException(status_code=400, detail=f"Frontend name '{frontend.name}' already exists")
|
||||
|
||||
# CRITICAL VALIDATION: Check if default_backend has active servers
|
||||
# Use case: Prevent frontend from routing to DOWN backend (no servers = 503 errors)
|
||||
# HAProxy allows this (syntax valid) but it's bad practice for production
|
||||
if frontend.default_backend:
|
||||
backend_has_servers = await conn.fetchval("""
|
||||
SELECT EXISTS(
|
||||
SELECT 1 FROM backend_servers bs
|
||||
JOIN backends b ON bs.backend_name = b.name AND bs.cluster_id = b.cluster_id
|
||||
WHERE b.name = $1
|
||||
AND (b.cluster_id = $2 OR b.cluster_id IS NULL)
|
||||
AND b.is_active = TRUE
|
||||
AND bs.is_active = TRUE
|
||||
)
|
||||
""", frontend.default_backend, cluster_id)
|
||||
|
||||
if not backend_has_servers:
|
||||
await close_database_connection(conn)
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=f"Backend '{frontend.default_backend}' has no active servers. Please add at least one server to the backend before assigning it to a frontend."
|
||||
)
|
||||
|
||||
# CRITICAL FIX: Preserve SSL configuration if not explicitly changed
|
||||
# If SSL is currently enabled but incoming data has ssl_enabled=False or ssl_certificate_id=None,
|
||||
# check if this is an intentional change or just missing data from the form
|
||||
|
||||
@@ -415,6 +415,16 @@ async def create_waf_rule(waf_rule_data: dict, cluster_id: Optional[int] = None,
|
||||
cluster_info = f" in cluster {cluster_id}" if cluster_id else ""
|
||||
raise HTTPException(status_code=400, detail=f"WAF rule '{waf_rule.name}' already exists{cluster_info}")
|
||||
|
||||
# CRITICAL VALIDATION: At least one frontend must be selected
|
||||
# Use case: Prevent unintentional application to ALL frontends
|
||||
# WAF rule without frontend = no config change = should not create pending version
|
||||
if not waf_rule.frontend_ids or len(waf_rule.frontend_ids) == 0:
|
||||
await close_database_connection(conn)
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="At least one frontend must be selected for WAF rule. Please select target frontend(s) where this WAF rule should be applied."
|
||||
)
|
||||
|
||||
async with conn.transaction():
|
||||
rule_id = await conn.fetchval("""
|
||||
INSERT INTO waf_rules (name, rule_type, config, action, priority, description, enabled, is_active, cluster_id)
|
||||
@@ -673,6 +683,15 @@ async def update_waf_rule(rule_id: int, waf_rule_data: dict, request: Request, a
|
||||
frontend_ids_in_payload = 'frontend_ids' in waf_rule_data
|
||||
|
||||
if frontend_ids_in_payload:
|
||||
# CRITICAL VALIDATION: If frontend_ids explicitly provided, must have at least one
|
||||
# Use case: Prevent clearing all frontends (WAF rule would apply to nothing)
|
||||
if not waf_rule.frontend_ids or len(waf_rule.frontend_ids) == 0:
|
||||
await close_database_connection(conn)
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="At least one frontend must be selected for WAF rule. Cannot remove all frontend assignments. Please select target frontend(s)."
|
||||
)
|
||||
|
||||
# Frontend IDs were explicitly provided in the request (could be [] or [1,2,3])
|
||||
await conn.execute("DELETE FROM frontend_waf_rules WHERE waf_rule_id = $1", rule_id)
|
||||
frontend_assignments, cluster_ids = await assign_frontends_and_get_clusters(conn, rule_id, waf_rule.frontend_ids or [], existing_rule.get("cluster_id"))
|
||||
|
||||
@@ -1473,13 +1473,31 @@ const WAFManagement = () => {
|
||||
<Form.Item
|
||||
name="frontend_ids"
|
||||
label="Target Frontends"
|
||||
tooltip="WAF rule will be applied to selected frontends. This can be left empty for a globally available WAF rule."
|
||||
rules={[
|
||||
{
|
||||
required: true,
|
||||
message: 'Please select at least one frontend'
|
||||
},
|
||||
{
|
||||
validator: (_, value) => {
|
||||
if (!value || value.length === 0) {
|
||||
return Promise.reject('At least one frontend must be selected for WAF rule');
|
||||
}
|
||||
return Promise.resolve();
|
||||
}
|
||||
}
|
||||
]}
|
||||
tooltip="Select which frontend(s) this WAF rule should be applied to. At least one frontend is required."
|
||||
>
|
||||
<Select
|
||||
mode="multiple"
|
||||
placeholder="Select frontends to apply this WAF rule"
|
||||
placeholder="Select frontends to apply this WAF rule (Required *)"
|
||||
disabled={!selectedCluster}
|
||||
maxTagCount="responsive"
|
||||
showSearch
|
||||
filterOption={(input, option) =>
|
||||
option.children.toLowerCase().indexOf(input.toLowerCase()) >= 0
|
||||
}
|
||||
>
|
||||
{frontends.map(frontend => (
|
||||
<Option key={frontend.id} value={frontend.id}>
|
||||
|
||||
Reference in New Issue
Block a user