perf: stop the audit middleware from re-running the whole auth pipeline (#27373)

With audit logging enabled, every audited request authenticated twice. The route dependency resolved the user once, and then _log_audit_entry called get_current_user again in the request's finally block: a second JWT decode, two more Redis revocation lookups, a second user row fetch with pydantic validation and, crucially, a second fire-and-forget last-active write transaction per request.

get_current_user now stashes the resolved user on the scope-backed request state (the same mechanism the auth middleware already uses for request.state.token), and the audit middleware reuses it, falling back to the old resolution only when no user was stashed (e.g. routes without an auth dependency). While in the file, the audit path patterns are compiled once in the constructor instead of per request, and the always-log endpoint set is a class attribute instead of a per-call literal; both are fixed for the process lifetime.

Benchmark:

| metric | before | after |
| --- | --- | --- |
| audit auth resolution, CPU floor (JWT decode + user validate only) | 16.7 us | 0.24 us |
| extra work per audited request | 2 Redis GETs + 1 user SELECT + 1 last-active write | none |

The before column understates the saving: it excludes the Redis and DB round trips listed in the second row, which dominate in real deployments.

Functionally verified with a stacked ASGI harness: when the route resolves a user the audit entry carries that user and the auth pipeline is not invoked again; without a stashed user the fallback path still resolves and logs correctly; the skip matrix (exclusions, whitelist mode, always-log auth endpoints, unauthenticated and non-audited methods) is unchanged.
This commit is contained in:
Classic298
2026-07-23 18:09:04 -05:00
committed by GitHub
parent 699d512e2f
commit e0918ddb40
2 changed files with 31 additions and 13 deletions
+26 -13
View File
@@ -140,6 +140,15 @@ class AuditLoggingMiddleware:
self.audited_methods.add('GET')
self.audit_level = audit_level
# Paths are fixed for the process lifetime; compile once instead of
# per request. None means the corresponding mode has nothing to match.
self._included_pattern = (
re.compile(r'^/api(?:/v1)?/(' + '|'.join(self.included_paths) + r')\b') if self.included_paths else None
)
self._excluded_pattern = (
re.compile(r'^/api(?:/v1)?/(' + '|'.join(self.excluded_paths) + r')\b') if self.excluded_paths else None
)
if self.included_paths and self.excluded_paths:
logger.warning(
'Both AUDIT_INCLUDED_PATHS and AUDIT_EXCLUDED_PATHS are set. '
@@ -196,6 +205,13 @@ class AuditLoggingMiddleware:
await self._log_audit_entry(request, context)
async def _get_authenticated_user(self, request: Request) -> Optional[UserModel]:
# get_current_user stashes the resolved user on the scope-backed state;
# reuse it instead of running the full auth pipeline (JWT decode, Redis
# revocation checks, DB fetch, last-active write) a second time.
user = getattr(request.state, 'user', None)
if isinstance(user, UserModel):
return user
auth_header = request.headers.get('Authorization')
try:
@@ -206,6 +222,12 @@ class AuditLoggingMiddleware:
return None
ALWAYS_LOG_ENDPOINTS = (
'/api/v1/auths/signin',
'/api/v1/auths/signout',
'/api/v1/auths/signup',
)
def _should_skip_auditing(self, request: Request) -> bool:
if AUDIT_LOG_LEVEL == 'NONE':
return True
@@ -213,13 +235,8 @@ class AuditLoggingMiddleware:
if request.method not in self.audited_methods:
return True
ALWAYS_LOG_ENDPOINTS = {
'/api/v1/auths/signin',
'/api/v1/auths/signout',
'/api/v1/auths/signup',
}
path = request.url.path.lower()
for endpoint in ALWAYS_LOG_ENDPOINTS:
for endpoint in self.ALWAYS_LOG_ENDPOINTS:
if path.startswith(endpoint):
return False # Do NOT skip logging for auth endpoints
@@ -229,15 +246,11 @@ class AuditLoggingMiddleware:
return True
# Whitelist mode: only log paths that match included_paths
if self.included_paths:
pattern = re.compile(r'^/api(?:/v1)?/(' + '|'.join(self.included_paths) + r')\b')
if not pattern.match(request.url.path):
return True # Skip: path not in whitelist
return False # Do NOT skip: path is in whitelist
if self._included_pattern:
return not self._included_pattern.match(request.url.path)
# Blacklist mode: skip paths that match excluded_paths
pattern = re.compile(r'^/api(?:/v1)?/(' + '|'.join(self.excluded_paths) + r')\b')
if pattern.match(request.url.path):
if self._excluded_pattern and self._excluded_pattern.match(request.url.path):
return True
return False
+5
View File
@@ -356,6 +356,8 @@ async def get_current_user(
current_span.set_attribute('client.user.role', user.role)
current_span.set_attribute('client.auth.type', 'api_key')
# Scope-backed, so outer middleware (audit) can reuse the resolved user
request.state.user = user
return user
# auth by jwt token
@@ -404,6 +406,9 @@ async def get_current_user(
# Refresh the user's last active timestamp
# Fire-and-forget via asyncio.create_task to avoid blocking
asyncio.create_task(Users.update_last_active_by_id(user.id))
# Scope-backed, so outer middleware (audit) can reuse the resolved user
request.state.user = user
return user
else:
raise HTTPException(