From 9873aa202b01d2274dc89d908829ce02119ea88f Mon Sep 17 00:00:00 2001 From: Louis King Date: Tue, 28 Apr 2026 13:33:52 +0100 Subject: [PATCH] Remove header-based auth (ProxyHeadersMiddleware, is_authenticated config, OAuth2 SPA flows) Remove the reverse-proxy header authentication pattern (X-Forwarded-User, X-Auth-Request-User, Basic auth forwarding) from the web dashboard. Admin access is now controlled solely by the WEB_ADMIN_ENABLED flag. - Remove web_trusted_proxy_hosts config field and ProxyHeadersMiddleware - Remove _is_authenticated_proxy_request() and api_proxy() 401 guard - Remove is_authenticated from SPA config JSON - Remove OAuth2 login/sign-out UI from admin pages and router - Remove auth_required i18n keys (en, nl) - Remove auth-related tests and fixtures - Delete docs/hosting/nginx-proxy-manager.md - Update README, AGENTS.md, .env.example, docs/i18n.md, agents docs-sync refs 572 tests pass, pre-commit clean. --- .../references/documentation-checklist.md | 24 +- .env.example | 8 +- AGENTS.md | 5 +- README.md | 11 +- docs/hosting/nginx-proxy-manager.md | 60 ----- docs/i18n.md | 2 - .../20260428-1251-remove-header-auth/plan.md | 213 +++++++++++++++ .../20260428-1251-remove-header-auth/tasks.md | 88 +++++++ src/meshcore_hub/common/config.py | 8 +- src/meshcore_hub/web/app.py | 52 ---- .../web/static/js/spa/pages/admin/index.js | 12 - .../web/static/js/spa/pages/admin/members.js | 12 - .../static/js/spa/pages/admin/node-tags.js | 12 - .../web/static/js/spa/pages/node-detail.js | 2 +- src/meshcore_hub/web/static/js/spa/router.js | 2 +- src/meshcore_hub/web/static/locales/en.json | 2 - src/meshcore_hub/web/static/locales/nl.json | 2 - tests/test_web/test_admin.py | 243 +----------------- tests/test_web/test_advertisements.py | 12 - tests/test_web/test_app.py | 137 +--------- tests/test_web/test_home.py | 24 -- 21 files changed, 328 insertions(+), 603 deletions(-) delete mode 100644 docs/hosting/nginx-proxy-manager.md create mode 100644 docs/plans/20260428-1251-remove-header-auth/plan.md create mode 100644 docs/plans/20260428-1251-remove-header-auth/tasks.md diff --git a/.agents/skills/docs-sync/references/documentation-checklist.md b/.agents/skills/docs-sync/references/documentation-checklist.md index b4c88a5..fd7bd3c 100644 --- a/.agents/skills/docs-sync/references/documentation-checklist.md +++ b/.agents/skills/docs-sync/references/documentation-checklist.md @@ -33,8 +33,7 @@ For each variable in each table: - [ ] Volume names documented with `COMPOSE_PROJECT_NAME` prefix convention - [ ] Bind mounts documented (SEED_HOME, CONTENT_HOME) - [ ] Traefik integration instructions reference `TRAEFIK_DOMAIN` -- [ ] Reverse Proxy section links to `docs/hosting/nginx-proxy-manager.md` -- [ ] Production network setup (`proxy-net`) documented +- [ ] Reverse Proxy section documents proxy setup and production network - [ ] Quick start examples use correct current commands ### Features Section @@ -151,20 +150,7 @@ docs/letsmesh.md documents the LetsMesh packet normalization and decoding behavi - [ ] GPS/location update behavior documented matches advertisement handler logic - [ ] No stale decoder behavior documented (e.g., references to Node.js decoder) -## 3c. docs/hosting/nginx-proxy-manager.md - -### NPM Admin Setup Guide - -docs/hosting/nginx-proxy-manager.md documents the Nginx Proxy Manager reverse proxy setup for admin authentication. Verify: - -- [ ] Dual-hostname setup (public + admin) documented -- [ ] Proxy host settings (scheme, port, websockets) match `docker-compose.dev.yml` port mappings -- [ ] `WEB_ADMIN_ENABLED` requirement documented -- [ ] Nginx `Advanced` config block headers match those checked by `web/auth.py` -- [ ] Verification curl command uses correct endpoint (`/config.js`) -- [ ] Troubleshooting steps reference correct config variables - -## 3d. docs/seeding.md +## 3c. docs/seeding.md ### Seed Data Documentation @@ -179,7 +165,7 @@ docs/seeding.md documents the seed data format and import process for node tags - [ ] Member field table fields match `MemberCreate` Pydantic schema - [ ] Example seed files referenced in `example/seed/` exist -## 3e. docs/i18n.md +## 3d. docs/i18n.md ### Translation Reference Guide @@ -195,7 +181,7 @@ docs/i18n.md is a comprehensive reference for translators. Verify: - [ ] No stale translation keys documented (removed from `en.json`) - [ ] Translation tips are accurate for current i18n system -## 3f. docs/webhooks.md +## 3e. docs/webhooks.md ### Webhook Configuration Documentation @@ -209,7 +195,7 @@ docs/webhooks.md documents the webhook configuration, URL routing logic, and pay - [ ] Event types listed match actual webhook event types - [ ] Configuration examples use correct env var names -## 3g. docs/content.md +## 3f. docs/content.md ### Custom Content Documentation diff --git a/.env.example b/.env.example index 8893f73..556c6fe 100644 --- a/.env.example +++ b/.env.example @@ -335,16 +335,10 @@ WEB_PORT=8080 # Default: 30 # WEB_AUTO_REFRESH_SECONDS=30 -# Enable admin interface at /a/ (requires auth proxy in front) +# Enable admin interface at /a/ # Default: false # WEB_ADMIN_ENABLED=false -# Comma-separated list of trusted proxy hosts for admin authentication headers -# Default: * (trust all hosts) -# Recommended: set to your reverse proxy IP in production -# A startup warning is emitted when using the default * with admin enabled -# WEB_TRUSTED_PROXY_HOSTS=* - # Timezone for displaying dates/times on the web dashboard # Uses standard IANA timezone names (e.g., America/New_York, Europe/London) # Default: UTC diff --git a/AGENTS.md b/AGENTS.md index be61ef7..3bd8dec 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -40,7 +40,6 @@ MeshCore Hub is a Python 3.14+ monorepo for managing and orchestrating MeshCore - [SCHEMAS.md](SCHEMAS.md) - MeshCore event JSON schemas and database mappings - [docs/upgrading.md](docs/upgrading.md) - Upgrade guide for breaking changes - [docs/letsmesh.md](docs/letsmesh.md) - LetsMesh packet decoding details -- [docs/hosting/nginx-proxy-manager.md](docs/hosting/nginx-proxy-manager.md) - Nginx Proxy Manager admin setup - [docs/seeding.md](docs/seeding.md) - Seed data format and import guide - [docs/i18n.md](docs/i18n.md) - Translation reference guide @@ -358,7 +357,6 @@ meshcore-hub/ ├── docs/ # Documentation │ ├── images/ # Screenshots and images │ ├── hosting/ # Reverse proxy hosting guides -│ │ └── nginx-proxy-manager.md │ ├── content.md # Custom content setup guide │ ├── i18n.md # Translation reference guide │ ├── letsmesh.md # LetsMesh packet decoding details @@ -639,8 +637,7 @@ Key variables: - `COLLECTOR_CHANNEL_KEYS` - Additional decoder channel keys for decrypting GroupText packets - `COLLECTOR_INCLUDE_TEST_CHANNEL` - Include built-in 'test' channel messages (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_ADMIN_ENABLED` - Enable admin interface at /a/ (default: `false`) - `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`) diff --git a/README.md b/README.md index 83612ef..2a1713b 100644 --- a/README.md +++ b/README.md @@ -193,11 +193,7 @@ Configure your reverse proxy to forward to the containers: #### Reverse Proxy -MeshCore Hub is designed to run behind a reverse proxy in production. Guides for specific reverse proxies: - -- [Nginx Proxy Manager](docs/hosting/nginx-proxy-manager.md) — Admin authentication setup with dual hostnames - -A Traefik override file is also provided with pre-configured labels: +MeshCore Hub is designed to run behind a reverse proxy in production. A Traefik override file is provided with pre-configured labels: ```bash # Download the Traefik override @@ -373,8 +369,7 @@ The collector automatically cleans up old event data and inactive nodes: | `WEB_LOCALE` | `en` | Locale/language for the web dashboard (e.g., `en`, `es`, `fr`) | | `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. | +| `WEB_ADMIN_ENABLED` | `false` | Enable admin interface at /a/ | | `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 | @@ -572,7 +567,6 @@ meshcore-hub/ ├── docs/ # Documentation │ ├── images/ # Screenshots and images │ ├── hosting/ # Reverse proxy hosting guides -│ │ └── nginx-proxy-manager.md │ ├── content.md # Custom content setup guide │ ├── i18n.md # Translation reference guide │ ├── letsmesh.md # LetsMesh packet decoding details @@ -589,7 +583,6 @@ meshcore-hub/ - [docs/upgrading.md](docs/upgrading.md) - Upgrade guide for breaking changes - [docs/letsmesh.md](docs/letsmesh.md) - LetsMesh packet decoding details - [docs/seeding.md](docs/seeding.md) - Seed data format and import guide -- [docs/hosting/nginx-proxy-manager.md](docs/hosting/nginx-proxy-manager.md) - Nginx Proxy Manager admin setup - [docs/i18n.md](docs/i18n.md) - Translation reference guide - [docs/content.md](docs/content.md) - Custom content setup guide - [docs/webhooks.md](docs/webhooks.md) - Webhook configuration reference diff --git a/docs/hosting/nginx-proxy-manager.md b/docs/hosting/nginx-proxy-manager.md deleted file mode 100644 index f57642b..0000000 --- a/docs/hosting/nginx-proxy-manager.md +++ /dev/null @@ -1,60 +0,0 @@ -# Nginx Proxy Manager (NPM) Admin Setup - -This guide covers setting up MeshCore Hub behind Nginx Proxy Manager with admin authentication. - -## Overview - -Use two hostnames so the public map/site stays open while admin stays protected: - -1. **Public host**: no Access List (normal users). -2. **Admin host**: Access List enabled (operators only). - -Both proxy hosts should forward to the same web container: - -| Setting | Value | -| ---------------------- | -------------------------------------------- | -| Scheme | `http` | -| Forward Hostname/IP | Your MeshCore Hub host | -| Forward Port | `18080` (or your mapped web port) | -| Websockets Support | `ON` | -| Block Common Exploits | `ON` | - -**Important:** - -- Do not host this app under a subpath (for example `/meshcore`); proxy it at `/`. -- `WEB_ADMIN_ENABLED` must be `true`. - -## Advanced Configuration - -In NPM, for the **admin host**, paste this in the `Advanced` field: - -```nginx -# Forward authenticated identity for MeshCore Hub admin checks -proxy_set_header Authorization $http_authorization; -proxy_set_header X-Forwarded-User $remote_user; -proxy_set_header X-Auth-Request-User $remote_user; -proxy_set_header X-Forwarded-Email ""; -proxy_set_header X-Forwarded-Groups ""; -``` - -Then attach your NPM Access List (Basic auth users) to that admin host. - -## Verifying Auth Forwarding - -```bash -curl -s -u 'admin:password' "https://admin.example.com/config.js?t=$(date +%s)" \ - | grep -o '"is_authenticated":[^,]*' -``` - -Expected: - -```text -"is_authenticated": true -``` - -If it still shows `false`, check: - -1. You are using the admin hostname, not the public hostname. -2. The Access List is attached to that admin host. -3. The `Advanced` block above is present exactly. -4. `WEB_ADMIN_ENABLED=true` is loaded in the running web container. diff --git a/docs/i18n.md b/docs/i18n.md index 88b313f..50e013a 100644 --- a/docs/i18n.md +++ b/docs/i18n.md @@ -340,8 +340,6 @@ Admin panel content: | `access_denied` | Access Denied | Access denied heading | | `admin_not_enabled` | The admin interface is not enabled. | Admin disabled message | | `admin_enable_hint` | Set WEB_ADMIN_ENABLED=true to enable admin features. | Configuration hint (HTML allowed) | -| `auth_required` | Authentication Required | Auth required heading | -| `auth_required_description` | You must sign in to access the admin interface. | Auth required description | | `welcome` | Welcome to the admin panel. | Admin welcome message | | `members_description` | Manage network members and operators. | Members card description | | `tags_description` | Manage custom tags and metadata for network nodes. | Tags card description | diff --git a/docs/plans/20260428-1251-remove-header-auth/plan.md b/docs/plans/20260428-1251-remove-header-auth/plan.md new file mode 100644 index 0000000..7885cc2 --- /dev/null +++ b/docs/plans/20260428-1251-remove-header-auth/plan.md @@ -0,0 +1,213 @@ +# Plan: Remove Header-Based Auth + +**Date:** 2026-04-28 +**Status:** Approved +**Scope:** Remove all header-based proxy authentication mechanisms; keep `WEB_ADMIN_ENABLED` as a simple feature flag and preserve Admin UI components. + +## Background + +The web dashboard currently uses a two-layer auth architecture: + +1. **Web layer (header-based proxy auth)** — An external reverse proxy (OAuth2Proxy, Nginx with basic auth) authenticates users and injects headers (`X-Forwarded-User`, `X-Auth-Request-User`, `Authorization: Basic`). The web app reads these headers via `_is_authenticated_proxy_request()` to gate admin UI and block mutating API proxy calls. +2. **API layer (Bearer token auth)** — The backend API uses `HTTPBearer` tokens (`require_read`/`require_admin` in `api/auth.py`). This is independent and **not in scope**. + +The plan is to implement native OAuth/OIDC support in a future unit of work. This plan removes the header-based login mechanisms only. + +## Decisions + +- **Keep `WEB_ADMIN_ENABLED`** as a simple feature flag (Option A). It toggles admin UI visibility without any auth dependency. The future OIDC work will add proper session auth. +- **Remove the `/oauth2/` skip rule** from the SPA router — no longer needed. +- **Keep `sign_in`/`sign_out` i18n keys** in `common` section — forward-compatible for future OIDC work. +- **API Bearer token auth** (`api/auth.py`, `require_read`, `require_admin`) is completely untouched. + +--- + +## Phase 1: Python Source Changes + +### 1.1 `src/meshcore_hub/common/config.py` + +- **Remove** `web_trusted_proxy_hosts` field (lines 283-287) entirely +- **Update** `web_admin_enabled` description from `"requires OAuth2Proxy in front"` to `"Enable admin interface at /a/"` + +### 1.2 `src/meshcore_hub/web/app.py` + +- **Remove** `_is_authenticated_proxy_request()` function (lines 79-92) +- **Remove** `ProxyHeadersMiddleware` import and setup block (lines 248-254) +- **Remove** the startup warning block for trusted proxy hosts (lines 261-266) +- **In `_build_config_json()`**: Remove `"is_authenticated"` key (line 180) +- **In `api_proxy()` handler**: + - Remove the auth proxy header forwarding block (lines 387-390) + - Remove the 401 guard block for unauthenticated mutating requests (lines 392-404) +- **Keep** `admin_enabled` parameter in `create_app()` signature and `app.state.admin_enabled` (lines 199, 257-258, 286) — stays as a simple feature flag +- **Keep** `"admin_enabled"` in `_build_config_json()` (line 173) +- **Keep** `"admin_enabled"` in SPA catch-all template context (line 736) + +--- + +## Phase 2: JavaScript SPA Changes + +### 2.1 `src/meshcore_hub/web/static/js/spa/pages/admin/index.js` + +- **Remove** the `!config.is_authenticated` block (lines 20-28) with `/oauth2/start` link +- **Remove** the Sign Out link to `/oauth2/sign_out` (line 42) +- **Keep** the `!config.admin_enabled` guard (lines 8-17) and the authenticated admin content (lines 31-69) + +### 2.2 `src/meshcore_hub/web/static/js/spa/pages/admin/node-tags.js` + +- **Remove** the `!config.is_authenticated` block (lines 24-32) with `/oauth2/start` link +- **Remove** the Sign Out link to `/oauth2/sign_out` (line 316) +- **Keep** the `!config.admin_enabled` guard and all CRUD logic + +### 2.3 `src/meshcore_hub/web/static/js/spa/pages/admin/members.js` + +- **Remove** the `!config.is_authenticated` block (lines 23-31) with `/oauth2/start` link +- **Remove** the Sign Out link to `/oauth2/sign_out` (line 97) +- **Keep** the `!config.admin_enabled` guard and all CRUD logic + +### 2.4 `src/meshcore_hub/web/static/js/spa/pages/node-detail.js` + +- **Change** line 129 from `(config.admin_enabled && config.is_authenticated)` to just `config.admin_enabled` + +### 2.5 `src/meshcore_hub/web/static/js/spa/router.js` + +- **Remove** `href.startsWith('/oauth2/') ||` from the skip-rules (line 149) + +--- + +## Phase 3: Template Changes + +### 3.1 `src/meshcore_hub/web/templates/spa.html` + +- **No changes needed** — the admin footer link is gated by `admin_enabled` which stays + +--- + +## Phase 4: I18N Changes + +### 4.1 `src/meshcore_hub/web/static/locales/en.json` + +- **Remove** `"auth_required"` and `"auth_required_description"` from `admin` section +- **Keep** `access_denied`, `admin_not_enabled`, `admin_enable_hint`, `welcome`, `members_description`, `tags_description` +- **Keep** `"sign_in"` and `"sign_out"` in `common` section (forward-compatible for OIDC) + +### 4.2 `src/meshcore_hub/web/static/locales/nl.json` + +- Same changes as `en.json` — remove `auth_required` and `auth_required_description` + +### 4.3 `docs/i18n.md` + +- **Remove** rows for `auth_required` and `auth_required_description` from the admin section table (lines 343-344) + +--- + +## Phase 5: Test Changes + +### 5.1 `tests/test_web/test_admin.py` + +- **Remove** fixtures: `auth_headers`, `auth_headers_basic`, `auth_headers_auth_request` (lines 52-75) +- **Remove** entire `TestAdminApiProxyAuth` class (lines 240-347) — all server-side header auth enforcement tests +- **In `TestAdminHome`**: + - Remove `test_admin_home_config_authenticated` (line 120) + - Remove `test_admin_home_config_authenticated_with_basic_auth` (line 132) + - Remove `test_admin_home_config_authenticated_with_auth_request_header` (line 146) + - Remove `test_admin_home_unauthenticated_config` (line 194) + - Remove `auth_headers` parameter from remaining tests that use it (lines 104, 110, 168, 172) + - Keep `test_admin_home_returns_spa_shell`, `test_admin_home_config_admin_enabled`, `test_admin_home_disabled_*` +- **In `TestAdminNodeTags`**: + - Remove `auth_headers` parameter from remaining tests (lines 210, 218, 225) + - Keep route tests (they just test the shell is served) + - Remove `test_node_tags_page_unauthenticated` (line 233) +- **Keep** `TestAdminFooterLink` class entirely + +### 5.2 `tests/test_web/test_app.py` + +- **Remove** entire `TestTrustedProxyHostsWarning` class (lines 149-279) — all 4 tests +- **Keep** `TestConfigJsonXssEscaping` class entirely (unrelated to auth) + +### 5.3 `tests/test_web/test_home.py` + +- **Remove** `test_home_unauthenticated` (line 91) — tests `is_authenticated: false` +- **Remove** `test_home_authenticated` (line 103) — tests `is_authenticated: true` with headers + +### 5.4 `tests/test_web/test_advertisements.py` + +- **Remove** `test_advertisements_config_unauthenticated` (line 94) — tests `is_authenticated: false` + +### 5.5 `tests/test_web/conftest.py` + +- **No changes needed** — the `admin_app` fixture uses `admin_enabled=True` which stays + +--- + +## Phase 6: Documentation & Config Changes + +### 6.1 `docs/hosting/nginx-proxy-manager.md` + +- **Delete the entire file** — it's entirely about header-based auth proxy setup + +### 6.2 `README.md` + +- **Update** `WEB_ADMIN_ENABLED` row (line 376) — simplify description to `"Enable admin interface at /a/"` without the auth proxy mention +- **Remove** `WEB_TRUSTED_PROXY_HOSTS` row (line 377) + +### 6.3 `AGENTS.md` + +- **Update** `WEB_ADMIN_ENABLED` description (line 642) — remove `(default: false, requires auth proxy)` to `(default: false)` +- **Remove** `WEB_TRUSTED_PROXY_HOSTS` line (line 643) +- **Update** line 696 `WEB_ADMIN_ENABLED` reference if needed + +### 6.4 `.env.example` + +- **Update** the `WEB_ADMIN_ENABLED` comment block (lines 338-340) — remove "requires auth proxy in front" +- **Remove** the `WEB_TRUSTED_PROXY_HOSTS` comment block (lines 342-346) + +### 6.5 `docker-compose.yml` + +- **Keep** `WEB_ADMIN_ENABLED` line (line 268) — it's still a valid env var for the feature flag + +### 6.6 `.agents/skills/docs-sync/references/documentation-checklist.md` + +- **Remove** the entire NPM checklist block (lines 158-165) since the doc is being deleted +- **Remove** the `WEB_ADMIN_ENABLED` checklist item (line 162) + +### 6.7 `.agents/skills/docs-sync/references/docker-source-guide.md` + +- **Keep** the `WEB_ADMIN_ENABLED` row (line 196) since it's still a valid env var + +--- + +## Phase 7: Verification + +After all changes: + +1. `pytest tests/test_web/` — all web tests pass +2. `pytest tests/test_api/test_auth.py` — API auth tests untouched, still pass +3. `pre-commit run --all-files` — linting/type checking passes +4. `pytest` — full suite passes with no regressions + +--- + +## Files Modified Summary + +| # | File | Action | +|---|------|--------| +| 1 | `src/meshcore_hub/common/config.py` | Remove `web_trusted_proxy_hosts`, update `web_admin_enabled` description | +| 2 | `src/meshcore_hub/web/app.py` | Remove `_is_authenticated_proxy_request()`, `ProxyHeadersMiddleware`, header forwarding, 401 guard, `is_authenticated` in config | +| 3 | `src/meshcore_hub/web/static/js/spa/pages/admin/index.js` | Remove auth guard + sign in/out links | +| 4 | `src/meshcore_hub/web/static/js/spa/pages/admin/node-tags.js` | Remove auth guard + sign out link | +| 5 | `src/meshcore_hub/web/static/js/spa/pages/admin/members.js` | Remove auth guard + sign out link | +| 6 | `src/meshcore_hub/web/static/js/spa/pages/node-detail.js` | Simplify `admin_enabled && is_authenticated` → `admin_enabled` | +| 7 | `src/meshcore_hub/web/static/js/spa/router.js` | Remove `/oauth2/` skip rule | +| 8 | `src/meshcore_hub/web/static/locales/en.json` | Remove `auth_required*` keys | +| 9 | `src/meshcore_hub/web/static/locales/nl.json` | Remove `auth_required*` keys | +| 10 | `tests/test_web/test_admin.py` | Remove auth fixtures, `TestAdminApiProxyAuth`, auth-related assertions | +| 11 | `tests/test_web/test_app.py` | Remove `TestTrustedProxyHostsWarning` class | +| 12 | `tests/test_web/test_home.py` | Remove auth-related tests | +| 13 | `tests/test_web/test_advertisements.py` | Remove unauthenticated config test | +| 14 | `docs/hosting/nginx-proxy-manager.md` | **Delete file** | +| 15 | `README.md` | Update env var table | +| 16 | `AGENTS.md` | Update env var descriptions | +| 17 | `.env.example` | Update/remove env var comments | +| 18 | `docs/i18n.md` | Remove `auth_required*` rows | +| 19 | `.agents/skills/docs-sync/references/documentation-checklist.md` | Remove NPM checklist | +| 20 | `.agents/skills/docs-sync/references/docker-source-guide.md` | Update `WEB_ADMIN_ENABLED` row | diff --git a/docs/plans/20260428-1251-remove-header-auth/tasks.md b/docs/plans/20260428-1251-remove-header-auth/tasks.md new file mode 100644 index 0000000..67a5e08 --- /dev/null +++ b/docs/plans/20260428-1251-remove-header-auth/tasks.md @@ -0,0 +1,88 @@ +# Remove Header-Based Auth — Task Checklist + +**Plan:** `docs/plans/20260428-1251-remove-header-auth/plan.md` +**Status:** Complete + +--- + +## Phase 1: Python Source Changes + +- [x] **1.1** `src/meshcore_hub/common/config.py` — Remove `web_trusted_proxy_hosts` field (lines 283-287) +- [x] **1.2** `src/meshcore_hub/common/config.py` — Update `web_admin_enabled` description to remove `"requires OAuth2Proxy in front"` +- [x] **1.3** `src/meshcore_hub/web/app.py` — Remove `_is_authenticated_proxy_request()` function (lines 79-92) +- [x] **1.4** `src/meshcore_hub/web/app.py` — Remove `ProxyHeadersMiddleware` import and setup block (lines 248-254) +- [x] **1.5** `src/meshcore_hub/web/app.py` — Remove startup warning block for trusted proxy hosts (lines 261-266) +- [x] **1.6** `src/meshcore_hub/web/app.py` — Remove `"is_authenticated"` key from `_build_config_json()` (line 180) +- [x] **1.7** `src/meshcore_hub/web/app.py` — Remove auth proxy header forwarding in `api_proxy()` handler (lines 387-390) +- [x] **1.8** `src/meshcore_hub/web/app.py` — Remove 401 guard block for unauthenticated mutating requests in `api_proxy()` handler (lines 392-404) + +## Phase 2: JavaScript SPA Changes + +- [x] **2.1** `src/meshcore_hub/web/static/js/spa/pages/admin/index.js` — Remove `!config.is_authenticated` block with `/oauth2/start` link (lines 20-28) +- [x] **2.2** `src/meshcore_hub/web/static/js/spa/pages/admin/index.js` — Remove Sign Out link to `/oauth2/sign_out` (line 42) +- [x] **2.3** `src/meshcore_hub/web/static/js/spa/pages/admin/node-tags.js` — Remove `!config.is_authenticated` block with `/oauth2/start` link (lines 24-32) +- [x] **2.4** `src/meshcore_hub/web/static/js/spa/pages/admin/node-tags.js` — Remove Sign Out link to `/oauth2/sign_out` (line 316) +- [x] **2.5** `src/meshcore_hub/web/static/js/spa/pages/admin/members.js` — Remove `!config.is_authenticated` block with `/oauth2/start` link (lines 23-31) +- [x] **2.6** `src/meshcore_hub/web/static/js/spa/pages/admin/members.js` — Remove Sign Out link to `/oauth2/sign_out` (line 97) +- [x] **2.7** `src/meshcore_hub/web/static/js/spa/pages/node-detail.js` — Change `(config.admin_enabled && config.is_authenticated)` to `config.admin_enabled` (line 129) +- [x] **2.8** `src/meshcore_hub/web/static/js/spa/router.js` — Remove `href.startsWith('/oauth2/') ||` from skip-rules (line 149) + +## Phase 3: I18N Changes + +- [x] **3.1** `src/meshcore_hub/web/static/locales/en.json` — Remove `"auth_required"` and `"auth_required_description"` from `admin` section +- [x] **3.2** `src/meshcore_hub/web/static/locales/nl.json` — Remove `"auth_required"` and `"auth_required_description"` from `admin` section + +## Phase 4: Test Changes + +- [x] **4.1** `tests/test_web/test_admin.py` — Remove `auth_headers`, `auth_headers_basic`, `auth_headers_auth_request` fixtures (lines 52-75) +- [x] **4.2** `tests/test_web/test_admin.py` — Remove entire `TestAdminApiProxyAuth` class (lines 240-347) +- [x] **4.3** `tests/test_web/test_admin.py` — In `TestAdminHome`: remove `test_admin_home_config_authenticated`, `test_admin_home_config_authenticated_with_basic_auth`, `test_admin_home_config_authenticated_with_auth_request_header`, `test_admin_home_unauthenticated_config` +- [x] **4.4** `tests/test_web/test_admin.py` — In `TestAdminHome`: remove `auth_headers` param from remaining tests (`test_admin_home_returns_spa_shell`, `test_admin_home_config_admin_enabled`, `test_admin_home_disabled_*`) +- [x] **4.5** `tests/test_web/test_admin.py` — In `TestAdminNodeTags`: remove `auth_headers` param from remaining tests, remove `test_node_tags_page_unauthenticated` +- [x] **4.6** `tests/test_web/test_app.py` — Remove entire `TestTrustedProxyHostsWarning` class (lines 149-279) +- [x] **4.7** `tests/test_web/test_home.py` — Remove `test_home_unauthenticated` and `test_home_authenticated` +- [x] **4.8** `tests/test_web/test_advertisements.py` — Remove `test_advertisements_config_unauthenticated` + +## Phase 5: Documentation & Config Changes + +- [x] **5.1** Delete `docs/hosting/nginx-proxy-manager.md` entirely +- [x] **5.2** `README.md` — Update `WEB_ADMIN_ENABLED` row to remove auth proxy mention; remove `WEB_TRUSTED_PROXY_HOSTS` row +- [x] **5.3** `AGENTS.md` — Update `WEB_ADMIN_ENABLED` description, remove `WEB_TRUSTED_PROXY_HOSTS` line +- [x] **5.4** `.env.example` — Update `WEB_ADMIN_ENABLED` comment to remove "requires auth proxy in front"; remove `WEB_TRUSTED_PROXY_HOSTS` block +- [x] **5.5** `docs/i18n.md` — Remove `auth_required` and `auth_required_description` rows from admin section table +- [x] **5.6** `.agents/skills/docs-sync/references/documentation-checklist.md` — Remove NPM checklist block and `WEB_ADMIN_ENABLED` checklist item +- [x] **5.7** `.agents/skills/docs-sync/references/docker-source-guide.md` — Update `WEB_ADMIN_ENABLED` row description + +## Phase 6: Verification + +- [x] **6.1** Run `pytest tests/test_web/ -v` — 170 passed +- [x] **6.2** Run `pytest tests/test_api/ -v` — 164 passed +- [x] **6.3** Run `pytest` — 572 passed, 22 skipped (E2E) +- [x] **6.4** Run `pre-commit run --all-files` — all hooks passed + +--- + +## File Change Summary + +| # | File | Action | Phase(s) | +|---|------|--------|----------| +| 1 | `src/meshcore_hub/common/config.py` | Modify | 1 | +| 2 | `src/meshcore_hub/web/app.py` | Modify | 1 | +| 3 | `src/meshcore_hub/web/static/js/spa/pages/admin/index.js` | Modify | 2 | +| 4 | `src/meshcore_hub/web/static/js/spa/pages/admin/node-tags.js` | Modify | 2 | +| 5 | `src/meshcore_hub/web/static/js/spa/pages/admin/members.js` | Modify | 2 | +| 6 | `src/meshcore_hub/web/static/js/spa/pages/node-detail.js` | Modify | 2 | +| 7 | `src/meshcore_hub/web/static/js/spa/router.js` | Modify | 2 | +| 8 | `src/meshcore_hub/web/static/locales/en.json` | Modify | 3 | +| 9 | `src/meshcore_hub/web/static/locales/nl.json` | Modify | 3 | +| 10 | `tests/test_web/test_admin.py` | Modify | 4 | +| 11 | `tests/test_web/test_app.py` | Modify | 4 | +| 12 | `tests/test_web/test_home.py` | Modify | 4 | +| 13 | `tests/test_web/test_advertisements.py` | Modify | 4 | +| 14 | `docs/hosting/nginx-proxy-manager.md` | Delete | 5 | +| 15 | `README.md` | Modify | 5 | +| 16 | `AGENTS.md` | Modify | 5 | +| 17 | `.env.example` | Modify | 5 | +| 18 | `docs/i18n.md` | Modify | 5 | +| 19 | `.agents/skills/docs-sync/references/documentation-checklist.md` | Modify | 5 | +| 20 | `.agents/skills/docs-sync/references/docker-source-guide.md` | Modify | 5 | diff --git a/src/meshcore_hub/common/config.py b/src/meshcore_hub/common/config.py index d1ae246..359bdd3 100644 --- a/src/meshcore_hub/common/config.py +++ b/src/meshcore_hub/common/config.py @@ -280,16 +280,10 @@ 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, - description="Enable admin interface at /a/ (requires OAuth2Proxy in front)", + description="Enable admin interface at /a/", ) # API connection diff --git a/src/meshcore_hub/web/app.py b/src/meshcore_hub/web/app.py index 3aa99be..6d398bc 100644 --- a/src/meshcore_hub/web/app.py +++ b/src/meshcore_hub/web/app.py @@ -15,7 +15,6 @@ from fastapi import FastAPI, Request, Response from fastapi.responses import HTMLResponse, JSONResponse, PlainTextResponse from fastapi.staticfiles import StaticFiles from fastapi.templating import Jinja2Templates -from uvicorn.middleware.proxy_headers import ProxyHeadersMiddleware from meshcore_hub import __version__ from meshcore_hub.collector.letsmesh_decoder import LetsMeshPacketDecoder @@ -76,22 +75,6 @@ def _resolve_logo(media_home: Path) -> tuple[str, bool, Path | None]: return "/static/img/logo.svg", True, None -def _is_authenticated_proxy_request(request: Request) -> bool: - """Check whether request is authenticated by an upstream auth proxy. - - Supported patterns: - - OAuth2/OIDC proxy headers: X-Forwarded-User, X-Auth-Request-User - - Forwarded Basic auth header: Authorization: Basic ... - """ - if request.headers.get("x-forwarded-user"): - return True - if request.headers.get("x-auth-request-user"): - return True - - auth_header = request.headers.get("authorization", "") - return auth_header.lower().startswith("basic ") - - @asynccontextmanager async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: """Application lifespan handler.""" @@ -177,7 +160,6 @@ def _build_config_json(app: FastAPI, request: Request) -> str: "version": __version__, "timezone": app.state.timezone_abbr, "timezone_iana": app.state.timezone, - "is_authenticated": _is_authenticated_proxy_request(request), "default_theme": app.state.web_theme, "locale": app.state.web_locale, "datetime_locale": app.state.web_datetime_locale, @@ -245,26 +227,11 @@ def create_app( redoc_url=None, ) - # Trust proxy headers (X-Forwarded-Proto, X-Forwarded-For) for HTTPS detection - 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) @@ -384,25 +351,6 @@ def create_app( if "content-type" in request.headers: headers["content-type"] = request.headers["content-type"] - # Forward auth proxy headers for admin operations - for h in ("x-forwarded-user", "x-forwarded-email", "x-forwarded-groups"): - if h in request.headers: - headers[h] = request.headers[h] - - # Block mutating requests from unauthenticated users when admin is - # enabled. OAuth2Proxy is expected to set X-Forwarded-User for - # authenticated sessions; without it, write operations must be - # rejected server-side to prevent auth bypass. - if ( - request.method in ("POST", "PUT", "DELETE", "PATCH") - and request.app.state.admin_enabled - and not _is_authenticated_proxy_request(request) - ): - return JSONResponse( - {"detail": "Authentication required"}, - status_code=401, - ) - try: response = await client.request( method=request.method, diff --git a/src/meshcore_hub/web/static/js/spa/pages/admin/index.js b/src/meshcore_hub/web/static/js/spa/pages/admin/index.js index 9196572..43780ae 100644 --- a/src/meshcore_hub/web/static/js/spa/pages/admin/index.js +++ b/src/meshcore_hub/web/static/js/spa/pages/admin/index.js @@ -17,17 +17,6 @@ export async function render(container, params, router) { return; } - if (!config.is_authenticated) { - litRender(html` -
- ${iconLock('h-16 w-16 opacity-30 mb-4')} -

${t('admin.auth_required')}

-

${t('admin.auth_required_description')}

- ${t('common.sign_in')} -
`, container); - return; - } - litRender(html`
@@ -39,7 +28,6 @@ export async function render(container, params, router) {
- ${t('common.sign_out')}
diff --git a/src/meshcore_hub/web/static/js/spa/pages/admin/members.js b/src/meshcore_hub/web/static/js/spa/pages/admin/members.js index 9a5972a..d916892 100644 --- a/src/meshcore_hub/web/static/js/spa/pages/admin/members.js +++ b/src/meshcore_hub/web/static/js/spa/pages/admin/members.js @@ -20,17 +20,6 @@ export async function render(container, params, router) { return; } - if (!config.is_authenticated) { - litRender(html` -
- ${iconLock('h-16 w-16 opacity-30 mb-4')} -

${t('admin.auth_required')}

-

${t('admin.auth_required_description')}

- ${t('common.sign_in')} -
`, container); - return; - } - const flashMessage = (params.query && params.query.message) || ''; const flashError = (params.query && params.query.error) || ''; @@ -94,7 +83,6 @@ export async function render(container, params, router) {
- ${t('common.sign_out')} ${flashHtml} diff --git a/src/meshcore_hub/web/static/js/spa/pages/admin/node-tags.js b/src/meshcore_hub/web/static/js/spa/pages/admin/node-tags.js index 788cc19..5293ef1 100644 --- a/src/meshcore_hub/web/static/js/spa/pages/admin/node-tags.js +++ b/src/meshcore_hub/web/static/js/spa/pages/admin/node-tags.js @@ -21,17 +21,6 @@ export async function render(container, params, router) { return; } - if (!config.is_authenticated) { - litRender(html` -
- ${iconLock('h-16 w-16 opacity-30 mb-4')} -

${t('admin.auth_required')}

-

${t('admin.auth_required_description')}

- ${t('common.sign_in')} -
`, container); - return; - } - const selectedPublicKey = (params.query && params.query.public_key) || ''; const flashMessage = (params.query && params.query.message) || ''; const flashError = (params.query && params.query.error) || ''; @@ -313,7 +302,6 @@ export async function render(container, params, router) { - ${t('common.sign_out')} ${flashHtml} diff --git a/src/meshcore_hub/web/static/js/spa/pages/node-detail.js b/src/meshcore_hub/web/static/js/spa/pages/node-detail.js index 8c2475e..02006e9 100644 --- a/src/meshcore_hub/web/static/js/spa/pages/node-detail.js +++ b/src/meshcore_hub/web/static/js/spa/pages/node-detail.js @@ -126,7 +126,7 @@ export async function render(container, params, router) { ` : html`

${t('common.no_entity_defined', { entity: t('entities.tags').toLowerCase() })}

`; - const adminTagsHtml = (config.admin_enabled && config.is_authenticated) + const adminTagsHtml = config.admin_enabled ? html`
${tags.length > 0 ? t('common.edit_entity', { entity: t('entities.tags') }) : t('common.add_entity', { entity: t('entities.tags') })}
` diff --git a/src/meshcore_hub/web/static/js/spa/router.js b/src/meshcore_hub/web/static/js/spa/router.js index a825709..d2a847b 100644 --- a/src/meshcore_hub/web/static/js/spa/router.js +++ b/src/meshcore_hub/web/static/js/spa/router.js @@ -146,7 +146,7 @@ export class Router { // Skip non-SPA paths (static files, API, media, OAuth, SEO) if (href.startsWith('/static/') || href.startsWith('/media/') || - href.startsWith('/api/') || href.startsWith('/oauth2/') || + href.startsWith('/api/') || href.startsWith('/health') || href === '/robots.txt' || href === '/sitemap.xml') return; diff --git a/src/meshcore_hub/web/static/locales/en.json b/src/meshcore_hub/web/static/locales/en.json index bf3c6c6..1c376e6 100644 --- a/src/meshcore_hub/web/static/locales/en.json +++ b/src/meshcore_hub/web/static/locales/en.json @@ -186,8 +186,6 @@ "access_denied": "Access Denied", "admin_not_enabled": "The admin interface is not enabled.", "admin_enable_hint": "Set WEB_ADMIN_ENABLED=true to enable admin features.", - "auth_required": "Authentication Required", - "auth_required_description": "You must sign in to access the admin interface.", "welcome": "Welcome to the admin panel.", "members_description": "Manage network members and operators.", "tags_description": "Manage custom tags and metadata for network nodes." diff --git a/src/meshcore_hub/web/static/locales/nl.json b/src/meshcore_hub/web/static/locales/nl.json index fb3726b..819617c 100644 --- a/src/meshcore_hub/web/static/locales/nl.json +++ b/src/meshcore_hub/web/static/locales/nl.json @@ -181,8 +181,6 @@ "access_denied": "Toegang geweigerd", "admin_not_enabled": "De beheerinterface is niet ingeschakeld.", "admin_enable_hint": "Stel WEB_ADMIN_ENABLED=true in om beheerfuncties in te schakelen.", - "auth_required": "Authenticatie vereist", - "auth_required_description": "U moet inloggen om toegang te krijgen tot de beheerinterface.", "welcome": "Welkom bij het beheerpaneel.", "members_description": "Beheer netwerkleden en operators.", "tags_description": "Beheer aangepaste labels en metadata voor netwerkknooppunten." diff --git a/tests/test_web/test_admin.py b/tests/test_web/test_admin.py index ac70df1..3926ac0 100644 --- a/tests/test_web/test_admin.py +++ b/tests/test_web/test_admin.py @@ -49,32 +49,6 @@ def admin_app_disabled(mock_http_client: MockHttpClient) -> Any: return app -@pytest.fixture -def auth_headers() -> dict: - """Authentication headers for admin requests.""" - return { - "X-Forwarded-User": "test-user-id", - "X-Forwarded-Email": "test@example.com", - "X-Forwarded-Preferred-Username": "testuser", - } - - -@pytest.fixture -def auth_headers_basic() -> dict[str, str]: - """Basic auth header forwarded by reverse proxy.""" - return { - "Authorization": "Basic dGVzdDp0ZXN0", - } - - -@pytest.fixture -def auth_headers_auth_request() -> dict[str, str]: - """Auth-request style header from upstream proxy.""" - return { - "X-Auth-Request-User": "test-user-id", - } - - @pytest.fixture def admin_client(admin_app: Any, mock_http_client: MockHttpClient) -> TestClient: """Create a test client with admin enabled.""" @@ -96,18 +70,18 @@ class TestAdminHome: In the SPA architecture, admin routes serve the same shell HTML. Admin access control is handled client-side based on - window.__APP_CONFIG__.admin_enabled and is_authenticated. + window.__APP_CONFIG__.admin_enabled. """ - def test_admin_home_returns_spa_shell(self, admin_client, auth_headers): + def test_admin_home_returns_spa_shell(self, admin_client): """Test admin home page returns the SPA shell.""" - response = admin_client.get("/a/", headers=auth_headers) + response = admin_client.get("/a/") assert response.status_code == 200 assert "window.__APP_CONFIG__" in response.text - def test_admin_home_config_admin_enabled(self, admin_client, auth_headers): + def test_admin_home_config_admin_enabled(self, admin_client): """Test admin config shows admin_enabled: true.""" - response = admin_client.get("/a/", headers=auth_headers) + response = admin_client.get("/a/") text = response.text config_start = text.find("window.__APP_CONFIG__ = ") + len( "window.__APP_CONFIG__ = " @@ -117,235 +91,46 @@ class TestAdminHome: assert config["admin_enabled"] is True - def test_admin_home_config_authenticated(self, admin_client, auth_headers): - """Test admin config shows is_authenticated: true with auth headers.""" - response = admin_client.get("/a/", headers=auth_headers) - text = response.text - config_start = text.find("window.__APP_CONFIG__ = ") + len( - "window.__APP_CONFIG__ = " - ) - config_end = text.find(";", config_start) - config = json.loads(text[config_start:config_end]) - - assert config["is_authenticated"] is True - - def test_admin_home_config_authenticated_with_basic_auth( - self, admin_client, auth_headers_basic - ): - """Test admin config shows is_authenticated: true with basic auth header.""" - response = admin_client.get("/a/", headers=auth_headers_basic) - text = response.text - config_start = text.find("window.__APP_CONFIG__ = ") + len( - "window.__APP_CONFIG__ = " - ) - config_end = text.find(";", config_start) - config = json.loads(text[config_start:config_end]) - - assert config["is_authenticated"] is True - - def test_admin_home_config_authenticated_with_auth_request_header( - self, admin_client, auth_headers_auth_request - ): - """Test admin config shows is_authenticated with X-Auth-Request-User.""" - response = admin_client.get("/a/", headers=auth_headers_auth_request) - text = response.text - config_start = text.find("window.__APP_CONFIG__ = ") + len( - "window.__APP_CONFIG__ = " - ) - config_end = text.find(";", config_start) - config = json.loads(text[config_start:config_end]) - - assert config["is_authenticated"] is True - def test_admin_home_disabled_returns_spa_shell( - self, admin_client_disabled, auth_headers + self, + admin_client_disabled, ): """Test admin page returns SPA shell even when disabled. The SPA catch-all serves the shell for all routes. Client-side code checks admin_enabled to show/hide admin UI. """ - response = admin_client_disabled.get("/a/", headers=auth_headers) + response = admin_client_disabled.get("/a/") assert response.status_code == 200 assert "window.__APP_CONFIG__" in response.text - def test_admin_home_disabled_config(self, admin_client_disabled, auth_headers): - """Test admin config shows admin_enabled: false when disabled.""" - response = admin_client_disabled.get("/a/", headers=auth_headers) - text = response.text - config_start = text.find("window.__APP_CONFIG__ = ") + len( - "window.__APP_CONFIG__ = " - ) - config_end = text.find(";", config_start) - config = json.loads(text[config_start:config_end]) - - assert config["admin_enabled"] is False - - def test_admin_home_unauthenticated_returns_spa_shell(self, admin_client): - """Test admin page returns SPA shell without authentication. - - The SPA catch-all serves the shell for all routes. - Client-side code checks is_authenticated to show access denied. - """ - response = admin_client.get("/a/") - assert response.status_code == 200 - assert "window.__APP_CONFIG__" in response.text - - def test_admin_home_unauthenticated_config(self, admin_client): - """Test admin config shows is_authenticated: false without auth headers.""" - response = admin_client.get("/a/") - text = response.text - config_start = text.find("window.__APP_CONFIG__ = ") + len( - "window.__APP_CONFIG__ = " - ) - config_end = text.find(";", config_start) - config = json.loads(text[config_start:config_end]) - - assert config["is_authenticated"] is False - class TestAdminNodeTags: """Tests for admin node tags page (SPA).""" - def test_node_tags_page_returns_spa_shell(self, admin_client, auth_headers): + def test_node_tags_page_returns_spa_shell(self, admin_client): """Test node tags page returns the SPA shell.""" - response = admin_client.get("/a/node-tags", headers=auth_headers) + response = admin_client.get("/a/node-tags") assert response.status_code == 200 assert "window.__APP_CONFIG__" in response.text - def test_node_tags_page_with_public_key(self, admin_client, auth_headers): + def test_node_tags_page_with_public_key(self, admin_client): """Test node tags page with public_key param returns SPA shell.""" response = admin_client.get( "/a/node-tags?public_key=abc123def456abc123def456abc123de", - headers=auth_headers, ) assert response.status_code == 200 assert "window.__APP_CONFIG__" in response.text def test_node_tags_page_disabled_returns_spa_shell( - self, admin_client_disabled, auth_headers + self, + admin_client_disabled, ): """Test node tags page returns SPA shell even when admin is disabled.""" - response = admin_client_disabled.get("/a/node-tags", headers=auth_headers) + response = admin_client_disabled.get("/a/node-tags") assert response.status_code == 200 assert "window.__APP_CONFIG__" in response.text - def test_node_tags_page_unauthenticated(self, admin_client): - """Test node tags page returns SPA shell without authentication.""" - response = admin_client.get("/a/node-tags") - assert response.status_code == 200 - assert "window.__APP_CONFIG__" in response.text - - -class TestAdminApiProxyAuth: - """Tests for admin API proxy authentication enforcement. - - When admin is enabled, mutating requests (POST/PUT/DELETE/PATCH) through - the API proxy must require authentication via X-Forwarded-User header. - This prevents unauthenticated users from performing admin operations - even though the web app's HTTP client has a service-level API key. - """ - - def test_proxy_post_blocked_without_auth(self, admin_client, mock_http_client): - """POST to API proxy returns 401 without auth headers.""" - mock_http_client.set_response("POST", "/api/v1/members", 201, {"id": "new"}) - response = admin_client.post( - "/api/v1/members", - json={"name": "Test", "member_id": "test"}, - ) - assert response.status_code == 401 - assert "Authentication required" in response.json()["detail"] - - def test_proxy_put_blocked_without_auth(self, admin_client, mock_http_client): - """PUT to API proxy returns 401 without auth headers.""" - mock_http_client.set_response("PUT", "/api/v1/members/1", 200, {"id": "1"}) - response = admin_client.put( - "/api/v1/members/1", - json={"name": "Updated"}, - ) - assert response.status_code == 401 - - def test_proxy_delete_blocked_without_auth(self, admin_client, mock_http_client): - """DELETE to API proxy returns 401 without auth headers.""" - mock_http_client.set_response("DELETE", "/api/v1/members/1", 204, None) - response = admin_client.delete("/api/v1/members/1") - assert response.status_code == 401 - - def test_proxy_patch_blocked_without_auth(self, admin_client, mock_http_client): - """PATCH to API proxy returns 401 without auth headers.""" - mock_http_client.set_response("PATCH", "/api/v1/members/1", 200, {"id": "1"}) - response = admin_client.patch( - "/api/v1/members/1", - json={"name": "Patched"}, - ) - assert response.status_code == 401 - - def test_proxy_post_allowed_with_auth( - self, admin_client, auth_headers, mock_http_client - ): - """POST to API proxy succeeds with auth headers.""" - mock_http_client.set_response("POST", "/api/v1/members", 201, {"id": "new"}) - response = admin_client.post( - "/api/v1/members", - json={"name": "Test", "member_id": "test"}, - headers=auth_headers, - ) - assert response.status_code == 201 - - def test_proxy_post_allowed_with_basic_auth( - self, admin_client, auth_headers_basic, mock_http_client - ): - """POST to API proxy succeeds with basic auth header.""" - mock_http_client.set_response("POST", "/api/v1/members", 201, {"id": "new"}) - response = admin_client.post( - "/api/v1/members", - json={"name": "Test", "member_id": "test"}, - headers=auth_headers_basic, - ) - assert response.status_code == 201 - - def test_proxy_put_allowed_with_auth( - self, admin_client, auth_headers, mock_http_client - ): - """PUT to API proxy succeeds with auth headers.""" - mock_http_client.set_response("PUT", "/api/v1/members/1", 200, {"id": "1"}) - response = admin_client.put( - "/api/v1/members/1", - json={"name": "Updated"}, - headers=auth_headers, - ) - assert response.status_code == 200 - - def test_proxy_delete_allowed_with_auth( - self, admin_client, auth_headers, mock_http_client - ): - """DELETE to API proxy succeeds with auth headers.""" - mock_http_client.set_response("DELETE", "/api/v1/members/1", 204, None) - response = admin_client.delete( - "/api/v1/members/1", - headers=auth_headers, - ) - # 204 from the mock API - assert response.status_code == 204 - - def test_proxy_get_allowed_without_auth(self, admin_client, mock_http_client): - """GET to API proxy is allowed without auth (read-only).""" - response = admin_client.get("/api/v1/nodes") - assert response.status_code == 200 - - def test_proxy_post_allowed_when_admin_disabled( - self, admin_client_disabled, mock_http_client - ): - """POST to API proxy allowed when admin is disabled (no proxy auth).""" - mock_http_client.set_response("POST", "/api/v1/members", 201, {"id": "new"}) - response = admin_client_disabled.post( - "/api/v1/members", - json={"name": "Test", "member_id": "test"}, - ) - # Should reach the API (which may return its own auth error, but - # the proxy itself should not block it) - assert response.status_code == 201 - class TestAdminFooterLink: """Tests for admin link in footer.""" diff --git a/tests/test_web/test_advertisements.py b/tests/test_web/test_advertisements.py index a2769b6..89d350a 100644 --- a/tests/test_web/test_advertisements.py +++ b/tests/test_web/test_advertisements.py @@ -90,15 +90,3 @@ class TestAdvertisementsConfig: config = json.loads(text[config_start:config_end]) assert config["network_name"] == "Test Network" - - def test_advertisements_config_unauthenticated(self, client: TestClient) -> None: - """Test that SPA config shows unauthenticated without auth header.""" - response = client.get("/advertisements") - text = response.text - config_start = text.find("window.__APP_CONFIG__ = ") + len( - "window.__APP_CONFIG__ = " - ) - config_end = text.find(";", config_start) - config = json.loads(text[config_start:config_end]) - - assert config["is_authenticated"] is False diff --git a/tests/test_web/test_app.py b/tests/test_web/test_app.py index d7e1faa..b7d2781 100644 --- a/tests/test_web/test_app.py +++ b/tests/test_web/test_app.py @@ -1,9 +1,7 @@ -"""Tests for web app: config JSON escaping and trusted proxy hosts warnings.""" +"""Tests for web app: config JSON escaping.""" import json -import logging from typing import Any -from unittest.mock import patch import pytest from fastapi.testclient import TestClient @@ -144,136 +142,3 @@ class TestConfigJsonXssEscaping: 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"] == "*" diff --git a/tests/test_web/test_home.py b/tests/test_web/test_home.py index 518f921..3d2363c 100644 --- a/tests/test_web/test_home.py +++ b/tests/test_web/test_home.py @@ -87,27 +87,3 @@ class TestHomePage: """Test that home page includes the SPA application script.""" response = client.get("/") assert "/static/js/spa/app.js" in response.text - - def test_home_unauthenticated(self, client: TestClient) -> None: - """Test that home page config shows unauthenticated by default.""" - response = client.get("/") - text = response.text - config_start = text.find("window.__APP_CONFIG__ = ") + len( - "window.__APP_CONFIG__ = " - ) - config_end = text.find(";", config_start) - config = json.loads(text[config_start:config_end]) - - assert config["is_authenticated"] is False - - def test_home_authenticated(self, client: TestClient) -> None: - """Test that home page config shows authenticated with auth header.""" - response = client.get("/", headers={"X-Forwarded-User": "test-user"}) - text = response.text - config_start = text.find("window.__APP_CONFIG__ = ") + len( - "window.__APP_CONFIG__ = " - ) - config_end = text.find(";", config_start) - config = json.loads(text[config_start:config_end]) - - assert config["is_authenticated"] is True