Return uniform 200 response for new and existing API key registrations#110
Conversation
The /api_key/:email endpoint previously returned HTTP 200 for new accounts and HTTP 409 for existing ones. This difference acts as a user enumeration oracle — a caller can determine whether any email address is registered with DPLA. Return 200 with the same body in both cases to close that gap.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR consolidates API key response handling so both new and existing API key outcomes return the same "Your API key has been sent to $email." message and a 200 OK; tests and a mock Postgres client were updated accordingly. ChangesAPI key response unification
sequenceDiagram
participant Client
participant Routes
participant Registry
Client->>Routes: POST /api_key/<email>
Routes->>Registry: request API key (new or existing)
Registry-->>Routes: NewApiKey(email) / ExistingApiKey(email)
Routes-->>Client: apiKeyMessage(email) (200 OK)
🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs:
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/test/scala/dpla/api/v2/endToEnd/PostgresErrorTest.scala`:
- Around line 47-56: The test PostgresErrorTest has a flaky assertion: the
request uses Post("/v2/api_key/email@example.com") but the
apiKeyRegistryExistingKey mock returns x@example.org, so the expected string
"Your API key has been sent to email@example.com." is wrong; fix by making the
test derive the request email and the expected response from a single variable
(e.g., val testEmail = "email@example.com") and use that in the Post call and
the entityAs assertion, or alternatively modify the apiKeyRegistryExistingKey
mock to echo the requested email so Routes.applicationRoutes (constructed via
new Routes(..., apiKeyRegistryExistingKey, ...)) produces deterministic output
matching the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9d0961c9-972e-4830-9631-1e06fbf5b188
📒 Files selected for processing (1)
src/test/scala/dpla/api/v2/endToEnd/PostgresErrorTest.scala
Summary
The
POST /api_key/:emailendpoint currently returns HTTP 200 when registering a new account and HTTP 409 when the email already exists. This difference is a user enumeration oracle — any caller can determine whether an arbitrary email address is registered with DPLA by observing the status code.This PR closes that gap by returning HTTP 200 with the same response body (
"Your API key has been sent to $email.") for both the new-account and existing-account cases.Changes
Routes.scala:ExistingApiKeynow callsapiKeyMessageinstead ofexistingKeyResponse(which returned 409 Conflict)existingKeyResponsemethod removed (no longer referenced)newKeyMessagerenamed toapiKeyMessagewith neutral wording ("Your API key has been sent to..." rather than "API key created and sent to...")Background
Discovered during a scanner investigation (2026-05-12): an automated scanner was calling
POST /api_key/at scale against lists of email addresses — including real@openai.comaddresses — and using the 200 vs 409 response to enumerate registered users. The uniform response removes the signal without affecting any legitimate use of the endpoint.The
DisabledApiKey(409) path is unchanged — disabled-account handling is a separate concern.Security Fix: User Enumeration Prevention
This PR fixes a user-enumeration vulnerability in the API key registration endpoint. Previously POST /api_key/:email returned 200 for new accounts and 409 Conflict for existing accounts, allowing attackers to distinguish registered emails by status code.
Changes
Files changed (high-level):
Impact Notes