feat: harden backend security controls

.github/workflows/ci.yml

@@ -20,6 +20,16 @@ jobs:
           cache: maven
       - name: Run tests
         run: mvn -B -ntp test
+      - name: Build backend image for scan
+        run: docker build -t transaction-api:${{ github.sha }} -f Dockerfile .
+      - name: Scan backend image
+        uses: aquasecurity/trivy-action@v0.36.0
+        with:
+          image-ref: transaction-api:${{ github.sha }}
+          vuln-type: os,library
+          severity: CRITICAL
+          ignore-unfixed: true
+          exit-code: "1"
 
   deploy-dev:
     needs: test

DEV.md

@@ -119,6 +119,8 @@ Required:
 Optional:
 - `APP_SECURITY_ADMIN_EMAILS` (comma-separated admin allowlist)
 - `APP_SECURITY_JWT_DYNAMO_MAX_STALE=PT72H`
+- `APP_RATE_LIMIT_TRUST_FORWARDED_HEADERS=false`
+- `APP_RATE_LIMIT_MAX_BUCKETS=10000`
 - `APP_SESSION_TIMEOUT=PT2H`
 - `APP_SESSION_COOKIE_SECURE=true`
 - `APP_SESSION_COOKIE_SAME_SITE=lax`

pom.xml

@@ -10,7 +10,7 @@
     <description>Spring Boot service for recording financial transactions</description>
     <properties>
         <java.version>21</java.version>
-        <spring-boot.version>3.5.0</spring-boot.version>
+        <spring-boot.version>3.5.15</spring-boot.version>
         <aws.sdk.version>2.41.17</aws.sdk.version>
     </properties>
 

src/main/java/com/transactionapi/controller/AuthController.java

@@ -104,7 +104,7 @@ public UserProfileResponse login(
         SecurityContextHolder.setContext(context);
         securityContextRepository.saveContext(context, request, response);
 
-        return UserProfileResponse.from(user);
+        return UserProfileResponse.from(user, userIdResolver.isAdmin(jwtAuthentication));
     }
 
     @PostMapping("/logout")

src/main/java/com/transactionapi/controller/UserController.java

@@ -2,6 +2,7 @@
 
 import com.transactionapi.constants.ApiPaths;
 import com.transactionapi.dto.UserProfileResponse;
+import com.transactionapi.model.User;
 import com.transactionapi.security.UserIdResolver;
 import com.transactionapi.service.UserService;
 import org.springframework.security.core.Authentication;
@@ -26,13 +27,15 @@ public UserController(UserIdResolver userIdResolver, UserService userService) {
     public UserProfileResponse getProfile(Authentication authentication) {
         String userId = userIdResolver.requireUserId(authentication);
         String email = userIdResolver.resolveEmail(authentication);
-        return UserProfileResponse.from(userService.getOrCreateUser(userId, email));
+        User user = userService.getOrCreateUser(userId, email);
+        return UserProfileResponse.from(user, userIdResolver.isAdmin(authentication));
     }
 
     @PostMapping("/legal-agreement")
     public UserProfileResponse acceptLegalAgreement(Authentication authentication) {
         String userId = userIdResolver.requireUserId(authentication);
         String email = userIdResolver.resolveEmail(authentication);
-        return UserProfileResponse.from(userService.acceptLegalAgreement(userId, email));
+        User user = userService.acceptLegalAgreement(userId, email);
+        return UserProfileResponse.from(user, userIdResolver.isAdmin(authentication));
     }
 }

src/main/java/com/transactionapi/dto/UserProfileResponse.java

@@ -16,6 +16,7 @@ public record UserProfileResponse(
         UUID id,
         String authId,
         String email,
+        boolean admin,
         boolean premium,
         Instant createdAt,
         Instant updatedAt,
@@ -32,6 +33,10 @@ public record UserProfileResponse(
         Instant privacyPolicyAcceptedAt
 ) {
     public static UserProfileResponse from(User user) {
+        return from(user, false);
+    }
+
+    public static UserProfileResponse from(User user, boolean admin) {
         ThemeMode themeMode = user.getThemeMode() != null ? user.getThemeMode() : ThemeMode.LIGHT;
         PnlDisplayMode pnlDisplayMode = user.getPnlDisplayMode() != null ? user.getPnlDisplayMode() : PnlDisplayMode.PNL;
         TradeSortField defaultTradeSortBy = user.getDefaultTradeSortBy() != null
@@ -44,6 +49,7 @@ public static UserProfileResponse from(User user) {
                 user.getId(),
                 user.getAuthId(),
                 user.getEmail(),
+                admin,
                 user.isPremium(),
                 user.getCreatedAt(),
                 user.getUpdatedAt(),

src/main/java/com/transactionapi/security/RateLimiterService.java

@@ -12,16 +12,19 @@ public class RateLimiterService {
     private final int maxRequests;
     private final int maxPublicShareRequests;
     private final long windowMillis;
+    private final int maxBuckets;
     private final Map<String, ConcurrentLinkedDeque<Long>> buckets = new ConcurrentHashMap<>();
 
     public RateLimiterService(
             @Value("${app.rate-limit.per-minute:100}") int maxRequests,
             @Value("${app.rate-limit.public-share-per-minute:20}") int maxPublicShareRequests,
-            @Value("${app.rate-limit.window-ms:60000}") long windowMillis
+            @Value("${app.rate-limit.window-ms:60000}") long windowMillis,
+            @Value("${app.rate-limit.max-buckets:10000}") int maxBuckets
     ) {
         this.maxRequests = maxRequests;
         this.maxPublicShareRequests = maxPublicShareRequests;
         this.windowMillis = windowMillis;
+        this.maxBuckets = Math.max(1, maxBuckets);
     }
 
     public boolean allow(String userId) {
@@ -36,7 +39,14 @@ private boolean allowWithLimit(String key, int limit) {
         long now = System.currentTimeMillis();
         long cutoff = now - windowMillis;
 
-        ConcurrentLinkedDeque<Long> deque = buckets.computeIfAbsent(key, k -> new ConcurrentLinkedDeque<>());
+        ConcurrentLinkedDeque<Long> deque = buckets.get(key);
+        if (deque == null) {
+            cleanupExpiredBuckets(cutoff);
+            if (buckets.size() >= maxBuckets) {
+                return false;
+            }
+            deque = buckets.computeIfAbsent(key, k -> new ConcurrentLinkedDeque<>());
+        }
 
         while (!deque.isEmpty() && deque.peekFirst() < cutoff) {
             deque.pollFirst();
@@ -49,4 +59,15 @@ private boolean allowWithLimit(String key, int limit) {
         deque.addLast(now);
         return true;
     }
+
+    private void cleanupExpiredBuckets(long cutoff) {
+        buckets.forEach((bucketKey, deque) -> {
+            while (!deque.isEmpty() && deque.peekFirst() < cutoff) {
+                deque.pollFirst();
+            }
+            if (deque.isEmpty()) {
+                buckets.remove(bucketKey, deque);
+            }
+        });
+    }
 }

src/main/java/com/transactionapi/security/RateLimitingFilter.java

@@ -6,6 +6,7 @@
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
 import java.io.IOException;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.http.HttpStatus;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.context.SecurityContextHolder;
@@ -18,6 +19,9 @@ public class RateLimitingFilter extends OncePerRequestFilter {
     private final RateLimiterService rateLimiterService;
     private final UserIdResolver userIdResolver;
 
+    @Value("${app.rate-limit.trust-forwarded-headers:false}")
+    private boolean trustForwardedHeaders;
+
     public RateLimitingFilter(RateLimiterService rateLimiterService, UserIdResolver userIdResolver) {
         this.rateLimiterService = rateLimiterService;
         this.userIdResolver = userIdResolver;
@@ -70,6 +74,10 @@ protected void doFilterInternal(
     }
 
     private String getClientIp(HttpServletRequest request) {
+        if (!trustForwardedHeaders) {
+            return request.getRemoteAddr() != null ? request.getRemoteAddr() : "unknown";
+        }
+
         String ip = request.getHeader("X-Forwarded-For");
         if (ip == null || ip.isEmpty() || "unknown".equalsIgnoreCase(ip)) {
             ip = request.getHeader("X-Real-IP");

src/main/java/com/transactionapi/security/UserIdResolver.java

@@ -80,6 +80,14 @@ public void requireAdmin(Authentication authentication) {
         }
     }
 
+    public boolean isAdmin(Authentication authentication) {
+        if (adminEmailSet.isEmpty()) {
+            return false;
+        }
+        String email = resolveEmail(authentication);
+        return StringUtils.hasText(email) && adminEmailSet.contains(email.toLowerCase());
+    }
+
     private void enforceAllowedEmails(Authentication authentication) {
         if (allowedEmailSet.isEmpty()) {
             return;

src/main/java/com/transactionapi/service/ShareLinkService.java

@@ -68,7 +68,7 @@ public Optional<ShareLink> getShareLink(String code, String requestingUserId) {
                     if (!link.isRequiresAuth()) {
                         return true;
                     }
-                    return requestingUserId != null && !requestingUserId.isBlank();
+                    return link.getUserId().equals(requestingUserId);
                 })
                 .map(link -> {
                     link.setAccessCount(link.getAccessCount() + 1);

src/main/resources/application.properties

@@ -38,3 +38,5 @@ app.security.jwt.dynamo.max-stale=PT72H
 app.security.allow-header-auth=false
 app.security.header-name=X-User-Id
 app.security.dev-user-id=
+app.rate-limit.trust-forwarded-headers=${APP_RATE_LIMIT_TRUST_FORWARDED_HEADERS:false}
+app.rate-limit.max-buckets=${APP_RATE_LIMIT_MAX_BUCKETS:10000}

src/test/java/com/transactionapi/controller/AuthControllerTest.java

@@ -29,7 +29,7 @@
 import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.test.web.servlet.MvcResult;
 
-@SpringBootTest
+@SpringBootTest(properties = "app.security.admin-emails=admin@example.com")
 @AutoConfigureMockMvc
 @ActiveProfiles("test")
 class AuthControllerTest {
@@ -66,6 +66,7 @@ void loginCreatesSessionCookieAndAuthenticatesSubsequentRequests() throws Except
                 .andExpect(status().isOk())
                 .andExpect(jsonPath("$.authId").value("google-sub-1"))
                 .andExpect(jsonPath("$.email").value("user@example.com"))
+                .andExpect(jsonPath("$.admin").value(false))
                 .andExpect(jsonPath("$.termsAcceptedAt").doesNotExist())
                 .andExpect(jsonPath("$.privacyPolicyAcceptedAt").doesNotExist())
                 .andReturn();
@@ -75,7 +76,42 @@ void loginCreatesSessionCookieAndAuthenticatesSubsequentRequests() throws Except
         mockMvc.perform(get(ApiPaths.USER_ME).session(session))
                 .andExpect(status().isOk())
                 .andExpect(jsonPath("$.authId").value("google-sub-1"))
-                .andExpect(jsonPath("$.email").value("user@example.com"));
+                .andExpect(jsonPath("$.email").value("user@example.com"))
+                .andExpect(jsonPath("$.admin").value(false));
+    }
+
+    @Test
+    void configuredAdminGetsAdminProfileOnLoginProfileAndLegalAcceptance() throws Exception {
+        Jwt jwt = Jwt.withTokenValue("google-id-token")
+                .header("alg", "RS256")
+                .subject("google-sub-admin")
+                .claim("email", "admin@example.com")
+                .claim("name", "Admin User")
+                .build();
+        when(jwtDecoder.decode("google-id-token")).thenReturn(jwt);
+
+        MvcResult loginResult = loginWithCsrf("google-id-token")
+                .andExpect(status().isOk())
+                .andExpect(jsonPath("$.authId").value("google-sub-admin"))
+                .andExpect(jsonPath("$.email").value("admin@example.com"))
+                .andExpect(jsonPath("$.admin").value(true))
+                .andReturn();
+
+        MockHttpSession session = (MockHttpSession) loginResult.getRequest().getSession(false);
+
+        mockMvc.perform(get(ApiPaths.USER_ME).session(session))
+                .andExpect(status().isOk())
+                .andExpect(jsonPath("$.admin").value(true));
+
+        mockMvc.perform(
+                        post(ApiPaths.USER_LEGAL_AGREEMENT)
+                                .session(session)
+                                .with(csrf())
+                )
+                .andExpect(status().isOk())
+                .andExpect(jsonPath("$.admin").value(true))
+                .andExpect(jsonPath("$.termsAcceptedAt").isNotEmpty())
+                .andExpect(jsonPath("$.privacyPolicyAcceptedAt").isNotEmpty());
     }
 
     @Test

src/test/java/com/transactionapi/controller/ShareLinkControllerTest.java

@@ -167,6 +167,16 @@ void getShareLink_authRequired_authenticated_shouldSucceed() throws Exception {
                 .andExpect(jsonPath("$.requiresAuth").value(true));
     }
 
+    @Test
+    @WithMockUser
+    void getShareLink_authRequired_differentAuthenticatedUser_shouldReturn404() throws Exception {
+        when(shareLinkService.getShareLink(eq("xyz98765"), eq(USER_ID)))
+                .thenReturn(Optional.empty());
+
+        mockMvc.perform(get(ApiPaths.SHARES + "/xyz98765"))
+                .andExpect(status().isNotFound());
+    }
+
     @Test
     void getShareLink_notFound_shouldReturn404() throws Exception {
         when(shareLinkService.getShareLink(eq("notfound"), isNull()))

src/test/java/com/transactionapi/security/RateLimiterServiceTest.java

@@ -8,10 +8,27 @@ class RateLimiterServiceTest {
 
     @Test
     void blocksAfterLimit() {
-        RateLimiterService limiter = new RateLimiterService(2, 2, 60_000);
+        RateLimiterService limiter = new RateLimiterService(2, 2, 60_000, 100);
 
         assertThat(limiter.allow("user-1")).isTrue();
         assertThat(limiter.allow("user-1")).isTrue();
         assertThat(limiter.allow("user-1")).isFalse();
     }
+
+    @Test
+    void blocksNewKeysAfterBucketLimit() {
+        RateLimiterService limiter = new RateLimiterService(2, 2, 60_000, 1);
+
+        assertThat(limiter.allow("user-1")).isTrue();
+        assertThat(limiter.allow("user-2")).isFalse();
+    }
+
+    @Test
+    void evictsExpiredBucketsBeforeApplyingBucketLimit() throws Exception {
+        RateLimiterService limiter = new RateLimiterService(2, 2, 1, 1);
+
+        assertThat(limiter.allow("user-1")).isTrue();
+        Thread.sleep(5);
+        assertThat(limiter.allow("user-2")).isTrue();
+    }
 }

src/test/java/com/transactionapi/security/RateLimitingFilterTest.java

@@ -0,0 +1,75 @@
+package com.transactionapi.security;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import com.transactionapi.constants.ApiPaths;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.springframework.mock.web.MockFilterChain;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.http.HttpStatus;
+import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.test.util.ReflectionTestUtils;
+
+class RateLimitingFilterTest {
+
+    private final RateLimiterService rateLimiterService = org.mockito.Mockito.mock(RateLimiterService.class);
+    private final UserIdResolver userIdResolver = org.mockito.Mockito.mock(UserIdResolver.class);
+
+    @BeforeEach
+    void clearSecurityContext() {
+        SecurityContextHolder.clearContext();
+    }
+
+    @Test
+    void unauthenticatedPublicShareUsesRemoteAddressByDefault() throws Exception {
+        RateLimitingFilter filter = new RateLimitingFilter(rateLimiterService, userIdResolver);
+        MockHttpServletRequest request = publicShareRequest();
+        request.setRemoteAddr("203.0.113.10");
+        request.addHeader("X-Forwarded-For", "198.51.100.7");
+        when(rateLimiterService.allowPublicShare("203.0.113.10")).thenReturn(true);
+
+        filter.doFilter(request, new MockHttpServletResponse(), new MockFilterChain());
+
+        verify(rateLimiterService).allowPublicShare("203.0.113.10");
+    }
+
+    @Test
+    void unauthenticatedPublicShareRejectsSpoofedForwardedHeaderWhenRemoteBucketIsLimited() throws Exception {
+        RateLimitingFilter filter = new RateLimitingFilter(rateLimiterService, userIdResolver);
+        MockHttpServletRequest request = publicShareRequest();
+        request.setRemoteAddr("203.0.113.10");
+        request.addHeader("X-Forwarded-For", "198.51.100.7");
+        MockHttpServletResponse response = new MockHttpServletResponse();
+        when(rateLimiterService.allowPublicShare("203.0.113.10")).thenReturn(false);
+
+        filter.doFilter(request, response, new MockFilterChain());
+
+        assertThat(response.getStatus()).isEqualTo(HttpStatus.TOO_MANY_REQUESTS.value());
+        assertThat(response.getContentAsString()).contains("Too many requests");
+        verify(rateLimiterService).allowPublicShare("203.0.113.10");
+    }
+
+    @Test
+    void unauthenticatedPublicShareUsesForwardedHeaderOnlyWhenTrusted() throws Exception {
+        RateLimitingFilter filter = new RateLimitingFilter(rateLimiterService, userIdResolver);
+        ReflectionTestUtils.setField(filter, "trustForwardedHeaders", true);
+        MockHttpServletRequest request = publicShareRequest();
+        request.setRemoteAddr("203.0.113.10");
+        request.addHeader("X-Forwarded-For", "198.51.100.7, 10.0.0.1");
+        when(rateLimiterService.allowPublicShare("198.51.100.7")).thenReturn(true);
+
+        filter.doFilter(request, new MockHttpServletResponse(), new MockFilterChain());
+
+        verify(rateLimiterService).allowPublicShare("198.51.100.7");
+    }
+
+    private MockHttpServletRequest publicShareRequest() {
+        MockHttpServletRequest request = new MockHttpServletRequest("GET", ApiPaths.SHARES + "/abc12345");
+        request.setServletPath(ApiPaths.SHARES + "/abc12345");
+        return request;
+    }
+}