feat: enable audit logging for authentication and task operations
Add audit_service.log_event() calls to track key user activities: - auth_login: successful and failed login attempts with IP/user agent - auth_logout: single session and all sessions logout - task_delete: task deletion with user context - file_upload: file upload with filename, size, and type - admin_cleanup: manual cleanup trigger with statistics Each event captures client IP (from X-Forwarded-For/X-Real-IP headers), user agent, and relevant metadata for compliance and debugging. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -390,7 +390,7 @@ async def root():
|
|||||||
|
|
||||||
# Include V2 API routers
|
# Include V2 API routers
|
||||||
from app.routers import auth, tasks, admin, translate
|
from app.routers import auth, tasks, admin, translate
|
||||||
from fastapi import UploadFile, File, Depends, HTTPException, status
|
from fastapi import UploadFile, File, Depends, HTTPException, status, Request
|
||||||
from sqlalchemy.orm import Session
|
from sqlalchemy.orm import Session
|
||||||
import hashlib
|
import hashlib
|
||||||
|
|
||||||
@@ -399,6 +399,23 @@ from app.models.user import User
|
|||||||
from app.models.task import TaskFile
|
from app.models.task import TaskFile
|
||||||
from app.schemas.task import UploadResponse, TaskStatusEnum
|
from app.schemas.task import UploadResponse, TaskStatusEnum
|
||||||
from app.services.task_service import task_service
|
from app.services.task_service import task_service
|
||||||
|
from app.services.audit_service import audit_service
|
||||||
|
|
||||||
|
|
||||||
|
def get_client_ip(request: Request) -> str:
|
||||||
|
"""Extract client IP address from request"""
|
||||||
|
forwarded = request.headers.get("X-Forwarded-For")
|
||||||
|
if forwarded:
|
||||||
|
return forwarded.split(",")[0].strip()
|
||||||
|
real_ip = request.headers.get("X-Real-IP")
|
||||||
|
if real_ip:
|
||||||
|
return real_ip
|
||||||
|
return request.client.host if request.client else "unknown"
|
||||||
|
|
||||||
|
|
||||||
|
def get_user_agent(request: Request) -> str:
|
||||||
|
"""Extract user agent from request"""
|
||||||
|
return request.headers.get("User-Agent", "unknown")[:500]
|
||||||
|
|
||||||
app.include_router(auth.router)
|
app.include_router(auth.router)
|
||||||
app.include_router(tasks.router)
|
app.include_router(tasks.router)
|
||||||
@@ -409,6 +426,7 @@ app.include_router(translate.router)
|
|||||||
# File upload endpoint
|
# File upload endpoint
|
||||||
@app.post("/api/v2/upload", response_model=UploadResponse, tags=["Upload"], summary="Upload file for OCR")
|
@app.post("/api/v2/upload", response_model=UploadResponse, tags=["Upload"], summary="Upload file for OCR")
|
||||||
async def upload_file(
|
async def upload_file(
|
||||||
|
request: Request,
|
||||||
file: UploadFile = File(..., description="File to upload (PNG, JPG, PDF, etc.)"),
|
file: UploadFile = File(..., description="File to upload (PNG, JPG, PDF, etc.)"),
|
||||||
db: Session = Depends(get_db),
|
db: Session = Depends(get_db),
|
||||||
current_user: User = Depends(get_current_user)
|
current_user: User = Depends(get_current_user)
|
||||||
@@ -470,6 +488,25 @@ async def upload_file(
|
|||||||
|
|
||||||
logger.info(f"Uploaded file {file.filename} ({file_size} bytes) for task {task.task_id}, user {current_user.email}")
|
logger.info(f"Uploaded file {file.filename} ({file_size} bytes) for task {task.task_id}, user {current_user.email}")
|
||||||
|
|
||||||
|
# Log file upload event
|
||||||
|
audit_service.log_event(
|
||||||
|
db=db,
|
||||||
|
event_type="file_upload",
|
||||||
|
event_category="file",
|
||||||
|
description=f"File uploaded: {file.filename} ({file_size} bytes)",
|
||||||
|
user_id=current_user.id,
|
||||||
|
ip_address=get_client_ip(request),
|
||||||
|
user_agent=get_user_agent(request),
|
||||||
|
resource_type="task",
|
||||||
|
resource_id=task.task_id,
|
||||||
|
success=True,
|
||||||
|
metadata={
|
||||||
|
"filename": file.filename,
|
||||||
|
"file_size": file_size,
|
||||||
|
"file_type": file.content_type
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
return {
|
return {
|
||||||
"task_id": task.task_id,
|
"task_id": task.task_id,
|
||||||
"filename": file.filename,
|
"filename": file.filename,
|
||||||
|
|||||||
@@ -405,6 +405,22 @@ async def trigger_cleanup(
|
|||||||
f"{result['total_files_deleted']} files, {result['total_bytes_freed']} bytes"
|
f"{result['total_files_deleted']} files, {result['total_bytes_freed']} bytes"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Log admin cleanup action
|
||||||
|
audit_service.log_event(
|
||||||
|
db=db,
|
||||||
|
event_type="admin_cleanup",
|
||||||
|
event_category="admin",
|
||||||
|
description=f"Manual cleanup: {result['total_files_deleted']} files, {result['total_bytes_freed']} bytes freed",
|
||||||
|
user_id=admin_user.id,
|
||||||
|
success=True,
|
||||||
|
metadata={
|
||||||
|
"files_deleted": result['total_files_deleted'],
|
||||||
|
"bytes_freed": result['total_bytes_freed'],
|
||||||
|
"users_processed": result['users_processed'],
|
||||||
|
"max_files_per_user": files_to_keep
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
return {
|
return {
|
||||||
"success": True,
|
"success": True,
|
||||||
"message": "Cleanup completed successfully",
|
"message": "Cleanup completed successfully",
|
||||||
|
|||||||
@@ -17,6 +17,7 @@ from app.models.user import User
|
|||||||
from app.models.session import Session as UserSession
|
from app.models.session import Session as UserSession
|
||||||
from app.schemas.auth import LoginRequest, Token, UserResponse
|
from app.schemas.auth import LoginRequest, Token, UserResponse
|
||||||
from app.services.external_auth_service import external_auth_service
|
from app.services.external_auth_service import external_auth_service
|
||||||
|
from app.services.audit_service import audit_service
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
@@ -66,6 +67,17 @@ async def login(
|
|||||||
logger.warning(
|
logger.warning(
|
||||||
f"External auth failed for user {login_data.username}: {error_msg}"
|
f"External auth failed for user {login_data.username}: {error_msg}"
|
||||||
)
|
)
|
||||||
|
# Log failed login attempt
|
||||||
|
audit_service.log_event(
|
||||||
|
db=db,
|
||||||
|
event_type="auth_login",
|
||||||
|
event_category="authentication",
|
||||||
|
description=f"Login failed for {login_data.username}: {error_msg}",
|
||||||
|
ip_address=get_client_ip(request),
|
||||||
|
user_agent=get_user_agent(request),
|
||||||
|
success=False,
|
||||||
|
error_message=error_msg
|
||||||
|
)
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=status.HTTP_401_UNAUTHORIZED,
|
status_code=status.HTTP_401_UNAUTHORIZED,
|
||||||
detail=error_msg or "Authentication failed",
|
detail=error_msg or "Authentication failed",
|
||||||
@@ -151,6 +163,20 @@ async def login(
|
|||||||
expires_delta=internal_token_expires
|
expires_delta=internal_token_expires
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Log successful login
|
||||||
|
audit_service.log_event(
|
||||||
|
db=db,
|
||||||
|
event_type="auth_login",
|
||||||
|
event_category="authentication",
|
||||||
|
description=f"User logged in successfully",
|
||||||
|
user_id=user.id,
|
||||||
|
ip_address=get_client_ip(request),
|
||||||
|
user_agent=get_user_agent(request),
|
||||||
|
resource_type="session",
|
||||||
|
resource_id=str(session.id),
|
||||||
|
success=True
|
||||||
|
)
|
||||||
|
|
||||||
return {
|
return {
|
||||||
"access_token": internal_access_token,
|
"access_token": internal_access_token,
|
||||||
"token_type": "bearer",
|
"token_type": "bearer",
|
||||||
@@ -165,6 +191,7 @@ async def login(
|
|||||||
|
|
||||||
@router.post("/logout", summary="User logout")
|
@router.post("/logout", summary="User logout")
|
||||||
async def logout(
|
async def logout(
|
||||||
|
request: Request,
|
||||||
session_id: Optional[int] = None,
|
session_id: Optional[int] = None,
|
||||||
db: Session = Depends(get_db),
|
db: Session = Depends(get_db),
|
||||||
current_user: User = Depends(get_current_user)
|
current_user: User = Depends(get_current_user)
|
||||||
@@ -174,9 +201,6 @@ async def logout(
|
|||||||
|
|
||||||
- **session_id**: Session ID to logout (optional, logs out all if not provided)
|
- **session_id**: Session ID to logout (optional, logs out all if not provided)
|
||||||
"""
|
"""
|
||||||
# TODO: Implement proper current_user dependency from JWT token
|
|
||||||
# For now, this is a placeholder
|
|
||||||
|
|
||||||
if session_id:
|
if session_id:
|
||||||
# Logout specific session
|
# Logout specific session
|
||||||
session = db.query(UserSession).filter(
|
session = db.query(UserSession).filter(
|
||||||
@@ -188,6 +212,20 @@ async def logout(
|
|||||||
db.delete(session)
|
db.delete(session)
|
||||||
db.commit()
|
db.commit()
|
||||||
logger.info(f"Logged out session {session_id} for user {current_user.email}")
|
logger.info(f"Logged out session {session_id} for user {current_user.email}")
|
||||||
|
|
||||||
|
# Log logout event
|
||||||
|
audit_service.log_event(
|
||||||
|
db=db,
|
||||||
|
event_type="auth_logout",
|
||||||
|
event_category="authentication",
|
||||||
|
description=f"User logged out session {session_id}",
|
||||||
|
user_id=current_user.id,
|
||||||
|
ip_address=get_client_ip(request),
|
||||||
|
user_agent=get_user_agent(request),
|
||||||
|
resource_type="session",
|
||||||
|
resource_id=str(session_id),
|
||||||
|
success=True
|
||||||
|
)
|
||||||
return {"message": "Logged out successfully"}
|
return {"message": "Logged out successfully"}
|
||||||
else:
|
else:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
@@ -206,6 +244,19 @@ async def logout(
|
|||||||
|
|
||||||
db.commit()
|
db.commit()
|
||||||
logger.info(f"Logged out all {count} sessions for user {current_user.email}")
|
logger.info(f"Logged out all {count} sessions for user {current_user.email}")
|
||||||
|
|
||||||
|
# Log logout event
|
||||||
|
audit_service.log_event(
|
||||||
|
db=db,
|
||||||
|
event_type="auth_logout",
|
||||||
|
event_category="authentication",
|
||||||
|
description=f"User logged out all {count} sessions",
|
||||||
|
user_id=current_user.id,
|
||||||
|
ip_address=get_client_ip(request),
|
||||||
|
user_agent=get_user_agent(request),
|
||||||
|
success=True,
|
||||||
|
metadata={"sessions_count": count}
|
||||||
|
)
|
||||||
return {"message": f"Logged out {count} sessions"}
|
return {"message": f"Logged out {count} sessions"}
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -9,7 +9,7 @@ from pathlib import Path
|
|||||||
import shutil
|
import shutil
|
||||||
import hashlib
|
import hashlib
|
||||||
|
|
||||||
from fastapi import APIRouter, Depends, HTTPException, status, Query, UploadFile, File, BackgroundTasks
|
from fastapi import APIRouter, Depends, HTTPException, status, Query, UploadFile, File, BackgroundTasks, Request
|
||||||
from fastapi.responses import FileResponse, StreamingResponse
|
from fastapi.responses import FileResponse, StreamingResponse
|
||||||
from sqlalchemy.orm import Session
|
from sqlalchemy.orm import Session
|
||||||
import json
|
import json
|
||||||
@@ -47,6 +47,7 @@ from app.services.task_service import task_service
|
|||||||
from app.services.file_access_service import file_access_service
|
from app.services.file_access_service import file_access_service
|
||||||
from app.services.ocr_service import OCRService
|
from app.services.ocr_service import OCRService
|
||||||
from app.services.service_pool import get_service_pool, PoolConfig
|
from app.services.service_pool import get_service_pool, PoolConfig
|
||||||
|
from app.services.audit_service import audit_service
|
||||||
|
|
||||||
# Import dual-track components
|
# Import dual-track components
|
||||||
try:
|
try:
|
||||||
@@ -67,6 +68,22 @@ logger = logging.getLogger(__name__)
|
|||||||
router = APIRouter(prefix="/api/v2/tasks", tags=["Tasks"])
|
router = APIRouter(prefix="/api/v2/tasks", tags=["Tasks"])
|
||||||
|
|
||||||
|
|
||||||
|
def get_client_ip(request: Request) -> str:
|
||||||
|
"""Extract client IP address from request"""
|
||||||
|
forwarded = request.headers.get("X-Forwarded-For")
|
||||||
|
if forwarded:
|
||||||
|
return forwarded.split(",")[0].strip()
|
||||||
|
real_ip = request.headers.get("X-Real-IP")
|
||||||
|
if real_ip:
|
||||||
|
return real_ip
|
||||||
|
return request.client.host if request.client else "unknown"
|
||||||
|
|
||||||
|
|
||||||
|
def get_user_agent(request: Request) -> str:
|
||||||
|
"""Extract user agent from request"""
|
||||||
|
return request.headers.get("User-Agent", "unknown")[:500]
|
||||||
|
|
||||||
|
|
||||||
def process_task_ocr(
|
def process_task_ocr(
|
||||||
task_id: str,
|
task_id: str,
|
||||||
task_db_id: int,
|
task_db_id: int,
|
||||||
@@ -518,6 +535,7 @@ async def update_task(
|
|||||||
@router.delete("/{task_id}", status_code=status.HTTP_204_NO_CONTENT)
|
@router.delete("/{task_id}", status_code=status.HTTP_204_NO_CONTENT)
|
||||||
async def delete_task(
|
async def delete_task(
|
||||||
task_id: str,
|
task_id: str,
|
||||||
|
request: Request,
|
||||||
db: Session = Depends(get_db),
|
db: Session = Depends(get_db),
|
||||||
current_user: User = Depends(get_current_user)
|
current_user: User = Depends(get_current_user)
|
||||||
):
|
):
|
||||||
@@ -539,6 +557,20 @@ async def delete_task(
|
|||||||
)
|
)
|
||||||
|
|
||||||
logger.info(f"Deleted task {task_id} for user {current_user.email}")
|
logger.info(f"Deleted task {task_id} for user {current_user.email}")
|
||||||
|
|
||||||
|
# Log task deletion
|
||||||
|
audit_service.log_event(
|
||||||
|
db=db,
|
||||||
|
event_type="task_delete",
|
||||||
|
event_category="task",
|
||||||
|
description=f"Task deleted",
|
||||||
|
user_id=current_user.id,
|
||||||
|
ip_address=get_client_ip(request),
|
||||||
|
user_agent=get_user_agent(request),
|
||||||
|
resource_type="task",
|
||||||
|
resource_id=task_id,
|
||||||
|
success=True
|
||||||
|
)
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
52
openspec/changes/enable-audit-logging/proposal.md
Normal file
52
openspec/changes/enable-audit-logging/proposal.md
Normal file
@@ -0,0 +1,52 @@
|
|||||||
|
# Enable Audit Logging
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
Activate the existing audit logging infrastructure by adding `audit_service.log_event()` calls to key system operations. The audit log table and service already exist but are not being used.
|
||||||
|
|
||||||
|
## Motivation
|
||||||
|
- Audit logs page exists but shows no data because events are not being recorded
|
||||||
|
- Security compliance requires tracking of authentication and administrative actions
|
||||||
|
- Administrators need visibility into system usage and potential security issues
|
||||||
|
|
||||||
|
## Current State
|
||||||
|
- `AuditLog` model exists in `backend/app/models/audit_log.py`
|
||||||
|
- `AuditService` with `log_event()` method exists in `backend/app/services/audit_service.py`
|
||||||
|
- `AuditLogsPage` frontend exists at `/admin/audit-logs`
|
||||||
|
- Admin API endpoint `GET /api/v2/admin/audit-logs` exists
|
||||||
|
- **Problem**: No code calls `audit_service.log_event()` - logs are always empty
|
||||||
|
|
||||||
|
## Proposed Changes
|
||||||
|
|
||||||
|
### Events to Log
|
||||||
|
|
||||||
|
| Event Type | Category | Location | Description |
|
||||||
|
|------------|----------|----------|-------------|
|
||||||
|
| `auth_login` | authentication | auth.py | User login (success/failure) |
|
||||||
|
| `auth_logout` | authentication | auth.py | User logout |
|
||||||
|
| `auth_token_refresh` | authentication | auth.py | Token refresh |
|
||||||
|
| `task_create` | task | tasks.py | Task created |
|
||||||
|
| `task_process` | task | tasks.py | Task processing started |
|
||||||
|
| `task_complete` | task | tasks.py | Task completed |
|
||||||
|
| `task_delete` | task | tasks.py | Task deleted |
|
||||||
|
| `admin_cleanup` | admin | admin.py | Manual cleanup triggered |
|
||||||
|
| `admin_view_users` | admin | admin.py | Admin viewed user list |
|
||||||
|
| `file_upload` | file | main.py | File uploaded |
|
||||||
|
|
||||||
|
### Implementation Approach
|
||||||
|
1. Add helper function to extract client info (IP, user agent) from Request
|
||||||
|
2. Add `audit_service.log_event()` calls to each operation point
|
||||||
|
3. Ensure all events capture: user_id, IP address, user agent, resource info
|
||||||
|
|
||||||
|
## Non-Goals
|
||||||
|
- Creating new audit log model (already exists)
|
||||||
|
- Changing audit log API endpoints (already work)
|
||||||
|
- Modifying frontend audit logs page (already complete)
|
||||||
|
|
||||||
|
## Affected Specs
|
||||||
|
- None (infrastructure already in place)
|
||||||
|
|
||||||
|
## Testing
|
||||||
|
- Verify audit logs appear after login/logout
|
||||||
|
- Verify task operations are logged
|
||||||
|
- Verify admin actions are logged
|
||||||
|
- Check audit logs page displays new entries
|
||||||
33
openspec/changes/enable-audit-logging/tasks.md
Normal file
33
openspec/changes/enable-audit-logging/tasks.md
Normal file
@@ -0,0 +1,33 @@
|
|||||||
|
# Tasks: Enable Audit Logging
|
||||||
|
|
||||||
|
## 1. Helper Utilities
|
||||||
|
- [x] 1.1 Create helper function to extract client info (IP, user agent) from FastAPI Request
|
||||||
|
|
||||||
|
## 2. Authentication Events
|
||||||
|
- [x] 2.1 Log `auth_login` on successful/failed login in auth.py
|
||||||
|
- [x] 2.2 Log `auth_logout` on logout in auth.py
|
||||||
|
- [ ] 2.3 Log `auth_token_refresh` on token refresh (deferred - low priority)
|
||||||
|
|
||||||
|
## 3. Task Events
|
||||||
|
- [ ] 3.1 Log `task_create` when task is created (deferred - covered by file_upload)
|
||||||
|
- [ ] 3.2 Log `task_process` when task processing starts (deferred - background task)
|
||||||
|
- [ ] 3.3 Log `task_complete` when task completes (deferred - background task)
|
||||||
|
- [x] 3.4 Log `task_delete` when task is deleted
|
||||||
|
|
||||||
|
## 4. Admin Events
|
||||||
|
- [x] 4.1 Log `admin_cleanup` when manual cleanup is triggered
|
||||||
|
- [ ] 4.2 Log `admin_view_users` when admin views user list (deferred - low priority)
|
||||||
|
|
||||||
|
## 5. File Events
|
||||||
|
- [x] 5.1 Log `file_upload` when file is uploaded
|
||||||
|
|
||||||
|
## 6. Testing
|
||||||
|
- [ ] 6.1 Verify login creates audit log entry
|
||||||
|
- [ ] 6.2 Verify task operations create audit log entries
|
||||||
|
- [ ] 6.3 Verify audit logs page shows entries
|
||||||
|
- [x] 6.4 Test backend module imports
|
||||||
|
|
||||||
|
## Notes
|
||||||
|
- Core audit events implemented: login, logout, task delete, file upload, admin cleanup
|
||||||
|
- Background task events (task_process, task_complete) deferred - would require significant refactoring
|
||||||
|
- Low priority admin events deferred for future implementation
|
||||||
Reference in New Issue
Block a user