Skip to content
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

Feature/openapi rate limit function #5267

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
1b3e122
feat(portal): Add current limiting function to ConsumerToken
youngzil Nov 3, 2024
8b9d09b
Merge branch 'master' into feature/openapi-rate-limit-function
youngzil Nov 3, 2024
1bae71f
fix:add CHANGES.md and optimize some codes
youngzil Nov 3, 2024
a4c84f8
Merge branch 'master' into feature/openapi-rate-limit-function
youngzil Nov 10, 2024
826ea74
feat(openapi): 重构 ConsumerToken 限流功能
youngzil Nov 17, 2024
9e2c4e8
refactor(Consumer): Spelling error in attribute
youngzil Nov 17, 2024
ace5076
Merge branch 'master' into feature/openapi-rate-limit-function
youngzil Nov 20, 2024
a622f56
refactor(openapi): Refactor consumer authentication filters and relat…
youngzil Nov 21, 2024
4939d70
test(apollo-portal): Optimize the rate limiting test of ConsumerAuthe…
youngzil Nov 21, 2024
a9da81d
Merge branch 'master' into feature/openapi-rate-limit-function
youngzil Nov 23, 2024
22ebb4f
featapi(open): Updated management page to show consumer rate limit in…
youngzil Nov 23, 2024
edb9d8b
fix(portal): Optimized the robustness of the code
youngzil Nov 23, 2024
6011aeb
fix(portal): Optimized the robustness of the code
youngzil Nov 23, 2024
8463974
fix(portal): fix unit test
youngzil Nov 23, 2024
0fe67f5
Merge branch 'refs/heads/master' into feature/openapi-rate-limit-func…
youngzil Nov 26, 2024
5390f62
feat(openapi): Added consumer rate limit query function and optimized…
youngzil Nov 26, 2024
9126bb3
fix(portal): Fix the processing logic when the consumer obtains an em…
youngzil Nov 27, 2024
ddfd8b3
refactor(ConsumerService): Optimize the implementation of getRateLimi…
youngzil Nov 27, 2024
46df20c
Update CHANGES.md
nobodyiam Nov 28, 2024
e07de19
Update CHANGES.md
nobodyiam Nov 28, 2024
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
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Apollo 2.4.0
* [Feature: openapi query namespace support not fill item](https://github.com/apolloconfig/apollo/pull/5249)
* [Refactor: align database ClusterName and NamespaceName fields lengths](https://github.com/apolloconfig/apollo/pull/5263)
* [Feature: Added the value length limit function for AppId-level configuration items](https://github.com/apolloconfig/apollo/pull/5264)
* [Feature: Added current limiting function to ConsumerToken](https://github.com/apolloconfig/apollo/pull/5267)
nobodyiam marked this conversation as resolved.
Show resolved Hide resolved

------------------
All issues and pull requests are [here](https://github.com/apolloconfig/apollo/milestone/15?closed=1)
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ public class ConsumerToken extends BaseEntity {
@Column(name = "`Token`", nullable = false)
private String token;

@Column(name = "LimitCount")
private Integer limitCount;

youngzil marked this conversation as resolved.
Show resolved Hide resolved
@Column(name = "`Expires`", nullable = false)
private Date expires;

Expand All @@ -60,6 +63,14 @@ public void setToken(String token) {
this.token = token;
}

public Integer getLimitCount() {
return limitCount;
}

public void setLimitCount(Integer limitCount) {
this.limitCount = limitCount;
}
youngzil marked this conversation as resolved.
Show resolved Hide resolved

public Date getExpires() {
return expires;
}
Expand All @@ -71,6 +82,7 @@ public void setExpires(Date expires) {
@Override
public String toString() {
return toStringHelper().add("consumerId", consumerId).add("token", token)
.add("limitCount", limitCount)
.add("expires", expires).toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,15 @@
*/
package com.ctrip.framework.apollo.openapi.filter;

import com.ctrip.framework.apollo.openapi.entity.ConsumerToken;
import com.ctrip.framework.apollo.openapi.util.ConsumerAuditUtil;
import com.ctrip.framework.apollo.openapi.util.ConsumerAuthUtil;

import com.ctrip.framework.apollo.portal.component.config.PortalConfig;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
import com.google.common.util.concurrent.RateLimiter;
import java.io.IOException;

import java.util.concurrent.TimeUnit;
import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
Expand All @@ -29,18 +33,33 @@
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.commons.lang3.tuple.ImmutablePair;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.http.HttpHeaders;

/**
* @author Jason Song([email protected])
*/
public class ConsumerAuthenticationFilter implements Filter {

private static final Logger logger = LoggerFactory.getLogger(ConsumerAuthenticationFilter.class);

private final ConsumerAuthUtil consumerAuthUtil;
private final ConsumerAuditUtil consumerAuditUtil;
private final PortalConfig portalConfig;
youngzil marked this conversation as resolved.
Show resolved Hide resolved

public ConsumerAuthenticationFilter(ConsumerAuthUtil consumerAuthUtil, ConsumerAuditUtil consumerAuditUtil) {
private static final int WARMUP_MILLIS = 1000; // ms
private static final int RATE_LIMITER_CACHE_MAX_SIZE = 50000;

private static final Cache<String, ImmutablePair<Long, RateLimiter>> LIMITER = CacheBuilder.newBuilder()
youngzil marked this conversation as resolved.
Show resolved Hide resolved
.expireAfterWrite(1, TimeUnit.DAYS)
youngzil marked this conversation as resolved.
Show resolved Hide resolved
.maximumSize(RATE_LIMITER_CACHE_MAX_SIZE).build();

public ConsumerAuthenticationFilter(ConsumerAuthUtil consumerAuthUtil, ConsumerAuditUtil consumerAuditUtil, PortalConfig portalConfig) {
this.consumerAuthUtil = consumerAuthUtil;
this.consumerAuditUtil = consumerAuditUtil;
this.portalConfig = portalConfig;
}

@Override
Expand All @@ -55,14 +74,33 @@ public void doFilter(ServletRequest req, ServletResponse resp, FilterChain chain
HttpServletResponse response = (HttpServletResponse) resp;

String token = request.getHeader(HttpHeaders.AUTHORIZATION);
ConsumerToken consumerToken = consumerAuthUtil.getConsumerToken(token);

Long consumerId = consumerAuthUtil.getConsumerId(token);

if (consumerId == null) {
if (null == consumerToken || consumerToken.getConsumerId() <= 0) {
youngzil marked this conversation as resolved.
Show resolved Hide resolved
response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized");
return;
}

Integer limitCount = consumerToken.getLimitCount();
if (limitCount == null) {
limitCount = 0;
}
if (portalConfig.isOpenApiLimitEnabled() && limitCount > 0) {
try {
ImmutablePair<Long, RateLimiter> rateLimiterPair = getOrCreateRateLimiterPair(token, limitCount);
long warmupToMillis = rateLimiterPair.getLeft() + WARMUP_MILLIS;
nobodyiam marked this conversation as resolved.
Show resolved Hide resolved
if (System.currentTimeMillis() > warmupToMillis && !rateLimiterPair.getRight().tryAcquire()) {
response.sendError(HttpServletResponse.SC_FORBIDDEN, "Too many call requests, the flow is limited");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is 429 more suitable for this scenario?

return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Potential race condition in warmup check

The warmup check compares two timestamps without synchronization, which could lead to inconsistent behavior in high-concurrency scenarios. Consider using atomic operations or moving the warmup logic into the RateLimiter itself.

- long warmupToMillis = rateLimiterPair.getLeft() + WARMUP_MILLIS;
- if (System.currentTimeMillis() > warmupToMillis && !rateLimiterPair.getRight().tryAcquire()) {
+ RateLimiter limiter = rateLimiterPair.getRight();
+ if (!limiter.tryAcquire()) {
   response.sendError(HttpServletResponse.SC_FORBIDDEN, "Too many call requests, the flow is limited");
   return;
 }

Committable suggestion skipped: line range outside the PR's diff.

} catch (Exception e) {
logger.error("ConsumerAuthenticationFilter ratelimit error", e);
response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "Rate limiting failed");
return;
}
}

long consumerId = consumerToken.getConsumerId();
consumerAuthUtil.storeConsumerId(request, consumerId);
consumerAuditUtil.audit(request, consumerId);

Expand All @@ -73,4 +111,14 @@ public void doFilter(ServletRequest req, ServletResponse resp, FilterChain chain
public void destroy() {
//nothing
}

private ImmutablePair<Long, RateLimiter> getOrCreateRateLimiterPair(String key, Integer limitCount) {
ImmutablePair<Long, RateLimiter> rateLimiterPair = LIMITER.getIfPresent(key);
if (rateLimiterPair == null) {
rateLimiterPair = ImmutablePair.of(System.currentTimeMillis(), RateLimiter.create(limitCount));
LIMITER.put(key, rateLimiterPair);
}
return rateLimiterPair;
}
youngzil marked this conversation as resolved.
Show resolved Hide resolved

}
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,15 @@ public ConsumerToken getConsumerTokenByAppId(String appId) {
return consumerTokenRepository.findByConsumerId(consumer.getId());
}

public Long getConsumerIdByToken(String token) {
public ConsumerToken getConsumerTokenByToken(String token) {
if (Strings.isNullOrEmpty(token)) {
return null;
}
ConsumerToken consumerToken = consumerTokenRepository.findTopByTokenAndExpiresAfter(token,
new Date());
return consumerTokenRepository.findTopByTokenAndExpiresAfter(token, new Date());
}

public Long getConsumerIdByToken(String token) {
ConsumerToken consumerToken = getConsumerTokenByToken(token);
return consumerToken == null ? null : consumerToken.getConsumerId();
}

Expand Down Expand Up @@ -311,7 +314,9 @@ public void createConsumerAudits(Iterable<ConsumerAudit> consumerAudits) {
@Transactional
public ConsumerToken createConsumerToken(ConsumerToken entity) {
entity.setId(0); //for protection

if (entity.getLimitCount() <= 0) {
entity.setLimitCount(portalConfig.openApiLimitCount());
}
youngzil marked this conversation as resolved.
Show resolved Hide resolved
return consumerTokenRepository.save(entity);
}

Expand All @@ -322,6 +327,7 @@ private ConsumerToken generateConsumerToken(Consumer consumer, Date expires) {

ConsumerToken consumerToken = new ConsumerToken();
consumerToken.setConsumerId(consumerId);
consumerToken.setLimitCount(portalConfig.openApiLimitCount());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rate limit should be set on the UI page, with a default value that users can modify.

consumerToken.setExpires(expires);
consumerToken.setDataChangeCreatedBy(createdBy);
consumerToken.setDataChangeCreatedTime(createdTime);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package com.ctrip.framework.apollo.openapi.util;

import com.ctrip.framework.apollo.openapi.entity.ConsumerToken;
import com.ctrip.framework.apollo.openapi.service.ConsumerService;
import org.springframework.stereotype.Service;

Expand All @@ -37,6 +38,10 @@ public Long getConsumerId(String token) {
return consumerService.getConsumerIdByToken(token);
}

public ConsumerToken getConsumerToken(String token) {
return consumerService.getConsumerTokenByToken(token);
}
nobodyiam marked this conversation as resolved.
Show resolved Hide resolved

public void storeConsumerId(HttpServletRequest request, Long consumerId) {
request.setAttribute(CONSUMER_ID, consumerId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,14 @@ public String consumerTokenSalt() {
return getValue("consumer.token.salt", "apollo-portal");
}

public int openApiLimitCount() {
return getIntProperty("open.api.limit.count", 20);
}

public boolean isOpenApiLimitEnabled() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since rate limits are set at the token level, adding a global flag seems unnecessary. Instead, we could add an "Enable Rate Limit" option on the consumer token creation page. Users can toggle this option to configure the rate limit if desired; otherwise, leaving it unchecked will set the rate limit to 0.

return getBooleanProperty("open.api.limit.enabled", false);
}

public boolean isEmailEnabled() {
return getBooleanProperty("email.enabled", false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.ctrip.framework.apollo.openapi.filter.ConsumerAuthenticationFilter;
import com.ctrip.framework.apollo.openapi.util.ConsumerAuditUtil;
import com.ctrip.framework.apollo.openapi.util.ConsumerAuthUtil;
import com.ctrip.framework.apollo.portal.component.config.PortalConfig;
import org.springframework.boot.web.servlet.FilterRegistrationBean;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
Expand All @@ -28,12 +29,13 @@ public class AuthFilterConfiguration {

@Bean
public FilterRegistrationBean<ConsumerAuthenticationFilter> openApiAuthenticationFilter(
ConsumerAuthUtil consumerAuthUtil,
ConsumerAuditUtil consumerAuditUtil) {
ConsumerAuthUtil consumerAuthUtil,
ConsumerAuditUtil consumerAuditUtil,
PortalConfig portalConfig) {

FilterRegistrationBean<ConsumerAuthenticationFilter> openApiFilter = new FilterRegistrationBean<>();

openApiFilter.setFilter(new ConsumerAuthenticationFilter(consumerAuthUtil, consumerAuditUtil));
openApiFilter.setFilter(new ConsumerAuthenticationFilter(consumerAuthUtil, consumerAuditUtil, portalConfig));
openApiFilter.addUrlPatterns("/openapi/*");

return openApiFilter;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.ctrip.framework.apollo;

import com.ctrip.framework.apollo.openapi.auth.ConsumerPermissionValidator;
import com.ctrip.framework.apollo.openapi.entity.ConsumerToken;
import com.ctrip.framework.apollo.openapi.util.ConsumerAuthUtil;
import com.ctrip.framework.apollo.portal.component.PermissionValidator;
import org.springframework.context.annotation.Bean;
Expand Down Expand Up @@ -50,6 +51,11 @@ public ConsumerPermissionValidator consumerPermissionValidator() {
public ConsumerAuthUtil consumerAuthUtil() {
final ConsumerAuthUtil mock = mock(ConsumerAuthUtil.class);
when(mock.getConsumerId(any())).thenReturn(1L);

ConsumerToken someConsumerToken = new ConsumerToken();
someConsumerToken.setConsumerId(1L);
someConsumerToken.setLimitCount(20);
when(mock.getConsumerToken(any())).thenReturn(someConsumerToken);
return mock;
}

Expand Down
Loading
Loading