fix: prevent user enumeration on password recovery endpoint

[anurag6569201]

What does this PR do?

This PR fixes a user enumeration vulnerability in the POST /v1/account/recovery endpoint.

Previously, the endpoint threw a 404 (Exception::USER_NOT_FOUND) for unregistered emails and a 401 (Exception::USER_BLOCKED) for blocked users. This allowed malicious actors to enumerate registered and active users via an unauthenticated endpoint.

This fix standardizes the behavior so that the endpoint always returns a 201 Created with an identical response payload structure regardless of whether the account exists, is blocked, or is active.

Key Changes:

• Removed the exceptions for USER_NOT_FOUND and USER_BLOCKED.
• For invalid/blocked requests, we generate realistic mock values (ID::unique()) for $id, userId, and userInternalId to ensure the response payload perfectly mirrors a successful request.
• Ensure the mock token is immediately deleted and the event/mail queues handle the mock data without leaking information.
• Maintains an identical database operation timing profile for invalid users to mitigate timing attacks.

Test Plan

Added comprehensive E2E tests in tests/e2e/Services/Account/AccountCustomClientTest.php to explicitly verify that the enumeration vulnerabilities are closed:

1. Unregistered User: Verified that sending a recovery request for a non-existent email (not-found@localhost.test) returns a 201 Created and a valid mock payload structure with no secret or phrase.
2. Blocked User: Created a user, updated their status to blocked, and verified that a recovery request for their email successfully returns a 201 Created with a valid mock payload.

To verify locally:

docker compose exec appwrite test tests/e2e/Services/Account/AccountCustomClientTest.php --filter=testCreateRecovery

Related PRs and Issues

• Fixes 🐛 Bug Report: account.createRecovery / POST to /account/recovery allows user enumeration appwrite/appwrite#13435

Checklist

• Have you read the Contributing Guidelines on issues?
• If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

Source merge-base: 50861bafc095b5409801bd34f165a714b5703154
Source head: 3197e0ba5d2a018b55c8ff3999db585355457dc1

[shipwright-agent]

⛔ Shipwright · Blocked

Recommendation: do not merge PR #5 · Tier T1
Checks: 0 total · 0 needing attention

Next step: resolve the blocking findings before merge.

Findings (8)

• CRITICAL For non-deliverable cases, the code creates a recovery token with a random userId and userInternalId, then returns it in the response. · app/controllers/api/account.php:3891
  • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
• CRITICAL The recovery endpoint now returns 201 with a valid token ID and userId for unknown or blocked emails, enabling unauthenticated account enumeration. · app/controllers/api/account.php:3891
  • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
• HIGH The recovery token is created and returned to the client even when the email is not deliverable (unknown or blocked user). · app/controllers/api/account.php:3891
  • Fix: Fix the review finding before release.
• HIGH The test asserts that the response body contains a non-empty userId for a blocked user, but the code sets userId to a random ID for non-deliverable cases. · tests/e2e/Services/Account/AccountCustomClientTest.php:1945
  • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
• HIGH The event queue is still populated with a userId and tokenId for non-deliverable cases, and setUser is called with an empty Document. · app/controllers/api/account.php:4045
  • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
• MEDIUM For non-deliverable cases, the code creates a recovery token with a random userId and userInternalId, but then returns this token in the response. · app/controllers/api/account.php:3891
  • Fix: Fix the review finding before release.
• MEDIUM The event queue is still populated with a userId and tokenId for non-deliverable cases, and setUser is called with an empty Document. · app/controllers/api/account.php:4045
  • Fix: Fix the review finding before release.
• LOW The test asserts that the response body contains a non-empty userId for a blocked user, but the code sets userId to a random ID for non-deliverable cases. · tests/e2e/Services/Account/AccountCustomClientTest.php:1945
  • Fix: Fix the review finding before release.

Fireworks usage: 9,413 input · 846 output · 10,259 total tokens · $0.0026 · 12s · 0 fix iteration(s)

Open the Shipwright check for full evidence and the audit bundle. Use /shipwright rerun to verify again.

[shipwright-agent]

Shipwright · HIGH

The test asserts that the response body contains a non-empty userId for a blocked user, but the code sets userId to a random ID for non-deliverable cases.

Impact: The test asserts that the response body contains a non-empty userId for a blocked user, but the code sets userId to a random ID for non-deliverable cases. This test would pass even if the userId is random, but it does not verify that the userId corresponds to the blocked user, which may mask the security issue.

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

[shipwright-agent]

Shipwright · LOW

The test asserts that the response body contains a non-empty userId for a blocked user, but the code sets userId to a random ID for non-deliverable cases.

Impact: The test asserts that the response body contains a non-empty userId for a blocked user, but the code sets userId to a random ID for non-deliverable cases. This test would pass even if the userId is random, but it does not verify that the userId corresponds to the blocked user, which may mask the security issue.

Suggested fix: Fix the review finding before release.