From 159285b5298ba3b05ab7c1e7e65f48adcc735740 Mon Sep 17 00:00:00 2001 From: sephiroth-j <23166289+sephiroth-j@users.noreply.github.com> Date: Mon, 15 Apr 2024 22:56:00 +0200 Subject: [PATCH] url-decode cookie value fixes #30 --- CHANGELOG.md | 1 + .../spring/security/ltpa2/Ltpa2Filter.java | 9 +++++- .../ltpa2/reactive/Ltpa2AuthConverter.java | 6 +++- .../spring/security/ltpa2/Constants.java | 5 ++++ .../security/ltpa2/Ltpa2FilterTest.java | 30 ++++++++++++++++--- .../reactive/Ltpa2AuthConverterTest.java | 2 +- 6 files changed, 46 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a21c66..94e04e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### ⭐ New Features ### 🐞 Bugs Fixed - removed usage of `org.springframework.util.Base64Utils` since it is deprecated now +- cookie value was not url-decoded (fixes [#30](https://github.com/sephiroth-j/spring-security-ltpa2-core/issues/30)) ## v2.0.0 - 2022-11-27 ### ⚠ Breaking diff --git a/src/main/java/de/sephirothj/spring/security/ltpa2/Ltpa2Filter.java b/src/main/java/de/sephirothj/spring/security/ltpa2/Ltpa2Filter.java index db2c38e..f9ecfb1 100644 --- a/src/main/java/de/sephirothj/spring/security/ltpa2/Ltpa2Filter.java +++ b/src/main/java/de/sephirothj/spring/security/ltpa2/Ltpa2Filter.java @@ -21,6 +21,7 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.security.PublicKey; import java.util.stream.Stream; import javax.crypto.SecretKey; @@ -40,6 +41,7 @@ import org.springframework.security.web.authentication.AuthenticationFailureHandler; import org.springframework.security.web.authentication.preauth.PreAuthenticatedAuthenticationToken; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; import org.springframework.web.filter.OncePerRequestFilter; /** @@ -144,7 +146,12 @@ private String getTokenFromHeader(@Nullable final String header) @NonNull private String getTokenFromCookies(@Nullable final Cookie... cookies) { - return cookies != null ? Stream.of(cookies).filter(c -> c.getName().equals(cookieName)).findFirst().map(Cookie::getValue).orElse(EMPTY_STRING) : EMPTY_STRING; + return cookies != null ? Stream.of(cookies) + .filter(c -> c.getName().equals(cookieName)) + .findFirst() + .map(Cookie::getValue) + .map(value -> StringUtils.uriDecode(value, StandardCharsets.ISO_8859_1)) + .orElse(EMPTY_STRING) : EMPTY_STRING; } /** diff --git a/src/main/java/de/sephirothj/spring/security/ltpa2/reactive/Ltpa2AuthConverter.java b/src/main/java/de/sephirothj/spring/security/ltpa2/reactive/Ltpa2AuthConverter.java index c2f7b16..002a557 100644 --- a/src/main/java/de/sephirothj/spring/security/ltpa2/reactive/Ltpa2AuthConverter.java +++ b/src/main/java/de/sephirothj/spring/security/ltpa2/reactive/Ltpa2AuthConverter.java @@ -17,6 +17,7 @@ import de.sephirothj.spring.security.ltpa2.Ltpa2Token; import de.sephirothj.spring.security.ltpa2.Ltpa2Utils; +import java.nio.charset.StandardCharsets; import java.security.PublicKey; import javax.crypto.SecretKey; import lombok.Setter; @@ -32,6 +33,7 @@ import org.springframework.security.web.authentication.preauth.PreAuthenticatedAuthenticationToken; import org.springframework.security.web.server.authentication.ServerAuthenticationConverter; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; import org.springframework.web.server.ServerWebExchange; import reactor.core.publisher.Mono; import reactor.core.publisher.SynchronousSink; @@ -166,7 +168,9 @@ public Mono convert(ServerWebExchange exchange) .filter(header -> !header.isEmpty() && header.startsWith(headerValueIdentifier)) .map(header -> header.substring(header.indexOf(headerValueIdentifier) + headerValueIdentifier.length())) // try cookie as fallback - .switchIfEmpty(Mono.defer(() -> Mono.justOrEmpty(request.getCookies().getFirst(cookieName)).map(HttpCookie::getValue))) + .switchIfEmpty(Mono.defer(() -> Mono.justOrEmpty(request.getCookies().getFirst(cookieName)) + .map(HttpCookie::getValue) + .map(value -> StringUtils.uriDecode(value, StandardCharsets.ISO_8859_1)))) .doOnNext(encryptedToken -> log.debug("raw LTPA2 token: {}", encryptedToken)) .map(encryptedToken -> Ltpa2Utils.decryptLtpa2Token(encryptedToken, sharedKey)) .onErrorResume(e -> diff --git a/src/test/java/de/sephirothj/spring/security/ltpa2/Constants.java b/src/test/java/de/sephirothj/spring/security/ltpa2/Constants.java index c24a20a..60b4388 100644 --- a/src/test/java/de/sephirothj/spring/security/ltpa2/Constants.java +++ b/src/test/java/de/sephirothj/spring/security/ltpa2/Constants.java @@ -48,4 +48,9 @@ public class Constants * base64-encoded ltpa2 token */ public final String TEST_TOKEN = "Wl3qcMXdvCZjScDwB18/5VYujKDYsptVWXwNVW2yKuZw6h5Kg4amiGDeQCh2xmtNVPgCkzyk66ZWrdY70+nQEe+gotHjJtrcoW/VnbbQAwrQE5GojqK+1RdjvnwmQ9QULqcYAItw4ggZ2JF3CRR5uZ3NSFgkZpzkcMbfuYSWipNXsqEUHKONUlrg0Oc6lNKqWknx87HoPKmTnkGD5gdecu1FJCKUXSk1tanAjN3RaEWY8woxMIJQEMw/yeOrA9Fe+1nWjGAR5ITgkm+whpXfzl3n3g7kWHaBJf8DUUlKRsww4oCe3+t85b1WqoTC6FZw2qovLwn3ioRm1eIBDPO+KQZD60Ps4f+QEOjFzkLQC2f6BlZKc8KMHhffRQRpBgOD6kYV/wGDRHuvkK5vMAeJtQ=="; + + /** + * base64-encoded and uri-encoded ltpa2 token + */ + public final String TEST_TOKEN_URI_ENCODED = "Wl3qcMXdvCZjScDwB18%2F5VYujKDYsptVWXwNVW2yKuZw6h5Kg4amiGDeQCh2xmtNVPgCkzyk66ZWrdY70+nQEe+gotHjJtrcoW%2FVnbbQAwrQE5GojqK+1RdjvnwmQ9QULqcYAItw4ggZ2JF3CRR5uZ3NSFgkZpzkcMbfuYSWipNXsqEUHKONUlrg0Oc6lNKqWknx87HoPKmTnkGD5gdecu1FJCKUXSk1tanAjN3RaEWY8woxMIJQEMw%2FyeOrA9Fe+1nWjGAR5ITgkm+whpXfzl3n3g7kWHaBJf8DUUlKRsww4oCe3+t85b1WqoTC6FZw2qovLwn3ioRm1eIBDPO+KQZD60Ps4f+QEOjFzkLQC2f6BlZKc8KMHhffRQRpBgOD6kYV%2FwGDRHuvkK5vMAeJtQ=="; } diff --git a/src/test/java/de/sephirothj/spring/security/ltpa2/Ltpa2FilterTest.java b/src/test/java/de/sephirothj/spring/security/ltpa2/Ltpa2FilterTest.java index 8c5bd51..c3f2d4d 100644 --- a/src/test/java/de/sephirothj/spring/security/ltpa2/Ltpa2FilterTest.java +++ b/src/test/java/de/sephirothj/spring/security/ltpa2/Ltpa2FilterTest.java @@ -160,16 +160,15 @@ void getTokenFromRequestTestWithCookieOnly() { Ltpa2Filter uut = new Ltpa2Filter(); HttpServletRequest request = Mockito.mock(HttpServletRequest.class); - String expected = "the-token"; Cookie[] cookies = { - new Cookie("LtpaToken2", expected) + new Cookie("LtpaToken2", "the-token%2F") }; given(request.getCookies()).willReturn(cookies); String actual = ReflectionTestUtils.invokeMethod(uut, "getTokenFromRequest", request); - assertThat(actual).isEqualTo(expected); + assertThat(actual).isEqualTo("the-token/"); } @Test @@ -242,7 +241,7 @@ void shouldNotFilterTestWithHeaderOnly() throws ServletException } @Test - void shouldNotFilterTestWithCookieIOnly() throws ServletException + void shouldNotFilterTestWithCookieOnly() throws ServletException { Ltpa2Filter uut = new Ltpa2Filter(); HttpServletRequest request = Mockito.mock(HttpServletRequest.class); @@ -276,6 +275,29 @@ void doFilterInternalShouldSetAuthentication() throws ServletException, IOExcept }); } + @Test + void doFilterInternalShouldSetAuthenticationWithCookieOnly() throws ServletException, IOException, GeneralSecurityException + { + HttpServletRequest request = MockMvcRequestBuilders.get("/").cookie(new Cookie("LtpaToken2", Constants.TEST_TOKEN_URI_ENCODED)).buildRequest(new MockServletContext()); + UserDetailsService userDetailsService = Mockito.mock(UserDetailsService.class); + UserDetails mockUser = User.withUsername("test-user").roles("DEVELOPERS").password("dummy password").build(); + given(userDetailsService.loadUserByUsername(anyString())).willReturn(mockUser); + Ltpa2Filter uut = new Ltpa2Filter(); + uut.setAllowExpiredToken(true); + uut.setUserDetailsService(userDetailsService); + uut.setSharedKey(LtpaKeyUtils.decryptSharedKey(Constants.ENCRYPTED_SHARED_KEY, Constants.ENCRYPTION_PASSWORD)); + uut.setSignerKey(LtpaKeyUtils.decodePublicKey(Constants.ENCODED_PUBLIC_KEY)); + + uut.doFilter(request, new MockHttpServletResponse(), new MockFilterChain()); + + verify(userDetailsService).loadUserByUsername(anyString()); + assertThat(SecurityContextHolder.getContext().getAuthentication()).isInstanceOf(PreAuthenticatedAuthenticationToken.class).satisfies(auth -> + { + assertThat(auth.getPrincipal()).isEqualTo(mockUser); + assertThat(auth.getAuthorities()).containsExactlyInAnyOrderElementsOf((Iterable) mockUser.getAuthorities()); + }); + } + @Test void doFilterInternalShouldCause403ForUnknownUsers() throws ServletException, IOException, GeneralSecurityException { diff --git a/src/test/java/de/sephirothj/spring/security/ltpa2/reactive/Ltpa2AuthConverterTest.java b/src/test/java/de/sephirothj/spring/security/ltpa2/reactive/Ltpa2AuthConverterTest.java index a05acea..b8e89d1 100644 --- a/src/test/java/de/sephirothj/spring/security/ltpa2/reactive/Ltpa2AuthConverterTest.java +++ b/src/test/java/de/sephirothj/spring/security/ltpa2/reactive/Ltpa2AuthConverterTest.java @@ -111,7 +111,7 @@ void getTokenFromCookieTestWithDefaultName() throws GeneralSecurityException uut.setSignerKey(LtpaKeyUtils.decodePublicKey(Constants.ENCODED_PUBLIC_KEY)); uut.setAllowExpiredToken(true); - MockServerHttpRequest.BaseBuilder requestBuilder = MockServerHttpRequest.get("/").cookie(new HttpCookie("LtpaToken2", Constants.TEST_TOKEN)); + MockServerHttpRequest.BaseBuilder requestBuilder = MockServerHttpRequest.get("/").cookie(new HttpCookie("LtpaToken2", Constants.TEST_TOKEN_URI_ENCODED)); StepVerifier.create(uut.convert(MockServerWebExchange.from(requestBuilder.build()))) .assertNext(this::verifyAuth) .verifyComplete();