mirror of
https://github.com/ipnet-mesh/meshcore-hub.git
synced 2026-03-28 17:42:56 +01:00
fix: harden security across auth, XSS, and proxy trust
- Use hmac.compare_digest for constant-time API key comparison in auth and metrics endpoints to prevent timing attacks - Escape user-controlled data in admin JS templates (members, node-tags) to prevent XSS via innerHTML - Escape </script> sequences in embedded JSON config to prevent XSS breakout from <script> blocks - Add configurable WEB_TRUSTED_PROXY_HOSTS setting instead of trusting all proxy headers unconditionally - Warn on startup when admin is enabled with default trust-all proxy - Remove legacy HTML dashboard endpoint (unused, superseded by SPA) - Add comprehensive auth and dashboard test coverage
This commit is contained in:
@@ -612,6 +612,7 @@ Key variables:
|
||||
- `MQTT_TLS` - Enable TLS/SSL for MQTT (default: `false`)
|
||||
- `API_READ_KEY`, `API_ADMIN_KEY` - API authentication keys
|
||||
- `WEB_ADMIN_ENABLED` - Enable admin interface at /a/ (default: `false`, requires auth proxy)
|
||||
- `WEB_TRUSTED_PROXY_HOSTS` - Comma-separated list of trusted proxy hosts for admin authentication headers. Default: `*` (all hosts). Recommended: set to your reverse proxy IP in production. A startup warning is emitted when using the default `*` with admin enabled.
|
||||
- `WEB_THEME` - Default theme for the web dashboard (default: `dark`, options: `dark`, `light`). Users can override via the theme toggle in the navbar, which persists their preference in browser localStorage.
|
||||
- `WEB_AUTO_REFRESH_SECONDS` - Auto-refresh interval in seconds for list pages (default: `30`, `0` to disable)
|
||||
- `TZ` - Timezone for web dashboard date/time display (default: `UTC`, e.g., `America/New_York`, `Europe/London`)
|
||||
|
||||
1
PLAN.md
1
PLAN.md
@@ -516,6 +516,7 @@ LetsMesh compatibility parity note:
|
||||
| WEB_PORT | 8080 | Web bind port |
|
||||
| API_BASE_URL | http://localhost:8000 | API endpoint |
|
||||
| API_KEY | | API key for queries |
|
||||
| WEB_TRUSTED_PROXY_HOSTS | * | Comma-separated list of trusted proxy hosts for admin authentication headers. Default: `*` (all hosts). Recommended: set to your reverse proxy IP in production. |
|
||||
| WEB_LOCALE | en | UI translation locale |
|
||||
| WEB_DATETIME_LOCALE | en-US | Date formatting locale for UI timestamps |
|
||||
| TZ | UTC | Timezone used for UI timestamp rendering |
|
||||
|
||||
@@ -394,6 +394,7 @@ The collector automatically cleans up old event data and inactive nodes:
|
||||
| `WEB_DATETIME_LOCALE` | `en-US` | Locale used for date formatting in the web dashboard (e.g., `en-US` for MM/DD/YYYY, `en-GB` for DD/MM/YYYY). |
|
||||
| `WEB_AUTO_REFRESH_SECONDS` | `30` | Auto-refresh interval in seconds for list pages (0 to disable) |
|
||||
| `WEB_ADMIN_ENABLED` | `false` | Enable admin interface at /a/ (requires auth proxy: `X-Forwarded-User`/`X-Auth-Request-User` or forwarded `Authorization: Basic ...`) |
|
||||
| `WEB_TRUSTED_PROXY_HOSTS` | `*` | Comma-separated list of trusted proxy hosts for admin authentication headers. Default: `*` (all hosts). Recommended: set to your reverse proxy IP in production. A startup warning is emitted when using the default `*` with admin enabled. |
|
||||
| `TZ` | `UTC` | Timezone for displaying dates/times (e.g., `America/New_York`, `Europe/London`) |
|
||||
| `NETWORK_DOMAIN` | *(none)* | Network domain name (optional) |
|
||||
| `NETWORK_NAME` | `MeshCore Network` | Display name for the network |
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
"""Authentication middleware for the API."""
|
||||
|
||||
import hmac
|
||||
import logging
|
||||
from typing import Annotated
|
||||
|
||||
@@ -79,7 +80,9 @@ async def require_read(
|
||||
)
|
||||
|
||||
# Check if token matches any key
|
||||
if token == read_key or token == admin_key:
|
||||
if (read_key and hmac.compare_digest(token, read_key)) or (
|
||||
admin_key and hmac.compare_digest(token, admin_key)
|
||||
):
|
||||
return token
|
||||
|
||||
raise HTTPException(
|
||||
@@ -124,7 +127,7 @@ async def require_admin(
|
||||
)
|
||||
|
||||
# Check if token matches admin key
|
||||
if token == admin_key:
|
||||
if hmac.compare_digest(token, admin_key):
|
||||
return token
|
||||
|
||||
raise HTTPException(
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
"""Prometheus metrics endpoint for MeshCore Hub API."""
|
||||
|
||||
import base64
|
||||
import hmac
|
||||
import logging
|
||||
import time
|
||||
from typing import Any
|
||||
@@ -54,7 +55,9 @@ def verify_basic_auth(request: Request) -> bool:
|
||||
try:
|
||||
decoded = base64.b64decode(auth_header[6:]).decode("utf-8")
|
||||
username, password = decoded.split(":", 1)
|
||||
return username == "metrics" and password == read_key
|
||||
return hmac.compare_digest(username, "metrics") and hmac.compare_digest(
|
||||
password, read_key
|
||||
)
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
|
||||
@@ -2,8 +2,7 @@
|
||||
|
||||
from datetime import datetime, timedelta, timezone
|
||||
|
||||
from fastapi import APIRouter, Request
|
||||
from fastapi.responses import HTMLResponse
|
||||
from fastapi import APIRouter
|
||||
from sqlalchemy import func, select
|
||||
|
||||
from meshcore_hub.api.auth import RequireRead
|
||||
@@ -362,175 +361,3 @@ async def get_node_count_history(
|
||||
data.append(DailyActivityPoint(date=date_str, count=count))
|
||||
|
||||
return NodeCountHistory(days=days, data=data)
|
||||
|
||||
|
||||
@router.get("/", response_class=HTMLResponse)
|
||||
async def dashboard(
|
||||
request: Request,
|
||||
session: DbSession,
|
||||
) -> HTMLResponse:
|
||||
"""Simple HTML dashboard page."""
|
||||
now = datetime.now(timezone.utc)
|
||||
today_start = now.replace(hour=0, minute=0, second=0, microsecond=0)
|
||||
yesterday = now - timedelta(days=1)
|
||||
|
||||
# Get stats
|
||||
total_nodes = session.execute(select(func.count()).select_from(Node)).scalar() or 0
|
||||
|
||||
active_nodes = (
|
||||
session.execute(
|
||||
select(func.count()).select_from(Node).where(Node.last_seen >= yesterday)
|
||||
).scalar()
|
||||
or 0
|
||||
)
|
||||
|
||||
total_messages = (
|
||||
session.execute(select(func.count()).select_from(Message)).scalar() or 0
|
||||
)
|
||||
|
||||
messages_today = (
|
||||
session.execute(
|
||||
select(func.count())
|
||||
.select_from(Message)
|
||||
.where(Message.received_at >= today_start)
|
||||
).scalar()
|
||||
or 0
|
||||
)
|
||||
|
||||
# Get recent nodes
|
||||
recent_nodes = (
|
||||
session.execute(select(Node).order_by(Node.last_seen.desc()).limit(10))
|
||||
.scalars()
|
||||
.all()
|
||||
)
|
||||
|
||||
# Get recent messages
|
||||
recent_messages = (
|
||||
session.execute(select(Message).order_by(Message.received_at.desc()).limit(10))
|
||||
.scalars()
|
||||
.all()
|
||||
)
|
||||
|
||||
# Build HTML
|
||||
html = f"""
|
||||
<!DOCTYPE html>
|
||||
<html>
|
||||
<head>
|
||||
<title>MeshCore Hub Dashboard</title>
|
||||
<meta charset="utf-8">
|
||||
<meta name="viewport" content="width=device-width, initial-scale=1">
|
||||
<meta http-equiv="refresh" content="30">
|
||||
<style>
|
||||
body {{
|
||||
font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, sans-serif;
|
||||
margin: 0;
|
||||
padding: 20px;
|
||||
background: #f5f5f5;
|
||||
color: #333;
|
||||
}}
|
||||
h1 {{ color: #2c3e50; }}
|
||||
.container {{ max-width: 1200px; margin: 0 auto; }}
|
||||
.stats {{
|
||||
display: grid;
|
||||
grid-template-columns: repeat(auto-fit, minmax(200px, 1fr));
|
||||
gap: 20px;
|
||||
margin-bottom: 30px;
|
||||
}}
|
||||
.stat-card {{
|
||||
background: white;
|
||||
padding: 20px;
|
||||
border-radius: 8px;
|
||||
box-shadow: 0 2px 4px rgba(0,0,0,0.1);
|
||||
}}
|
||||
.stat-card h3 {{ margin: 0 0 10px 0; color: #666; font-size: 14px; }}
|
||||
.stat-card .value {{ font-size: 32px; font-weight: bold; color: #2c3e50; }}
|
||||
.section {{
|
||||
background: white;
|
||||
padding: 20px;
|
||||
border-radius: 8px;
|
||||
box-shadow: 0 2px 4px rgba(0,0,0,0.1);
|
||||
margin-bottom: 20px;
|
||||
}}
|
||||
table {{ width: 100%; border-collapse: collapse; }}
|
||||
th, td {{ padding: 10px; text-align: left; border-bottom: 1px solid #eee; }}
|
||||
th {{ background: #f8f9fa; font-weight: 600; }}
|
||||
.text-muted {{ color: #666; }}
|
||||
.truncate {{ max-width: 200px; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; }}
|
||||
</style>
|
||||
</head>
|
||||
<body>
|
||||
<div class="container">
|
||||
<h1>MeshCore Hub Dashboard</h1>
|
||||
<p class="text-muted">Last updated: {now.strftime('%Y-%m-%d %H:%M:%S UTC')}</p>
|
||||
|
||||
<div class="stats">
|
||||
<div class="stat-card">
|
||||
<h3>Total Nodes</h3>
|
||||
<div class="value">{total_nodes}</div>
|
||||
</div>
|
||||
<div class="stat-card">
|
||||
<h3>Active Nodes (24h)</h3>
|
||||
<div class="value">{active_nodes}</div>
|
||||
</div>
|
||||
<div class="stat-card">
|
||||
<h3>Total Messages</h3>
|
||||
<div class="value">{total_messages}</div>
|
||||
</div>
|
||||
<div class="stat-card">
|
||||
<h3>Messages Today</h3>
|
||||
<div class="value">{messages_today}</div>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<div class="section">
|
||||
<h2>Recent Nodes</h2>
|
||||
<table>
|
||||
<thead>
|
||||
<tr>
|
||||
<th>Name</th>
|
||||
<th>Public Key</th>
|
||||
<th>Type</th>
|
||||
<th>Last Seen</th>
|
||||
</tr>
|
||||
</thead>
|
||||
<tbody>
|
||||
{"".join(f'''
|
||||
<tr>
|
||||
<td>{n.name or '-'}</td>
|
||||
<td class="truncate">{n.public_key[:16]}...</td>
|
||||
<td>{n.adv_type or '-'}</td>
|
||||
<td>{n.last_seen.strftime('%Y-%m-%d %H:%M') if n.last_seen else '-'}</td>
|
||||
</tr>
|
||||
''' for n in recent_nodes)}
|
||||
</tbody>
|
||||
</table>
|
||||
</div>
|
||||
|
||||
<div class="section">
|
||||
<h2>Recent Messages</h2>
|
||||
<table>
|
||||
<thead>
|
||||
<tr>
|
||||
<th>Type</th>
|
||||
<th>From/Channel</th>
|
||||
<th>Text</th>
|
||||
<th>Received</th>
|
||||
</tr>
|
||||
</thead>
|
||||
<tbody>
|
||||
{"".join(f'''
|
||||
<tr>
|
||||
<td>{m.message_type}</td>
|
||||
<td>{m.pubkey_prefix or f'Ch {m.channel_idx}' or '-'}</td>
|
||||
<td class="truncate">{m.text[:50]}{'...' if len(m.text) > 50 else ''}</td>
|
||||
<td>{m.received_at.strftime('%Y-%m-%d %H:%M') if m.received_at else '-'}</td>
|
||||
</tr>
|
||||
''' for m in recent_messages)}
|
||||
</tbody>
|
||||
</table>
|
||||
</div>
|
||||
</div>
|
||||
</body>
|
||||
</html>
|
||||
"""
|
||||
return HTMLResponse(content=html)
|
||||
|
||||
@@ -352,6 +352,12 @@ class WebSettings(CommonSettings):
|
||||
ge=0,
|
||||
)
|
||||
|
||||
# Trusted proxy hosts for X-Forwarded-For header processing
|
||||
web_trusted_proxy_hosts: str = Field(
|
||||
default="*",
|
||||
description="Comma-separated list of trusted proxy hosts or '*' for all",
|
||||
)
|
||||
|
||||
# Admin interface (disabled by default for security)
|
||||
web_admin_enabled: bool = Field(
|
||||
default=False,
|
||||
|
||||
@@ -180,7 +180,11 @@ def _build_config_json(app: FastAPI, request: Request) -> str:
|
||||
"logo_invert_light": app.state.logo_invert_light,
|
||||
}
|
||||
|
||||
return json.dumps(config)
|
||||
# Escape "</script>" sequences to prevent XSS breakout from the
|
||||
# <script> block where this JSON is embedded via |safe in the
|
||||
# Jinja2 template. "<\/" is valid JSON per the spec and parsed
|
||||
# correctly by JavaScript's JSON.parse().
|
||||
return json.dumps(config).replace("</", "<\\/")
|
||||
|
||||
|
||||
def create_app(
|
||||
@@ -236,7 +240,24 @@ def create_app(
|
||||
)
|
||||
|
||||
# Trust proxy headers (X-Forwarded-Proto, X-Forwarded-For) for HTTPS detection
|
||||
app.add_middleware(ProxyHeadersMiddleware, trusted_hosts="*")
|
||||
trusted_hosts_raw = settings.web_trusted_proxy_hosts
|
||||
if trusted_hosts_raw == "*":
|
||||
trusted_hosts: str | list[str] = "*"
|
||||
else:
|
||||
trusted_hosts = [h.strip() for h in trusted_hosts_raw.split(",") if h.strip()]
|
||||
app.add_middleware(ProxyHeadersMiddleware, trusted_hosts=trusted_hosts)
|
||||
|
||||
# Compute effective admin flag (parameter overrides setting)
|
||||
effective_admin = (
|
||||
admin_enabled if admin_enabled is not None else settings.web_admin_enabled
|
||||
)
|
||||
|
||||
# Warn when admin is enabled but proxy trust is wide open
|
||||
if effective_admin and settings.web_trusted_proxy_hosts == "*":
|
||||
logger.warning(
|
||||
"WEB_ADMIN_ENABLED is true but WEB_TRUSTED_PROXY_HOSTS is '*' (trust all). "
|
||||
"Consider restricting to your reverse proxy IP for production deployments."
|
||||
)
|
||||
|
||||
# Add cache control headers based on resource type
|
||||
app.add_middleware(CacheControlMiddleware)
|
||||
@@ -256,9 +277,7 @@ def create_app(
|
||||
)
|
||||
app.state.api_url = api_url or settings.api_base_url
|
||||
app.state.api_key = api_key or settings.api_key
|
||||
app.state.admin_enabled = (
|
||||
admin_enabled if admin_enabled is not None else settings.web_admin_enabled
|
||||
)
|
||||
app.state.admin_enabled = effective_admin
|
||||
app.state.network_name = network_name or settings.network_name
|
||||
app.state.network_city = network_city or settings.network_city
|
||||
app.state.network_country = network_country or settings.network_country
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { apiGet, apiPost, apiPut, apiDelete } from '../../api.js';
|
||||
import {
|
||||
html, litRender, nothing,
|
||||
getConfig, errorAlert, successAlert, t,
|
||||
getConfig, errorAlert, successAlert, t, escapeHtml,
|
||||
} from '../../components.js';
|
||||
import { iconLock } from '../../icons.js';
|
||||
|
||||
@@ -304,7 +304,7 @@ ${flashHtml}
|
||||
const memberName = row.dataset.memberName;
|
||||
const confirmMsg = t('common.delete_entity_confirm', {
|
||||
entity: t('entities.member').toLowerCase(),
|
||||
name: memberName
|
||||
name: escapeHtml(memberName)
|
||||
});
|
||||
container.querySelector('#delete_confirm_message').innerHTML = confirmMsg;
|
||||
container.querySelector('#deleteModal').showModal();
|
||||
|
||||
@@ -2,7 +2,7 @@ import { apiGet, apiPost, apiPut, apiDelete } from '../../api.js';
|
||||
import {
|
||||
html, litRender, nothing, unsafeHTML,
|
||||
getConfig, typeEmoji, formatDateTimeShort, errorAlert,
|
||||
successAlert, truncateKey, t,
|
||||
successAlert, truncateKey, t, escapeHtml,
|
||||
} from '../../components.js';
|
||||
import { iconTag, iconLock } from '../../icons.js';
|
||||
|
||||
@@ -240,7 +240,8 @@ export async function render(container, params, router) {
|
||||
<div class="modal-box">
|
||||
<h3 class="font-bold text-lg">${t('common.copy_all_entity_to_another_node', { entity: t('entities.tags') })}</h3>
|
||||
<form id="copy-all-form" class="py-4">
|
||||
<p class="mb-4">${unsafeHTML(t('common.copy_all_entity_description', { count: tags.length, entity: t('entities.tags').toLowerCase(), name: nodeName }))}</p>
|
||||
<!-- unsafeHTML needed for translation HTML tags; nodeName is pre-escaped -->
|
||||
<p class="mb-4">${unsafeHTML(t('common.copy_all_entity_description', { count: tags.length, entity: t('entities.tags').toLowerCase(), name: escapeHtml(nodeName) }))}</p>
|
||||
<div class="form-control mb-4">
|
||||
<label class="label"><span class="label-text">${t('admin_node_tags.destination_node')}</span></label>
|
||||
<select id="copyAllDestination" class="select select-bordered w-full" required>
|
||||
@@ -269,7 +270,8 @@ export async function render(container, params, router) {
|
||||
<div class="modal-box">
|
||||
<h3 class="font-bold text-lg">${t('common.delete_all_entity', { entity: t('entities.tags') })}</h3>
|
||||
<div class="py-4">
|
||||
<p class="mb-4">${unsafeHTML(t('common.delete_all_entity_confirm', { count: tags.length, entity: t('entities.tags').toLowerCase(), name: nodeName }))}</p>
|
||||
<!-- unsafeHTML needed for translation HTML tags; nodeName is pre-escaped -->
|
||||
<p class="mb-4">${unsafeHTML(t('common.delete_all_entity_confirm', { count: tags.length, entity: t('entities.tags').toLowerCase(), name: escapeHtml(nodeName) }))}</p>
|
||||
<div class="alert alert-error mb-4">
|
||||
<svg xmlns="http://www.w3.org/2000/svg" class="stroke-current shrink-0 h-6 w-6" fill="none" viewBox="0 0 24 24"><path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 9v2m0 4h.01m-6.938 4h13.856c1.54 0 2.502-1.667 1.732-3L13.732 4c-.77-1.333-2.694-1.333-3.464 0L3.34 16c-.77 1.333.192 3 1.732 3z" /></svg>
|
||||
<span>${t('admin_node_tags.delete_all_warning')}</span>
|
||||
@@ -449,7 +451,7 @@ ${contentHtml}`, container);
|
||||
activeTagKey = row.dataset.tagKey;
|
||||
const confirmMsg = t('common.delete_entity_confirm', {
|
||||
entity: t('entities.tag').toLowerCase(),
|
||||
name: `"<span class="font-mono font-semibold">${activeTagKey}</span>"`
|
||||
name: `"<span class="font-mono font-semibold">${escapeHtml(activeTagKey)}</span>"`
|
||||
});
|
||||
container.querySelector('#delete_tag_confirm_message').innerHTML = confirmMsg;
|
||||
container.querySelector('#deleteModal').showModal();
|
||||
|
||||
@@ -1,8 +1,28 @@
|
||||
"""Tests for API authentication."""
|
||||
"""Tests for API authentication.
|
||||
|
||||
Verifies that constant-time key comparison (hmac.compare_digest) works
|
||||
correctly with no behavioral regressions from the original == operator.
|
||||
"""
|
||||
|
||||
import base64
|
||||
|
||||
|
||||
class TestAuthenticationFlow:
|
||||
"""Tests for authentication behavior."""
|
||||
def _make_basic_auth(username: str, password: str) -> str:
|
||||
"""Create a Basic auth header value."""
|
||||
credentials = base64.b64encode(f"{username}:{password}".encode()).decode()
|
||||
return f"Basic {credentials}"
|
||||
|
||||
|
||||
def _clear_metrics_cache() -> None:
|
||||
"""Clear the metrics module cache."""
|
||||
from meshcore_hub.api.metrics import _cache
|
||||
|
||||
_cache["output"] = b""
|
||||
_cache["expires_at"] = 0.0
|
||||
|
||||
|
||||
class TestReadAuthentication:
|
||||
"""Tests for read-level authentication (require_read)."""
|
||||
|
||||
def test_no_auth_when_keys_not_configured(self, client_no_auth):
|
||||
"""Test that no auth is required when keys are not configured."""
|
||||
@@ -30,46 +50,47 @@ class TestAuthenticationFlow:
|
||||
)
|
||||
assert response.status_code == 200
|
||||
|
||||
def test_read_key_accepted_on_multiple_endpoints(self, client_with_auth):
|
||||
"""Test that read key is accepted across different read endpoints."""
|
||||
for endpoint in ["/api/v1/nodes", "/api/v1/messages"]:
|
||||
response = client_with_auth.get(
|
||||
endpoint,
|
||||
headers={"Authorization": "Bearer test-read-key"},
|
||||
)
|
||||
assert response.status_code == 200, f"Read key rejected on {endpoint}"
|
||||
|
||||
def test_read_endpoints_accept_admin_key(self, client_with_auth):
|
||||
"""Test that read endpoints accept admin key."""
|
||||
"""Test that admin key also grants read access."""
|
||||
response = client_with_auth.get(
|
||||
"/api/v1/nodes",
|
||||
headers={"Authorization": "Bearer test-admin-key"},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
|
||||
def test_admin_endpoints_reject_read_key(self, client_with_auth):
|
||||
"""Test that admin endpoints reject read key."""
|
||||
response = client_with_auth.post(
|
||||
"/api/v1/commands/send-message",
|
||||
json={
|
||||
"destination": "abc123def456abc123def456abc123de",
|
||||
"text": "Test",
|
||||
},
|
||||
headers={"Authorization": "Bearer test-read-key"},
|
||||
)
|
||||
assert response.status_code == 403
|
||||
def test_admin_key_grants_read_on_multiple_endpoints(self, client_with_auth):
|
||||
"""Test that admin key grants read access across different endpoints."""
|
||||
for endpoint in ["/api/v1/nodes", "/api/v1/messages"]:
|
||||
response = client_with_auth.get(
|
||||
endpoint,
|
||||
headers={"Authorization": "Bearer test-admin-key"},
|
||||
)
|
||||
assert (
|
||||
response.status_code == 200
|
||||
), f"Admin key rejected on read endpoint {endpoint}"
|
||||
|
||||
def test_admin_endpoints_accept_admin_key(self, client_with_auth):
|
||||
"""Test that admin endpoints accept admin key."""
|
||||
response = client_with_auth.post(
|
||||
"/api/v1/commands/send-message",
|
||||
json={
|
||||
"destination": "abc123def456abc123def456abc123de",
|
||||
"text": "Test",
|
||||
},
|
||||
headers={"Authorization": "Bearer test-admin-key"},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
|
||||
def test_invalid_key_rejected(self, client_with_auth):
|
||||
"""Test that invalid keys are rejected."""
|
||||
def test_invalid_key_rejected_on_read_endpoint(self, client_with_auth):
|
||||
"""Test that invalid keys are rejected with 401 on read endpoints."""
|
||||
response = client_with_auth.get(
|
||||
"/api/v1/nodes",
|
||||
headers={"Authorization": "Bearer invalid-key"},
|
||||
)
|
||||
assert response.status_code == 401
|
||||
|
||||
def test_no_auth_header_rejected_on_read_endpoint(self, client_with_auth):
|
||||
"""Test that missing auth header is rejected on read endpoints."""
|
||||
response = client_with_auth.get("/api/v1/nodes")
|
||||
assert response.status_code == 401
|
||||
|
||||
def test_missing_bearer_prefix_rejected(self, client_with_auth):
|
||||
"""Test that tokens without Bearer prefix are rejected."""
|
||||
response = client_with_auth.get(
|
||||
@@ -87,6 +108,124 @@ class TestAuthenticationFlow:
|
||||
assert response.status_code == 401
|
||||
|
||||
|
||||
class TestAdminAuthentication:
|
||||
"""Tests for admin-level authentication (require_admin)."""
|
||||
|
||||
def test_admin_endpoints_accept_admin_key(self, client_with_auth):
|
||||
"""Test that admin endpoints accept admin key."""
|
||||
response = client_with_auth.post(
|
||||
"/api/v1/commands/send-message",
|
||||
json={
|
||||
"destination": "abc123def456abc123def456abc123de",
|
||||
"text": "Test",
|
||||
},
|
||||
headers={"Authorization": "Bearer test-admin-key"},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
|
||||
def test_admin_endpoints_reject_read_key(self, client_with_auth):
|
||||
"""Test that admin endpoints reject read key with 403."""
|
||||
response = client_with_auth.post(
|
||||
"/api/v1/commands/send-message",
|
||||
json={
|
||||
"destination": "abc123def456abc123def456abc123de",
|
||||
"text": "Test",
|
||||
},
|
||||
headers={"Authorization": "Bearer test-read-key"},
|
||||
)
|
||||
assert response.status_code == 403
|
||||
|
||||
def test_admin_endpoints_reject_invalid_key(self, client_with_auth):
|
||||
"""Test that admin endpoints reject invalid keys with 403."""
|
||||
response = client_with_auth.post(
|
||||
"/api/v1/commands/send-message",
|
||||
json={
|
||||
"destination": "abc123def456abc123def456abc123de",
|
||||
"text": "Test",
|
||||
},
|
||||
headers={"Authorization": "Bearer completely-wrong-key"},
|
||||
)
|
||||
assert response.status_code == 403
|
||||
|
||||
def test_admin_endpoints_reject_no_auth_header(self, client_with_auth):
|
||||
"""Test that admin endpoints reject missing auth header with 401."""
|
||||
response = client_with_auth.post(
|
||||
"/api/v1/commands/send-message",
|
||||
json={
|
||||
"destination": "abc123def456abc123def456abc123de",
|
||||
"text": "Test",
|
||||
},
|
||||
)
|
||||
assert response.status_code == 401
|
||||
|
||||
|
||||
class TestMetricsAuthentication:
|
||||
"""Tests for metrics endpoint authentication (Basic auth with hmac.compare_digest)."""
|
||||
|
||||
def test_metrics_no_auth_when_no_read_key(self, client_no_auth):
|
||||
"""Test that metrics requires no auth when no read key is configured."""
|
||||
_clear_metrics_cache()
|
||||
response = client_no_auth.get("/metrics")
|
||||
assert response.status_code == 200
|
||||
|
||||
def test_metrics_accepts_valid_basic_auth(self, client_with_auth):
|
||||
"""Test that metrics accepts correct Basic credentials."""
|
||||
_clear_metrics_cache()
|
||||
response = client_with_auth.get(
|
||||
"/metrics",
|
||||
headers={"Authorization": _make_basic_auth("metrics", "test-read-key")},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
|
||||
def test_metrics_rejects_no_auth_when_key_set(self, client_with_auth):
|
||||
"""Test 401 when read key is set but no auth provided."""
|
||||
_clear_metrics_cache()
|
||||
response = client_with_auth.get("/metrics")
|
||||
assert response.status_code == 401
|
||||
assert "WWW-Authenticate" in response.headers
|
||||
|
||||
def test_metrics_rejects_wrong_password(self, client_with_auth):
|
||||
"""Test that metrics rejects incorrect password."""
|
||||
_clear_metrics_cache()
|
||||
response = client_with_auth.get(
|
||||
"/metrics",
|
||||
headers={"Authorization": _make_basic_auth("metrics", "wrong-key")},
|
||||
)
|
||||
assert response.status_code == 401
|
||||
|
||||
def test_metrics_rejects_wrong_username(self, client_with_auth):
|
||||
"""Test that metrics rejects incorrect username."""
|
||||
_clear_metrics_cache()
|
||||
response = client_with_auth.get(
|
||||
"/metrics",
|
||||
headers={"Authorization": _make_basic_auth("admin", "test-read-key")},
|
||||
)
|
||||
assert response.status_code == 401
|
||||
|
||||
def test_metrics_rejects_bearer_auth(self, client_with_auth):
|
||||
"""Test that Bearer auth does not work for metrics."""
|
||||
_clear_metrics_cache()
|
||||
response = client_with_auth.get(
|
||||
"/metrics",
|
||||
headers={"Authorization": "Bearer test-read-key"},
|
||||
)
|
||||
assert response.status_code == 401
|
||||
|
||||
def test_metrics_rejects_admin_key_as_password(self, client_with_auth):
|
||||
"""Test that admin key is not accepted as metrics password.
|
||||
|
||||
Metrics uses only the read key for Basic auth, not the admin key.
|
||||
"""
|
||||
_clear_metrics_cache()
|
||||
response = client_with_auth.get(
|
||||
"/metrics",
|
||||
headers={
|
||||
"Authorization": _make_basic_auth("metrics", "test-admin-key"),
|
||||
},
|
||||
)
|
||||
assert response.status_code == 401
|
||||
|
||||
|
||||
class TestHealthEndpoint:
|
||||
"""Tests for health check endpoint."""
|
||||
|
||||
|
||||
@@ -35,35 +35,71 @@ class TestDashboardStats:
|
||||
assert data["total_advertisements"] == 1
|
||||
|
||||
|
||||
class TestDashboardHtml:
|
||||
"""Tests for GET /dashboard endpoint."""
|
||||
class TestDashboardHtmlRemoved:
|
||||
"""Tests that legacy HTML dashboard endpoint has been removed."""
|
||||
|
||||
def test_dashboard_html_response(self, client_no_auth):
|
||||
"""Test dashboard returns HTML."""
|
||||
def test_dashboard_html_endpoint_removed(self, client_no_auth):
|
||||
"""Test that GET /dashboard no longer returns HTML (legacy endpoint removed)."""
|
||||
response = client_no_auth.get("/api/v1/dashboard")
|
||||
assert response.status_code == 200
|
||||
assert "text/html" in response.headers["content-type"]
|
||||
assert "<!DOCTYPE html>" in response.text
|
||||
assert "MeshCore Hub Dashboard" in response.text
|
||||
assert response.status_code in (404, 405)
|
||||
|
||||
def test_dashboard_contains_stats(
|
||||
self, client_no_auth, sample_node, sample_message
|
||||
):
|
||||
"""Test dashboard HTML contains stat values."""
|
||||
response = client_no_auth.get("/api/v1/dashboard")
|
||||
assert response.status_code == 200
|
||||
# Check that stats are present
|
||||
assert "Total Nodes" in response.text
|
||||
assert "Active Nodes" in response.text
|
||||
assert "Total Messages" in response.text
|
||||
def test_dashboard_html_endpoint_removed_trailing_slash(self, client_no_auth):
|
||||
"""Test that GET /dashboard/ also returns 404/405."""
|
||||
response = client_no_auth.get("/api/v1/dashboard/")
|
||||
assert response.status_code in (404, 405)
|
||||
|
||||
def test_dashboard_contains_recent_data(self, client_no_auth, sample_node):
|
||||
"""Test dashboard HTML contains recent nodes."""
|
||||
response = client_no_auth.get("/api/v1/dashboard")
|
||||
|
||||
class TestDashboardAuthenticatedJsonRoutes:
|
||||
"""Tests that dashboard JSON sub-routes return valid JSON with authentication."""
|
||||
|
||||
def test_stats_returns_json_when_authenticated(self, client_with_auth):
|
||||
"""Test GET /dashboard/stats returns 200 with valid JSON when authenticated."""
|
||||
response = client_with_auth.get(
|
||||
"/api/v1/dashboard/stats",
|
||||
headers={"Authorization": "Bearer test-read-key"},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert "Recent Nodes" in response.text
|
||||
# The node name should appear in the table
|
||||
assert sample_node.name in response.text
|
||||
data = response.json()
|
||||
assert "total_nodes" in data
|
||||
assert "active_nodes" in data
|
||||
assert "total_messages" in data
|
||||
assert "total_advertisements" in data
|
||||
|
||||
def test_activity_returns_json_when_authenticated(self, client_with_auth):
|
||||
"""Test GET /dashboard/activity returns 200 with valid JSON when authenticated."""
|
||||
response = client_with_auth.get(
|
||||
"/api/v1/dashboard/activity",
|
||||
headers={"Authorization": "Bearer test-read-key"},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert "days" in data
|
||||
assert "data" in data
|
||||
assert isinstance(data["data"], list)
|
||||
|
||||
def test_message_activity_returns_json_when_authenticated(self, client_with_auth):
|
||||
"""Test GET /dashboard/message-activity returns 200 with valid JSON when authenticated."""
|
||||
response = client_with_auth.get(
|
||||
"/api/v1/dashboard/message-activity",
|
||||
headers={"Authorization": "Bearer test-read-key"},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert "days" in data
|
||||
assert "data" in data
|
||||
assert isinstance(data["data"], list)
|
||||
|
||||
def test_node_count_returns_json_when_authenticated(self, client_with_auth):
|
||||
"""Test GET /dashboard/node-count returns 200 with valid JSON when authenticated."""
|
||||
response = client_with_auth.get(
|
||||
"/api/v1/dashboard/node-count",
|
||||
headers={"Authorization": "Bearer test-read-key"},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert "days" in data
|
||||
assert "data" in data
|
||||
assert isinstance(data["data"], list)
|
||||
|
||||
|
||||
class TestDashboardActivity:
|
||||
|
||||
279
tests/test_web/test_app.py
Normal file
279
tests/test_web/test_app.py
Normal file
@@ -0,0 +1,279 @@
|
||||
"""Tests for web app: config JSON escaping and trusted proxy hosts warnings."""
|
||||
|
||||
import json
|
||||
import logging
|
||||
from typing import Any
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from meshcore_hub.web.app import _build_config_json, create_app
|
||||
|
||||
from .conftest import ALL_FEATURES_ENABLED, MockHttpClient
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def xss_app(mock_http_client: MockHttpClient) -> Any:
|
||||
"""Create a web app with a network name containing a script injection payload."""
|
||||
app = create_app(
|
||||
api_url="http://localhost:8000",
|
||||
api_key="test-api-key",
|
||||
network_name="</script><script>alert(1)</script>",
|
||||
network_city="Test City",
|
||||
network_country="Test Country",
|
||||
network_radio_config="Test Radio Config",
|
||||
network_contact_email="test@example.com",
|
||||
features=ALL_FEATURES_ENABLED,
|
||||
)
|
||||
app.state.http_client = mock_http_client
|
||||
return app
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def xss_client(xss_app: Any, mock_http_client: MockHttpClient) -> TestClient:
|
||||
"""Create a test client whose network_name contains a script injection payload."""
|
||||
xss_app.state.http_client = mock_http_client
|
||||
return TestClient(xss_app, raise_server_exceptions=True)
|
||||
|
||||
|
||||
class TestConfigJsonXssEscaping:
|
||||
"""Tests that _build_config_json escapes </script> to prevent XSS breakout."""
|
||||
|
||||
def test_script_tag_escaped_in_rendered_html(self, xss_client: TestClient) -> None:
|
||||
"""Config value containing </script> is escaped to <\\/script> in the HTML."""
|
||||
response = xss_client.get("/")
|
||||
assert response.status_code == 200
|
||||
|
||||
html = response.text
|
||||
|
||||
# The literal "</script>" must NOT appear inside the config JSON block.
|
||||
# Find the config JSON assignment to isolate the embedded block.
|
||||
config_marker = "window.__APP_CONFIG__ = "
|
||||
start = html.find(config_marker)
|
||||
assert start != -1, "Config JSON block not found in rendered HTML"
|
||||
start += len(config_marker)
|
||||
end = html.find(";", start)
|
||||
config_block = html[start:end]
|
||||
|
||||
# The raw closing tag must be escaped
|
||||
assert "</script>" not in config_block
|
||||
assert "<\\/script>" in config_block
|
||||
|
||||
def test_normal_config_values_unaffected(self, client: TestClient) -> None:
|
||||
"""Config values without special characters render unchanged."""
|
||||
response = client.get("/")
|
||||
assert response.status_code == 200
|
||||
|
||||
html = response.text
|
||||
config_marker = "window.__APP_CONFIG__ = "
|
||||
start = html.find(config_marker)
|
||||
assert start != -1
|
||||
start += len(config_marker)
|
||||
end = html.find(";", start)
|
||||
config_block = html[start:end]
|
||||
|
||||
config = json.loads(config_block)
|
||||
assert config["network_name"] == "Test Network"
|
||||
assert config["network_city"] == "Test City"
|
||||
assert config["network_country"] == "Test Country"
|
||||
|
||||
def test_escaped_json_is_parseable(self, xss_client: TestClient) -> None:
|
||||
"""The escaped JSON is still valid and parseable by json.loads."""
|
||||
response = xss_client.get("/")
|
||||
assert response.status_code == 200
|
||||
|
||||
html = response.text
|
||||
config_marker = "window.__APP_CONFIG__ = "
|
||||
start = html.find(config_marker)
|
||||
assert start != -1
|
||||
start += len(config_marker)
|
||||
end = html.find(";", start)
|
||||
config_block = html[start:end]
|
||||
|
||||
# json.loads handles <\/ sequences correctly (they are valid JSON)
|
||||
config = json.loads(config_block)
|
||||
assert isinstance(config, dict)
|
||||
# The parsed value should contain the original unescaped string
|
||||
assert config["network_name"] == "</script><script>alert(1)</script>"
|
||||
|
||||
def test_build_config_json_direct_escaping(self, web_app: Any) -> None:
|
||||
"""Calling _build_config_json directly escapes </ sequences."""
|
||||
from starlette.requests import Request
|
||||
|
||||
# Inject a malicious value into the app state
|
||||
web_app.state.network_name = "</script><script>alert(1)</script>"
|
||||
|
||||
scope = {
|
||||
"type": "http",
|
||||
"method": "GET",
|
||||
"path": "/",
|
||||
"query_string": b"",
|
||||
"headers": [],
|
||||
}
|
||||
request = Request(scope)
|
||||
|
||||
result = _build_config_json(web_app, request)
|
||||
|
||||
# Raw output must not contain literal "</script>"
|
||||
assert "</script>" not in result
|
||||
assert "<\\/script>" in result
|
||||
|
||||
# Result must still be valid JSON
|
||||
parsed = json.loads(result)
|
||||
assert parsed["network_name"] == "</script><script>alert(1)</script>"
|
||||
|
||||
def test_build_config_json_no_escaping_needed(self, web_app: Any) -> None:
|
||||
"""_build_config_json leaves normal values intact when no </ present."""
|
||||
from starlette.requests import Request
|
||||
|
||||
scope = {
|
||||
"type": "http",
|
||||
"method": "GET",
|
||||
"path": "/",
|
||||
"query_string": b"",
|
||||
"headers": [],
|
||||
}
|
||||
request = Request(scope)
|
||||
|
||||
result = _build_config_json(web_app, request)
|
||||
|
||||
# No escaping artifacts for normal values
|
||||
assert "<\\/" not in result
|
||||
|
||||
parsed = json.loads(result)
|
||||
assert parsed["network_name"] == "Test Network"
|
||||
assert parsed["network_city"] == "Test City"
|
||||
|
||||
|
||||
class TestTrustedProxyHostsWarning:
|
||||
"""Tests for trusted proxy hosts startup warning in create_app."""
|
||||
|
||||
def test_warning_logged_when_admin_enabled_and_wildcard_hosts(
|
||||
self, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""A warning is logged when WEB_ADMIN_ENABLED=true and WEB_TRUSTED_PROXY_HOSTS is '*'."""
|
||||
with patch("meshcore_hub.common.config.get_web_settings") as mock_get_settings:
|
||||
from meshcore_hub.common.config import WebSettings
|
||||
|
||||
settings = WebSettings(
|
||||
_env_file=None,
|
||||
web_admin_enabled=True,
|
||||
web_trusted_proxy_hosts="*",
|
||||
)
|
||||
mock_get_settings.return_value = settings
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="meshcore_hub.web.app"):
|
||||
create_app(
|
||||
api_url="http://localhost:8000",
|
||||
admin_enabled=True,
|
||||
features=ALL_FEATURES_ENABLED,
|
||||
)
|
||||
|
||||
assert any(
|
||||
"WEB_ADMIN_ENABLED is true but WEB_TRUSTED_PROXY_HOSTS is '*'" in msg
|
||||
for msg in caplog.messages
|
||||
), f"Expected warning not found in log messages: {caplog.messages}"
|
||||
|
||||
def test_no_warning_when_trusted_proxy_hosts_is_specific(
|
||||
self, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""No warning is logged when WEB_TRUSTED_PROXY_HOSTS is set to a specific value."""
|
||||
with patch("meshcore_hub.common.config.get_web_settings") as mock_get_settings:
|
||||
from meshcore_hub.common.config import WebSettings
|
||||
|
||||
settings = WebSettings(
|
||||
_env_file=None,
|
||||
web_admin_enabled=True,
|
||||
web_trusted_proxy_hosts="10.0.0.1",
|
||||
)
|
||||
mock_get_settings.return_value = settings
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="meshcore_hub.web.app"):
|
||||
create_app(
|
||||
api_url="http://localhost:8000",
|
||||
admin_enabled=True,
|
||||
features=ALL_FEATURES_ENABLED,
|
||||
)
|
||||
|
||||
assert not any(
|
||||
"WEB_TRUSTED_PROXY_HOSTS" in msg for msg in caplog.messages
|
||||
), f"Unexpected warning found in log messages: {caplog.messages}"
|
||||
|
||||
def test_no_warning_when_admin_disabled(
|
||||
self, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""No warning is logged when WEB_ADMIN_ENABLED is false even with wildcard hosts."""
|
||||
with patch("meshcore_hub.common.config.get_web_settings") as mock_get_settings:
|
||||
from meshcore_hub.common.config import WebSettings
|
||||
|
||||
settings = WebSettings(
|
||||
_env_file=None,
|
||||
web_admin_enabled=False,
|
||||
web_trusted_proxy_hosts="*",
|
||||
)
|
||||
mock_get_settings.return_value = settings
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="meshcore_hub.web.app"):
|
||||
create_app(
|
||||
api_url="http://localhost:8000",
|
||||
features=ALL_FEATURES_ENABLED,
|
||||
)
|
||||
|
||||
assert not any(
|
||||
"WEB_TRUSTED_PROXY_HOSTS" in msg for msg in caplog.messages
|
||||
), f"Unexpected warning found in log messages: {caplog.messages}"
|
||||
|
||||
def test_proxy_hosts_comma_list_parsed_correctly(self) -> None:
|
||||
"""A comma-separated WEB_TRUSTED_PROXY_HOSTS is split into a list for middleware."""
|
||||
from uvicorn.middleware.proxy_headers import ProxyHeadersMiddleware
|
||||
|
||||
with patch("meshcore_hub.common.config.get_web_settings") as mock_get_settings:
|
||||
from meshcore_hub.common.config import WebSettings
|
||||
|
||||
settings = WebSettings(
|
||||
_env_file=None,
|
||||
web_trusted_proxy_hosts="10.0.0.1, 10.0.0.2, 172.16.0.1",
|
||||
)
|
||||
mock_get_settings.return_value = settings
|
||||
|
||||
app = create_app(
|
||||
api_url="http://localhost:8000",
|
||||
features=ALL_FEATURES_ENABLED,
|
||||
)
|
||||
|
||||
# Find the ProxyHeadersMiddleware entry in app.user_middleware
|
||||
proxy_entries = [
|
||||
m for m in app.user_middleware if m.cls is ProxyHeadersMiddleware
|
||||
]
|
||||
assert len(proxy_entries) == 1, "ProxyHeadersMiddleware not found in middleware"
|
||||
assert proxy_entries[0].kwargs["trusted_hosts"] == [
|
||||
"10.0.0.1",
|
||||
"10.0.0.2",
|
||||
"172.16.0.1",
|
||||
]
|
||||
|
||||
def test_wildcard_proxy_hosts_passed_as_string(self) -> None:
|
||||
"""Wildcard WEB_TRUSTED_PROXY_HOSTS='*' is passed as a string to middleware."""
|
||||
from uvicorn.middleware.proxy_headers import ProxyHeadersMiddleware
|
||||
|
||||
with patch("meshcore_hub.common.config.get_web_settings") as mock_get_settings:
|
||||
from meshcore_hub.common.config import WebSettings
|
||||
|
||||
settings = WebSettings(
|
||||
_env_file=None,
|
||||
web_trusted_proxy_hosts="*",
|
||||
)
|
||||
mock_get_settings.return_value = settings
|
||||
|
||||
app = create_app(
|
||||
api_url="http://localhost:8000",
|
||||
features=ALL_FEATURES_ENABLED,
|
||||
)
|
||||
|
||||
# Find the ProxyHeadersMiddleware entry in app.user_middleware
|
||||
proxy_entries = [
|
||||
m for m in app.user_middleware if m.cls is ProxyHeadersMiddleware
|
||||
]
|
||||
assert len(proxy_entries) == 1, "ProxyHeadersMiddleware not found in middleware"
|
||||
assert proxy_entries[0].kwargs["trusted_hosts"] == "*"
|
||||
Reference in New Issue
Block a user