updates
This commit is contained in:
@@ -123,6 +123,34 @@ 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)
|
||||
try:
|
||||
from ...payments.services.accountant_security_service import accountant_security_service
|
||||
from ...shared.utils.role_helpers import is_accountant, is_admin
|
||||
from ...shared.config.logging_config import get_logger
|
||||
|
||||
logger = get_logger(__name__)
|
||||
|
||||
if is_accountant(user, db) or is_admin(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
|
||||
# Individual routes can enforce MFA for sensitive operations
|
||||
logger.warning(
|
||||
f'User {user.id} ({user.email}) accessed system without MFA enabled. '
|
||||
f'MFA is required for {user.role.name if user.role else "unknown"} role. '
|
||||
f'Reason: {reason}'
|
||||
)
|
||||
# Store MFA requirement in user object for route-level checks
|
||||
user._mfa_setup_required = True
|
||||
user._mfa_setup_reason = reason
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
# Don't block authentication if MFA check fails (log and continue)
|
||||
logger = get_logger(__name__)
|
||||
logger.warning(f'Error checking MFA enforcement: {str(e)}')
|
||||
|
||||
return user
|
||||
|
||||
def authorize_roles(*allowed_roles: str):
|
||||
@@ -130,6 +158,8 @@ def authorize_roles(*allowed_roles: str):
|
||||
def role_checker(current_user: User=Depends(get_current_user), db: Session=Depends(get_db)) -> User:
|
||||
# PERFORMANCE: Use eager-loaded relationship if available, otherwise query
|
||||
# This reduces database queries since get_current_user now eager loads the role
|
||||
from ...shared.utils.role_helpers import get_user_role_name
|
||||
|
||||
if hasattr(current_user, 'role') and current_user.role is not None:
|
||||
user_role_name = current_user.role.name
|
||||
else:
|
||||
@@ -139,8 +169,21 @@ def authorize_roles(*allowed_roles: str):
|
||||
raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail='User role not found')
|
||||
user_role_name = role.name
|
||||
|
||||
# Backwards‑compatible support for accountant sub‑roles:
|
||||
# any role starting with "accountant_" is treated as "accountant"
|
||||
# when a route allows plain "accountant".
|
||||
if user_role_name not in allowed_roles:
|
||||
raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail='You do not have permission to access this resource')
|
||||
# Normalise accountant_* → accountant for role checks
|
||||
if (
|
||||
user_role_name.startswith("accountant_")
|
||||
and "accountant" in allowed_roles
|
||||
):
|
||||
# treated as allowed
|
||||
return current_user
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail='You do not have permission to access this resource'
|
||||
)
|
||||
return current_user
|
||||
return role_checker
|
||||
|
||||
|
||||
Reference in New Issue
Block a user