Skip to content

chore: login metric added #40325

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 24, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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";
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@

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);
}
}
Original file line number Diff line number Diff line change
@@ -1,21 +1,50 @@
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;

@Override
public Mono<Void> onAuthenticationFailure(WebFilterExchange webFilterExchange, AuthenticationException exception) {
String source = exception instanceof OAuth2AuthenticationException
? ((OAuth2AuthenticationException) exception).getError().getErrorCode()
: "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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
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;
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,23 @@
import com.appsmith.server.authentication.handlers.AccessDeniedHandler;
import com.appsmith.server.authentication.handlers.CustomServerOAuth2AuthorizationRequestResolver;
import com.appsmith.server.authentication.handlers.LogoutSuccessHandler;
import com.appsmith.server.authentication.handlers.ce.AuthenticationFailureHandlerCE;
import com.appsmith.server.authentication.oauth2clientrepositories.CustomOauth2ClientRepositoryManager;
import com.appsmith.server.constants.FieldName;
import com.appsmith.server.constants.Url;
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;
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;
Expand All @@ -42,7 +45,6 @@
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;
Expand Down Expand Up @@ -90,7 +92,7 @@ public class SecurityConfig {
private ServerAuthenticationSuccessHandler authenticationSuccessHandler;

@Autowired
private ServerAuthenticationFailureHandler authenticationFailureHandler;
private AuthenticationFailureHandlerCE authenticationFailureHandler;

@Autowired
private ServerAuthenticationEntryPoint authenticationEntryPoint;
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -182,14 +191,17 @@ public SecurityWebFilterChain securityWebFilterChain(ServerHttpSecurity http) {
// 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),
Expand Down Expand Up @@ -226,7 +238,12 @@ public SecurityWebFilterChain securityWebFilterChain(ServerHttpSecurity http) {
.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, authenticationFailureHandler),
Url.LOGIN_URL),
SecurityWebFiltersOrder.FORM_LOGIN)
.httpBasic(httpBasicSpec -> httpBasicSpec.authenticationFailureHandler(failureHandler))
.formLogin(formLoginSpec -> formLoginSpec
Expand Down Expand Up @@ -264,7 +281,8 @@ public SecurityWebFilterChain securityWebFilterChain(ServerHttpSecurity http) {
}

/**
* 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() {
Expand All @@ -283,7 +301,8 @@ private User createAnonymousUser() {
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();
Expand All @@ -299,7 +318,8 @@ private Mono<Void> sanityCheckFilter(ServerWebExchange exchange, WebFilterChain
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)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
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) {
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();
});
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package com.appsmith.server.filters;

import com.appsmith.server.authentication.handlers.ce.AuthenticationFailureHandlerCE;
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;
import org.springframework.security.web.server.WebFilterExchange;
import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.WebFilter;
import org.springframework.web.server.WebFilterChain;
Expand All @@ -15,16 +18,24 @@
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
public class LoginRateLimitFilter implements WebFilter {

private final ServerRedirectStrategy redirectStrategy = new DefaultServerRedirectStrategy();
private final RateLimitService rateLimitService;
private final MeterRegistry meterRegistry;
private final AuthenticationFailureHandlerCE authenticationFailureHandler;

public LoginRateLimitFilter(RateLimitService rateLimitService) {
public LoginRateLimitFilter(
RateLimitService rateLimitService,
MeterRegistry meterRegistry,
AuthenticationFailureHandlerCE authenticationFailureHandler) {
this.rateLimitService = rateLimitService;
this.meterRegistry = meterRegistry;
this.authenticationFailureHandler = authenticationFailureHandler;
}

@Override
Expand Down Expand Up @@ -60,6 +71,22 @@ private Mono<Void> 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);

// Create a WebFilterExchange for the error handler
WebFilterExchange webFilterExchange = new WebFilterExchange(exchange, chain -> Mono.empty());
authenticationFailureHandler.handleErrorRedirect(webFilterExchange).subscribe();

meterRegistry
.counter(
LOGIN_FAILURE,
"source",
"rate_limit",
"errorCode",
"RateLimitExceeded",
"message",
RateLimitConstants.RATE_LIMIT_REACHED_ACCOUNT_SUSPENDED)
.increment();

return redirectWithUrl(exchange, url);
}

Expand Down