Skip to content

Commit ad9ae53

Browse files
subrata71subrata71
committed
fix: account takeover vulnerability
1 parent 3788dee commit ad9ae53

4 files changed

Lines changed: 210 additions & 7 deletions

File tree

‎app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/UserController.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import com.appsmith.server.constants.Url;
44
import com.appsmith.server.controllers.ce.UserControllerCE;
5+
import com.appsmith.server.helpers.HostUrlHelper;
56
import com.appsmith.server.services.SessionUserService;
67
import com.appsmith.server.services.UserDataService;
78
import com.appsmith.server.services.UserService;
@@ -20,6 +21,7 @@ public class UserController extends UserControllerCE {
2021
public UserController(
2122
UserService service,
2223
SessionUserService sessionUserService,
24+
HostUrlHelper hostUrlHelper,
2325
UserWorkspaceService userWorkspaceService,
2426
UserSignup userSignup,
2527
UserDataService userDataService,
@@ -28,6 +30,7 @@ public UserController(
2830
super(
2931
service,
3032
sessionUserService,
33+
hostUrlHelper,
3134
userWorkspaceService,
3235
userSignup,
3336
userDataService,

‎app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/UserControllerCE.java‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import com.appsmith.server.dtos.ResponseDTO;
1111
import com.appsmith.server.dtos.UserProfileDTO;
1212
import com.appsmith.server.dtos.UserUpdateDTO;
13+
import com.appsmith.server.helpers.HostUrlHelper;
1314
import com.appsmith.server.services.SessionUserService;
1415
import com.appsmith.server.services.UserDataService;
1516
import com.appsmith.server.services.UserService;
@@ -47,6 +48,7 @@ public class UserControllerCE {
4748

4849
private final UserService service;
4950
private final SessionUserService sessionUserService;
51+
private final HostUrlHelper hostUrlHelper;
5052
private final UserWorkspaceService userWorkspaceService;
5153
private final UserSignup userSignup;
5254
private final UserDataService userDataService;
@@ -82,15 +84,16 @@ public Mono<ResponseDTO<User>> leaveWorkspace(@PathVariable String workspaceId)
8284
}
8385

8486
/**
85-
* This function initiates the process to reset a user's password. We require the Origin header from the request
86-
* in order to construct client facing URLs that will be sent to the user over email.
87-
* @param originHeader The Origin header in the request. This is a mandatory parameter.
87+
* This function initiates the process to reset a user's password. The client-facing base URL is derived from the
88+
* current request context to construct URLs that will be sent to the user over email.
8889
*/
8990
@JsonView(Views.Public.class)
9091
@PostMapping("/forgotPassword")
9192
public Mono<ResponseDTO<Boolean>> forgotPasswordRequest(
92-
@RequestBody ResetUserPasswordDTO userPasswordDTO, @RequestHeader("Origin") String originHeader) {
93-
userPasswordDTO.setBaseUrl(originHeader);
93+
@RequestBody ResetUserPasswordDTO userPasswordDTO,
94+
@RequestHeader("Origin") String originHeader,
95+
ServerWebExchange exchange) {
96+
userPasswordDTO.setBaseUrl(hostUrlHelper.getClientFacingBaseUrl(originHeader, exchange));
9497
// We shouldn't leak information on whether this operation was successful or not to the client. This can enable
9598
// username scraping, where the response of this API can prove whether an email has an account or not.
9699
return service.forgotPasswordTokenGenerate(userPasswordDTO)
@@ -189,8 +192,9 @@ public Mono<ResponseDTO<Map<String, Boolean>>> getFeatureFlags() {
189192
@PostMapping("/resendEmailVerification")
190193
public Mono<ResponseDTO<Boolean>> resendEmailVerification(
191194
@RequestBody ResendEmailVerificationDTO resendEmailVerificationDTO,
192-
@RequestHeader("Origin") String originHeader) {
193-
resendEmailVerificationDTO.setBaseUrl(originHeader);
195+
@RequestHeader("Origin") String originHeader,
196+
ServerWebExchange exchange) {
197+
resendEmailVerificationDTO.setBaseUrl(hostUrlHelper.getClientFacingBaseUrl(originHeader, exchange));
194198
return service.resendEmailVerification(resendEmailVerificationDTO, null)
195199
.thenReturn(new ResponseDTO<>(HttpStatus.OK, true));
196200
}

‎app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/HostUrlHelper.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package com.appsmith.server.helpers;
2+
3+
import com.appsmith.server.helpers.ce.HostUrlHelperCE;
4+
import lombok.extern.slf4j.Slf4j;
5+
import org.springframework.stereotype.Component;
6+
7+
@Slf4j
8+
@Component
9+
public class HostUrlHelper extends HostUrlHelperCE {}

‎app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/HostUrlHelperCE.java‎

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
package com.appsmith.server.helpers.ce;
2+
3+
import lombok.extern.slf4j.Slf4j;
4+
import org.springframework.beans.factory.annotation.Value;
5+
import org.springframework.http.HttpHeaders;
6+
import org.springframework.http.server.reactive.ServerHttpRequest;
7+
import org.springframework.util.StringUtils;
8+
import org.springframework.web.server.ServerWebExchange;
9+
10+
import static com.google.common.net.HttpHeaders.FORWARDED;
11+
import static com.google.common.net.HttpHeaders.X_FORWARDED_HOST;
12+
import static com.google.common.net.HttpHeaders.X_FORWARDED_PROTO;
13+
14+
@Slf4j
15+
public class HostUrlHelperCE {
16+
17+
@Value("${appsmith.domain:}")
18+
private String configuredRedirectDomain;
19+
20+
@Value("${APPSMITH_BASE_URL:}")
21+
private String configuredBaseUrl;
22+
23+
public static final String APPSMITH_FORWARDED_HOST = "appsmith-forwarded-host";
24+
public static final String APPSMITH_FORWARDED_PROTO = "appsmith-forwarded-proto";
25+
26+
/**
27+
* Resolve the client facing base URL for the current request.
28+
* <p>
29+
* The method first prefers the {@code APPSMITH_BASE_URL} environment variable to ensure that an operator supplied
30+
* canonical URL is always respected. When that variable is not configured, it derives the host from the incoming
31+
* request metadata (forwarded headers, host header etc). The {@code Origin} header is deliberately ignored for
32+
* deriving the base URL because it is supplied by the client and can be trivially spoofed. Instead, we only log the
33+
* {@code Origin} value when it disagrees with the derived host to aid troubleshooting.
34+
*
35+
* @param originInHeader origin header value supplied by the client, used solely for logging discrepancies
36+
* @param exchange current request exchange used to derive request based host details
37+
* @return canonical base URL that should be used when communicating with the client. Returns an empty string when
38+
* it cannot be determined.
39+
*/
40+
public String getClientFacingBaseUrl(String originInHeader, ServerWebExchange exchange) {
41+
String derivedHost;
42+
43+
if (StringUtils.hasText(configuredBaseUrl)) {
44+
derivedHost = sanitizeConfiguredDomain(configuredBaseUrl, exchange);
45+
} else {
46+
derivedHost = getHostUrl(exchange.getRequest());
47+
}
48+
49+
if (!StringUtils.hasText(derivedHost)) {
50+
log.warn("Unable to determine client facing host from configured base URL or request headers");
51+
derivedHost = "";
52+
}
53+
54+
// Log mismatch only when both values are present and not equal
55+
if (StringUtils.hasText(originInHeader)
56+
&& StringUtils.hasText(derivedHost)
57+
&& !originInHeader.equalsIgnoreCase(derivedHost)) {
58+
log.warn(
59+
"Origin header and derived host mismatch; origin in header: {}, derived host: {}",
60+
originInHeader,
61+
derivedHost);
62+
}
63+
64+
return derivedHost;
65+
}
66+
67+
public static String getHostUrl(ServerHttpRequest request) {
68+
HttpHeaders headers = request.getHeaders();
69+
String hostUrl = null;
70+
71+
String query = request.getURI().getQuery();
72+
// Check for appsmith-forwarded parameters in query string
73+
// This is done so that we only check for the query parameter when the request is for the OAuth2 login
74+
if (request.getURI().toString().contains("/login/oauth2/code/google") && query != null && !query.isEmpty()) {
75+
String hostFromQuery = getHostUrlFromQuery(query);
76+
if (hostFromQuery != null) {
77+
log.info("Using host URL from query parameters: {}", hostFromQuery);
78+
return hostFromQuery;
79+
}
80+
}
81+
82+
// Prefer Forwarded header (RFC 7239)
83+
String forwarded = headers.getFirst(FORWARDED);
84+
if (forwarded != null) {
85+
try {
86+
if (forwarded.contains("host=")) {
87+
hostUrl = forwarded.split("host=")[1].split("[;,]")[0].trim();
88+
log.trace("Using Forwarded header host: {}", hostUrl);
89+
return ensureScheme(hostUrl, request);
90+
}
91+
} catch (Exception e) {
92+
log.debug("Failed to parse Forwarded header: {}", forwarded, e);
93+
}
94+
}
95+
96+
// Use X-Forwarded-Host if available
97+
String xForwardedHost = headers.getFirst(X_FORWARDED_HOST);
98+
if (xForwardedHost != null) {
99+
String scheme = headers.getFirst(X_FORWARDED_PROTO);
100+
if (scheme == null) {
101+
scheme = request.getURI().getScheme();
102+
}
103+
hostUrl = scheme + "://" + xForwardedHost;
104+
log.trace("Using X-Forwarded-Host: {}", hostUrl);
105+
return hostUrl;
106+
}
107+
108+
// Fall back to standard Host header
109+
String host = headers.getFirst(HttpHeaders.HOST);
110+
if (host != null) {
111+
String scheme = request.getURI().getScheme();
112+
hostUrl = scheme + "://" + host;
113+
log.trace("Using standard Host header: {}", hostUrl);
114+
return hostUrl;
115+
}
116+
117+
log.debug("No host information found in request headers");
118+
return null;
119+
}
120+
121+
private static String ensureScheme(String hostUrl, ServerHttpRequest request) {
122+
if (!hostUrl.contains("://")) {
123+
String scheme = request.getHeaders().getFirst(X_FORWARDED_PROTO);
124+
if (scheme == null) {
125+
scheme = request.getURI().getScheme();
126+
}
127+
return scheme + "://" + hostUrl;
128+
}
129+
return hostUrl;
130+
}
131+
132+
public static boolean isSecureScheme(ServerWebExchange exchange) {
133+
String scheme = exchange.getRequest().getHeaders().getFirst(X_FORWARDED_PROTO);
134+
if (scheme == null) {
135+
scheme = exchange.getRequest().getURI().getScheme();
136+
}
137+
return "https".equals(scheme);
138+
}
139+
140+
/**
141+
* Extract host URL from query parameters if they contain appsmith-forwarded-host and appsmith-forwarded-proto
142+
* @param query The query string to parse
143+
* @return The host URL constructed from query parameters, or null if parameters not found
144+
*/
145+
private static String getHostUrlFromQuery(String query) {
146+
String host = null;
147+
String proto = null;
148+
149+
// Parse the query string
150+
String[] params = query.split("&");
151+
for (String param : params) {
152+
if (param.startsWith(APPSMITH_FORWARDED_HOST + "=")) {
153+
host = param.substring(APPSMITH_FORWARDED_HOST.length() + 1);
154+
} else if (param.startsWith(APPSMITH_FORWARDED_PROTO + "=")) {
155+
proto = param.substring(APPSMITH_FORWARDED_PROTO.length() + 1);
156+
}
157+
158+
// If we have both parameters, we can return the host URL
159+
if (host != null && proto != null) {
160+
return proto + "://" + host;
161+
}
162+
}
163+
164+
return null;
165+
}
166+
167+
private static String sanitizeConfiguredDomain(String domain, ServerWebExchange exchange) {
168+
String sanitizedDomain = domain;
169+
while (sanitizedDomain.endsWith("/")) {
170+
sanitizedDomain = sanitizedDomain.substring(0, sanitizedDomain.length() - 1);
171+
}
172+
173+
if (sanitizedDomain.startsWith("http://") || sanitizedDomain.startsWith("https://")) {
174+
return sanitizedDomain;
175+
}
176+
177+
String scheme = exchange.getRequest().getHeaders().getFirst(X_FORWARDED_PROTO);
178+
if (!StringUtils.hasText(scheme)) {
179+
scheme = exchange.getRequest().getURI().getScheme();
180+
}
181+
if (!StringUtils.hasText(scheme)) {
182+
scheme = "https";
183+
}
184+
185+
return scheme + "://" + sanitizedDomain;
186+
}
187+
}

0 commit comments

Comments
 (0)