fix: remove duplicate Audit Logs nav entry and add login/logout audit log events
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
+69
-1
@@ -15,6 +15,7 @@ from starlette.responses import RedirectResponse
|
|||||||
|
|
||||||
from app.config import settings
|
from app.config import settings
|
||||||
from app.database import get_db
|
from app.database import get_db
|
||||||
|
from app.middleware.audit_log import get_client_ip
|
||||||
|
|
||||||
# Conditional imports: only used when multi_user_enabled=True. Imported here at
|
# Conditional imports: only used when multi_user_enabled=True. Imported here at
|
||||||
# module level (not inside auth()) so they don't incur repeated import overhead.
|
# module level (not inside auth()) so they don't incur repeated import overhead.
|
||||||
@@ -344,6 +345,13 @@ async def oauth_callback(request: Request, db: Session = Depends(get_db)):
|
|||||||
|
|
||||||
# Log the successful authentication
|
# Log the successful authentication
|
||||||
logger.info("[SECURITY] OAUTH_LOGIN_SUCCESS user=%s admin=%s", user_data.get("email", "unknown"), is_admin)
|
logger.info("[SECURITY] OAUTH_LOGIN_SUCCESS user=%s admin=%s", user_data.get("email", "unknown"), is_admin)
|
||||||
|
_record_login_event(
|
||||||
|
db,
|
||||||
|
request,
|
||||||
|
user_data.get("email") or user_data.get("preferred_username") or "unknown",
|
||||||
|
success=True,
|
||||||
|
method="oauth",
|
||||||
|
)
|
||||||
|
|
||||||
# Redirect first-time users to onboarding
|
# Redirect first-time users to onboarding
|
||||||
user_id = (
|
user_id = (
|
||||||
@@ -364,6 +372,48 @@ async def oauth_callback(request: Request, db: Session = Depends(get_db)):
|
|||||||
return RedirectResponse(url=f"/login?error=Authentication+failed:+{str(e)}", status_code=status.HTTP_302_FOUND)
|
return RedirectResponse(url=f"/login?error=Authentication+failed:+{str(e)}", status_code=status.HTTP_302_FOUND)
|
||||||
|
|
||||||
|
|
||||||
|
def _record_login_event(
|
||||||
|
db: Session,
|
||||||
|
request: Request,
|
||||||
|
username: str,
|
||||||
|
*,
|
||||||
|
success: bool,
|
||||||
|
method: str = "local",
|
||||||
|
detail: str | None = None,
|
||||||
|
) -> None:
|
||||||
|
"""Write a login or login-failure audit event to the database.
|
||||||
|
|
||||||
|
Failures are silently swallowed so that an audit-service error never
|
||||||
|
prevents a legitimate login or surfaces an unrelated 500 error to the user.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
db: Active database session.
|
||||||
|
request: The current HTTP request (used to extract the client IP).
|
||||||
|
username: The username that attempted authentication.
|
||||||
|
success: ``True`` for a successful login, ``False`` for a failure.
|
||||||
|
method: Authentication method, e.g. ``"local"`` or ``"oauth"``.
|
||||||
|
detail: Optional extra context for failures (e.g. ``"wrong_password"``).
|
||||||
|
"""
|
||||||
|
try:
|
||||||
|
from app.utils.audit_service import record_event
|
||||||
|
|
||||||
|
action = "login" if success else "login.failure"
|
||||||
|
details: dict = {"method": method}
|
||||||
|
if detail:
|
||||||
|
details["reason"] = detail
|
||||||
|
record_event(
|
||||||
|
db,
|
||||||
|
action=action,
|
||||||
|
user=username,
|
||||||
|
resource_type="session",
|
||||||
|
ip_address=get_client_ip(request),
|
||||||
|
details=details,
|
||||||
|
severity="info" if success else "warning",
|
||||||
|
)
|
||||||
|
except Exception:
|
||||||
|
logger.debug("Failed to write login audit event for user=%s", username, exc_info=True)
|
||||||
|
|
||||||
|
|
||||||
async def auth(request: Request, db: Session = Depends(get_db)):
|
async def auth(request: Request, db: Session = Depends(get_db)):
|
||||||
"""Handle local username/password authentication.
|
"""Handle local username/password authentication.
|
||||||
|
|
||||||
@@ -413,6 +463,7 @@ async def auth(request: Request, db: Session = Depends(get_db)):
|
|||||||
username,
|
username,
|
||||||
local_user.is_active,
|
local_user.is_active,
|
||||||
)
|
)
|
||||||
|
_record_login_event(db, request, username, success=False, detail="account_not_verified")
|
||||||
return RedirectResponse(
|
return RedirectResponse(
|
||||||
url="/login?error=Please+verify+your+email+address+before+logging+in",
|
url="/login?error=Please+verify+your+email+address+before+logging+in",
|
||||||
status_code=302,
|
status_code=302,
|
||||||
@@ -425,10 +476,12 @@ async def auth(request: Request, db: Session = Depends(get_db)):
|
|||||||
)
|
)
|
||||||
if not pw_ok:
|
if not pw_ok:
|
||||||
logger.warning("[SECURITY] LOCAL_LOGIN_FAILURE reason=wrong_password user=%s", username)
|
logger.warning("[SECURITY] LOCAL_LOGIN_FAILURE reason=wrong_password user=%s", username)
|
||||||
|
_record_login_event(db, request, username, success=False, detail="wrong_password")
|
||||||
return RedirectResponse(url="/login?error=Invalid+username+or+password", status_code=302)
|
return RedirectResponse(url="/login?error=Invalid+username+or+password", status_code=302)
|
||||||
user_data = _build_session_user(local_user)
|
user_data = _build_session_user(local_user)
|
||||||
request.session["user"] = user_data
|
request.session["user"] = user_data
|
||||||
logger.info("[SECURITY] LOCAL_LOGIN_SUCCESS user=%s", local_user.email)
|
logger.info("[SECURITY] LOCAL_LOGIN_SUCCESS user=%s", local_user.email)
|
||||||
|
_record_login_event(db, request, local_user.email, success=True)
|
||||||
_ensure_user_profile(db, user_data, is_admin=bool(local_user.is_admin))
|
_ensure_user_profile(db, user_data, is_admin=bool(local_user.is_admin))
|
||||||
profile = db.query(_UserProfile).filter(_UserProfile.user_id == local_user.email).first()
|
profile = db.query(_UserProfile).filter(_UserProfile.user_id == local_user.email).first()
|
||||||
if profile and not profile.onboarding_completed:
|
if profile and not profile.onboarding_completed:
|
||||||
@@ -473,6 +526,7 @@ async def auth(request: Request, db: Session = Depends(get_db)):
|
|||||||
}
|
}
|
||||||
request.session["user"] = admin_user_data
|
request.session["user"] = admin_user_data
|
||||||
logger.info("[SECURITY] LOCAL_LOGIN_SUCCESS user=%s", username)
|
logger.info("[SECURITY] LOCAL_LOGIN_SUCCESS user=%s", username)
|
||||||
|
_record_login_event(db, request, username, success=True)
|
||||||
_ensure_user_profile(db, admin_user_data, is_admin=True)
|
_ensure_user_profile(db, admin_user_data, is_admin=True)
|
||||||
redirect_url = request.session.pop("redirect_after_login", "/upload")
|
redirect_url = request.session.pop("redirect_after_login", "/upload")
|
||||||
return RedirectResponse(url=redirect_url, status_code=302)
|
return RedirectResponse(url=redirect_url, status_code=302)
|
||||||
@@ -485,16 +539,30 @@ async def auth(request: Request, db: Session = Depends(get_db)):
|
|||||||
admin_configured,
|
admin_configured,
|
||||||
not username and not password,
|
not username and not password,
|
||||||
)
|
)
|
||||||
|
_record_login_event(db, request, username or "anonymous", success=False, detail="invalid_credentials")
|
||||||
return RedirectResponse(url="/login?error=Invalid+username+or+password", status_code=302)
|
return RedirectResponse(url="/login?error=Invalid+username+or+password", status_code=302)
|
||||||
|
|
||||||
|
|
||||||
async def logout(request: Request):
|
async def logout(request: Request, db: Session = Depends(get_db)):
|
||||||
"""Handle user logout"""
|
"""Handle user logout"""
|
||||||
user = request.session.get("user")
|
user = request.session.get("user")
|
||||||
username = "unknown"
|
username = "unknown"
|
||||||
if isinstance(user, dict):
|
if isinstance(user, dict):
|
||||||
username = user.get("preferred_username") or user.get("email") or "unknown"
|
username = user.get("preferred_username") or user.get("email") or "unknown"
|
||||||
logger.info(f"[SECURITY] LOGOUT user={username}")
|
logger.info(f"[SECURITY] LOGOUT user={username}")
|
||||||
|
try:
|
||||||
|
from app.utils.audit_service import record_event
|
||||||
|
|
||||||
|
record_event(
|
||||||
|
db,
|
||||||
|
action="logout",
|
||||||
|
user=username,
|
||||||
|
resource_type="session",
|
||||||
|
ip_address=get_client_ip(request),
|
||||||
|
severity="info",
|
||||||
|
)
|
||||||
|
except Exception:
|
||||||
|
logger.debug("Failed to write logout audit event for user=%s", username, exc_info=True)
|
||||||
request.session.pop("user", None)
|
request.session.pop("user", None)
|
||||||
return RedirectResponse(url="/login?message=You+have+been+logged+out+successfully", status_code=302)
|
return RedirectResponse(url="/login?message=You+have+been+logged+out+successfully", status_code=302)
|
||||||
|
|
||||||
|
|||||||
@@ -177,9 +177,6 @@
|
|||||||
<a href="/admin/audit-logs" role="menuitem" class="flex items-center px-4 py-2 text-sm text-gray-700 hover:bg-gray-100">
|
<a href="/admin/audit-logs" role="menuitem" class="flex items-center px-4 py-2 text-sm text-gray-700 hover:bg-gray-100">
|
||||||
<i class="fas fa-shield-halved w-4 mr-2 text-indigo-500" aria-hidden="true"></i> Audit Logs
|
<i class="fas fa-shield-halved w-4 mr-2 text-indigo-500" aria-hidden="true"></i> Audit Logs
|
||||||
</a>
|
</a>
|
||||||
<a href="/admin/audit-logs" role="menuitem" class="flex items-center px-4 py-2 text-sm text-gray-700 hover:bg-gray-100">
|
|
||||||
<i class="fas fa-shield-halved w-4 mr-2 text-indigo-500" aria-hidden="true"></i> Audit Logs
|
|
||||||
</a>
|
|
||||||
<a href="/status" role="menuitem" class="flex items-center px-4 py-2 text-sm text-gray-700 hover:bg-gray-100"
|
<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 %}>
|
{% 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> {{ _("nav.status") }}
|
<i class="fas fa-circle-dot w-4 mr-2 text-gray-500" aria-hidden="true"></i> {{ _("nav.status") }}
|
||||||
|
|||||||
Reference in New Issue
Block a user