diff --git a/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/filters/IdempotencyStoreFilter.java b/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/filters/IdempotencyStoreFilter.java index 675a27bb49a..d770a347972 100644 --- a/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/filters/IdempotencyStoreFilter.java +++ b/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 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 extractIdempotentKeyFromHttpServletRequest(HttpServletRequest request) { return Optional.ofNullable(request.getHeader(fineractProperties.getIdempotencyKeyHeaderName())); } diff --git a/fineract-provider/src/test/java/org/apache/fineract/infrastructure/core/filters/IdempotencyStoreFilterTest.java b/fineract-provider/src/test/java/org/apache/fineract/infrastructure/core/filters/IdempotencyStoreFilterTest.java new file mode 100644 index 00000000000..b1235e8412d --- /dev/null +++ b/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 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")); + } +}