diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/LoginSpan.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/LoginSpan.java new file mode 100644 index 0000000000..fff4b0e7d1 --- /dev/null +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/LoginSpan.java @@ -0,0 +1,8 @@ +package com.appsmith.external.constants.spans; + +import static com.appsmith.external.constants.spans.BaseSpan.APPSMITH_SPAN_PREFIX; + +public class LoginSpan { + public static final String LOGIN_FAILURE = APPSMITH_SPAN_PREFIX + "login_failure"; + public static final String LOGIN_ATTEMPT = APPSMITH_SPAN_PREFIX + "login_total"; +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationFailureHandler.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationFailureHandler.java index c8fffa933f..56bedc1cdb 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationFailureHandler.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationFailureHandler.java @@ -2,12 +2,14 @@ package com.appsmith.server.authentication.handlers; import com.appsmith.server.authentication.handlers.ce.AuthenticationFailureHandlerCE; import com.appsmith.server.authentication.helpers.AuthenticationFailureRetryHandler; +import io.micrometer.core.instrument.MeterRegistry; import org.springframework.stereotype.Component; @Component public class AuthenticationFailureHandler extends AuthenticationFailureHandlerCE { - public AuthenticationFailureHandler(AuthenticationFailureRetryHandler authenticationFailureRetryHandler) { - super(authenticationFailureRetryHandler); + public AuthenticationFailureHandler( + AuthenticationFailureRetryHandler authenticationFailureRetryHandler, MeterRegistry meterRegistry) { + super(authenticationFailureRetryHandler, meterRegistry); } } 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 258ad6c16e..32614f4b4b 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 @@ -1,21 +1,52 @@ package com.appsmith.server.authentication.handlers.ce; import com.appsmith.server.authentication.helpers.AuthenticationFailureRetryHandler; +import io.micrometer.core.instrument.MeterRegistry; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.security.core.AuthenticationException; +import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.web.server.WebFilterExchange; import org.springframework.security.web.server.authentication.ServerAuthenticationFailureHandler; import reactor.core.publisher.Mono; +import static com.appsmith.external.constants.spans.LoginSpan.LOGIN_FAILURE; + @Slf4j @RequiredArgsConstructor public class AuthenticationFailureHandlerCE implements ServerAuthenticationFailureHandler { private final AuthenticationFailureRetryHandler authenticationFailureRetryHandler; + private final MeterRegistry meterRegistry; + + private static final String SOURCE_FORM = "form"; @Override public Mono onAuthenticationFailure(WebFilterExchange webFilterExchange, AuthenticationException exception) { + String source = exception instanceof OAuth2AuthenticationException + ? ((OAuth2AuthenticationException) exception).getError().getErrorCode() + : SOURCE_FORM; + + String errorMessage = exception.getMessage(); + + meterRegistry + .counter(LOGIN_FAILURE, "source", source, "message", errorMessage) + .increment(); return authenticationFailureRetryHandler.retryAndRedirectOnAuthenticationFailure(webFilterExchange, exception); } + + public Mono handleErrorRedirect(WebFilterExchange webFilterExchange) { + String error = + webFilterExchange.getExchange().getRequest().getQueryParams().getFirst("error"); + String message = + webFilterExchange.getExchange().getRequest().getQueryParams().getFirst("message"); + + if ("true".equals(error)) { + meterRegistry + .counter(LOGIN_FAILURE, "source", "redirect", "message", message) + .increment(); + } + + return Mono.empty(); + } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/NoTagsMeterFilter.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/NoTagsMeterFilter.java index 2a23a7cd2f..cfdba9a711 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/NoTagsMeterFilter.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/NoTagsMeterFilter.java @@ -8,6 +8,8 @@ import io.micrometer.core.instrument.config.MeterFilter; import java.util.List; import java.util.Map; +import static com.appsmith.external.constants.spans.LoginSpan.LOGIN_ATTEMPT; +import static com.appsmith.external.constants.spans.LoginSpan.LOGIN_FAILURE; import static com.appsmith.external.constants.spans.ce.ActionSpanCE.*; import static com.appsmith.external.git.constants.ce.GitSpanCE.FS_FETCH_REMOTE; import static com.appsmith.external.git.constants.ce.GitSpanCE.FS_RESET; @@ -28,7 +30,9 @@ public class NoTagsMeterFilter implements MeterFilter { Map.entry(FS_RESET, List.of()), Map.entry(JGIT_RESET_HARD, List.of()), Map.entry(FS_FETCH_REMOTE, List.of()), - Map.entry(JGIT_FETCH_REMOTE, List.of())); + Map.entry(JGIT_FETCH_REMOTE, List.of()), + Map.entry(LOGIN_FAILURE, List.of("source", "message")), + Map.entry(LOGIN_ATTEMPT, List.of("source"))); @Override public Meter.Id map(Meter.Id id) { 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 9f787fd267..2b00f5b483 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 @@ -2,6 +2,7 @@ package com.appsmith.server.configurations; import com.appsmith.external.exceptions.ErrorDTO; import com.appsmith.server.authentication.handlers.AccessDeniedHandler; +import com.appsmith.server.authentication.handlers.AuthenticationFailureHandler; import com.appsmith.server.authentication.handlers.CustomServerOAuth2AuthorizationRequestResolver; import com.appsmith.server.authentication.handlers.LogoutSuccessHandler; import com.appsmith.server.authentication.oauth2clientrepositories.CustomOauth2ClientRepositoryManager; @@ -11,6 +12,7 @@ import com.appsmith.server.domains.User; import com.appsmith.server.dtos.ResponseDTO; import com.appsmith.server.exceptions.AppsmithErrorCode; import com.appsmith.server.filters.ConditionalFilter; +import com.appsmith.server.filters.LoginMetricsFilter; import com.appsmith.server.filters.LoginRateLimitFilter; import com.appsmith.server.helpers.RedirectHelper; import com.appsmith.server.ratelimiting.RateLimitService; @@ -18,6 +20,7 @@ import com.appsmith.server.services.AnalyticsService; import com.appsmith.server.services.UserService; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; +import io.micrometer.core.instrument.MeterRegistry; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Bean; @@ -42,7 +45,6 @@ import org.springframework.security.oauth2.client.registration.ReactiveClientReg import org.springframework.security.web.server.SecurityWebFilterChain; import org.springframework.security.web.server.ServerAuthenticationEntryPoint; import org.springframework.security.web.server.authentication.ServerAuthenticationEntryPointFailureHandler; -import org.springframework.security.web.server.authentication.ServerAuthenticationFailureHandler; import org.springframework.security.web.server.authentication.ServerAuthenticationSuccessHandler; import org.springframework.security.web.server.util.matcher.PathPatternParserServerWebExchangeMatcher; import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher; @@ -90,7 +92,7 @@ public class SecurityConfig { private ServerAuthenticationSuccessHandler authenticationSuccessHandler; @Autowired - private ServerAuthenticationFailureHandler authenticationFailureHandler; + private AuthenticationFailureHandler authenticationFailureHandler; @Autowired private ServerAuthenticationEntryPoint authenticationEntryPoint; @@ -119,21 +121,28 @@ public class SecurityConfig { @Autowired private CsrfConfig csrfConfig; + @Autowired + private MeterRegistry meterRegistry; + @Value("${appsmith.internal.password}") private String INTERNAL_PASSWORD; private static final String INTERNAL = "INTERNAL"; /** - * This routerFunction is required to map /public/** endpoints to the src/main/resources/public folder - * This is to allow static resources to be served by the server. Couldn't find an easier way to do this, + * This routerFunction is required to map /public/** endpoints to the + * src/main/resources/public folder + * This is to allow static resources to be served by the server. Couldn't find + * an easier way to do this, * hence using RouterFunctions to implement this feature. *

* Future folks: Please check out links: * - ... - * - ... + * - ... * - Class ResourceHandlerRegistry - * for details. If you figure out a cleaner approach, please modify this function + * for details. If you figure out a cleaner approach, please modify this + * function */ @Bean public RouterFunction publicRouter() { @@ -182,14 +191,17 @@ public class SecurityConfig { // Disabled because we use CSP's `frame-ancestors` instead. .frameOptions(options -> options.disable())) .anonymous(anonymousSpec -> anonymousSpec.principal(createAnonymousUser())) - // This returns 401 unauthorized for all requests that are not authenticated but authentication is + // This returns 401 unauthorized for all requests that are not authenticated but + // authentication is // required - // The client will redirect to the login page if we return 401 as Http status response + // The client will redirect to the login page if we return 401 as Http status + // response .exceptionHandling(exceptionHandlingSpec -> exceptionHandlingSpec .authenticationEntryPoint(authenticationEntryPoint) .accessDeniedHandler(accessDeniedHandler)) .authorizeExchange(authorizeExchangeSpec -> authorizeExchangeSpec - // The following endpoints are allowed to be accessed without authentication + // The following endpoints are allowed to be accessed without + // authentication .matchers( ServerWebExchangeMatchers.pathMatchers(HttpMethod.GET, Url.HEALTH_CHECK), ServerWebExchangeMatchers.pathMatchers(HttpMethod.POST, USER_URL), @@ -226,7 +238,10 @@ public class SecurityConfig { .authenticated()) // Add Pre Auth rate limit filter before authentication filter .addFilterBefore( - new ConditionalFilter(new LoginRateLimitFilter(rateLimitService), Url.LOGIN_URL), + new ConditionalFilter(new LoginMetricsFilter(meterRegistry), Url.LOGIN_URL), + SecurityWebFiltersOrder.FORM_LOGIN) + .addFilterBefore( + new ConditionalFilter(new LoginRateLimitFilter(rateLimitService, meterRegistry), Url.LOGIN_URL), SecurityWebFiltersOrder.FORM_LOGIN) .httpBasic(httpBasicSpec -> httpBasicSpec.authenticationFailureHandler(failureHandler)) .formLogin(formLoginSpec -> formLoginSpec @@ -264,7 +279,8 @@ public class SecurityConfig { } /** - * This bean configures the parameters that need to be set when a Cookie is created for a logged in user + * This bean configures the parameters that need to be set when a Cookie is + * created for a logged in user */ @Bean public WebSessionIdResolver webSessionIdResolver() { @@ -283,7 +299,8 @@ public class SecurityConfig { private Mono sanityCheckFilter(ServerWebExchange exchange, WebFilterChain chain) { final HttpHeaders headers = exchange.getRequest().getHeaders(); - // 1. Check if the content-type is valid at all. Mostly just checks if it contains a `/`. + // 1. Check if the content-type is valid at all. Mostly just checks if it + // contains a `/`. MediaType contentType; try { contentType = headers.getContentType(); @@ -299,7 +316,8 @@ public class SecurityConfig { return writeErrorResponse(exchange, chain, "Unsupported Content-Type"); } - // 3. Check Appsmith version, if present. Not making this a mandatory check for now, but reconsider later. + // 3. Check Appsmith version, if present. Not making this a mandatory check for + // now, but reconsider later. final String versionHeaderValue = headers.getFirst(CsrfConfig.VERSION_HEADER); final String serverVersion = projectProperties.getVersion(); if (versionHeaderValue != null && !serverVersion.equals(versionHeaderValue)) { diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/LoginMetricsFilter.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/LoginMetricsFilter.java new file mode 100644 index 0000000000..8ca2f8f332 --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/LoginMetricsFilter.java @@ -0,0 +1,49 @@ +package com.appsmith.server.filters; + +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.Timer; +import lombok.extern.slf4j.Slf4j; +import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.server.WebFilter; +import org.springframework.web.server.WebFilterChain; +import reactor.core.publisher.Mono; + +import static com.appsmith.external.constants.spans.LoginSpan.LOGIN_ATTEMPT; +import static com.appsmith.external.constants.spans.LoginSpan.LOGIN_FAILURE; + +@Slf4j +public class LoginMetricsFilter implements WebFilter { + + private final MeterRegistry meterRegistry; + + public LoginMetricsFilter(MeterRegistry meterRegistry) { + this.meterRegistry = meterRegistry; + } + + @Override + public Mono filter(ServerWebExchange exchange, WebFilterChain chain) { + return Mono.defer(() -> { + Timer.Sample sample = Timer.start(meterRegistry); + return chain.filter(exchange) + .doOnSuccess(aVoid -> { + sample.stop(Timer.builder(LOGIN_ATTEMPT).register(meterRegistry)); + }) + .doOnError(throwable -> { + sample.stop(Timer.builder(LOGIN_ATTEMPT) + .tag("message", throwable.getMessage()) + .register(meterRegistry)); + + meterRegistry + .counter( + LOGIN_FAILURE, + "source", + "form_login", + "errorCode", + "AuthenticationFailed", + "message", + throwable.getMessage()) + .increment(); + }); + }); + } +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/LoginRateLimitFilter.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/LoginRateLimitFilter.java index d64ca5d820..ce3746bb70 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/LoginRateLimitFilter.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/LoginRateLimitFilter.java @@ -3,6 +3,7 @@ package com.appsmith.server.filters; import com.appsmith.server.constants.FieldName; import com.appsmith.server.constants.RateLimitConstants; import com.appsmith.server.ratelimiting.RateLimitService; +import io.micrometer.core.instrument.MeterRegistry; import lombok.extern.slf4j.Slf4j; import org.springframework.security.web.server.DefaultServerRedirectStrategy; import org.springframework.security.web.server.ServerRedirectStrategy; @@ -15,6 +16,7 @@ import java.net.URI; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; +import static com.appsmith.external.constants.spans.LoginSpan.LOGIN_FAILURE; import static java.lang.Boolean.FALSE; @Slf4j @@ -22,9 +24,11 @@ public class LoginRateLimitFilter implements WebFilter { private final ServerRedirectStrategy redirectStrategy = new DefaultServerRedirectStrategy(); private final RateLimitService rateLimitService; + private final MeterRegistry meterRegistry; - public LoginRateLimitFilter(RateLimitService rateLimitService) { + public LoginRateLimitFilter(RateLimitService rateLimitService, MeterRegistry meterRegistry) { this.rateLimitService = rateLimitService; + this.meterRegistry = meterRegistry; } @Override @@ -60,6 +64,18 @@ public class LoginRateLimitFilter implements WebFilter { // 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); + + meterRegistry + .counter( + LOGIN_FAILURE, + "source", + "rate_limit", + "errorCode", + "RateLimitExceeded", + "message", + RateLimitConstants.RATE_LIMIT_REACHED_ACCOUNT_SUSPENDED) + .increment(); + return redirectWithUrl(exchange, url); }