Skip to content
This repository has been archived by the owner on Sep 29, 2021. It is now read-only.

Commit

Permalink
seperate out different flows in AuthenticatingHttpConnector
Browse files Browse the repository at this point in the history 
this should make it easier to follow the various flows for different
authentication types and avoid the hairy `while true` loop.
  • Loading branch information
@mattnworb
mattnworb committed Dec 16, 2015
1 parent 38b89c5 commit 8f9decf
Show file tree
Hide file tree
Showing 2 changed files with 137 additions and 54 deletions.
123 changes: 75 additions & 48 deletions helios-client/src/main/java/com/spotify/helios/client/AuthenticatingHttpConnector.java
View file
Expand Up @@ -20,8 +20,8 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Optional;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Queues;

import com.spotify.helios.client.HttpsHandlers.CertificateFileHttpsHandler;
import com.spotify.helios.client.HttpsHandlers.SshAgentHttpsHandler;
import com.spotify.helios.common.HeliosException;
import com.spotify.sshagentproxy.AgentProxy;
Expand All @@ -38,9 +38,10 @@
import java.net.URISyntaxException;
import java.net.UnknownHostException;
import java.nio.file.Path;
import java.util.Deque;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Queue;

import static java.net.HttpURLConnection.HTTP_FORBIDDEN;
import static java.net.HttpURLConnection.HTTP_UNAUTHORIZED;
Expand All @@ -53,7 +54,6 @@ public class AuthenticatingHttpConnector implements HttpConnector {

private static final Logger log = LoggerFactory.getLogger(AuthenticatingHttpConnector.class);

// TODO (mbrown): username is here to pass to SshAgentSSLSocketFactory, could be handled nicer
private final String user;
private final Optional<AgentProxy> agentProxy;
private final Optional<Path> clientCertificatePath;
Expand Down Expand Up @@ -109,63 +109,90 @@ public HttpURLConnection connect(final URI uri, final String method, final byte[
throw new HeliosException(e);
}

final Deque<Identity> ids;
if (ipUri.getScheme().equalsIgnoreCase("https")) {
ids = Queues.newArrayDeque(identities);
} else {
ids = Queues.newArrayDeque();
try {
log.debug("connecting to {}", ipUri);

// prioritize using the certificate file if set
if (clientCertificatePath.isPresent() && clientKeyPath.isPresent()) {
return connectWithCertificateFile(ipUri, method, entity, headers);
}
// ssh-agent based authentication
else if (agentProxy.isPresent() && !identities.isEmpty()) {
return connectWithIdentities(identities, ipUri, method, entity, headers);
} else {
// no authentication
return doConnect(ipUri, method, entity, headers);
}

} catch (ConnectException | SocketTimeoutException | UnknownHostException e) {
// UnknownHostException happens if we can't resolve hostname into IP address.
// UnknownHostException's getMessage method returns just the hostname which is a
// useless message, so log the exception class name to provide more info.
log.debug(e.toString());
throw new HeliosException("Unable to connect to master", e);
} catch (IOException e) {
throw new HeliosException(e);
}
}

try {
while (true) {
final Identity identity = ids.poll();

try {
log.debug("connecting to {}", ipUri);

if (clientCertificatePath.isPresent() && clientKeyPath.isPresent()) {
delegate.setExtraHttpsHandler(new HttpsHandlers.CertificateFileHttpsHandler(
user, clientCertificatePath.get(), clientKeyPath.get()));
} else if (agentProxy.isPresent() && identity != null) {
delegate.setExtraHttpsHandler(new SshAgentHttpsHandler(
user, agentProxy.get(), identity));
} else {
// no authentication set up!
}
private HttpURLConnection connectWithCertificateFile(final URI ipUri, final String method,
final byte[] entity,
final Map<String, List<String>> headers)
throws HeliosException {

final HttpURLConnection connection = delegate.connect(ipUri, method, entity, headers);
delegate.setExtraHttpsHandler(
new CertificateFileHttpsHandler(user, clientCertificatePath.get(), clientKeyPath.get()));

final int responseCode = connection.getResponseCode();
if (((responseCode == HTTP_FORBIDDEN) || (responseCode == HTTP_UNAUTHORIZED))
&& !ids.isEmpty()) {
// there was some sort of security error. if we have any more SSH identities to try,
// retry with the next available identity
log.debug("retrying with next SSH identity since {} failed",
identity == null ? "the previous one" : identity.getComment());
continue;
}
return doConnect(ipUri, method, entity, headers);
}

return connection;
} catch (ConnectException | SocketTimeoutException | UnknownHostException e) {
// UnknownHostException happens if we can't resolve hostname into IP address.
// UnknownHostException's getMessage method returns just the hostname which is a
// useless message, so log the exception class name to provide more info.
log.debug(e.toString());
throw new HeliosException("Unable to connect to master", e);
}
private HttpURLConnection connectWithIdentities(final List<Identity> identities, final URI uri,
final String method, final byte[] entity,
final Map<String, List<String>> headers)
throws IOException, HeliosException {

if (identities.isEmpty()) {
throw new IllegalArgumentException("identities cannot be empty");
}

final Queue<Identity> queue = new LinkedList<>(identities);
HttpURLConnection connection = null;
while (!queue.isEmpty()) {
final Identity identity = queue.poll();

delegate.setExtraHttpsHandler(new SshAgentHttpsHandler(user, agentProxy.get(), identity));

connection = doConnect(uri, method, entity, headers);

// check the status and retry the request if necessary
final int responseCode = connection.getResponseCode();

final boolean retryResponse =
responseCode == HTTP_FORBIDDEN || responseCode == HTTP_UNAUTHORIZED;

if (retryResponse && !queue.isEmpty()) {
// there was some sort of security error. if we have any more SSH identities to try,
// retry with the next available identity
log.debug("retrying with next SSH identity since {} failed",
identity == null ? "the previous one" : identity.getComment());
continue;
}
} catch (IOException e) {
throw new HeliosException(e);
break;
}
return connection;
}

private HttpURLConnection doConnect(final URI uri, final String method, final byte[] entity,
final Map<String, List<String>> headers)
throws HeliosException {
return delegate.connect(uri, method, entity, headers);
}

private URI toIpUri(Endpoint endpoint, URI uri) throws URISyntaxException {
final URI endpointUri = endpoint.getUri();
final String fullpath = endpointUri.getPath() + uri.getPath();

final String uriScheme = endpointUri.getScheme();

return new URI(uriScheme,
return new URI(
endpointUri.getScheme(),
endpointUri.getUserInfo(),
endpoint.getIp().getHostAddress(),
endpointUri.getPort(),
Expand Down
68 changes: 62 additions & 6 deletions helios-client/src/test/java/com/spotify/helios/client/AuthenticatingHttpConnectorTest.java
View file
Expand Up @@ -115,6 +115,26 @@ private Identity mockIdentity() {
return identity;
}

@Test(expected = IllegalArgumentException.class)
public void misMatchedCertificateArguments1() {
new AuthenticatingHttpConnector("user",
Optional.<AgentProxy>absent(),
Optional.of(Paths.get("/foo")),
Optional.<Path>absent(),
EndpointIterator.of(endpoints),
connector);
}

@Test(expected = IllegalArgumentException.class)
public void misMatchedCertificateArguments2() {
new AuthenticatingHttpConnector("user",
Optional.<AgentProxy>absent(),
Optional.<Path>absent(),
Optional.of(Paths.get("/foo")),
EndpointIterator.of(endpoints),
connector);
}

@Test
public void testNoIdentities_ResponseIsOK() throws Exception {
final AuthenticatingHttpConnector authConnector = createAuthenticatingConnector(
Expand All @@ -125,9 +145,9 @@ public void testNoIdentities_ResponseIsOK() throws Exception {

final HttpsURLConnection connection = mock(HttpsURLConnection.class);
when(connector.connect(argThat(matchesAnyEndpoint(path)),
eq(method),
eq(entity),
eq(headers))
eq(method),
eq(entity),
eq(headers))
).thenReturn(connection);
when(connection.getResponseCode()).thenReturn(200);

Expand All @@ -146,9 +166,9 @@ public void testCertFile_ResponseIsOK() throws Exception {

final HttpsURLConnection connection = mock(HttpsURLConnection.class);
when(connector.connect(argThat(matchesAnyEndpoint(path)),
eq(method),
eq(entity),
eq(headers))
eq(method),
eq(entity),
eq(headers))
).thenReturn(connection);
when(connection.getResponseCode()).thenReturn(200);

Expand Down Expand Up @@ -216,6 +236,42 @@ public void testOneIdentity_ResponseIsUnauthorized() throws Exception {
returnedConnection, connection);
}

@Test
public void testTwoIdentities_ResponseIsUnauthorized() throws Exception {

final AgentProxy proxy = mock(AgentProxy.class);
final Identity id1 = mockIdentity();
final Identity id2 = mockIdentity();

final AuthenticatingHttpConnector authConnector =
createAuthenticatingConnector(Optional.of(proxy), ImmutableList.of(id1, id2));

final String path = "/another/one";

// set up two seperate connect() calls - the first returns 401 and the second 200 OK
final HttpsURLConnection connection1 = mock(HttpsURLConnection.class);
when(connection1.getResponseCode()).thenReturn(401);

final HttpsURLConnection connection2 = mock(HttpsURLConnection.class);
when(connection2.getResponseCode()).thenReturn(200);

when(connector.connect(argThat(matchesAnyEndpoint(path)),
eq(method),
eq(entity),
eq(headers))
).thenReturn(connection1, connection2);

URI uri = new URI("https://helios" + path);

HttpURLConnection returnedConnection = authConnector.connect(uri, method, entity, headers);

verify(connector).setExtraHttpsHandler(sshAgentHttpsHandlerWithArgs(USER, proxy, id1));
verify(connector).setExtraHttpsHandler(sshAgentHttpsHandlerWithArgs(USER, proxy, id2));

assertSame("Expect returned connection to be the second one, with successful response code",
returnedConnection, connection2);
}

private static Endpoint endpoint(final URI uri, final InetAddress ip) {
return new Endpoint() {
@Override
Expand Down

0 comments on commit 8f9decf

Please sign in to comment.