Fix isort failure in mod_api/__init__.py

Author: pulk17

migrations/versions/d4f8e2a1b3c7_.py

@@ -0,0 +1,44 @@
+"""Add api_token table for scoped API token auth.
+
+Revision ID: d4f8e2a1b3c7
+Revises: c8f3a2b1d4e5
+Create Date: 2026-06-11 03:00:00.000000
+
+"""
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = 'd4f8e2a1b3c7'
+down_revision = 'c8f3a2b1d4e5'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    """Apply the migration."""
+    op.add_column('user', sa.Column('github_login', sa.String(length=255), nullable=True))
+    op.create_table(
+        'api_token',
+        sa.Column('id', sa.Integer(), nullable=False, autoincrement=True),
+        sa.Column('user_id', sa.Integer(), nullable=False),
+        sa.Column('token_name', sa.String(length=50), nullable=False),
+        sa.Column('token_hash', sa.String(length=255), nullable=False),
+        sa.Column('token_prefix', sa.String(length=16), nullable=False),
+        sa.Column('scopes_json', sa.Text(), nullable=False),
+        sa.Column('created_at', sa.DateTime(timezone=True), nullable=False),
+        sa.Column('expires_at', sa.DateTime(timezone=True), nullable=False),
+        sa.Column('revoked_at', sa.DateTime(timezone=True), nullable=True),
+        sa.PrimaryKeyConstraint('id'),
+        sa.ForeignKeyConstraint(['user_id'], ['user.id'], onupdate='CASCADE', ondelete='CASCADE'),
+        sa.UniqueConstraint('user_id', 'token_name', name='uq_user_token_name'),
+        mysql_engine='InnoDB'
+    )
+    op.create_index('ix_api_token_token_prefix', 'api_token', ['token_prefix'])
+
+
+def downgrade():
+    """Revert the migration."""
+    op.drop_index('ix_api_token_token_prefix', table_name='api_token')
+    op.drop_table('api_token')
+    op.drop_column('user', 'github_login')

mod_api/__init__.py

@@ -9,13 +9,28 @@
 
 mod_api = Blueprint('api', __name__)
 
-# Middleware (registers before_request hooks and error handlers)
-# WARNING: auth must be imported before rate_limit. The auth middleware
-# manually calls check_rate_limit() for unauthenticated paths. If
-# rate_limit is imported first, its before_request hook fires first and
-# the auth middleware's manual call would double-count requests.
-from mod_api.middleware import auth  # noqa: E402, F401
-from mod_api.middleware import error_handler  # noqa: E402, F401
-from mod_api.middleware import rate_limit  # noqa: E402, F401
-from mod_api.middleware import security  # noqa: E402, F401
+# Middleware imports
+from mod_api.middleware import auth  # noqa: E402
+from mod_api.middleware import error_handler  # noqa: E402
+from mod_api.middleware import rate_limit  # noqa: E402
+from mod_api.middleware import security  # noqa: E402
+
+# Explicitly register before_request hooks in the exact order they should run
+mod_api.before_request(auth.authenticate_request)
+mod_api.before_request(rate_limit.check_rate_limit)
+mod_api.before_request(auth.enforce_auth_error)
+
+# Explicitly register after_request hooks.
+# NOTE: Flask executes after_request hooks in REVERSE registration order.
+# Registration:  security → rate_limit → (convert is app-level, see below)
+# Execution:     rate_limit → security
+# This means rate-limit headers are added first, then security headers layer
+# on top — both on the same response object.
+mod_api.after_request(security.add_security_headers)
+mod_api.after_request(rate_limit.add_rate_limit_headers)
+
+# Registered as after_app_request so it fires for ALL requests (including
+# routing-level 404s/405s that never enter the blueprint).
+mod_api.after_app_request(error_handler.convert_api_errors_to_json)
+
 # Route modules will be imported in subsequent PRs.

mod_api/middleware/auth.py

@@ -15,7 +15,6 @@
 
 from flask import g, request
 
-from mod_api import mod_api
 from mod_api.middleware.error_handler import make_error_response
 from mod_api.models.api_token import ApiToken
 
@@ -30,41 +29,47 @@
 
 def _unauthorized():
     """Shorthand for a 401 response with the standard auth failure message."""
-    from mod_api.middleware.rate_limit import check_rate_limit
-    rate_limit_resp = check_rate_limit()
-    if rate_limit_resp:
-        return rate_limit_resp
-
     return make_error_response(
         'unauthorized', _AUTH_FAILED_MSG, http_status=401)
 
 
-@mod_api.before_request
 def authenticate_request():
-    """Validate Bearer token and attach user context to the request."""
+    """Validate Bearer token and attach user context to the request.
+
+    If auth fails, sets g.auth_error instead of returning immediately,
+    so that subsequent hooks (like rate limiting) still run.
+    """
     if request.endpoint in _PUBLIC_ENDPOINTS:
         g.api_user = None
         g.api_token = None
         return
 
     auth_header = request.headers.get('Authorization', '')
     if not auth_header:
-        return _unauthorized()
+        g.auth_error = _unauthorized()
+        return
 
     parts = auth_header.split(' ', 1)
     if len(parts) != 2 or parts[0] != 'Bearer':
-        return _unauthorized()
+        g.auth_error = _unauthorized()
+        return
 
     token_value = parts[1].strip()
     if not token_value or not token_value.startswith('spci_'):
-        return _unauthorized()
+        g.auth_error = _unauthorized()
+        return
 
     # Look up by prefix, then verify the full hash against each candidate.
     prefix = ApiToken.extract_prefix(token_value)
     candidates = ApiToken.query.filter_by(token_prefix=prefix).all()
 
     if not candidates:
-        return _unauthorized()
+        # Dummy verification to prevent timing attacks on non-existent tokens
+        ApiToken.verify_token(
+            'dummy',
+            '$argon2id$v=19$m=65536,t=3,p=4$ZHVtbXlfc2FsdF9mb3JfdGltaW5n$A1H8jT2lJ1t5fX9gK0rX4M')
+        g.auth_error = _unauthorized()
+        return
 
     matched_token = None
     for candidate in candidates:
@@ -73,15 +78,23 @@ def authenticate_request():
             break
 
     if matched_token is None:
-        return _unauthorized()
+        g.auth_error = _unauthorized()
+        return
 
     if not matched_token.is_valid:
-        return _unauthorized()
+        g.auth_error = _unauthorized()
+        return
 
     g.api_token = matched_token
     g.api_user = matched_token.user
 
 
+def enforce_auth_error():
+    """Return any stored auth errors after rate limiting."""
+    if hasattr(g, 'auth_error') and g.auth_error is not None:
+        return g.auth_error
+
+
 def require_scope(*scopes: str):
     """Reject the request if the token lacks any of the ``scopes``."""
     def decorator(f):

mod_api/middleware/error_handler.py

@@ -1,6 +1,6 @@
 """Structured JSON error responses for API routes."""
 
-from flask import jsonify, make_response, request
+from flask import current_app, jsonify, request
 from marshmallow import ValidationError as MarshmallowValidationError
 from sqlalchemy.exc import SQLAlchemyError
 
@@ -101,6 +101,7 @@ def handle_429(error):
 @mod_api.errorhandler(500)
 def handle_500(error):
     """Handle unexpected server errors for API routes."""
+    current_app.logger.exception(error)
     return make_error_response(
         'internal_error',
         'An unexpected error occurred.',
@@ -122,27 +123,42 @@ def handle_marshmallow_validation_error(error):
 @mod_api.errorhandler(SQLAlchemyError)
 def handle_sqlalchemy_error(error):
     """Log database errors."""
-    from flask import g
-    log = getattr(g, 'log', None)
-    if log:
-        log.error(f'Database error in API: {type(error).__name__}')
+    current_app.logger.exception(error)
     return make_error_response(
         'internal_error',
         'An unexpected database error occurred.',
         http_status=500,
     )
 
 
-@mod_api.after_app_request
+@mod_api.errorhandler(ValueError)
+def handle_value_error(error):
+    """Catch plain ValueErrors raised by model @validates (e.g. scopes_json)."""
+    return make_error_response(
+        'invalid_input',
+        str(error),
+        http_status=400,
+    )
+
+
 def convert_api_errors_to_json(response):
     """Catch routing errors that were handled by global app handlers and convert them to JSON."""
     if request.path.startswith(_API_PREFIX):
         if response.status_code >= 500:
-            return make_error_response(
+            new_resp = make_error_response(
                 'internal_error', 'An unexpected error occurred.', http_status=response.status_code
             )
+            response.data = new_resp.data
+            response.mimetype = new_resp.mimetype
+            return response
         if response.status_code == 404:
-            return make_error_response('not_found', 'Resource not found.', http_status=404)
+            new_resp = make_error_response('not_found', 'Resource not found.', http_status=404)
+            response.data = new_resp.data
+            response.mimetype = new_resp.mimetype
+            return response
         if response.status_code == 405:
-            return make_error_response('method_not_allowed', 'Method not allowed.', http_status=405)
+            new_resp = make_error_response('method_not_allowed', 'Method not allowed.', http_status=405)
+            response.data = new_resp.data
+            response.mimetype = new_resp.mimetype
+            return response
     return response

mod_api/middleware/rate_limit.py

@@ -17,9 +17,9 @@
 import threading
 import time
 
-from flask import g, request
+from flask import current_app, g, request
 
-from mod_api import mod_api
+from mod_api.middleware.error_handler import make_error_response
 
 _rate_limit_store = {}  # key -> {'count': int, 'window_start': float}
 _rate_limit_lock = threading.Lock()
@@ -45,7 +45,7 @@ def _evict_stale_entries():
 
 
 def _get_client_ip():
-    """Extract the real client IP, ignoring X-Forwarded-For to prevent spoofing."""
+    """Extract the real client IP (ProxyFix handles X-Forwarded-For securely)."""
     return request.remote_addr
 
 
@@ -68,10 +68,8 @@ def _get_limits():
     return 120, 60
 
 
-@mod_api.before_request
 def check_rate_limit():
-    """Reject the request if the client has exceeded their rate limit."""
-    from flask import current_app
+    """Apply rate limits based on client IP or API token."""
     if current_app.config.get('TESTING'):
         return
 
@@ -92,8 +90,6 @@ def check_rate_limit():
                 reset_at = int(entry['window_start'] + window_seconds)
                 retry_after = max(1, reset_at - int(now))
 
-                from mod_api.middleware.error_handler import \
-                    make_error_response
                 response = make_error_response(
                     'rate_limited',
                     f'Rate limit exceeded. Retry after {retry_after} seconds.',
@@ -111,11 +107,9 @@ def check_rate_limit():
                 return response
 
 
-@mod_api.after_request
 def add_rate_limit_headers(response):
-    """Attach X-RateLimit-* headers to every response."""
-    from flask import current_app
-    if current_app.config.get('TESTING'):
+    """Inject X-RateLimit-* headers based on the current window."""
+    if current_app.config.get('TESTING') or response.status_code == 429:
         return response
 
     key = _get_rate_limit_key()

mod_api/middleware/security.py

@@ -1,7 +1,6 @@
-from mod_api import mod_api
+"""Security headers middleware for API responses."""
 
 
-@mod_api.after_request
 def add_security_headers(response):
     """Attach security headers to all API responses."""
     response.headers['Strict-Transport-Security'] = 'max-age=31536000; includeSubDomains'