diff --git a/backend/app/core/security.py b/backend/app/core/security.py index ee9257a..957b1aa 100644 --- a/backend/app/core/security.py +++ b/backend/app/core/security.py @@ -1,22 +1,22 @@ from datetime import datetime, timedelta, timezone +import bcrypt from jose import JWTError, jwt -from passlib.context import CryptContext from app.core.config import settings -pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto") - def verify_password(plain: str, hashed: str) -> bool: + if not plain or not hashed: + return False try: - return bool(pwd_context.verify(plain, hashed)) - except ValueError: + return bcrypt.checkpw(plain.encode("utf-8"), hashed.encode("utf-8")) + except (ValueError, TypeError): return False def hash_password(password: str) -> str: - return str(pwd_context.hash(password)) + return bcrypt.hashpw(password.encode("utf-8"), bcrypt.gensalt()).decode("utf-8") def create_access_token(subject: str) -> str: diff --git a/backend/app/services/status_checker.py b/backend/app/services/status_checker.py index 4f6ba75..8a75126 100644 --- a/backend/app/services/status_checker.py +++ b/backend/app/services/status_checker.py @@ -24,6 +24,11 @@ async def check_node(check_method: str, target: str | None, ip: str | None) -> d host = target or raw_ip if not host: return {"status": "unknown", "response_time_ms": None} + # Reject hostnames that look like CLI flags — defends ping/tcp invocations + # against arg-injection if a malicious admin sets target like "-O". + if host.startswith("-"): + logger.warning("Rejecting check target that starts with '-': %r", host) + return {"status": "unknown", "response_time_ms": None} start = time.monotonic() try: diff --git a/backend/requirements.txt b/backend/requirements.txt index e504051..53d5bb7 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -7,8 +7,7 @@ alembic==1.13.3 pydantic==2.9.2 pydantic-settings==2.5.2 python-jose[cryptography]==3.5.0 -passlib[bcrypt]==1.7.4 -bcrypt==4.0.1 +bcrypt==4.2.1 python-multipart==0.0.27 apscheduler==3.10.4 python-nmap==0.7.1 diff --git a/backend/scripts/hash_password.py b/backend/scripts/hash_password.py index 9ee5c5b..6c1587d 100644 --- a/backend/scripts/hash_password.py +++ b/backend/scripts/hash_password.py @@ -1,13 +1,11 @@ -"""Generate a bcrypt password hash for config.yml.""" +"""Generate a bcrypt password hash for the AUTH_PASSWORD_HASH env var.""" import sys -from passlib.context import CryptContext - -pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto") +import bcrypt if len(sys.argv) < 2: print("Usage: python scripts/hash_password.py ") sys.exit(1) password = sys.argv[1] -print(pwd_context.hash(password)) +print(bcrypt.hashpw(password.encode("utf-8"), bcrypt.gensalt()).decode("utf-8")) diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index de3d6dd..7dc8c21 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -5,23 +5,21 @@ os.environ.setdefault("SECRET_KEY", "test-only-secret-key-not-for-production") import pytest from httpx import ASGITransport, AsyncClient -from passlib.context import CryptContext from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine +from app.core.security import hash_password from app.db.database import Base, get_db from app.main import app TEST_DB_URL = "sqlite+aiosqlite:///:memory:" -_pwd_ctx = CryptContext(schemes=["bcrypt"], deprecated="auto") - @pytest.fixture(autouse=True, scope="session") def test_credentials(): """Configure test auth credentials directly on settings.""" from app.core.config import settings settings.auth_username = "admin" - settings.auth_password_hash = _pwd_ctx.hash("admin") + settings.auth_password_hash = hash_password("admin") @pytest.fixture diff --git a/backend/tests/test_auth.py b/backend/tests/test_auth.py index f65f578..09b569d 100644 --- a/backend/tests/test_auth.py +++ b/backend/tests/test_auth.py @@ -68,3 +68,71 @@ async def test_login_with_malformed_hash_returns_401_not_500(client: AsyncClient assert res.status_code == 401 finally: settings.auth_password_hash = original + + +# --- JWT-level cases --- + +async def test_expired_token_rejected(client: AsyncClient): + """A JWT whose `exp` is in the past must be refused.""" + from datetime import datetime, timedelta, timezone + + from jose import jwt + + from app.core.config import settings + payload = { + "sub": "admin", + "exp": datetime.now(timezone.utc) - timedelta(minutes=1), + } + token = jwt.encode(payload, settings.secret_key, algorithm=settings.algorithm) + res = await client.get("/api/v1/nodes", headers={"Authorization": f"Bearer {token}"}) + assert res.status_code == 401 + + +async def test_malformed_token_rejected(client: AsyncClient): + res = await client.get("/api/v1/nodes", headers={"Authorization": "Bearer not-a-jwt"}) + assert res.status_code == 401 + + +async def test_token_signed_with_wrong_secret_rejected(client: AsyncClient): + """A token signed with a different key must not be accepted.""" + from datetime import datetime, timedelta, timezone + + from jose import jwt + + from app.core.config import settings + payload = { + "sub": "admin", + "exp": datetime.now(timezone.utc) + timedelta(minutes=5), + } + forged = jwt.encode(payload, "different-secret", algorithm=settings.algorithm) + res = await client.get("/api/v1/nodes", headers={"Authorization": f"Bearer {forged}"}) + assert res.status_code == 401 + + +async def test_missing_authorization_header_rejected(client: AsyncClient): + res = await client.get("/api/v1/nodes") + assert res.status_code == 401 + + +async def test_empty_password_does_not_pass_when_hash_empty(client: AsyncClient): + """No credentials configured server-side must not authenticate an empty password.""" + from app.core.config import settings + original_hash = settings.auth_password_hash + settings.auth_password_hash = "" + try: + res = await client.post("/api/v1/auth/login", json={"username": "admin", "password": ""}) + assert res.status_code == 401 + finally: + settings.auth_password_hash = original_hash + + +# --- Password helper --- + +def test_verify_password_handles_empty_inputs(): + """verify_password must be safe against empty plain / empty hash without raising.""" + from app.core.security import hash_password, verify_password + h = hash_password("hunter2") + assert verify_password("hunter2", h) is True + assert verify_password("", h) is False + assert verify_password("hunter2", "") is False + assert verify_password("", "") is False diff --git a/backend/tests/test_scan.py b/backend/tests/test_scan.py index 6b4b640..7199ea9 100644 --- a/backend/tests/test_scan.py +++ b/backend/tests/test_scan.py @@ -37,6 +37,95 @@ async def pending_device(db_session): return device +# --- _background_scan error handling --- + +@pytest.fixture +async def mem_db(): + from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine + + from app.db.database import Base + engine = create_async_engine("sqlite+aiosqlite:///:memory:") + async with engine.begin() as conn: + await conn.run_sync(Base.metadata.create_all) + factory = async_sessionmaker(engine, expire_on_commit=False) + yield factory + await engine.dispose() + + +@pytest.mark.asyncio +async def test_background_scan_marks_run_failed_on_exception(mem_db): + """If run_scan() raises, the ScanRun must transition running → failed and the + session rollback path must execute without a follow-on exception.""" + from app.api.routes.scan import _background_scan + + async with mem_db() as session: + run = ScanRun(status="running", ranges=["10.0.0.0/24"]) + session.add(run) + await session.commit() + run_id = run.id + + with ( + patch("app.api.routes.scan.AsyncSessionLocal", mem_db), + patch( + "app.api.routes.scan.run_scan", + new_callable=AsyncMock, + side_effect=RuntimeError("boom"), + ), + ): + await _background_scan(run_id, ["10.0.0.0/24"]) + + async with mem_db() as session: + refreshed = await session.get(ScanRun, run_id) + assert refreshed is not None + assert refreshed.status == "failed" + + +@pytest.mark.asyncio +async def test_background_scan_leaves_non_running_status_alone(mem_db): + """If the run was already stopped/cancelled before run_scan failed, _background_scan + must NOT overwrite that terminal status with 'failed'.""" + from app.api.routes.scan import _background_scan + + async with mem_db() as session: + run = ScanRun(status="cancelled", ranges=["10.0.0.0/24"]) + session.add(run) + await session.commit() + run_id = run.id + + with ( + patch("app.api.routes.scan.AsyncSessionLocal", mem_db), + patch( + "app.api.routes.scan.run_scan", + new_callable=AsyncMock, + side_effect=RuntimeError("boom"), + ), + ): + await _background_scan(run_id, ["10.0.0.0/24"]) + + async with mem_db() as session: + refreshed = await session.get(ScanRun, run_id) + assert refreshed is not None + assert refreshed.status == "cancelled" + + +@pytest.mark.asyncio +async def test_background_scan_success_path_invokes_run_scan(mem_db): + from app.api.routes.scan import _background_scan + + async with mem_db() as session: + run = ScanRun(status="running", ranges=["10.0.0.0/24"]) + session.add(run) + await session.commit() + run_id = run.id + + with ( + patch("app.api.routes.scan.AsyncSessionLocal", mem_db), + patch("app.api.routes.scan.run_scan", new_callable=AsyncMock) as mock_run_scan, + ): + await _background_scan(run_id, ["10.0.0.0/24"]) + mock_run_scan.assert_awaited_once() + + # --- Trigger scan --- @pytest.mark.asyncio diff --git a/backend/tests/test_status_checker.py b/backend/tests/test_status_checker.py index 3d23cd8..205e62d 100644 --- a/backend/tests/test_status_checker.py +++ b/backend/tests/test_status_checker.py @@ -216,6 +216,31 @@ async def test_ping_uses_windows_args_on_win32(): assert "-c" not in captured["args"] +# --- check_node target validation --- + +@pytest.mark.asyncio +async def test_check_node_rejects_flag_like_target(): + """A target starting with '-' must never reach subprocess invocation.""" + from app.services.status_checker import check_node + + with patch("asyncio.create_subprocess_exec") as mock_exec: + result = await check_node("ping", "-O", None) + + mock_exec.assert_not_called() + assert result["status"] == "unknown" + + +@pytest.mark.asyncio +async def test_check_node_rejects_flag_like_ip(): + from app.services.status_checker import check_node + + with patch("asyncio.create_subprocess_exec") as mock_exec: + result = await check_node("ping", None, "-O") + + mock_exec.assert_not_called() + assert result["status"] == "unknown" + + # --- _tcp_connect --- @pytest.mark.asyncio