From 20268ce33140bfed9e4ce1ff40e5d570a7b7cba9 Mon Sep 17 00:00:00 2001 From: Michael Vitz Date: Wed, 11 Jul 2018 12:08:40 +0200 Subject: [PATCH 1/2] refactor!: Move codec abstraction into own package --- .../com/innoq/spring/cookie/flash/CookieFlashMapManager.java | 1 + .../spring/cookie/flash/{ => codec}/FlashMapListCodec.java | 2 +- .../cookie/flash/codec/jackson/JacksonFlashMapListCodec.java | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) rename src/main/java/com/innoq/spring/cookie/flash/{ => codec}/FlashMapListCodec.java (94%) diff --git a/src/main/java/com/innoq/spring/cookie/flash/CookieFlashMapManager.java b/src/main/java/com/innoq/spring/cookie/flash/CookieFlashMapManager.java index 2c397d1..0c0280a 100644 --- a/src/main/java/com/innoq/spring/cookie/flash/CookieFlashMapManager.java +++ b/src/main/java/com/innoq/spring/cookie/flash/CookieFlashMapManager.java @@ -15,6 +15,7 @@ */ package com.innoq.spring.cookie.flash; +import com.innoq.spring.cookie.flash.codec.FlashMapListCodec; import com.innoq.spring.cookie.security.CookieValueSigner; import org.springframework.util.Assert; import org.springframework.web.servlet.FlashMap; diff --git a/src/main/java/com/innoq/spring/cookie/flash/FlashMapListCodec.java b/src/main/java/com/innoq/spring/cookie/flash/codec/FlashMapListCodec.java similarity index 94% rename from src/main/java/com/innoq/spring/cookie/flash/FlashMapListCodec.java rename to src/main/java/com/innoq/spring/cookie/flash/codec/FlashMapListCodec.java index 02a0524..88ecb4f 100644 --- a/src/main/java/com/innoq/spring/cookie/flash/FlashMapListCodec.java +++ b/src/main/java/com/innoq/spring/cookie/flash/codec/FlashMapListCodec.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package com.innoq.spring.cookie.flash; +package com.innoq.spring.cookie.flash.codec; import org.springframework.web.servlet.FlashMap; diff --git a/src/main/java/com/innoq/spring/cookie/flash/codec/jackson/JacksonFlashMapListCodec.java b/src/main/java/com/innoq/spring/cookie/flash/codec/jackson/JacksonFlashMapListCodec.java index b314749..69ae433 100644 --- a/src/main/java/com/innoq/spring/cookie/flash/codec/jackson/JacksonFlashMapListCodec.java +++ b/src/main/java/com/innoq/spring/cookie/flash/codec/jackson/JacksonFlashMapListCodec.java @@ -19,7 +19,7 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.module.SimpleModule; -import com.innoq.spring.cookie.flash.FlashMapListCodec; +import com.innoq.spring.cookie.flash.codec.FlashMapListCodec; import org.springframework.util.Assert; import org.springframework.web.servlet.FlashMap; From 6d9bd90758370522944a139920f98b54047f8efb Mon Sep 17 00:00:00 2001 From: Michael Vitz Date: Wed, 11 Jul 2018 12:10:17 +0200 Subject: [PATCH 2/2] feature: Add failure handler for cookie verification This feature allows to customise the behaviour of what should be done in case the cookie can not be verified. By default the cookie is silently ignored, as before. Closes #1 --- pom.xml | 10 +++++ .../cookie/flash/CookieFlashMapManager.java | 15 +++++-- .../CookieVerificationFailureHandler.java | 31 ++++++++++++++ ...gnoreCookieVerificationFailureHandler.java | 35 ++++++++++++++++ .../flash/CookieFlashMapManagerTest.java | 41 +++++++++++++++++++ 5 files changed, 128 insertions(+), 4 deletions(-) create mode 100644 src/main/java/com/innoq/spring/cookie/flash/verification/CookieVerificationFailureHandler.java create mode 100644 src/main/java/com/innoq/spring/cookie/flash/verification/IgnoreCookieVerificationFailureHandler.java diff --git a/pom.xml b/pom.xml index eb610f6..113d857 100644 --- a/pom.xml +++ b/pom.xml @@ -103,6 +103,11 @@ assertj-core 3.15.0 + + org.mockito + mockito-core + 3.3.3 + com.fasterxml.jackson jackson-bom @@ -152,6 +157,11 @@ junit-jupiter-engine test + + org.mockito + mockito-core + test + org.springframework spring-test diff --git a/src/main/java/com/innoq/spring/cookie/flash/CookieFlashMapManager.java b/src/main/java/com/innoq/spring/cookie/flash/CookieFlashMapManager.java index 0c0280a..ffb227a 100644 --- a/src/main/java/com/innoq/spring/cookie/flash/CookieFlashMapManager.java +++ b/src/main/java/com/innoq/spring/cookie/flash/CookieFlashMapManager.java @@ -16,6 +16,7 @@ package com.innoq.spring.cookie.flash; import com.innoq.spring.cookie.flash.codec.FlashMapListCodec; +import com.innoq.spring.cookie.flash.verification.CookieVerificationFailureHandler; import com.innoq.spring.cookie.security.CookieValueSigner; import org.springframework.util.Assert; import org.springframework.web.servlet.FlashMap; @@ -37,6 +38,8 @@ public final class CookieFlashMapManager extends AbstractFlashMapManager { private final FlashMapListCodec codec; private final CookieValueSigner signer; private final String cookieName; + private CookieVerificationFailureHandler verificationFailureHandler = + CookieVerificationFailureHandler.ignoreFailures(); public CookieFlashMapManager(FlashMapListCodec codec, CookieValueSigner signer) { @@ -53,6 +56,12 @@ public CookieFlashMapManager(FlashMapListCodec codec, this.cookieName = cookieName; } + public void setVerificationFailureHandler( + CookieVerificationFailureHandler verificationFailureHandler) { + Assert.notNull(verificationFailureHandler, "CookieVerificationFailureHandler must not be null"); + this.verificationFailureHandler = verificationFailureHandler; + } + @Override protected List retrieveFlashMaps(HttpServletRequest request) { final Cookie cookie = getCookie(request, cookieName); @@ -88,16 +97,14 @@ protected Object getFlashMapsMutex(HttpServletRequest request) { private List decode(String value) { final String[] signatureAndPayload = reverse(value).split("--", 2); if (signatureAndPayload.length != 2) { - // TODO logging - return null; + return verificationFailureHandler.onInvalidValue(value); } final String signature = reverse(signatureAndPayload[0]); final String payload = reverse(signatureAndPayload[1]); if (!isVerified(payload, signature)) { - // TODO logging - return null; + return verificationFailureHandler.onInvalidSignature(payload, signature); } return codec.decode(payload); diff --git a/src/main/java/com/innoq/spring/cookie/flash/verification/CookieVerificationFailureHandler.java b/src/main/java/com/innoq/spring/cookie/flash/verification/CookieVerificationFailureHandler.java new file mode 100644 index 0000000..18bb4fc --- /dev/null +++ b/src/main/java/com/innoq/spring/cookie/flash/verification/CookieVerificationFailureHandler.java @@ -0,0 +1,31 @@ +/** + * Copyright 2018 innoQ Deutschland GmbH + * + * Licensed 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 com.innoq.spring.cookie.flash.verification; + +import org.springframework.web.servlet.FlashMap; + +import java.util.List; + +public interface CookieVerificationFailureHandler { + + List onInvalidValue(String value); + + List onInvalidSignature(String payload, String signature); + + static CookieVerificationFailureHandler ignoreFailures() { + return IgnoreCookieVerificationFailureHandler.INSTANCE; + } +} diff --git a/src/main/java/com/innoq/spring/cookie/flash/verification/IgnoreCookieVerificationFailureHandler.java b/src/main/java/com/innoq/spring/cookie/flash/verification/IgnoreCookieVerificationFailureHandler.java new file mode 100644 index 0000000..0452c95 --- /dev/null +++ b/src/main/java/com/innoq/spring/cookie/flash/verification/IgnoreCookieVerificationFailureHandler.java @@ -0,0 +1,35 @@ +/** + * Copyright 2018 innoQ Deutschland GmbH + * + * Licensed 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 com.innoq.spring.cookie.flash.verification; + +import org.springframework.web.servlet.FlashMap; + +import java.util.List; + +enum IgnoreCookieVerificationFailureHandler + implements CookieVerificationFailureHandler { + INSTANCE; + + @Override + public List onInvalidValue(String value) { + return null; + } + + @Override + public List onInvalidSignature(String payload, String signature) { + return null; + } +} diff --git a/src/test/java/com/innoq/spring/cookie/flash/CookieFlashMapManagerTest.java b/src/test/java/com/innoq/spring/cookie/flash/CookieFlashMapManagerTest.java index be88e42..9dcf455 100644 --- a/src/test/java/com/innoq/spring/cookie/flash/CookieFlashMapManagerTest.java +++ b/src/test/java/com/innoq/spring/cookie/flash/CookieFlashMapManagerTest.java @@ -16,6 +16,7 @@ package com.innoq.spring.cookie.flash; import com.innoq.spring.cookie.flash.codec.jackson.JacksonFlashMapListCodec; +import com.innoq.spring.cookie.flash.verification.CookieVerificationFailureHandler; import com.innoq.spring.cookie.security.CookieValueSigner; import org.junit.jupiter.api.Test; import org.springframework.mock.web.MockHttpServletRequest; @@ -30,6 +31,9 @@ import static java.util.Collections.emptyList; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.entry; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; class CookieFlashMapManagerTest { @@ -65,6 +69,43 @@ void retrieveFlashMaps_withValidCookie_returnsFlashMaps() { assertThat(flashMap.getTargetRequestPath()).isEqualTo("/foo"); } + @Test + void retrieveFlashMaps_withInvalidCookieValue_callsGivenVerificationFailureHandlerAndUsesItsReturnValue() { + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); + request.setCookies(new Cookie("flash", "ABCDEF")); + + CookieVerificationFailureHandler verificationFailureHandler = + mock(CookieVerificationFailureHandler.class); + when(verificationFailureHandler.onInvalidValue("ABCDEF")).thenReturn(null); + + sut.setVerificationFailureHandler(verificationFailureHandler); + + List flashMaps = sut.retrieveFlashMaps(request); + + assertThat(flashMaps).isNull(); + verify(verificationFailureHandler).onInvalidValue("ABCDEF"); + } + + @Test + void retrieveFlashMaps_withInvalidCookieValueSignatur_callsGivenVerificationFailureHandlerAndUsesItsReturnValue() { + String cookieValue = "abcd--efgh"; + + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); + request.setCookies(new Cookie("flash", cookieValue)); + + CookieVerificationFailureHandler verificationFailureHandler = + mock(CookieVerificationFailureHandler.class); + when(verificationFailureHandler.onInvalidSignature("abcd", "efgh")) + .thenReturn(null); + + sut.setVerificationFailureHandler(verificationFailureHandler); + + List flashMaps = sut.retrieveFlashMaps(request); + + assertThat(flashMaps).isNull(); + verify(verificationFailureHandler).onInvalidSignature("abcd", "efgh"); + } + @Test void updateFlashMaps_withSingleFlashMap_writesCookie() { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/");