fix: setup routing, async bcrypt, password hashing, clean command
- Add SetupGuard component: redirects to /setup if setup not complete, shown as spinner while loading. All routes except /setup now wrapped. - SetupPage redirects to /login on mount when setup already done. - Fix async blocking: offload bcrypt.hashpw and bcrypt.checkpw to run_in_executor so they never stall the asyncio event loop. - Hash password with SHA-256 (SubtleCrypto) before transmission; added src/utils/crypto.ts with sha256Hex(). Backend stores bcrypt(sha256). - Add Makefile with make up/down/restart/logs/clean targets. - Add tests: _check_password async, concurrent bcrypt, expired session, login-without-setup, run_setup event-loop interleaving. - Update Architekture.md and Features.md to reflect all changes.
This commit is contained in:
@@ -7,6 +7,7 @@ survive server restarts.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import secrets
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
@@ -25,9 +26,12 @@ from app.utils.time_utils import add_minutes, utc_now
|
||||
log: structlog.stdlib.BoundLogger = structlog.get_logger()
|
||||
|
||||
|
||||
def _check_password(plain: str, hashed: str) -> bool:
|
||||
async def _check_password(plain: str, hashed: str) -> bool:
|
||||
"""Return ``True`` if *plain* matches the bcrypt *hashed* password.
|
||||
|
||||
Runs in a thread executor so the blocking bcrypt operation does not stall
|
||||
the asyncio event loop.
|
||||
|
||||
Args:
|
||||
plain: The plain-text password to verify.
|
||||
hashed: The stored bcrypt hash string.
|
||||
@@ -35,7 +39,12 @@ def _check_password(plain: str, hashed: str) -> bool:
|
||||
Returns:
|
||||
``True`` on a successful match, ``False`` otherwise.
|
||||
"""
|
||||
return bool(bcrypt.checkpw(plain.encode(), hashed.encode()))
|
||||
plain_bytes = plain.encode()
|
||||
hashed_bytes = hashed.encode()
|
||||
loop = asyncio.get_running_loop()
|
||||
return await loop.run_in_executor(
|
||||
None, lambda: bool(bcrypt.checkpw(plain_bytes, hashed_bytes))
|
||||
)
|
||||
|
||||
|
||||
async def login(
|
||||
@@ -61,7 +70,7 @@ async def login(
|
||||
log.warning("bangui_login_no_hash")
|
||||
raise ValueError("No password is configured — run setup first.")
|
||||
|
||||
if not _check_password(password, stored_hash):
|
||||
if not await _check_password(password, stored_hash):
|
||||
log.warning("bangui_login_wrong_password")
|
||||
raise ValueError("Incorrect password.")
|
||||
|
||||
|
||||
@@ -7,6 +7,7 @@ enforcing the rule that setup can only run once.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
import bcrypt
|
||||
@@ -72,8 +73,13 @@ async def run_setup(
|
||||
log.info("bangui_setup_started")
|
||||
|
||||
# Hash the master password — bcrypt automatically generates a salt.
|
||||
# Run in a thread executor so the blocking bcrypt operation does not stall
|
||||
# the asyncio event loop.
|
||||
password_bytes = master_password.encode()
|
||||
hashed = bcrypt.hashpw(password_bytes, bcrypt.gensalt()).decode()
|
||||
loop = asyncio.get_running_loop()
|
||||
hashed: str = await loop.run_in_executor(
|
||||
None, lambda: bcrypt.hashpw(password_bytes, bcrypt.gensalt()).decode()
|
||||
)
|
||||
|
||||
await settings_repo.set_setting(db, _KEY_PASSWORD_HASH, hashed)
|
||||
await settings_repo.set_setting(db, _KEY_DATABASE_PATH, database_path)
|
||||
|
||||
@@ -2,6 +2,8 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import inspect
|
||||
from pathlib import Path
|
||||
|
||||
import aiosqlite
|
||||
@@ -30,6 +32,50 @@ async def db(tmp_path: Path) -> aiosqlite.Connection: # type: ignore[misc]
|
||||
await conn.close()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
async def db_no_setup(tmp_path: Path) -> aiosqlite.Connection: # type: ignore[misc]
|
||||
"""Provide an initialised DB with no setup performed."""
|
||||
conn: aiosqlite.Connection = await aiosqlite.connect(str(tmp_path / "auth_nosetup.db"))
|
||||
conn.row_factory = aiosqlite.Row
|
||||
await init_db(conn)
|
||||
yield conn
|
||||
await conn.close()
|
||||
|
||||
|
||||
class TestCheckPasswordAsync:
|
||||
async def test_check_password_is_coroutine_function(self) -> None:
|
||||
"""_check_password must be a coroutine function (runs in thread executor)."""
|
||||
assert inspect.iscoroutinefunction(auth_service._check_password) # noqa: SLF001
|
||||
|
||||
async def test_check_password_returns_true_on_match(self) -> None:
|
||||
"""_check_password returns True for a matching plain/hash pair."""
|
||||
import bcrypt
|
||||
|
||||
hashed = bcrypt.hashpw(b"secret", bcrypt.gensalt()).decode()
|
||||
result = await auth_service._check_password("secret", hashed) # noqa: SLF001
|
||||
assert result is True
|
||||
|
||||
async def test_check_password_returns_false_on_mismatch(self) -> None:
|
||||
"""_check_password returns False when the password does not match."""
|
||||
import bcrypt
|
||||
|
||||
hashed = bcrypt.hashpw(b"secret", bcrypt.gensalt()).decode()
|
||||
result = await auth_service._check_password("wrong", hashed) # noqa: SLF001
|
||||
assert result is False
|
||||
|
||||
async def test_check_password_does_not_block_event_loop(self) -> None:
|
||||
"""_check_password awaits without blocking; event-loop tasks can interleave."""
|
||||
import bcrypt
|
||||
|
||||
hashed = bcrypt.hashpw(b"secret", bcrypt.gensalt()).decode()
|
||||
# Running two concurrent checks must complete without deadlock.
|
||||
results = await asyncio.gather(
|
||||
auth_service._check_password("secret", hashed), # noqa: SLF001
|
||||
auth_service._check_password("wrong", hashed), # noqa: SLF001
|
||||
)
|
||||
assert results == [True, False]
|
||||
|
||||
|
||||
class TestLogin:
|
||||
async def test_login_returns_session_on_correct_password(
|
||||
self, db: aiosqlite.Connection
|
||||
@@ -47,6 +93,13 @@ class TestLogin:
|
||||
with pytest.raises(ValueError, match="Incorrect password"):
|
||||
await auth_service.login(db, password="wrongpassword", session_duration_minutes=60)
|
||||
|
||||
async def test_login_raises_when_no_hash_configured(
|
||||
self, db_no_setup: aiosqlite.Connection
|
||||
) -> None:
|
||||
"""login() raises ValueError when setup has not been run."""
|
||||
with pytest.raises(ValueError, match="No password is configured"):
|
||||
await auth_service.login(db_no_setup, password="any", session_duration_minutes=60)
|
||||
|
||||
async def test_login_persists_session(self, db: aiosqlite.Connection) -> None:
|
||||
"""login() stores the session in the database."""
|
||||
from app.repositories import session_repo
|
||||
@@ -73,6 +126,27 @@ class TestValidateSession:
|
||||
with pytest.raises(ValueError, match="not found"):
|
||||
await auth_service.validate_session(db, "deadbeef" * 8)
|
||||
|
||||
async def test_validate_raises_for_expired_session(
|
||||
self, db: aiosqlite.Connection
|
||||
) -> None:
|
||||
"""validate_session() raises ValueError and removes an expired session."""
|
||||
from app.repositories import session_repo
|
||||
|
||||
# Create a session that expired in the past.
|
||||
past_token = "expiredtoken01" * 4 # 56 chars, unique enough for tests
|
||||
await session_repo.create_session(
|
||||
db,
|
||||
token=past_token,
|
||||
created_at="2000-01-01T00:00:00+00:00",
|
||||
expires_at="2000-01-01T01:00:00+00:00",
|
||||
)
|
||||
|
||||
with pytest.raises(ValueError, match="expired"):
|
||||
await auth_service.validate_session(db, past_token)
|
||||
|
||||
# The expired session must have been deleted.
|
||||
assert await session_repo.get_session(db, past_token) is None
|
||||
|
||||
|
||||
class TestLogout:
|
||||
async def test_logout_removes_session(self, db: aiosqlite.Connection) -> None:
|
||||
|
||||
@@ -2,6 +2,8 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import inspect
|
||||
from pathlib import Path
|
||||
|
||||
import aiosqlite
|
||||
@@ -115,3 +117,34 @@ class TestGetTimezone:
|
||||
session_duration_minutes=60,
|
||||
)
|
||||
assert await setup_service.get_timezone(db) == "America/New_York"
|
||||
|
||||
|
||||
class TestRunSetupAsync:
|
||||
"""Verify the async/non-blocking bcrypt behavior of run_setup."""
|
||||
|
||||
async def test_run_setup_is_coroutine_function(self) -> None:
|
||||
"""run_setup must be declared as an async function."""
|
||||
assert inspect.iscoroutinefunction(setup_service.run_setup)
|
||||
|
||||
async def test_password_hash_does_not_block_event_loop(
|
||||
self, db: aiosqlite.Connection
|
||||
) -> None:
|
||||
"""run_setup completes without blocking; other coroutines can interleave."""
|
||||
|
||||
async def noop() -> str:
|
||||
"""A trivial coroutine that should run concurrently with setup."""
|
||||
await asyncio.sleep(0)
|
||||
return "ok"
|
||||
|
||||
setup_coro = setup_service.run_setup(
|
||||
db,
|
||||
master_password="mypassword1",
|
||||
database_path="bangui.db",
|
||||
fail2ban_socket="/var/run/fail2ban/fail2ban.sock",
|
||||
timezone="UTC",
|
||||
session_duration_minutes=60,
|
||||
)
|
||||
# Both tasks should finish without error.
|
||||
results = await asyncio.gather(setup_coro, noop())
|
||||
assert results[1] == "ok"
|
||||
assert await setup_service.is_setup_complete(db) is True
|
||||
|
||||
Reference in New Issue
Block a user