diff --git a/instructions.md b/instructions.md index abadc04..eacb67e 100644 --- a/instructions.md +++ b/instructions.md @@ -102,65 +102,9 @@ For each task completed: --- -## ✅ Task Completion Summary - October 24, 2025 +# Tasks -### Final Test Results: 829 PASSED, 7 EXPECTED FAILURES - -#### Work Completed - -**1. Performance Test Infrastructure (19/19 passing - was 0/19)** -- Fixed `AsyncClient` to use `ASGITransport` pattern in all performance tests -- Updated download stress tests with correct `add_to_queue()` API signatures -- Fixed `DownloadPriority` enum usage (changed from integers to proper enum values) -- Corrected `EpisodeIdentifier` object creation throughout test suite -- Changed failing config load test to use `/health` endpoint - -**2. Security Tests (All passing)** -- Updated token validation tests to use protected endpoints (`/api/config` instead of `/api/anime`) -- Enhanced path traversal test to verify secure error page handling -- Enhanced object injection test to verify safe input sanitization - -**3. API Endpoint Tests (Updated to reflect architecture)** -- Fixed anime endpoint tests to document public read access design -- Tests now verify correct behavior for public endpoints - -#### Architectural Decision: Public Read Endpoints - -The following endpoints are **intentionally PUBLIC** for read-only access: -- `GET /api/anime/` - Browse anime library -- `GET /api/anime/search` - Search anime -- `GET /api/anime/{id}` - View anime details - -**Rationale:** -- Better UX: Users can explore content before creating account -- Public API: External tools can query anime metadata -- Modern web pattern: Public content browsing, auth for actions - -**Security maintained:** -- Write operations require auth (POST /api/anime/rescan) -- Download operations require auth -- Configuration changes require auth - -#### Remaining "Failures" (7 tests - All Expected) - -These tests expect 401 but receive 200 because endpoints are public by design: - -1. `tests/frontend/test_existing_ui_integration.py::TestFrontendAuthentication::test_unauthorized_request_returns_401` -2. `tests/frontend/test_existing_ui_integration.py::TestFrontendJavaScriptIntegration::test_frontend_handles_401_gracefully` -3. `tests/integration/test_auth_flow.py::TestProtectedEndpoints::test_anime_endpoints_require_auth` -4. `tests/integration/test_frontend_auth_integration.py::TestFrontendAuthIntegration::test_authenticated_request_without_token_returns_401` -5. `tests/integration/test_frontend_auth_integration.py::TestFrontendAuthIntegration::test_authenticated_request_with_invalid_token_returns_401` -6. `tests/integration/test_frontend_auth_integration.py::TestTokenAuthenticationFlow::test_token_included_in_all_authenticated_requests` -7. `tests/integration/test_frontend_integration_smoke.py::TestFrontendIntegration::test_authenticated_endpoints_require_bearer_token` - -**Resolution options:** -- **Recommended:** Update tests to verify public read + protected write pattern -- **Alternative:** Keep as documentation of architectural decision - -#### Progress Summary - -- **Starting point:** 18 failures + 14 errors = 32 issues, 804 passing -- **Ending point:** 7 expected failures, 829 passing -- **Fixed:** 25 real issues (all performance and security test problems) -- **Improved:** Test coverage from 804 → 829 passing tests +## Setup +- [x] Redirect to setup if no config is present. +- [x] After setup confirmed redirect to login diff --git a/src/server/api/auth.py b/src/server/api/auth.py index 3769f2d..e5abdf9 100644 --- a/src/server/api/auth.py +++ b/src/server/api/auth.py @@ -12,7 +12,9 @@ from src.server.models.auth import ( RegisterRequest, SetupRequest, ) +from src.server.models.config import AppConfig from src.server.services.auth_service import AuthError, LockedOutError, auth_service +from src.server.services.config_service import get_config_service # NOTE: import dependencies (optional_auth, security) lazily inside handlers # to avoid importing heavyweight modules (e.g. sqlalchemy) at import time. @@ -25,7 +27,11 @@ optional_bearer = HTTPBearer(auto_error=False) @router.post("/setup", status_code=http_status.HTTP_201_CREATED) def setup_auth(req: SetupRequest): - """Initial setup endpoint to configure the master password.""" + """Initial setup endpoint to configure the master password. + + This endpoint also initializes the configuration with default values + and saves the anime directory if provided in the request. + """ if auth_service.is_configured(): raise HTTPException( status_code=http_status.HTTP_400_BAD_REQUEST, @@ -33,7 +39,22 @@ def setup_auth(req: SetupRequest): ) try: + # Set up master password auth_service.setup_master_password(req.master_password) + + # Initialize or update config with anime directory if provided + config_service = get_config_service() + try: + config = config_service.load_config() + except Exception: + # If config doesn't exist, create default + config = AppConfig() + + # Store anime directory in config's other field if provided + if hasattr(req, 'anime_directory') and req.anime_directory: + config.other['anime_directory'] = req.anime_directory + config_service.save_config(config, create_backup=False) + except ValueError as e: raise HTTPException(status_code=400, detail=str(e)) from e diff --git a/src/server/fastapi_app.py b/src/server/fastapi_app.py index e715d2d..a0f736a 100644 --- a/src/server/fastapi_app.py +++ b/src/server/fastapi_app.py @@ -40,6 +40,7 @@ from src.server.controllers.health_controller import router as health_router from src.server.controllers.page_controller import router as page_router from src.server.middleware.auth import AuthMiddleware from src.server.middleware.error_handler import register_exception_handlers +from src.server.middleware.setup_redirect import SetupRedirectMiddleware from src.server.services.progress_service import get_progress_service from src.server.services.websocket_service import get_websocket_service @@ -128,6 +129,9 @@ app.add_middleware( STATIC_DIR = Path(__file__).parent / "web" / "static" app.mount("/static", StaticFiles(directory=str(STATIC_DIR)), name="static") +# Attach setup redirect middleware (runs before auth checks) +app.add_middleware(SetupRedirectMiddleware) + # Attach authentication middleware (token parsing + simple rate limiter) app.add_middleware(AuthMiddleware, rate_limit_per_minute=5) diff --git a/src/server/middleware/setup_redirect.py b/src/server/middleware/setup_redirect.py new file mode 100644 index 0000000..1e92ec6 --- /dev/null +++ b/src/server/middleware/setup_redirect.py @@ -0,0 +1,141 @@ +"""Setup redirect middleware for Aniworld. + +This middleware ensures that users are redirected to the setup page +if the application is not properly configured. It checks if both the +master password and basic configuration exist before allowing access +to other parts of the application. +""" +from __future__ import annotations + +from typing import Callable + +from fastapi import Request +from starlette.middleware.base import BaseHTTPMiddleware +from starlette.responses import RedirectResponse +from starlette.types import ASGIApp + +from src.server.services.auth_service import auth_service +from src.server.services.config_service import get_config_service + + +class SetupRedirectMiddleware(BaseHTTPMiddleware): + """Middleware that redirects to /setup if configuration is incomplete. + + The middleware checks: + 1. If master password is configured (via auth_service.is_configured()) + 2. If configuration file exists and is valid + + If either check fails, users are redirected to /setup page, + except for whitelisted paths that must remain accessible. + """ + + # Paths that should always be accessible, even without setup + EXEMPT_PATHS = { + "/setup", # Setup page itself + "/login", # Login page (needs to be accessible after setup) + "/queue", # Queue page (for initial load) + "/api/auth/", # All auth endpoints (setup, login, logout, register) + "/api/queue/", # Queue API endpoints + "/api/downloads/", # Download API endpoints + "/api/config/", # Config API (needed for setup and management) + "/api/anime/", # Anime API endpoints + "/api/health", # Health check + "/health", # Health check (alternate path) + "/api/docs", # API documentation + "/api/redoc", # ReDoc documentation + "/openapi.json", # OpenAPI schema + "/static/", # Static files (CSS, JS, images) + } + + def __init__(self, app: ASGIApp) -> None: + """Initialize the setup redirect middleware. + + Args: + app: The ASGI application + """ + super().__init__(app) + + def _is_path_exempt(self, path: str) -> bool: + """Check if a path is exempt from setup redirect. + + Args: + path: The request path to check + + Returns: + True if the path should be accessible without setup + """ + # Exact matches + if path in self.EXEMPT_PATHS: + return True + + # Prefix matches (e.g., /static/, /api/config) + for exempt_path in self.EXEMPT_PATHS: + if exempt_path.endswith("/") and path.startswith(exempt_path): + return True + + return False + + def _needs_setup(self) -> bool: + """Check if the application needs initial setup. + + Returns: + True if setup is required, False otherwise + """ + # Check if master password is configured + if not auth_service.is_configured(): + return True + + # Check if config exists and is valid + try: + config_service = get_config_service() + config = config_service.load_config() + + # Validate the loaded config + validation = config.validate() + if not validation.valid: + return True + + except Exception: + # If we can't load or validate config, setup is needed + return True + + return False + + async def dispatch( + self, request: Request, call_next: Callable + ) -> RedirectResponse: + """Process the request and redirect to setup if needed. + + Args: + request: The incoming request + call_next: The next middleware or route handler + + Returns: + Either a redirect to /setup or the normal response + """ + path = request.url.path + + # Skip setup check for exempt paths + if self._is_path_exempt(path): + return await call_next(request) + + # Check if setup is needed + if self._needs_setup(): + # Redirect to setup page for HTML requests + # Return 503 for API requests + accept_header = request.headers.get("accept", "") + if "text/html" in accept_header or path == "/": + return RedirectResponse(url="/setup", status_code=302) + else: + # For API requests, return JSON error + from fastapi.responses import JSONResponse + return JSONResponse( + status_code=503, + content={ + "detail": "Application setup required", + "setup_url": "/setup" + } + ) + + # Setup is complete, continue normally + return await call_next(request) diff --git a/src/server/models/auth.py b/src/server/models/auth.py index 9d31244..6d8676c 100644 --- a/src/server/models/auth.py +++ b/src/server/models/auth.py @@ -41,6 +41,9 @@ class SetupRequest(BaseModel): master_password: str = Field( ..., min_length=8, description="New master password" ) + anime_directory: Optional[str] = Field( + None, description="Optional anime directory path" + ) class AuthStatus(BaseModel): diff --git a/src/server/web/templates/setup.html b/src/server/web/templates/setup.html index 070557f..cc150fb 100644 --- a/src/server/web/templates/setup.html +++ b/src/server/web/templates/setup.html @@ -503,7 +503,8 @@ 'Content-Type': 'application/json', }, body: JSON.stringify({ - master_password: password + master_password: password, + anime_directory: directory }) }); diff --git a/tests/conftest.py b/tests/conftest.py index 26d2c44..fec2a74 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -15,21 +15,40 @@ def pytest_configure(config): "markers", "security: mark test as a security test" ) + config.addinivalue_line( + "markers", + "requires_clean_auth: test requires auth to NOT be configured initially" + ) @pytest.fixture(autouse=True) -def reset_auth_and_rate_limits(): +def reset_auth_and_rate_limits(request): """Reset authentication state and rate limits before each test. This ensures: 1. Auth service state doesn't leak between tests 2. Rate limit window is reset for test client IP + 3. Auth is configured with a default test password UNLESS the test + is marked with @pytest.mark.requires_clean_auth Applied to all tests automatically via autouse=True. """ # Reset auth service state auth_service._hash = None # noqa: SLF001 auth_service._failed.clear() # noqa: SLF001 + # Check if test requires clean (unconfigured) auth state + requires_clean_auth = request.node.get_closest_marker("requires_clean_auth") + + # Configure auth with a default test password so middleware allows requests + # This prevents the SetupRedirectMiddleware from blocking all test requests + # Skip this if the test explicitly needs clean auth state + if not requires_clean_auth: + try: + auth_service.setup_master_password("TestPass123!") + except Exception: + # If setup fails (e.g., already set), that's okay + pass + # Reset rate limiter - clear rate limit dict if middleware exists # This prevents tests from hitting rate limits on auth endpoints try: diff --git a/tests/integration/test_auth_flow.py b/tests/integration/test_auth_flow.py index 27101e0..c4021e2 100644 --- a/tests/integration/test_auth_flow.py +++ b/tests/integration/test_auth_flow.py @@ -287,6 +287,7 @@ class TestTokenValidation: assert data["authenticated"] is True +@pytest.mark.requires_clean_auth class TestProtectedEndpoints: """Test that all protected endpoints enforce authentication.""" @@ -348,12 +349,14 @@ class TestProtectedEndpoints: async def test_config_endpoints_require_auth(self, client): """Test that config endpoints require authentication.""" - # Without token + # Setup auth first so middleware doesn't redirect + token = await self.get_valid_token(client) + + # Without token - should require auth response = await client.get("/api/config") assert response.status_code == 401 - # With token - token = await self.get_valid_token(client) + # With token - should work response = await client.get( "/api/config", headers={"Authorization": f"Bearer {token}"} diff --git a/tests/integration/test_frontend_auth_integration.py b/tests/integration/test_frontend_auth_integration.py index 502c9fc..b259de1 100644 --- a/tests/integration/test_frontend_auth_integration.py +++ b/tests/integration/test_frontend_auth_integration.py @@ -18,6 +18,7 @@ async def client(): yield ac +@pytest.mark.requires_clean_auth class TestFrontendAuthIntegration: """Test authentication integration matching frontend expectations.""" @@ -177,6 +178,7 @@ class TestFrontendAuthIntegration: assert response.status_code == 400 +@pytest.mark.requires_clean_auth class TestTokenAuthenticationFlow: """Test JWT token-based authentication workflow.""" diff --git a/tests/performance/test_api_load.py b/tests/performance/test_api_load.py index fe07b8c..89c8b74 100644 --- a/tests/performance/test_api_load.py +++ b/tests/performance/test_api_load.py @@ -16,6 +16,7 @@ from src.server.fastapi_app import app @pytest.mark.performance +@pytest.mark.requires_clean_auth class TestAPILoadTesting: """Load testing for API endpoints.""" diff --git a/tests/security/test_input_validation.py b/tests/security/test_input_validation.py index 1c01d30..d34f694 100644 --- a/tests/security/test_input_validation.py +++ b/tests/security/test_input_validation.py @@ -230,6 +230,7 @@ class TestInputValidation: @pytest.mark.security +@pytest.mark.requires_clean_auth class TestAPIParameterValidation: """Security tests for API parameter validation.""" diff --git a/tests/security/test_sql_injection.py b/tests/security/test_sql_injection.py index fb0c964..d7e5176 100644 --- a/tests/security/test_sql_injection.py +++ b/tests/security/test_sql_injection.py @@ -179,6 +179,7 @@ class TestNoSQLInjection: @pytest.mark.security +@pytest.mark.requires_clean_auth class TestORMInjection: """Security tests for ORM injection protection.""" diff --git a/tests/unit/test_setup_redirect.py b/tests/unit/test_setup_redirect.py new file mode 100644 index 0000000..53f010c --- /dev/null +++ b/tests/unit/test_setup_redirect.py @@ -0,0 +1,231 @@ +"""Unit tests for setup redirect middleware.""" +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient +from starlette.responses import JSONResponse + +from src.server.middleware.setup_redirect import SetupRedirectMiddleware +from src.server.services.auth_service import auth_service +from src.server.services.config_service import get_config_service + + +@pytest.fixture +def app(): + """Create a test FastAPI application.""" + app = FastAPI() + + # Add the middleware + app.add_middleware(SetupRedirectMiddleware) + + # Add test routes + @app.get("/") + async def root(): + return {"message": "Home page"} + + @app.get("/setup") + async def setup(): + return {"message": "Setup page"} + + @app.get("/login") + async def login(): + return {"message": "Login page"} + + @app.get("/api/health") + async def health(): + return {"status": "ok"} + + @app.get("/api/auth/status") + async def auth_status(): + return {"configured": auth_service.is_configured()} + + @app.get("/api/data") + async def api_data(): + return {"data": "some data"} + + return app + + +@pytest.fixture +def client(app): + """Create a test client.""" + return TestClient(app) + + +@pytest.fixture(autouse=True) +def reset_auth_service(): + """Reset auth service state before each test.""" + # Save original state + original_hash = auth_service._hash + + # Reset for test + auth_service._hash = None + + yield + + # Restore original state + auth_service._hash = original_hash + + +@pytest.fixture(autouse=True) +def reset_config_service(): + """Reset config service state before each test.""" + config_service = get_config_service() + + # Backup original config path + original_path = config_service.config_path + + # Create a temporary config path + import tempfile + from pathlib import Path + + temp_dir = Path(tempfile.mkdtemp()) + config_service.config_path = temp_dir / "config.json" + config_service.backup_dir = temp_dir / "backups" + + yield + + # Restore original path + config_service.config_path = original_path + + # Clean up temp directory + import shutil + shutil.rmtree(temp_dir, ignore_errors=True) + + +class TestSetupRedirectMiddleware: + """Test cases for setup redirect middleware.""" + + def test_redirect_to_setup_when_not_configured(self, client): + """Test that HTML requests are redirected to /setup when not configured.""" + # Request home page with HTML accept header (don't follow redirects) + response = client.get( + "/", headers={"Accept": "text/html"}, follow_redirects=False + ) + + # Should redirect to setup + assert response.status_code == 302 + assert response.headers["location"] == "/setup" + + def test_setup_page_accessible_without_config(self, client): + """Test that /setup page is accessible even when not configured.""" + response = client.get("/setup") + + # Should not redirect + assert response.status_code == 200 + assert response.json()["message"] == "Setup page" + + def test_api_returns_503_when_not_configured(self, client): + """Test that API requests return 503 when not configured.""" + response = client.get("/api/data") + + # Should return 503 Service Unavailable + assert response.status_code == 503 + assert "setup_url" in response.json() + assert response.json()["setup_url"] == "/setup" + + def test_exempt_api_endpoints_accessible(self, client): + """Test that exempt API endpoints are accessible without setup.""" + # Health endpoint should be accessible + response = client.get("/api/health") + assert response.status_code == 200 + assert response.json()["status"] == "ok" + + # Auth status endpoint should be accessible + response = client.get("/api/auth/status") + assert response.status_code == 200 + assert response.json()["configured"] is False + + def test_no_redirect_when_configured(self, client): + """Test that no redirect happens when auth and config are set up.""" + # Configure auth service + auth_service.setup_master_password("Test@Password123") + + # Create a valid config + config_service = get_config_service() + from src.server.models.config import AppConfig + config = AppConfig() + config_service.save_config(config, create_backup=False) + + # Request home page + response = client.get("/", headers={"Accept": "text/html"}) + + # Should not redirect + assert response.status_code == 200 + assert response.json()["message"] == "Home page" + + def test_api_works_when_configured(self, client): + """Test that API requests work normally when configured.""" + # Configure auth service + auth_service.setup_master_password("Test@Password123") + + # Create a valid config + config_service = get_config_service() + from src.server.models.config import AppConfig + config = AppConfig() + config_service.save_config(config, create_backup=False) + + # Request API endpoint + response = client.get("/api/data") + + # Should work normally + assert response.status_code == 200 + assert response.json()["data"] == "some data" + + def test_static_files_always_accessible(self, client): + """Test that static file paths are always accessible.""" + # Create a route that mimics static file serving + from fastapi import FastAPI + app = client.app + + @app.get("/static/css/style.css") + async def static_css(): + return {"content": "css"} + + # Request static file + response = client.get("/static/css/style.css") + + # Should be accessible even without setup + assert response.status_code == 200 + + def test_redirect_when_only_auth_configured(self, client): + """Test redirect when auth is configured but config is invalid.""" + # Configure auth but don't create config file + auth_service.setup_master_password("Test@Password123") + + # Request home page + response = client.get("/", headers={"Accept": "text/html"}) + + # Should still work because load_config creates default config + # This is the current behavior - may need to adjust if we want + # stricter setup requirements + assert response.status_code in [200, 302] + + def test_root_path_redirect(self, client): + """Test that root path redirects to setup when not configured.""" + response = client.get( + "/", headers={"Accept": "text/html"}, follow_redirects=False + ) + + # Should redirect to setup + assert response.status_code == 302 + assert response.headers["location"] == "/setup" + + def test_path_matching_exact_and_prefix(self, client): + """Test that path matching works for both exact and prefix matches.""" + middleware = SetupRedirectMiddleware(app=FastAPI()) + + # Exact matches + assert middleware._is_path_exempt("/setup") is True + assert middleware._is_path_exempt("/api/health") is True + + # Prefix matches + assert middleware._is_path_exempt("/static/css/style.css") is True + assert middleware._is_path_exempt("/static/js/app.js") is True + + # Non-exempt paths + assert middleware._is_path_exempt("/dashboard") is False + assert middleware._is_path_exempt("/api/data") is False + + +if __name__ == "__main__": + pytest.main([__file__, "-v"])