From 174c7aad99b66f5373fc5c46b696b96ec41ad1d2 Mon Sep 17 00:00:00 2001
From: Julian Raufelder <Julian@Raufelder.com>
Date: Mon, 27 Nov 2023 19:43:50 +0100
Subject: [PATCH] Apply code review suggestions

---
 .../hub/license/LicenseHolder.java            |  4 +-
 .../hub/license/LicenseHolderTest.java        | 52 +++++++++++++++++--
 2 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/backend/src/main/java/org/cryptomator/hub/license/LicenseHolder.java b/backend/src/main/java/org/cryptomator/hub/license/LicenseHolder.java
index 8098fa93..7d51b4f7 100644
--- a/backend/src/main/java/org/cryptomator/hub/license/LicenseHolder.java
+++ b/backend/src/main/java/org/cryptomator/hub/license/LicenseHolder.java
@@ -21,6 +21,7 @@
 import java.nio.charset.StandardCharsets;
 import java.time.Instant;
 import java.util.HashMap;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.stream.Collectors;
@@ -101,8 +102,7 @@ void refreshLicenseScheduler() throws InterruptedException {
 
 	//visible for testing
 	void refreshLicense(String refreshUrl, String license, HttpClient client) throws InterruptedException {
-		var parameters = new HashMap<String, String>();
-		parameters.put("token", license);
+		var parameters = Map.of("token", license);
 		var body = parameters.entrySet() //
 				.stream() //
 				.map(e -> e.getKey() + "=" + URLEncoder.encode(e.getValue(), StandardCharsets.UTF_8)) //
diff --git a/backend/src/test/java/org/cryptomator/hub/license/LicenseHolderTest.java b/backend/src/test/java/org/cryptomator/hub/license/LicenseHolderTest.java
index 2a08d2b7..03a2df7e 100644
--- a/backend/src/test/java/org/cryptomator/hub/license/LicenseHolderTest.java
+++ b/backend/src/test/java/org/cryptomator/hub/license/LicenseHolderTest.java
@@ -27,8 +27,12 @@
 import java.net.http.HttpClient;
 import java.net.http.HttpRequest;
 import java.net.http.HttpResponse;
+import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.concurrent.Flow;
 
 @QuarkusTest
 public class LicenseHolderTest {
@@ -219,19 +223,26 @@ public void teardown() {
 		public void testRefreshingExistingValidTokenInculdingRefreshURL() throws IOException, InterruptedException {
 			var existingJWT = Mockito.mock(DecodedJWT.class);
 			var receivedJWT = Mockito.mock(DecodedJWT.class);
-			Mockito.when(existingJWT.getToken()).thenReturn("token");
+			Mockito.when(existingJWT.getToken()).thenReturn("token&foo=bar");
 			Mockito.when(validator.validate("token", "42")).thenReturn(receivedJWT);
-			Mockito.when(validator.validate("oldToken", "42")).thenReturn(existingJWT);
+			Mockito.when(validator.validate("token&foo=bar", "42")).thenReturn(existingJWT);
 			Settings settingsMock = new Settings();
 			settingsMock.hubId = "42";
-			settingsMock.licenseKey = "oldToken";
+			settingsMock.licenseKey = "token&foo=bar";
 			settingsClass.when(Settings::get).thenReturn(settingsMock);
 
+			var refreshTokenContainingSpecialChars = HttpRequest.newBuilder() //
+					.uri(URI.create(refreshURL)) //
+					.headers("Content-Type", "application/x-www-form-urlencoded") //
+					.POST(HttpRequest.BodyPublishers.ofString("token&foo=bar"))  //
+					.build();
+
 			var httpClient = Mockito.mock(HttpClient.class);
 			var response = Mockito.mock(HttpResponse.class);
 			Mockito.doAnswer(invocation -> {
 				HttpRequest httpRequest = invocation.getArgument(0);
-				Assertions.assertEquals(refreshRequst, httpRequest);
+				Assertions.assertEquals(refreshTokenContainingSpecialChars, httpRequest);
+				Assertions.assertEquals("token=token%26foo%3Dbar", getResponseFromRequest(httpRequest));
 				return response;
 			}).when(httpClient).send(Mockito.any(), Mockito.eq(HttpResponse.BodyHandlers.ofString()));
 			Mockito.when(response.body()).thenReturn("token");
@@ -268,6 +279,7 @@ public void testInvalidTokenReceivedLeadsToNoOp(String receivedToken, int receiv
 			Mockito.doAnswer(invocation -> {
 				HttpRequest httpRequest = invocation.getArgument(0);
 				Assertions.assertEquals(refreshRequst, httpRequest);
+				Assertions.assertEquals("token=token", getResponseFromRequest(httpRequest));
 				return response;
 			}).when(httpClient).send(Mockito.any(), Mockito.eq(HttpResponse.BodyHandlers.ofString()));
 			Mockito.when(response.body()).thenReturn(receivedToken);
@@ -328,6 +340,38 @@ public void testNoOpExistingValidTokenExculdingRefreshURL() throws InterruptedEx
 			Mockito.verify(session, Mockito.never()).persist(Mockito.any());
 			Assertions.assertEquals(existingJWT, holder.get());
 		}
+
+		private String getResponseFromRequest(HttpRequest httpRequest) {
+			return httpRequest.bodyPublisher().map(p -> {
+				var bodySubscriber = HttpResponse.BodySubscribers.ofString(StandardCharsets.UTF_8);
+				var flowSubscriber = new StringSubscriber(bodySubscriber);
+				p.subscribe(flowSubscriber);
+				return bodySubscriber.getBody().toCompletableFuture().join();
+			}).get();
+		}
+
+		private record StringSubscriber(HttpResponse.BodySubscriber<String> wrapped) implements Flow.Subscriber<ByteBuffer> {
+
+			@Override
+			public void onSubscribe(Flow.Subscription subscription) {
+				wrapped.onSubscribe(subscription);
+			}
+
+			@Override
+			public void onNext(ByteBuffer item) {
+				wrapped.onNext(List.of(item));
+			}
+
+			@Override
+			public void onError(Throwable throwable) {
+				wrapped.onError(throwable);
+			}
+
+			@Override
+			public void onComplete() {
+				wrapped.onComplete();
+			}
+		}
 	}
 
 	@Nested