diff --git a/app/api/diagnostic.py b/app/api/diagnostic.py index 0ccd1fe4..c9bccf0f 100644 --- a/app/api/diagnostic.py +++ b/app/api/diagnostic.py @@ -3,11 +3,10 @@ Diagnostic API endpoints """ import logging -from typing import Annotated -from fastapi import APIRouter, Depends, Request +from fastapi import APIRouter, Request -from app.auth import get_current_user, require_login +from app.auth import require_login from app.config import settings # Set up logging @@ -15,48 +14,6 @@ logger = logging.getLogger(__name__) router = APIRouter() -CurrentUser = Annotated[dict, Depends(get_current_user)] - - -@router.get("/diagnostic/settings") -@require_login -async def diagnostic_settings(request: Request, current_user: CurrentUser): - """ - API endpoint to dump settings to the log and view basic config information - This endpoint doesn't expose sensitive information like passwords or tokens - """ - from app.utils.config_validator import dump_all_settings - - # Dump full settings to log for admin to see - dump_all_settings() - - # Return safe subset of settings for API response - safe_settings = { - "workdir": settings.workdir, - "external_hostname": settings.external_hostname, - "configured_services": { - "email": bool(getattr(settings, "email_host", None)), - "s3": bool(getattr(settings, "s3_bucket_name", None)), - "dropbox": bool(getattr(settings, "dropbox_refresh_token", None)), - "onedrive": bool(getattr(settings, "onedrive_refresh_token", None)), - "nextcloud": bool(getattr(settings, "nextcloud_upload_url", None)), - "sftp": bool(getattr(settings, "sftp_host", None)), - "paperless": bool(getattr(settings, "paperless_host", None)), - "google_drive": bool(getattr(settings, "google_drive_credentials_json", None)), - "uptime_kuma": bool(getattr(settings, "uptime_kuma_url", None)), - "auth": bool(getattr(settings, "authentik_config_url", None)), - "openai": bool(getattr(settings, "openai_api_key", None)), - "azure": bool(getattr(settings, "azure_api_key", None) and getattr(settings, "azure_endpoint", None)), - }, - "imap_enabled": bool(getattr(settings, "imap1_host", None) or getattr(settings, "imap2_host", None)), - } - - return { - "status": "success", - "settings": safe_settings, - "message": "Full settings have been dumped to application logs", - } - @router.post("/diagnostic/test-notification") @require_login diff --git a/app/views/status.py b/app/views/status.py index 9eed0b38..8c44c993 100644 --- a/app/views/status.py +++ b/app/views/status.py @@ -8,7 +8,7 @@ from datetime import datetime from fastapi import Request -from app.utils.config_validator import get_provider_status, get_settings_for_display +from app.utils.config_validator import get_provider_status from app.views.base import APIRouter, require_login, settings, templates logger = logging.getLogger(__name__) @@ -86,27 +86,3 @@ async def status_dashboard(request: Request): "settings": {"notification_urls": notification_urls}, }, ) - - -@router.get("/env") -@require_login -async def env_debug(request: Request): - """ - Debug endpoint to view environment variables and settings - Uses actual debug setting from config - """ - # Use the actual debug setting from configuration - debug_enabled = settings.debug - - # Get settings data - settings_data = get_settings_for_display(show_values=debug_enabled) - - return templates.TemplateResponse( - "env_debug.html", - { - "request": request, - "settings": settings_data, - "debug_enabled": debug_enabled, - "app_version": settings.version, - }, - ) diff --git a/docs/RateLimitingStrategy.md b/docs/RateLimitingStrategy.md index b2a3d540..5ee700d2 100644 --- a/docs/RateLimitingStrategy.md +++ b/docs/RateLimitingStrategy.md @@ -60,7 +60,6 @@ All API endpoints are protected with a default rate limit unless explicitly exem - `GET /api/files/{file_id}/metadata` - Get file metadata - `GET /api/files/{file_id}/preview` - Get file preview - `GET /api/files/{file_id}/download` - Download file -- `GET /api/diagnostic/settings` - Get settings - `GET /api/logs` - Get logs **Rationale**: Read-only operations are less resource-intensive but still need protection against scraping and excessive polling. The default limit of 100 requests per minute allows legitimate applications while preventing abuse. diff --git a/frontend/templates/base.html b/frontend/templates/base.html index 34f1b074..46a67ef8 100644 --- a/frontend/templates/base.html +++ b/frontend/templates/base.html @@ -79,9 +79,6 @@ Credentials - - Environment - File Manager @@ -142,9 +139,6 @@ Credentials - - Environment - File Manager diff --git a/frontend/templates/env_debug.html b/frontend/templates/env_debug.html deleted file mode 100644 index 3a035cc8..00000000 --- a/frontend/templates/env_debug.html +++ /dev/null @@ -1,80 +0,0 @@ -{% extends "base.html" %} -{% block title %}Environment Configuration{% endblock %} - -{% block content %} -
-
-

Environment Configuration

-

- This page displays the current configuration settings for the application. For security reasons, - sensitive values like passwords, tokens, and keys may be hidden. -

- -
- - {% for category, items in settings.items() %} -
-

{{ category }} Configuration

-
- - - - - - - - - - {% for item in items %} - - - - - - {% endfor %} - -
SettingValueStatus
- {{ item.name }} - - {% if item.value is none %} - NULL - {% elif item.value == "" %} - (empty string) - {% elif item.value == "********" %} - ******** - {% else %} - {{ item.value }} - {% endif %} - - {% if item.is_configured %} - - Configured - - {% else %} - - Not Configured - - {% endif %} -
-
-
- {% endfor %} - -
-

Environment Variables

-

- Configuration is loaded from environment variables or .env files. - Make sure your environment variables are correctly set. -

- - -
-
-{% endblock %} diff --git a/frontend/templates/status_dashboard.html b/frontend/templates/status_dashboard.html index 1e4e95d2..086fecae 100644 --- a/frontend/templates/status_dashboard.html +++ b/frontend/templates/status_dashboard.html @@ -221,11 +221,11 @@

Configuration Settings

- For more detailed configuration settings and environment variables, check the environment debug page. + For more detailed configuration settings and environment variables, check the settings page.

- + View Detailed Configuration
diff --git a/tests/test_api_diagnostic_extended.py b/tests/test_api_diagnostic_extended.py index 2721fb0f..f2afbff3 100644 --- a/tests/test_api_diagnostic_extended.py +++ b/tests/test_api_diagnostic_extended.py @@ -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.""" diff --git a/tests/test_coverage_config_settings.py b/tests/test_coverage_config_settings.py index f6e8c143..d36bed39 100644 --- a/tests/test_coverage_config_settings.py +++ b/tests/test_coverage_config_settings.py @@ -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.""" diff --git a/tests/test_diagnostic.py b/tests/test_diagnostic.py index 8300f064..c34bbf4b 100644 --- a/tests/test_diagnostic.py +++ b/tests/test_diagnostic.py @@ -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 diff --git a/tests/test_endpoint_registration.py b/tests/test_endpoint_registration.py index 3c0476b1..5d6719eb 100644 --- a/tests/test_endpoint_registration.py +++ b/tests/test_endpoint_registration.py @@ -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: diff --git a/tests/test_views_status.py b/tests/test_views_status.py index e1e617b8..2fe19b71 100644 --- a/tests/test_views_status.py +++ b/tests/test_views_status.py @@ -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]