FINERACT-2591: Add warning when Idempotency-Key header is missing

fineract-core/src/main/java/org/apache/fineract/infrastructure/core/filters/IdempotencyStoreFilter.java

@@ -27,6 +27,7 @@
 import java.util.Optional;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.mutable.Mutable;
 import org.apache.commons.lang3.mutable.MutableObject;
 import org.apache.fineract.commands.service.SynchronousCommandProcessingService;
@@ -47,6 +48,7 @@ public class IdempotencyStoreFilter extends OncePerRequestFilter {
     @Override
     protected void doFilterInternal(@NonNull HttpServletRequest request, @NonNull HttpServletResponse response,
             @NonNull FilterChain filterChain) throws ServletException, IOException {
+        warnIfIdempotencyKeyHeaderMissing(request);
         Mutable<ContentCachingResponseWrapper> wrapper = new MutableObject<>();
         if (helper.isAllowedContentTypeRequest(request)) {
             wrapper.setValue(new ContentCachingResponseWrapper(response));
@@ -68,6 +70,20 @@ protected void doFilterInternal(@NonNull HttpServletRequest request, @NonNull Ht
         }
     }
 
+    private void warnIfIdempotencyKeyHeaderMissing(HttpServletRequest request) {
+        if (!helper.isAllowedContentTypeRequest(request)) {
+            return;
+        }
+        String headerName = fineractProperties.getIdempotencyKeyHeaderName();
+        if (StringUtils.isNotBlank(request.getHeader(headerName))) {
+            return;
+        }
+        if (log.isWarnEnabled()) {
+            log.warn("Idempotency key header [{}] is missing. Clients should provide it to avoid unintended duplicate command processing.",
+                    headerName);
+        }
+    }
+
     private Optional<String> extractIdempotentKeyFromHttpServletRequest(HttpServletRequest request) {
         return Optional.ofNullable(request.getHeader(fineractProperties.getIdempotencyKeyHeaderName()));
     }

fineract-provider/src/test/java/org/apache/fineract/infrastructure/core/filters/IdempotencyStoreFilterTest.java

@@ -0,0 +1,136 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.core.filters;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.verify;
+
+import ch.qos.logback.classic.Level;
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.read.ListAppender;
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import org.apache.fineract.infrastructure.core.config.FineractProperties;
+import org.apache.fineract.infrastructure.core.domain.FineractRequestContextHolder;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.mockito.junit.jupiter.MockitoSettings;
+import org.mockito.quality.Strictness;
+import org.slf4j.LoggerFactory;
+
+@ExtendWith(MockitoExtension.class)
+@MockitoSettings(strictness = Strictness.LENIENT)
+public class IdempotencyStoreFilterTest {
+
+    private static final String IDEMPOTENCY_KEY_HEADER_NAME = "Idempotency-Key";
+
+    @Mock
+    private HttpServletRequest request;
+
+    @Mock
+    private HttpServletResponse response;
+
+    @Mock
+    private FilterChain filterChain;
+
+    @Mock
+    private FineractRequestContextHolder fineractRequestContextHolder;
+
+    @Mock
+    private IdempotencyStoreHelper helper;
+
+    private Logger logger;
+    private ListAppender<ILoggingEvent> listAppender;
+    private FineractProperties fineractProperties;
+    private IdempotencyStoreFilter underTest;
+
+    @BeforeEach
+    public void setUp() {
+        fineractProperties = new FineractProperties();
+        fineractProperties.setIdempotencyKeyHeaderName(IDEMPOTENCY_KEY_HEADER_NAME);
+        underTest = new IdempotencyStoreFilter(fineractRequestContextHolder, helper, fineractProperties);
+        logger = (Logger) LoggerFactory.getLogger(IdempotencyStoreFilter.class.getName());
+        listAppender = new ListAppender<>();
+        listAppender.start();
+        logger.addAppender(listAppender);
+    }
+
+    @AfterEach
+    public void tearDown() {
+        logger.detachAppender(listAppender);
+    }
+
+    @Test
+    public void testDoFilterInternalShouldLogWarningWhenJsonRequestHasNoIdempotencyKeyHeader() throws Exception {
+        given(helper.isAllowedContentTypeRequest(request)).willReturn(true);
+        given(request.getHeader(IDEMPOTENCY_KEY_HEADER_NAME)).willReturn(null);
+
+        underTest.doFilterInternal(request, response, filterChain);
+
+        verify(filterChain).doFilter(eq(request), any(HttpServletResponse.class));
+        Assertions.assertTrue(hasMissingIdempotencyKeyHeaderWarning());
+    }
+
+    @Test
+    public void testDoFilterInternalShouldLogWarningWhenJsonRequestHasBlankIdempotencyKeyHeader() throws Exception {
+        given(helper.isAllowedContentTypeRequest(request)).willReturn(true);
+        given(request.getHeader(IDEMPOTENCY_KEY_HEADER_NAME)).willReturn("   ");
+
+        underTest.doFilterInternal(request, response, filterChain);
+
+        verify(filterChain).doFilter(eq(request), any(HttpServletResponse.class));
+        Assertions.assertTrue(hasMissingIdempotencyKeyHeaderWarning());
+    }
+
+    @Test
+    public void testDoFilterInternalShouldNotLogWarningWhenJsonRequestHasIdempotencyKeyHeader() throws Exception {
+        given(helper.isAllowedContentTypeRequest(request)).willReturn(true);
+        given(request.getHeader(IDEMPOTENCY_KEY_HEADER_NAME)).willReturn("client-key");
+
+        underTest.doFilterInternal(request, response, filterChain);
+
+        verify(filterChain).doFilter(eq(request), any(HttpServletResponse.class));
+        Assertions.assertFalse(hasMissingIdempotencyKeyHeaderWarning());
+    }
+
+    @Test
+    public void testDoFilterInternalShouldNotLogWarningWhenRequestIsNotJson() throws Exception {
+        given(helper.isAllowedContentTypeRequest(request)).willReturn(false);
+        given(request.getHeader(IDEMPOTENCY_KEY_HEADER_NAME)).willReturn(null);
+
+        underTest.doFilterInternal(request, response, filterChain);
+
+        verify(filterChain).doFilter(eq(request), eq(response));
+        Assertions.assertFalse(hasMissingIdempotencyKeyHeaderWarning());
+    }
+
+    private boolean hasMissingIdempotencyKeyHeaderWarning() {
+        return listAppender.list.stream()
+                .anyMatch(event -> event.getLevel().equals(Level.WARN) && event.getFormattedMessage().contains("Idempotency key header"));
+    }
+}