[ANCHOR-380] Add client_id to getRate() in Sep31Service (#1068)

<!-- If you're making a doc PR or something tiny where the below is
irrelevant, delete this
template and use a short description, but in your description aim to
include both what the
change is, and why it is being made, with enough context for anyone to
understand. -->

### Description

Fix the bug where `client_id` was not passed to `getRate()` when the
customer of the wallet exists.

### Context
Bug fix

### Testing

./gradlew test

---------

Co-authored-by: Jiahui Hu <[email protected]>

core/src/main/java/org/stellar/anchor/config/ClientsConfig.java

@@ -0,0 +1,27 @@
+package org.stellar.anchor.config;
+
+import lombok.AllArgsConstructor;
+import lombok.Data;
+import lombok.NoArgsConstructor;
+
+public interface ClientsConfig {
+  @Data
+  @AllArgsConstructor
+  @NoArgsConstructor
+  class ClientConfig {
+    String name;
+    ClientType type;
+    String signingKey;
+    String domain;
+    String callbackUrl;
+  }
+
+  enum ClientType {
+    CUSTODIAL,
+    NONCUSTODIAL
+  }
+
+  ClientConfig getClientConfigBySigningKey(String signingKey);
+
+  ClientConfig getClientConfigByDomain(String domain);
+}

core/src/main/java/org/stellar/anchor/config/Sep10Config.java

@@ -67,22 +67,18 @@ public interface Sep10Config {
   boolean isClientAttributionRequired();
 
   /**
-   * Get the list of allowed client domains if the client attribution is required.
+   * Get the list of allowed client domains.
    *
    * @return the list of allowed client domains.
    */
-  List<String> getClientAttributionAllowList();
+  List<String> getAllowedClientDomains();
 
   /**
-   * Whether to require authenticating clients to be in the list of known custodial accounts. # # If
-   * the flag is set to true, the client must be one of the custodial clients defined in the clients
-   * section # of this configuration file.
+   * Get the list of allowed client names.
    *
-   * <p>The flag is only relevant for custodial wallets.
-   *
-   * @return true if known custodial account is required.
+   * @return the list of allowed client names.
    */
-  boolean isKnownCustodialAccountRequired();
+  List<String> getAllowedClientNames();
 
   /**
    * Set the list of known custodial accounts.

core/src/main/java/org/stellar/anchor/sep10/Sep10Service.java

@@ -76,17 +76,9 @@ public ChallengeResponse createChallenge(ChallengeRequest challengeRequest) thro
           sep10Config.getKnownCustodialAccountList().contains(challengeRequest.getAccount().trim());
     }
 
-    if (sep10Config.isKnownCustodialAccountRequired() && !custodialWallet) {
-      // validate that requesting account is allowed access
-      infoF("requesting account: {} is not in allow list", challengeRequest.getAccount().trim());
-      throw new SepNotAuthorizedException("unable to process");
-    }
-
-    if (custodialWallet) {
-      if (challengeRequest.getClientDomain() != null) {
-        throw new SepValidationException(
-            "client_domain must not be specified if the account is an custodial-wallet account");
-      }
+    if (custodialWallet && challengeRequest.getClientDomain() != null) {
+      throw new SepValidationException(
+          "client_domain must not be specified if the account is an custodial-wallet account");
     }
 
     if (!custodialWallet && sep10Config.isClientAttributionRequired()) {
@@ -95,7 +87,7 @@ public ChallengeResponse createChallenge(ChallengeRequest challengeRequest) thro
         throw new SepValidationException("client_domain is required");
       }
 
-      List<String> allowList = sep10Config.getClientAttributionAllowList();
+      List<String> allowList = sep10Config.getAllowedClientDomains();
       if (!allowList.contains(challengeRequest.getClientDomain())) {
         infoF(
             "client_domain provided ({}) is not in the configured allow list",

core/src/main/java/org/stellar/anchor/sep31/Sep31Service.java

@@ -13,7 +13,6 @@
 import static org.stellar.anchor.util.MathHelper.formatAmount;
 import static org.stellar.anchor.util.MetricConstants.SEP31_TRANSACTION_CREATED;
 import static org.stellar.anchor.util.MetricConstants.SEP31_TRANSACTION_PATCHED;
-import static org.stellar.anchor.util.SepHelper.*;
 import static org.stellar.anchor.util.SepHelper.amountEquals;
 import static org.stellar.anchor.util.SepHelper.generateSepTransactionId;
 import static org.stellar.anchor.util.SepHelper.validateAmount;
@@ -64,7 +63,10 @@
 import org.stellar.anchor.asset.AssetService;
 import org.stellar.anchor.auth.Sep10Jwt;
 import org.stellar.anchor.config.AppConfig;
+import org.stellar.anchor.config.ClientsConfig;
+import org.stellar.anchor.config.ClientsConfig.ClientConfig;
 import org.stellar.anchor.config.CustodyConfig;
+import org.stellar.anchor.config.Sep10Config;
 import org.stellar.anchor.config.Sep31Config;
 import org.stellar.anchor.custody.CustodyService;
 import org.stellar.anchor.event.EventService;
@@ -75,12 +77,13 @@
 import org.stellar.anchor.util.TransactionHelper;
 
 public class Sep31Service {
-
   private final AppConfig appConfig;
+  private final Sep10Config sep10Config;
   private final Sep31Config sep31Config;
   private final Sep31TransactionStore sep31TransactionStore;
   private final Sep31DepositInfoGenerator sep31DepositInfoGenerator;
   private final Sep38QuoteStore sep38QuoteStore;
+  private final ClientsConfig clientsConfig;
   private final AssetService assetService;
   private final FeeIntegration feeIntegration;
   private final CustomerIntegration customerIntegration;
@@ -93,10 +96,12 @@ public class Sep31Service {
 
   public Sep31Service(
       AppConfig appConfig,
+      Sep10Config sep10Config,
       Sep31Config sep31Config,
       Sep31TransactionStore sep31TransactionStore,
       Sep31DepositInfoGenerator sep31DepositInfoGenerator,
       Sep38QuoteStore sep38QuoteStore,
+      ClientsConfig clientsConfig,
       AssetService assetService,
       FeeIntegration feeIntegration,
       CustomerIntegration customerIntegration,
@@ -106,10 +111,12 @@ public Sep31Service(
     debug("appConfig:", appConfig);
     debug("sep31Config:", sep31Config);
     this.appConfig = appConfig;
+    this.sep10Config = sep10Config;
     this.sep31Config = sep31Config;
     this.sep31TransactionStore = sep31TransactionStore;
     this.sep31DepositInfoGenerator = sep31DepositInfoGenerator;
     this.sep38QuoteStore = sep38QuoteStore;
+    this.clientsConfig = clientsConfig;
     this.assetService = assetService;
     this.feeIntegration = feeIntegration;
     this.customerIntegration = customerIntegration;
@@ -557,13 +564,28 @@ void updateFee() throws SepValidationException, AnchorException {
                     .receiveAmount(null)
                     .senderId(request.getSenderId())
                     .receiverId(request.getReceiverId())
-                    .clientId(token.getAccount())
+                    .clientId(getClientName())
                     .build())
             .getFee();
     infoF("Fee for request ({}) is ({})", request, fee);
     Context.get().setFee(fee);
   }
 
+  String getClientName() throws BadRequestException {
+    return getClientName(Context.get().getSep10Jwt().getAccount());
+  }
+
+  String getClientName(String account) throws BadRequestException {
+    ClientConfig client = clientsConfig.getClientConfigBySigningKey(account);
+    if (sep10Config.isClientAttributionRequired() && client == null) {
+      throw new BadRequestException("Client not found");
+    }
+    if (client != null && !sep10Config.getAllowedClientDomains().contains(client.getDomain())) {
+      client = null;
+    }
+    return client == null ? null : client.getName();
+  }
+
   /**
    * validateSenderAndReceiver will validate if the SEP-31 sender and receiver exist and their
    * status is ACCEPTED.
@@ -704,7 +726,6 @@ private static Sep31InfoResponse sep31InfoResponseFromAssetInfoList(List<AssetIn
 
   @Data
   public static class Context {
-
     private Sep31Transaction transaction;
     private Sep31PostTransactionRequest request;
     private Sep38Quote quote;

core/src/main/java/org/stellar/anchor/sep38/Sep38Service.java

@@ -2,9 +2,10 @@
 
 import static org.stellar.anchor.api.sep.sep38.Sep38Context.SEP31;
 import static org.stellar.anchor.api.sep.sep38.Sep38Context.SEP6;
+import static org.stellar.anchor.api.sep.sep38.Sep38QuoteResponse.*;
 import static org.stellar.anchor.event.EventService.EventQueue.TRANSACTION;
 import static org.stellar.anchor.util.BeanHelper.updateField;
-import static org.stellar.anchor.util.Log.debug;
+import static org.stellar.anchor.util.Log.*;
 import static org.stellar.anchor.util.MathHelper.decimal;
 import static org.stellar.anchor.util.MathHelper.formatAmount;
 import static org.stellar.anchor.util.MetricConstants.SEP38_PRICE_QUERIED;
@@ -20,9 +21,7 @@
 import java.time.Instant;
 import java.util.*;
 import org.apache.commons.lang3.StringUtils;
-import org.stellar.anchor.api.callback.GetRateRequest;
-import org.stellar.anchor.api.callback.GetRateResponse;
-import org.stellar.anchor.api.callback.RateIntegration;
+import org.stellar.anchor.api.callback.*;
 import org.stellar.anchor.api.event.AnchorEvent;
 import org.stellar.anchor.api.exception.AnchorException;
 import org.stellar.anchor.api.exception.BadRequestException;
@@ -139,7 +138,8 @@ public void validateAsset(String prefix, String assetName) throws AnchorExceptio
     }
   }
 
-  public GetPriceResponse getPrice(Sep38GetPriceRequest getPriceRequest) throws AnchorException {
+  public GetPriceResponse getPrice(Sep10Jwt token, Sep38GetPriceRequest getPriceRequest)
+      throws AnchorException {
     String sellAssetName = getPriceRequest.getSellAssetName();
     String sellAmount = getPriceRequest.getSellAmount();
     String sellDeliveryMethod = getPriceRequest.getSellDeliveryMethod();
@@ -215,7 +215,7 @@ public GetPriceResponse getPrice(Sep38GetPriceRequest getPriceRequest) throws An
       validateAmountLimit("sell_", sellAmount, sendMinLimit, sendMaxLimit);
     }
 
-    GetRateRequest request =
+    GetRateRequest.GetRateRequestBuilder rrBuilder =
         GetRateRequest.builder()
             .type(GetRateRequest.Type.INDICATIVE)
             .sellAsset(sellAssetName)
@@ -224,8 +224,15 @@ public GetPriceResponse getPrice(Sep38GetPriceRequest getPriceRequest) throws An
             .buyAsset(buyAssetName)
             .buyAmount(buyAmount)
             .buyDeliveryMethod(buyDeliveryMethod)
-            .countryCode(countryCode)
-            .build();
+            .countryCode(countryCode);
+
+    String clientId = getClientIdFromToken(token);
+    if (clientId != null) {
+      rrBuilder.clientId(clientId);
+    }
+
+    // Get the rate
+    GetRateRequest request = rrBuilder.build();
     GetRateResponse rateResponse = this.rateIntegration.getRate(request);
     GetRateResponse.Rate rate = rateResponse.getRate();
 
@@ -348,7 +355,7 @@ public Sep38QuoteResponse postQuote(Sep10Jwt token, Sep38PostQuoteRequest reques
       validateAmountLimit("sell_", request.getSellAmount(), sendMinLimit, sendMaxLimit);
     }
 
-    GetRateRequest getRateRequest =
+    GetRateRequest.GetRateRequestBuilder getRateRequestBuilder =
         GetRateRequest.builder()
             .type(GetRateRequest.Type.FIRM)
             .sellAsset(request.getSellAssetName())
@@ -358,11 +365,17 @@ public Sep38QuoteResponse postQuote(Sep10Jwt token, Sep38PostQuoteRequest reques
             .buyAmount(request.getBuyAmount())
             .buyDeliveryMethod(request.getBuyDeliveryMethod())
             .countryCode(request.getCountryCode())
-            .expireAfter(request.getExpireAfter())
-            .clientId(account)
-            .build();
-    GetRateResponse.Rate rate = this.rateIntegration.getRate(getRateRequest).getRate();
+            .expireAfter(request.getExpireAfter());
+
+    // Sets the clientId if the customer exists
+    String clientId = getClientIdFromToken(token);
+    if (clientId != null) {
+      getRateRequestBuilder.clientId(clientId);
+    }
+    GetRateRequest getRateRequest = getRateRequestBuilder.build();
 
+    // Get and set the rate
+    GetRateResponse.Rate rate = this.rateIntegration.getRate(getRateRequest).getRate();
     if (rate.getBuyAmount() == null || rate.getSellAmount() == null) {
       throw new ServerErrorException(
           String.format(
@@ -375,8 +388,8 @@ public Sep38QuoteResponse postQuote(Sep10Jwt token, Sep38PostQuoteRequest reques
             decimal(rate.getSellAmount(), pricePrecision),
             decimal(rate.getBuyAmount(), pricePrecision));
 
-    Sep38QuoteResponse.Sep38QuoteResponseBuilder builder =
-        Sep38QuoteResponse.builder()
+    Sep38QuoteResponse.Sep38QuoteResponseBuilder responseBuilder =
+        builder()
             .id(rate.getId())
             .expiresAt(rate.getExpiresAt())
             .price(rate.getPrice())
@@ -386,7 +399,8 @@ public Sep38QuoteResponse postQuote(Sep10Jwt token, Sep38PostQuoteRequest reques
 
     String sellAmount = formatAmount(decimal(rate.getSellAmount()), sellAsset.getDecimals());
     String buyAmount = formatAmount(decimal(rate.getBuyAmount()), buyAsset.getDecimals());
-    builder = builder.totalPrice(totalPrice).sellAmount(sellAmount).buyAmount(buyAmount);
+    responseBuilder =
+        responseBuilder.totalPrice(totalPrice).sellAmount(sellAmount).buyAmount(buyAmount);
 
     // SEP31: when buy_amount is specified (sell amount found from rate integration)
     if (context == SEP31 && isNotEmpty(buyAmount)) {
@@ -449,7 +463,7 @@ public Sep38QuoteResponse postQuote(Sep10Jwt token, Sep38PostQuoteRequest reques
 
     // increment counter
     sep38QuoteCreated.increment();
-    return builder.build();
+    return responseBuilder.build();
   }
 
   public Sep38QuoteResponse getQuote(Sep10Jwt token, String quoteId) throws AnchorException {
@@ -488,7 +502,7 @@ public Sep38QuoteResponse getQuote(Sep10Jwt token, String quoteId) throws Anchor
       throw new NotFoundException("quote not found");
     }
 
-    return Sep38QuoteResponse.builder()
+    return builder()
         .id(quote.getId())
         .expiresAt(quote.getExpiresAt())
         .totalPrice(quote.getTotalPrice())
@@ -507,4 +521,9 @@ private String getTotalPrice(BigDecimal bSellAmount, BigDecimal bBuyAmount) {
 
     return formatAmount(bTotalPrice, pricePrecision);
   }
+
+  private String getClientIdFromToken(Sep10Jwt token) {
+    if (token == null) return null;
+    return token.getAccount();
+  }
 }

core/src/test/kotlin/org/stellar/anchor/sep10/Sep10ServiceTest.kt

@@ -15,7 +15,6 @@ import okhttp3.mockwebserver.MockResponse
 import okhttp3.mockwebserver.MockWebServer
 import org.junit.jupiter.api.*
 import org.junit.jupiter.api.Assertions.assertEquals
-import org.junit.jupiter.api.Assertions.assertInstanceOf
 import org.junit.jupiter.api.MethodOrderer.OrderAnnotation
 import org.junit.jupiter.params.ParameterizedTest
 import org.junit.jupiter.params.provider.CsvSource
@@ -372,7 +371,7 @@ internal class Sep10ServiceTest {
   )
   fun `test create challenge ok`(clientAttributionRequired: String, clientDomain: String) {
     every { sep10Config.isClientAttributionRequired } returns clientAttributionRequired.toBoolean()
-    every { sep10Config.clientAttributionAllowList } returns listOf(TEST_CLIENT_DOMAIN)
+    every { sep10Config.allowedClientDomains } returns listOf(TEST_CLIENT_DOMAIN)
     val cr =
       ChallengeRequest.builder()
         .account(TEST_ACCOUNT)
@@ -605,8 +604,7 @@ internal class Sep10ServiceTest {
   }
 
   @Test
-  fun `test createChallenge() ok when knownCustodialAccountRequired is enabled`() {
-    every { sep10Config.isKnownCustodialAccountRequired } returns true
+  fun `test createChallenge() ok`() {
     every { sep10Config.knownCustodialAccountList } returns listOf(TEST_ACCOUNT)
     val cr =
       ChallengeRequest.builder()
@@ -617,13 +615,11 @@ internal class Sep10ServiceTest {
         .build()
 
     assertDoesNotThrow { sep10Service.createChallenge(cr) }
-    verify(exactly = 1) { sep10Config.isKnownCustodialAccountRequired }
     verify(exactly = 2) { sep10Config.knownCustodialAccountList }
   }
 
   @Test
   fun `Test createChallenge() when isKnownCustodialAccountRequired is not enabled`() {
-    every { sep10Config.isKnownCustodialAccountRequired } returns false
     every { sep10Config.knownCustodialAccountList } returns
       listOf("G321E23CFSUMSVQK4Y5JHPPYK73VYCNHZHA7ENKCV37P6SUEO6XQBKPP")
     val cr =
@@ -635,27 +631,6 @@ internal class Sep10ServiceTest {
         .build()
 
     assertDoesNotThrow { sep10Service.createChallenge(cr) }
-    verify(exactly = 1) { sep10Config.isKnownCustodialAccountRequired }
     verify(exactly = 2) { sep10Config.knownCustodialAccountList }
   }
-
-  @Test
-  fun `test createChallenge() failure when isRequireKnownOmnibusAccount is enabled and account mis-match`() {
-    every { sep10Config.isKnownCustodialAccountRequired } returns true
-    every { sep10Config.knownCustodialAccountList } returns
-      listOf("G321E23CFSUMSVQK4Y5JHPPYK73VYCNHZHA7ENKCV37P6SUEO6XQBKPP")
-    val cr =
-      ChallengeRequest.builder()
-        .account(TEST_ACCOUNT)
-        .memo(TEST_MEMO)
-        .homeDomain(TEST_HOME_DOMAIN)
-        .clientDomain(null)
-        .build()
-
-    val ex = assertThrows<SepException> { sep10Service.createChallenge(cr) }
-    verify(exactly = 1) { sep10Config.isKnownCustodialAccountRequired }
-    verify(exactly = 2) { sep10Config.knownCustodialAccountList }
-    assertInstanceOf(SepNotAuthorizedException::class.java, ex)
-    assertEquals("unable to process", ex.message)
-  }
 }