diff --git a/app/server/appsmith-server/pom.xml b/app/server/appsmith-server/pom.xml index 8e8ac34437..f1159a554f 100644 --- a/app/server/appsmith-server/pom.xml +++ b/app/server/appsmith-server/pom.xml @@ -135,11 +135,6 @@ org.springframework.boot spring-boot-starter-aop - - com.bucket4j - bucket4j-redis - 8.3.0 - org.hibernate.validator hibernate-validator @@ -372,6 +367,7 @@ reactiveCaching 1.0-SNAPSHOT + org.openjdk.jmh jmh-core diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationSuccessHandler.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationSuccessHandler.java index dc22dae39d..1c024df02e 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationSuccessHandler.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationSuccessHandler.java @@ -3,7 +3,6 @@ package com.appsmith.server.authentication.handlers; import com.appsmith.server.authentication.handlers.ce.AuthenticationSuccessHandlerCE; import com.appsmith.server.configurations.CommonConfig; import com.appsmith.server.helpers.RedirectHelper; -import com.appsmith.server.ratelimiting.RateLimitService; import com.appsmith.server.repositories.UserRepository; import com.appsmith.server.repositories.WorkspaceRepository; import com.appsmith.server.services.AnalyticsService; @@ -40,7 +39,6 @@ public class AuthenticationSuccessHandler extends AuthenticationSuccessHandlerCE FeatureFlagService featureFlagService, CommonConfig commonConfig, UserIdentifierService userIdentifierService, - RateLimitService rateLimitService, TenantService tenantService, UserService userService) { @@ -59,7 +57,6 @@ public class AuthenticationSuccessHandler extends AuthenticationSuccessHandlerCE featureFlagService, commonConfig, userIdentifierService, - rateLimitService, tenantService, userService); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationFailureHandlerCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationFailureHandlerCE.java index c9dac2df77..99ac2d3499 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationFailureHandlerCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationFailureHandlerCE.java @@ -26,12 +26,13 @@ import static com.appsmith.server.helpers.RedirectHelper.REDIRECT_URL_QUERY_PARA @RequiredArgsConstructor public class AuthenticationFailureHandlerCE implements ServerAuthenticationFailureHandler { - private final ServerRedirectStrategy redirectStrategy = new DefaultServerRedirectStrategy(); + private ServerRedirectStrategy redirectStrategy = new DefaultServerRedirectStrategy(); @Override public Mono onAuthenticationFailure(WebFilterExchange webFilterExchange, AuthenticationException exception) { log.error("In the login failure handler. Cause: {}", exception.getMessage(), exception); ServerWebExchange exchange = webFilterExchange.getExchange(); + // On authentication failure, we send a redirect to the client's login error page. The browser will re-load the // login page again with an error message shown to the user. MultiValueMap queryParams = exchange.getRequest().getQueryParams(); @@ -40,46 +41,41 @@ public class AuthenticationFailureHandlerCE implements ServerAuthenticationFailu String redirectUrl = queryParams.getFirst(REDIRECT_URL_QUERY_PARAM); if (state != null && !state.isEmpty()) { - originHeader = getOriginFromState(state); + // This is valid for OAuth2 login failures. We derive the client login URL from the state query parameter + // that would have been set when we initiated the OAuth2 request. + String[] stateArray = state.split(","); + for (int i = 0; i < stateArray.length; i++) { + String stateVar = stateArray[i]; + if (stateVar != null + && stateVar.startsWith(Security.STATE_PARAMETER_ORIGIN) + && stateVar.contains("=")) { + // This is the origin of the request that we want to redirect to + originHeader = stateVar.split("=")[1]; + } + } } else { - originHeader = - getOriginFromReferer(exchange.getRequest().getHeaders().getOrigin()); - } - - // Construct the redirect URL based on the exception type - String url = constructRedirectUrl(exception, originHeader, redirectUrl); - - return redirectWithUrl(exchange, url); - } - - private String getOriginFromState(String state) { - String[] stateArray = state.split(","); - for (int i = 0; i < stateArray.length; i++) { - String stateVar = stateArray[i]; - if (stateVar != null && stateVar.startsWith(Security.STATE_PARAMETER_ORIGIN) && stateVar.contains("=")) { - return stateVar.split("=")[1]; + // This is a form login authentication failure + originHeader = exchange.getRequest().getHeaders().getOrigin(); + if (originHeader == null || originHeader.isEmpty()) { + // Check the referer header if the origin is not available + String refererHeader = exchange.getRequest().getHeaders().getFirst(Security.REFERER_HEADER); + if (refererHeader != null && !refererHeader.isBlank()) { + URI uri = null; + try { + uri = new URI(refererHeader); + String authority = uri.getAuthority(); + String scheme = uri.getScheme(); + originHeader = scheme + "://" + authority; + } catch (URISyntaxException e) { + originHeader = "/"; + } + } } } - return "/"; - } - // this method extracts the origin from the referer header - private String getOriginFromReferer(String refererHeader) { - if (refererHeader != null && !refererHeader.isBlank()) { - try { - URI uri = new URI(refererHeader); - String authority = uri.getAuthority(); - String scheme = uri.getScheme(); - return scheme + "://" + authority; - } catch (URISyntaxException e) { - return "/"; - } - } - return "/"; - } - - // this method constructs the redirect URL based on the exception type - private String constructRedirectUrl(AuthenticationException exception, String originHeader, String redirectUrl) { + // Authentication failure message can hold sensitive information, directly or indirectly. So we don't show all + // possible error messages. Only the ones we know are okay to be read by the user. Like a whitelist. + URI defaultRedirectLocation; String url = ""; if (exception instanceof OAuth2AuthenticationException && AppsmithError.SIGNUP_DISABLED @@ -100,11 +96,7 @@ public class AuthenticationFailureHandlerCE implements ServerAuthenticationFailu if (redirectUrl != null && !redirectUrl.trim().isEmpty()) { url = url + "&" + REDIRECT_URL_QUERY_PARAM + "=" + redirectUrl; } - return url; - } - - private Mono redirectWithUrl(ServerWebExchange exchange, String url) { - URI defaultRedirectLocation = URI.create(url); + defaultRedirectLocation = URI.create(url); return this.redirectStrategy.sendRedirect(exchange, defaultRedirectLocation); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationSuccessHandlerCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationSuccessHandlerCE.java index 6171d815e9..ba3bfd7d56 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationSuccessHandlerCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationSuccessHandlerCE.java @@ -4,7 +4,6 @@ import com.appsmith.external.constants.AnalyticsEvents; import com.appsmith.server.authentication.handlers.CustomServerOAuth2AuthorizationRequestResolver; import com.appsmith.server.configurations.CommonConfig; import com.appsmith.server.constants.FieldName; -import com.appsmith.server.constants.RateLimitConstants; import com.appsmith.server.constants.Security; import com.appsmith.server.domains.Application; import com.appsmith.server.domains.LoginSource; @@ -12,7 +11,6 @@ import com.appsmith.server.domains.User; import com.appsmith.server.domains.Workspace; import com.appsmith.server.dtos.ResendEmailVerificationDTO; import com.appsmith.server.helpers.RedirectHelper; -import com.appsmith.server.ratelimiting.RateLimitService; import com.appsmith.server.repositories.UserRepository; import com.appsmith.server.repositories.WorkspaceRepository; import com.appsmith.server.services.AnalyticsService; @@ -73,8 +71,8 @@ public class AuthenticationSuccessHandlerCE implements ServerAuthenticationSucce private final FeatureFlagService featureFlagService; private final CommonConfig commonConfig; private final UserIdentifierService userIdentifierService; - private final RateLimitService rateLimitService; private final TenantService tenantService; + private final UserService userService; private Mono isVerificationRequired(String userEmail, String method) { @@ -266,7 +264,6 @@ public class AuthenticationSuccessHandlerCE implements ServerAuthenticationSucce log.debug("Login succeeded for user: {}", authentication.getPrincipal()); Mono redirectionMono = null; User user = (User) authentication.getPrincipal(); - rateLimitService.resetCounter(RateLimitConstants.BUCKET_KEY_FOR_LOGIN_API, user.getEmail()); String originHeader = webFilterExchange.getExchange().getRequest().getHeaders().getOrigin(); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/RedisConfig.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/RedisConfig.java index 51c5483f84..9fef4b0d17 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/RedisConfig.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/RedisConfig.java @@ -5,7 +5,6 @@ import com.appsmith.server.dtos.OAuth2AuthorizedClientDTO; import com.appsmith.server.dtos.UserSessionDTO; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.json.JsonMapper; -import io.lettuce.core.RedisClient; import io.lettuce.core.resource.ClientResources; import io.micrometer.observation.ObservationRegistry; import lombok.extern.slf4j.Slf4j; @@ -101,12 +100,6 @@ public class RedisConfig { } } - @Bean - public RedisClient redisClient() { - final URI redisUri = URI.create(redisURL); - return RedisClient.create(redisUri.getScheme() + "://" + redisUri.getHost() + ":" + redisUri.getPort()); - } - private void fillAuthentication(URI redisUri, RedisConfiguration.WithAuthentication config) { final String userInfo = redisUri.getUserInfo(); if (StringUtils.isNotEmpty(userInfo)) { diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java index c24371c29b..671a89b4f2 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java @@ -7,10 +7,7 @@ import com.appsmith.server.constants.FieldName; import com.appsmith.server.constants.Url; import com.appsmith.server.domains.User; import com.appsmith.server.filters.CSRFFilter; -import com.appsmith.server.filters.ConditionalFilter; -import com.appsmith.server.filters.PreAuth; import com.appsmith.server.helpers.RedirectHelper; -import com.appsmith.server.ratelimiting.RateLimitService; import com.appsmith.server.services.AnalyticsService; import com.appsmith.server.services.UserService; import com.fasterxml.jackson.databind.ObjectMapper; @@ -96,9 +93,6 @@ public class SecurityConfig { @Autowired private RedirectHelper redirectHelper; - @Autowired - private RateLimitService rateLimitService; - @Value("${appsmith.internal.password}") private String INTERNAL_PASSWORD; @@ -214,10 +208,6 @@ public class SecurityConfig { .anyExchange() .authenticated() .and() - // Add Pre Auth rate limit filter before authentication filter - .addFilterBefore( - new ConditionalFilter(new PreAuth(rateLimitService), Url.LOGIN_URL), - SecurityWebFiltersOrder.FORM_LOGIN) .httpBasic(httpBasicSpec -> httpBasicSpec.authenticationFailureHandler(failureHandler)) .formLogin(formLoginSpec -> formLoginSpec .authenticationFailureHandler(failureHandler) @@ -227,6 +217,7 @@ public class SecurityConfig { ServerWebExchangeMatchers.pathMatchers(HttpMethod.POST, Url.LOGIN_URL)) .authenticationSuccessHandler(authenticationSuccessHandler) .authenticationFailureHandler(authenticationFailureHandler)) + // For Github SSO Login, check transformation class: CustomOAuth2UserServiceImpl // For Google SSO Login, check transformation class: CustomOAuth2UserServiceImpl .oauth2Login(oAuth2LoginSpec -> oAuth2LoginSpec diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/RateLimitConstants.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/RateLimitConstants.java deleted file mode 100644 index ca2e6b9422..0000000000 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/RateLimitConstants.java +++ /dev/null @@ -1,7 +0,0 @@ -package com.appsmith.server.constants; - -public class RateLimitConstants { - public static final String RATE_LIMIT_REACHED_ACCOUNT_SUSPENDED = - "Your account is suspended for 24 hours. Please reset your password to continue"; - public static final String BUCKET_KEY_FOR_LOGIN_API = "login"; -} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/ConditionalFilter.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/ConditionalFilter.java deleted file mode 100644 index 3455677795..0000000000 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/ConditionalFilter.java +++ /dev/null @@ -1,26 +0,0 @@ -package com.appsmith.server.filters; - -import org.springframework.web.server.ServerWebExchange; -import org.springframework.web.server.WebFilter; -import org.springframework.web.server.WebFilterChain; -import reactor.core.publisher.Mono; - -public class ConditionalFilter implements WebFilter { - - private final WebFilter filter; - private final String targetUrl; - - public ConditionalFilter(WebFilter filter, String targetUrl) { - this.filter = filter; - this.targetUrl = targetUrl; - } - - @Override - public Mono filter(ServerWebExchange exchange, WebFilterChain chain) { - if (exchange.getRequest().getPath().toString().equals(targetUrl)) { - return filter.filter(exchange, chain); - } - - return chain.filter(exchange); - } -} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/PreAuth.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/PreAuth.java deleted file mode 100644 index 21ea1aded6..0000000000 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/PreAuth.java +++ /dev/null @@ -1,72 +0,0 @@ -package com.appsmith.server.filters; - -import com.appsmith.server.constants.FieldName; -import com.appsmith.server.constants.RateLimitConstants; -import com.appsmith.server.ratelimiting.RateLimitService; -import lombok.extern.slf4j.Slf4j; -import org.springframework.security.web.server.DefaultServerRedirectStrategy; -import org.springframework.security.web.server.ServerRedirectStrategy; -import org.springframework.web.server.ServerWebExchange; -import org.springframework.web.server.WebFilter; -import org.springframework.web.server.WebFilterChain; -import reactor.core.publisher.Mono; - -import java.net.URI; -import java.net.URLDecoder; -import java.net.URLEncoder; -import java.nio.charset.StandardCharsets; - -@Slf4j -public class PreAuth implements WebFilter { - - private final ServerRedirectStrategy redirectStrategy = new DefaultServerRedirectStrategy(); - private final RateLimitService rateLimitService; - - public PreAuth(RateLimitService rateLimitService) { - this.rateLimitService = rateLimitService; - } - - @Override - public Mono filter(ServerWebExchange exchange, WebFilterChain chain) { - return getUsername(exchange).flatMap(username -> { - if (!username.isEmpty()) { - return rateLimitService - .tryIncreaseCounter(RateLimitConstants.BUCKET_KEY_FOR_LOGIN_API, username) - .flatMap(counterIncreaseAttemptSuccessful -> { - if (!counterIncreaseAttemptSuccessful) { - log.error("Rate limit exceeded. Redirecting to login page."); - return handleRateLimitExceeded(exchange); - } - - return chain.filter(exchange); - }); - } else { - // If username is empty, simply continue with the filter chain - return chain.filter(exchange); - } - }); - } - - private Mono getUsername(ServerWebExchange exchange) { - return exchange.getFormData().flatMap(formData -> { - String username = formData.getFirst(FieldName.USERNAME.toString()); - if (username != null && !username.isEmpty()) { - return Mono.just(URLDecoder.decode(username, StandardCharsets.UTF_8)); - } - - return Mono.just(""); - }); - } - - private Mono handleRateLimitExceeded(ServerWebExchange exchange) { - // Set the error in the URL query parameter for rate limiting - String url = "/user/login?error=true&message=" - + URLEncoder.encode(RateLimitConstants.RATE_LIMIT_REACHED_ACCOUNT_SUSPENDED, StandardCharsets.UTF_8); - return redirectWithUrl(exchange, url); - } - - private Mono redirectWithUrl(ServerWebExchange exchange, String url) { - URI defaultRedirectLocation = URI.create(url); - return this.redirectStrategy.sendRedirect(exchange, defaultRedirectLocation); - } -} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/ratelimiting/RateLimitConfig.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/ratelimiting/RateLimitConfig.java deleted file mode 100644 index 5c4aeb7c85..0000000000 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/ratelimiting/RateLimitConfig.java +++ /dev/null @@ -1,75 +0,0 @@ -package com.appsmith.server.ratelimiting; - -import com.appsmith.server.constants.RateLimitConstants; -import io.github.bucket4j.Bandwidth; -import io.github.bucket4j.BucketConfiguration; -import io.github.bucket4j.Refill; -import io.github.bucket4j.distributed.BucketProxy; -import io.github.bucket4j.distributed.ExpirationAfterWriteStrategy; -import io.github.bucket4j.redis.lettuce.cas.LettuceBasedProxyManager; -import io.lettuce.core.RedisClient; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; - -import java.time.Duration; -import java.util.HashMap; -import java.util.Map; -import java.util.Optional; - -@Configuration -public class RateLimitConfig { - private static final Map apiConfigurations = new HashMap<>(); - - @Autowired - private final RedisClient redisClient; - - public RateLimitConfig(RedisClient redisClient) { - this.redisClient = redisClient; - } - - static { - apiConfigurations.put( - RateLimitConstants.BUCKET_KEY_FOR_LOGIN_API, createBucketConfiguration(Duration.ofDays(1), 5)); - // Add more API configurations as needed - } - - @Bean - public LettuceBasedProxyManager proxyManager() { - /* - we want a single proxyManager to manage all buckets. - If we set too short an expiration time, - the proxyManager expires and renews the buckets with their initial configuration - */ - Duration longExpiration = Duration.ofDays(3650); // 10 years - return LettuceBasedProxyManager.builderFor(redisClient) - .withExpirationStrategy(ExpirationAfterWriteStrategy.fixedTimeToLive(longExpiration)) - .build(); - } - - @Bean - public Map apiBuckets() { - Map apiBuckets = new HashMap<>(); - - apiConfigurations.forEach((apiIdentifier, configuration) -> - apiBuckets.put(apiIdentifier, proxyManager().builder().build(apiIdentifier.getBytes(), configuration))); - - return apiBuckets; - } - - public BucketProxy getOrCreateAPIUserSpecificBucket(String apiIdentifier, String userId) { - String bucketIdentifier = apiIdentifier + userId; - Optional bucketProxy = proxyManager().getProxyConfiguration(bucketIdentifier.getBytes()); - if (bucketProxy.isPresent()) { - return proxyManager().builder().build(bucketIdentifier.getBytes(), bucketProxy.get()); - } - - return proxyManager().builder().build(bucketIdentifier.getBytes(), apiConfigurations.get(apiIdentifier)); - } - - private static BucketConfiguration createBucketConfiguration(Duration refillDuration, int limit) { - Refill refillConfig = Refill.intervally(limit, refillDuration); - Bandwidth limitConfig = Bandwidth.classic(limit, refillConfig); - return BucketConfiguration.builder().addLimit(limitConfig).build(); - } -} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/ratelimiting/RateLimitService.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/ratelimiting/RateLimitService.java deleted file mode 100644 index 51a9b048d9..0000000000 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/ratelimiting/RateLimitService.java +++ /dev/null @@ -1,45 +0,0 @@ -package com.appsmith.server.ratelimiting; - -import io.github.bucket4j.distributed.BucketProxy; -import lombok.extern.slf4j.Slf4j; -import org.springframework.stereotype.Service; -import reactor.core.publisher.Mono; - -import java.util.Map; - -@Slf4j -@Service -public class RateLimitService { - - private final Map apiBuckets; - private final RateLimitConfig rateLimitConfig; - // this number of tokens var can later be customised per API in the configuration. - private final Integer DEFAULT_NUMBER_OF_TOKENS_CONSUMED_PER_REQUEST = 1; - - public RateLimitService(Map apiBuckets, RateLimitConfig rateLimitConfig) { - this.apiBuckets = apiBuckets; - this.rateLimitConfig = rateLimitConfig; - } - - public Mono tryIncreaseCounter(String apiIdentifier, String userIdentifier) { - log.debug( - "RateLimitService.tryIncreaseCounter() called with apiIdentifier = {}, userIdentifier = {}", - apiIdentifier, - userIdentifier); - // handle the case where API itself is not rate limited - log.debug( - apiBuckets.containsKey(apiIdentifier) ? "apiBuckets contains key" : "apiBuckets does not contain key"); - if (!apiBuckets.containsKey(apiIdentifier)) return Mono.just(false); - - BucketProxy userSpecificBucket = - rateLimitConfig.getOrCreateAPIUserSpecificBucket(apiIdentifier, userIdentifier); - log.debug("userSpecificBucket = {}", userSpecificBucket); - return Mono.just(userSpecificBucket.tryConsume(DEFAULT_NUMBER_OF_TOKENS_CONSUMED_PER_REQUEST)); - } - - public void resetCounter(String apiIdentifier, String userIdentifier) { - rateLimitConfig - .getOrCreateAPIUserSpecificBucket(apiIdentifier, userIdentifier) - .reset(); - } -} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/ratelimiting/annotations/RateLimit.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/ratelimiting/annotations/RateLimit.java deleted file mode 100644 index 5732a88446..0000000000 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/ratelimiting/annotations/RateLimit.java +++ /dev/null @@ -1,20 +0,0 @@ -package com.appsmith.server.ratelimiting.annotations; - -import org.springframework.core.annotation.AliasFor; -import org.springframework.web.bind.annotation.RequestMapping; - -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -@Target({ElementType.METHOD}) -@Retention(RetentionPolicy.RUNTIME) -@RequestMapping -public @interface RateLimit { - - @AliasFor(annotation = RequestMapping.class, attribute = "value") - String[] value() default {}; - - String api() default ""; -} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/ratelimiting/aspects/RateLimitAspect.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/ratelimiting/aspects/RateLimitAspect.java deleted file mode 100644 index 5e67421bb0..0000000000 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/ratelimiting/aspects/RateLimitAspect.java +++ /dev/null @@ -1,51 +0,0 @@ -package com.appsmith.server.ratelimiting.aspects; - -import com.appsmith.server.domains.User; -import com.appsmith.server.dtos.ResponseDTO; -import com.appsmith.server.exceptions.AppsmithError; -import com.appsmith.server.exceptions.AppsmithException; -import com.appsmith.server.ratelimiting.RateLimitService; -import com.appsmith.server.ratelimiting.annotations.*; -import com.appsmith.server.services.SessionUserService; -import org.aspectj.lang.ProceedingJoinPoint; -import org.aspectj.lang.annotation.Around; -import org.aspectj.lang.annotation.Aspect; -import org.springframework.stereotype.Component; -import reactor.core.publisher.Mono; - -@Aspect -@Component -public class RateLimitAspect { - private final RateLimitService rateLimitService; - private final SessionUserService sessionUserService; - - public RateLimitAspect(RateLimitService rateLimitService, SessionUserService sessionUserService) { - this.rateLimitService = rateLimitService; - this.sessionUserService = sessionUserService; - } - - @Around(value = "@annotation(rateLimit)") - public Object applyRateLimit(ProceedingJoinPoint joinPoint, RateLimit rateLimit) throws Throwable { - String apiIdentifier = rateLimit.api(); - Mono loggedInUser = sessionUserService.getCurrentUser(); - Mono userIdentifier = loggedInUser.map(User::getEmail); - - return userIdentifier.flatMap(email -> { - Mono isAllowedMono = rateLimitService.tryIncreaseCounter(apiIdentifier, email); - return isAllowedMono.flatMap(isAllowed -> { - if (!isAllowed) { - AppsmithException exception = new AppsmithException(AppsmithError.TOO_MANY_REQUESTS); - return Mono.just(new ResponseDTO<>(exception.getHttpStatus(), exception.getMessage(), null)); - } - - try { - Object result = joinPoint.proceed(); - return result instanceof Mono ? (Mono) result : Mono.just(result); - } catch (Throwable e) { - AppsmithError error = AppsmithError.INTERNAL_SERVER_ERROR; - throw new AppsmithException(error, e.getMessage()); - } - }); - }); - } -} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java index 7ec5c51774..eb9e04dca5 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java @@ -6,7 +6,6 @@ import com.appsmith.server.configurations.EmailConfig; import com.appsmith.server.helpers.RedirectHelper; import com.appsmith.server.helpers.UserUtils; import com.appsmith.server.notifications.EmailSender; -import com.appsmith.server.ratelimiting.RateLimitService; import com.appsmith.server.repositories.ApplicationRepository; import com.appsmith.server.repositories.EmailVerificationTokenRepository; import com.appsmith.server.repositories.PasswordResetTokenRepository; @@ -49,8 +48,8 @@ public class UserServiceImpl extends UserServiceCEImpl implements UserService { PermissionGroupService permissionGroupService, UserUtils userUtils, EmailVerificationTokenRepository emailVerificationTokenRepository, - RedirectHelper redirectHelper, - RateLimitService rateLimitService) { + RedirectHelper redirectHelper) { + super( scheduler, validator, @@ -74,7 +73,6 @@ public class UserServiceImpl extends UserServiceCEImpl implements UserService { permissionGroupService, userUtils, emailVerificationTokenRepository, - redirectHelper, - rateLimitService); + redirectHelper); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java index a828b92d92..ae52d7340f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java @@ -8,7 +8,6 @@ import com.appsmith.server.configurations.CommonConfig; import com.appsmith.server.configurations.EmailConfig; import com.appsmith.server.constants.Appsmith; import com.appsmith.server.constants.FieldName; -import com.appsmith.server.constants.RateLimitConstants; import com.appsmith.server.domains.EmailVerificationToken; import com.appsmith.server.domains.LoginSource; import com.appsmith.server.domains.PasswordResetToken; @@ -31,7 +30,6 @@ import com.appsmith.server.helpers.RedirectHelper; import com.appsmith.server.helpers.UserUtils; import com.appsmith.server.helpers.ValidationUtils; import com.appsmith.server.notifications.EmailSender; -import com.appsmith.server.ratelimiting.RateLimitService; import com.appsmith.server.repositories.ApplicationRepository; import com.appsmith.server.repositories.EmailVerificationTokenRepository; import com.appsmith.server.repositories.PasswordResetTokenRepository; @@ -117,7 +115,6 @@ public class UserServiceCEImpl extends BaseService private final TenantService tenantService; private final PermissionGroupService permissionGroupService; private final UserUtils userUtils; - private final RateLimitService rateLimitService; private final RedirectHelper redirectHelper; private static final WebFilterChain EMPTY_WEB_FILTER_CHAIN = serverWebExchange -> Mono.empty(); @@ -162,8 +159,7 @@ public class UserServiceCEImpl extends BaseService PermissionGroupService permissionGroupService, UserUtils userUtils, EmailVerificationTokenRepository emailVerificationTokenRepository, - RedirectHelper redirectHelper, - RateLimitService rateLimitService) { + RedirectHelper redirectHelper) { super(scheduler, validator, mongoConverter, reactiveMongoTemplate, repository, analyticsService); this.workspaceService = workspaceService; this.sessionUserService = sessionUserService; @@ -180,7 +176,6 @@ public class UserServiceCEImpl extends BaseService this.tenantService = tenantService; this.permissionGroupService = permissionGroupService; this.userUtils = userUtils; - this.rateLimitService = rateLimitService; this.emailVerificationTokenRepository = emailVerificationTokenRepository; this.redirectHelper = redirectHelper; } @@ -428,17 +423,12 @@ public class UserServiceCEImpl extends BaseService emailTokenDTO.getToken()))) .flatMap(passwordResetTokenRepository::delete) .then(repository.save(userFromDb)) - .doOnSuccess(result -> { - // In a separate thread, we delete all other sessions of this user. - sessionUserService - .logoutAllSessions(userFromDb.getEmail()) - .subscribeOn(Schedulers.boundedElastic()) - .subscribe(); - - // we reset the counter for user's login attempts once password is reset - rateLimitService.resetCounter( - RateLimitConstants.BUCKET_KEY_FOR_LOGIN_API, userFromDb.getEmail()); - }) + .doOnSuccess(result -> + // In a separate thread, we delete all other sessions of this user. + sessionUserService + .logoutAllSessions(userFromDb.getEmail()) + .subscribeOn(Schedulers.boundedElastic()) + .subscribe()) .thenReturn(true); })); }