security(v3): harden multi-tenant isolation and enhance failure feedback
This commit is contained in:
@@ -110,7 +110,7 @@ async def get_obj(
|
||||
obj_type_l1: str = Path(min_length=2, max_length=50),
|
||||
obj_id: str = Path(min_length=11, max_length=22),
|
||||
view: str = Query('default'),
|
||||
account: AccountContext = Depends(get_account_context),
|
||||
account: AccountContext = Depends(get_account_context_optional),
|
||||
serialization: SerializationParams = Depends(),
|
||||
delay: DelayParams = Depends(),
|
||||
):
|
||||
@@ -143,8 +143,13 @@ async def get_obj(
|
||||
|
||||
if sql_result := sql_select(table_name=table_name, record_id=record_id):
|
||||
if not obj_cfg.get('public_read', False):
|
||||
# Strict context check for non-public objects
|
||||
if account.auth_method == 'guest' or (account.account_id is None and not account.super):
|
||||
reason = account.auth_error or "Account context required."
|
||||
return mk_resp(data=False, status_code=403, response=response, status_message=reason)
|
||||
|
||||
if not check_account_access(sql_result, account, obj_name):
|
||||
return mk_resp(data=False, status_code=403, response=response, status_message="Access denied.")
|
||||
return mk_resp(data=False, status_code=403, response=response, status_message="Access denied. Record belongs to another account.")
|
||||
resp_data = base_name(**sql_result).dict(by_alias=serialization.by_alias, exclude_unset=serialization.exclude_unset, exclude_defaults=serialization.exclude_defaults, exclude_none=serialization.exclude_none)
|
||||
return mk_resp(data=resp_data, response=response)
|
||||
else:
|
||||
@@ -334,7 +339,7 @@ async def search_obj_li(
|
||||
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.")
|
||||
|
||||
if not account.super and account.auth_method != 'bypass' and account.account_id:
|
||||
if not is_public_read and not account.super and account.auth_method != 'bypass':
|
||||
if search_query.and_filters is None: search_query.and_filters = []
|
||||
if obj_name == 'account':
|
||||
search_query.and_filters.append(SearchFilter(field='id', op='eq', value=account.account_id))
|
||||
|
||||
@@ -38,6 +38,7 @@ def get_account_context_optional(
|
||||
resolved_token_payload = None
|
||||
auth_method = 'guest'
|
||||
api_key_authorized = False
|
||||
auth_error = None
|
||||
|
||||
# 1. Mandatory Machine Auth (API Key)
|
||||
# Prefer header, fallback to query param
|
||||
@@ -56,16 +57,22 @@ def get_account_context_optional(
|
||||
if (not enable_from or enable_from <= now) and (not enable_to or now <= enable_to):
|
||||
api_key_authorized = True
|
||||
else:
|
||||
log.error(f"Security: API Key {key_to_check} expired/not yet valid.")
|
||||
auth_error = "API Key expired or not yet valid."
|
||||
log.error(f"Security: {auth_error} Key: {key_to_check}")
|
||||
else:
|
||||
log.error(f"Security: API Key {key_to_check} is disabled.")
|
||||
auth_error = "API Key is disabled."
|
||||
log.error(f"Security: {auth_error} Key: {key_to_check}")
|
||||
else:
|
||||
log.error(f"Security: API Key {key_to_check} not found.")
|
||||
auth_error = "API Key not found or invalid."
|
||||
log.error(f"Security: {auth_error} Key: {key_to_check}")
|
||||
else:
|
||||
auth_error = "Mandatory API Key missing."
|
||||
|
||||
# 2. Context Resolution (Only if API Key is valid)
|
||||
if api_key_authorized:
|
||||
# Default to machine auth if no account context is provided
|
||||
auth_method = 'api_key'
|
||||
auth_error = "Account context required for this operation."
|
||||
|
||||
# A. Resolve via Account ID Header
|
||||
if x_account_id:
|
||||
@@ -73,6 +80,9 @@ def get_account_context_optional(
|
||||
if looked_up_id := redis_lookup_id_random(table_name='account', record_id_random=x_account_id):
|
||||
resolved_account_id = looked_up_id
|
||||
auth_method = 'account_header'
|
||||
auth_error = None
|
||||
else:
|
||||
auth_error = f"Account ID '{x_account_id}' not found."
|
||||
|
||||
# B. Resolve via JWT / Token Query Param
|
||||
elif x_no_account_id_token:
|
||||
@@ -88,23 +98,34 @@ def get_account_context_optional(
|
||||
if looked_up_id := redis_lookup_id_random(table_name='account', record_id_random=resolved_account_id_random):
|
||||
resolved_account_id = looked_up_id
|
||||
auth_method = 'jwt_token'
|
||||
auth_error = None
|
||||
else:
|
||||
auth_error = f"Account ID '{resolved_account_id_random}' from token not found."
|
||||
else:
|
||||
# JWT is valid but has no account_id (e.g. platform-wide guest)
|
||||
# We keep auth_method as 'jwt_token' but account_id as None.
|
||||
auth_method = 'jwt_token'
|
||||
auth_error = "Valid token provided, but no account context found in payload."
|
||||
else:
|
||||
log.warning("Security: Failed to decode JWT token.")
|
||||
auth_error = "Failed to decode JWT token."
|
||||
log.warning(f"Security: {auth_error}")
|
||||
|
||||
# Legacy Fallback (just a raw random ID string)
|
||||
if auth_method in ['guest', 'api_key']:
|
||||
resolved_account_id_random = x_no_account_id_token
|
||||
if auth_method in ['guest', 'api_key', 'jwt_token'] and auth_error:
|
||||
if looked_up_id := redis_lookup_id_random(table_name='account', record_id_random=x_no_account_id_token):
|
||||
resolved_account_id = looked_up_id
|
||||
resolved_account_id_random = x_no_account_id_token
|
||||
auth_method = 'token_query'
|
||||
auth_error = None
|
||||
|
||||
# C. Resolve via Administrative Bypass
|
||||
elif x_no_account_id and x_no_account_id.lower() not in ['false', '0', 'null', 'undefined', 'none', 'no_account_id_here']:
|
||||
resolved_account_id = 1
|
||||
resolved_account_id_random = '--- NO ACCOUNT ---'
|
||||
auth_method = 'bypass'
|
||||
auth_error = None
|
||||
|
||||
log.info(f"V3 Auth: method={auth_method}, authorized={api_key_authorized}, account={resolved_account_id_random}")
|
||||
log.info(f"V3 Auth: method={auth_method}, authorized={api_key_authorized}, account={resolved_account_id_random}, error={auth_error}")
|
||||
|
||||
is_admin = (auth_method == 'bypass')
|
||||
is_manager = (auth_method == 'bypass')
|
||||
@@ -122,7 +143,8 @@ def get_account_context_optional(
|
||||
administrator=is_admin,
|
||||
manager=is_manager,
|
||||
super=is_super,
|
||||
token_payload=resolved_token_payload
|
||||
token_payload=resolved_token_payload,
|
||||
auth_error=auth_error
|
||||
)
|
||||
|
||||
def get_account_context(
|
||||
@@ -134,8 +156,9 @@ def get_account_context(
|
||||
) -> AccountContext:
|
||||
"""Strict version of account context resolution."""
|
||||
ctx = get_account_context_optional(x_account_id, x_no_account_id, x_no_account_id_token, x_aether_api_key, x_aether_api_key_query)
|
||||
if ctx.auth_method == 'guest':
|
||||
raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail='Account context required.')
|
||||
if ctx.auth_method == 'guest' or (ctx.account_id is None and not ctx.super):
|
||||
reason = ctx.auth_error or "Account context required."
|
||||
raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=reason)
|
||||
return ctx
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user