Skip to content

Commit 8addd63

Browse files
authored
Fix dynamic rate limiting for customers in state evaluation (eclipse#1623)
* fix access control expose headers for dev setup * use UserService to ensure only valid tokens are taken into account * only apply free tier limit if we did not identify a customer
1 parent e7eb26e commit 8addd63

File tree

3 files changed

+18
-8
lines changed

3 files changed

+18
-8
lines changed

server/src/dev/resources/application.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ ovsx:
177177
- url: '/(api|vscode)/.*'
178178
http-response-headers:
179179
Access-Control-Allow-Origin: '*'
180-
Access-Control-Expose-Headers: X-Rate-Limit-Retry-After-Seconds, X-Rate-Limit-Remaining
180+
Access-Control-Expose-Headers: X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, Retry-After
181181
default-http-content-type: application/json
182182
default-http-response-body: >
183183
{

server/src/main/java/org/eclipse/openvsx/ratelimit/IdentityService.java

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import com.giffing.bucket4j.spring.boot.starter.context.ExpressionParams;
1616
import jakarta.servlet.http.HttpServletRequest;
17+
import org.eclipse.openvsx.UserService;
1718
import org.eclipse.openvsx.ratelimit.config.RateLimitConfig;
1819
import org.eclipse.openvsx.ratelimit.config.RateLimitProperties;
1920
import org.slf4j.Logger;
@@ -38,19 +39,22 @@ public class IdentityService {
3839

3940
private final TierService tierService;
4041
private final CustomerService customerService;
42+
private final UserService userService;
4143
private final RateLimitProperties rateLimitProperties;
4244

4345
public IdentityService(
4446
ExpressionParser expressionParser,
4547
ConfigurableBeanFactory beanFactory,
4648
TierService tierService,
4749
CustomerService customerService,
50+
UserService userService,
4851
RateLimitProperties rateLimitProperties
4952
) {
5053
this.expressionParser = expressionParser;
5154
this.beanFactory = beanFactory;
5255
this.tierService = tierService;
5356
this.customerService = customerService;
57+
this.userService = userService;
5458
this.rateLimitProperties = rateLimitProperties;
5559
}
5660

@@ -60,9 +64,14 @@ public ResolvedIdentity resolveIdentity(HttpServletRequest request) {
6064

6165
var token = request.getParameter("token");
6266
if (token != null) {
63-
// we use the token as a cache key
64-
// if the token is invalid, request will fail anyway
65-
cacheKey = "token_" + token.hashCode();
67+
// This will update the database with the time the token is last accessed,
68+
// but we need to ensure that we only take valid tokens into account for rate limiting.
69+
// If this turns out to be a bottleneck, we need to cache the token hashcode.
70+
var tokenEntity = userService.useAccessToken(token);
71+
if (tokenEntity != null) {
72+
// if a valid token is present we use it as a cache key
73+
cacheKey = "token_" + token.hashCode();
74+
}
6675
}
6776

6877
var customer = customerService.getCustomerByIpAddress(ipAddress);
@@ -75,7 +84,6 @@ public ResolvedIdentity resolveIdentity(HttpServletRequest request) {
7584
var sessionId = session != null ? session.getId() : null;
7685
if (sessionId != null) {
7786
// we use the session id as a cache key
78-
// if the session is invalid, request will fail anyway
7987
cacheKey = "session_" + sessionId.hashCode();
8088
}
8189
}

server/src/main/java/org/eclipse/openvsx/ratelimit/RateLimitService.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,17 +108,19 @@ public BucketPair getBucket(ResolvedIdentity identity) {
108108
var builder = BucketConfiguration.builder();
109109

110110
var hasLimits = false;
111+
var useCustomerBandwidth = identity.cacheKey().startsWith("customer_");
111112

112-
if (identity.customer() != null && identity.cacheKey().startsWith("customer_")) {
113+
// if this request is coming from an identified customer
114+
if (identity.customer() != null && useCustomerBandwidth) {
113115
var customer = identity.customer();
114116
if (customer.getState() == EnforcementState.ENFORCEMENT) {
115117
builder.addLimit(getBandWidth(customer.getTier()));
116118
hasLimits = true;
117119
}
118120
}
119121

120-
// add the free tier bandwidth if no limits have been set so far
121-
if (!hasLimits && identity.freeTier() != null) {
122+
// add the free tier bandwidth for any other requests if available
123+
if (!useCustomerBandwidth && identity.freeTier() != null) {
122124
builder.addLimit(getBandWidth(identity.freeTier()));
123125
hasLimits = true;
124126
}

0 commit comments

Comments
 (0)