Overly broad exception handling in API key authentication

[jaffarkeikei]

Description

Found in backend/metering_billing/auth/auth_utils.py: The get_organization_from_key function catches broad Exception instead of specific exceptions, which can mask unexpected errors and make debugging difficult.

Current Code

def get_organization_from_key(key):
    try:
        api_key = APIToken.objects.get_from_key(key)
    except Exception:  # ❌ Too broad
        raise NoMatchingAPIKey(f"API Key starting with {key[:5]} not known")
    organization = api_key.organization
    return organization

Problem

1. Masks unexpected errors: Catches ALL exceptions (syntax errors, attribute errors, database errors, etc.)
2. Makes debugging harder: Legitimate bugs are converted to "API Key not found" errors
3. Misleading error messages: A database connection error becomes "API key not known"
4. Security implications: Could hide rate limiting, database issues, or other critical problems
5. Violates Python best practices: Should catch specific exceptions only

Example of Hidden Errors

If there's a database connection issue:

# Database connection fails → OperationalError
# Current code: Returns "API Key starting with 12345 not known"
# Should return: "Database connection error" or retry

If there's a bug in the model:

# AttributeError in get_from_key → caught and hidden
# Current code: Returns "API Key not known"
# Should return: The actual AttributeError for debugging

Recommended Fix

Catch only the specific exception(s) that indicate "key not found":

from django.core.exceptions import ObjectDoesNotExist
# Or whatever specific exception get_from_key raises

def get_organization_from_key(key):
    try:
        api_key = APIToken.objects.get_from_key(key)
    except (ObjectDoesNotExist, ValueError, KeyError) as e:  # ✅ Specific exceptions
        raise NoMatchingAPIKey(f"API Key starting with {key[:5]} not known") from e
    except Exception as e:
        # Log unexpected errors but don't hide them
        logger.error(f"Unexpected error during API key lookup: {e}")
        raise  # Re-raise to surface the real issue
    
    organization = api_key.organization
    return organization

Alternative (if uncertain about specific exceptions):

def get_organization_from_key(key):
    try:
        api_key = APIToken.objects.get_from_key(key)
    except Exception as e:
        # Check if it's an expected "not found" scenario
        if "not found" in str(e).lower() or isinstance(e, (ObjectDoesNotExist, ValueError)):
            raise NoMatchingAPIKey(f"API Key starting with {key[:5]} not known") from e
        else:
            # Unexpected error - log and re-raise
            logger.error(f"Unexpected error in get_organization_from_key: {e}", exc_info=True)
            raise
    
    organization = api_key.organization
    return organization

Impact

• Severity: Medium
• Makes debugging authentication issues significantly harder
• Could hide critical infrastructure problems
• Common pattern in the codebase (found in multiple files via grep)

Related Code

Similar overly broad exception handling found in:

• backend/metering_billing/tasks.py:496
• backend/metering_billing/demos.py (multiple locations)
• Various migration files

Consider auditing all bare except Exception: blocks.

Location

backend/metering_billing/auth/auth_utils.py, lines 18-21

References

• Python Best Practices - Don't catch Exception
• PEP 8 - Programming Recommendations