bucket4j-spring-boot-starter icon indicating copy to clipboard operation
bucket4j-spring-boot-starter copied to clipboard

execute-condition not working correctly when using UsernamePasswordAuthenticationFilter

Open anaconda875 opened this issue 5 years ago • 8 comments

Hi bro, I have a spring boot project, just simply try to apply spring security with JWT. I have a config class that extends WebSecurityConfigurerAdapter, like this:

@Override
protected void configure(HttpSecurity http) throws Exception {
	ExpressionUrlAuthorizationConfigurer<HttpSecurity>.ExpressionInterceptUrlRegistry registry = BLA BLA....;
	registry.anyRequest().authenticated(); // require authentication for any endpoint that's not whitelisted`
	http.addFilterAfter(jwtAuthenticationFilter(), UsernamePasswordAuthenticationFilter.class);
}

You can see I added a filter to validate the token. And here is the filter:

Override
protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException {
       String jwt = tokenExtractor.extract(request);
       if (StringUtils.hasText(jwt) && tokenVerifier.verify(jwt)) {
           UserCredential user = tokenParser.getUserCredential(jwt, setting.getTokenSigningKey());
           UsernamePasswordAuthenticationToken auth = new UsernamePasswordAuthenticationToken(
                   user, null, buildAuthorities(user));

            SecurityContextHolder.getContext().setAuthentication(auth);
        }
        filterChain.doFilter(request, response);`
    }

I integrated the bucket4j-spring-boot-starter with this config:

bucket4j.enabled=true
bucket4j.filters[0].cache-name=buckets
bucket4j.filters[0].filter-method=servlet
bucket4j.filters[0].url=.*
bucket4j.filters[0].rate-limits[0][email protected]() == null
bucket4j.filters[0].rate-limits[0].bandwidths[0].capacity=200
bucket4j.filters[0].rate-limits[0].bandwidths[0].time=60
bucket4j.filters[0].rate-limits[0].bandwidths[0].unit=seconds
bucket4j.filters[0].rate-limits[0].expression=getRemoteAddr()

In the SecurityService.class:

@Async
public String username() throws InterruptedException {
  String result = null;
   Authentication authentication = SecurityContextHolder.getContext().getAuthentication();  //NULL HERE
   if(authentication != null && !authentication.getName().equalsIgnoreCase("anonymousUser")) {
      result = authentication.getName();
    }
    System.out.println("username RETURN: " + result);
    return result;
}  

And here is the log - ALWAYS NULL image

I tried to debug and found that this line

Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
return null. //Why??? 

I continued to debug and found that the method username() from SecurityService is invoked BEFORE the method doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) of the filter, so that the line SecurityContextHolder.getContext().setAuthentication(auth); is never be invoked => when I tried to get Authentication, always null here. I kindly ask you to support me for this case. Many thanks

anaconda875 avatar Oct 13 '20 11:10 anaconda875

I think there are two problems. The first is the @Async method to get the username. The method is executed in a new thread without a relation to the logged in user. The second problem is maybe the filter order:

bucket4j.filters[0].filter-order= # Per default the lowest integer plus 10. Set it to a number higher then zero to execute it after e.g. Spring Security.

MarcGiffing avatar Oct 19 '20 19:10 MarcGiffing

Hi @MarcGiffing , after I increase the order, it seems work, but another problem I'm facing now is your ServletRequestFilter is never be executed when 401 unauthorized is returned. More detail:

@Component
public class JwtAuthenticationEntryPoint implements AuthenticationEntryPoint {

	@Autowired
	@Qualifier("handlerExceptionResolver")
	private HandlerExceptionResolver resolver;

	@Override
	public void commence(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse,
			AuthenticationException e) throws IOException, ServletException {
		log.error("Responding with unauthorized error.");
		resolver.resolveException(httpServletRequest, httpServletResponse, null,
				new PicAuthenticationException("Sorry, You're not authorized to access this resource.", e));
	}
}

(5)

You can re-check my config and my jwt-filter at (1) and (2) in my first comment. I tried to debug and found that when a request without a valid credential (token), then the JwtAuthenticationEntryPoint (5) commence got executed and STOP the filter chain immediately, so that your filter (ServletRequestFilter) never be executed. I also try to remove all the logic in the commence, just log the info, but the same situation. Ps: if you have skype, just add me for more detail. My skype: [email protected]. My fb: baongosot Thank you!

anaconda875 avatar Oct 20 '20 03:10 anaconda875

You have to define two Filter configuration. One before and one after Spring Security.

bucket4j.filters[0] bucket4j.filters[1]

MarcGiffing avatar Oct 21 '20 17:10 MarcGiffing

If I define 2 Filter configurations. One before and one after Spring Security like that. then any request will be fall into the Filter before Spring security, and the getAuthentication() will be null as the first comment Let's summarize what I tried:

  1. Filter before Spring security => getAuthentication() will be null as the first comment.
  2. Filter after Spring security => if unauthorize => commence() will be executed => stop filter chain => your ServletRequestFilter never be executed.
  3. Define two Filter configuration. One before and one after Spring Security => like at said at the top of THIS COMMENT

Conclusion: If have any filter before Spring Security, getAuthentication() will be null => cannot detect user name. If have filter after Spring Security and unauthorize => the filter will be stop I really need your help, please consider to make contact. Many thanks

anaconda875 avatar Oct 23 '20 09:10 anaconda875

I don't understand the problem. The first filter before Spring Security has never a username due to the fact that Sprin Security was never called/checked. So the first filter is used for all unauthorized users. The second filter is used for the authenticated users. Here you can define user specifc rate limit. You can define different bandwidths for each case.

bucket4j.enabled=true
bucket4j.filters[0].cache-name=buckets
bucket4j.filters[0].filter-method=servlet
bucket4j.filters[0].url=.*
bucket4j.filters[0].filter-order=-1000
bucket4j.filters[0].rate-limits[0].execute-condition=1 # Spring Security was not executed so you can't check for the username
bucket4j.filters[0].rate-limits[0].bandwidths[0].capacity=200
bucket4j.filters[0].rate-limits[0].bandwidths[0].time=60
bucket4j.filters[0].rate-limits[0].bandwidths[0].unit=seconds
bucket4j.filters[0].rate-limits[0].expression=getRemoteAddr()

bucket4j.filters[1].cache-name=buckets
bucket4j.filters[1].filter-method=servlet
bucket4j.filters[1].url=.*
bucket4j.filters[1].filter-order=1000
bucket4j.filters[1].rate-limits[0][email protected]() == null
bucket4j.filters[1].rate-limits[0].bandwidths[0].capacity=200
bucket4j.filters[1].rate-limits[0].bandwidths[0].time=60
bucket4j.filters[1].rate-limits[0].bandwidths[0].unit=seconds
bucket4j.filters[1].rate-limits[0].expression=securityService.username() # user specific rate limit

MarcGiffing avatar Oct 27 '20 20:10 MarcGiffing

I've already define like this, the first filter is for all unauthorized users. The second filter is used for the authenticated users. But the problem is, even authenticated user's requests still fall to the first filter. So the first bandwidth will be apply. That is unexpected issue I got.

anaconda875 avatar Nov 16 '20 02:11 anaconda875

Maybe you can try this code I get roles or username, but when i hit rest api happen error : java.lang.NullPointerException: null maybe bucket4j runs the code first before @service class gets its value

@Service
public class LimitService {
    public String roles() {
        List<String> roles = SecurityContextHolder.getContext().getAuthentication().getAuthorities()
                .stream()
                .map(item -> item.getAuthority())
                .collect(Collectors.toList());
        return roles.get(0);
    }
}

andikscript avatar Jul 16 '22 10:07 andikscript

any updates on this ?

lucasgbsampaio avatar Aug 17 '22 18:08 lucasgbsampaio

I got this problem too

WaOnEmperoR avatar Nov 05 '22 14:11 WaOnEmperoR

@anaconda875, did you solve this issue or how you continued with this behaviour?

For testing I've got these filters, however, after user authenticates, he falls under filter1 since this is executed before Spring Security and username/role is always anonymous/null, since SecurityContextHolder.getContext().authentication returns null, so authenticated user is always going to be seen as not authenticated.

bucket4j:
  enabled: true
  filters:
    - id: filter1
      cache-name: rate-limit-buckets
      filter-method: servlet
      filter-order: -200
      url: .*
      metrics:
        tags:
          - key: IP
            expression: getRemoteAddr()
            types: REJECTED_COUNTER
          - key: USERNAME
            expression: "@rateLimitService.username()"
          - key: URL
            expression: getRequestURI()
      rate-limits:
        - execute-condition: "@rateLimitService.role() == 'ROLE_ANONYMOUS'"
          cache-key: getRemoteAddr()
          bandwidths:
            - capacity: 2
              time: 20
              unit: seconds
    - id: filter2
      cache-name: rate-limit-buckets
      filter-method: servlet
      filter-order: 1
      url: .*
      metrics:
        tags:
          - key: IP
            expression: getRemoteAddr()
            types: REJECTED_COUNTER
          - key: USERNAME
            expression: "@rateLimitService.username()"
          - key: URL
            expression: getRequestURI()
      rate-limits:
        - execute-condition: "@rateLimitService.role() == 'ROLE_USER'"
          cache-key: "@rateLimitService.username()"
          bandwidths:
            - capacity: 5
              time: 20
              unit: seconds

@MarcGiffing, has this issue been solved somehow or how to deal with it? Thanks.

cjacky475 avatar Feb 21 '24 08:02 cjacky475

I really don't know how to solve this problem. Can someone provide a proposal?

If the request comes in, we don't know if the user is a valid user. Therefore we can't do any rate limiting. The Spring Security Filter has to do its job and authenticate the user. If it's a valid user we can proceed with a rate filter after the Spring Security filter. If it's an unauthenticated user the request is rejected so we don't need a filter. I Spring Security doesn't reject the request we can provide a second filter after Spring Security which handles the anonymous request. @cjacky475 you can set the first filter to the same filter-order as the second one.

If you want to prevent an DOS attack you can add a filter before Spring Security with a capacity which is also unusual for authenticated users.

MarcGiffing avatar Feb 23 '24 16:02 MarcGiffing

@MarcGiffing A workaround could be to for example create another cache that expires entries after X amount of time, and when the securityService succesfully retrieves an authentication (so the user is logged in), you put that users IP in the cache. Then create another method in your security service that returns a boolean to check if the IP is present in the cache. Finally in your first filter you can put a skip-condition that calls that method to check if the IP is in the cache, if so, it will skip the first filter. A downside of this method is that when multiple people from the same IP send a request, first with a valid authentication, then 1000 times without valid authentication, filter1 will let the latter through without rate limiting since the IP is already accepted. Maybe this can be prevented by removing the IP from the cache if spring security rejects authentication? something to think about

Edwin9292 avatar Feb 23 '24 18:02 Edwin9292

@MarcGiffing

you can set the first filter to the same filter-order as the second one.

For an anonymous user without a security context I would never reach a filter with filter order above 0.

So basically as I understand from this library's perspective I have these options:

  1. Use ONLY filter with order below 0 and treat every user the same - rate limit for specific endpoint will be the same for all users, does not matter you're authenticated or not.
  2. Use ONLY filter with order above 0 and rate limit only authenticated users (the problem I am facing right now, I cannot rate limit both authenticated and anomymous users).

So overall, I might be very mistaken, but the examples provided are incorrect - for example in ehcache example you've got a filter with order -200 and 1, but in fact only the first filter will ever be applied and never reach the second filter? Also, taking this example in documentation:

rate-limits:
      - execute-condition:  @securityService.notSignedIn() # only for not logged in users
        expression: "getRemoteAddr()"
        bandwidths:
        - capacity: 10
          time: 1
          unit: minutes

<...>
also check if user is admin/user
<...>

Again, how is this correct? The filter order is not specified, so by default it's negative. How do you detect if user is anonymous or admin, when we don't have security context? Am I missing something here?

All I was imagining that the library itself will perform practices to differentiate between anonymous and authenticated user, because now I can't understand the point of having rate limiting applied only for authenticated users (leaving anonymous users behind) or apply same rate limit for all users (having filter with negative order)

@Edwin9292

So in theory we say that authenticated user is someone that has gained a security context within Spring Boot, right? Mostly we have JWT or session based logins, thus if user is authenticated, he passes either JWT or session cookie. What about this:

  1. Anonymous user comes in (he does not have any JWT or session cookies). We apply strict rate limits by IP.
  2. User comes with JWT/session cookie, tries to access endpoint, Spring Boot allows it, thus post authorization triggers via MVC's dispatcher servlet, our rate limiter does trigger only mild rate limit by username (also we can check role, etc). We got spring session, context.
  3. User comes with fake/broken JWT/session cookie. Server does respond with error code. We apply strict rate limits by IP.

How is this approach? Is this possible?

cjacky475 avatar Feb 24 '24 14:02 cjacky475

@cjacky475 I'd say step 1 and 2 are possible by the solution i mentioned. But not 100% sure how to implement nr 3. Probably need to hook into the spring security or listen to authentication rejections in some way then. I'm sure there are ways to do that, just haven't done it before.

An other option could be like marc said, just put a general rate limiter in before spring security, and a more specific (on username for example) after spring security. Then you'd have to config the first filter with a limit thats higher than the max for the second (to prevent it from limiting valid requests) but lower than where someone could cripple the authentication system. edit: Probably need to use a different cache and/or key too then, so they dont get double rate limited.

Edwin9292 avatar Feb 24 '24 14:02 Edwin9292

@cjacky475 I'd say step 1 and 2 are possible by the solution i mentioned. But not 100% sure how to implement nr 3. Probably need to hook into the spring security or listen to authentication rejections in some way then. I'm sure there are ways to do that, just haven't done it before.

An other option could be like marc said, just put a general rate limiter in before spring security, and a more specific (on username for example) after spring security. Then you'd have to config the first filter with a limit thats higher than the max for the second (to prevent it from limiting valid requests) but lower than where someone could cripple the authentication system. edit: Probably need to use a different cache and/or key too then, so they dont get double rate limited.

Yes, probably in some way you mentioned. We can listen for failure via AuthenticationFailureHandler, can't we?

I am not sure how and where Marc mentioned "use a general rate limiter and a more specific one", since this thread is about the idea that this does not work. If you put a "general" rate limiter before spring security, all users fall under it and no other filters will be reached. Also, if that were not the case, then the idea (workaround of some sort as you mentioned) that first filter must have a limit that's higher than the max of the second defeats the purpose of the rate limiting, doesn't it? I probably want to allow as little as possible of traffic from anonymous users, and allow more for authenticated users, but via this examples, I allow at least (more) the amount of traffic for general/anonymous users comparing to what is allowed for authenticated users.

I believe the idea you mentioned + what I mentioned with broken JWT (in case someone tries to brake the model) must be implemented for this rate-limiting to work as it should, because as I mentioned now at best I can provide is a general rate limiting for all users or only for authenticated ones.

cjacky475 avatar Feb 24 '24 17:02 cjacky475

I am not sure how and where Marc mentioned "use a general rate limiter and a more specific one",

I might have understood his comment wrong then, my bad.

The AuthenticationFailureHandler looks like what you need. You could even switch it around. Create a handler that adds an IP to a map or cache when authentication fails. Then use a helper class (like the security service) with a method to check if the IP of the request sender is in that map. Use that method as execute condition. That way only people with a failed authentication will be rate limited by the first filter. You still probably want to remove them from the map/cache after a while or when they succesfully authenticate. This could for example be done by using an expiring cache and remove the IP from the map when they reach filter 2.

Just some suggestions :) If you have any other questions or need help with the implementation, feel free to ask.

Edwin9292 avatar Feb 24 '24 18:02 Edwin9292

OK, I thought about it. Maybe we can configure the Bucket4j filter to behave in two ways. A PRE_CHECK filter and a POST_CHECK filter(bad naming). The PRE_CHECK filter behaves like it is now. The POST_CHECK filter will not increase the rate limit counter on an incoming requests, but checks if the request should be rejected. If not the Spring Security filter is executed and detects that the user is not authenticated. So the request will rejected. In the Bucket4j filter, when the filter chain comes back, we can check the values in the HttpResponse and decide when the rate limit counter should be increased. If the rate limit is reached we also have to increase the counter directly.

Example:

bucket4j:
  enabled: true
  filters:
    - cache-name: rate-limit-buckets
      filter-method: servlet
      filter-kind: post_check
      url: .*
      rate-limits:
         execute-condition: 1 # MUST BE EXUCUTED FOR ALL USERS
         cache-key: getRemoteAddr()
         post-execute-condition: statusCode() eq 401 # User is Unauthorized
          bandwidths:
            - capacity: 1
              time: 10
              unit: seconds

The downside is, that we have to access the cache twice. When the request is incoming and if the post-execute-condition matches.

Does it make sense?

MarcGiffing avatar Feb 24 '24 18:02 MarcGiffing

Maybe we can configure the Bucket4j filter to behave in two ways. A PRE_CHECK filter and a POST_CHECK filter(bad naming). The PRE_CHECK filter behaves like it is now. The POST_CHECK filter will not increase the rate limit counter on an incoming requests, but checks if the request should be rejected. If not the Spring Security filter is executed and detects that the user is not authenticated. So the request will rejected. In the Bucket4j filter, when the filter chain comes back, we can check the values in the HttpResponse and decide when the rate limit counter should be increased. If the rate limit is reached we also have to increase the counter directly.

@MarcGiffing, I did not quite understand this whole scenario.

  1. So do you or you don't have to use this PRE_CHECK (pre-spring-auth) filter for all users? Because if you do, all authenticated and anonymous users are capped under same capacity for the first request, right?
  2. If you don't use PRE_CHECK (only POST_CHECK) (just looking at your example above), you are basically letting Spring Security handle the request and this is a red flag for DDOS.

Did I misunderstand something or how your idea solves this issue?

I believe @Edwin9292's idea was pretty neat and logical, basically keeping an IP cache of users that Spring Auth marked as failed, this way we will apply anonymous filter only for those who are under that IP cache. Simple, yet effective? Edwin, you could also create a PR for your example, not sure if this should be integrated in a library itself or what, maybe Marc meant something different with his idea that I couldn't understand.

cjacky475 avatar Feb 26 '24 06:02 cjacky475

@cjacky475

  1. Yes we have to implement a pre-spring-auth filter for all users. But this filter will not consume any tokens. It will get the current remaining tokens but will not consume it. Bucket4j provides the possibility to estimate the remaining tokens. It will only check if there are token left to consume.
    1. If there is no token left the rate limit is applied
    2. If there are tokens left Spring Security is executed.
      1. Spring Security authorize the user. Everything is OK. No tokens are consumed for this filter, end of the story. You an add additional filters only for authenticated users.
      2. If Spring Security rejects the requests with the Status Code 401 the post filter checks for the status code and consumes the token. So the tokens are consumed after Spring Security based on the post-execute-condition.

I think this solution is generic and can also be applied for other use cases.

I still think the IP-Blocking is not a solution for everyone. If you're users are in a company which shares the same IP you may have a problem.

I can provide a draft in the evening. It seems to work so far.

MarcGiffing avatar Feb 26 '24 08:02 MarcGiffing