From 6551183a8df8e25badbf7fc4a077674f7b999a49 Mon Sep 17 00:00:00 2001 From: Lucas Saldanha Date: Tue, 12 Feb 2019 17:27:50 +1300 Subject: [PATCH] [NC-2118] Method to reload permissions file (#834) * Extracting EnodeURL logic to specific object * Moving permissioning config builder to permissioning package * Validating accounts in permissions file * Implemented controller reload method * Reload whitelist from file API method * Spotless * Refactoring account validation * Errorprone * Fixing tests after rebase * Spotless * PR review Signed-off-by: Adrian Sutton --- .../jsonrpc/JsonRpcMethodsFactory.java | 15 +- .../PermReloadPermissionsFromFile.java | 57 +++++ .../internal/response/JsonRpcError.java | 4 + .../PermReloadPermissionsFromFileTest.java | 94 +++++++ .../pantheon/ethereum/p2p/NoopP2PNetwork.java | 2 +- .../NodeWhitelistController.java | 35 +++ .../NodeWhitelistControllerTest.java | 56 +++++ ethereum/permissioning/build.gradle | 2 + .../AccountWhitelistController.java | 38 ++- .../PermissioningConfigurationBuilder.java | 16 +- .../permissioning}/TomlConfigFileParser.java | 2 +- .../AccountWhitelistControllerTest.java | 46 ++++ ...PermissioningConfigurationBuilderTest.java | 118 ++++----- .../test/resources/permissioning_config.toml | 4 + ...ermissioning_config_absent_whitelists.toml | 0 ...sioning_config_account_whitelist_only.toml | 0 ...permissioning_config_empty_whitelists.toml | 0 .../permissioning_config_invalid_account.toml | 3 + .../permissioning_config_invalid_enode.toml | 0 ...missioning_config_node_whitelist_only.toml | 0 ..._config_node_whitelist_only_multiline.toml | 0 ...permissioning_config_unrecognized_key.toml | 0 .../pegasys/pantheon/cli/PantheonCommand.java | 2 +- .../custom/EnodeToURIPropertyConverter.java | 98 +------- .../EnodeToURIPropertyConverterTest.java | 206 +--------------- ...rmissioningConfigurationValidatorTest.java | 2 +- .../pegasys/pantheon/util/enode/EnodeURL.java | 165 +++++++++++++ .../pantheon/util/enode/EnodeURLTest.java | 230 ++++++++++++++++++ 28 files changed, 830 insertions(+), 365 deletions(-) create mode 100644 ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermReloadPermissionsFromFile.java create mode 100644 ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermReloadPermissionsFromFileTest.java rename {pantheon/src/main/java/tech/pegasys/pantheon => ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning}/PermissioningConfigurationBuilder.java (88%) rename {pantheon/src/main/java/tech/pegasys/pantheon => ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning}/TomlConfigFileParser.java (98%) rename {pantheon/src/test/java/tech/pegasys/pantheon => ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning}/PermissioningConfigurationBuilderTest.java (70%) create mode 100644 ethereum/permissioning/src/test/resources/permissioning_config.toml rename {pantheon => ethereum/permissioning}/src/test/resources/permissioning_config_absent_whitelists.toml (100%) rename {pantheon => ethereum/permissioning}/src/test/resources/permissioning_config_account_whitelist_only.toml (100%) rename {pantheon => ethereum/permissioning}/src/test/resources/permissioning_config_empty_whitelists.toml (100%) create mode 100644 ethereum/permissioning/src/test/resources/permissioning_config_invalid_account.toml rename {pantheon => ethereum/permissioning}/src/test/resources/permissioning_config_invalid_enode.toml (100%) rename {pantheon => ethereum/permissioning}/src/test/resources/permissioning_config_node_whitelist_only.toml (100%) rename {pantheon => ethereum/permissioning}/src/test/resources/permissioning_config_node_whitelist_only_multiline.toml (100%) rename {pantheon => ethereum/permissioning}/src/test/resources/permissioning_config_unrecognized_key.toml (100%) create mode 100644 util/src/main/java/tech/pegasys/pantheon/util/enode/EnodeURL.java create mode 100644 util/src/test/java/tech/pegasys/pantheon/util/enode/EnodeURLTest.java diff --git a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/JsonRpcMethodsFactory.java b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/JsonRpcMethodsFactory.java index f5cf8e9993..f371c52660 100644 --- a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/JsonRpcMethodsFactory.java +++ b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/JsonRpcMethodsFactory.java @@ -72,6 +72,7 @@ import tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.permissioning.Per import tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.permissioning.PermAddNodesToWhitelist; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.permissioning.PermGetAccountsWhitelist; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.permissioning.PermGetNodesWhitelist; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.permissioning.PermReloadPermissionsFromFile; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.permissioning.PermRemoveAccountsFromWhitelist; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.permissioning.PermRemoveNodesFromWhitelist; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.privacy.EeaSendRawTransaction; @@ -240,19 +241,17 @@ public class JsonRpcMethodsFactory { enabledMethods, new PermAddNodesToWhitelist(p2pNetwork, parameter), new PermRemoveNodesFromWhitelist(p2pNetwork, parameter), - new PermGetNodesWhitelist(p2pNetwork)); + new PermGetNodesWhitelist(p2pNetwork), + new PermGetAccountsWhitelist(accountsWhitelistController), + new PermAddAccountsToWhitelist(accountsWhitelistController, parameter), + new PermRemoveAccountsFromWhitelist(accountsWhitelistController, parameter), + new PermReloadPermissionsFromFile( + accountsWhitelistController, p2pNetwork.getNodeWhitelistController())); } if (rpcApis.contains(RpcApis.ADMIN)) { addMethods(enabledMethods, new AdminPeers(p2pNetwork)); addMethods(enabledMethods, new AdminAddPeer(p2pNetwork, parameter)); } - if (rpcApis.contains(RpcApis.PERM)) { - addMethods( - enabledMethods, - new PermGetAccountsWhitelist(accountsWhitelistController), - new PermAddAccountsToWhitelist(accountsWhitelistController, parameter), - new PermRemoveAccountsFromWhitelist(accountsWhitelistController, parameter)); - } if (rpcApis.contains(RpcApis.EEA)) { addMethods( enabledMethods, diff --git a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermReloadPermissionsFromFile.java b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermReloadPermissionsFromFile.java new file mode 100644 index 0000000000..a7d1a0d3a7 --- /dev/null +++ b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermReloadPermissionsFromFile.java @@ -0,0 +1,57 @@ +/* + * Copyright 2018 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.permissioning; + +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.JsonRpcRequest; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.JsonRpcMethod; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcError; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcErrorResponse; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcResponse; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcSuccessResponse; +import tech.pegasys.pantheon.ethereum.p2p.permissioning.NodeWhitelistController; +import tech.pegasys.pantheon.ethereum.permissioning.AccountWhitelistController; + +import java.util.Optional; + +public class PermReloadPermissionsFromFile implements JsonRpcMethod { + + private final Optional accountWhitelistController; + private final Optional nodesWhitelistController; + + public PermReloadPermissionsFromFile( + final Optional accountWhitelistController, + final Optional nodesWhitelistController) { + this.accountWhitelistController = accountWhitelistController; + this.nodesWhitelistController = nodesWhitelistController; + } + + @Override + public String getName() { + return "perm_reloadPermissionsFromFile"; + } + + @Override + public JsonRpcResponse response(final JsonRpcRequest request) { + if (!accountWhitelistController.isPresent() && !nodesWhitelistController.isPresent()) { + return new JsonRpcErrorResponse(request.getId(), JsonRpcError.PERMISSIONING_NOT_ENABLED); + } + + try { + accountWhitelistController.ifPresent(AccountWhitelistController::reload); + nodesWhitelistController.ifPresent(NodeWhitelistController::reload); + return new JsonRpcSuccessResponse(request.getId()); + } catch (Exception e) { + return new JsonRpcErrorResponse(request.getId(), JsonRpcError.WHITELIST_RELOAD_ERROR); + } + } +} diff --git a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/response/JsonRpcError.java b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/response/JsonRpcError.java index 19e234f281..de141e9dbc 100644 --- a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/response/JsonRpcError.java +++ b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/response/JsonRpcError.java @@ -71,6 +71,10 @@ public enum JsonRpcError { WHITELIST_FILE_SYNC( -32000, "The permissioning whitelist configuration file is out of sync. The changes have been applied, but not persisted to disk"), + WHITELIST_RELOAD_ERROR( + -32000, + "Error reloading permissions file. Please use perm_getAccountsWhitelist and perm_getNodesWhitelist to review the current state of the whitelists."), + PERMISSIONING_NOT_ENABLED(-32000, "Node/Account whitelisting has not been enabled"), // Private transaction errors ENCLAVE_IS_DOWN(-32000, "Enclave is down"); diff --git a/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermReloadPermissionsFromFileTest.java b/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermReloadPermissionsFromFileTest.java new file mode 100644 index 0000000000..1a53c25d6a --- /dev/null +++ b/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermReloadPermissionsFromFileTest.java @@ -0,0 +1,94 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.permissioning; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.verify; + +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.JsonRpcRequest; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcError; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcErrorResponse; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcResponse; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcSuccessResponse; +import tech.pegasys.pantheon.ethereum.p2p.permissioning.NodeWhitelistController; +import tech.pegasys.pantheon.ethereum.permissioning.AccountWhitelistController; + +import java.util.Optional; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class PermReloadPermissionsFromFileTest { + + @Mock private AccountWhitelistController accountWhitelistController; + @Mock private NodeWhitelistController nodeWhitelistController; + private PermReloadPermissionsFromFile method; + + @Before + public void before() { + method = + new PermReloadPermissionsFromFile( + Optional.of(accountWhitelistController), Optional.of(nodeWhitelistController)); + } + + @Test + public void getNameShouldReturnExpectedName() { + assertThat(method.getName()).isEqualTo("perm_reloadPermissionsFromFile"); + } + + @Test + public void whenBothControllersAreNotPresentMethodShouldReturnPermissioningDisabled() { + JsonRpcResponse expectedErrorResponse = + new JsonRpcErrorResponse(null, JsonRpcError.PERMISSIONING_NOT_ENABLED); + + method = new PermReloadPermissionsFromFile(Optional.empty(), Optional.empty()); + + JsonRpcResponse response = method.response(reloadRequest()); + + assertThat(response).isEqualToComparingFieldByField(expectedErrorResponse); + } + + @Test + public void whenControllersReloadSucceedsMethodShouldReturnSuccess() { + JsonRpcResponse response = method.response(reloadRequest()); + + verify(accountWhitelistController).reload(); + verify(nodeWhitelistController).reload(); + + assertThat(response).isEqualToComparingFieldByField(successResponse()); + } + + @Test + public void whenControllerReloadFailsMethodShouldReturnError() { + doThrow(new RuntimeException()).when(accountWhitelistController).reload(); + JsonRpcResponse expectedErrorResponse = + new JsonRpcErrorResponse(null, JsonRpcError.WHITELIST_RELOAD_ERROR); + + JsonRpcResponse response = method.response(reloadRequest()); + + assertThat(response).isEqualToComparingFieldByField(expectedErrorResponse); + } + + private JsonRpcSuccessResponse successResponse() { + return new JsonRpcSuccessResponse(null); + } + + private JsonRpcRequest reloadRequest() { + return new JsonRpcRequest("2.0", "perm_reloadPermissionsFromFile", null); + } +} diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/NoopP2PNetwork.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/NoopP2PNetwork.java index 13883b3b4c..1aaaf4a682 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/NoopP2PNetwork.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/NoopP2PNetwork.java @@ -85,7 +85,7 @@ public class NoopP2PNetwork implements P2PNetwork { @Override public Optional getNodeWhitelistController() { - throw new P2pDisabledException("P2P networking disabled. Node whitelist unavailable."); + return Optional.empty(); } @Override diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistController.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistController.java index 7d8c74995e..6dbef2c6b8 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistController.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistController.java @@ -15,6 +15,7 @@ package tech.pegasys.pantheon.ethereum.p2p.permissioning; import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer; import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfiguration; +import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfigurationBuilder; import tech.pegasys.pantheon.ethereum.permissioning.WhitelistFileSyncException; import tech.pegasys.pantheon.ethereum.permissioning.WhitelistOperationResult; import tech.pegasys.pantheon.ethereum.permissioning.WhitelistPersistor; @@ -29,9 +30,14 @@ import java.util.Optional; import java.util.stream.Collectors; import com.google.common.annotations.VisibleForTesting; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; public class NodeWhitelistController { + private static final Logger LOG = LogManager.getLogger(); + + private PermissioningConfiguration configuration; private List nodesWhitelist = new ArrayList<>(); private final WhitelistPersistor whitelistPersistor; @@ -43,7 +49,12 @@ public class NodeWhitelistController { public NodeWhitelistController( final PermissioningConfiguration configuration, final WhitelistPersistor whitelistPersistor) { + this.configuration = configuration; this.whitelistPersistor = whitelistPersistor; + readNodesFromConfig(configuration); + } + + private void readNodesFromConfig(final PermissioningConfiguration configuration) { if (configuration.isNodeWhitelistEnabled() && configuration.getNodeWhitelist() != null) { for (URI uri : configuration.getNodeWhitelist()) { nodesWhitelist.add(DefaultPeer.fromURI(uri)); @@ -180,6 +191,30 @@ public class NodeWhitelistController { return nodesWhitelist; } + public synchronized void reload() throws RuntimeException { + final List currentAccountsList = new ArrayList<>(nodesWhitelist); + nodesWhitelist.clear(); + + try { + final PermissioningConfiguration updatedConfig = + PermissioningConfigurationBuilder.permissioningConfigurationFromToml( + configuration.getConfigurationFilePath(), + configuration.isNodeWhitelistEnabled(), + configuration.isAccountWhitelistEnabled()); + + readNodesFromConfig(updatedConfig); + configuration = updatedConfig; + } catch (Exception e) { + LOG.warn( + "Error reloading permissions file. In-memory whitelisted nodes will be reverted to previous valid configuration. " + + "Details: {}", + e.getMessage()); + nodesWhitelist.clear(); + nodesWhitelist.addAll(currentAccountsList); + throw new RuntimeException(e); + } + } + public static class NodesWhitelistResult { private final WhitelistOperationResult result; private final Optional message; diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistControllerTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistControllerTest.java index 962c3fea2d..28301cbe2c 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistControllerTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/permissioning/NodeWhitelistControllerTest.java @@ -14,11 +14,14 @@ package tech.pegasys.pantheon.ethereum.p2p.permissioning; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowable; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; import static tech.pegasys.pantheon.ethereum.p2p.permissioning.NodeWhitelistController.NodesWhitelistResult; import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer; @@ -30,6 +33,10 @@ import tech.pegasys.pantheon.ethereum.permissioning.WhitelistPersistor; import tech.pegasys.pantheon.util.bytes.BytesValue; import java.io.IOException; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -265,4 +272,53 @@ public class NodeWhitelistControllerTest { verify(whitelistPersistor, times(2)).updateConfig(any(), any()); verifyNoMoreInteractions(whitelistPersistor); } + + @Test + public void reloadNodeWhitelistWithValidConfigFileShouldUpdateWhitelist() throws Exception { + final String expectedEnodeURL = + "enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.9:4567"; + final Path permissionsFile = createPermissionsFileWithNode(expectedEnodeURL); + final PermissioningConfiguration permissioningConfig = mock(PermissioningConfiguration.class); + + when(permissioningConfig.getConfigurationFilePath()) + .thenReturn(permissionsFile.toAbsolutePath().toString()); + when(permissioningConfig.isNodeWhitelistEnabled()).thenReturn(true); + when(permissioningConfig.getNodeWhitelist()) + .thenReturn(Arrays.asList(URI.create(expectedEnodeURL))); + controller = new NodeWhitelistController(permissioningConfig); + + controller.reload(); + + assertThat(controller.getNodesWhitelist()) + .containsExactly(DefaultPeer.fromURI(expectedEnodeURL)); + } + + @Test + public void reloadNodeWhitelistWithErrorReadingConfigFileShouldKeepOldWhitelist() { + final String expectedEnodeURI = + "enode://aaaa80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.9:4567"; + final PermissioningConfiguration permissioningConfig = mock(PermissioningConfiguration.class); + + when(permissioningConfig.getConfigurationFilePath()).thenReturn("foo"); + when(permissioningConfig.isNodeWhitelistEnabled()).thenReturn(true); + when(permissioningConfig.getNodeWhitelist()) + .thenReturn(Arrays.asList(URI.create(expectedEnodeURI))); + controller = new NodeWhitelistController(permissioningConfig); + + final Throwable thrown = catchThrowable(() -> controller.reload()); + + assertThat(thrown) + .isInstanceOf(RuntimeException.class) + .hasMessageContaining("Unable to read permissions TOML config file"); + + assertThat(controller.getNodesWhitelist()) + .containsExactly(DefaultPeer.fromURI(expectedEnodeURI)); + } + + private Path createPermissionsFileWithNode(final String node) throws IOException { + final String nodePermissionsFileContent = "nodes-whitelist=[\"" + node + "\"]"; + final Path permissionsFile = Files.createTempFile("node_permissions", ""); + Files.write(permissionsFile, nodePermissionsFileContent.getBytes(StandardCharsets.UTF_8)); + return permissionsFile; + } } diff --git a/ethereum/permissioning/build.gradle b/ethereum/permissioning/build.gradle index 620f3bfb7d..ae3cf8e423 100644 --- a/ethereum/permissioning/build.gradle +++ b/ethereum/permissioning/build.gradle @@ -27,8 +27,10 @@ jar { dependencies { implementation project(':util') + implementation 'com.google.guava:guava' implementation 'net.consensys.cava:cava-toml' + implementation 'org.apache.logging.log4j:log4j-api' testImplementation 'junit:junit' testImplementation 'org.assertj:assertj-core' diff --git a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/AccountWhitelistController.java b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/AccountWhitelistController.java index 2516922e52..19fd54cbc2 100644 --- a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/AccountWhitelistController.java +++ b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/AccountWhitelistController.java @@ -20,9 +20,15 @@ import java.util.Collection; import java.util.HashSet; import java.util.List; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + public class AccountWhitelistController { + private static final Logger LOG = LogManager.getLogger(); + private static final int ACCOUNT_BYTES_SIZE = 20; + private PermissioningConfiguration configuration; private List accountWhitelist = new ArrayList<>(); private final WhitelistPersistor whitelistPersistor; @@ -32,7 +38,12 @@ public class AccountWhitelistController { public AccountWhitelistController( final PermissioningConfiguration configuration, final WhitelistPersistor whitelistPersistor) { + this.configuration = configuration; this.whitelistPersistor = whitelistPersistor; + readAccountsFromConfig(configuration); + } + + private void readAccountsFromConfig(final PermissioningConfiguration configuration) { if (configuration != null && configuration.isAccountWhitelistEnabled()) { if (!configuration.getAccountWhitelist().isEmpty()) { addAccounts(configuration.getAccountWhitelist()); @@ -135,10 +146,10 @@ public class AccountWhitelistController { } private boolean containsInvalidAccount(final List accounts) { - return !accounts.stream().allMatch(this::isValidAccountString); + return !accounts.stream().allMatch(AccountWhitelistController::isValidAccountString); } - private boolean isValidAccountString(final String account) { + static boolean isValidAccountString(final String account) { try { BytesValue bytesValue = BytesValue.fromHexString(account); return bytesValue.size() == ACCOUNT_BYTES_SIZE; @@ -146,4 +157,27 @@ public class AccountWhitelistController { return false; } } + + public synchronized void reload() throws RuntimeException { + final ArrayList currentAccountsList = new ArrayList<>(accountWhitelist); + accountWhitelist.clear(); + + try { + final PermissioningConfiguration updatedConfig = + PermissioningConfigurationBuilder.permissioningConfigurationFromToml( + configuration.getConfigurationFilePath(), + configuration.isNodeWhitelistEnabled(), + configuration.isAccountWhitelistEnabled()); + readAccountsFromConfig(updatedConfig); + configuration = updatedConfig; + } catch (Exception e) { + LOG.warn( + "Error reloading permissions file. In-memory whitelisted accounts will be reverted to previous valid configuration. " + + "Details: {}", + e.getMessage()); + accountWhitelist.clear(); + accountWhitelist.addAll(currentAccountsList); + throw new RuntimeException(e); + } + } } diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/PermissioningConfigurationBuilder.java b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/PermissioningConfigurationBuilder.java similarity index 88% rename from pantheon/src/main/java/tech/pegasys/pantheon/PermissioningConfigurationBuilder.java rename to ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/PermissioningConfigurationBuilder.java index 22e41ff2f8..f01bed504d 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/PermissioningConfigurationBuilder.java +++ b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/PermissioningConfigurationBuilder.java @@ -10,10 +10,9 @@ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the * specific language governing permissions and limitations under the License. */ -package tech.pegasys.pantheon; +package tech.pegasys.pantheon.ethereum.permissioning; -import tech.pegasys.pantheon.cli.custom.EnodeToURIPropertyConverter; -import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfiguration; +import tech.pegasys.pantheon.util.enode.EnodeURL; import java.net.URI; import java.util.List; @@ -67,6 +66,15 @@ public class PermissioningConfigurationBuilder { .parallelStream() .map(Object::toString) .collect(Collectors.toList()); + + accountsWhitelistToml.stream() + .filter(s -> !AccountWhitelistController.isValidAccountString(s)) + .findFirst() + .ifPresent( + s -> { + throw new IllegalArgumentException("Invalid account " + s); + }); + permissioningConfiguration.setAccountWhitelist(accountsWhitelistToml); } else { throw new Exception( @@ -81,7 +89,7 @@ public class PermissioningConfigurationBuilder { .toList() .parallelStream() .map(Object::toString) - .map(EnodeToURIPropertyConverter::convertToURI) + .map(EnodeURL::asURI) .collect(Collectors.toList()); permissioningConfiguration.setNodeWhitelist(nodesWhitelistToml); } else { diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/TomlConfigFileParser.java b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/TomlConfigFileParser.java similarity index 98% rename from pantheon/src/main/java/tech/pegasys/pantheon/TomlConfigFileParser.java rename to ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/TomlConfigFileParser.java index 338be03e57..aee91f3dfb 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/TomlConfigFileParser.java +++ b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/TomlConfigFileParser.java @@ -10,7 +10,7 @@ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the * specific language governing permissions and limitations under the License. */ -package tech.pegasys.pantheon; +package tech.pegasys.pantheon.ethereum.permissioning; import static java.nio.charset.StandardCharsets.UTF_8; diff --git a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/AccountWhitelistControllerTest.java b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/AccountWhitelistControllerTest.java index 820e52d749..74f9a75d6a 100644 --- a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/AccountWhitelistControllerTest.java +++ b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/AccountWhitelistControllerTest.java @@ -14,6 +14,7 @@ package tech.pegasys.pantheon.ethereum.permissioning; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowable; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.times; @@ -22,6 +23,9 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -178,4 +182,46 @@ public class AccountWhitelistControllerTest { verify(whitelistPersistor, times(2)).updateConfig(any(), any()); verifyNoMoreInteractions(whitelistPersistor); } + + @Test + public void reloadAccountWhitelistWithValidConfigFileShouldUpdateWhitelist() throws Exception { + final String expectedAccount = "0x627306090abab3a6e1400e9345bc60c78a8bef57"; + final Path permissionsFile = createPermissionsFileWithAccount(expectedAccount); + + when(permissioningConfig.getConfigurationFilePath()) + .thenReturn(permissionsFile.toAbsolutePath().toString()); + when(permissioningConfig.isAccountWhitelistEnabled()).thenReturn(true); + when(permissioningConfig.getAccountWhitelist()) + .thenReturn(Arrays.asList("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73")); + controller = new AccountWhitelistController(permissioningConfig, whitelistPersistor); + + controller.reload(); + + assertThat(controller.getAccountWhitelist()).containsExactly(expectedAccount); + } + + @Test + public void reloadAccountWhitelistWithErrorReadingConfigFileShouldKeepOldWhitelist() { + when(permissioningConfig.getConfigurationFilePath()).thenReturn("foo"); + when(permissioningConfig.isAccountWhitelistEnabled()).thenReturn(true); + when(permissioningConfig.getAccountWhitelist()) + .thenReturn(Arrays.asList("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73")); + controller = new AccountWhitelistController(permissioningConfig, whitelistPersistor); + + final Throwable thrown = catchThrowable(() -> controller.reload()); + + assertThat(thrown) + .isInstanceOf(RuntimeException.class) + .hasMessageContaining("Unable to read permissions TOML config file"); + + assertThat(controller.getAccountWhitelist()) + .containsExactly("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73"); + } + + private Path createPermissionsFileWithAccount(final String account) throws IOException { + final String nodePermissionsFileContent = "accounts-whitelist=[\"" + account + "\"]"; + final Path permissionsFile = Files.createTempFile("account_permissions", ""); + Files.write(permissionsFile, nodePermissionsFileContent.getBytes(StandardCharsets.UTF_8)); + return permissionsFile; + } } diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/PermissioningConfigurationBuilderTest.java b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/PermissioningConfigurationBuilderTest.java similarity index 70% rename from pantheon/src/test/java/tech/pegasys/pantheon/PermissioningConfigurationBuilderTest.java rename to ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/PermissioningConfigurationBuilderTest.java index 9baef1ed08..cab95ffe5b 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/PermissioningConfigurationBuilderTest.java +++ b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/PermissioningConfigurationBuilderTest.java @@ -10,37 +10,38 @@ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the * specific language governing permissions and limitations under the License. */ -package tech.pegasys.pantheon; +package tech.pegasys.pantheon.ethereum.permissioning; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.AssertionsForClassTypes.fail; - -import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfiguration; +import static org.assertj.core.api.Assertions.catchThrowable; import java.net.URI; import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import com.google.common.io.Resources; import org.junit.Test; public class PermissioningConfigurationBuilderTest { - static final String PERMISSIONING_CONFIG_VALID = "permissioning_config.toml"; - static final String PERMISSIONING_CONFIG_ACCOUNT_WHITELIST_ONLY = + private static final String PERMISSIONING_CONFIG_VALID = "permissioning_config.toml"; + private static final String PERMISSIONING_CONFIG_ACCOUNT_WHITELIST_ONLY = "permissioning_config_account_whitelist_only.toml"; - static final String PERMISSIONING_CONFIG_NODE_WHITELIST_ONLY = + private static final String PERMISSIONING_CONFIG_NODE_WHITELIST_ONLY = "permissioning_config_node_whitelist_only.toml"; - static final String PERMISSIONING_CONFIG_INVALID_ENODE = + private static final String PERMISSIONING_CONFIG_INVALID_ENODE = "permissioning_config_invalid_enode.toml"; - static final String PERMISSIONING_CONFIG_EMPTY_WHITELISTS = + private static final String PERMISSIONING_CONFIG_INVALID_ACCOUNT = + "permissioning_config_invalid_account.toml"; + private static final String PERMISSIONING_CONFIG_EMPTY_WHITELISTS = "permissioning_config_empty_whitelists.toml"; - static final String PERMISSIONING_CONFIG_ABSENT_WHITELISTS = + private static final String PERMISSIONING_CONFIG_ABSENT_WHITELISTS = "permissioning_config_absent_whitelists.toml"; - static final String PERMISSIONING_CONFIG_UNRECOGNIZED_KEY = + private static final String PERMISSIONING_CONFIG_UNRECOGNIZED_KEY = "permissioning_config_unrecognized_key.toml"; - static final String PERMISSIONING_CONFIG_NODE_WHITELIST_ONLY_MULTILINE = + private static final String PERMISSIONING_CONFIG_NODE_WHITELIST_ONLY_MULTILINE = "permissioning_config_node_whitelist_only_multiline.toml"; private final String VALID_NODE_ID = @@ -48,7 +49,6 @@ public class PermissioningConfigurationBuilderTest { @Test public void permissioningConfig() throws Exception { - final String uri = "enode://" + VALID_NODE_ID + "@192.168.0.9:4567"; final String uri2 = "enode://" + VALID_NODE_ID + "@192.169.0.9:4568"; @@ -56,9 +56,7 @@ public class PermissioningConfigurationBuilderTest { final Path toml = Files.createTempFile("toml", ""); Files.write(toml, Resources.toByteArray(configFile)); - PermissioningConfiguration permissioningConfiguration = - PermissioningConfigurationBuilder.permissioningConfiguration( - toml.toAbsolutePath().toString(), true, true); + PermissioningConfiguration permissioningConfiguration = permissioningConfig(toml); assertThat(permissioningConfiguration.isAccountWhitelistEnabled()).isTrue(); assertThat(permissioningConfiguration.getAccountWhitelist()) @@ -70,7 +68,6 @@ public class PermissioningConfigurationBuilderTest { @Test public void permissioningConfigWithOnlyNodeWhitelistSet() throws Exception { - final String uri = "enode://" + VALID_NODE_ID + "@192.168.0.9:4567"; final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_NODE_WHITELIST_ONLY); @@ -88,7 +85,6 @@ public class PermissioningConfigurationBuilderTest { @Test public void permissioningConfigWithOnlyAccountWhitelistSet() throws Exception { - final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_ACCOUNT_WHITELIST_ONLY); final Path toml = Files.createTempFile("toml", ""); Files.write(toml, Resources.toByteArray(configFile)); @@ -104,31 +100,38 @@ public class PermissioningConfigurationBuilderTest { } @Test - public void permissioningConfigWithInvalidEnode() throws Exception { + public void permissioningConfigWithInvalidAccount() throws Exception { + final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_INVALID_ACCOUNT); + final Path toml = Files.createTempFile("toml", ""); + Files.write(toml, Resources.toByteArray(configFile)); + + final Throwable thrown = catchThrowable(() -> permissioningConfig(toml)); + assertThat(thrown) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Invalid account 0xfoo"); + } + + @Test + public void permissioningConfigWithInvalidEnode() throws Exception { final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_INVALID_ENODE); final Path toml = Files.createTempFile("toml", ""); Files.write(toml, Resources.toByteArray(configFile)); - try { - PermissioningConfigurationBuilder.permissioningConfiguration( - toml.toAbsolutePath().toString(), true, true); - fail("Expecting IllegalArgumentException: Enode URL contains an invalid node ID"); - } catch (IllegalArgumentException e) { - assertThat(e.getMessage()).startsWith("Enode URL contains an invalid node ID"); - } + final Throwable thrown = catchThrowable(() -> permissioningConfig(toml)); + + assertThat(thrown) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Enode URL contains an invalid node ID"); } @Test public void permissioningConfigWithEmptyWhitelistMustNotError() throws Exception { - final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_EMPTY_WHITELISTS); final Path toml = Files.createTempFile("toml", ""); Files.write(toml, Resources.toByteArray(configFile)); - PermissioningConfiguration permissioningConfiguration = - PermissioningConfigurationBuilder.permissioningConfiguration( - toml.toAbsolutePath().toString(), true, true); + PermissioningConfiguration permissioningConfiguration = permissioningConfig(toml); assertThat(permissioningConfiguration.isNodeWhitelistEnabled()).isTrue(); assertThat(permissioningConfiguration.getNodeWhitelist()).isEmpty(); @@ -138,57 +141,41 @@ public class PermissioningConfigurationBuilderTest { @Test public void permissioningConfigWithAbsentWhitelistMustThrowException() throws Exception { - final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_ABSENT_WHITELISTS); final Path toml = Files.createTempFile("toml", ""); Files.write(toml, Resources.toByteArray(configFile)); - try { - PermissioningConfigurationBuilder.permissioningConfiguration( - toml.toAbsolutePath().toString(), true, true); - fail("expected exception: no valid whitelists in the TOML file"); - } catch (Exception e) { - assertThat(e.getMessage().contains("Unexpected end of line")).isTrue(); - } + final Throwable thrown = catchThrowable(() -> permissioningConfig(toml)); + + assertThat(thrown).isInstanceOf(Exception.class).hasMessageContaining("Unexpected end of line"); } @Test public void permissioningConfigWithUnrecognizedKeyMustThrowException() throws Exception { - final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_UNRECOGNIZED_KEY); final Path toml = Files.createTempFile("toml", ""); Files.write(toml, Resources.toByteArray(configFile)); - try { - PermissioningConfigurationBuilder.permissioningConfiguration( - toml.toAbsolutePath().toString(), true, true); - fail("expected exception: didn't find a recognized key in the TOML file"); - } catch (Exception e) { - assertThat(e.getMessage().contains("config option missing")).isTrue(); - assertThat(e.getMessage().contains(PermissioningConfigurationBuilder.ACCOUNTS_WHITELIST)) - .isTrue(); - } + final Throwable thrown = catchThrowable(() -> permissioningConfig(toml)); + + assertThat(thrown) + .isInstanceOf(Exception.class) + .hasMessageContaining("config option missing") + .hasMessageContaining(PermissioningConfigurationBuilder.ACCOUNTS_WHITELIST); } @Test public void permissioningConfigWithEmptyFileMustThrowException() throws Exception { - // write an empty file final Path toml = Files.createTempFile("toml", ""); - try { - PermissioningConfigurationBuilder.permissioningConfiguration( - toml.toAbsolutePath().toString(), true, true); - fail("expected exception: empty TOML file"); + final Throwable thrown = catchThrowable(() -> permissioningConfig(toml)); - } catch (Exception e) { - assertThat(e.getMessage().contains("Empty TOML result")).isTrue(); - } + assertThat(thrown).isInstanceOf(Exception.class).hasMessageContaining("Empty TOML result"); } @Test public void permissioningConfigFromFileMustSetFilePath() throws Exception { - final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_VALID); final Path toml = Files.createTempFile("toml", ""); Files.write(toml, Resources.toByteArray(configFile)); @@ -202,14 +189,12 @@ public class PermissioningConfigurationBuilderTest { @Test public void permissioningConfigFromNonexistentFileMustThrowException() { + final Throwable thrown = + catchThrowable(() -> permissioningConfig(Paths.get("file-does-not-exist"))); - try { - PermissioningConfigurationBuilder.permissioningConfigurationFromToml( - "file-does-not-exist", true, true); - fail("expected exception: file does not exist"); - } catch (Exception e) { - assertThat(e.getMessage().contains("Configuration file does not exist")).isTrue(); - } + assertThat(thrown) + .isInstanceOf(Exception.class) + .hasMessageContaining("Configuration file does not exist"); } @Test @@ -223,4 +208,9 @@ public class PermissioningConfigurationBuilderTest { assertThat(permissioningConfiguration.isNodeWhitelistEnabled()).isTrue(); assertThat(permissioningConfiguration.getNodeWhitelist().size()).isEqualTo(5); } + + private PermissioningConfiguration permissioningConfig(final Path toml) throws Exception { + return PermissioningConfigurationBuilder.permissioningConfiguration( + toml.toAbsolutePath().toString(), true, true); + } } diff --git a/ethereum/permissioning/src/test/resources/permissioning_config.toml b/ethereum/permissioning/src/test/resources/permissioning_config.toml new file mode 100644 index 0000000000..0457556cfd --- /dev/null +++ b/ethereum/permissioning/src/test/resources/permissioning_config.toml @@ -0,0 +1,4 @@ +# Permissioning TOML file + +accounts-whitelist=["0x0000000000000000000000000000000000000009"] +nodes-whitelist=["enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.9:4567","enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.169.0.9:4568"] diff --git a/pantheon/src/test/resources/permissioning_config_absent_whitelists.toml b/ethereum/permissioning/src/test/resources/permissioning_config_absent_whitelists.toml similarity index 100% rename from pantheon/src/test/resources/permissioning_config_absent_whitelists.toml rename to ethereum/permissioning/src/test/resources/permissioning_config_absent_whitelists.toml diff --git a/pantheon/src/test/resources/permissioning_config_account_whitelist_only.toml b/ethereum/permissioning/src/test/resources/permissioning_config_account_whitelist_only.toml similarity index 100% rename from pantheon/src/test/resources/permissioning_config_account_whitelist_only.toml rename to ethereum/permissioning/src/test/resources/permissioning_config_account_whitelist_only.toml diff --git a/pantheon/src/test/resources/permissioning_config_empty_whitelists.toml b/ethereum/permissioning/src/test/resources/permissioning_config_empty_whitelists.toml similarity index 100% rename from pantheon/src/test/resources/permissioning_config_empty_whitelists.toml rename to ethereum/permissioning/src/test/resources/permissioning_config_empty_whitelists.toml diff --git a/ethereum/permissioning/src/test/resources/permissioning_config_invalid_account.toml b/ethereum/permissioning/src/test/resources/permissioning_config_invalid_account.toml new file mode 100644 index 0000000000..da69df0743 --- /dev/null +++ b/ethereum/permissioning/src/test/resources/permissioning_config_invalid_account.toml @@ -0,0 +1,3 @@ +# Permissioning TOML file (account whitelist only) + +accounts-whitelist=["0xfoo"] diff --git a/pantheon/src/test/resources/permissioning_config_invalid_enode.toml b/ethereum/permissioning/src/test/resources/permissioning_config_invalid_enode.toml similarity index 100% rename from pantheon/src/test/resources/permissioning_config_invalid_enode.toml rename to ethereum/permissioning/src/test/resources/permissioning_config_invalid_enode.toml diff --git a/pantheon/src/test/resources/permissioning_config_node_whitelist_only.toml b/ethereum/permissioning/src/test/resources/permissioning_config_node_whitelist_only.toml similarity index 100% rename from pantheon/src/test/resources/permissioning_config_node_whitelist_only.toml rename to ethereum/permissioning/src/test/resources/permissioning_config_node_whitelist_only.toml diff --git a/pantheon/src/test/resources/permissioning_config_node_whitelist_only_multiline.toml b/ethereum/permissioning/src/test/resources/permissioning_config_node_whitelist_only_multiline.toml similarity index 100% rename from pantheon/src/test/resources/permissioning_config_node_whitelist_only_multiline.toml rename to ethereum/permissioning/src/test/resources/permissioning_config_node_whitelist_only_multiline.toml diff --git a/pantheon/src/test/resources/permissioning_config_unrecognized_key.toml b/ethereum/permissioning/src/test/resources/permissioning_config_unrecognized_key.toml similarity index 100% rename from pantheon/src/test/resources/permissioning_config_unrecognized_key.toml rename to ethereum/permissioning/src/test/resources/permissioning_config_unrecognized_key.toml diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java index b093597b2c..d563bbe9b5 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java @@ -25,7 +25,6 @@ import static tech.pegasys.pantheon.metrics.prometheus.MetricsConfiguration.DEFA import static tech.pegasys.pantheon.metrics.prometheus.MetricsConfiguration.DEFAULT_METRICS_PUSH_PORT; import static tech.pegasys.pantheon.metrics.prometheus.MetricsConfiguration.createDefault; -import tech.pegasys.pantheon.PermissioningConfigurationBuilder; import tech.pegasys.pantheon.Runner; import tech.pegasys.pantheon.RunnerBuilder; import tech.pegasys.pantheon.cli.custom.CorsAllowedOriginsProperty; @@ -47,6 +46,7 @@ import tech.pegasys.pantheon.ethereum.jsonrpc.RpcApi; import tech.pegasys.pantheon.ethereum.jsonrpc.RpcApis; import tech.pegasys.pantheon.ethereum.jsonrpc.websocket.WebSocketConfiguration; import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfiguration; +import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfigurationBuilder; import tech.pegasys.pantheon.metrics.MetricsSystem; import tech.pegasys.pantheon.metrics.prometheus.MetricsConfiguration; import tech.pegasys.pantheon.metrics.prometheus.PrometheusMetricsSystem; diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/custom/EnodeToURIPropertyConverter.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/custom/EnodeToURIPropertyConverter.java index e7ae5931ba..b3ee68ed89 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/custom/EnodeToURIPropertyConverter.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/custom/EnodeToURIPropertyConverter.java @@ -12,103 +12,29 @@ */ package tech.pegasys.pantheon.cli.custom; -import static com.google.common.base.Preconditions.checkArgument; - -import tech.pegasys.pantheon.util.NetworkUtility; +import tech.pegasys.pantheon.util.enode.EnodeURL; import java.net.URI; -import java.util.regex.Matcher; -import java.util.regex.Pattern; +import java.util.function.Function; +import com.google.common.annotations.VisibleForTesting; import picocli.CommandLine.ITypeConverter; public class EnodeToURIPropertyConverter implements ITypeConverter { - private static final String IP_REPLACE_MARKER = "$$IP_PATTERN$$"; - private static final String IPV4_PATTERN = - "(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)(\\.(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)){3}"; - private static final String IPV6_PATTERN = "\\[(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}\\]"; - private static final String IPV6_COMPACT_PATTERN = - "\\[((?:[0-9A-Fa-f]{1,4}(?::[0-9A-Fa-f]{1,4})*)?)::((?:[0-9A-Fa-f]{1,4}(?::[0-9A-Fa-f]{1,4})*)?)\\]"; - private static final String DISCOVERY_PORT_PATTERN = "\\?discport=(?\\d+)"; - private static final String HEX_STRING_PATTERN = "[0-9a-fA-F]+"; - - private static final String ENODE_URL_PATTERN = - "enode://(?\\w+)@(?" + IP_REPLACE_MARKER + "):(?\\d+)"; - - @Override - public URI convert(final String value) throws IllegalArgumentException { - return convertToURI(value); - } - - public static URI convertToURI(final String value) throws IllegalArgumentException { - checkArgument( - value != null && !value.isEmpty(), "Can't convert null/empty string to EnodeURLProperty."); - - final boolean containsDiscoveryPort = value.contains("discport"); - final boolean isIPV4 = Pattern.compile(".*" + IPV4_PATTERN + ".*").matcher(value).matches(); - final boolean isIPV6 = Pattern.compile(".*" + IPV6_PATTERN + ".*").matcher(value).matches(); - final boolean isIPV6Compact = - Pattern.compile(".*" + IPV6_COMPACT_PATTERN + ".*").matcher(value).matches(); - - String pattern = ENODE_URL_PATTERN; - if (isIPV4) { - pattern = pattern.replace(IP_REPLACE_MARKER, IPV4_PATTERN); - } else if (isIPV6) { - pattern = pattern.replace(IP_REPLACE_MARKER, IPV6_PATTERN); - } else if (isIPV6Compact) { - pattern = pattern.replace(IP_REPLACE_MARKER, IPV6_COMPACT_PATTERN); - } else { - throw new IllegalArgumentException("Invalid enode URL IP format."); - } + private final Function converter; - if (containsDiscoveryPort) { - pattern += DISCOVERY_PORT_PATTERN; - } - if (isIPV6) { - pattern = pattern.replace(IP_REPLACE_MARKER, IPV6_PATTERN); - } else { - pattern = pattern.replace(IP_REPLACE_MARKER, IPV4_PATTERN); - } - - final Matcher matcher = Pattern.compile(pattern).matcher(value); - checkArgument( - matcher.matches(), - "Invalid enode URL syntax. Enode URL should have the following format 'enode://@:[?discport=]'."); - - final String nodeId = getAndValidateNodeId(matcher); - final String ip = matcher.group("ip"); - final Integer listeningPort = getAndValidatePort(matcher, "listening"); - - if (containsDiscoveryPort(value)) { - final Integer discoveryPort = getAndValidatePort(matcher, "discovery"); - return URI.create( - String.format("enode://%s@%s:%d?discport=%d", nodeId, ip, listeningPort, discoveryPort)); - } else { - return URI.create(String.format("enode://%s@%s:%d", nodeId, ip, listeningPort)); - } - } - - private static String getAndValidateNodeId(final Matcher matcher) { - final String invalidNodeIdErrorMsg = - "Enode URL contains an invalid node ID. Node ID must have 128 characters and shouldn't include the '0x' hex prefix."; - final String nodeId = matcher.group("nodeId"); - - checkArgument(nodeId.matches(HEX_STRING_PATTERN), invalidNodeIdErrorMsg); - checkArgument(nodeId.length() == 128, invalidNodeIdErrorMsg); - - return nodeId; + EnodeToURIPropertyConverter() { + this.converter = (s) -> new EnodeURL(s).toURI(); } - private static Integer getAndValidatePort(final Matcher matcher, final String portName) { - int port = Integer.valueOf(matcher.group(portName)); - checkArgument( - NetworkUtility.isValidPort(port), - "Invalid " + portName + " port range. Port should be between 0 - 65535"); - return port; + @VisibleForTesting + EnodeToURIPropertyConverter(final Function converter) { + this.converter = converter; } - private static boolean containsDiscoveryPort(final String value) { - return value.contains("discport"); + @Override + public URI convert(final String value) { + return converter.apply(value); } } diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/custom/EnodeToURIPropertyConverterTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/custom/EnodeToURIPropertyConverterTest.java index 47702b2426..7b93b8460c 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/custom/EnodeToURIPropertyConverterTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/custom/EnodeToURIPropertyConverterTest.java @@ -12,212 +12,24 @@ */ package tech.pegasys.pantheon.cli.custom; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.catchThrowable; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import java.net.URI; +import java.util.function.Function; import org.junit.Test; public class EnodeToURIPropertyConverterTest { - private final String VALID_NODE_ID = - "6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"; - private final String IPV4_ADDRESS = "192.168.0.1"; - private final String IPV6_FULL_ADDRESS = "[2001:db8:85a3:0:0:8a2e:0370:7334]"; - private final String IPV6_COMPACT_ADDRESS = "[2001:db8:85a3::8a2e:0370:7334]"; - private final int P2P_PORT = 30303; - private final String DISCOVERY_QUERY = "discport=30301"; - - private final EnodeToURIPropertyConverter converter = new EnodeToURIPropertyConverter(); - - @Test - public void convertEnodeURLWithDiscoveryPortShouldBuildExpectedURI() { - final String value = - "enode://" + VALID_NODE_ID + "@" + IPV4_ADDRESS + ":" + P2P_PORT + "?" + DISCOVERY_QUERY; - final URI expectedURI = URI.create(value); - - final URI convertedURI = converter.convert(value); - - assertThat(convertedURI).isEqualTo(expectedURI); - assertThat(convertedURI.getUserInfo()).isEqualTo(VALID_NODE_ID); - assertThat(convertedURI.getHost()).isEqualTo(IPV4_ADDRESS); - assertThat(convertedURI.getPort()).isEqualTo(P2P_PORT); - assertThat(convertedURI.getQuery()).isEqualTo(DISCOVERY_QUERY); - } - - @Test - public void convertEnodeURLWithoutDiscoveryPortShouldBuildExpectedURI() { - final String value = "enode://" + VALID_NODE_ID + "@" + IPV4_ADDRESS + ":" + P2P_PORT; - final URI expectedURI = URI.create(value); - - final URI convertedURI = converter.convert(value); - - assertThat(convertedURI).isEqualTo(expectedURI); - assertThat(convertedURI.getUserInfo()).isEqualTo(VALID_NODE_ID); - assertThat(convertedURI.getHost()).isEqualTo(IPV4_ADDRESS); - assertThat(convertedURI.getPort()).isEqualTo(P2P_PORT); - } - @Test - public void convertEnodeURLWithIPV6ShouldBuildExpectedURI() { - final String value = - "enode://" - + VALID_NODE_ID - + "@" - + IPV6_FULL_ADDRESS - + ":" - + P2P_PORT - + "?" - + DISCOVERY_QUERY; - final URI expectedURI = URI.create(value); + @SuppressWarnings("unchecked") + public void converterDelegatesToFunction() { + Function function = mock(Function.class); - final URI convertedURI = converter.convert(value); - - assertThat(convertedURI).isEqualTo(expectedURI); - assertThat(convertedURI.getUserInfo()).isEqualTo(VALID_NODE_ID); - assertThat(convertedURI.getHost()).isEqualTo(IPV6_FULL_ADDRESS); - assertThat(convertedURI.getPort()).isEqualTo(P2P_PORT); - assertThat(convertedURI.getQuery()).isEqualTo(DISCOVERY_QUERY); - } - - @Test - public void convertEnodeURLWithIPV6InCompactFormShouldBuildExpectedURI() { - final String value = - "enode://" - + VALID_NODE_ID - + "@" - + IPV6_COMPACT_ADDRESS - + ":" - + P2P_PORT - + "?" - + DISCOVERY_QUERY; - final URI expectedURI = URI.create(value); - - final URI convertedURI = converter.convert(value); - - assertThat(convertedURI).isEqualTo(expectedURI); - assertThat(convertedURI.getUserInfo()).isEqualTo(VALID_NODE_ID); - assertThat(convertedURI.getHost()).isEqualTo(IPV6_COMPACT_ADDRESS); - assertThat(convertedURI.getPort()).isEqualTo(P2P_PORT); - assertThat(convertedURI.getQuery()).isEqualTo(DISCOVERY_QUERY); - } - - @Test - public void convertEnodeURLWithoutNodeIdShouldFail() { - final String value = "enode://@" + IPV4_ADDRESS + ":" + P2P_PORT; - final Throwable thrown = catchThrowable(() -> converter.convert(value)); - - assertThat(thrown) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage( - "Invalid enode URL syntax. Enode URL should have the following format 'enode://@:[?discport=]'."); - } - - @Test - public void convertEnodeURLWithInvalidSizeNodeIdShouldFail() { - final String value = "enode://wrong_size_string@" + IPV4_ADDRESS + ":" + P2P_PORT; - final Throwable thrown = catchThrowable(() -> converter.convert(value)); - - assertThat(thrown) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage( - "Enode URL contains an invalid node ID. Node ID must have 128 characters and shouldn't include the '0x' hex prefix."); - } - - @Test - public void convertEnodeURLWithInvalidHexCharacterNodeIdShouldFail() { - final String value = - "enode://0x000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000@" - + IPV4_ADDRESS - + ":" - + P2P_PORT; - final Throwable thrown = catchThrowable(() -> converter.convert(value)); - - assertThat(thrown) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage( - "Enode URL contains an invalid node ID. Node ID must have 128 characters and shouldn't include the '0x' hex prefix."); - } - - @Test - public void convertEnodeURLWithoutIpShouldFail() { - final String value = "enode://" + VALID_NODE_ID + "@:" + P2P_PORT; - final Throwable thrown = catchThrowable(() -> converter.convert(value)); - - assertThat(thrown) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Invalid enode URL IP format."); - } - - @Test - public void convertEnodeURLWithInvalidIpFormatShouldFail() { - final String value = "enode://" + VALID_NODE_ID + "@192.0.1:" + P2P_PORT; - final Throwable thrown = catchThrowable(() -> converter.convert(value)); - - assertThat(thrown) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Invalid enode URL IP format."); - } - - @Test - public void convertEnodeURLWithoutListeningPortShouldFail() { - final String value = "enode://" + VALID_NODE_ID + "@" + IPV4_ADDRESS + ":"; - final Throwable thrown = catchThrowable(() -> converter.convert(value)); - - assertThat(thrown) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage( - "Invalid enode URL syntax. Enode URL should have the following format 'enode://@:[?discport=]'."); - } - - @Test - public void convertEnodeURLWithoutListeningPortAndWithDiscoveryPortShouldFail() { - final String value = "enode://" + VALID_NODE_ID + "@" + IPV4_ADDRESS + ":?30301"; - final Throwable thrown = catchThrowable(() -> converter.convert(value)); - - assertThat(thrown) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage( - "Invalid enode URL syntax. Enode URL should have the following format 'enode://@:[?discport=]'."); - } - - @Test - public void convertEnodeURLWithAboveRangeListeningPortShouldFail() { - final String value = "enode://" + VALID_NODE_ID + "@" + IPV4_ADDRESS + ":98765"; - final Throwable thrown = catchThrowable(() -> converter.convert(value)); - - assertThat(thrown) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Invalid listening port range. Port should be between 0 - 65535"); - } - - @Test - public void convertEnodeURLWithAboveRangeDiscoveryPortShouldFail() { - final String value = - "enode://" + VALID_NODE_ID + "@" + IPV4_ADDRESS + ":" + P2P_PORT + "?discport=98765"; - final Throwable thrown = catchThrowable(() -> converter.convert(value)); - - assertThat(thrown) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Invalid discovery port range. Port should be between 0 - 65535"); - } - - @Test - public void convertNullEnodeURLShouldFail() { - final Throwable thrown = catchThrowable(() -> converter.convert(null)); - - assertThat(thrown) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Can't convert null/empty string to EnodeURLProperty."); - } - - @Test - public void convertEmptyEnodeURLShouldFail() { - final Throwable thrown = catchThrowable(() -> converter.convert("")); + new EnodeToURIPropertyConverter(function).convert("foo"); - assertThat(thrown) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Can't convert null/empty string to EnodeURLProperty."); + verify(function).apply(eq("foo")); } } diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/util/PermissioningConfigurationValidatorTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/util/PermissioningConfigurationValidatorTest.java index 2fc06a3f74..c2a9de0f99 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/util/PermissioningConfigurationValidatorTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/util/PermissioningConfigurationValidatorTest.java @@ -15,10 +15,10 @@ package tech.pegasys.pantheon.util; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.fail; -import tech.pegasys.pantheon.PermissioningConfigurationBuilder; import tech.pegasys.pantheon.cli.EthNetworkConfig; import tech.pegasys.pantheon.cli.NetworkName; import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfiguration; +import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfigurationBuilder; import java.net.URL; import java.nio.file.Files; diff --git a/util/src/main/java/tech/pegasys/pantheon/util/enode/EnodeURL.java b/util/src/main/java/tech/pegasys/pantheon/util/enode/EnodeURL.java new file mode 100644 index 0000000000..17602a11b6 --- /dev/null +++ b/util/src/main/java/tech/pegasys/pantheon/util/enode/EnodeURL.java @@ -0,0 +1,165 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package tech.pegasys.pantheon.util.enode; + +import static com.google.common.base.Preconditions.checkArgument; + +import tech.pegasys.pantheon.util.NetworkUtility; + +import java.net.URI; +import java.util.OptionalInt; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import com.google.common.base.Objects; + +public class EnodeURL { + + private static final String IP_REPLACE_MARKER = "$$IP_PATTERN$$"; + private static final String IPV4_PATTERN = + "(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)(\\.(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)){3}"; + private static final String IPV6_PATTERN = "\\[(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}\\]"; + private static final String IPV6_COMPACT_PATTERN = + "\\[((?:[0-9A-Fa-f]{1,4}(?::[0-9A-Fa-f]{1,4})*)?)::((?:[0-9A-Fa-f]{1,4}(?::[0-9A-Fa-f]{1,4})*)?)\\]"; + private static final String DISCOVERY_PORT_PATTERN = "\\?discport=(?\\d+)"; + private static final String HEX_STRING_PATTERN = "[0-9a-fA-F]+"; + + private static final String ENODE_URL_PATTERN = + "enode://(?\\w+)@(?" + IP_REPLACE_MARKER + "):(?\\d+)"; + + private final String nodeId; + private final String ip; + private final Integer listeningPort; + private final OptionalInt discoveryPort; + + public EnodeURL( + final String nodeId, + final String ip, + final Integer listeningPort, + final OptionalInt discoveryPort) { + this.nodeId = nodeId; + this.ip = ip; + this.listeningPort = listeningPort; + this.discoveryPort = discoveryPort; + } + + public EnodeURL(final String nodeId, final String ip, final Integer listeningPort) { + this.nodeId = nodeId; + this.ip = ip; + this.listeningPort = listeningPort; + this.discoveryPort = OptionalInt.empty(); + } + + public EnodeURL(final String value) { + checkArgument( + value != null && !value.isEmpty(), "Can't convert null/empty string to EnodeURLProperty."); + + final boolean containsDiscoveryPort = value.contains("discport"); + final boolean isIPV4 = Pattern.compile(".*" + IPV4_PATTERN + ".*").matcher(value).matches(); + final boolean isIPV6 = Pattern.compile(".*" + IPV6_PATTERN + ".*").matcher(value).matches(); + final boolean isIPV6Compact = + Pattern.compile(".*" + IPV6_COMPACT_PATTERN + ".*").matcher(value).matches(); + + String pattern = ENODE_URL_PATTERN; + if (isIPV4) { + pattern = pattern.replace(IP_REPLACE_MARKER, IPV4_PATTERN); + } else if (isIPV6) { + pattern = pattern.replace(IP_REPLACE_MARKER, IPV6_PATTERN); + } else if (isIPV6Compact) { + pattern = pattern.replace(IP_REPLACE_MARKER, IPV6_COMPACT_PATTERN); + } else { + throw new IllegalArgumentException("Invalid enode URL IP format."); + } + + if (containsDiscoveryPort) { + pattern += DISCOVERY_PORT_PATTERN; + } + if (isIPV6) { + pattern = pattern.replace(IP_REPLACE_MARKER, IPV6_PATTERN); + } else { + pattern = pattern.replace(IP_REPLACE_MARKER, IPV4_PATTERN); + } + + final Matcher matcher = Pattern.compile(pattern).matcher(value); + checkArgument( + matcher.matches(), + "Invalid enode URL syntax. Enode URL should have the following format 'enode://@:[?discport=]'."); + + this.nodeId = getAndValidateNodeId(matcher); + this.ip = matcher.group("ip"); + this.listeningPort = getAndValidatePort(matcher, "listening"); + + if (containsDiscoveryPort(value)) { + this.discoveryPort = OptionalInt.of(getAndValidatePort(matcher, "discovery")); + } else { + this.discoveryPort = OptionalInt.empty(); + } + } + + public URI toURI() { + if (discoveryPort.isPresent()) { + return URI.create( + String.format( + "enode://%s@%s:%d?discport=%d", nodeId, ip, listeningPort, discoveryPort.getAsInt())); + } else { + return URI.create(String.format("enode://%s@%s:%d", nodeId, ip, listeningPort)); + } + } + + public static URI asURI(final String url) { + return new EnodeURL(url).toURI(); + } + + private static String getAndValidateNodeId(final Matcher matcher) { + final String invalidNodeIdErrorMsg = + "Enode URL contains an invalid node ID. Node ID must have 128 characters and shouldn't include the '0x' hex prefix."; + final String nodeId = matcher.group("nodeId"); + + checkArgument(nodeId.matches(HEX_STRING_PATTERN), invalidNodeIdErrorMsg); + checkArgument(nodeId.length() == 128, invalidNodeIdErrorMsg); + + return nodeId; + } + + private static Integer getAndValidatePort(final Matcher matcher, final String portName) { + int port = Integer.valueOf(matcher.group(portName)); + checkArgument( + NetworkUtility.isValidPort(port), + "Invalid " + portName + " port range. Port should be between 0 - 65535"); + return port; + } + + private static boolean containsDiscoveryPort(final String value) { + return value.contains("discport"); + } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + EnodeURL enodeURL = (EnodeURL) o; + return Objects.equal(nodeId, enodeURL.nodeId) + && Objects.equal(ip, enodeURL.ip) + && Objects.equal(listeningPort, enodeURL.listeningPort) + && Objects.equal(discoveryPort, enodeURL.discoveryPort); + } + + @Override + public int hashCode() { + return Objects.hashCode(nodeId, ip, listeningPort, discoveryPort); + } +} diff --git a/util/src/test/java/tech/pegasys/pantheon/util/enode/EnodeURLTest.java b/util/src/test/java/tech/pegasys/pantheon/util/enode/EnodeURLTest.java new file mode 100644 index 0000000000..10e0243bac --- /dev/null +++ b/util/src/test/java/tech/pegasys/pantheon/util/enode/EnodeURLTest.java @@ -0,0 +1,230 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package tech.pegasys.pantheon.util.enode; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowable; + +import java.net.URI; +import java.util.OptionalInt; + +import org.junit.Test; + +public class EnodeURLTest { + + private final String VALID_NODE_ID = + "6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"; + private final String IPV4_ADDRESS = "192.168.0.1"; + private final String IPV6_FULL_ADDRESS = "[2001:db8:85a3:0:0:8a2e:0370:7334]"; + private final String IPV6_COMPACT_ADDRESS = "[2001:db8:85a3::8a2e:0370:7334]"; + private final int P2P_PORT = 30303; + private final int DISCOVERY_PORT = 30301; + private final String DISCOVERY_QUERY = "discport=" + DISCOVERY_PORT; + + @Test + public void createEnodeURLWithDiscoveryPortShouldBuildExpectedEnodeURLObject() { + final EnodeURL expectedEnodeURL = + new EnodeURL(VALID_NODE_ID, IPV4_ADDRESS, P2P_PORT, OptionalInt.of(DISCOVERY_PORT)); + final String enodeURLString = + "enode://" + VALID_NODE_ID + "@" + IPV4_ADDRESS + ":" + P2P_PORT + "?" + DISCOVERY_QUERY; + + final EnodeURL enodeURL = new EnodeURL(enodeURLString); + + assertThat(enodeURL).isEqualTo(expectedEnodeURL); + } + + @Test + public void createEnodeURLWithoutDiscoveryPortShouldBuildExpectedEnodeURLObject() { + final EnodeURL expectedEnodeURL = new EnodeURL(VALID_NODE_ID, IPV4_ADDRESS, P2P_PORT); + final String enodeURLString = "enode://" + VALID_NODE_ID + "@" + IPV4_ADDRESS + ":" + P2P_PORT; + + final EnodeURL enodeURL = new EnodeURL(enodeURLString); + + assertThat(enodeURL).isEqualTo(expectedEnodeURL); + } + + @Test + public void createEnodeURLWithIPV6ShouldBuildExpectedEnodeURLObject() { + final EnodeURL expectedEnodeURL = + new EnodeURL(VALID_NODE_ID, IPV6_FULL_ADDRESS, P2P_PORT, OptionalInt.of(DISCOVERY_PORT)); + final String enodeURLString = + "enode://" + + VALID_NODE_ID + + "@" + + IPV6_FULL_ADDRESS + + ":" + + P2P_PORT + + "?" + + DISCOVERY_QUERY; + + final EnodeURL enodeURL = new EnodeURL(enodeURLString); + + assertThat(enodeURL).isEqualTo(expectedEnodeURL); + } + + @Test + public void createEnodeURLWithIPV6InCompactFormShouldBuildExpectedEnodeURLObject() { + final EnodeURL expectedEnodeURL = + new EnodeURL(VALID_NODE_ID, IPV6_COMPACT_ADDRESS, P2P_PORT, OptionalInt.of(DISCOVERY_PORT)); + final String enodeURLString = + "enode://" + + VALID_NODE_ID + + "@" + + IPV6_COMPACT_ADDRESS + + ":" + + P2P_PORT + + "?" + + DISCOVERY_QUERY; + + final EnodeURL enodeURL = new EnodeURL(enodeURLString); + + assertThat(enodeURL).isEqualTo(expectedEnodeURL); + } + + @Test + public void createEnodeURLWithoutNodeIdShouldFail() { + final String enodeURLString = "enode://@" + IPV4_ADDRESS + ":" + P2P_PORT; + final Throwable thrown = catchThrowable(() -> new EnodeURL(enodeURLString)); + + assertThat(thrown) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Invalid enode URL syntax. Enode URL should have the following format 'enode://@:[?discport=]'."); + } + + @Test + public void createEnodeURLWithInvalidSizeNodeIdShouldFail() { + final String enodeURLString = "enode://wrong_size_string@" + IPV4_ADDRESS + ":" + P2P_PORT; + final Throwable thrown = catchThrowable(() -> new EnodeURL(enodeURLString)); + + assertThat(thrown) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Enode URL contains an invalid node ID. Node ID must have 128 characters and shouldn't include the '0x' hex prefix."); + } + + @Test + public void createEnodeURLWithInvalidHexCharacterNodeIdShouldFail() { + final String enodeURLString = + "enode://0x000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000@" + + IPV4_ADDRESS + + ":" + + P2P_PORT; + final Throwable thrown = catchThrowable(() -> new EnodeURL(enodeURLString)); + + assertThat(thrown) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Enode URL contains an invalid node ID. Node ID must have 128 characters and shouldn't include the '0x' hex prefix."); + } + + @Test + public void createEnodeURLWithoutIpShouldFail() { + final String enodeURLString = "enode://" + VALID_NODE_ID + "@:" + P2P_PORT; + final Throwable thrown = catchThrowable(() -> new EnodeURL(enodeURLString)); + + assertThat(thrown) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid enode URL IP format."); + } + + @Test + public void createEnodeURLWithInvalidIpFormatShouldFail() { + final String enodeURLString = "enode://" + VALID_NODE_ID + "@192.0.1:" + P2P_PORT; + final Throwable thrown = catchThrowable(() -> new EnodeURL(enodeURLString)); + + assertThat(thrown) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid enode URL IP format."); + } + + @Test + public void createEnodeURLWithoutListeningPortShouldFail() { + final String enodeURLString = "enode://" + VALID_NODE_ID + "@" + IPV4_ADDRESS + ":"; + final Throwable thrown = catchThrowable(() -> new EnodeURL(enodeURLString)); + + assertThat(thrown) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Invalid enode URL syntax. Enode URL should have the following format 'enode://@:[?discport=]'."); + } + + @Test + public void createEnodeURLWithoutListeningPortAndWithDiscoveryPortShouldFail() { + final String enodeURLString = "enode://" + VALID_NODE_ID + "@" + IPV4_ADDRESS + ":?30301"; + final Throwable thrown = catchThrowable(() -> new EnodeURL(enodeURLString)); + + assertThat(thrown) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Invalid enode URL syntax. Enode URL should have the following format 'enode://@:[?discport=]'."); + } + + @Test + public void createEnodeURLWithAboveRangeListeningPortShouldFail() { + final String enodeURLString = "enode://" + VALID_NODE_ID + "@" + IPV4_ADDRESS + ":98765"; + final Throwable thrown = catchThrowable(() -> new EnodeURL(enodeURLString)); + + assertThat(thrown) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid listening port range. Port should be between 0 - 65535"); + } + + @Test + public void createEnodeURLWithAboveRangeDiscoveryPortShouldFail() { + final String enodeURLString = + "enode://" + VALID_NODE_ID + "@" + IPV4_ADDRESS + ":" + P2P_PORT + "?discport=98765"; + final Throwable thrown = catchThrowable(() -> new EnodeURL(enodeURLString)); + + assertThat(thrown) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid discovery port range. Port should be between 0 - 65535"); + } + + @Test + public void createEnodeURLWithNullEnodeURLShouldFail() { + final Throwable thrown = catchThrowable(() -> new EnodeURL(null)); + + assertThat(thrown) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Can't convert null/empty string to EnodeURLProperty."); + } + + @Test + public void createEnodeURLWithEmptyEnodeURLShouldFail() { + final Throwable thrown = catchThrowable(() -> new EnodeURL("")); + + assertThat(thrown) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Can't convert null/empty string to EnodeURLProperty."); + } + + @Test + public void toURIWithDiscoveryPortCreateExpectedURI() { + final String enodeURLString = + "enode://" + VALID_NODE_ID + "@" + IPV4_ADDRESS + ":" + P2P_PORT + "?" + DISCOVERY_QUERY; + final URI expectedURI = URI.create(enodeURLString); + final URI createdURI = new EnodeURL(enodeURLString).toURI(); + + assertThat(createdURI).isEqualTo(expectedURI); + } + + @Test + public void toURIWithoutDiscoveryPortCreateExpectedURI() { + final String enodeURLString = "enode://" + VALID_NODE_ID + "@" + IPV4_ADDRESS + ":" + P2P_PORT; + final URI expectedURI = URI.create(enodeURLString); + final URI createdURI = new EnodeURL(enodeURLString).toURI(); + + assertThat(createdURI).isEqualTo(expectedURI); + } +}