Skip to content

Commit 13ee80c

Browse files
wyattwalterclaude
andauthored
fix(security): return uniform responses on pre-auth email endpoints and remove unused routes (#41996)
## What Makes two unauthenticated email endpoints return a consistent response regardless of whether an account exists, and removes two unused public routes. ## Why `POST /users/forgotPassword` and `POST /users/resendEmailVerification` returned different responses for a known vs unknown email (and, for resend, also revealed verified state). This let an unauthenticated caller determine which email addresses have accounts. ## Changes - **forgotPassword** — unknown email and over-limit both return the same generic success (HTTP 200), sending no email. The per-account reset limit is unchanged. - **resendEmailVerification** — unknown, already-verified, and verification-disabled cases all return the same generic success, sending no email. Adds a per-email send limit: over the limit it still returns the generic success (so it can't be used to probe), and it fails open if the limiter is unavailable so legitimate verification emails are never blocked. - Removes two `permitAll` routes (`GET /users/invite/verify`, `PUT /users/invite/confirm`) that have no server handler. ## Testing - Controller + service tests assert identical status and body across known-under-limit / known-over-limit / known-verified / unknown / verification-disabled for both endpoints, plus the silent per-email limit and its fail-open path. - Relevant server module tests pass. Fixes https://linear.app/appsmith/issue/APP-15349/security-medium-user-enumeration-via-forgot-password <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/29493040653> > Commit: 6017e79 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=29493040653&attempt=2" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Thu, 16 Jul 2026 19:50:33 UTC <!-- end of auto-generated comment: Cypress test results --> ## Automation /ok-to-test tags="@tag.All" --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 800cf99 commit 13ee80c

8 files changed

Lines changed: 718 additions & 64 deletions

File tree

app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,8 +218,6 @@ public SecurityWebFilterChain securityWebFilterChain(ServerHttpSecurity http) {
218218
ServerWebExchangeMatchers.pathMatchers(
219219
HttpMethod.GET, USER_URL + "/verifyPasswordResetToken"),
220220
ServerWebExchangeMatchers.pathMatchers(HttpMethod.PUT, USER_URL + "/resetPassword"),
221-
ServerWebExchangeMatchers.pathMatchers(HttpMethod.GET, USER_URL + "/invite/verify"),
222-
ServerWebExchangeMatchers.pathMatchers(HttpMethod.PUT, USER_URL + "/invite/confirm"),
223221
ServerWebExchangeMatchers.pathMatchers(HttpMethod.GET, USER_URL + "/me"),
224222
ServerWebExchangeMatchers.pathMatchers(HttpMethod.GET, ASSET_URL + "/*"),
225223
ServerWebExchangeMatchers.pathMatchers(HttpMethod.GET, ACTION_URL + "/**"),

app/server/appsmith-server/src/main/java/com/appsmith/server/constants/RateLimitConstants.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,7 @@ public class RateLimitConstants {
55
"Your account is suspended for 24 hours. Please reset your password to continue";
66
public static final String BUCKET_KEY_FOR_LOGIN_API = "login";
77
public static final String BUCKET_KEY_FOR_TEST_DATASOURCE_API = "test_datasource_or_execute_query";
8+
9+
// Per-email throttle for the unauthenticated resend-email-verification endpoint (anti-abuse).
10+
public static final String BUCKET_KEY_FOR_RESEND_EMAIL_VERIFICATION_API = "resend_email_verification";
811
}

app/server/appsmith-server/src/main/java/com/appsmith/server/ratelimiting/RateLimitConfig.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ public RateLimitConfig(AbstractRedisClient redisClient) {
3838
apiConfigurationMap.put(
3939
RateLimitConstants.BUCKET_KEY_FOR_TEST_DATASOURCE_API,
4040
createBucketConfiguration(Duration.ofSeconds(5), 3));
41+
// Per-email cap for resend-email-verification: at most 5 requests per email per rolling 24h window.
42+
apiConfigurationMap.put(
43+
RateLimitConstants.BUCKET_KEY_FOR_RESEND_EMAIL_VERIFICATION_API,
44+
createBucketConfiguration(Duration.ofDays(1), 5));
4145
// Add more API configurations as needed
4246
}
4347

app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java

Lines changed: 72 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,12 @@ private Mono<Boolean> processForgotPasswordTokenGeneration(
225225
.findByEmailAndOrganizationId(email, organizationId)
226226
.switchIfEmpty(repository.findFirstByEmailIgnoreCaseAndOrganizationIdOrderByCreatedAtDesc(
227227
email, organizationId))
228-
.switchIfEmpty(Mono.error(
229-
new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.USER, email)))
228+
// Anti-enumeration (CWE-204, GHSA-fvrq-g89c-fgg6): do not surface a distinct
229+
// NO_RESOURCE_FOUND (HTTP 404) for an unknown email. When no user matches, this inner
230+
// Mono simply stays empty; the token save and email dispatch below are skipped and
231+
// thenReturn(true) still emits the same generic success (HTTP 200, {"data": true}) that
232+
// the valid path returns. This makes the response for an unknown email indistinguishable
233+
// from a known one, so an unauthenticated caller cannot enumerate registered accounts.
230234
.flatMap(user -> {
231235
// an user found with the provided email address
232236
// Generate the password reset link for the user
@@ -240,11 +244,22 @@ private Mono<Boolean> processForgotPasswordTokenGeneration(
240244
passwordResetToken.setFirstRequestTime(Instant.now());
241245
return Mono.just(passwordResetToken);
242246
}))
243-
.map(resetToken -> {
244-
// check the validity of the token
245-
validateResetLimit(resetToken);
247+
.flatMap(resetToken -> {
248+
// Track this request and check the per-account reset limit.
249+
// Anti-enumeration (CWE-204, GHSA-fvrq-g89c-fgg6): when a known
250+
// account is over its limit we must NOT surface a distinct 429 —
251+
// that would tell an unauthenticated caller the account exists.
252+
// Instead we complete the chain empty (skipping the token save and
253+
// email dispatch below) so thenReturn(true) emits the same generic
254+
// success returned for the unknown-user and under-limit cases.
255+
// Anti-spam: over the limit we deliberately do not issue a fresh
256+
// reset link or send another email; the request count is still
257+
// tracked so the limit keeps functioning.
258+
if (!isWithinResetLimit(resetToken)) {
259+
return Mono.empty();
260+
}
246261
resetToken.setTokenHash(passwordEncoder.encode(token));
247-
return resetToken;
262+
return Mono.just(resetToken);
248263
});
249264
});
250265
})
@@ -269,28 +284,40 @@ private Mono<Boolean> processForgotPasswordTokenGeneration(
269284
}
270285

271286
/**
272-
* This method checks whether the reset request limit has been exceeded.
273-
* If the limit has been exceeded, it raises an Exception.
274-
* Otherwise, it'll update the counter and date in the resetToken object
287+
* Tracks a password-reset request against the per-account limit (max 3 per rolling 24h window)
288+
* and reports whether the caller is still within that limit.
289+
* <p>
290+
* When under the limit the counter is incremented (and the window start stamped on the first
291+
* request); when the 24h window has elapsed the counter is reset and the request is allowed.
292+
* When the limit is exceeded within the window this returns {@code false} without mutating the
293+
* counter, so the limit keeps functioning across subsequent requests.
294+
* <p>
295+
* This intentionally no longer throws {@link AppsmithError#TOO_MANY_REQUESTS}: on the
296+
* unauthenticated forgot-password endpoint a distinct 429 would reveal that the account exists
297+
* (CWE-204). Callers treat {@code false} as "complete with the generic success response and send
298+
* no email" rather than surfacing a distinguishable error.
275299
*
276300
* @param resetToken {@link PasswordResetToken}
301+
* @return {@code true} if the request is within the limit and a reset email may be sent;
302+
* {@code false} if the account is over its limit and no email should be sent
277303
*/
278-
private void validateResetLimit(PasswordResetToken resetToken) {
304+
private boolean isWithinResetLimit(PasswordResetToken resetToken) {
279305
if (resetToken.getRequestCount() >= 3) {
280306
Duration duration = Duration.between(resetToken.getFirstRequestTime(), Instant.now());
281307
long l = duration.toHours();
282-
if (l >= 24) { // ok, reset the counter
308+
if (l >= 24) { // window elapsed: reset the counter and allow the request
283309
resetToken.setRequestCount(1);
284310
resetToken.setFirstRequestTime(Instant.now());
285-
} else { // too many requests, raise an exception
286-
throw new AppsmithException(AppsmithError.TOO_MANY_REQUESTS);
287-
}
288-
} else {
289-
resetToken.setRequestCount(resetToken.getRequestCount() + 1);
290-
if (resetToken.getFirstRequestTime() == null) {
291-
resetToken.setFirstRequestTime(Instant.now());
311+
return true;
292312
}
313+
// too many requests within the window: over the limit
314+
return false;
315+
}
316+
resetToken.setRequestCount(resetToken.getRequestCount() + 1);
317+
if (resetToken.getFirstRequestTime() == null) {
318+
resetToken.setFirstRequestTime(Instant.now());
293319
}
320+
return true;
294321
}
295322

296323
private Boolean isEmailVerificationTokenValid(EmailVerificationToken emailVerificationToken) {
@@ -878,16 +905,21 @@ private Mono<Boolean> processResendEmailVerification(
878905
email, organizationId)))
879906
.cache();
880907

881-
return userMono.switchIfEmpty(
882-
Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.USER, email)))
883-
.flatMap(user -> {
908+
// Verification-email dispatch chain. Anti-enumeration (CWE-204): the unknown-email,
909+
// already-verified and verification-not-enabled branches all complete empty instead of throwing a
910+
// distinct error (previously 404 / 400), so the thenReturn(true) at the end emits the same generic
911+
// success (HTTP 200, {"data": true}) for every case and no email is dispatched. An unauthenticated
912+
// caller therefore cannot tell whether the address is registered, already verified, or whether
913+
// verification is enabled at the instance level.
914+
Mono<Boolean> dispatchMono = userMono.flatMap(user -> {
884915
if (TRUE.equals(user.getEmailVerified())) {
885-
return Mono.error(new AppsmithException(AppsmithError.USER_ALREADY_VERIFIED));
916+
// Already verified: send no email, but keep the response indistinguishable.
917+
return Mono.<EmailVerificationToken>empty();
886918
}
887919
return instanceVariablesHelper.isEmailVerificationEnabled().flatMap(emailVerificationEnabled -> {
888-
// Email verification not enabled at instance level
920+
// Email verification not enabled at instance level: send no email, response unchanged.
889921
if (!TRUE.equals(emailVerificationEnabled)) {
890-
return Mono.error(new AppsmithException(AppsmithError.EMAIL_VERIFICATION_NOT_ENABLED));
922+
return Mono.<EmailVerificationToken>empty();
891923
}
892924
return emailVerificationTokenRepository
893925
.findByEmail(user.getEmail())
@@ -936,6 +968,22 @@ private Mono<Boolean> processResendEmailVerification(
936968
user, verificationUrl, resendEmailVerificationDTO.getBaseUrl());
937969
})
938970
.thenReturn(true);
971+
972+
// Per-email rate limit (anti-abuse). Applied uniformly on the submitted address so behaviour does
973+
// not vary by account existence. Over the limit we skip dispatch and still return the same generic
974+
// success — surfacing a distinct 429 here would re-introduce an enumeration oracle on this
975+
// unauthenticated endpoint.
976+
return rateLimitService
977+
.tryIncreaseCounter(
978+
RateLimitConstants.BUCKET_KEY_FOR_RESEND_EMAIL_VERIFICATION_API, email.toLowerCase())
979+
.flatMap(withinLimit -> TRUE.equals(withinLimit) ? dispatchMono : Mono.just(true))
980+
// Fail open: if the limiter itself errors (e.g. Redis is unreachable), do not block a
981+
// legitimate verification email. This runs the same dispatch path regardless of account
982+
// state, so it does not reintroduce an enumeration oracle.
983+
.onErrorResume(error -> {
984+
log.error("Resend email verification rate limiter errored; failing open.", error);
985+
return dispatchMono;
986+
});
939987
}
940988

941989
private String getEmailVerificationErrorRedirectUrl(AppsmithError appsmithError, String userEmail, Object... args) {
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
package com.appsmith.server.controllers;
2+
3+
import com.appsmith.server.configurations.RedisTestContainerConfig;
4+
import com.appsmith.server.configurations.SecurityTestConfig;
5+
import com.appsmith.server.helpers.RedisUtils;
6+
import com.appsmith.server.services.UserService;
7+
import org.junit.jupiter.api.Test;
8+
import org.mockito.Mockito;
9+
import org.springframework.beans.factory.annotation.Autowired;
10+
import org.springframework.boot.test.autoconfigure.web.reactive.AutoConfigureWebTestClient;
11+
import org.springframework.boot.test.context.SpringBootTest;
12+
import org.springframework.boot.test.mock.mockito.MockBean;
13+
import org.springframework.context.annotation.Import;
14+
import org.springframework.http.HttpHeaders;
15+
import org.springframework.http.HttpStatus;
16+
import org.springframework.http.MediaType;
17+
import org.springframework.test.web.reactive.server.EntityExchangeResult;
18+
import org.springframework.test.web.reactive.server.WebTestClient;
19+
import org.springframework.web.reactive.function.BodyInserters;
20+
import reactor.core.publisher.Mono;
21+
22+
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
23+
import static org.junit.jupiter.api.Assertions.assertEquals;
24+
import static org.mockito.ArgumentMatchers.any;
25+
26+
/**
27+
* Controller-level anti-enumeration guard (CWE-204, GHSA-fvrq-g89c-fgg6) for the unauthenticated
28+
* {@code POST /api/v1/users/forgotPassword} endpoint.
29+
*
30+
* <p>The property under test is indistinguishability: the HTTP response (status line AND body) must be
31+
* identical whether the email is a known account under its reset limit, a known account over its limit, or
32+
* an unknown address. The full end-to-end rate-limit behaviour (counter accumulation, over-limit, window
33+
* reset) is covered at the service layer by {@code UserServiceTest}; here the service is mocked so the
34+
* controller's own response contract is what is pinned. Post-fix the service yields the same generic
35+
* success ({@code true}) for all three cases, so all three must produce byte-identical HTTP responses. A
36+
* regression that let any case surface a distinct error (e.g. a 404 or 429 from a thrown exception) would
37+
* change that case's status and body and fail this test.
38+
*
39+
* <p>Note on the body: this endpoint is annotated {@code @JsonView(Views.Public.class)} and returns a bare
40+
* {@code Boolean}; under the application's view-based serialization the success body is empty (the client
41+
* only depends on the 200 status). This test therefore asserts the full response is identical across cases
42+
* rather than pinning a particular JSON envelope — identical status + body is the anti-enumeration bar.
43+
*/
44+
@SpringBootTest
45+
@AutoConfigureWebTestClient
46+
@Import({SecurityTestConfig.class, RedisUtils.class, RedisTestContainerConfig.class})
47+
public class UserControllerForgotPasswordTest {
48+
49+
@MockBean
50+
UserService userService;
51+
52+
@Autowired
53+
private WebTestClient webTestClient;
54+
55+
private EntityExchangeResult<byte[]> forgotPassword(String email) {
56+
return webTestClient
57+
.post()
58+
.uri("/api/v1/users/forgotPassword")
59+
.header(HttpHeaders.ORIGIN, "https://my-instance.example.com")
60+
.contentType(MediaType.APPLICATION_JSON)
61+
.body(BodyInserters.fromValue("{\"email\":\"" + email + "\"}"))
62+
.exchange()
63+
.expectBody()
64+
.returnResult();
65+
}
66+
67+
@Test
68+
public void forgotPassword_returnsIdenticalResponse_forKnownUnderLimit_knownOverLimit_andUnknown() {
69+
// Post-fix the service yields the same generic success for every one of these cases; an over-limit
70+
// or unknown email no longer surfaces a distinct error (404 / 429) to the controller.
71+
Mockito.when(userService.forgotPasswordTokenGenerate(any())).thenReturn(Mono.just(true));
72+
73+
EntityExchangeResult<byte[]> knownUnderLimit = forgotPassword("api_user");
74+
EntityExchangeResult<byte[]> knownOverLimit = forgotPassword("api_user");
75+
EntityExchangeResult<byte[]> unknown = forgotPassword("nobody-does-not-exist@example.com");
76+
77+
// All three succeed with HTTP 200 ...
78+
assertEquals(HttpStatus.OK, knownUnderLimit.getStatus());
79+
assertEquals(HttpStatus.OK, knownOverLimit.getStatus());
80+
assertEquals(HttpStatus.OK, unknown.getStatus());
81+
82+
// ... and their bodies are byte-identical, so the caller cannot tell the cases apart.
83+
byte[] under = normalize(knownUnderLimit.getResponseBody());
84+
assertArrayEquals(under, normalize(knownOverLimit.getResponseBody()));
85+
assertArrayEquals(under, normalize(unknown.getResponseBody()));
86+
}
87+
88+
@Test
89+
public void forgotPassword_emptyServiceCompletion_returnsSameSuccessResponse() {
90+
// Base-URL-unresolved path (Root Cause 1): the service completes empty; the controller's
91+
// defaultIfEmpty(true) must still produce the same HTTP 200 success as the value-emitting path.
92+
Mockito.when(userService.forgotPasswordTokenGenerate(any())).thenReturn(Mono.just(true));
93+
EntityExchangeResult<byte[]> emitted = forgotPassword("api_user");
94+
95+
Mockito.when(userService.forgotPasswordTokenGenerate(any())).thenReturn(Mono.empty());
96+
EntityExchangeResult<byte[]> empty = forgotPassword("api_user");
97+
98+
assertEquals(HttpStatus.OK, emitted.getStatus());
99+
assertEquals(HttpStatus.OK, empty.getStatus());
100+
assertArrayEquals(normalize(emitted.getResponseBody()), normalize(empty.getResponseBody()));
101+
}
102+
103+
private static byte[] normalize(byte[] body) {
104+
return body == null ? new byte[0] : body;
105+
}
106+
}

0 commit comments

Comments
 (0)