Skip to content

Commit

Permalink
url-decode cookie value
Browse files Browse the repository at this point in the history
fixes #30
  • Loading branch information
sephiroth-j committed Apr 15, 2024
1 parent bf9e91e commit 159285b
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

/**
Expand Down Expand Up @@ -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)

This comment has been minimized.

Copy link
@MichaelKunze

MichaelKunze Apr 16, 2024

Das hättest du auch mit Optional.ofNullable(cookies) lösen können...

.filter(c -> c.getName().equals(cookieName))
.findFirst()
.map(Cookie::getValue)
.map(value -> StringUtils.uriDecode(value, StandardCharsets.ISO_8859_1))
.orElse(EMPTY_STRING) : EMPTY_STRING;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -166,7 +168,9 @@ public Mono<Authentication> 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 ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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==";
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 159285b

Please sign in to comment.