Repository: hyperledger/quilt
Pull request #397 (2019-12-05 23:13:22)

Fixes rust auth handler for ILP-over-HTTP
Signed-off-by: David Fuelling <sappenin@gmail.com>

codecov[bot]: # [Codecov](https://codecov.io/gh/hyperledger/quilt/pull/397?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@4406c17`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/hyperledger/quilt/pull/397/graphs/tree.svg?width=650&token=XEfCq6zC6W&height=150&src=pr)](https://codecov.io/gh/hyperledger/quilt/pull/397?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##             master     #397   +/-   ##
=========================================
  Coverage          ?   82.52%           
  Complexity        ?      829           
=========================================
  Files             ?      249           
  Lines             ?     3754           
  Branches          ?      229           
=========================================
  Hits              ?     3098           
  Misses            ?      560           
  Partials          ?       96
```


| [Impacted Files](https://codecov.io/gh/hyperledger/quilt/pull/397?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...a/org/interledger/spsp/PaymentPointerResolver.java](https://codecov.io/gh/hyperledger/quilt/pull/397/diff?src=pr&el=tree#diff-c3BzcC1wYXJlbnQvc3BzcC1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2ludGVybGVkZ2VyL3Nwc3AvUGF5bWVudFBvaW50ZXJSZXNvbHZlci5qYXZh) | `100% <100%> (ø)` | `2 <1> (?)` | |
| [...er/spsp/client/rust/InterledgerRustNodeClient.java](https://codecov.io/gh/hyperledger/quilt/pull/397/diff?src=pr&el=tree#diff-c3BzcC1wYXJlbnQvc3BzcC1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvaW50ZXJsZWRnZXIvc3BzcC9jbGllbnQvcnVzdC9JbnRlcmxlZGdlclJ1c3ROb2RlQ2xpZW50LmphdmE=) | `76.66% <100%> (ø)` | `7 <2> (?)` | |
| [.../org/interledger/spsp/client/SimpleSpspClient.java](https://codecov.io/gh/hyperledger/quilt/pull/397/diff?src=pr&el=tree#diff-c3BzcC1wYXJlbnQvc3BzcC1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvaW50ZXJsZWRnZXIvc3BzcC9jbGllbnQvU2ltcGxlU3BzcENsaWVudC5qYXZh) | `83.33% <100%> (ø)` | `6 <3> (?)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/hyperledger/quilt/pull/397?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/hyperledger/quilt/pull/397?src=pr&el=footer). Last update [4406c17...bc4d19d](https://codecov.io/gh/hyperledger/quilt/pull/397?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/spsp-parent/spsp-client/src/main/java/org/interledger/spsp/client/SimpleSpspClient.java b/spsp-parent/spsp-client/src/main/java/org/interledger/spsp/client/SimpleSpspClient.java
index a64fa86..e85e497 100644
--- a/spsp-parent/spsp-client/src/main/java/org/interledger/spsp/client/SimpleSpspClient.java
+++ b/spsp-parent/spsp-client/src/main/java/org/interledger/spsp/client/SimpleSpspClient.java
@@ -11,6 +11,7 @@ import okhttp3.Request;
 import okhttp3.Response;
 
 import java.io.IOException;
+import java.util.Objects;
 
 /**
  * Basic implementation of an {@link SpspClient} that uses default okhttp3 client and jackson mapper.
@@ -27,22 +28,27 @@ public class SimpleSpspClient implements SpspClient {
     this(SpspClientDefaults.OK_HTTP_CLIENT, PaymentPointerResolver.defaultResolver(), SpspClientDefaults.MAPPER);
   }
 
-  public SimpleSpspClient(OkHttpClient httpClient,
-                          PaymentPointerResolver paymentPointerResolver,
-                          ObjectMapper objectMapper) {
-    this.httpClient = httpClient;
-    this.paymentPointerResolver = paymentPointerResolver;
-    this.objectMapper = objectMapper;
+  public SimpleSpspClient(
+      final OkHttpClient httpClient, final PaymentPointerResolver paymentPointerResolver,
+      final ObjectMapper objectMapper
+  ) {
+    this.httpClient = Objects.requireNonNull(httpClient);
+    this.paymentPointerResolver = Objects.requireNonNull(paymentPointerResolver);
+    this.objectMapper = Objects.requireNonNull(objectMapper);
   }
 
   @Override
-  public StreamConnectionDetails getStreamConnectionDetails(PaymentPointer paymentPointer)
+  public StreamConnectionDetails getStreamConnectionDetails(final PaymentPointer paymentPointer)
       throws InvalidReceiverClientException {
-    return getStreamConnectionDetails(HttpUrl.parse(paymentPointerResolver.resolve(paymentPointer)));
+    Objects.requireNonNull(paymentPointer);
+    return getStreamConnectionDetails(paymentPointerResolver.resolveHttpUrl(paymentPointer));
   }
 
   @Override
-  public StreamConnectionDetails getStreamConnectionDetails(HttpUrl spspUrl) throws InvalidReceiverClientException {
+  public StreamConnectionDetails getStreamConnectionDetails(final HttpUrl spspUrl)
+      throws InvalidReceiverClientException {
+    Objects.requireNonNull(spspUrl);
+
     return execute(new Request.Builder()
         .url(spspUrl)
         .header("Accept", ACCEPT_SPSP_JSON)
diff --git a/spsp-parent/spsp-client/src/main/java/org/interledger/spsp/client/rust/InterledgerRustNodeClient.java b/spsp-parent/spsp-client/src/main/java/org/interledger/spsp/client/rust/InterledgerRustNodeClient.java
index 80bce6f..5b1eae9 100644
--- a/spsp-parent/spsp-client/src/main/java/org/interledger/spsp/client/rust/InterledgerRustNodeClient.java
+++ b/spsp-parent/spsp-client/src/main/java/org/interledger/spsp/client/rust/InterledgerRustNodeClient.java
@@ -1,7 +1,7 @@
 package org.interledger.spsp.client.rust;
 
-import org.interledger.spsp.client.SpspClientDefaults;
 import org.interledger.spsp.client.InvalidReceiverClientException;
+import org.interledger.spsp.client.SpspClientDefaults;
 import org.interledger.spsp.client.SpspClientException;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
@@ -19,6 +19,7 @@ import org.immutables.value.Value.Immutable;
 
 import java.io.IOException;
 import java.math.BigDecimal;
+import java.util.Objects;
 
 /**
  * Client for interacting with API on a Rust Interledger Node.
@@ -30,20 +31,43 @@ public class InterledgerRustNodeClient {
   private final OkHttpClient httpClient;
   private final String authToken;
   private final ObjectMapper objectMapper;
-  private final String baseUri;
+  private final HttpUrl baseUrl;
 
-  public InterledgerRustNodeClient(OkHttpClient okHttpClient,
-                                   String authToken,
-                                   String baseUri) {
-    this.httpClient = okHttpClient;
-    this.authToken = authToken;
-    this.objectMapper = SpspClientDefaults.MAPPER;
-    this.baseUri = baseUri;
+  /**
+   * Required-args constructor.
+   *
+   * @param okHttpClient A {@link OkHttpClient}.
+   * @param authToken    An authentication token for the Rust Connector.
+   * @param baseUrl      An {@link String} that contains the Connector's base URL.
+   *
+   * @deprecated Use the constructor that accepts a {@link HttpUrl} instead.
+   */
+  @Deprecated
+  public InterledgerRustNodeClient(OkHttpClient okHttpClient, String authToken, String baseUrl) {
+    this(okHttpClient, authToken, HttpUrl.parse(baseUrl));
+  }
+
+  /**
+   * Required-args constructor.
+   *
+   * @param okHttpClient A {@link OkHttpClient}.
+   * @param authToken    An authentication token for the Rust Connector.
+   * @param baseUrl      An {@link HttpUrl} that contains the Connector's base URL.
+   *
+   * @deprecated Use the constructor that accepts a {@link HttpUrl} instead.
+   */
+  public InterledgerRustNodeClient(
+    final OkHttpClient okHttpClient, final String authToken, final HttpUrl baseUrl
+  ) {
+    this.httpClient = Objects.requireNonNull(okHttpClient);
+    this.authToken = Objects.requireNonNull(authToken);
+    this.objectMapper = Objects.requireNonNull(SpspClientDefaults.MAPPER);
+    this.baseUrl = Objects.requireNonNull(baseUrl);
   }
 
   public RustNodeAccount createAccount(RustNodeAccount rustNodeAccount) throws IOException {
     return execute(requestBuilder()
-      .url(HttpUrl.parse(baseUri + "/accounts"))
+      .url(baseUrl.newBuilder().addPathSegment("accounts").build())
       .post(RequestBody.create(objectMapper.writeValueAsString(rustNodeAccount), JSON))
       .build(), RustNodeAccount.class);
   }
@@ -55,7 +79,9 @@ public class InterledgerRustNodeClient {
 
   public BigDecimal getBalance(String accountName) throws SpspClientException {
     return execute(requestBuilder()
-      .url(HttpUrl.parse(baseUri + "/accounts/" + accountName + "/balance"))
+      .url(
+        baseUrl.newBuilder().addPathSegment("accounts").addPathSegment(accountName).addPathSegment("balance").build()
+      )
       .get()
       .build(), BalanceResponse.class)
       .getBalance();
@@ -80,6 +106,7 @@ public class InterledgerRustNodeClient {
   @JsonDeserialize(builder = ImmutableBalanceResponse.Builder.class)
   @JsonSerialize(as = ImmutableBalanceResponse.class)
   public interface BalanceResponse {
+
     BigDecimal getBalance();
   }
 
diff --git a/spsp-parent/spsp-core/pom.xml b/spsp-parent/spsp-core/pom.xml
index 193e7f3..0ac2696 100644
--- a/spsp-parent/spsp-core/pom.xml
+++ b/spsp-parent/spsp-core/pom.xml
@@ -36,6 +36,10 @@
       <groupId>com.google.guava</groupId>
       <artifactId>guava</artifactId>
     </dependency>
+    <dependency>
+      <groupId>com.squareup.okhttp3</groupId>
+      <artifactId>okhttp</artifactId>
+    </dependency>
     <dependency>
       <groupId>junit</groupId>
       <artifactId>junit</artifactId>
diff --git a/spsp-parent/spsp-core/src/main/java/org/interledger/spsp/PaymentPointerResolver.java b/spsp-parent/spsp-core/src/main/java/org/interledger/spsp/PaymentPointerResolver.java
index 20d2fb0..5ea3d74 100644
--- a/spsp-parent/spsp-core/src/main/java/org/interledger/spsp/PaymentPointerResolver.java
+++ b/spsp-parent/spsp-core/src/main/java/org/interledger/spsp/PaymentPointerResolver.java
@@ -1,5 +1,12 @@
 package org.interledger.spsp;
 
+import okhttp3.HttpUrl;
+
+import java.util.Objects;
+
+/**
+ * Defines how to resolve a payment pointer.
+ */
 public interface PaymentPointerResolver {
 
   static PaymentPointerResolver defaultResolver() {
@@ -7,17 +14,43 @@ public interface PaymentPointerResolver {
   }
 
   /**
-   * Resolves PaymentPointer to it's endpoint URL per
-   * https://interledger.org/rfcs/0026-payment-pointers/#payment-setup-protocol-receiver-endpoint-resolution
+   * Resolves PaymentPointer to it's endpoint URL.
+   *
+   * @param paymentPointer ilp payment pointer
+   *
+   * @return endpoint url
+   *
+   * @see "https://interledger.org/rfcs/0026-payment-pointers/#payment-setup-protocol-receiver-endpoint-resolution"
+   * @deprecated Prefer the variant of {@link #resolve} that returns an {@link HttpUrl};
+   */
+  @Deprecated
+  default String resolve(final PaymentPointer paymentPointer) {
+    Objects.requireNonNull(paymentPointer);
+    return resolveHttpUrl(paymentPointer).toString();
+  }
+
+  /**
+   * Resolves PaymentPointer to it's endpoint URL.
+   *
    * @param paymentPointer ilp payment pointer
+   *
    * @return endpoint url
+   *
+   * @see "https://interledger.org/rfcs/0026-payment-pointers/#payment-setup-protocol-receiver-endpoint-resolution"
    */
-  String resolve(PaymentPointer paymentPointer);
+  HttpUrl resolveHttpUrl(PaymentPointer paymentPointer);
 
+  /**
+   * The default implementation of {@link PaymentPointerResolver}.
+   */
   class DefaultPaymentPointerResolver implements PaymentPointerResolver {
+
     @Override
-    public String resolve(PaymentPointer paymentPointer) {
-      return "https://" + paymentPointer.host() + paymentPointer.path();
+    public HttpUrl resolveHttpUrl(final PaymentPointer paymentPointer) {
+      Objects.requireNonNull(paymentPointer);
+
+      // Prefer parsing here over manual assembly so that any existing forward-slashes are interpreted properly.
+      return HttpUrl.parse("https://" + paymentPointer.host() + paymentPointer.path());
     }
   }
 


Test patch:
diff --git a/spsp-parent/spsp-client/src/test/java/org/interledger/spsp/client/SimpleSpspClientTest.java b/spsp-parent/spsp-client/src/test/java/org/interledger/spsp/client/SimpleSpspClientTest.java
index 02e4468..ec4c0db 100644
--- a/spsp-parent/spsp-client/src/test/java/org/interledger/spsp/client/SimpleSpspClientTest.java
+++ b/spsp-parent/spsp-client/src/test/java/org/interledger/spsp/client/SimpleSpspClientTest.java
@@ -28,7 +28,7 @@ public class SimpleSpspClientTest {
   @Before
   public void setUp() {
     client = new SimpleSpspClient(SpspClientDefaults.OK_HTTP_CLIENT,
-        (pointer) -> wireMockRule.baseUrl() + pointer.path(),
+        (pointer) -> HttpUrl.parse(wireMockRule.baseUrl() + pointer.path()),
         SpspClientDefaults.MAPPER);
   }
 
@@ -110,4 +110,4 @@ public class SimpleSpspClientTest {
     return PaymentPointer.of("$test.wiremock/" + testClient);
   }
 
-}
\ No newline at end of file
+}
diff --git a/spsp-parent/spsp-core/src/test/java/org/interledger/spsp/PaymentPointerResolverTest.java b/spsp-parent/spsp-core/src/test/java/org/interledger/spsp/PaymentPointerResolverTest.java
index 181ec44..a88ad60 100644
--- a/spsp-parent/spsp-core/src/test/java/org/interledger/spsp/PaymentPointerResolverTest.java
+++ b/spsp-parent/spsp-core/src/test/java/org/interledger/spsp/PaymentPointerResolverTest.java
@@ -2,6 +2,7 @@ package org.interledger.spsp;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+import okhttp3.HttpUrl;
 import org.junit.Test;
 
 /**
@@ -16,9 +17,57 @@ public class PaymentPointerResolverTest {
     assertThat(resolver.resolve(PaymentPointer.of("$example.com/foo"))).isEqualTo("https://example.com/foo");
   }
 
+  @Test
+  public void resolveWithLongPath() {
+    assertThat(resolver.resolve(PaymentPointer.of("$example.com/foo/bar/baz")))
+      .isEqualTo("https://example.com/foo/bar/baz");
+  }
+
+  @Test
+  public void resolveWithSubdomain() {
+    assertThat(resolver.resolve(PaymentPointer.of("$david.example.com")))
+      .isEqualTo("https://david.example.com/.well-known/pay");
+  }
+
+  @Test
+  public void resolveWithSubdomainAndPath() {
+    assertThat(resolver.resolve(PaymentPointer.of("$david.example.com/foo")))
+      .isEqualTo("https://david.example.com/foo");
+  }
+
   @Test
   public void toUrlWithout() {
     assertThat(resolver.resolve(PaymentPointer.of("$example.com"))).isEqualTo("https://example.com/.well-known/pay");
   }
 
+  @Test
+  public void resolveHttpUrl() {
+    assertThat(resolver.resolveHttpUrl(PaymentPointer.of("$example.com/foo")))
+      .isEqualTo(HttpUrl.parse("https://example.com/foo"));
+  }
+
+  @Test
+  public void resolveHttpUrlWithLongPath() {
+    assertThat(resolver.resolveHttpUrl(PaymentPointer.of("$example.com/foo/bar/baz")))
+      .isEqualTo(HttpUrl.parse("https://example.com/foo/bar/baz"));
+  }
+
+  @Test
+  public void resolveHttpUrlWithSubdomain() {
+    assertThat(resolver.resolveHttpUrl(PaymentPointer.of("$david.example.com")))
+      .isEqualTo(HttpUrl.parse("https://david.example.com/.well-known/pay"));
+  }
+
+  @Test
+  public void resolveHttpUrlWithSubdomainAndPath() {
+    assertThat(resolver.resolveHttpUrl(PaymentPointer.of("$david.example.com/foo")))
+      .isEqualTo(HttpUrl.parse("https://david.example.com/foo"));
+  }
+
+  @Test
+  public void resolveHttpUrltoUrlWithoutPath() {
+    assertThat(resolver.resolveHttpUrl(PaymentPointer.of("$example.com")))
+      .isEqualTo(HttpUrl.parse("https://example.com/.well-known/pay"));
+  }
+
 }
diff --git a/stream-parent/stream-client/src/test/java/org/interledger/stream/sender/SimpleStreamSenderIT.java b/stream-parent/stream-client/src/test/java/org/interledger/stream/sender/SimpleStreamSenderIT.java
index b2fc7d6..f3e4519 100644
--- a/stream-parent/stream-client/src/test/java/org/interledger/stream/sender/SimpleStreamSenderIT.java
+++ b/stream-parent/stream-client/src/test/java/org/interledger/stream/sender/SimpleStreamSenderIT.java
@@ -111,13 +111,24 @@ public class SimpleStreamSenderIT {
   public void setUp() throws IOException {
     OkHttpClient httpClient = this.constructOkHttpClient();
 
-    String interledgerNodeBaseUri = this.getInterledgerBaseUri();
-    this.nodeClient = new InterledgerRustNodeClient(httpClient,
+    final HttpUrl interledgerNodeBaseUrl = this.getInterledgerBaseUri();
+    this.nodeClient = new InterledgerRustNodeClient(
+        httpClient,
         AUTH_TOKEN,
-        interledgerNodeBaseUri);
+        interledgerNodeBaseUrl
+    );
 
-    this.spspClient = new SimpleSpspClient(httpClient,
-        (pointer) -> interledgerNodeBaseUri + pointer.path(), SpspClientDefaults.MAPPER);
+    this.spspClient = new SimpleSpspClient(
+        httpClient,
+        (pointer) -> {
+          String pointerPath = pointer.path();
+          if (pointerPath.startsWith("/")) {
+            pointerPath = pointerPath.substring(1);
+          }
+          return interledgerNodeBaseUrl.newBuilder().addPathSegments(pointerPath).build();
+        },
+        SpspClientDefaults.MAPPER
+    );
 
     final IlpOverHttpLinkSettings linkSettings = IlpOverHttpLinkSettings.builder()
         .incomingHttpLinkSettings(IncomingLinkSettings.builder()
@@ -127,7 +138,7 @@ public class SimpleStreamSenderIT {
         .outgoingHttpLinkSettings(OutgoingLinkSettings.builder()
             .authType(AuthType.SIMPLE)
             .tokenSubject(SENDER_ACCOUNT_USERNAME)
-            .url(HttpUrl.parse(interledgerNodeBaseUri + "/ilp"))
+            .url(this.constructIlpOverHttpUrl(SENDER_ACCOUNT_USERNAME))
             .encryptedTokenSharedSecret(AUTH_TOKEN)
             .build())
         .build();
@@ -138,7 +149,7 @@ public class SimpleStreamSenderIT {
         httpClient,
         SpspClientDefaults.MAPPER,
         InterledgerCodecContextFactory.oer(),
-        new SimpleBearerTokenSupplier(SENDER_ACCOUNT_USERNAME + ":" + AUTH_TOKEN)
+        () -> AUTH_TOKEN
     );
     link.setLinkId(LinkId.of(LINK_ID));
 
@@ -504,7 +515,6 @@ public class SimpleStreamSenderIT {
   public void sendMoneyWithWrongLinkPassword() throws IOException {
     final String connectorAccountUsername = UUID.randomUUID().toString().replace("-", "");
 
-    final String interledgerNodeBaseUri = this.getInterledgerBaseUri();
     final OkHttpClient httpClient = this.constructOkHttpClient();
     final IlpOverHttpLinkSettings linkSettings = IlpOverHttpLinkSettings.builder()
         .incomingHttpLinkSettings(IncomingLinkSettings.builder()
@@ -514,7 +524,7 @@ public class SimpleStreamSenderIT {
         .outgoingHttpLinkSettings(OutgoingLinkSettings.builder()
             .authType(AuthType.SIMPLE)
             .tokenSubject(connectorAccountUsername)
-            .url(HttpUrl.parse(interledgerNodeBaseUri + "/ilp"))
+            .url(this.constructIlpOverHttpUrl(SENDER_ACCOUNT_USERNAME))
             .encryptedTokenSharedSecret("wrong-password")
             .build())
         .build();
@@ -613,8 +623,34 @@ public class SimpleStreamSenderIT {
         .routingRelation(RustNodeAccount.RoutingRelation.PEER);
   }
 
-  private String getInterledgerBaseUri() {
-    return "http://" + interledgerNode.getContainerIpAddress() + ":" + interledgerNode.getFirstMappedPort();
+  /**
+   * Helper method to return the base URL for the Rust Connector.
+   *
+   * @return An {@link HttpUrl} to communicate with.
+   */
+  private HttpUrl getInterledgerBaseUri() {
+    return new HttpUrl.Builder()
+        .scheme("http")
+        .host(interledgerNode.getContainerIpAddress())
+        .port(interledgerNode.getFirstMappedPort())
+        .build();
+  }
+
+  /**
+   * Helper method to construct the ILP-over-HTTP endpoint method for a given account in the Rust Connector.
+   *
+   * @param accountId The account identifier to send ILP packets to.
+   *
+   * @return An {@link HttpUrl} to use with ILP-over-HTTP using {@code accountId}.
+   *
+   * @see "https://github.com/interledger-rs/interledger-rs/pull/553"
+   */
+  private HttpUrl constructIlpOverHttpUrl(final String accountId) {
+    return getInterledgerBaseUri().newBuilder()
+        .addPathSegment("accounts")
+        .addPathSegment(accountId)
+        .addPathSegment("ilp")
+        .build();
   }
 
   private OkHttpClient constructOkHttpClient() {