This commit is contained in:
Iliyan Angelov
2025-12-09 17:07:38 +02:00
parent e43a95eafb
commit 9de9d9701e
14 changed files with 270 additions and 91 deletions

View File

@@ -2,6 +2,7 @@ from fastapi import APIRouter, Depends, HTTPException, status, Cookie, Response,
from fastapi.responses import JSONResponse
from sqlalchemy.orm import Session
from pathlib import Path
from datetime import datetime
import aiofiles
import uuid
import os
@@ -13,6 +14,9 @@ from ...security.middleware.auth import get_current_user
from ..models.user import User
from ...analytics.services.audit_service import audit_service
from ...shared.config.logging_config import get_logger
from ...payments.services.accountant_security_service import accountant_security_service
from ...auth.services.mfa_service import mfa_service
from ...shared.utils.role_helpers import is_admin
from slowapi import Limiter, _rate_limit_exceeded_handler
from slowapi.util import get_remote_address
from slowapi.errors import RateLimitExceeded
@@ -36,6 +40,99 @@ def get_limiter(request: Request) -> Limiter:
limiter = request.app.state.limiter
return limiter
@router.post('/admin/step-up/verify')
async def verify_admin_step_up(
request: Request,
step_up_data: dict,
current_user: User = Depends(get_current_user),
db: Session = Depends(get_db)
):
"""
Step-up verification for admins: accept password or MFA token.
Uses the accountant security session store but bypasses accountant role checks.
"""
if not is_admin(current_user, db):
raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail='Forbidden')
try:
from ...payments.models.accountant_session import AccountantSession
mfa_token = step_up_data.get('mfa_token')
password = step_up_data.get('password')
session_token = step_up_data.get('session_token')
if not session_token:
session_token = request.headers.get('X-Session-Token') or request.cookies.get('session_token')
if not session_token:
active_session = db.query(AccountantSession).filter(
AccountantSession.user_id == current_user.id,
AccountantSession.is_active == True,
AccountantSession.expires_at > datetime.utcnow()
).order_by(AccountantSession.last_activity.desc()).first()
if active_session:
session_token = active_session.session_token
else:
new_session = accountant_security_service.create_session(
db=db,
user_id=current_user.id,
ip_address=request.client.host if request.client else None,
user_agent=request.headers.get('User-Agent')
)
session_token = new_session.session_token
if mfa_token:
try:
is_valid = mfa_service.verify_mfa(db, current_user.id, mfa_token)
if not is_valid:
raise HTTPException(status_code=401, detail='Invalid MFA token')
except ValueError as e:
raise HTTPException(status_code=400, detail=str(e))
elif password:
import bcrypt
if not bcrypt.checkpw(password.encode('utf-8'), current_user.password.encode('utf-8')):
raise HTTPException(status_code=401, detail='Invalid password')
else:
raise HTTPException(status_code=400, detail='Either mfa_token or password is required')
success = accountant_security_service.complete_step_up(
db=db,
session_token=session_token,
user_id=current_user.id
)
if not success:
raise HTTPException(status_code=400, detail='Failed to complete step-up authentication')
client_ip = request.client.host if request.client else None
user_agent = request.headers.get('User-Agent')
accountant_security_service.log_activity(
db=db,
user_id=current_user.id,
activity_type='admin_step_up_authentication',
activity_description='Admin step-up authentication completed',
ip_address=client_ip,
user_agent=user_agent,
risk_level='low',
metadata={'method': 'mfa' if mfa_token else 'password'}
)
db.commit()
return JSONResponse(
status_code=status.HTTP_200_OK,
content={'status': 'success', 'data': {'step_up_completed': True}, 'message': 'Step-up authentication completed successfully'}
)
except HTTPException:
raise
except Exception as e:
db.rollback()
logger.error(f'Error verifying admin step-up: {str(e)}', exc_info=True)
raise HTTPException(status_code=500, detail=str(e))
def get_base_url(request: Request) -> str:
return os.getenv('SERVER_URL') or f'http://{request.headers.get('host', 'localhost:8000')}'
@@ -158,16 +255,16 @@ async def login(
expected_role=login_request.expectedRole
)
# After successful login, check if user is accountant/admin and enforce MFA
# After successful login, check if user is accountant and enforce MFA
requires_mfa_setup = False
if result.get('user') and not result.get('requires_mfa'):
user = db.query(User).filter(User.id == result['user']['id']).first()
if user:
try:
from ...payments.services.accountant_security_service import accountant_security_service
from ...shared.utils.role_helpers import is_accountant, is_admin
from ...shared.utils.role_helpers import is_accountant
if is_accountant(user, db) or is_admin(user, db):
if is_accountant(user, db):
# Check if MFA is required but not enabled
is_enforced, reason = accountant_security_service.is_mfa_enforced(user, db)
if not is_enforced:
@@ -207,7 +304,7 @@ async def login(
db=db,
user_id=user.id,
activity_type='login',
activity_description='Accountant/admin login successful',
activity_description='Accountant login successful',
ip_address=client_ip,
user_agent=user_agent,
risk_level='low',
@@ -255,15 +352,15 @@ async def login(
)
return {'status': 'success', 'requires_mfa': True, 'user_id': result['user_id']}
# After successful login (MFA passed if required), check MFA for accountant/admin roles
# After successful login (MFA passed if required), check MFA for accountant role
if not requires_mfa_setup:
user = db.query(User).filter(User.id == result['user']['id']).first()
if user:
try:
from ...payments.services.accountant_security_service import accountant_security_service
from ...shared.utils.role_helpers import is_accountant, is_admin
from ...shared.utils.role_helpers import is_accountant
if is_accountant(user, db) or is_admin(user, db):
if is_accountant(user, db):
# Check if MFA is required but not enabled
is_enforced, reason = accountant_security_service.is_mfa_enforced(user, db)
if not is_enforced:
@@ -281,36 +378,54 @@ async def login(
status='success'
)
logger.info(f'User {user.id} logged in but MFA setup required: {reason}')
else:
# MFA is enabled and enforced - create accountant session for tracking
try:
accountant_session = accountant_security_service.create_session(
db=db,
user_id=user.id,
ip_address=client_ip,
user_agent=user_agent,
device_fingerprint=None # Can be enhanced with device fingerprinting
)
# Log login activity
is_unusual = accountant_security_service.detect_unusual_activity(
db=db,
user_id=user.id,
ip_address=client_ip
)
accountant_security_service.log_activity(
db=db,
user_id=user.id,
activity_type='login',
activity_description='Accountant/admin login successful',
ip_address=client_ip,
user_agent=user_agent,
risk_level='low',
is_unusual=is_unusual
)
except Exception as e:
logger.warning(f'Error creating accountant session: {e}')
# Always create an accountant security session so step-up auth
# works even if MFA is not yet enabled (password re-auth fallback).
try:
accountant_session = accountant_security_service.create_session(
db=db,
user_id=user.id,
ip_address=client_ip,
user_agent=user_agent,
device_fingerprint=None # Can be enhanced with device fingerprinting
)
# Commit the session to database so it's available for step-up auth
db.commit()
# Store session_token in cookie for step-up authentication
from ...shared.config.settings import settings
session_max_age = 4 * 60 * 60 # 4 hours (matches ACCOUNTANT_SESSION_TIMEOUT_HOURS)
samesite_value = 'strict' if settings.is_production else 'lax'
response.set_cookie(
key='session_token',
value=accountant_session.session_token,
httponly=True,
secure=settings.is_production,
samesite=samesite_value,
max_age=session_max_age,
path='/'
)
# Log login activity
is_unusual = accountant_security_service.detect_unusual_activity(
db=db,
user_id=user.id,
ip_address=client_ip
)
accountant_security_service.log_activity(
db=db,
user_id=user.id,
activity_type='login',
activity_description='Accountant/admin login successful',
ip_address=client_ip,
user_agent=user_agent,
risk_level='low',
is_unusual=is_unusual
)
except Exception as e:
db.rollback()
logger.warning(f'Error creating accountant session: {e}')
except Exception as e:
logger.warning(f'Error enforcing MFA for accountant: {e}')

View File

@@ -5,7 +5,7 @@ from typing import Optional
import bcrypt
from ...shared.config.database import get_db
from ...security.middleware.auth import get_current_user, authorize_roles
from ...security.middleware.step_up_auth import require_step_up_auth
from ...security.middleware.step_up_auth import require_step_up_auth, require_admin_step_up_auth
from ..models.user import User
from ..models.role import Role
from ...bookings.models.booking import Booking, BookingStatus
@@ -57,7 +57,7 @@ async def get_user_by_id(id: int, current_user: User=Depends(authorize_roles('ad
except Exception as e:
raise HTTPException(status_code=500, detail=str(e))
@router.post('/', dependencies=[Depends(authorize_roles('admin')), Depends(require_step_up_auth("user creation"))])
@router.post('/', dependencies=[Depends(authorize_roles('admin')), Depends(require_admin_step_up_auth("user creation"))])
async def create_user(
request: Request,
user_data: CreateUserRequest,
@@ -147,7 +147,7 @@ async def create_user(
db.rollback()
raise HTTPException(status_code=500, detail=str(e))
@router.put('/{id}')
@router.put('/{id}', dependencies=[Depends(authorize_roles('admin')), Depends(require_admin_step_up_auth("user management"))])
async def update_user(
id: int,
request: Request,
@@ -164,28 +164,8 @@ async def update_user(
if not can_manage_users(current_user, db) and current_user.id != id:
raise HTTPException(status_code=403, detail='Forbidden')
# SECURITY: Require step-up auth for admin user management operations
is_admin_managing_user = can_manage_users(current_user, db) and current_user.id != id
if is_admin_managing_user:
# Check if step-up auth is required (this will raise if not authenticated)
from ...payments.services.accountant_security_service import accountant_security_service
session_token = request.headers.get('X-Session-Token') or request.cookies.get('session_token')
requires_step_up, reason = accountant_security_service.require_step_up(
db=db,
user_id=current_user.id,
session_token=session_token,
action_description="user management"
)
if requires_step_up:
from fastapi import status
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail={
'error': 'step_up_required',
'message': reason or 'Step-up authentication required for user management operations',
'action': 'user_management'
}
)
# Step-up is no longer enforced for admins via accountant security.
# If a separate admin step-up is desired, wire it here instead.
user = db.query(User).options(joinedload(User.role)).filter(User.id == id).first()
if not user:

View File

@@ -21,7 +21,7 @@ router = APIRouter(prefix='/accountant/security', tags=['accountant-security'])
async def verify_step_up(
request: Request,
step_up_data: dict,
current_user: User = Depends(authorize_roles('admin', 'accountant')),
current_user: User = Depends(authorize_roles('accountant')),
db: Session = Depends(get_db)
):
"""Verify step-up authentication (MFA token or password re-entry)."""
@@ -36,7 +36,9 @@ async def verify_step_up(
# Try to get from header or cookie
session_token = request.headers.get('X-Session-Token') or request.cookies.get('session_token')
# If still no session token, try to find the most recent active session for this user
# If still no session token, try to find the most recent active session for this user.
# If none exists (e.g., password-only admin without MFA), create a fresh session so
# password-based step-up can proceed without forcing a full re-login.
if not session_token:
active_session = db.query(AccountantSession).filter(
AccountantSession.user_id == current_user.id,
@@ -47,7 +49,13 @@ async def verify_step_up(
if active_session:
session_token = active_session.session_token
else:
raise HTTPException(status_code=400, detail='No active session found. Please log in again.')
new_session = accountant_security_service.create_session(
db=db,
user_id=current_user.id,
ip_address=request.client.host if request.client else None,
user_agent=request.headers.get('User-Agent')
)
session_token = new_session.session_token
# Verify MFA if token provided
if mfa_token:
@@ -107,7 +115,7 @@ async def verify_step_up(
@router.get('/sessions')
async def get_active_sessions(
current_user: User = Depends(authorize_roles('admin', 'accountant')),
current_user: User = Depends(authorize_roles('accountant')),
db: Session = Depends(get_db)
):
"""Get active sessions for current user."""
@@ -143,7 +151,7 @@ async def get_active_sessions(
@router.post('/sessions/{session_id}/revoke')
async def revoke_session(
session_id: int,
current_user: User = Depends(authorize_roles('admin', 'accountant')),
current_user: User = Depends(authorize_roles('accountant')),
db: Session = Depends(get_db)
):
"""Revoke a specific session."""
@@ -172,7 +180,7 @@ async def revoke_session(
@router.post('/sessions/revoke-all')
async def revoke_all_sessions(
current_user: User = Depends(authorize_roles('admin', 'accountant')),
current_user: User = Depends(authorize_roles('accountant')),
db: Session = Depends(get_db)
):
"""Revoke all active sessions for current user."""
@@ -196,7 +204,7 @@ async def get_activity_logs(
limit: int = Query(50, ge=1, le=100),
risk_level: Optional[str] = Query(None),
is_unusual: Optional[bool] = Query(None),
current_user: User = Depends(authorize_roles('admin', 'accountant')),
current_user: User = Depends(authorize_roles('accountant')),
db: Session = Depends(get_db)
):
"""Get activity logs for current user or all users (admin only)."""

View File

@@ -6,7 +6,7 @@ from typing import Optional, Dict, Any, Tuple
from datetime import datetime, timedelta
from ...auth.models.user import User
from ..models.accountant_session import AccountantSession, AccountantActivityLog
from ...shared.utils.role_helpers import is_accountant, is_admin
from ...shared.utils.role_helpers import is_accountant
from ...shared.config.logging_config import get_logger
import secrets
import hashlib
@@ -27,10 +27,8 @@ class AccountantSecurityService:
@staticmethod
def requires_mfa(user: User, db: Session) -> bool:
"""Check if user role requires MFA."""
# Admin and all accountant roles require MFA
if is_admin(user, db) or is_accountant(user, db):
return True
return False
# Only accountant roles are handled by this service
return is_accountant(user, db)
@staticmethod
def is_mfa_enforced(user: User, db: Session) -> Tuple[bool, Optional[str]]:
@@ -40,7 +38,7 @@ class AccountantSecurityService:
"""
if AccountantSecurityService.requires_mfa(user, db):
if not user.mfa_enabled:
return False, "MFA is required for accountant/admin roles but not enabled"
return False, "MFA is required for accountant role but not enabled"
return True, None
return False, None
@@ -120,12 +118,18 @@ class AccountantSecurityService:
db: Session,
user_id: int,
session_token: Optional[str] = None,
action_description: str = "high-risk action"
action_description: str = "high-risk action",
enforce_role_check: bool = True,
) -> Tuple[bool, Optional[str]]:
"""
Check if step-up authentication is required.
Returns (requires_step_up: bool, reason: str | None)
"""
user = db.query(User).filter(User.id == user_id).first()
if enforce_role_check:
# Only enforce step-up for accountant roles when role check is enabled
if user and not is_accountant(user, db):
return False, None
# If no session token provided, try to find the most recent active session for this user
if not session_token:
active_session = db.query(AccountantSession).filter(

View File

@@ -123,7 +123,7 @@ def get_current_user(
detail=f'Account is temporarily locked due to multiple failed login attempts. Please try again in {remaining_minutes} minute(s).'
)
# SECURITY: Check MFA for accountant/admin roles (warn but allow access for MFA setup)
# SECURITY: Check MFA for accountant roles (warn but allow access for MFA setup)
# Note: MFA enforcement for financial endpoints is handled by route-level dependencies
# This check only logs warnings to allow users to access MFA setup pages
try:
@@ -133,7 +133,7 @@ def get_current_user(
logger = get_logger(__name__)
if is_accountant(user, db) or is_admin(user, db):
if is_accountant(user, db):
is_enforced, reason = accountant_security_service.is_mfa_enforced(user, db)
if not is_enforced and reason:
# Log warning but allow access so user can set up MFA

View File

@@ -8,7 +8,7 @@ from ...shared.config.database import get_db
from ...security.middleware.auth import get_current_user
from ...auth.models.user import User
from ...payments.services.accountant_security_service import accountant_security_service
from ...shared.utils.role_helpers import is_accountant, is_admin
from ...shared.utils.role_helpers import is_accountant
from ...shared.config.logging_config import get_logger
logger = get_logger(__name__)
@@ -25,8 +25,8 @@ def require_step_up_auth(
current_user: User = Depends(get_current_user),
db: Session = Depends(get_db)
) -> User:
# Only enforce for accountant/admin roles
if not (is_accountant(current_user, db) or is_admin(current_user, db)):
# Only enforce for accountant roles; admins are handled separately
if not is_accountant(current_user, db):
return current_user # Regular users don't need step-up
# Get session token from request
@@ -60,6 +60,48 @@ def require_step_up_auth(
return step_up_checker
def require_admin_step_up_auth(
action_description: str = "this high-risk admin action"
):
"""
Dependency to require step-up authentication for admin-only high-risk operations.
Uses the same step-up mechanism but bypasses accountant role checks.
"""
async def step_up_checker(
request: Request,
current_user: User = Depends(get_current_user),
db: Session = Depends(get_db)
) -> User:
from ...shared.utils.role_helpers import is_admin
if not is_admin(current_user, db):
return current_user # Only admins are subject to this dependency
session_token = request.headers.get('X-Session-Token') or request.cookies.get('session_token')
requires_step_up, reason = accountant_security_service.require_step_up(
db=db,
user_id=current_user.id,
session_token=session_token,
action_description=action_description,
enforce_role_check=False, # allow admin
)
if requires_step_up:
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail={
'error': 'step_up_required',
'message': reason or f'Step-up authentication required for {action_description}',
'action': action_description
}
)
return current_user
return step_up_checker
def enforce_mfa_for_accountants():
"""
Dependency to enforce MFA for accountant/admin roles.
@@ -69,8 +111,8 @@ def enforce_mfa_for_accountants():
current_user: User = Depends(get_current_user),
db: Session = Depends(get_db)
) -> User:
# Only enforce for accountant/admin roles
if not (is_accountant(current_user, db) or is_admin(current_user, db)):
# Only enforce for accountant roles; admins are handled separately
if not is_accountant(current_user, db):
return current_user # Regular users don't need MFA
# Check if MFA is required and enforced
@@ -131,8 +173,8 @@ def authorize_financial_access(*allowed_roles: str):
detail='You do not have permission to access this resource'
)
# Then enforce MFA for accountant/admin roles
if is_accountant(current_user, db) or is_admin(current_user, db):
# Then enforce MFA only for accountant roles (admins handled elsewhere)
if is_accountant(current_user, db):
is_enforced, reason = accountant_security_service.is_mfa_enforced(current_user, db)
if not is_enforced and reason:

View File

@@ -38,6 +38,7 @@ const StepUpAuthModal: React.FC<StepUpAuthModalProps> = ({
actionDescription = 'this action',
}) => {
const { userInfo } = useAuthStore();
const isAdmin = (userInfo?.role || (userInfo as any)?.role_name)?.toLowerCase() === 'admin';
const [verificationMethod, setVerificationMethod] = useState<'mfa' | 'password'>('mfa');
const [isVerifying, setIsVerifying] = useState(false);
const [error, setError] = useState<string | null>(null);
@@ -92,9 +93,13 @@ const StepUpAuthModal: React.FC<StepUpAuthModalProps> = ({
setIsVerifying(true);
setError(null);
const response = await accountantSecurityService.verifyStepUp({
const response = await (isAdmin
? accountantSecurityService.verifyAdminStepUp({
mfa_token: data.mfaToken,
})
: accountantSecurityService.verifyStepUp({
mfa_token: data.mfaToken,
});
}));
if (response.status === 'success' && response.data.step_up_completed) {
toast.success('Identity verified successfully');
@@ -106,10 +111,16 @@ const StepUpAuthModal: React.FC<StepUpAuthModalProps> = ({
throw new Error('Step-up verification failed');
}
} catch (error: any) {
// Prevent page refresh by ensuring error is caught and handled
const errorMessage =
error.response?.data?.detail || error.response?.data?.message || 'Failed to verify identity. Please try again.';
error.response?.data?.detail ||
(typeof error.response?.data === 'string' ? error.response.data : null) ||
error.response?.data?.message ||
error.message ||
'Failed to verify identity. Please try again.';
setError(errorMessage);
toast.error(errorMessage);
// Don't close modal on error - let user try again
} finally {
setIsVerifying(false);
}
@@ -120,9 +131,13 @@ const StepUpAuthModal: React.FC<StepUpAuthModalProps> = ({
setIsVerifying(true);
setError(null);
const response = await accountantSecurityService.verifyStepUp({
const response = await (isAdmin
? accountantSecurityService.verifyAdminStepUp({
password: data.password,
})
: accountantSecurityService.verifyStepUp({
password: data.password,
});
}));
if (response.status === 'success' && response.data.step_up_completed) {
toast.success('Identity verified successfully');
@@ -134,10 +149,16 @@ const StepUpAuthModal: React.FC<StepUpAuthModalProps> = ({
throw new Error('Step-up verification failed');
}
} catch (error: any) {
// Prevent page refresh by ensuring error is caught and handled
const errorMessage =
error.response?.data?.detail || error.response?.data?.message || 'Invalid password. Please try again.';
error.response?.data?.detail ||
(typeof error.response?.data === 'string' ? error.response.data : null) ||
error.response?.data?.message ||
error.message ||
'Invalid password. Please try again.';
setError(errorMessage);
toast.error(errorMessage);
// Don't close modal on error - let user try again
} finally {
setIsVerifying(false);
}

View File

@@ -45,6 +45,15 @@ class AccountantSecurityService {
return response.data;
}
async verifyAdminStepUp(data: {
mfa_token?: string;
password?: string;
session_token?: string;
}): Promise<{ status: string; data: { step_up_completed: boolean } }> {
const response = await apiClient.post('/auth/admin/step-up/verify', data);
return response.data;
}
async getSessions(): Promise<{ status: string; data: { sessions: AccountantSession[] } }> {
const response = await apiClient.get('/accountant/security/sessions');
return response.data;