Merge pull request #553 from christianlouis/copilot/restrict-status-page-access
feat(status): restrict status page to admin-only, move to admin menu
This commit is contained in:
+16
-2
@@ -6,7 +6,8 @@ import logging
|
||||
import os
|
||||
from datetime import datetime
|
||||
|
||||
from fastapi import Request
|
||||
from fastapi import Request, status
|
||||
from fastapi.responses import RedirectResponse
|
||||
|
||||
from app.utils.config_validator import get_provider_status
|
||||
from app.views.base import APIRouter, require_login, settings, templates
|
||||
@@ -15,12 +16,25 @@ logger = logging.getLogger(__name__)
|
||||
router = APIRouter()
|
||||
|
||||
|
||||
def _require_admin(request: Request):
|
||||
"""Return the session user if they are an admin, else return None."""
|
||||
user = request.session.get("user")
|
||||
if not user or not user.get("is_admin"):
|
||||
user_email = user.get("email", "anonymous") if user else "anonymous"
|
||||
logger.warning(f"Non-admin user {user_email} attempted to access /status")
|
||||
return None
|
||||
return user
|
||||
|
||||
|
||||
@router.get("/status")
|
||||
@require_login
|
||||
async def status_dashboard(request: Request):
|
||||
"""
|
||||
Status dashboard showing all configured integration targets
|
||||
Status dashboard showing all configured integration targets (admin only).
|
||||
"""
|
||||
user = _require_admin(request)
|
||||
if user is None:
|
||||
return RedirectResponse(url="/", status_code=status.HTTP_302_FOUND)
|
||||
# Get provider status
|
||||
providers = get_provider_status()
|
||||
|
||||
|
||||
@@ -168,18 +168,14 @@
|
||||
<a href="/admin/backup" role="menuitem" class="flex items-center px-4 py-2 text-sm text-gray-700 hover:bg-gray-100">
|
||||
<i class="fas fa-database w-4 mr-2 text-green-600" aria-hidden="true"></i> Backup & Restore
|
||||
</a>
|
||||
<a href="/status" role="menuitem" class="flex items-center px-4 py-2 text-sm text-gray-700 hover:bg-gray-100"
|
||||
{% if request and request.url.path == '/status' %}aria-current="page"{% endif %}>
|
||||
<i class="fas fa-circle-dot w-4 mr-2 text-gray-500" aria-hidden="true"></i> Status
|
||||
</a>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<!-- Status (de-emphasised, shown after Admin) -->
|
||||
<a href="/status"
|
||||
class="px-2 py-2 rounded-md text-xs font-medium text-gray-400 hover:text-gray-600 hover:bg-gray-100"
|
||||
title="System Status"
|
||||
{% if request and request.url.path == '/status' %}aria-current="page"{% endif %}>
|
||||
<i class="fas fa-circle-dot mr-0.5" aria-hidden="true"></i> Status
|
||||
</a>
|
||||
|
||||
{% endif %}{# end multi_user_enabled / is_logged_in check #}
|
||||
|
||||
<!-- Help – always visible, for every visitor regardless of auth state -->
|
||||
@@ -314,16 +310,13 @@
|
||||
<a href="/admin/backup" class="block px-3 py-3 rounded-md text-base font-medium text-gray-700 hover:text-gray-900 hover:bg-gray-50">
|
||||
<i class="fas fa-database mr-2 text-green-500" aria-hidden="true"></i> Backup & Restore
|
||||
</a>
|
||||
<a href="/status" class="block px-3 py-3 rounded-md text-base font-medium text-gray-700 hover:text-gray-900 hover:bg-gray-50"
|
||||
{% if request and request.url.path == '/status' %}aria-current="page"{% endif %}>
|
||||
<i class="fas fa-circle-dot mr-2 text-gray-400" aria-hidden="true"></i> Status
|
||||
</a>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<!-- Status (de-emphasised) -->
|
||||
<a href="/status"
|
||||
class="block px-3 py-3 rounded-md text-sm font-medium text-gray-400 hover:text-gray-600 hover:bg-gray-50"
|
||||
{% if request and request.url.path == '/status' %}aria-current="page"{% endif %}>
|
||||
<i class="fas fa-circle-dot mr-1" aria-hidden="true"></i> Status
|
||||
</a>
|
||||
|
||||
{% endif %}{# end multi_user_enabled / is_logged_in check #}
|
||||
|
||||
<!-- Help – always visible, for every visitor regardless of auth state -->
|
||||
|
||||
+117
-2
@@ -1,16 +1,74 @@
|
||||
"""Tests for app/views/status.py module."""
|
||||
|
||||
from unittest.mock import Mock, mock_open, patch
|
||||
from unittest.mock import MagicMock, Mock, mock_open, patch
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestRequireAdmin:
|
||||
"""Unit tests for the _require_admin helper in views/status.py."""
|
||||
|
||||
def test_returns_none_when_no_user_in_session(self):
|
||||
"""_require_admin returns None when session has no user."""
|
||||
from app.views.status import _require_admin
|
||||
|
||||
mock_request = MagicMock()
|
||||
mock_request.session = {}
|
||||
|
||||
result = _require_admin(mock_request)
|
||||
assert result is None
|
||||
|
||||
def test_returns_none_for_non_admin_user(self):
|
||||
"""_require_admin returns None when user is not an admin."""
|
||||
from app.views.status import _require_admin
|
||||
|
||||
mock_request = MagicMock()
|
||||
mock_request.session = {"user": {"email": "user@example.com", "is_admin": False}}
|
||||
|
||||
result = _require_admin(mock_request)
|
||||
assert result is None
|
||||
|
||||
def test_logs_warning_for_non_admin(self):
|
||||
"""_require_admin logs a warning when a non-admin attempts access."""
|
||||
from app.views.status import _require_admin
|
||||
|
||||
mock_request = MagicMock()
|
||||
mock_request.session = {"user": {"email": "user@example.com", "is_admin": False}}
|
||||
|
||||
with patch("app.views.status.logger") as mock_logger:
|
||||
_require_admin(mock_request)
|
||||
mock_logger.warning.assert_called_once()
|
||||
|
||||
def test_logs_warning_when_no_user(self):
|
||||
"""_require_admin logs a warning when there is no user in the session."""
|
||||
from app.views.status import _require_admin
|
||||
|
||||
mock_request = MagicMock()
|
||||
mock_request.session = {}
|
||||
|
||||
with patch("app.views.status.logger") as mock_logger:
|
||||
_require_admin(mock_request)
|
||||
mock_logger.warning.assert_called_once()
|
||||
|
||||
def test_returns_user_for_admin(self):
|
||||
"""_require_admin returns the user dict when the user is an admin."""
|
||||
from app.views.status import _require_admin
|
||||
|
||||
admin_user = {"email": "admin@example.com", "is_admin": True}
|
||||
mock_request = MagicMock()
|
||||
mock_request.session = {"user": admin_user}
|
||||
|
||||
result = _require_admin(mock_request)
|
||||
assert result == admin_user
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
class TestStatusViews:
|
||||
"""Tests for status view routes."""
|
||||
|
||||
def test_status_dashboard(self, client):
|
||||
"""Test status dashboard page."""
|
||||
"""Test status dashboard page returns 200 (auth disabled in tests)."""
|
||||
response = client.get("/status")
|
||||
assert response.status_code == 200
|
||||
|
||||
@@ -19,6 +77,63 @@ class TestStatusViews:
|
||||
class TestStatusDashboard:
|
||||
"""Tests for status_dashboard function."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_redirects_non_admin_to_home(self):
|
||||
"""status_dashboard redirects to '/' when user is not an admin."""
|
||||
from fastapi.responses import RedirectResponse
|
||||
|
||||
from app.views.status import status_dashboard
|
||||
|
||||
mock_request = MagicMock()
|
||||
mock_request.session = {"user": {"email": "user@example.com", "is_admin": False}}
|
||||
|
||||
result = await status_dashboard(mock_request)
|
||||
|
||||
assert isinstance(result, RedirectResponse)
|
||||
assert result.status_code == 302
|
||||
assert result.headers["location"] == "/"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_redirects_when_no_user_in_session(self):
|
||||
"""status_dashboard redirects to '/' when no user is in the session."""
|
||||
from fastapi.responses import RedirectResponse
|
||||
|
||||
from app.views.status import status_dashboard
|
||||
|
||||
mock_request = MagicMock()
|
||||
mock_request.session = {}
|
||||
|
||||
result = await status_dashboard(mock_request)
|
||||
|
||||
assert isinstance(result, RedirectResponse)
|
||||
assert result.status_code == 302
|
||||
|
||||
@patch("app.views.status.get_provider_status")
|
||||
@patch("app.views.status.templates")
|
||||
@patch("app.views.status.settings")
|
||||
@patch("app.views.status.os.path.exists")
|
||||
@pytest.mark.asyncio
|
||||
async def test_returns_template_for_admin_user(self, mock_exists, mock_settings, mock_templates, mock_providers):
|
||||
"""status_dashboard renders the template for an authenticated admin user."""
|
||||
from app.views.status import status_dashboard
|
||||
|
||||
mock_exists.return_value = False
|
||||
mock_providers.return_value = {}
|
||||
mock_settings.version = "1.0.0"
|
||||
mock_settings.build_date = "2024-01-01"
|
||||
mock_settings.debug = False
|
||||
mock_settings.git_sha = "abc123"
|
||||
mock_settings.notification_urls = []
|
||||
|
||||
mock_request = MagicMock()
|
||||
mock_request.session = {"user": {"email": "admin@example.com", "is_admin": True}}
|
||||
|
||||
await status_dashboard(mock_request)
|
||||
|
||||
mock_templates.TemplateResponse.assert_called_once()
|
||||
call_args = mock_templates.TemplateResponse.call_args
|
||||
assert call_args[0][0] == "status_dashboard.html"
|
||||
|
||||
@patch("app.views.status.get_provider_status")
|
||||
@patch("app.views.status.templates")
|
||||
@patch("app.views.status.settings")
|
||||
|
||||
Reference in New Issue
Block a user