From 4de96dfd8614bfc5c65100c893ab189f18cb6a63 Mon Sep 17 00:00:00 2001 From: Santiago Pericas-Geertsen Date: Thu, 17 Aug 2023 09:28:58 -0400 Subject: [PATCH] Enables OIDC integration tests. Fixes two additional issues in WebClient: (#7390) 1. In ClientUri, UriQueryWriteable is initialized from a decoded string. If initialized using an encoded string as before, a second encoding will take place internally. 2. The WebClientCookieManager now correctly handles cookie values, including those that may contain commas (such as expiration values). There are new or updated tests to cover the changes in (1) and (2). --- .../tests/integration/oidc/CookieBasedLoginIT.java | 2 -- .../tests/integration/oidc/QueryBasedLoginIT.java | 2 -- .../integration/oidc/TenantIdentificationIT.java | 2 -- .../oidc/src/test/resources/application.yaml | 3 ++- .../java/io/helidon/webclient/api/ClientUri.java | 4 ++-- .../webclient/api/WebClientCookieManager.java | 4 ++-- .../java/io/helidon/webclient/api/ClientUriTest.java | 5 ++++- .../java/io/helidon/webclient/tests/CookieTest.java | 12 ++++++++++++ 8 files changed, 22 insertions(+), 12 deletions(-) diff --git a/tests/integration/oidc/src/test/java/io/helidon/tests/integration/oidc/CookieBasedLoginIT.java b/tests/integration/oidc/src/test/java/io/helidon/tests/integration/oidc/CookieBasedLoginIT.java index cd22b37fcbe..181ee4a2eda 100644 --- a/tests/integration/oidc/src/test/java/io/helidon/tests/integration/oidc/CookieBasedLoginIT.java +++ b/tests/integration/oidc/src/test/java/io/helidon/tests/integration/oidc/CookieBasedLoginIT.java @@ -22,7 +22,6 @@ import jakarta.ws.rs.core.Response; import org.jsoup.Jsoup; import org.jsoup.nodes.Document; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import static io.helidon.tests.integration.oidc.TestResource.EXPECTED_POST_LOGOUT_TEST_MESSAGE; @@ -31,7 +30,6 @@ import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.MatcherAssert.assertThat; -@Disabled("https://github.com/helidon-io/helidon/issues/7094") class CookieBasedLoginIT extends CommonLoginBase { @Test diff --git a/tests/integration/oidc/src/test/java/io/helidon/tests/integration/oidc/QueryBasedLoginIT.java b/tests/integration/oidc/src/test/java/io/helidon/tests/integration/oidc/QueryBasedLoginIT.java index 6daee1fd024..d0fe52bd51d 100644 --- a/tests/integration/oidc/src/test/java/io/helidon/tests/integration/oidc/QueryBasedLoginIT.java +++ b/tests/integration/oidc/src/test/java/io/helidon/tests/integration/oidc/QueryBasedLoginIT.java @@ -26,7 +26,6 @@ import org.glassfish.jersey.client.ClientProperties; import org.jsoup.Jsoup; import org.jsoup.nodes.Document; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import static io.helidon.tests.integration.oidc.TestResource.EXPECTED_TEST_MESSAGE; @@ -34,7 +33,6 @@ import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.MatcherAssert.assertThat; -@Disabled("https://github.com/helidon-io/helidon/issues/7094") @AddConfig(key = "security.providers.1.oidc.cookie-use", value = "false") @AddConfig(key = "security.providers.1.oidc.query-param-use", value = "true") class QueryBasedLoginIT extends CommonLoginBase { diff --git a/tests/integration/oidc/src/test/java/io/helidon/tests/integration/oidc/TenantIdentificationIT.java b/tests/integration/oidc/src/test/java/io/helidon/tests/integration/oidc/TenantIdentificationIT.java index f5123cbac36..3e010a8bcfa 100644 --- a/tests/integration/oidc/src/test/java/io/helidon/tests/integration/oidc/TenantIdentificationIT.java +++ b/tests/integration/oidc/src/test/java/io/helidon/tests/integration/oidc/TenantIdentificationIT.java @@ -33,7 +33,6 @@ import jakarta.ws.rs.core.HttpHeaders; import jakarta.ws.rs.core.Response; import org.glassfish.jersey.client.ClientProperties; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import static org.hamcrest.CoreMatchers.is; @@ -42,7 +41,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.greaterThanOrEqualTo; -@Disabled("https://github.com/helidon-io/helidon/issues/7094") @HelidonTest(resetPerTest = true) @AddBean(TestResource.class) @AddConfig(key = "security.providers.1.oidc.oidc-metadata-well-known", value = "false") diff --git a/tests/integration/oidc/src/test/resources/application.yaml b/tests/integration/oidc/src/test/resources/application.yaml index d5286506cb1..63fd7abe138 100644 --- a/tests/integration/oidc/src/test/resources/application.yaml +++ b/tests/integration/oidc/src/test/resources/application.yaml @@ -15,8 +15,9 @@ # client: - cookies: + cookie-manager: automatic-store-enabled: true + send-expect-continue: false follow-redirects: true security: diff --git a/webclient/api/src/main/java/io/helidon/webclient/api/ClientUri.java b/webclient/api/src/main/java/io/helidon/webclient/api/ClientUri.java index 583918345c4..45149e92469 100644 --- a/webclient/api/src/main/java/io/helidon/webclient/api/ClientUri.java +++ b/webclient/api/src/main/java/io/helidon/webclient/api/ClientUri.java @@ -193,8 +193,8 @@ public ClientUri resolve(URI uri) { uriBuilder.path(resolvePath(uriBuilder.path().path(), uri.getPath())); - if (uri.getRawQuery() != null) { - query.fromQueryString(uri.getRawQuery()); + if (uri.getQuery() != null) { + query.fromQueryString(uri.getQuery()); } if (uri.getRawFragment() != null) { diff --git a/webclient/api/src/main/java/io/helidon/webclient/api/WebClientCookieManager.java b/webclient/api/src/main/java/io/helidon/webclient/api/WebClientCookieManager.java index df13b9ff51c..6681449d9f8 100644 --- a/webclient/api/src/main/java/io/helidon/webclient/api/WebClientCookieManager.java +++ b/webclient/api/src/main/java/io/helidon/webclient/api/WebClientCookieManager.java @@ -142,12 +142,12 @@ public void response(ClientUri uri, ClientResponseHeaders headers) { if (headers.contains(Http.HeaderNames.SET_COOKIE)) { cookies = new HashMap<>(); - cookies.put(SET_COOKIE, headers.values(Http.HeaderNames.SET_COOKIE)); + cookies.put(SET_COOKIE, headers.get(Http.HeaderNames.SET_COOKIE).allValues()); } if (headers.contains(Http.HeaderNames.SET_COOKIE2)) { cookies = cookies == null ? new HashMap<>() : cookies; - cookies.put(SET_COOKIE2, headers.values(Http.HeaderNames.SET_COOKIE2)); + cookies.put(SET_COOKIE2, headers.get(Http.HeaderNames.SET_COOKIE2).allValues()); } if (cookies != null) { diff --git a/webclient/api/src/test/java/io/helidon/webclient/api/ClientUriTest.java b/webclient/api/src/test/java/io/helidon/webclient/api/ClientUriTest.java index 9619297d4ce..004064ae63c 100644 --- a/webclient/api/src/test/java/io/helidon/webclient/api/ClientUriTest.java +++ b/webclient/api/src/test/java/io/helidon/webclient/api/ClientUriTest.java @@ -70,7 +70,10 @@ void testQueryParams() { assertThat(helper.path(), is(UriPath.create("/loom/quick"))); assertThat(helper.port(), is(8080)); assertThat(helper.scheme(), is("http")); - assertThat(helper.query(), is(query)); + assertThat(helper.query().value("p1"), is("v1")); + assertThat(helper.query().value("p2"), is("v2")); + assertThat(helper.query().value("p3"), is("//v3//")); + assertThat(helper.query().getRaw("p3"), is("%2F%2Fv3%2F%2F")); } @Test diff --git a/webclient/tests/http1/src/test/java/io/helidon/webclient/tests/CookieTest.java b/webclient/tests/http1/src/test/java/io/helidon/webclient/tests/CookieTest.java index e4e215cabf7..49622e7377e 100644 --- a/webclient/tests/http1/src/test/java/io/helidon/webclient/tests/CookieTest.java +++ b/webclient/tests/http1/src/test/java/io/helidon/webclient/tests/CookieTest.java @@ -87,6 +87,14 @@ void testCookiePut() { } } + @Test + @Order(3) + void testCookieGetAfterPut() { + try (Http1ClientResponse response = client.get("/cookie").request()) { + assertThat(response.status(), is(Http.Status.OK_200)); + } + } + private static void getHandler(ServerRequest req, ServerResponse res) { if (req.headers().contains(Http.HeaderNames.COOKIE)) { Http.Header cookies = req.headers().get(Http.HeaderNames.COOKIE); @@ -111,6 +119,10 @@ private static void putHandler(ServerRequest req, ServerResponse res) { && cookies.allValues().contains("flavor2=vanilla") && cookies.allValues().contains("flavor3=strawberry") && cookies.allValues().contains("flavor4=raspberry")) { + // clear flavor1 and flavor2 + res.header(Http.HeaderNames.SET_COOKIE, + "flavor1=; Expires=Thu, 01-Jan-1970 00:00:10 GMT; Max-Age=0", + "flavor2=; Expires=Thu, 01-Jan-1970 00:00:10 GMT; Max-Age=0"); res.status(Http.Status.OK_200).send(); } else { res.status(Http.Status.BAD_REQUEST_400).send();