From bcb186652dd7b3535812c13e8f3c6830544f37be Mon Sep 17 00:00:00 2001 From: vsxd Date: Wed, 18 Mar 2026 11:30:15 +0800 Subject: [PATCH] Add anonymous download rate limiting --- .../config/DownloadRateLimitProperties.java | 38 +++++ .../skillhub/filter/RequestLoggingFilter.java | 13 +- .../AnonymousDownloadIdentityService.java | 131 ++++++++++++++++++ .../skillhub/ratelimit/ClientIpResolver.java | 54 ++++++++ .../ratelimit/RateLimitInterceptor.java | 51 ++++--- .../src/main/resources/application.yml | 5 + .../DownloadRateLimitControllerTest.java | 102 ++++++++++++++ .../filter/RequestLoggingFilterTest.java | 48 +++++++ 8 files changed, 423 insertions(+), 19 deletions(-) create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/config/DownloadRateLimitProperties.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/ratelimit/AnonymousDownloadIdentityService.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/ratelimit/ClientIpResolver.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/DownloadRateLimitControllerTest.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/RequestLoggingFilterTest.java diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/config/DownloadRateLimitProperties.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/config/DownloadRateLimitProperties.java new file mode 100644 index 00000000..3d922e90 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/config/DownloadRateLimitProperties.java @@ -0,0 +1,38 @@ +package com.iflytek.skillhub.config; + +import java.time.Duration; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.stereotype.Component; + +@Component +@ConfigurationProperties(prefix = "skillhub.ratelimit.download") +public class DownloadRateLimitProperties { + + private String anonymousCookieName = "skillhub_anon_dl"; + private Duration anonymousCookieMaxAge = Duration.ofDays(30); + private String anonymousCookieSecret = "change-me-in-production"; + + public String getAnonymousCookieName() { + return anonymousCookieName; + } + + public void setAnonymousCookieName(String anonymousCookieName) { + this.anonymousCookieName = anonymousCookieName; + } + + public Duration getAnonymousCookieMaxAge() { + return anonymousCookieMaxAge; + } + + public void setAnonymousCookieMaxAge(Duration anonymousCookieMaxAge) { + this.anonymousCookieMaxAge = anonymousCookieMaxAge; + } + + public String getAnonymousCookieSecret() { + return anonymousCookieSecret; + } + + public void setAnonymousCookieSecret(String anonymousCookieSecret) { + this.anonymousCookieSecret = anonymousCookieSecret; + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/RequestLoggingFilter.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/RequestLoggingFilter.java index 7c46bd34..7783f3c3 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/RequestLoggingFilter.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/RequestLoggingFilter.java @@ -24,6 +24,7 @@ public class RequestLoggingFilter extends OncePerRequestFilter { private static final Logger log = LoggerFactory.getLogger(RequestLoggingFilter.class); + private static final int MAX_LOG_BODY_LENGTH = 512; @Override protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) @@ -86,7 +87,7 @@ private String getRequestBody(ContentCachingRequestWrapper request) { byte[] buf = request.getContentAsByteArray(); if (buf.length > 0) { try { - return new String(buf, request.getCharacterEncoding()); + return truncateBody(new String(buf, request.getCharacterEncoding())); } catch (UnsupportedEncodingException e) { return "[unknown encoding]"; } @@ -98,11 +99,19 @@ private String getResponseBody(ContentCachingResponseWrapper response) { byte[] buf = response.getContentAsByteArray(); if (buf.length > 0) { try { - return new String(buf, response.getCharacterEncoding()); + return truncateBody(new String(buf, response.getCharacterEncoding())); } catch (UnsupportedEncodingException e) { return "[unknown encoding]"; } } return null; } + + private String truncateBody(String body) { + if (body == null || body.length() <= MAX_LOG_BODY_LENGTH) { + return body; + } + return body.substring(0, MAX_LOG_BODY_LENGTH) + + "... [truncated, original length=" + body.length() + "]"; + } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/ratelimit/AnonymousDownloadIdentityService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/ratelimit/AnonymousDownloadIdentityService.java new file mode 100644 index 00000000..dee29900 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/ratelimit/AnonymousDownloadIdentityService.java @@ -0,0 +1,131 @@ +package com.iflytek.skillhub.ratelimit; + +import com.iflytek.skillhub.config.DownloadRateLimitProperties; +import jakarta.servlet.http.Cookie; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import java.nio.charset.StandardCharsets; +import java.security.GeneralSecurityException; +import java.security.MessageDigest; +import java.security.SecureRandom; +import java.time.Duration; +import java.util.Arrays; +import java.util.Base64; +import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; +import org.springframework.http.ResponseCookie; +import org.springframework.stereotype.Component; + +@Component +public class AnonymousDownloadIdentityService { + + private static final String COOKIE_VERSION = "v1"; + private static final SecureRandom RANDOM = new SecureRandom(); + + private final DownloadRateLimitProperties properties; + private final ClientIpResolver clientIpResolver; + + public AnonymousDownloadIdentityService(DownloadRateLimitProperties properties, + ClientIpResolver clientIpResolver) { + this.properties = properties; + this.clientIpResolver = clientIpResolver; + } + + public AnonymousDownloadIdentity resolve(HttpServletRequest request, HttpServletResponse response) { + String ip = clientIpResolver.resolve(request); + String cookieId = extractValidCookieId(request); + if (cookieId == null) { + cookieId = generateId(); + response.addHeader("Set-Cookie", buildCookie(cookieId, request).toString()); + } + return new AnonymousDownloadIdentity(hash(ip), hash(cookieId)); + } + + private String extractValidCookieId(HttpServletRequest request) { + Cookie[] cookies = request.getCookies(); + if (cookies == null) { + return null; + } + return Arrays.stream(cookies) + .filter(cookie -> properties.getAnonymousCookieName().equals(cookie.getName())) + .map(Cookie::getValue) + .map(this::parseAndVerify) + .filter(value -> value != null && !value.isBlank()) + .findFirst() + .orElse(null); + } + + private String parseAndVerify(String cookieValue) { + if (cookieValue == null) { + return null; + } + String[] parts = cookieValue.split("\\.", 3); + if (parts.length != 3 || !COOKIE_VERSION.equals(parts[0])) { + return null; + } + byte[] expected = sign(parts[1]); + byte[] actual; + try { + actual = Base64.getUrlDecoder().decode(parts[2]); + } catch (IllegalArgumentException ex) { + return null; + } + return MessageDigest.isEqual(expected, actual) ? parts[1] : null; + } + + private ResponseCookie buildCookie(String cookieId, HttpServletRequest request) { + Duration maxAge = properties.getAnonymousCookieMaxAge(); + return ResponseCookie.from(properties.getAnonymousCookieName(), encodeCookieValue(cookieId)) + .httpOnly(true) + .secure(isSecure(request)) + .sameSite("Lax") + .path("/") + .maxAge(maxAge) + .build(); + } + + private boolean isSecure(HttpServletRequest request) { + if (request.isSecure()) { + return true; + } + String forwardedProto = request.getHeader("X-Forwarded-Proto"); + return forwardedProto != null && forwardedProto.equalsIgnoreCase("https"); + } + + private String encodeCookieValue(String id) { + return COOKIE_VERSION + "." + id + "." + Base64.getUrlEncoder().withoutPadding().encodeToString(sign(id)); + } + + private byte[] sign(String value) { + try { + Mac mac = Mac.getInstance("HmacSHA256"); + mac.init(new SecretKeySpec(properties.getAnonymousCookieSecret().getBytes(StandardCharsets.UTF_8), "HmacSHA256")); + return mac.doFinal(value.getBytes(StandardCharsets.UTF_8)); + } catch (GeneralSecurityException ex) { + throw new IllegalStateException("Failed to sign anonymous download cookie", ex); + } + } + + private String generateId() { + byte[] bytes = new byte[16]; + RANDOM.nextBytes(bytes); + return Base64.getUrlEncoder().withoutPadding().encodeToString(bytes); + } + + private String hash(String raw) { + try { + MessageDigest digest = MessageDigest.getInstance("SHA-256"); + byte[] bytes = digest.digest(raw.getBytes(StandardCharsets.UTF_8)); + StringBuilder builder = new StringBuilder(bytes.length * 2); + for (byte b : bytes) { + builder.append(String.format("%02x", b)); + } + return builder.toString(); + } catch (GeneralSecurityException ex) { + throw new IllegalStateException("Failed to hash anonymous download identity", ex); + } + } + + public record AnonymousDownloadIdentity(String ipHash, String cookieHash) { + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/ratelimit/ClientIpResolver.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/ratelimit/ClientIpResolver.java new file mode 100644 index 00000000..816b6e3c --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/ratelimit/ClientIpResolver.java @@ -0,0 +1,54 @@ +package com.iflytek.skillhub.ratelimit; + +import jakarta.servlet.http.HttpServletRequest; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import org.springframework.stereotype.Component; + +@Component +public class ClientIpResolver { + + private static final Pattern FORWARDED_FOR_PATTERN = Pattern.compile("for=\"?\\[?([^;,\"]+)\\]?\"?"); + + public String resolve(HttpServletRequest request) { + String forwarded = trimToNull(request.getHeader("Forwarded")); + if (forwarded != null) { + Matcher matcher = FORWARDED_FOR_PATTERN.matcher(forwarded); + if (matcher.find()) { + return normalizeCandidate(matcher.group(1)); + } + } + + String xForwardedFor = trimToNull(request.getHeader("X-Forwarded-For")); + if (xForwardedFor != null) { + return normalizeCandidate(xForwardedFor.split(",")[0]); + } + + String xRealIp = trimToNull(request.getHeader("X-Real-IP")); + if (xRealIp != null) { + return normalizeCandidate(xRealIp); + } + + return normalizeCandidate(request.getRemoteAddr()); + } + + private String trimToNull(String value) { + if (value == null) { + return null; + } + String trimmed = value.trim(); + return trimmed.isEmpty() || "unknown".equalsIgnoreCase(trimmed) ? null : trimmed; + } + + private String normalizeCandidate(String candidate) { + String normalized = trimToNull(candidate); + if (normalized == null) { + return "unknown"; + } + int zoneIndex = normalized.indexOf('%'); + if (zoneIndex >= 0) { + normalized = normalized.substring(0, zoneIndex); + } + return normalized; + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/ratelimit/RateLimitInterceptor.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/ratelimit/RateLimitInterceptor.java index f1cf5ef7..102c9757 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/ratelimit/RateLimitInterceptor.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/ratelimit/RateLimitInterceptor.java @@ -15,13 +15,19 @@ public class RateLimitInterceptor implements HandlerInterceptor { private final RateLimiter rateLimiter; + private final ClientIpResolver clientIpResolver; + private final AnonymousDownloadIdentityService anonymousDownloadIdentityService; private final ApiResponseFactory apiResponseFactory; private final ObjectMapper objectMapper; public RateLimitInterceptor(RateLimiter rateLimiter, + ClientIpResolver clientIpResolver, + AnonymousDownloadIdentityService anonymousDownloadIdentityService, ApiResponseFactory apiResponseFactory, ObjectMapper objectMapper) { this.rateLimiter = rateLimiter; + this.clientIpResolver = clientIpResolver; + this.anonymousDownloadIdentityService = anonymousDownloadIdentityService; this.apiResponseFactory = apiResponseFactory; this.objectMapper = objectMapper; } @@ -46,12 +52,9 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons // Get limit based on authentication status int limit = isAuthenticated ? rateLimit.authenticated() : rateLimit.anonymous(); - // Build rate limit key - String identifier = isAuthenticated ? "user:" + userId : "ip:" + getClientIp(request); - String key = "ratelimit:" + rateLimit.category() + ":" + identifier; - - // Check rate limit - boolean allowed = rateLimiter.tryAcquire(key, limit, rateLimit.windowSeconds()); + boolean allowed = isAuthenticated + ? rateLimiter.tryAcquire("ratelimit:" + rateLimit.category() + ":user:" + userId, limit, rateLimit.windowSeconds()) + : checkAnonymousLimit(request, response, rateLimit, limit); if (!allowed) { response.setStatus(HttpStatus.TOO_MANY_REQUESTS.value()); @@ -64,18 +67,32 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons return true; } - private String getClientIp(HttpServletRequest request) { - String ip = request.getHeader("X-Forwarded-For"); - if (ip == null || ip.isEmpty() || "unknown".equalsIgnoreCase(ip)) { - ip = request.getHeader("X-Real-IP"); - } - if (ip == null || ip.isEmpty() || "unknown".equalsIgnoreCase(ip)) { - ip = request.getRemoteAddr(); + private boolean checkAnonymousLimit(HttpServletRequest request, + HttpServletResponse response, + RateLimit rateLimit, + int limit) { + if (!"download".equals(rateLimit.category())) { + return rateLimiter.tryAcquire( + "ratelimit:" + rateLimit.category() + ":ip:" + clientIpResolver.resolve(request), + limit, + rateLimit.windowSeconds() + ); } - // Take first IP if multiple - if (ip != null && ip.contains(",")) { - ip = ip.split(",")[0].trim(); + + AnonymousDownloadIdentityService.AnonymousDownloadIdentity identity = + anonymousDownloadIdentityService.resolve(request, response); + boolean ipAllowed = rateLimiter.tryAcquire( + "ratelimit:download:ip:" + identity.ipHash(), + limit, + rateLimit.windowSeconds() + ); + if (!ipAllowed) { + return false; } - return ip; + return rateLimiter.tryAcquire( + "ratelimit:download:anon:" + identity.cookieHash(), + limit, + rateLimit.windowSeconds() + ); } } diff --git a/server/skillhub-app/src/main/resources/application.yml b/server/skillhub-app/src/main/resources/application.yml index c5e36b50..285f5c98 100644 --- a/server/skillhub-app/src/main/resources/application.yml +++ b/server/skillhub-app/src/main/resources/application.yml @@ -92,6 +92,11 @@ skillhub: weight: 0.35 candidate-multiplier: 8 max-candidates: 120 + ratelimit: + download: + anonymous-cookie-name: ${SKILLHUB_DOWNLOAD_ANON_COOKIE_NAME:skillhub_anon_dl} + anonymous-cookie-max-age: ${SKILLHUB_DOWNLOAD_ANON_COOKIE_MAX_AGE:P30D} + anonymous-cookie-secret: ${SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET:change-me-in-production} publish: max-file-count: 100 max-single-file-size: 1048576 # 1MB diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/DownloadRateLimitControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/DownloadRateLimitControllerTest.java new file mode 100644 index 00000000..c1809e45 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/DownloadRateLimitControllerTest.java @@ -0,0 +1,102 @@ +package com.iflytek.skillhub.controller.portal; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.iflytek.skillhub.TestRedisConfig; +import com.iflytek.skillhub.auth.device.DeviceAuthService; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.domain.skill.service.SkillDownloadService; +import com.iflytek.skillhub.domain.skill.service.SkillQueryService; +import com.iflytek.skillhub.ratelimit.RateLimiter; +import java.io.ByteArrayInputStream; +import java.util.Map; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.annotation.Import; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.web.servlet.MockMvc; + +@SpringBootTest +@AutoConfigureMockMvc +@ActiveProfiles("test") +@Import(TestRedisConfig.class) +class DownloadRateLimitControllerTest { + + @Autowired + private MockMvc mockMvc; + + @MockBean + private SkillQueryService skillQueryService; + + @MockBean + private SkillDownloadService skillDownloadService; + + @MockBean + private NamespaceMemberRepository namespaceMemberRepository; + + @MockBean + private DeviceAuthService deviceAuthService; + + @MockBean + private RateLimiter rateLimiter; + + @Test + void anonymousDownloadUsesIpAndSignedCookieBuckets() throws Exception { + given(rateLimiter.tryAcquire(anyString(), anyInt(), anyInt())).willReturn(true); + given(skillDownloadService.downloadVersion("global", "demo-skill", "1.0.0", null, Map.of())) + .willReturn(new SkillDownloadService.DownloadResult( + new ByteArrayInputStream("zip".getBytes()), + "demo-skill-1.0.0.zip", + 3L, + "application/zip", + null + )); + + var result = mockMvc.perform(get("/api/v1/skills/global/demo-skill/versions/1.0.0/download") + .header("X-Forwarded-For", "203.0.113.10")) + .andExpect(status().isOk()) + .andExpect(header().string("Content-Disposition", "attachment; filename=\"demo-skill-1.0.0.zip\"")) + .andExpect(header().exists("Set-Cookie")) + .andReturn(); + + assertThat(result.getResponse().getHeaders("Set-Cookie")) + .anySatisfy(cookie -> { + assertThat(cookie).contains("skillhub_anon_dl="); + assertThat(cookie).contains("HttpOnly"); + assertThat(cookie).contains("SameSite=Lax"); + }); + + ArgumentCaptor keyCaptor = ArgumentCaptor.forClass(String.class); + verify(rateLimiter, times(2)).tryAcquire(keyCaptor.capture(), anyInt(), anyInt()); + assertThat(keyCaptor.getAllValues()).anyMatch(key -> key.startsWith("ratelimit:download:ip:")); + assertThat(keyCaptor.getAllValues()).anyMatch(key -> key.startsWith("ratelimit:download:anon:")); + } + + @Test + void anonymousDownloadReturnsTooManyRequestsWhenIpBucketIsExceeded() throws Exception { + given(rateLimiter.tryAcquire(anyString(), anyInt(), anyInt())).willAnswer(invocation -> + ((String) invocation.getArgument(0)).startsWith("ratelimit:download:ip:") ? false : true); + + mockMvc.perform(get("/api/v1/skills/global/demo-skill/versions/1.0.0/download") + .header("X-Forwarded-For", "203.0.113.10")) + .andExpect(status().isTooManyRequests()) + .andExpect(jsonPath("$.code").value(429)); + + verify(skillDownloadService, never()).downloadVersion(anyString(), anyString(), anyString(), anyString(), any()); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/RequestLoggingFilterTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/RequestLoggingFilterTest.java new file mode 100644 index 00000000..7d74d3ff --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/RequestLoggingFilterTest.java @@ -0,0 +1,48 @@ +package com.iflytek.skillhub.filter; + +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.boot.test.system.CapturedOutput; +import org.springframework.boot.test.system.OutputCaptureExtension; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; + +import static org.assertj.core.api.Assertions.assertThat; + +@ExtendWith(OutputCaptureExtension.class) +class RequestLoggingFilterTest { + + @Test + void doFilterInternal_truncatesLongRequestAndResponseBodiesInLogs(CapturedOutput output) + throws ServletException, IOException { + RequestLoggingFilter filter = new RequestLoggingFilter(); + String longBody = "x".repeat(5_000); + + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/api/test"); + request.setCharacterEncoding(StandardCharsets.UTF_8.name()); + request.setContentType("application/json"); + request.setContent(longBody.getBytes(StandardCharsets.UTF_8)); + + MockHttpServletResponse response = new MockHttpServletResponse(); + response.setCharacterEncoding(StandardCharsets.UTF_8.name()); + + FilterChain filterChain = (req, res) -> { + req.getReader().lines().count(); + res.setContentType("application/json"); + res.getWriter().write(longBody); + }; + + filter.doFilter(request, response, filterChain); + + assertThat(output).contains("Request Body: " + "x".repeat(512) + "... [truncated, original length=5000]"); + assertThat(output).contains("Response Body: " + "x".repeat(512) + "... [truncated, original length=5000]"); + assertThat(output).doesNotContain("Request Body: " + longBody); + assertThat(output).doesNotContain("Response Body: " + longBody); + assertThat(response.getContentAsString()).isEqualTo(longBody); + } +}