From 762254171400fb202ce8ac65148380e74a2823c7 Mon Sep 17 00:00:00 2001 From: mark-terry <36909937+mark-terry@users.noreply.github.com> Date: Mon, 11 Feb 2019 12:10:38 +1000 Subject: [PATCH] [NC-1968] Permissioning whitelist persistence. (#763) * [NC-1968] Initial commit. * [NC-1968] Acceptance tests. * [NC-1968] PR fixes. * [NC-1968] Merge conflicts. Signed-off-by: Adrian Sutton --- .../perm/WhiteListContainsKeyAndValue.java | 50 ++++++ .../tests/acceptance/dsl/jsonrpc/Perm.java | 11 ++ .../dsl/node/factory/PantheonNodeFactory.java | 46 ++++++ ...emoveNodesFromWhitelistAcceptanceTest.java | 8 +- .../WhitelistPersistorAcceptanceTest.java | 102 ++++++++++++ .../PermAddAccountsToWhitelist.java | 4 + .../PermAddNodesToWhitelist.java | 7 +- .../permissioning/PermGetNodesWhitelist.java | 21 +-- .../PermRemoveAccountsFromWhitelist.java | 4 + .../PermRemoveNodesFromWhitelist.java | 7 +- .../internal/response/JsonRpcError.java | 10 +- .../NodeWhitelistController.java | 74 ++++++++- .../ethereum/p2p/NettyP2PNetworkTest.java | 13 +- .../internal/PeerDiscoveryControllerTest.java | 26 +-- .../NodeWhitelistControllerTest.java | 37 ++++- ethereum/permissioning/build.gradle | 2 + .../AccountWhitelistController.java | 48 +++++- .../WhitelistFileSyncException.java | 15 ++ .../WhitelistOperationResult.java | 4 +- .../permissioning/WhitelistPersistor.java | 152 ++++++++++++++++++ .../AccountWhitelistControllerTest.java | 39 ++++- .../permissioning/WhitelistPersistorTest.java | 140 ++++++++++++++++ .../PermissioningConfigurationBuilder.java | 34 +--- .../pantheon/TomlConfigFileParser.java | 37 ++++- ...PermissioningConfigurationBuilderTest.java | 16 +- .../pantheon/cli/PantheonCommandTest.java | 2 +- ..._config_node_whitelist_only_multiline.toml | 9 ++ 27 files changed, 832 insertions(+), 86 deletions(-) create mode 100644 acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/condition/perm/WhiteListContainsKeyAndValue.java create mode 100644 acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/permissioning/WhitelistPersistorAcceptanceTest.java create mode 100644 ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistFileSyncException.java create mode 100644 ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistPersistor.java create mode 100644 ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistPersistorTest.java create mode 100644 pantheon/src/test/resources/permissioning_config_node_whitelist_only_multiline.toml diff --git a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/condition/perm/WhiteListContainsKeyAndValue.java b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/condition/perm/WhiteListContainsKeyAndValue.java new file mode 100644 index 0000000000..0527ffb097 --- /dev/null +++ b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/condition/perm/WhiteListContainsKeyAndValue.java @@ -0,0 +1,50 @@ +/* + * 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.tests.acceptance.dsl.condition.perm; + +import static org.assertj.core.api.Assertions.assertThat; + +import tech.pegasys.pantheon.ethereum.permissioning.WhitelistPersistor; +import tech.pegasys.pantheon.tests.acceptance.dsl.condition.Condition; +import tech.pegasys.pantheon.tests.acceptance.dsl.node.Node; + +import java.nio.file.Path; +import java.util.Collection; + +public class WhiteListContainsKeyAndValue implements Condition { + private final WhitelistPersistor.WHITELIST_TYPE whitelistType; + private final Collection whitelistValues; + private final Path configFilePath; + + public WhiteListContainsKeyAndValue( + final WhitelistPersistor.WHITELIST_TYPE whitelistType, + final Collection whitelistValues, + final Path configFilePath) { + this.whitelistType = whitelistType; + this.whitelistValues = whitelistValues; + this.configFilePath = configFilePath; + } + + @Override + public void verify(final Node node) { + boolean result; + try { + result = + WhitelistPersistor.verifyConfigFileMatchesState( + whitelistType, whitelistValues, configFilePath); + } catch (final Exception e) { + result = false; + } + assertThat(result).isTrue(); + } +} diff --git a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/jsonrpc/Perm.java b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/jsonrpc/Perm.java index e8bb0edd11..9356491830 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/jsonrpc/Perm.java +++ b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/jsonrpc/Perm.java @@ -12,6 +12,7 @@ */ package tech.pegasys.pantheon.tests.acceptance.dsl.jsonrpc; +import tech.pegasys.pantheon.ethereum.permissioning.WhitelistPersistor; import tech.pegasys.pantheon.tests.acceptance.dsl.condition.Condition; import tech.pegasys.pantheon.tests.acceptance.dsl.condition.perm.AddAccountsToWhitelistSuccessfully; import tech.pegasys.pantheon.tests.acceptance.dsl.condition.perm.AddNodeSuccess; @@ -19,9 +20,12 @@ import tech.pegasys.pantheon.tests.acceptance.dsl.condition.perm.GetExpectedAcco import tech.pegasys.pantheon.tests.acceptance.dsl.condition.perm.GetNodesWhitelistPopulated; import tech.pegasys.pantheon.tests.acceptance.dsl.condition.perm.RemoveAccountsFromWhitelistSuccessfully; import tech.pegasys.pantheon.tests.acceptance.dsl.condition.perm.RemoveNodeSuccess; +import tech.pegasys.pantheon.tests.acceptance.dsl.condition.perm.WhiteListContainsKeyAndValue; import tech.pegasys.pantheon.tests.acceptance.dsl.transaction.Transactions; +import java.nio.file.Path; import java.util.Arrays; +import java.util.Collection; import java.util.List; public class Perm { @@ -57,4 +61,11 @@ public class Perm { public Condition getNodesWhitelist(final int expectedNodeNum) { return new GetNodesWhitelistPopulated(transactions.getNodesWhiteList(), expectedNodeNum); } + + public Condition expectPermissioningWhitelistFileKeyValue( + final WhitelistPersistor.WHITELIST_TYPE whitelistType, + final Collection val, + final Path configFilePath) { + return new WhiteListContainsKeyAndValue(whitelistType, val, configFilePath); + } } diff --git a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/factory/PantheonNodeFactory.java b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/factory/PantheonNodeFactory.java index e740eaacb6..00b2a7168c 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/factory/PantheonNodeFactory.java +++ b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/factory/PantheonNodeFactory.java @@ -26,21 +26,25 @@ 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.WhitelistPersistor; import tech.pegasys.pantheon.tests.acceptance.dsl.node.GenesisConfigProvider; import tech.pegasys.pantheon.tests.acceptance.dsl.node.PantheonNode; import tech.pegasys.pantheon.tests.acceptance.dsl.node.RunnableNode; +import java.io.File; import java.io.IOException; import java.net.ServerSocket; import java.net.URI; import java.net.URISyntaxException; import java.nio.charset.Charset; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.List; import java.util.Optional; import java.util.function.Function; +import java.util.stream.Collectors; import com.google.common.io.Resources; @@ -172,11 +176,37 @@ public class PantheonNodeFactory { .build()); } + public PantheonNode createNodeWithWhitelistsEnabled( + final String name, + final List nodesWhitelist, + final List accountsWhitelist, + final String tempFilePath) + throws IOException { + PermissioningConfiguration permissioningConfiguration = + PermissioningConfiguration.createDefault(); + permissioningConfiguration.setNodeWhitelist(nodesWhitelist); + permissioningConfiguration.setAccountWhitelist(accountsWhitelist); + permissioningConfiguration.setConfigurationFilePath(tempFilePath); + + return create( + new PantheonFactoryConfigurationBuilder() + .setName(name) + .setJsonRpcConfiguration(jsonRpcConfigWithPermissioning()) + .setPermissioningConfiguration(permissioningConfiguration) + .build()); + } + public PantheonNode createNodeWithNodesWhitelist( final String name, final List nodesWhitelist) throws IOException { PermissioningConfiguration permissioningConfiguration = PermissioningConfiguration.createDefault(); permissioningConfiguration.setNodeWhitelist(nodesWhitelist); + File tempFile = createTempPermissioningConfigurationFile(); + permissioningConfiguration.setConfigurationFilePath(tempFile.getPath()); + initPermissioningConfigurationFile( + WhitelistPersistor.WHITELIST_TYPE.NODES, + nodesWhitelist.parallelStream().map(URI::toString).collect(Collectors.toList()), + tempFile.toPath()); return create( new PantheonFactoryConfigurationBuilder() @@ -186,11 +216,21 @@ public class PantheonNodeFactory { .build()); } + private void initPermissioningConfigurationFile( + final WhitelistPersistor.WHITELIST_TYPE listType, + final Collection whitelistVal, + final Path configFilePath) + throws IOException { + WhitelistPersistor.addNewConfigItem(listType, whitelistVal, configFilePath); + } + public PantheonNode createNodeWithAccountsWhitelist( final String name, final List accountsWhitelist) throws IOException { PermissioningConfiguration permissioningConfiguration = PermissioningConfiguration.createDefault(); permissioningConfiguration.setAccountWhitelist(accountsWhitelist); + permissioningConfiguration.setConfigurationFilePath( + createTempPermissioningConfigurationFile().getPath()); return create( new PantheonFactoryConfigurationBuilder() @@ -201,6 +241,12 @@ public class PantheonNodeFactory { .build()); } + private File createTempPermissioningConfigurationFile() throws IOException { + File tempFile = File.createTempFile("temp", "temp"); + tempFile.deleteOnExit(); + return tempFile; + } + public PantheonNode createNodeWithNoDiscovery(final String name) throws IOException { return create( new PantheonFactoryConfigurationBuilder().setName(name).setDiscoveryEnabled(false).build()); diff --git a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/jsonrpc/perm/PermRemoveNodesFromWhitelistAcceptanceTest.java b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/jsonrpc/perm/PermRemoveNodesFromWhitelistAcceptanceTest.java index 7ff1ea5783..2278d80f65 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/jsonrpc/perm/PermRemoveNodesFromWhitelistAcceptanceTest.java +++ b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/jsonrpc/perm/PermRemoveNodesFromWhitelistAcceptanceTest.java @@ -16,6 +16,7 @@ import tech.pegasys.pantheon.tests.acceptance.dsl.AcceptanceTestBase; import tech.pegasys.pantheon.tests.acceptance.dsl.node.Node; import java.net.URI; +import java.util.ArrayList; import com.google.common.collect.Lists; import org.junit.Before; @@ -31,13 +32,12 @@ public class PermRemoveNodesFromWhitelistAcceptanceTest extends AcceptanceTestBa "enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.2:4567"; private final String enode3 = "enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.3:4567"; + private final ArrayList nodesWhitelist = + Lists.newArrayList(URI.create(enode1), URI.create(enode2), URI.create(enode3)); @Before public void setUp() throws Exception { - node = - pantheon.createNodeWithNodesWhitelist( - "node1", - Lists.newArrayList(URI.create(enode1), URI.create(enode2), URI.create(enode3))); + node = pantheon.createNodeWithNodesWhitelist("node1", nodesWhitelist); cluster.start(node); } diff --git a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/permissioning/WhitelistPersistorAcceptanceTest.java b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/permissioning/WhitelistPersistorAcceptanceTest.java new file mode 100644 index 0000000000..95fb3a5cf6 --- /dev/null +++ b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/permissioning/WhitelistPersistorAcceptanceTest.java @@ -0,0 +1,102 @@ +/* + * 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.tests.acceptance.permissioning; + +import static tech.pegasys.pantheon.ethereum.permissioning.WhitelistPersistor.WHITELIST_TYPE; + +import tech.pegasys.pantheon.tests.acceptance.dsl.AcceptanceTestBase; +import tech.pegasys.pantheon.tests.acceptance.dsl.account.Account; +import tech.pegasys.pantheon.tests.acceptance.dsl.node.Node; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collections; + +import org.assertj.core.util.Lists; +import org.junit.Before; +import org.junit.Test; + +public class WhitelistPersistorAcceptanceTest extends AcceptanceTestBase { + + private Node node; + private Account senderA; + private Account senderB; + private Path tempFile; + + private final String enode1 = + "enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.10:4567"; + private final String enode2 = + "enode://5f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.10:4567"; + private final String enode3 = + "enode://4f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.10:4567"; + + @Before + public void setUp() throws Exception { + senderA = accounts.getPrimaryBenefactor(); + senderB = accounts.getSecondaryBenefactor(); + tempFile = Files.createTempFile("test", "test"); + node = + pantheon.createNodeWithWhitelistsEnabled( + "node", + new ArrayList<>(), + Collections.singletonList(senderA.getAddress()), + tempFile.toAbsolutePath().toString()); + cluster.start(node); + } + + @Test + public void manipulatedAccountsWhitelistIsPersisted() { + node.verify( + perm.expectPermissioningWhitelistFileKeyValue( + WHITELIST_TYPE.ACCOUNTS, Collections.singleton(senderA.getAddress()), tempFile)); + + node.execute(transactions.addAccountsToWhitelist(senderB.getAddress())); + node.verify(perm.expectAccountsWhitelist(senderA.getAddress(), senderB.getAddress())); + node.verify( + perm.expectPermissioningWhitelistFileKeyValue( + WHITELIST_TYPE.ACCOUNTS, + Lists.list(senderA.getAddress(), senderB.getAddress()), + tempFile)); + + node.execute(transactions.removeAccountsFromWhitelist(senderB.getAddress())); + node.verify(perm.expectAccountsWhitelist(senderA.getAddress())); + node.verify( + perm.expectPermissioningWhitelistFileKeyValue( + WHITELIST_TYPE.ACCOUNTS, Collections.singleton(senderA.getAddress()), tempFile)); + + node.execute(transactions.removeAccountsFromWhitelist(senderA.getAddress())); + node.verify(perm.expectAccountsWhitelist()); + node.verify( + perm.expectPermissioningWhitelistFileKeyValue( + WHITELIST_TYPE.ACCOUNTS, Collections.emptyList(), tempFile)); + } + + @Test + public void manipulatedNodesWhitelistIsPersisted() { + node.verify(perm.addNodesToWhitelist(Lists.newArrayList(enode1, enode2))); + node.verify( + perm.expectPermissioningWhitelistFileKeyValue( + WHITELIST_TYPE.NODES, Lists.newArrayList(enode1, enode2), tempFile)); + + node.verify(perm.removeNodesFromWhitelist(Lists.newArrayList(enode1))); + node.verify( + perm.expectPermissioningWhitelistFileKeyValue( + WHITELIST_TYPE.NODES, Collections.singleton(enode2), tempFile)); + + node.verify(perm.addNodesToWhitelist(Lists.newArrayList(enode1, enode3))); + node.verify( + perm.expectPermissioningWhitelistFileKeyValue( + WHITELIST_TYPE.NODES, Lists.newArrayList(enode2, enode1, enode3), tempFile)); + } +} diff --git a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermAddAccountsToWhitelist.java b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermAddAccountsToWhitelist.java index b6006b46dc..af47b1c86d 100644 --- a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermAddAccountsToWhitelist.java +++ b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermAddAccountsToWhitelist.java @@ -64,6 +64,10 @@ public class PermAddAccountsToWhitelist implements JsonRpcMethod { case ERROR_DUPLICATED_ENTRY: return new JsonRpcErrorResponse( request.getId(), JsonRpcError.ACCOUNT_WHITELIST_DUPLICATED_ENTRY); + case ERROR_WHITELIST_PERSIST_FAIL: + return new JsonRpcErrorResponse(request.getId(), JsonRpcError.WHITELIST_PERSIST_FAILURE); + case ERROR_WHITELIST_FILE_SYNC: + return new JsonRpcErrorResponse(request.getId(), JsonRpcError.WHITELIST_FILE_SYNC); case SUCCESS: return new JsonRpcSuccessResponse(request.getId()); default: diff --git a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermAddNodesToWhitelist.java b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermAddNodesToWhitelist.java index fb79a6442d..821498e75e 100644 --- a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermAddNodesToWhitelist.java +++ b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermAddNodesToWhitelist.java @@ -23,6 +23,7 @@ import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcSuccessRe import tech.pegasys.pantheon.ethereum.p2p.P2pDisabledException; import tech.pegasys.pantheon.ethereum.p2p.api.P2PNetwork; import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer; +import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; import tech.pegasys.pantheon.ethereum.p2p.permissioning.NodeWhitelistController; import java.util.List; @@ -51,7 +52,7 @@ public class PermAddNodesToWhitelist implements JsonRpcMethod { try { if (p2pNetwork.getNodeWhitelistController().isPresent()) { try { - List peers = + List peers = enodeListParam .getStringList() .parallelStream() @@ -72,6 +73,10 @@ public class PermAddNodesToWhitelist implements JsonRpcMethod { case ERROR_DUPLICATED_ENTRY: return new JsonRpcErrorResponse( req.getId(), JsonRpcError.NODE_WHITELIST_DUPLICATED_ENTRY); + case ERROR_WHITELIST_PERSIST_FAIL: + return new JsonRpcErrorResponse(req.getId(), JsonRpcError.WHITELIST_PERSIST_FAILURE); + case ERROR_WHITELIST_FILE_SYNC: + return new JsonRpcErrorResponse(req.getId(), JsonRpcError.WHITELIST_FILE_SYNC); default: throw new Exception(); } diff --git a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermGetNodesWhitelist.java b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermGetNodesWhitelist.java index 6adab1ab01..e8ddb9f0e4 100644 --- a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermGetNodesWhitelist.java +++ b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermGetNodesWhitelist.java @@ -20,15 +20,11 @@ import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcResponse; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcSuccessResponse; import tech.pegasys.pantheon.ethereum.p2p.P2pDisabledException; import tech.pegasys.pantheon.ethereum.p2p.api.P2PNetwork; -import tech.pegasys.pantheon.ethereum.p2p.peers.Endpoint; import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; import java.util.List; -import java.util.OptionalInt; import java.util.stream.Collectors; -import org.bouncycastle.util.encoders.Hex; - public class PermGetNodesWhitelist implements JsonRpcMethod { private final P2PNetwork p2pNetwork; @@ -49,7 +45,7 @@ public class PermGetNodesWhitelist implements JsonRpcMethod { List nodesWhitelist = p2pNetwork.getNodeWhitelistController().get().getNodesWhitelist(); List enodeList = - nodesWhitelist.parallelStream().map(this::buildEnodeURI).collect(Collectors.toList()); + nodesWhitelist.parallelStream().map(Peer::getEnodeURI).collect(Collectors.toList()); return new JsonRpcSuccessResponse(req.getId(), enodeList); } else { @@ -59,19 +55,4 @@ public class PermGetNodesWhitelist implements JsonRpcMethod { return new JsonRpcErrorResponse(req.getId(), JsonRpcError.P2P_DISABLED); } } - - private String buildEnodeURI(final Peer s) { - String url = Hex.toHexString(s.getId().extractArray()); - Endpoint endpoint = s.getEndpoint(); - String nodeIp = endpoint.getHost(); - OptionalInt tcpPort = endpoint.getTcpPort(); - int udpPort = endpoint.getUdpPort(); - - if (tcpPort.isPresent() && (tcpPort.getAsInt() != udpPort)) { - return String.format( - "enode://%s@%s:%d?discport=%d", url, nodeIp, tcpPort.getAsInt(), udpPort); - } else { - return String.format("enode://%s@%s:%d", url, nodeIp, udpPort); - } - } } diff --git a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermRemoveAccountsFromWhitelist.java b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermRemoveAccountsFromWhitelist.java index b85e809e49..2d8d8a9e7b 100644 --- a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermRemoveAccountsFromWhitelist.java +++ b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermRemoveAccountsFromWhitelist.java @@ -63,6 +63,10 @@ public class PermRemoveAccountsFromWhitelist implements JsonRpcMethod { case ERROR_DUPLICATED_ENTRY: return new JsonRpcErrorResponse( request.getId(), JsonRpcError.ACCOUNT_WHITELIST_DUPLICATED_ENTRY); + case ERROR_WHITELIST_PERSIST_FAIL: + return new JsonRpcErrorResponse(request.getId(), JsonRpcError.WHITELIST_PERSIST_FAILURE); + case ERROR_WHITELIST_FILE_SYNC: + return new JsonRpcErrorResponse(request.getId(), JsonRpcError.WHITELIST_FILE_SYNC); case SUCCESS: return new JsonRpcSuccessResponse(request.getId()); default: diff --git a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermRemoveNodesFromWhitelist.java b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermRemoveNodesFromWhitelist.java index 52333012f5..a77517532e 100644 --- a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermRemoveNodesFromWhitelist.java +++ b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermRemoveNodesFromWhitelist.java @@ -23,6 +23,7 @@ import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcSuccessRe import tech.pegasys.pantheon.ethereum.p2p.P2pDisabledException; import tech.pegasys.pantheon.ethereum.p2p.api.P2PNetwork; import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer; +import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; import tech.pegasys.pantheon.ethereum.p2p.permissioning.NodeWhitelistController; import java.util.List; @@ -51,7 +52,7 @@ public class PermRemoveNodesFromWhitelist implements JsonRpcMethod { try { if (p2pNetwork.getNodeWhitelistController().isPresent()) { try { - List peers = + List peers = enodeListParam .getStringList() .parallelStream() @@ -72,6 +73,10 @@ public class PermRemoveNodesFromWhitelist implements JsonRpcMethod { case ERROR_DUPLICATED_ENTRY: return new JsonRpcErrorResponse( req.getId(), JsonRpcError.NODE_WHITELIST_DUPLICATED_ENTRY); + case ERROR_WHITELIST_PERSIST_FAIL: + return new JsonRpcErrorResponse(req.getId(), JsonRpcError.WHITELIST_PERSIST_FAILURE); + case ERROR_WHITELIST_FILE_SYNC: + return new JsonRpcErrorResponse(req.getId(), JsonRpcError.WHITELIST_FILE_SYNC); default: throw new Exception(); } 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 21bd0ac88f..19e234f281 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 @@ -49,7 +49,7 @@ public enum JsonRpcError { // Wallet errors COINBASE_NOT_SPECIFIED(-32000, "Coinbase must be explicitly specified"), - // Permissioning errors + // Account whitelist errors ACCOUNT_WHITELIST_NOT_ENABLED(-32000, "Account whitelisting has not been enabled"), ACCOUNT_WHITELIST_EMPTY_ENTRY(-32000, "Request contains an empty list of accounts"), ACCOUNT_WHITELIST_INVALID_ENTRY(-32000, "Request contains an invalid account"), @@ -65,6 +65,14 @@ public enum JsonRpcError { NODE_WHITELIST_EXISTING_ENTRY(-32000, "Cannot add an existing node to whitelist"), NODE_WHITELIST_MISSING_ENTRY(-32000, "Cannot remove an absent node from whitelist"), + // Permissioning errors + WHITELIST_PERSIST_FAILURE( + -32000, "Unable to persist changes to whitelist configuration file. Changes reverted"), + WHITELIST_FILE_SYNC( + -32000, + "The permissioning whitelist configuration file is out of sync. The changes have been applied, but not persisted to disk"), + + // Private transaction errors ENCLAVE_IS_DOWN(-32000, "Enclave is down"); private final int code; 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 f10f3c6292..7d8c74995e 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,21 +15,35 @@ 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.WhitelistFileSyncException; import tech.pegasys.pantheon.ethereum.permissioning.WhitelistOperationResult; +import tech.pegasys.pantheon.ethereum.permissioning.WhitelistPersistor; +import java.io.IOException; import java.net.URI; import java.util.ArrayList; +import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Optional; +import java.util.stream.Collectors; import com.google.common.annotations.VisibleForTesting; public class NodeWhitelistController { - private final List nodesWhitelist = new ArrayList<>(); + private List nodesWhitelist = new ArrayList<>(); + private final WhitelistPersistor whitelistPersistor; - public NodeWhitelistController(final PermissioningConfiguration configuration) { + public NodeWhitelistController(final PermissioningConfiguration permissioningConfiguration) { + this( + permissioningConfiguration, + new WhitelistPersistor(permissioningConfiguration.getConfigurationFilePath())); + } + + public NodeWhitelistController( + final PermissioningConfiguration configuration, final WhitelistPersistor whitelistPersistor) { + this.whitelistPersistor = whitelistPersistor; if (configuration.isNodeWhitelistEnabled() && configuration.getNodeWhitelist() != null) { for (URI uri : configuration.getNodeWhitelist()) { nodesWhitelist.add(DefaultPeer.fromURI(uri)); @@ -45,45 +59,71 @@ public class NodeWhitelistController { return nodesWhitelist.remove(node); } - public NodesWhitelistResult addNodes(final List peers) { + public NodesWhitelistResult addNodes(final List peers) { final NodesWhitelistResult inputValidationResult = validInput(peers); if (inputValidationResult.result() != WhitelistOperationResult.SUCCESS) { return inputValidationResult; } - for (DefaultPeer peer : peers) { + for (Peer peer : peers) { if (nodesWhitelist.contains(peer)) { return new NodesWhitelistResult( WhitelistOperationResult.ERROR_EXISTING_ENTRY, String.format("Specified peer: %s already exists in whitelist.", peer.getId())); } } + + final List oldWhitelist = new ArrayList<>(this.nodesWhitelist); + peers.forEach(this::addNode); + try { + verifyConfigurationFileState(peerToEnodeURI(oldWhitelist)); + updateConfigurationFile(peerToEnodeURI(nodesWhitelist)); + verifyConfigurationFileState(peerToEnodeURI(nodesWhitelist)); + } catch (IOException e) { + revertState(oldWhitelist); + return new NodesWhitelistResult(WhitelistOperationResult.ERROR_WHITELIST_PERSIST_FAIL); + } catch (WhitelistFileSyncException e) { + return new NodesWhitelistResult(WhitelistOperationResult.ERROR_WHITELIST_FILE_SYNC); + } return new NodesWhitelistResult(WhitelistOperationResult.SUCCESS); } - private boolean peerListHasDuplicates(final List peers) { + private boolean peerListHasDuplicates(final List peers) { return !peers.stream().allMatch(new HashSet<>()::add); } - public NodesWhitelistResult removeNodes(final List peers) { + public NodesWhitelistResult removeNodes(final List peers) { final NodesWhitelistResult inputValidationResult = validInput(peers); if (inputValidationResult.result() != WhitelistOperationResult.SUCCESS) { return inputValidationResult; } - for (DefaultPeer peer : peers) { + for (Peer peer : peers) { if (!(nodesWhitelist.contains(peer))) { return new NodesWhitelistResult( WhitelistOperationResult.ERROR_ABSENT_ENTRY, String.format("Specified peer: %s does not exist in whitelist.", peer.getId())); } } + + final List oldWhitelist = new ArrayList<>(this.nodesWhitelist); + peers.forEach(this::removeNode); + try { + verifyConfigurationFileState(peerToEnodeURI(oldWhitelist)); + updateConfigurationFile(peerToEnodeURI(nodesWhitelist)); + verifyConfigurationFileState(peerToEnodeURI(nodesWhitelist)); + } catch (IOException e) { + revertState(oldWhitelist); + return new NodesWhitelistResult(WhitelistOperationResult.ERROR_WHITELIST_PERSIST_FAIL); + } catch (WhitelistFileSyncException e) { + return new NodesWhitelistResult(WhitelistOperationResult.ERROR_WHITELIST_FILE_SYNC); + } return new NodesWhitelistResult(WhitelistOperationResult.SUCCESS); } - private NodesWhitelistResult validInput(final List peers) { + private NodesWhitelistResult validInput(final List peers) { if (peers == null || peers.isEmpty()) { return new NodesWhitelistResult( WhitelistOperationResult.ERROR_EMPTY_ENTRY, String.format("Null/empty peers list")); @@ -98,6 +138,24 @@ public class NodeWhitelistController { return new NodesWhitelistResult(WhitelistOperationResult.SUCCESS); } + private void verifyConfigurationFileState(final Collection oldNodes) + throws IOException, WhitelistFileSyncException { + whitelistPersistor.verifyConfigFileMatchesState( + WhitelistPersistor.WHITELIST_TYPE.NODES, oldNodes); + } + + private void updateConfigurationFile(final Collection nodes) throws IOException { + whitelistPersistor.updateConfig(WhitelistPersistor.WHITELIST_TYPE.NODES, nodes); + } + + private void revertState(final List nodesWhitelist) { + this.nodesWhitelist = nodesWhitelist; + } + + private Collection peerToEnodeURI(final Collection peers) { + return peers.parallelStream().map(Peer::getEnodeURI).collect(Collectors.toList()); + } + public boolean isPermitted(final Peer node) { return nodesWhitelist.stream() .anyMatch( diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java index cf99a78c75..38e8f01830 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java @@ -45,6 +45,7 @@ import tech.pegasys.pantheon.metrics.noop.NoOpMetricsSystem; import tech.pegasys.pantheon.util.bytes.BytesValue; import java.net.InetAddress; +import java.nio.file.Files; import java.util.List; import java.util.Optional; import java.util.OptionalInt; @@ -410,12 +411,12 @@ public final class NettyP2PNetworkTest { final BytesValue localId = localKp.getPublicKey().getEncodedBytes(); final PeerBlacklist localBlacklist = new PeerBlacklist(); final PeerBlacklist remoteBlacklist = new PeerBlacklist(); - final Optional config = - Optional.ofNullable(PermissioningConfiguration.createDefault()); - final Optional localWhitelistController = - Optional.of(new NodeWhitelistController(config.get())); + final PermissioningConfiguration config = PermissioningConfiguration.createDefault(); + config.setConfigurationFilePath( + Files.createTempFile("test", "test").toAbsolutePath().toString()); + final NodeWhitelistController localWhitelistController = new NodeWhitelistController(config); // turn on whitelisting by adding a different node NOT remote node - localWhitelistController.ifPresent(nwc -> nwc.addNode(mockPeer())); + localWhitelistController.addNode(mockPeer()); final SubProtocol subprotocol = subProtocol(); final Capability cap = Capability.create(subprotocol.getName(), 63); @@ -431,7 +432,7 @@ public final class NettyP2PNetworkTest { () -> false, localBlacklist, new NoOpMetricsSystem(), - localWhitelistController); + Optional.of(localWhitelistController)); final P2PNetwork remoteNetwork = new NettyP2PNetwork( vertx, diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java index 5e38ae730b..125667fb6b 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java @@ -44,6 +44,8 @@ import tech.pegasys.pantheon.util.bytes.MutableBytesValue; import tech.pegasys.pantheon.util.uint.UInt256; import tech.pegasys.pantheon.util.uint.UInt256Value; +import java.io.IOException; +import java.nio.file.Files; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -874,14 +876,14 @@ public class PeerDiscoveryControllerTest { } @Test - public void shouldNotBondWithNonWhitelistedPeer() { + public void shouldNotBondWithNonWhitelistedPeer() throws IOException { final List peers = createPeersInLastBucket(localPeer, 3); final DiscoveryPeer discoPeer = peers.get(0); final DiscoveryPeer otherPeer = peers.get(1); final DiscoveryPeer otherPeer2 = peers.get(2); final PeerBlacklist blacklist = new PeerBlacklist(); - final PermissioningConfiguration config = new PermissioningConfiguration(); + final PermissioningConfiguration config = permissioningConfigurationWithTempFile(); NodeWhitelistController nodeWhitelistController = new NodeWhitelistController(config); // Whitelist peers @@ -893,7 +895,7 @@ public class PeerDiscoveryControllerTest { getControllerBuilder() .peers(discoPeer) .blacklist(blacklist) - .whitelist(Optional.of(nodeWhitelistController)) + .whitelist(nodeWhitelistController) .outboundMessageHandler(outboundMessageHandler) .build(); @@ -942,17 +944,16 @@ public class PeerDiscoveryControllerTest { } @Test - public void shouldNotRespondToPingFromNonWhitelistedDiscoveryPeer() { + public void shouldNotRespondToPingFromNonWhitelistedDiscoveryPeer() throws IOException { final List peers = createPeersInLastBucket(localPeer, 3); final DiscoveryPeer discoPeer = peers.get(0); final PeerBlacklist blacklist = new PeerBlacklist(); // don't add disco peer to whitelist - PermissioningConfiguration config = PermissioningConfiguration.createDefault(); + PermissioningConfiguration config = permissioningConfigurationWithTempFile(); config.setNodeWhitelist(new ArrayList<>()); - Optional nodeWhitelistController = - Optional.of(new NodeWhitelistController(config)); + NodeWhitelistController nodeWhitelistController = new NodeWhitelistController(config); controller = getControllerBuilder() @@ -1025,6 +1026,13 @@ public class PeerDiscoveryControllerTest { return controller; } + private PermissioningConfiguration permissioningConfigurationWithTempFile() throws IOException { + final PermissioningConfiguration config = PermissioningConfiguration.createDefault(); + config.setConfigurationFilePath( + Files.createTempFile("test", "test").toAbsolutePath().toString()); + return config; + } + static class ControllerBuilder { private Collection discoPeers = Collections.emptyList(); private PeerBlacklist blacklist = new PeerBlacklist(); @@ -1055,8 +1063,8 @@ public class PeerDiscoveryControllerTest { return this; } - ControllerBuilder whitelist(final Optional whitelist) { - this.whitelist = whitelist; + ControllerBuilder whitelist(final NodeWhitelistController whitelist) { + this.whitelist = Optional.of(whitelist); return this; } 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 839ba67583..962c3fea2d 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 @@ -12,27 +12,39 @@ */ package tech.pegasys.pantheon.ethereum.p2p.permissioning; +import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static tech.pegasys.pantheon.ethereum.p2p.permissioning.NodeWhitelistController.NodesWhitelistResult; 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.WhitelistFileSyncException; import tech.pegasys.pantheon.ethereum.permissioning.WhitelistOperationResult; +import tech.pegasys.pantheon.ethereum.permissioning.WhitelistPersistor; import tech.pegasys.pantheon.util.bytes.BytesValue; +import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import com.google.common.collect.Lists; 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 NodeWhitelistControllerTest { + @Mock private WhitelistPersistor whitelistPersistor; private NodeWhitelistController controller; private final String enode1 = @@ -42,7 +54,8 @@ public class NodeWhitelistControllerTest { @Before public void setUp() { - controller = new NodeWhitelistController(new PermissioningConfiguration()); + controller = + new NodeWhitelistController(PermissioningConfiguration.createDefault(), whitelistPersistor); } @Test @@ -230,4 +243,26 @@ public class NodeWhitelistControllerTest { assertThat(controller.isPermitted(peerWithTcpPortSet)).isFalse(); } + + @Test + public void stateShouldRevertIfWhitelistPersistFails() + throws IOException, WhitelistFileSyncException { + List newNode1 = singletonList(DefaultPeer.fromURI(enode1)); + List newNode2 = singletonList(DefaultPeer.fromURI(enode2)); + + assertThat(controller.getNodesWhitelist().size()).isEqualTo(0); + + controller.addNodes(newNode1); + assertThat(controller.getNodesWhitelist().size()).isEqualTo(1); + + doThrow(new IOException()).when(whitelistPersistor).updateConfig(any(), any()); + controller.addNodes(newNode2); + + assertThat(controller.getNodesWhitelist().size()).isEqualTo(1); + assertThat(controller.getNodesWhitelist()).isEqualTo(newNode1); + + verify(whitelistPersistor, times(3)).verifyConfigFileMatchesState(any(), any()); + verify(whitelistPersistor, times(2)).updateConfig(any(), any()); + verifyNoMoreInteractions(whitelistPersistor); + } } diff --git a/ethereum/permissioning/build.gradle b/ethereum/permissioning/build.gradle index 1d97ce6000..620f3bfb7d 100644 --- a/ethereum/permissioning/build.gradle +++ b/ethereum/permissioning/build.gradle @@ -27,6 +27,8 @@ jar { dependencies { implementation project(':util') + implementation 'com.google.guava:guava' + implementation 'net.consensys.cava:cava-toml' 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 e1405aa69b..2516922e52 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 @@ -14,16 +14,25 @@ package tech.pegasys.pantheon.ethereum.permissioning; import tech.pegasys.pantheon.util.bytes.BytesValue; +import java.io.IOException; import java.util.ArrayList; +import java.util.Collection; import java.util.HashSet; import java.util.List; public class AccountWhitelistController { private static final int ACCOUNT_BYTES_SIZE = 20; - private final List accountWhitelist = new ArrayList<>(); + private List accountWhitelist = new ArrayList<>(); + private final WhitelistPersistor whitelistPersistor; public AccountWhitelistController(final PermissioningConfiguration configuration) { + this(configuration, new WhitelistPersistor(configuration.getConfigurationFilePath())); + } + + public AccountWhitelistController( + final PermissioningConfiguration configuration, final WhitelistPersistor whitelistPersistor) { + this.whitelistPersistor = whitelistPersistor; if (configuration != null && configuration.isAccountWhitelistEnabled()) { if (!configuration.getAccountWhitelist().isEmpty()) { addAccounts(configuration.getAccountWhitelist()); @@ -42,7 +51,18 @@ public class AccountWhitelistController { return WhitelistOperationResult.ERROR_EXISTING_ENTRY; } + final List oldWhitelist = new ArrayList<>(this.accountWhitelist); this.accountWhitelist.addAll(accounts); + try { + verifyConfigurationFileState(oldWhitelist); + updateConfigurationFile(accountWhitelist); + verifyConfigurationFileState(accountWhitelist); + } catch (IOException e) { + revertState(oldWhitelist); + return WhitelistOperationResult.ERROR_WHITELIST_PERSIST_FAIL; + } catch (WhitelistFileSyncException e) { + return WhitelistOperationResult.ERROR_WHITELIST_FILE_SYNC; + } return WhitelistOperationResult.SUCCESS; } @@ -56,7 +76,19 @@ public class AccountWhitelistController { return WhitelistOperationResult.ERROR_ABSENT_ENTRY; } + final List oldWhitelist = new ArrayList<>(this.accountWhitelist); + this.accountWhitelist.removeAll(accounts); + try { + verifyConfigurationFileState(oldWhitelist); + updateConfigurationFile(accountWhitelist); + verifyConfigurationFileState(accountWhitelist); + } catch (IOException e) { + revertState(oldWhitelist); + return WhitelistOperationResult.ERROR_WHITELIST_PERSIST_FAIL; + } catch (WhitelistFileSyncException e) { + return WhitelistOperationResult.ERROR_WHITELIST_FILE_SYNC; + } return WhitelistOperationResult.SUCCESS; } @@ -76,6 +108,20 @@ public class AccountWhitelistController { return WhitelistOperationResult.SUCCESS; } + private void verifyConfigurationFileState(final Collection oldAccounts) + throws IOException, WhitelistFileSyncException { + whitelistPersistor.verifyConfigFileMatchesState( + WhitelistPersistor.WHITELIST_TYPE.ACCOUNTS, oldAccounts); + } + + private void updateConfigurationFile(final Collection accounts) throws IOException { + whitelistPersistor.updateConfig(WhitelistPersistor.WHITELIST_TYPE.ACCOUNTS, accounts); + } + + private void revertState(final List accountWhitelist) { + this.accountWhitelist = accountWhitelist; + } + private boolean inputHasDuplicates(final List accounts) { return !accounts.stream().allMatch(new HashSet<>()::add); } diff --git a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistFileSyncException.java b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistFileSyncException.java new file mode 100644 index 0000000000..d4a6eff22a --- /dev/null +++ b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistFileSyncException.java @@ -0,0 +1,15 @@ +/* + * 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.permissioning; + +public class WhitelistFileSyncException extends Exception {} diff --git a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistOperationResult.java b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistOperationResult.java index 6818120ba4..5063dc373c 100644 --- a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistOperationResult.java +++ b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistOperationResult.java @@ -18,5 +18,7 @@ public enum WhitelistOperationResult { ERROR_EMPTY_ENTRY, ERROR_EXISTING_ENTRY, ERROR_INVALID_ENTRY, - ERROR_ABSENT_ENTRY + ERROR_ABSENT_ENTRY, + ERROR_WHITELIST_PERSIST_FAIL, + ERROR_WHITELIST_FILE_SYNC } diff --git a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistPersistor.java b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistPersistor.java new file mode 100644 index 0000000000..9450689f12 --- /dev/null +++ b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistPersistor.java @@ -0,0 +1,152 @@ +/* + * 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.permissioning; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.AbstractMap; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Charsets; +import net.consensys.cava.toml.Toml; +import net.consensys.cava.toml.TomlParseResult; + +public class WhitelistPersistor { + + private File configurationFile; + + public enum WHITELIST_TYPE { + ACCOUNTS("accounts-whitelist"), + NODES("nodes-whitelist"); + + private String tomlKey; + + WHITELIST_TYPE(final String tomlKey) { + this.tomlKey = tomlKey; + } + + public String getTomlKey() { + return tomlKey; + } + } + + public WhitelistPersistor(final String configurationFile) { + this.configurationFile = new File(configurationFile); + } + + public static boolean verifyConfigFileMatchesState( + final WHITELIST_TYPE whitelistType, + final Collection checkLists, + final Path configurationFilePath) + throws IOException, WhitelistFileSyncException { + boolean listsMatch = + new HashSet<>(existingConfigItems(configurationFilePath).get(whitelistType)) + .equals(new HashSet<>(checkLists)); + if (!listsMatch) { + throw new WhitelistFileSyncException(); + } + return listsMatch; + } + + public boolean verifyConfigFileMatchesState( + final WHITELIST_TYPE whitelistType, final Collection checkLists) + throws IOException, WhitelistFileSyncException { + return verifyConfigFileMatchesState(whitelistType, checkLists, configurationFile.toPath()); + } + + public synchronized void updateConfig( + final WHITELIST_TYPE whitelistType, final Collection updatedWhitelistValues) + throws IOException { + removeExistingConfigItem(whitelistType); + addNewConfigItem(whitelistType, updatedWhitelistValues); + } + + private static Map> existingConfigItems( + final Path configurationFilePath) throws IOException { + TomlParseResult parsedToml = Toml.parse(configurationFilePath); + + return Arrays.stream(WHITELIST_TYPE.values()) + .map( + whitelist_type -> + new AbstractMap.SimpleImmutableEntry<>( + whitelist_type, parsedToml.getArrayOrEmpty(whitelist_type.getTomlKey()))) + .collect( + Collectors.toMap( + o -> o.getKey(), + o -> + o.getValue() + .toList() + .parallelStream() + .map(Object::toString) + .collect(Collectors.toList()))); + } + + @VisibleForTesting + void removeExistingConfigItem(final WHITELIST_TYPE whitelistType) throws IOException { + List otherConfigItems = + existingConfigItems(configurationFile.toPath()) + .entrySet() + .parallelStream() + .filter(listType -> !listType.getKey().equals(whitelistType)) + .map(keyVal -> valueListToTomlArray(keyVal.getKey(), keyVal.getValue())) + .collect(Collectors.toList()); + + Files.write( + configurationFile.toPath(), + otherConfigItems, + StandardOpenOption.WRITE, + StandardOpenOption.TRUNCATE_EXISTING); + } + + @VisibleForTesting + public static void addNewConfigItem( + final WHITELIST_TYPE whitelistType, + final Collection whitelistValues, + final Path configFilePath) + throws IOException { + String newConfigItem = valueListToTomlArray(whitelistType, whitelistValues); + + Files.write( + configFilePath, + newConfigItem.getBytes(Charsets.UTF_8), + StandardOpenOption.WRITE, + StandardOpenOption.APPEND); + } + + @VisibleForTesting + void addNewConfigItem( + final WHITELIST_TYPE whitelistType, final Collection whitelistValues) + throws IOException { + addNewConfigItem(whitelistType, whitelistValues, configurationFile.toPath()); + } + + private static String valueListToTomlArray( + final WHITELIST_TYPE whitelistType, final Collection whitelistValues) { + return String.format( + "%s=[%s]", + whitelistType.getTomlKey(), + whitelistValues + .parallelStream() + .map(uri -> String.format("\"%s\"", uri)) + .collect(Collectors.joining(","))); + } +} 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 ae3c4e00d5..820e52d749 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 @@ -12,11 +12,19 @@ */ package tech.pegasys.pantheon.ethereum.permissioning; +import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doThrow; +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 java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import org.junit.Before; import org.junit.Test; @@ -29,18 +37,19 @@ public class AccountWhitelistControllerTest { private AccountWhitelistController controller; @Mock private PermissioningConfiguration permissioningConfig; + @Mock private WhitelistPersistor whitelistPersistor; @Before public void before() { - controller = new AccountWhitelistController(permissioningConfig); + controller = new AccountWhitelistController(permissioningConfig, whitelistPersistor); } @Test public void whenPermConfigHasAccountsShouldAddAllAccountsToWhitelist() { when(permissioningConfig.isAccountWhitelistEnabled()).thenReturn(true); when(permissioningConfig.getAccountWhitelist()) - .thenReturn(Arrays.asList("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73")); - controller = new AccountWhitelistController(permissioningConfig); + .thenReturn(singletonList("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73")); + controller = new AccountWhitelistController(permissioningConfig, whitelistPersistor); assertThat(controller.getAccountWhitelist()) .contains("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73"); @@ -50,7 +59,7 @@ public class AccountWhitelistControllerTest { public void whenPermConfigContainsEmptyListOfAccountsContainsShouldReturnFalse() { when(permissioningConfig.isAccountWhitelistEnabled()).thenReturn(true); when(permissioningConfig.getAccountWhitelist()).thenReturn(new ArrayList<>()); - controller = new AccountWhitelistController(permissioningConfig); + controller = new AccountWhitelistController(permissioningConfig, whitelistPersistor); assertThat(controller.contains("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73")).isFalse(); } @@ -147,4 +156,26 @@ public class AccountWhitelistControllerTest { assertThat(removeResult).isEqualTo(WhitelistOperationResult.ERROR_EMPTY_ENTRY); } + + @Test + public void stateShouldRevertIfWhitelistPersistFails() + throws IOException, WhitelistFileSyncException { + List newAccount = singletonList("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73"); + List newAccount2 = singletonList("0xfe3b557e8fb62b89f4916b721be55ceb828dbd72"); + + assertThat(controller.getAccountWhitelist().size()).isEqualTo(0); + + controller.addAccounts(newAccount); + assertThat(controller.getAccountWhitelist().size()).isEqualTo(1); + + doThrow(new IOException()).when(whitelistPersistor).updateConfig(any(), any()); + controller.addAccounts(newAccount2); + + assertThat(controller.getAccountWhitelist().size()).isEqualTo(1); + assertThat(controller.getAccountWhitelist()).isEqualTo(newAccount); + + verify(whitelistPersistor, times(3)).verifyConfigFileMatchesState(any(), any()); + verify(whitelistPersistor, times(2)).updateConfig(any(), any()); + verifyNoMoreInteractions(whitelistPersistor); + } } diff --git a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistPersistorTest.java b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistPersistorTest.java new file mode 100644 index 0000000000..a3b2a0fdfa --- /dev/null +++ b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/WhitelistPersistorTest.java @@ -0,0 +1,140 @@ +/* + * 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.permissioning; + +import static org.assertj.core.api.Assertions.assertThat; + +import tech.pegasys.pantheon.ethereum.permissioning.WhitelistPersistor.WHITELIST_TYPE; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.StandardOpenOption; +import java.util.Collections; +import java.util.List; +import java.util.Set; +import java.util.stream.Stream; + +import com.google.common.collect.Lists; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +public class WhitelistPersistorTest { + + private WhitelistPersistor whitelistPersistor; + private File tempFile; + private final String accountsWhitelist = + String.format("%s=[%s]", WHITELIST_TYPE.ACCOUNTS.getTomlKey(), "\"account1\",\"account2\""); + private final String nodesWhitelist = + String.format("%s=[%s]", WHITELIST_TYPE.NODES.getTomlKey(), "\"node1\",\"node2\""); + + @Before + public void setUp() throws IOException { + List lines = Lists.newArrayList(nodesWhitelist, accountsWhitelist); + tempFile = File.createTempFile("test", "test"); + Files.write(tempFile.toPath(), lines, StandardOpenOption.WRITE, StandardOpenOption.CREATE); + whitelistPersistor = new WhitelistPersistor(tempFile.getAbsolutePath()); + } + + @Test + public void lineShouldBeRemoved() throws IOException { + WHITELIST_TYPE keyForRemoval = WHITELIST_TYPE.ACCOUNTS; + + assertThat(countLines()).isEqualTo(2); + assertThat(hasKey(keyForRemoval)).isTrue(); + + whitelistPersistor.removeExistingConfigItem(keyForRemoval); + + assertThat(countLines()).isEqualTo(1); + assertThat(hasKey(keyForRemoval)).isFalse(); + } + + @Test + public void lineShouldBeAdded() throws IOException { + final WHITELIST_TYPE key = WHITELIST_TYPE.NODES; + final Set updatedWhitelist = Collections.singleton("node5"); + + assertThat(countLines()).isEqualTo(2); + assertThat(hasKey(key)).isTrue(); + + whitelistPersistor.removeExistingConfigItem(WHITELIST_TYPE.NODES); + + assertThat(countLines()).isEqualTo(1); + assertThat(hasKey(key)).isFalse(); + + whitelistPersistor.addNewConfigItem(key, updatedWhitelist); + + assertThat(countLines()).isEqualTo(2); + assertThat(hasKey(key)).isTrue(); + } + + @Test + public void lineShouldBeReplaced() throws IOException { + WHITELIST_TYPE key = WHITELIST_TYPE.NODES; + String newValue = "node5"; + + assertThat(countLines()).isEqualTo(2); + assertThat(hasKeyAndExactLineContent(key, nodesWhitelist)).isTrue(); + + whitelistPersistor.updateConfig(key, Collections.singleton(newValue)); + + assertThat(countLines()).isEqualTo(2); + assertThat(hasKeyAndContainsValue(key, newValue)).isTrue(); + assertThat(hasKeyAndExactLineContent(key, nodesWhitelist)).isFalse(); + } + + @Test + public void outputIsValidTOML() throws IOException { + WHITELIST_TYPE key = WHITELIST_TYPE.ACCOUNTS; + List newValue = Lists.newArrayList("account5", "account6", "account4"); + String expectedValue = + String.format("%s=[%s]", "accounts-whitelist", "\"account5\",\"account6\",\"account4\""); + + whitelistPersistor.updateConfig(key, newValue); + + assertThat(hasKey(key)).isTrue(); + assertThat(hasKeyAndExactLineContent(key, expectedValue)).isTrue(); + } + + @After + public void tearDown() { + tempFile.delete(); + } + + private long countLines() throws IOException { + try (Stream lines = Files.lines(tempFile.toPath())) { + return lines.count(); + } + } + + private boolean hasKey(final WHITELIST_TYPE key) throws IOException { + try (Stream lines = Files.lines(tempFile.toPath())) { + return lines.anyMatch(s -> s.startsWith(key.getTomlKey())); + } + } + + private boolean hasKeyAndContainsValue(final WHITELIST_TYPE key, final String value) + throws IOException { + try (Stream lines = Files.lines(tempFile.toPath())) { + return lines.anyMatch(s -> s.startsWith(key.getTomlKey()) && s.contains(value)); + } + } + + private boolean hasKeyAndExactLineContent(final WHITELIST_TYPE key, final String exactKeyValue) + throws IOException { + try (Stream lines = Files.lines(tempFile.toPath())) { + return lines.anyMatch(s -> s.startsWith(key.getTomlKey()) && s.equals(exactKeyValue)); + } + } +} diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/PermissioningConfigurationBuilder.java b/pantheon/src/main/java/tech/pegasys/pantheon/PermissioningConfigurationBuilder.java index 7e86f70fd6..22e41ff2f8 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/PermissioningConfigurationBuilder.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/PermissioningConfigurationBuilder.java @@ -12,18 +12,13 @@ */ package tech.pegasys.pantheon; -import static java.nio.charset.StandardCharsets.UTF_8; - import tech.pegasys.pantheon.cli.custom.EnodeToURIPropertyConverter; import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfiguration; -import java.io.File; -import java.io.FileNotFoundException; import java.net.URI; import java.util.List; import java.util.stream.Collectors; -import com.google.common.io.Resources; import net.consensys.cava.toml.TomlArray; import net.consensys.cava.toml.TomlParseResult; @@ -50,12 +45,9 @@ public class PermissioningConfigurationBuilder { throws Exception { TomlParseResult permToml; - boolean foundValidOptions = false; try { - permToml = - TomlConfigFileParser.loadConfiguration( - permissioningConfigTomlAsString(permissioningConfigFile(configFilePath))); + permToml = TomlConfigFileParser.loadConfigurationFromFile(configFilePath); } catch (Exception e) { throw new Exception( "Unable to read permissions TOML config file : " + configFilePath + " " + e.getMessage()); @@ -70,7 +62,9 @@ public class PermissioningConfigurationBuilder { if (permissionedAccountEnabled) { if (accountWhitelistTomlArray != null) { List accountsWhitelistToml = - accountWhitelistTomlArray.toList().stream() + accountWhitelistTomlArray + .toList() + .parallelStream() .map(Object::toString) .collect(Collectors.toList()); permissioningConfiguration.setAccountWhitelist(accountsWhitelistToml); @@ -79,10 +73,13 @@ public class PermissioningConfigurationBuilder { ACCOUNTS_WHITELIST + " config option missing in TOML config file " + configFilePath); } } + if (permissionedNodeEnabled) { if (nodeWhitelistTomlArray != null) { List nodesWhitelistToml = - nodeWhitelistTomlArray.toList().stream() + nodeWhitelistTomlArray + .toList() + .parallelStream() .map(Object::toString) .map(EnodeToURIPropertyConverter::convertToURI) .collect(Collectors.toList()); @@ -94,19 +91,4 @@ public class PermissioningConfigurationBuilder { } return permissioningConfiguration; } - - private static String permissioningConfigTomlAsString(final File file) throws Exception { - return Resources.toString(file.toURI().toURL(), UTF_8); - } - - private static File permissioningConfigFile(final String filename) throws FileNotFoundException { - - final File permissioningConfigFile = new File(filename); - if (permissioningConfigFile.exists()) { - return permissioningConfigFile; - } else { - throw new FileNotFoundException( - "File does not exist: permissioning config path: " + filename); - } - } } diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/TomlConfigFileParser.java b/pantheon/src/main/java/tech/pegasys/pantheon/TomlConfigFileParser.java index b24d84f288..338be03e57 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/TomlConfigFileParser.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/TomlConfigFileParser.java @@ -12,14 +12,23 @@ */ package tech.pegasys.pantheon; +import static java.nio.charset.StandardCharsets.UTF_8; + +import java.io.File; +import java.io.FileNotFoundException; import java.util.stream.Collectors; +import com.google.common.io.Resources; import net.consensys.cava.toml.Toml; import net.consensys.cava.toml.TomlParseError; import net.consensys.cava.toml.TomlParseResult; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; public class TomlConfigFileParser { + protected static final Logger LOG = LogManager.getLogger(); + private static TomlParseResult checkConfigurationValidity( final TomlParseResult result, final String toml) throws Exception { if (result == null || result.isEmpty()) { @@ -34,10 +43,36 @@ public class TomlConfigFileParser { if (result.hasErrors()) { final String errors = result.errors().stream().map(TomlParseError::toString).collect(Collectors.joining("%n")); - ; throw new Exception("Invalid TOML configuration : " + errors); } return checkConfigurationValidity(result, toml); } + + public static TomlParseResult loadConfigurationFromFile(final String configFilePath) + throws Exception { + return loadConfiguration(configTomlAsString(tomlConfigFile(configFilePath))); + } + + private static String configTomlAsString(final File file) throws Exception { + return Resources.toString(file.toURI().toURL(), UTF_8); + } + + private static File tomlConfigFile(final String filename) throws Exception { + final File tomlConfigFile = new File(filename); + if (tomlConfigFile.exists()) { + if (!tomlConfigFile.canRead()) { + throw new Exception(String.format("Read access denied for file at: %s", filename)); + } + if (!tomlConfigFile.canWrite()) { + LOG.warn( + "Write access denied for file at: %s. Configuration modification operations will not be permitted.", + filename); + } + return tomlConfigFile; + } else { + throw new FileNotFoundException( + String.format("Configuration file does not exist: %s", filename)); + } + } } diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/PermissioningConfigurationBuilderTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/PermissioningConfigurationBuilderTest.java index 600040e420..9baef1ed08 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/PermissioningConfigurationBuilderTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/PermissioningConfigurationBuilderTest.java @@ -40,6 +40,8 @@ public class PermissioningConfigurationBuilderTest { "permissioning_config_absent_whitelists.toml"; static final String PERMISSIONING_CONFIG_UNRECOGNIZED_KEY = "permissioning_config_unrecognized_key.toml"; + static final String PERMISSIONING_CONFIG_NODE_WHITELIST_ONLY_MULTILINE = + "permissioning_config_node_whitelist_only_multiline.toml"; private final String VALID_NODE_ID = "6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"; @@ -206,7 +208,19 @@ public class PermissioningConfigurationBuilderTest { "file-does-not-exist", true, true); fail("expected exception: file does not exist"); } catch (Exception e) { - assertThat(e.getMessage().contains("File does not exist")).isTrue(); + assertThat(e.getMessage().contains("Configuration file does not exist")).isTrue(); } } + + @Test + public void permissioningConfigFromMultilineFileMustParseCorrectly() throws Exception { + final URL configFile = + Resources.getResource(PERMISSIONING_CONFIG_NODE_WHITELIST_ONLY_MULTILINE); + final PermissioningConfiguration permissioningConfiguration = + PermissioningConfigurationBuilder.permissioningConfigurationFromToml( + configFile.getPath(), true, false); + + assertThat(permissioningConfiguration.isNodeWhitelistEnabled()).isTrue(); + assertThat(permissioningConfiguration.getNodeWhitelist().size()).isEqualTo(5); + } } diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java index c498152a7f..9db9e7e501 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java @@ -332,7 +332,7 @@ public class PantheonCommandTest extends CommandTestAbstract { verifyZeroInteractions(mockRunnerBuilder); - assertThat(commandErrorOutput.toString()).contains("File does not exist"); + assertThat(commandErrorOutput.toString()).contains("Configuration file does not exist"); assertThat(commandOutput.toString()).isEmpty(); } diff --git a/pantheon/src/test/resources/permissioning_config_node_whitelist_only_multiline.toml b/pantheon/src/test/resources/permissioning_config_node_whitelist_only_multiline.toml new file mode 100644 index 0000000000..92af73ddd7 --- /dev/null +++ b/pantheon/src/test/resources/permissioning_config_node_whitelist_only_multiline.toml @@ -0,0 +1,9 @@ +# Permissioning TOML file (node whitelist only) + +nodes-whitelist=[ +"enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.1:4567", +"enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.2:4567", +"enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.3:4567", +"enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.4:4567", +"enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.5:4567", +]