Merge pull request #508 from christianlouis/copilot/fix-local-user-login-issue
This commit is contained in:
+11
-1
@@ -324,7 +324,17 @@ async def auth(request: Request, db: Session = Depends(get_db)):
|
|||||||
return RedirectResponse(url=redirect_url, status_code=302)
|
return RedirectResponse(url=redirect_url, status_code=302)
|
||||||
|
|
||||||
# --- Admin credentials (always available as a fallback / single-user mode) ---
|
# --- Admin credentials (always available as a fallback / single-user mode) ---
|
||||||
if username == settings.admin_username and password == settings.admin_password:
|
# Guard: only attempt the match when credentials are actually configured.
|
||||||
|
# Without this guard, Python's `None == None` would be True when neither
|
||||||
|
# ADMIN_USERNAME nor ADMIN_PASSWORD is set, allowing any request that omits
|
||||||
|
# those form fields to be authenticated as an admin — creating a phantom
|
||||||
|
# "None@local.docuelevate" admin profile with full privileges.
|
||||||
|
if (
|
||||||
|
settings.admin_username
|
||||||
|
and settings.admin_password
|
||||||
|
and username == settings.admin_username
|
||||||
|
and password == settings.admin_password
|
||||||
|
):
|
||||||
admin_user_data = {
|
admin_user_data = {
|
||||||
"id": "admin",
|
"id": "admin",
|
||||||
"name": "Administrator",
|
"name": "Administrator",
|
||||||
|
|||||||
@@ -817,6 +817,76 @@ class TestAuthFunction:
|
|||||||
assert isinstance(result, RedirectResponse)
|
assert isinstance(result, RedirectResponse)
|
||||||
assert result.headers["location"] == "/settings"
|
assert result.headers["location"] == "/settings"
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_auth_none_credentials_not_configured_blocks_login(self):
|
||||||
|
"""Login must fail when admin_username and admin_password are None (not configured).
|
||||||
|
|
||||||
|
Regression test: Python's ``None == None`` would previously evaluate to
|
||||||
|
``True``, allowing any request that omits the form fields to be
|
||||||
|
authenticated as admin and creating a phantom "None@local.docuelevate"
|
||||||
|
profile with full admin privileges.
|
||||||
|
"""
|
||||||
|
from app.auth import auth
|
||||||
|
|
||||||
|
mock_request = MagicMock(spec=Request)
|
||||||
|
# Form fields both absent → form_data.get() returns None
|
||||||
|
form_data = {}
|
||||||
|
mock_request.form = AsyncMock(return_value=form_data)
|
||||||
|
mock_request.session = {}
|
||||||
|
|
||||||
|
with patch("app.auth.settings") as mock_settings:
|
||||||
|
mock_settings.admin_username = None
|
||||||
|
mock_settings.admin_password = None
|
||||||
|
mock_settings.multi_user_enabled = False
|
||||||
|
|
||||||
|
result = await auth(mock_request, db=self._make_mock_db())
|
||||||
|
|
||||||
|
assert isinstance(result, RedirectResponse)
|
||||||
|
assert "/login?error=Invalid+username+or+password" in result.headers["location"]
|
||||||
|
assert "user" not in mock_request.session
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_auth_empty_string_credentials_not_configured_blocks_login(self):
|
||||||
|
"""Login must fail when admin_username and admin_password are empty strings."""
|
||||||
|
from app.auth import auth
|
||||||
|
|
||||||
|
mock_request = MagicMock(spec=Request)
|
||||||
|
form_data = {"username": "", "password": ""}
|
||||||
|
mock_request.form = AsyncMock(return_value=form_data)
|
||||||
|
mock_request.session = {}
|
||||||
|
|
||||||
|
with patch("app.auth.settings") as mock_settings:
|
||||||
|
mock_settings.admin_username = ""
|
||||||
|
mock_settings.admin_password = ""
|
||||||
|
mock_settings.multi_user_enabled = False
|
||||||
|
|
||||||
|
result = await auth(mock_request, db=self._make_mock_db())
|
||||||
|
|
||||||
|
assert isinstance(result, RedirectResponse)
|
||||||
|
assert "/login?error=Invalid+username+or+password" in result.headers["location"]
|
||||||
|
assert "user" not in mock_request.session
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_auth_none_password_not_configured_blocks_login(self):
|
||||||
|
"""Login must fail when only admin_password is None (not configured)."""
|
||||||
|
from app.auth import auth
|
||||||
|
|
||||||
|
mock_request = MagicMock(spec=Request)
|
||||||
|
form_data = {"username": "admin"}
|
||||||
|
mock_request.form = AsyncMock(return_value=form_data)
|
||||||
|
mock_request.session = {}
|
||||||
|
|
||||||
|
with patch("app.auth.settings") as mock_settings:
|
||||||
|
mock_settings.admin_username = "admin"
|
||||||
|
mock_settings.admin_password = None
|
||||||
|
mock_settings.multi_user_enabled = False
|
||||||
|
|
||||||
|
result = await auth(mock_request, db=self._make_mock_db())
|
||||||
|
|
||||||
|
assert isinstance(result, RedirectResponse)
|
||||||
|
assert "/login?error=Invalid+username+or+password" in result.headers["location"]
|
||||||
|
assert "user" not in mock_request.session
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.unit
|
@pytest.mark.unit
|
||||||
class TestLogoutFunction:
|
class TestLogoutFunction:
|
||||||
|
|||||||
Reference in New Issue
Block a user