chore: login metric added (#40325)
## Description Added custom login metrics <img width="1281" alt="image" src="https://github.com/user-attachments/assets/530b2155-c645-428a-94fa-fbec2f4106e5" /> Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.All" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/14617262543> > Commit: e36ee2c4f9432935a3dc75c30596a5042e5fee47 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=14617262543&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Wed, 23 Apr 2025 12:57:57 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Introduced login metrics tracking, including timing and failure counts for login attempts. - Added a new filter to monitor login attempts and record related metrics. - Enhanced authentication failure handling with detailed metrics for different failure sources and error redirects. - Improved rate limiting feedback with integrated metric recording. - **Bug Fixes** - More accurate tracking and reporting of login failures and their sources. - **Chores** - Updated security configuration to integrate new metrics and failure handling components. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
parent
84f3df4511
commit
4dbd235ac4
|
|
@ -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";
|
||||
}
|
||||
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<Void> 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<Void> 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();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
* <p>
|
||||
* Future folks: Please check out links:
|
||||
* - <a href="https://www.baeldung.com/spring-webflux-static-content">...</a>
|
||||
* - <a href="https://docs.spring.io/spring/docs/current/spring-framework-reference/web-reactive.html#webflux-config-static-resources">...</a>
|
||||
* - <a href=
|
||||
* "https://docs.spring.io/spring/docs/current/spring-framework-reference/web-reactive.html#webflux-config-static-resources">...</a>
|
||||
* - 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<ServerResponse> 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<Void> 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)) {
|
||||
|
|
|
|||
|
|
@ -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<Void> 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();
|
||||
});
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
@ -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);
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user