Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ANCNHOR-626] Update ClientsConfig with SigningKeys and Domains #1355

Merged
merged 7 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 2 additions & 15 deletions core/src/main/java/org/stellar/anchor/client/ClientFinder.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,12 @@ public String getClientName(String clientDomain, String account)
return client != null ? client.getName() : null;
}

// Check if the client domain and name are allowed
// Check if the client is allowed
if (client == null) {
throw new SepNotAuthorizedException("Client not found");
}

if (clientDomain != null && !isDomainAllowed(client.getDomain())) {
throw new SepNotAuthorizedException("Client domain not allowed");
}
if (!isClientNameAllowed(client.getName())) {
if (!sep10Config.getAllowedClientNames().contains(client.getName())) {
throw new SepNotAuthorizedException("Client name not allowed");
}

Expand All @@ -63,14 +60,4 @@ private ClientConfig getClient(String clientDomain, String account) {
ClientConfig clientByAccount = clientsConfig.getClientConfigBySigningKey(account);
return clientByDomain != null ? clientByDomain : clientByAccount;
}

private boolean isDomainAllowed(String domain) {
return sep10Config.getAllowedClientDomains().contains(domain)
|| sep10Config.getAllowedClientDomains().isEmpty();
}

private boolean isClientNameAllowed(String name) {
return sep10Config.getAllowedClientNames().contains(name)
|| sep10Config.getAllowedClientNames().isEmpty();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ public interface ClientsConfig {
class ClientConfig {
String name;
ClientType type;
String signingKey;
String domain;
@Deprecated String signingKey; // ANCHOR-696
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!!

Set<String> signingKeys;
@Deprecated String domain; // ANCHOR-696
Set<String> domains;
String callbackUrl;
boolean allowAnyDestination = false;
Set<String> destinationAccounts;
Expand Down
11 changes: 3 additions & 8 deletions core/src/main/java/org/stellar/anchor/sep31/Sep31Service.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,7 @@
import io.micrometer.core.instrument.Counter;
import java.math.BigDecimal;
import java.time.Instant;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.UUID;
import java.util.*;
import java.util.stream.Collectors;
import javax.transaction.Transactional;
import lombok.Data;
Expand Down Expand Up @@ -596,9 +591,9 @@ String getClientName(String account) throws BadRequestException {
if (sep10Config.isClientAttributionRequired() && client == null) {
throw new BadRequestException("Client not found");
}
if (client != null && !sep10Config.getAllowedClientDomains().contains(client.getDomain())) {

if (client != null && !sep10Config.getAllowedClientNames().contains(client.getName()))
client = null;
}
return client == null ? null : client.getName();
}

Expand Down
29 changes: 21 additions & 8 deletions core/src/test/kotlin/org/stellar/anchor/sep31/Sep31ServiceTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,10 @@ class Sep31ServiceTest {
ClientsConfig.ClientConfig(
"lobstr",
ClientsConfig.ClientType.NONCUSTODIAL,
"GBLGJA4TUN5XOGTV6WO2BWYUI2OZR5GYQ5PDPCRMQ5XEPJOYWB2X4CJO",
"lobstr.co",
null,
setOf("GBLGJA4TUN5XOGTV6WO2BWYUI2OZR5GYQ5PDPCRMQ5XEPJOYWB2X4CJO"),
null,
setOf("lobstr.co"),
"https://callback.lobstr.co/api/v2/anchor/callback",
false,
null
Expand All @@ -287,8 +289,18 @@ class Sep31ServiceTest {
return Stream.of(
Arguments.of(listOf<String>(), false, null, false),
Arguments.of(listOf<String>(), true, null, true),
Arguments.of(listOf(lobstrClientConfig.domain), false, lobstrClientConfig.name, false),
Arguments.of(listOf(lobstrClientConfig.domain), true, lobstrClientConfig.name, true),
Arguments.of(
listOf(lobstrClientConfig.domains.first()),
false,
lobstrClientConfig.name,
false
),
Arguments.of(
listOf(lobstrClientConfig.domains.first()),
true,
lobstrClientConfig.name,
true
),
)
}
}
Expand Down Expand Up @@ -789,7 +801,7 @@ class Sep31ServiceTest {
every { sep10Config.allowedClientDomains } returns listOf("vibrant.stellar.org")
every { clientsConfig.getClientConfigBySigningKey(any()) } returns
ClientsConfig.ClientConfig().apply {
domain = "vibrant.stellar.org"
domains = setOf("vibrant.stellar.org")
name = "vibrant"
}

Expand Down Expand Up @@ -1095,11 +1107,12 @@ class Sep31ServiceTest {
) {
every { sep10Config.allowedClientDomains } returns allowedClientDomains
every { sep10Config.isClientAttributionRequired } returns isClientAttributionRequired
every { clientsConfig.getClientConfigBySigningKey(lobstrClientConfig.signingKey) } returns
lobstrClientConfig
every {
clientsConfig.getClientConfigBySigningKey(lobstrClientConfig.signingKeys.first())
} returns lobstrClientConfig

// client name should be returned for valid input
val clientName = sep31Service.getClientName(lobstrClientConfig.signingKey)
val clientName = sep31Service.getClientName(lobstrClientConfig.signingKeys.first())
assertEquals(expectedClientName, clientName)

// exception maybe thrown for invalid input
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ class ClientFinderTest {
ClientsConfig.ClientConfig(
"name",
ClientsConfig.ClientType.CUSTODIAL,
"signing-key",
"domain",
null,
setOf("signing-key"),
null,
setOf("domain"),
"http://localhost:8000",
false,
emptySet()
Expand All @@ -41,7 +43,7 @@ class ClientFinderTest {
fun setup() {
MockKAnnotations.init(this, relaxUnitFun = true)
every { sep10Config.isClientAttributionRequired } returns true
every { sep10Config.allowedClientDomains } returns listOf(clientConfig.domain)
every { sep10Config.allowedClientDomains } returns clientConfig.domains.toList()
every { sep10Config.allowedClientNames } returns listOf(clientConfig.name)
every {
clientsConfig.getClientConfigByDomain(ExchangeAmountsCalculatorTest.token.clientDomain)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.stellar.anchor.platform.config;

import static io.jsonwebtoken.lang.Collections.setOf;
import static org.stellar.anchor.util.Log.debugF;
import static org.stellar.anchor.util.StringHelper.isEmpty;

Expand All @@ -9,13 +10,12 @@
import java.net.URL;
import java.util.List;
import java.util.Map;
import javax.annotation.PostConstruct;
import lombok.Data;
import org.jetbrains.annotations.NotNull;
import org.springframework.validation.Errors;
import org.springframework.validation.Validator;
import org.stellar.anchor.api.exception.SepException;
import org.stellar.anchor.config.ClientsConfig;
import org.stellar.anchor.sep10.Sep10Helper;

@Data
public class PropertyClientsConfig implements ClientsConfig, Validator {
Expand All @@ -24,14 +24,31 @@ public class PropertyClientsConfig implements ClientsConfig, Validator {
Map<String, String> domainToClientNameMap = null;
Map<String, String> signingKeyToClientNameMap = null;

@PostConstruct
public void postConstruct() {
// In favor of migrating to signingKeys and domains, copy signingKey to signingKeys and domain
// to domains if signingKeys and domains are not set.
clients.forEach(
JiahuiWho marked this conversation as resolved.
Show resolved Hide resolved
clientConfig -> {
if (!isEmpty(clientConfig.getSigningKey())) {
clientConfig.setSigningKeys(setOf(clientConfig.getSigningKey()));
}
if (!isEmpty(clientConfig.getDomain())) {
clientConfig.setDomains(setOf(clientConfig.getDomain()));
}
});
}

@Override
public ClientsConfig.ClientConfig getClientConfigBySigningKey(String signingKey) {
if (signingKeyToClientNameMap == null) {
signingKeyToClientNameMap = Maps.newHashMap();
clients.forEach(
clientConfig -> {
if (clientConfig.getSigningKey() != null) {
signingKeyToClientNameMap.put(clientConfig.getSigningKey(), clientConfig.getName());
if (clientConfig.getSigningKeys() != null && !clientConfig.getSigningKeys().isEmpty()) {
JiahuiWho marked this conversation as resolved.
Show resolved Hide resolved
for (String key : clientConfig.getSigningKeys()) {
signingKeyToClientNameMap.put(key, clientConfig.getName());
}
}
});
}
Expand All @@ -44,8 +61,10 @@ public ClientConfig getClientConfigByDomain(String domain) {
domainToClientNameMap = Maps.newHashMap();
clients.forEach(
clientConfig -> {
if (clientConfig.getDomain() != null) {
domainToClientNameMap.put(clientConfig.getDomain(), clientConfig.getName());
if (clientConfig.getDomains() != null && !clientConfig.getDomains().isEmpty()) {
JiahuiWho marked this conversation as resolved.
Show resolved Hide resolved
for (String domainName : clientConfig.getDomains()) {
domainToClientNameMap.put(domainName, clientConfig.getName());
}
}
});
}
Expand Down Expand Up @@ -84,9 +103,18 @@ private void validateClient(ClientConfig clientConfig, Errors errors) {
}

public void validateCustodialClient(ClientConfig clientConfig, Errors errors) {
if (isEmpty(clientConfig.getSigningKey())) {
if (isEmpty(clientConfig.getSigningKey())
JiahuiWho marked this conversation as resolved.
Show resolved Hide resolved
&& (clientConfig.getSigningKeys() == null || clientConfig.getSigningKeys().isEmpty())) {
errors.reject(
"empty-client-signing-key", "The client.signingKey cannot be empty and must be defined");
"empty-client-signing-keys",
"The client.signingKeys cannot be empty and must be defined");
}
if (!isEmpty(clientConfig.getSigningKey())
&& clientConfig.getSigningKeys() != null
&& !clientConfig.getSigningKeys().isEmpty()) {
errors.reject(
"client-signing-keys-conflict",
"The client.signingKey and The client.signingKeys cannot coexist, please choose one to use");
}
if (!isEmpty(clientConfig.getCallbackUrl())) {
try {
Expand All @@ -98,24 +126,17 @@ public void validateCustodialClient(ClientConfig clientConfig, Errors errors) {
}

public void validateNonCustodialClient(ClientConfig clientConfig, Errors errors) {
if (isEmpty(clientConfig.getDomain())) {
errors.reject("empty-client-domain", "The client.domain cannot be empty and must be defined");
if (isEmpty(clientConfig.getDomain())
&& (clientConfig.getDomains() == null || clientConfig.getDomains().isEmpty())) {
errors.reject(
JiahuiWho marked this conversation as resolved.
Show resolved Hide resolved
"empty-client-domains", "The client.domains cannot be empty and must be defined");
}

if (!isEmpty(clientConfig.getSigningKey())) {
try {
String clientSigningKey =
Sep10Helper.fetchSigningKeyFromClientDomain(clientConfig.getDomain(), false);
if (!clientConfig.getSigningKey().equals(clientSigningKey)) {
errors.reject(
"client-signing-key-does-not-match",
"The client.signingKey does not matched any valid registered keys");
}
} catch (SepException e) {
errors.reject(
"client-signing-key-toml-read-failure",
"SIGNING_KEY not present in 'client_domain' TOML or TOML file does not exist");
}
if (!isEmpty(clientConfig.getDomain())
&& clientConfig.getDomains() != null
&& !clientConfig.getDomains().isEmpty()) {
errors.reject(
"client-domains-conflict",
"The client.domain and the client.domains cannot coexist, please choose one to use");
}

if (!isEmpty(clientConfig.getCallbackUrl())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,8 @@ public PropertySep10Config(
this.secretConfig = secretConfig;
this.knownCustodialAccountList =
clientsConfig.getClients().stream()
.filter(
cfg -> cfg.getType() == CUSTODIAL && StringHelper.isNotEmpty(cfg.getSigningKey()))
.map(ClientConfig::getSigningKey)
.filter(cfg -> cfg.getType() == CUSTODIAL && !cfg.getSigningKeys().isEmpty())
.flatMap(cfg -> cfg.getSigningKeys().stream())
.collect(Collectors.toList());
}

Expand Down Expand Up @@ -170,7 +169,7 @@ void validateClientAttribution(Errors errors) {
if (clientAttributionRequired) {
List<String> nonCustodialClientNames =
clientsConfig.clients.stream()
.filter(cfg -> cfg.getType() == NONCUSTODIAL && isNotEmpty(cfg.getDomain()))
.filter(cfg -> cfg.getType() == NONCUSTODIAL)
.map(ClientConfig::getName)
.collect(Collectors.toList());

Expand Down Expand Up @@ -230,18 +229,15 @@ public List<String> getAllowedClientDomains() {
// if clientAllowList is not defined, all client domains from the clients section are allowed.
if (clientAllowList == null || clientAllowList.isEmpty()) {
return clientsConfig.clients.stream()
.map(ClientConfig::getDomain)
.filter(StringHelper::isNotEmpty)
.filter(cfg -> cfg.getDomains() != null && !cfg.getDomains().isEmpty())
.flatMap(cfg -> cfg.getDomains().stream())
.collect(Collectors.toList());
}

// If clientAllowList is defined, only the clients in the allow list are allowed.
return clientAllowList.stream()
.map(
domain ->
(clientsConfig.getClientConfigByName(domain) == null)
? null
: clientsConfig.getClientConfigByName(domain).getDomain())
.filter(domain -> clientsConfig.getClientConfigByName(domain) != null)
.flatMap(domain -> clientsConfig.getClientConfigByName(domain).getDomains().stream())
.filter(StringHelper::isNotEmpty)
.collect(Collectors.toList());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,13 @@ public void start() {
switch (clientConfig.getType()) {
case CUSTODIAL:
processorName =
CLIENT_STATUS_CALLBACK_EVENT_PROCESSOR_NAME_PREFIX + clientConfig.getSigningKey();
CLIENT_STATUS_CALLBACK_EVENT_PROCESSOR_NAME_PREFIX
+ clientConfig.getSigningKeys().stream();
break;
case NONCUSTODIAL:
processorName =
CLIENT_STATUS_CALLBACK_EVENT_PROCESSOR_NAME_PREFIX + clientConfig.getDomain();
CLIENT_STATUS_CALLBACK_EVENT_PROCESSOR_NAME_PREFIX
+ clientConfig.getDomains().stream();
break;
default:
errorF("Unknown client type: {}", clientConfig.getType());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,48 +180,41 @@ event_processor:
# - type: (required) `custodial` or `noncustodial`
#
# If the type is `custodial`,
# - signing_key: (required) the custodial SEP-10 signing key of the client. If the signing key of the client
# is the same as the `signing_key`, the callback will be activated.
# - signing_keys: (required) the custodial SEP-10 signing keys of the client.
# - callback_url: (optional) the URL of the client's callback API endpoint. Optional due to some wallets may opt to
# poll instead, or may use polling first before implementing callbacks at a later stage.
# - allow_any_destination: (optional) default to false. If set to true, allows any destination for deposits.
# - destination_accounts: (optional) list of accounts allowed to be used for the deposit. By default, only SEP-10
# authenticated account can be used to deposit funds into. If allows_any_destinations set to true,
# this configuration option is ignored.
# If the type is `noncustodial`,
# - domain: (required) the domain of the client. If the `client_domain` field of the transaction is the same as this
# domain, the callback will be activated.
# - domains: (required) the domain of the client.
# - callback_url: (optional) the URL of the client's callback API endpoint
# - signing_key: (optional) the signing key of the client. If the public key is specified, the client domain's TOML
# file will be fetched and the `SIGNING_KEY` of the TOML file will be compared with the public key.
# If they don't match, a validation error will be thrown.
#
# Examples:
# - name: circle
# type: custodial
# signing_key: GBI2IWJGR4UQPBIKPP6WG76X5PHSD2QTEBGIP6AZ3ZXWV46ZUSGNEGN2
# signing_keys: GBI2IWJGR4UQPBIKPP6WG76X5PHSD2QTEBGIP6AZ3ZXWV46ZUSGNEGN2,GACYKME36AI6UYAV7A5ZUA6MG4C4K2VAPNYMW5YLOM6E7GS6FSHDPV4F
# callback_url: https://callback.circle.com/api/v1/anchor/callback
# - name: lobstr
# type: noncustodial
# domain: lobstr.co
# domains: lobstr.co,lobstr.com
# callback_url: https://callback.lobstr.co/api/v2/anchor/callback
# - name: vibrant
# type: noncustodial
# domain: vibrant.co
# callback_url: https://callback.vibrant.com/api/v2/anchor/callback
# signing_key: GA22WORKYRXB6AW7XR5GIOAOQUY4KKCENEAI34FN3KJNWHKDZTZSVLTU
clients:
# - name:
# type: custodial
# signing_key:
# signing_keys:
# callback_url:
# allow_any_destination:
# destination_accounts:
# - name:
# type: noncustodial
# domain:
# domains:
# callback_url:
# signing_key:

######################
## Platform Server Configuration
Expand Down
Loading
Loading