Implement V3 API security hardening and multi-tenant data isolation
- Enhanced AuthContext with role-aware fields (administrator, manager, super). - Implemented deferred database lookups for user roles in get_v3_auth_context. - Added global account isolation in api_crud_v3.py using check_account_access and apply_forced_account_filter. - Hardened all V3 CRUD endpoints (GET, POST, PATCH, DELETE) and nested routes with ownership verification. - Enforced forced account filtering at the SQL level for Listing and Searching. - Updated documentation with details on the new security and data isolation architecture.
This commit is contained in:
Scott Idem
@@ -22,6 +22,42 @@ from app.db_sql import redis_lookup_id_random, sql_select, sql_insert, sql_updat
|
||||
|
||||
router = APIRouter()
|
||||
|
||||
def check_account_access(sql_result: Any, account: AccountContext, obj_name: str = None) -> bool:
|
||||
"""
|
||||
Verifies if the current account context has access to the given SQL result.
|
||||
Always returns True for super users.
|
||||
"""
|
||||
if account.super or account.auth_method == 'bypass':
|
||||
return True
|
||||
if not account.account_id:
|
||||
return False
|
||||
|
||||
res_account_id = None
|
||||
if isinstance(sql_result, dict):
|
||||
if obj_name == 'account':
|
||||
res_account_id = sql_result.get('id')
|
||||
else:
|
||||
res_account_id = sql_result.get('account_id')
|
||||
|
||||
if res_account_id is not None and res_account_id != account.account_id:
|
||||
return False
|
||||
return True
|
||||
|
||||
def apply_forced_account_filter(and_qry_dict: Optional[Dict], account: AccountContext, model: Any, obj_name: str) -> Dict:
|
||||
"""
|
||||
Adds a forced account_id filter to the query dictionary if the user is not a super user.
|
||||
"""
|
||||
forced = and_qry_dict or {}
|
||||
if account.super or account.auth_method == 'bypass':
|
||||
return forced
|
||||
|
||||
if obj_name == 'account':
|
||||
forced['id'] = account.account_id
|
||||
elif model and hasattr(model, '__fields__') and 'account_id' in model.__fields__:
|
||||
forced['account_id'] = account.account_id
|
||||
|
||||
return forced
|
||||
|
||||
def safe_json_loads(json_str: Optional[str]) -> Any:
|
||||
"""
|
||||
Safely load a JSON string, handling 'undefined' or invalid formats
|
||||
@@ -226,6 +262,10 @@ async def get_obj(
|
||||
return mk_resp(data=False, status_code=404, response=response, status_message=f"Object with ID '{obj_id}' not found.")
|
||||
|
||||
if sql_result := sql_select(table_name=table_name, record_id=record_id):
|
||||
# Security Check
|
||||
if not check_account_access(sql_result, account, obj_name):
|
||||
return mk_resp(data=False, status_code=403, response=response, status_message="Access denied.")
|
||||
|
||||
resp_data = base_name(**sql_result).dict(
|
||||
by_alias=serialization.by_alias,
|
||||
exclude_unset=serialization.exclude_unset,
|
||||
@@ -287,6 +327,11 @@ async def get_obj_li(
|
||||
if obj_name == 'site' and not (for_obj_type == 'account' and for_obj_id):
|
||||
return mk_resp(data=False, status_code=403, response=response, status_message="Listing sites is only permitted when filtered by account.")
|
||||
|
||||
# Explicit Account isolation
|
||||
if for_obj_type == 'account' and for_obj_id:
|
||||
if not account.super and for_obj_id != account.account_id_random:
|
||||
return mk_resp(data=False, status_code=403, response=response, status_message="Access denied to requested account.")
|
||||
|
||||
obj_cfg = obj_type_kv_li[obj_name]
|
||||
table_name = obj_cfg.get(f'tbl_{view}', obj_cfg.get('tbl_default', obj_cfg.get('tbl')))
|
||||
base_name = obj_cfg.get(f'mdl_{view}', obj_cfg.get('mdl_default', obj_cfg.get('mdl')))
|
||||
@@ -296,6 +341,9 @@ async def get_obj_li(
|
||||
|
||||
order_by_li = filter_order_by(order_by_li, base_name, table_name)
|
||||
status_filter = get_supported_filters(base_name, status_filter)
|
||||
|
||||
# Forced account isolation
|
||||
and_qry_dict_obj = apply_forced_account_filter(and_qry_dict_obj, account, base_name, obj_name)
|
||||
|
||||
if for_obj_type and for_obj_id:
|
||||
resolved_for_obj_id = redis_lookup_id_random(record_id_random=for_obj_id, table_name=for_obj_type)
|
||||
@@ -395,6 +443,19 @@ async def search_obj_li(
|
||||
status_filter = get_supported_filters(base_name, status_filter)
|
||||
searchable_fields = obj_cfg.get('searchable_fields')
|
||||
|
||||
# Explicit Account isolation
|
||||
if for_obj_type == 'account' and for_obj_id:
|
||||
if not account.super and for_obj_id != account.account_id_random:
|
||||
return mk_resp(data=False, status_code=403, response=response, status_message="Access denied to requested account.")
|
||||
|
||||
# Forced account isolation for the search query itself
|
||||
# We inject the account_id into the SearchQuery object if it's not a super user
|
||||
if not account.super and account.auth_method != 'bypass' and account.account_id:
|
||||
if obj_name == 'account':
|
||||
search_query.and_filter['id'] = account.account_id
|
||||
elif base_name and hasattr(base_name, '__fields__') and 'account_id' in base_name.__fields__:
|
||||
search_query.and_filter['account_id'] = account.account_id
|
||||
|
||||
if for_obj_type and for_obj_id:
|
||||
resolved_for_obj_id = redis_lookup_id_random(record_id_random=for_obj_id, table_name=for_obj_type)
|
||||
if not resolved_for_obj_id:
|
||||
@@ -474,6 +535,16 @@ async def post_obj(
|
||||
if not table_name_insert or not input_model or not table_name_select or not output_model:
|
||||
return mk_resp(data=False, status_code=500, response=response, status_message=f"Configuration for object type '{obj_name}' is incomplete.")
|
||||
|
||||
# Security: Force account_id for non-super users if model supports it
|
||||
if not account.super and account.auth_method != 'bypass' and account.account_id:
|
||||
if 'account_id' in input_model.__fields__:
|
||||
obj_data['account_id'] = account.account_id
|
||||
elif obj_name == 'account':
|
||||
# Users can't create accounts unless they are super (usually)
|
||||
# But if they can, it should be their own ID?
|
||||
# Actually, account creation is usually a super-admin task.
|
||||
return mk_resp(data=False, status_code=403, response=response, status_message="Account creation is restricted to super administrators.")
|
||||
|
||||
try:
|
||||
validated_obj = input_model(**obj_data)
|
||||
except Exception as e:
|
||||
@@ -537,6 +608,13 @@ async def patch_obj(
|
||||
if not record_id:
|
||||
return mk_resp(data=False, status_code=404, response=response, status_message=f"Object with ID '{obj_id}' not found.")
|
||||
|
||||
# Security Check
|
||||
if existing_obj := sql_select(table_name=table_name_select, record_id=record_id):
|
||||
if not check_account_access(existing_obj, account, obj_name):
|
||||
return mk_resp(data=False, status_code=403, response=response, status_message="Access denied.")
|
||||
else:
|
||||
return mk_resp(data=False, status_code=404, response=response, status_message=f"Object with ID '{obj_id}' not found in database.")
|
||||
|
||||
data_to_update = obj_data
|
||||
|
||||
if sql_update_result := sql_update(data=data_to_update, table_name=table_name_update, record_id=record_id):
|
||||
@@ -576,6 +654,7 @@ async def delete_obj(
|
||||
|
||||
obj_cfg = obj_type_kv_li[obj_name]
|
||||
table_name_delete = obj_cfg.get('tbl_update', obj_cfg.get('tbl'))
|
||||
table_name_select = obj_cfg.get('tbl_default', obj_cfg.get('tbl'))
|
||||
|
||||
if not table_name_delete:
|
||||
return mk_resp(data=False, status_code=500, response=response, status_message=f"Configuration for object type '{obj_name}' is incomplete.")
|
||||
@@ -584,6 +663,13 @@ async def delete_obj(
|
||||
if not record_id:
|
||||
return mk_resp(data=False, status_code=404, response=response, status_message=f"Object with ID '{obj_id}' not found.")
|
||||
|
||||
# Security Check
|
||||
if existing_obj := sql_select(table_name=table_name_select, record_id=record_id):
|
||||
if not check_account_access(existing_obj, account, obj_name):
|
||||
return mk_resp(data=False, status_code=403, response=response, status_message="Access denied.")
|
||||
else:
|
||||
return mk_resp(data=False, status_code=404, response=response, status_message=f"Object with ID '{obj_id}' not found in database.")
|
||||
|
||||
if method == 'hide':
|
||||
if sql_update(table_name=table_name_delete, record_id=record_id, data={'hide': True}):
|
||||
return mk_resp(data=True, response=response, status_message=f"Object with ID '{obj_id}' hidden successfully.")
|
||||
@@ -645,8 +731,8 @@ async def get_child_obj_li(
|
||||
order_by_li = safe_json_loads(order_by_li)
|
||||
|
||||
obj_name = child_obj_type
|
||||
if obj_name not in obj_type_kv_li:
|
||||
return mk_resp(data=False, status_code=400, response=response, status_message=f"Object type '{obj_name}' not found.")
|
||||
if obj_name not in obj_type_kv_li or parent_obj_type not in obj_type_kv_li:
|
||||
return mk_resp(data=False, status_code=400, response=response, status_message=f"Invalid object type(s).")
|
||||
|
||||
obj_cfg = obj_type_kv_li[obj_name]
|
||||
table_name = obj_cfg.get('tbl_default', obj_cfg.get('tbl'))
|
||||
@@ -662,6 +748,18 @@ async def get_child_obj_li(
|
||||
if not resolved_parent_id:
|
||||
return mk_resp(data=False, status_code=404, response=response, status_message=f"Parent object '{parent_obj_type}' with ID '{parent_obj_id}' not found.")
|
||||
|
||||
# Parent Security Check
|
||||
parent_cfg = obj_type_kv_li[parent_obj_type]
|
||||
parent_table = parent_cfg.get('tbl_default', parent_cfg.get('tbl'))
|
||||
if parent_sql_res := sql_select(table_name=parent_table, record_id=resolved_parent_id):
|
||||
if not check_account_access(parent_sql_res, account, parent_obj_type):
|
||||
return mk_resp(data=False, status_code=403, response=response, status_message="Access denied to parent object.")
|
||||
else:
|
||||
return mk_resp(data=False, status_code=404, response=response, status_message="Parent object not found.")
|
||||
|
||||
# Child security: Forced account isolation if child supports it
|
||||
and_qry_dict_obj = apply_forced_account_filter(and_qry_dict_obj, account, base_name, obj_name)
|
||||
|
||||
sql_result = sql_select(
|
||||
table_name=table_name,
|
||||
field_name=f'{parent_obj_type}_id',
|
||||
@@ -724,12 +822,26 @@ async def post_child_obj(
|
||||
if not resolved_parent_id:
|
||||
return mk_resp(data=False, status_code=404, response=response, status_message="Parent not found.")
|
||||
|
||||
# Parent Security Check
|
||||
parent_cfg = obj_type_kv_li[parent_obj_type]
|
||||
parent_table = parent_cfg.get('tbl_default', parent_cfg.get('tbl'))
|
||||
if parent_sql_res := sql_select(table_name=parent_table, record_id=resolved_parent_id):
|
||||
if not check_account_access(parent_sql_res, account, parent_obj_type):
|
||||
return mk_resp(data=False, status_code=403, response=response, status_message="Access denied to parent object.")
|
||||
else:
|
||||
return mk_resp(data=False, status_code=404, response=response, status_message="Parent object not found.")
|
||||
|
||||
obj_cfg = obj_type_kv_li[child_obj_type]
|
||||
table_name_insert = obj_cfg.get('tbl_update', obj_cfg.get('tbl'))
|
||||
table_name_select = obj_cfg.get('tbl_default', obj_cfg.get('tbl'))
|
||||
input_model = obj_cfg.get('mdl_in', obj_cfg.get('mdl'))
|
||||
output_model = obj_cfg.get('mdl_out', obj_cfg.get('mdl_default', obj_cfg.get('mdl')))
|
||||
|
||||
# Security: Force account_id for non-super users if child supports it
|
||||
if not account.super and account.auth_method != 'bypass' and account.account_id:
|
||||
if 'account_id' in input_model.__fields__:
|
||||
obj_data['account_id'] = account.account_id
|
||||
|
||||
obj_data[f'{parent_obj_type}_id'] = resolved_parent_id
|
||||
|
||||
try:
|
||||
|
||||
Reference in New Issue
Block a user