Skip to content

[Detail Bug] API: /api/me/api-key returns CSRF error message for non-CSRF authorization failures #229

Description

@detail-app

Detail Bug Report

https://app.detail.dev/org_befd6425-a158-4e24-9d4d-1e5c08769515/bugs/bug_97494ad8-106b-4830-bcf7-aa6cca922c7d

Introduced in #10 by @WilliamAGH on Jan 31, 2026

Summary

  • Context: Spring's ExceptionTranslationFilter therefore routes every AccessDeniedException for an authenticated caller to this CSRF-specific handler, including denials thrown from controllers for non-CSRF reasons.
  • Bug: SecurityConfig.appSecurityFilterChain wires CsrfAccessDeniedHandler — a handler whose own Javadoc and constant name state it is for CSRF failures only — as the single AccessDeniedHandler for the whole app chain (.exceptionHandling(exceptions -> exceptions.accessDeniedHandler(accessDeniedHandler))). Spring's ExceptionTranslationFilter therefore routes every AccessDeniedException for an authenticated caller to this CSRF-specific handler, including denials thrown from controllers for non-CSRF reasons. The handler, in turn, always emits {"status":"error","message":"CSRF token missing or invalid. Refresh the page and retry the request.","details":null} with HTTP 403.
  • Actual vs. expected: An authenticated Clerk session-JWT caller (valid CSRF token) of DELETE /api/me/api-key receives a 403 body blaming a missing/invalid CSRF token. The expected response is a 403 reporting the real authorization reason ("API key identity is required for revocation"), or at least a generic forbidden error that does not falsely claim a CSRF failure.
  • Impact: Latent — a wrong-error-message / handler-misrouting defect with zero in-tree production callers today. The wiring is wrong: CsrfAccessDeniedHandler installed as the global AccessDeniedHandler misreports any non-CSRF AccessDeniedException thrown for an authenticated caller. But no production request in this codebase produces the JwtAuthenticationToken required to reach the throwing branch of revokeCurrentApiKey, because DefaultBearerTokenResolver (the only configured bearer resolver, SecurityConfig.java:166) reads the Authorization header, a form access_token, and a query access_token — never cookies — while the SPA carries its Clerk session in an http-only __session cookie and attaches no Authorization: Bearer header. The CLI's ak_... Bearer is intercepted upstream by ClerkApiKeyAuthenticationFilter (added before BearerTokenAuthenticationFilter, SecurityConfig.java:158-160) and explicitly excluded from the resource-server resolver (SecurityConfig.java:168-170), authenticating as ClerkApiKeyAuthenticationToken and taking the 204 success path. Demonstrated user-facing impact today is exactly nil; the misreporting manifests only for a manually hand-crafted Authorization: Bearer <clerk-session-jwt> request that no in-tree client issues. The bug is reportable because the wiring is a real, fixable defect that will misreport the moment any future client authenticates to this endpoint with a non-ak_ Bearer token (e.g. once @EnableMethodSecurity is enabled and a JWT Bearer header is sent — see Triggering conditions below).

Code with Bug

src/main/java/com/williamcallahan/javachat/config/SecurityConfig.java

CsrfAccessDeniedHandler accessDeniedHandler = new CsrfAccessDeniedHandler(objectMapper);   // CSRF-specific handler

http.cors(...).csrf(...)
        .exceptionHandling(exceptions -> exceptions.accessDeniedHandler(accessDeniedHandler))   // <-- BUG 🔴 installed as the catch-all for EVERY AccessDeniedException
        .authorizeHttpRequests(auth -> ...);

src/main/java/com/williamcallahan/javachat/adapters/in/web/security/CsrfAccessDeniedHandler.java

@Override
public void handle(HttpServletRequest httpRequest, HttpServletResponse httpResponse,
        AccessDeniedException accessDeniedException) throws IOException, ServletException {
    if (httpResponse.isCommitted()) { return; }
    ApiErrorResponse csrfError = ApiErrorResponse.error(CSRF_INVALID_MESSAGE);   // <-- BUG 🔴 fixed CSRF message even for non-CSRF denials
    httpResponse.setStatus(HttpStatus.FORBIDDEN.value());
    httpResponse.setContentType(MediaType.APPLICATION_JSON_VALUE);
    objectMapper.writeValue(httpResponse.getOutputStream(), csrfError);
}

Example non-CSRF denial that gets misrouted:

src/main/java/com/williamcallahan/javachat/web/AuthenticatedUserController.java

if (!(authentication instanceof ClerkApiKeyAuthenticationToken clerkApiKey)) {
    throw new AccessDeniedException("API key identity is required for revocation");   // <-- BUG 🔴 real reason discarded by CSRF handler
}

Explanation

Spring Security routes AccessDeniedException for authenticated callers to the configured AccessDeniedHandler. Because SecurityConfig installs CsrfAccessDeniedHandler as the global handler, any authenticated, non-CSRF access denial (including ones thrown from controllers) is serialized as a CSRF failure. CsrfAccessDeniedHandler always returns a hard-coded CSRF message and ignores the exception cause, so the real authorization error text is never returned.

Codebase Inconsistency

CsrfAccessDeniedHandler’s own Javadoc states it is CSRF-specific ("Returns JSON 403 responses with clear invalid-CSRF messaging for API callers."), but it is installed as the app-wide AccessDeniedHandler.

Recommended Fix

Install a composed AccessDeniedHandler in SecurityConfig that routes CSRF exceptions to CsrfAccessDeniedHandler and all other AccessDeniedExceptions to a generic 403 JSON handler (so non-CSRF denials no longer claim CSRF failure).

History

This bug was introduced in commit 4f7f9ae. The commit (feat(security): expire CSRF tokens after 15 minutes, 2026-01-30) created CsrfAccessDeniedHandler and, in the same diff, wired it as the global .exceptionHandling(exceptions -> exceptions.accessDeniedHandler(accessDeniedHandler)) for the entire app filter chain — the parent commit had no accessDeniedHandler wired at all, so this is the true origin of the misrouting. The bug slipped in because the change's stated intent was narrowly CSRF-focused ("Return JSON 403 with expired vs missing/invalid messaging … Wire custom access denied handler into security config"), so the author installed a CSRF-specific handler as a catch-all AccessDeniedHandler without scoping it to MissingCsrfTokenException/InvalidCsrfTokenException; any future non-CSRF AccessDeniedException for an authenticated caller (which arrived seven months later in abcacee8's controller throw, 2026-08-21) would inherit the false "CSRF token missing or invalid" message. A later refactor, 287def4c (2026-07-13, "keep CSRF protection stateless"), only simplified the handler's messaging and removed the expired-token branch — it left the global wiring untouched and did not change the introducing commit.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions