feat(api): remove redundant /env and /api/diagnostic/settings endpoints
- Remove `/api/diagnostic/settings` endpoint (superseded by `/api/settings/`) - Remove `/env` view route (superseded by `/settings` admin page) - Delete `env_debug.html` template - Remove `/env` nav links from base.html (desktop + mobile) - Update status_dashboard.html to link to /settings instead of /env - Remove corresponding tests for deleted endpoints - Update RateLimitingStrategy.md docs Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
@@ -6,118 +6,6 @@ import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestDiagnosticSettings:
|
||||
"""Tests for GET /diagnostic/settings endpoint."""
|
||||
|
||||
@patch("app.utils.config_validator.dump_all_settings")
|
||||
def test_diagnostic_settings_success(self, mock_dump, client: TestClient):
|
||||
"""Test successful diagnostic settings retrieval."""
|
||||
from app.config import settings
|
||||
|
||||
with patch.object(settings, "workdir", "/tmp/test"):
|
||||
with patch.object(settings, "external_hostname", "test-host"):
|
||||
with patch.object(settings, "email_host", "smtp.test.com"):
|
||||
with patch.object(settings, "openai_api_key", "sk-test"):
|
||||
# The endpoint requires login, so we'd need to mock auth
|
||||
# Testing the function logic directly
|
||||
pass
|
||||
|
||||
@patch("app.utils.config_validator.dump_all_settings")
|
||||
def test_diagnostic_settings_logs_to_file(self, mock_dump):
|
||||
"""Test that settings are dumped to logs."""
|
||||
# Endpoint should call dump_all_settings
|
||||
# mock_dump should be called once
|
||||
|
||||
@patch("app.utils.config_validator.dump_all_settings")
|
||||
def test_diagnostic_settings_returns_safe_subset(self, mock_dump):
|
||||
"""Test that only safe settings are returned in response."""
|
||||
from app.config import settings
|
||||
|
||||
with patch.object(settings, "openai_api_key", "sk-secret-key"):
|
||||
# Response should NOT contain the actual API key
|
||||
# Should only return bool indicating it's configured
|
||||
pass
|
||||
|
||||
def test_diagnostic_settings_configured_services_all_false(self):
|
||||
"""Test configured_services when nothing is configured."""
|
||||
from app.config import settings
|
||||
|
||||
with patch.object(settings, "email_host", None):
|
||||
with patch.object(settings, "s3_bucket_name", None):
|
||||
with patch.object(settings, "dropbox_refresh_token", None):
|
||||
with patch.object(settings, "onedrive_refresh_token", None):
|
||||
with patch.object(settings, "nextcloud_upload_url", None):
|
||||
with patch.object(settings, "sftp_host", None):
|
||||
with patch.object(settings, "paperless_host", None):
|
||||
with patch.object(settings, "google_drive_credentials_json", None):
|
||||
with patch.object(settings, "uptime_kuma_url", None):
|
||||
with patch.object(settings, "authentik_config_url", None):
|
||||
with patch.object(settings, "openai_api_key", None):
|
||||
with patch.object(settings, "azure_ai_key", None):
|
||||
# All configured_services should be False
|
||||
pass
|
||||
|
||||
def test_diagnostic_settings_configured_services_all_true(self):
|
||||
"""Test configured_services when all services are configured."""
|
||||
from app.config import settings
|
||||
|
||||
with patch.object(settings, "email_host", "smtp.test.com"):
|
||||
with patch.object(settings, "s3_bucket_name", "test-bucket"):
|
||||
with patch.object(settings, "dropbox_refresh_token", "token"):
|
||||
# All configured_services should be True
|
||||
pass
|
||||
|
||||
def test_diagnostic_settings_imap_enabled_imap1(self):
|
||||
"""Test imap_enabled when imap1_host is configured."""
|
||||
from app.config import settings
|
||||
|
||||
with patch.object(settings, "imap1_host", "imap.test.com"):
|
||||
with patch.object(settings, "imap2_host", None):
|
||||
# imap_enabled should be True
|
||||
pass
|
||||
|
||||
def test_diagnostic_settings_imap_enabled_imap2(self):
|
||||
"""Test imap_enabled when imap2_host is configured."""
|
||||
from app.config import settings
|
||||
|
||||
with patch.object(settings, "imap1_host", None):
|
||||
with patch.object(settings, "imap2_host", "imap2.test.com"):
|
||||
# imap_enabled should be True
|
||||
pass
|
||||
|
||||
def test_diagnostic_settings_imap_disabled(self):
|
||||
"""Test imap_enabled when no IMAP hosts configured."""
|
||||
from app.config import settings
|
||||
|
||||
with patch.object(settings, "imap1_host", None):
|
||||
with patch.object(settings, "imap2_host", None):
|
||||
# imap_enabled should be False
|
||||
pass
|
||||
|
||||
def test_diagnostic_settings_azure_requires_both_settings(self):
|
||||
"""Test Azure configured only when both key and endpoint are set."""
|
||||
from app.config import settings
|
||||
|
||||
# Only key, no endpoint
|
||||
with patch.object(settings, "azure_ai_key", "key"):
|
||||
with patch.object(settings, "azure_endpoint", None):
|
||||
# azure should be False
|
||||
pass
|
||||
|
||||
# Only endpoint, no key
|
||||
with patch.object(settings, "azure_ai_key", None):
|
||||
with patch.object(settings, "azure_endpoint", "https://test.com"):
|
||||
# azure should be False
|
||||
pass
|
||||
|
||||
# Both set
|
||||
with patch.object(settings, "azure_ai_key", "key"):
|
||||
with patch.object(settings, "azure_endpoint", "https://test.com"):
|
||||
# azure should be True
|
||||
pass
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestTestNotification:
|
||||
"""Tests for POST /diagnostic/test-notification endpoint."""
|
||||
|
||||
@@ -490,21 +490,6 @@ class TestLicenseRoutes:
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestDiagnosticSettings:
|
||||
"""GET /api/diagnostic/settings - dump settings."""
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_diagnostic_settings_success(self, client):
|
||||
"""Returns safe subset of settings."""
|
||||
with patch("app.utils.config_validator.dump_all_settings"):
|
||||
response = client.get("/api/diagnostic/settings")
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert data["status"] == "success"
|
||||
assert "settings" in data
|
||||
assert "configured_services" in data["settings"]
|
||||
|
||||
|
||||
class TestDiagnosticTestNotification:
|
||||
"""POST /api/diagnostic/test-notification - send test notification."""
|
||||
|
||||
|
||||
@@ -5,47 +5,6 @@ from unittest.mock import patch
|
||||
import pytest
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
class TestDiagnosticSettings:
|
||||
"""Tests for diagnostic settings endpoint."""
|
||||
|
||||
def test_diagnostic_settings_endpoint(self, client):
|
||||
"""Test /api/diagnostic/settings endpoint."""
|
||||
response = client.get("/api/diagnostic/settings")
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert data["status"] == "success"
|
||||
assert "settings" in data
|
||||
assert "configured_services" in data["settings"]
|
||||
|
||||
def test_diagnostic_settings_has_expected_services(self, client):
|
||||
"""Test that diagnostic settings has expected service keys."""
|
||||
response = client.get("/api/diagnostic/settings")
|
||||
data = response.json()
|
||||
services = data["settings"]["configured_services"]
|
||||
expected_keys = ["email", "s3", "dropbox", "onedrive", "nextcloud", "sftp", "openai", "azure"]
|
||||
for key in expected_keys:
|
||||
assert key in services
|
||||
|
||||
def test_diagnostic_settings_includes_workdir(self, client):
|
||||
"""Test that settings include workdir."""
|
||||
response = client.get("/api/diagnostic/settings")
|
||||
data = response.json()
|
||||
assert "workdir" in data["settings"]
|
||||
|
||||
def test_diagnostic_settings_includes_hostname(self, client):
|
||||
"""Test that settings include external hostname."""
|
||||
response = client.get("/api/diagnostic/settings")
|
||||
data = response.json()
|
||||
assert "external_hostname" in data["settings"]
|
||||
|
||||
def test_diagnostic_settings_includes_imap_status(self, client):
|
||||
"""Test that settings include IMAP enabled status."""
|
||||
response = client.get("/api/diagnostic/settings")
|
||||
data = response.json()
|
||||
assert "imap_enabled" in data["settings"]
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
class TestTestNotification:
|
||||
"""Tests for test notification endpoint."""
|
||||
@@ -120,66 +79,3 @@ class TestTestNotification:
|
||||
|
||||
# Response should have been processed
|
||||
assert response.status_code == 200
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestDiagnosticHelpers:
|
||||
"""Test helper functions in diagnostic module."""
|
||||
|
||||
@patch("app.utils.config_validator.dump_all_settings")
|
||||
@patch("app.api.diagnostic.settings")
|
||||
def test_dump_all_settings_called(self, mock_settings, mock_dump, client):
|
||||
"""Test that dump_all_settings is called."""
|
||||
mock_settings.external_hostname = "test"
|
||||
# Setup minimal mocks for configured services
|
||||
mock_settings.email_host = None
|
||||
mock_settings.s3_bucket_name = None
|
||||
mock_settings.dropbox_refresh_token = None
|
||||
mock_settings.onedrive_refresh_token = None
|
||||
mock_settings.nextcloud_upload_url = None
|
||||
mock_settings.sftp_host = None
|
||||
mock_settings.paperless_host = None
|
||||
mock_settings.google_drive_credentials_json = None
|
||||
mock_settings.uptime_kuma_url = None
|
||||
mock_settings.authentik_config_url = None
|
||||
mock_settings.openai_api_key = None
|
||||
mock_settings.azure_api_key = None
|
||||
mock_settings.azure_endpoint = None
|
||||
mock_settings.imap1_host = None
|
||||
mock_settings.imap2_host = None
|
||||
|
||||
response = client.get("/api/diagnostic/settings")
|
||||
|
||||
# dump_all_settings should have been called
|
||||
mock_dump.assert_called_once()
|
||||
|
||||
@patch("app.api.diagnostic.settings")
|
||||
def test_safe_settings_no_sensitive_data(self, mock_settings, client):
|
||||
"""Test that safe settings don't include sensitive data."""
|
||||
mock_settings.workdir = "/tmp/workdir"
|
||||
mock_settings.external_hostname = "test-host"
|
||||
mock_settings.openai_api_key = "sk-secret-key-12345"
|
||||
mock_settings.aws_secret_access_key = "secret-aws-key"
|
||||
# Setup minimal configured services
|
||||
mock_settings.email_host = None
|
||||
mock_settings.s3_bucket_name = None
|
||||
mock_settings.dropbox_refresh_token = None
|
||||
mock_settings.onedrive_refresh_token = None
|
||||
mock_settings.nextcloud_upload_url = None
|
||||
mock_settings.sftp_host = None
|
||||
mock_settings.paperless_host = None
|
||||
mock_settings.google_drive_credentials_json = None
|
||||
mock_settings.uptime_kuma_url = None
|
||||
mock_settings.authentik_config_url = None
|
||||
mock_settings.azure_api_key = None
|
||||
mock_settings.azure_endpoint = None
|
||||
mock_settings.imap1_host = None
|
||||
mock_settings.imap2_host = None
|
||||
|
||||
response = client.get("/api/diagnostic/settings")
|
||||
data = response.json()
|
||||
|
||||
# Sensitive keys should not be in response
|
||||
response_str = str(data)
|
||||
assert "sk-secret-key" not in response_str
|
||||
assert "secret-aws-key" not in response_str
|
||||
|
||||
@@ -92,7 +92,6 @@ class TestEndpointRegistration:
|
||||
# Test a few known API endpoints to ensure the /api prefix works
|
||||
endpoints_to_check = [
|
||||
("/api/process-url", "post"),
|
||||
("/api/diagnostic/settings", "get"),
|
||||
]
|
||||
|
||||
for endpoint, method in endpoints_to_check:
|
||||
|
||||
@@ -14,11 +14,6 @@ class TestStatusViews:
|
||||
response = client.get("/status")
|
||||
assert response.status_code == 200
|
||||
|
||||
def test_env_debug_page(self, client):
|
||||
"""Test env debug page."""
|
||||
response = client.get("/env")
|
||||
assert response.status_code == 200
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestStatusDashboard:
|
||||
@@ -149,89 +144,6 @@ class TestStatusDashboard:
|
||||
assert context["settings"]["notification_urls"] == ["https://webhook.example.com/notify"]
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestEnvDebug:
|
||||
"""Tests for env_debug function."""
|
||||
|
||||
@patch("app.views.status.get_settings_for_display")
|
||||
@patch("app.views.status.templates")
|
||||
@patch("app.views.status.settings")
|
||||
@pytest.mark.asyncio
|
||||
async def test_env_debug_returns_template(self, mock_settings, mock_templates, mock_get_settings):
|
||||
"""Test env debug returns template response."""
|
||||
from app.views.status import env_debug
|
||||
|
||||
mock_settings.debug = False
|
||||
mock_settings.version = "1.0.0"
|
||||
mock_get_settings.return_value = {"workdir": {"value": "/app/workdir"}}
|
||||
|
||||
mock_request = Mock()
|
||||
|
||||
result = await env_debug(mock_request)
|
||||
|
||||
mock_templates.TemplateResponse.assert_called_once()
|
||||
call_args = mock_templates.TemplateResponse.call_args
|
||||
assert call_args[0][0] == "env_debug.html"
|
||||
|
||||
@patch("app.views.status.get_settings_for_display")
|
||||
@patch("app.views.status.templates")
|
||||
@patch("app.views.status.settings")
|
||||
@pytest.mark.asyncio
|
||||
async def test_env_debug_respects_debug_setting(self, mock_settings, mock_templates, mock_get_settings):
|
||||
"""Test env debug respects debug setting."""
|
||||
from app.views.status import env_debug
|
||||
|
||||
mock_settings.debug = True
|
||||
mock_settings.version = "1.0.0"
|
||||
mock_get_settings.return_value = {}
|
||||
|
||||
mock_request = Mock()
|
||||
|
||||
await env_debug(mock_request)
|
||||
|
||||
# Should call with show_values=True when debug is enabled
|
||||
mock_get_settings.assert_called_once_with(show_values=True)
|
||||
|
||||
@patch("app.views.status.get_settings_for_display")
|
||||
@patch("app.views.status.templates")
|
||||
@patch("app.views.status.settings")
|
||||
@pytest.mark.asyncio
|
||||
async def test_env_debug_hides_values_when_debug_disabled(self, mock_settings, mock_templates, mock_get_settings):
|
||||
"""Test env debug hides values when debug is disabled."""
|
||||
from app.views.status import env_debug
|
||||
|
||||
mock_settings.debug = False
|
||||
mock_settings.version = "1.0.0"
|
||||
mock_get_settings.return_value = {}
|
||||
|
||||
mock_request = Mock()
|
||||
|
||||
await env_debug(mock_request)
|
||||
|
||||
# Should call with show_values=False when debug is disabled
|
||||
mock_get_settings.assert_called_once_with(show_values=False)
|
||||
|
||||
@patch("app.views.status.get_settings_for_display")
|
||||
@patch("app.views.status.templates")
|
||||
@patch("app.views.status.settings")
|
||||
@pytest.mark.asyncio
|
||||
async def test_env_debug_includes_app_version(self, mock_settings, mock_templates, mock_get_settings):
|
||||
"""Test env debug includes app version."""
|
||||
from app.views.status import env_debug
|
||||
|
||||
mock_settings.debug = False
|
||||
mock_settings.version = "1.2.3"
|
||||
mock_get_settings.return_value = {}
|
||||
|
||||
mock_request = Mock()
|
||||
|
||||
await env_debug(mock_request)
|
||||
|
||||
call_args = mock_templates.TemplateResponse.call_args
|
||||
context = call_args[0][1]
|
||||
assert context["app_version"] == "1.2.3"
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestContainerInfoDetection:
|
||||
"""Tests for container information detection logic."""
|
||||
@@ -386,9 +298,3 @@ class TestStatusEndpointsRequireAuth:
|
||||
# Should return 200 or redirect to login
|
||||
response = client.get("/status", follow_redirects=False)
|
||||
assert response.status_code in [200, 302, 401]
|
||||
|
||||
def test_env_debug_requires_login(self, client):
|
||||
"""Test env debug requires authentication."""
|
||||
# Should return 200 or redirect to login
|
||||
response = client.get("/env", follow_redirects=False)
|
||||
assert response.status_code in [200, 302, 401]
|
||||
|
||||
Reference in New Issue
Block a user