fix(auth): return 503 when Clerk verify omits API key identity

[detail-app]

Detail bug report: View on Detail

Bug

ClerkApiKeyVerifier.verify translates every malformed Clerk Backend API reply into a clean application outcome — Optional.empty() for rejected/revoked/expired keys, ApiKeyOperationUnavailableException (→ HTTP 503) for empty responses, incomplete lifecycle state, transport failures, and non-credential HTTP errors. The one case it failed to translate was a 200 whose identity fields (id, subject) were null or blank: it passed those straight into new VerifiedApiKey(...), whose record invariant throws IllegalArgumentException. That throw happens outside verify's try/catch (which only wraps the REST call), so the exception escaped ClerkApiKeyAuthenticationFilter (which catches only ApiKeyOperationUnavailableException), reached CustomErrorController → ExceptionResponseBuilder.describeException, and the client received HTTP 500 with body {"status":"error","message":"Internal Server Error","details":"IllegalArgumentException"} — a crash-coded status leaking the internal exception class name. The structurally parallel missing-revoked/expired case was already a 503; only the identity fields were omitted from that treatment.

Fix

Add an identity-field guard in ClerkApiKeyVerifier.verify, mirroring the existing lifecycle-field guard: when a 200 has null/blank id or subject, throw ApiKeyOperationUnavailableException("Clerk returned an incomplete API key identity") before constructing VerifiedApiKey. The filter's existing catch then returns the consistent 503 with the human message and no exception class name.

The VerifiedApiKey record's IllegalArgumentException invariant is intentionally left unchanged (correct value-object design); the defect was verify's exception flow at the provider boundary, not the record's guard. ClerkApiKeyAuthenticationFilter is also unchanged — its 503 mapping for ApiKeyOperationUnavailableException was already correct.

Testing

• New ClerkApiKeyVerifierTest.rejectsIncompleteIdentity (parametrized, 5 cases: missing subject, missing id, both missing, blank subject, blank id) drives the real verifier against a MockRestServiceServer malformed 200 and asserts ApiKeyOperationUnavailableException — mirroring the sibling rejectsIncompleteLifecycleState. It fails on the unfixed code and passes on the fixed code.
• New AuthenticatedUserEndpointSecurityIntegrationTest.unavailableApiKeyVerificationReturns503AndLogsOneFailure runs the real Spring Security filter chain over MockMvc (mocked verifier throws ApiKeyOperationUnavailableException) and asserts HTTP 503, the fixed human message, exactly one ERROR log at the filter logger, and that the ak_… secret is absent from the log — mirroring the existing revocation-503 test for the previously-untested verify path.
• Routine checks all pass: ./gradlew compileJava compileTestJava, the targeted test classes (18 + 10, 0 failures), the full non-integration JVM suite (1169 tests, 0 failures), spotlessCheck, and pmdMain/pmdTest.
• End-to-end against a real Tomcat (jar run with Java 25 from the Gradle toolchain; CLERK_SECRET_KEY intentionally unset so verify throws the ApiKeyOperationUnavailableException family): anonymous GET /api/me → 401; GET /api/me and DELETE /api/me/api-key with a Bearer ak_… token → 503 with {"status":"error","message":"API key verification is temporarily unavailable. Please retry.","details":null} (no IllegalArgumentException leak); the server log showed one ERROR line per request at ClerkApiKeyAuthenticationFilter with the ApiKeyOperationUnavailableException cause.
• CLI smoke: node bin/javachat.js auth status with a JAVACHAT_API_KEY against the local server surfaced Key verification failed: HTTP 503 (not 500), matching the committed CLI test that asserts this 503 surfacing. CLI unit tests (npm test, 43/43) and npm run pack:check pass.
• Could not run: the malformed-identity 200 branch specifically and the control happy-path (well-formed 200 → 200 with userId) against live external services. They require a real CLERK_SECRET_KEY and a key whose Clerk verify reply is well-formed, or a controllable Clerk stand-in — but CLERK_VERIFY_ENDPOINT is a hardcoded constant with no override, and this environment has neither. Both branches are pinned by the committed MockRestServiceServer/@MockitoBean tests above, which are green.

Closes #223

---

Automatic Fixes PRs can be configured here.