PR #790: address comments for preemptiveBaicAuth Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/9cb715b1 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/9cb715b1 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/9cb715b1
Branch: refs/heads/master Commit: 9cb715b1be3162371be701072bb9c9783bbf3d9f Parents: 6b9c6ac Author: Aled Sage <aled.s...@gmail.com> Authored: Fri Aug 4 11:23:52 2017 +0100 Committer: Aled Sage <aled.s...@gmail.com> Committed: Fri Aug 4 11:23:52 2017 +0100 ---------------------------------------------------------------------- .../org/apache/brooklyn/feed/http/HttpFeed.java | 9 ++- .../apache/brooklyn/feed/http/HttpFeedTest.java | 58 +++++++++++++++++++- .../test/http/RecordingHttpRequestHandler.java | 3 +- 3 files changed, 64 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/9cb715b1/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java b/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java index 37fb32f..a2d3125 100644 --- a/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java +++ b/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java @@ -242,8 +242,15 @@ public class HttpFeed extends AbstractFeed { throw new IllegalArgumentException("Must not enable preemptiveBasicAuth when there are no credentials, in feed for "+baseUri); } String username = checkNotNull(creds.getUserPrincipal().getName(), "username"); + if (username == null) { + throw new IllegalArgumentException("Must not enable preemptiveBasicAuth when username is null, in feed for "+baseUri); + } + if (username.contains(":")) { + throw new IllegalArgumentException("Username must not contain colon when preemptiveBasicAuth is enabled, in feed for "+baseUri); + } + String password = creds.getPassword(); - String toencode = username + (password == null ? "" : ":"+password); + String toencode = username + ":" + (password == null ? "" : password); String headerVal = "Basic " + BaseEncoding.base64().encode((toencode).getBytes(StandardCharsets.UTF_8)); return ImmutableMap.<String,String>builder() http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/9cb715b1/core/src/test/java/org/apache/brooklyn/feed/http/HttpFeedTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/feed/http/HttpFeedTest.java b/core/src/test/java/org/apache/brooklyn/feed/http/HttpFeedTest.java index 08abbe6..af66e45 100644 --- a/core/src/test/java/org/apache/brooklyn/feed/http/HttpFeedTest.java +++ b/core/src/test/java/org/apache/brooklyn/feed/http/HttpFeedTest.java @@ -204,6 +204,7 @@ public class HttpFeedTest extends BrooklynAppUnitTestSupport { @Test public void testUsesFailureHandlerOn4xx() throws Exception { + if (server != null) server.shutdown(); server = BetterMockWebServer.newInstanceLocalhost(); for (int i = 0; i < 100; i++) { server.enqueue(new MockResponse() @@ -232,6 +233,7 @@ public class HttpFeedTest extends BrooklynAppUnitTestSupport { @Test public void testUsesExceptionHandlerOn4xxAndNoFailureHandler() throws Exception { + if (server != null) server.shutdown(); server = BetterMockWebServer.newInstanceLocalhost(); for (int i = 0; i < 100; i++) { server.enqueue(new MockResponse() @@ -332,9 +334,11 @@ public class HttpFeedTest extends BrooklynAppUnitTestSupport { } // because takes a wee while + // TODO time-sensitive brittle test - relies on assertion spotting sensor value in first 10 polls (i.e. 1 second) @SuppressWarnings("rawtypes") @Test(groups="Integration") public void testPollsMultiClearsOnSubsequentFailure() throws Exception { + if (server != null) server.shutdown(); server = BetterMockWebServer.newInstanceLocalhost(); for (int i = 0; i < 10; i++) { server.enqueue(new MockResponse() @@ -366,10 +370,39 @@ public class HttpFeedTest extends BrooklynAppUnitTestSupport { } @Test + public void testFailsIfUsernameNull() throws Exception { + try { + feed = HttpFeed.builder() + .entity(entity) + .baseUrl(new URL("http://shouldNeverBeCalled.org";)) + .credentials(null, "Pa55w0rd") + .poll(new HttpPollConfig<Integer>(SENSOR_INT) + .period(100) + .onSuccess(HttpValueFunctions.responseCode()) + .onException(Functions.constant(-1))) + .build(); + Asserts.shouldHaveFailedPreviously(); + } catch (IllegalArgumentException e) { + Asserts.expectedFailureContainsIgnoreCase(e, "may not be null"); + } + } + + @Test public void testPreemptiveBasicAuth() throws Exception { - final String username = "brooklyn"; - final String password = "hunter2"; + runPreemptiveBasicAuth("brooklyn", "hunter2"); + } + @Test + public void testPreemptiveBasicAuthWithNoPassword() throws Exception { + runPreemptiveBasicAuth("brooklyn", null); + } + + @Test + public void testPreemptiveBasicAuthWithColonAndWhitespaceInPassword() throws Exception { + runPreemptiveBasicAuth("brooklyn", " passwordWith:colon\t "); + } + + protected void runPreemptiveBasicAuth(String username, String password) throws Exception { feed = HttpFeed.builder() .entity(entity) .baseUrl(server.getUrl("/")) @@ -406,6 +439,25 @@ public class HttpFeedTest extends BrooklynAppUnitTestSupport { } } + @Test + public void testPreemptiveBasicAuthFailsIfUserContainsColon() throws Exception { + try { + feed = HttpFeed.builder() + .entity(entity) + .baseUrl(new URL("http://shouldNeverBeCalled.org";)) + .credentials("userWith:colon", "Pa55w0rd") + .preemptiveBasicAuth(true) + .poll(new HttpPollConfig<Integer>(SENSOR_INT) + .period(100) + .onSuccess(HttpValueFunctions.responseCode()) + .onException(Functions.constant(-1))) + .build(); + Asserts.shouldHaveFailedPreviously(); + } catch (IllegalArgumentException e) { + Asserts.expectedFailureContains(e, "must not contain colon"); + } + } + // Expected behaviour of o.a.http.client is that it first sends the request without credentials, // and then when given a challenge for basic-auth it re-sends the request with the basic-auth header. @Test @@ -444,7 +496,7 @@ public class HttpFeedTest extends BrooklynAppUnitTestSupport { } public static String getBasicAuthHeaderVal(String username, String password) { - String toencode = username + (password == null ? "" : ":"+password); + String toencode = username + ":" + (password == null ? "" : password); return "Basic " + BaseEncoding.base64().encode((toencode).getBytes(StandardCharsets.UTF_8)); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/9cb715b1/utils/common/src/main/java/org/apache/brooklyn/test/http/RecordingHttpRequestHandler.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/org/apache/brooklyn/test/http/RecordingHttpRequestHandler.java b/utils/common/src/main/java/org/apache/brooklyn/test/http/RecordingHttpRequestHandler.java index 5b9ced4..d62c390 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/test/http/RecordingHttpRequestHandler.java +++ b/utils/common/src/main/java/org/apache/brooklyn/test/http/RecordingHttpRequestHandler.java @@ -29,7 +29,6 @@ import org.apache.http.HttpRequest; import org.apache.http.HttpResponse; import org.apache.http.protocol.HttpContext; import org.apache.http.protocol.HttpRequestHandler; -import org.testng.Assert; import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; @@ -57,7 +56,7 @@ public class RecordingHttpRequestHandler implements HttpRequestHandler { return; } } - Assert.fail("No request matching filter "+ filter); + Asserts.fail("No request matching filter "+ filter); } public void assertHasRequestEventually(Predicate<? super HttpRequest> filter) {
