From f04c726700fcc987ed71136e3899a38eb8d9386f Mon Sep 17 00:00:00 2001 From: toktar Date: Mon, 5 Mar 2018 10:34:50 +0300 Subject: [PATCH 1/5] INDY-1148 - It's possible to create several nodes with the same alias - add dynamic validation of alias and test Signed-off-by: toktar --- plenum/server/pool_req_handler.py | 8 ++++++++ .../pool_transactions/test_node_adding.py | 20 +++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 plenum/test/pool_transactions/test_node_adding.py diff --git a/plenum/server/pool_req_handler.py b/plenum/server/pool_req_handler.py index 51da5c19ba..7fb8282208 100644 --- a/plenum/server/pool_req_handler.py +++ b/plenum/server/pool_req_handler.py @@ -83,6 +83,13 @@ def authErrorWhileAddingNode(self, request): if self.isNodeDataConflicting(operation.get(DATA, {})): return "existing data has conflicts with " \ "request data {}".format(operation.get(DATA)) + nodeNym = request.operation.get(TARGET_NYM) + nodeAlias = request.operation.get(DATA).get(ALIAS) + for curNodeNym, curNodeData in self.state.as_dict.items(): + curNodeData = self.stateSerializer.deserialize(curNodeData) + if nodeAlias == curNodeData.get( + ALIAS) and nodeNym.encode() != curNodeNym: + return "Alias " + curNodeData.get(ALIAS) + " already exists" def authErrorWhileUpdatingNode(self, request): # Check if steward of the node is updating it and its data does not @@ -203,3 +210,4 @@ def dataErrorWhileValidatingUpdate(self, data, nodeNym): if self.isNodeDataConflicting(data, nodeNym): return "existing data has conflicts with " \ "request data {}".format(data) + diff --git a/plenum/test/pool_transactions/test_node_adding.py b/plenum/test/pool_transactions/test_node_adding.py new file mode 100644 index 0000000000..d261dc4e0a --- /dev/null +++ b/plenum/test/pool_transactions/test_node_adding.py @@ -0,0 +1,20 @@ +from plenum.common.constants import STEWARD +from plenum.test.pool_transactions.helper import sendAddNewNode, addNewClient +from plenum.test.helper import waitRejectWithReason + + +def test_add_node_with_not_unique_alias(looper, + tdir, + tconf, + txnPoolNodeSet, + steward1, + stewardWallet): + newNodeName = "Alpha" + newSteward = addNewClient(STEWARD, looper, steward1, stewardWallet, + "testStewardMy") + sendAddNewNode(tdir, tconf, newNodeName, steward1, newSteward) + + for node in txnPoolNodeSet: + waitRejectWithReason(looper, steward1, + "Alias " + newNodeName + " already exists", + node.clientstack.name) From 859ba15881d92d7a4ef2891dc06ad27b1efd5135 Mon Sep 17 00:00:00 2001 From: toktar Date: Mon, 5 Mar 2018 13:24:16 +0300 Subject: [PATCH 2/5] INDY-1148 - It's possible to create several nodes with the same alias - put alias validation into PoolRequestHandler.isNodeDataConflicting Signed-off-by: toktar --- plenum/server/pool_req_handler.py | 14 +++++--------- plenum/test/pool_transactions/test_node_adding.py | 12 +++++++++--- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/plenum/server/pool_req_handler.py b/plenum/server/pool_req_handler.py index 7fb8282208..b5b5f71ebf 100644 --- a/plenum/server/pool_req_handler.py +++ b/plenum/server/pool_req_handler.py @@ -80,16 +80,10 @@ def authErrorWhileAddingNode(self, request): origin) if self.stewardHasNode(origin): return "{} already has a node".format(origin) - if self.isNodeDataConflicting(operation.get(DATA, {})): + if self.isNodeDataConflicting( + operation.get(DATA, request.operation.get(TARGET_NYM))): return "existing data has conflicts with " \ "request data {}".format(operation.get(DATA)) - nodeNym = request.operation.get(TARGET_NYM) - nodeAlias = request.operation.get(DATA).get(ALIAS) - for curNodeNym, curNodeData in self.state.as_dict.items(): - curNodeData = self.stateSerializer.deserialize(curNodeData) - if nodeAlias == curNodeData.get( - ALIAS) and nodeNym.encode() != curNodeNym: - return "Alias " + curNodeData.get(ALIAS) + " already exists" def authErrorWhileUpdatingNode(self, request): # Check if steward of the node is updating it and its data does not @@ -198,6 +192,9 @@ def isNodeDataConflicting(self, data, nodeNym=None): if (not nodeData and len(bag) != 3) or ( nodeData and len(bag) != 6): return True + if nodeData.get(ALIAS) == otherNodeData.get( + ALIAS) and not nodeNym or otherNode != nodeNym: + return True def dataErrorWhileValidatingUpdate(self, data, nodeNym): error = self.dataErrorWhileValidating(data, skipKeys=True) @@ -210,4 +207,3 @@ def dataErrorWhileValidatingUpdate(self, data, nodeNym): if self.isNodeDataConflicting(data, nodeNym): return "existing data has conflicts with " \ "request data {}".format(data) - diff --git a/plenum/test/pool_transactions/test_node_adding.py b/plenum/test/pool_transactions/test_node_adding.py index d261dc4e0a..7c41c67aa6 100644 --- a/plenum/test/pool_transactions/test_node_adding.py +++ b/plenum/test/pool_transactions/test_node_adding.py @@ -1,4 +1,5 @@ -from plenum.common.constants import STEWARD +from plenum.common.constants import STEWARD, DATA +from plenum.common.request import Request from plenum.test.pool_transactions.helper import sendAddNewNode, addNewClient from plenum.test.helper import waitRejectWithReason @@ -12,9 +13,14 @@ def test_add_node_with_not_unique_alias(looper, newNodeName = "Alpha" newSteward = addNewClient(STEWARD, looper, steward1, stewardWallet, "testStewardMy") - sendAddNewNode(tdir, tconf, newNodeName, steward1, newSteward) + newNode = sendAddNewNode(tdir, tconf, newNodeName, steward1, newSteward) + data = {} + for item in newNode: + if isinstance(item, Request): + data = item.operation.get(DATA) for node in txnPoolNodeSet: waitRejectWithReason(looper, steward1, - "Alias " + newNodeName + " already exists", + "existing data has conflicts with " + + "request data {}".format(data), node.clientstack.name) From 366ca07e818c7a3c7318f6e1df8084d2cbcc3993 Mon Sep 17 00:00:00 2001 From: toktar Date: Mon, 5 Mar 2018 15:05:35 +0300 Subject: [PATCH 3/5] INDY-1148 - It's possible to create several nodes with the same alias - change validation in pool_req_handler. Signed-off-by: toktar --- plenum/server/pool_req_handler.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/plenum/server/pool_req_handler.py b/plenum/server/pool_req_handler.py index b5b5f71ebf..e55e1e5ced 100644 --- a/plenum/server/pool_req_handler.py +++ b/plenum/server/pool_req_handler.py @@ -80,8 +80,7 @@ def authErrorWhileAddingNode(self, request): origin) if self.stewardHasNode(origin): return "{} already has a node".format(origin) - if self.isNodeDataConflicting( - operation.get(DATA, request.operation.get(TARGET_NYM))): + if self.isNodeDataConflicting(data): return "existing data has conflicts with " \ "request data {}".format(operation.get(DATA)) @@ -157,14 +156,14 @@ def isNodeDataSame(self, nodeNym, newData, isCommitted=True): nodeInfo.pop(f.IDENTIFIER.nm, None) return nodeInfo == newData - def isNodeDataConflicting(self, data, nodeNym=None): + def isNodeDataConflicting(self, data, updatingNym=None): # Check if node's ALIAS or IPs or ports conflicts with other nodes, # also, the node is not allowed to change its alias. # Check ALIAS change nodeData = {} - if nodeNym: - nodeData = self.getNodeData(nodeNym, isCommitted=False) + if updatingNym: + nodeData = self.getNodeData(updatingNym, isCommitted=False) if nodeData.get(ALIAS) != data.get(ALIAS): return True else: @@ -178,7 +177,7 @@ def isNodeDataConflicting(self, data, nodeNym=None): otherNodeData = self.stateSerializer.deserialize(otherNodeData) otherNodeData.pop(f.IDENTIFIER.nm, None) otherNodeData.pop(SERVICES, None) - if not nodeNym or otherNode != nodeNym: + if not updatingNym or otherNode != updatingNym: # The node's ip, port and alias shuuld be unique bag = set() for d in (nodeData, otherNodeData): @@ -192,8 +191,8 @@ def isNodeDataConflicting(self, data, nodeNym=None): if (not nodeData and len(bag) != 3) or ( nodeData and len(bag) != 6): return True - if nodeData.get(ALIAS) == otherNodeData.get( - ALIAS) and not nodeNym or otherNode != nodeNym: + if data.get(ALIAS) == otherNodeData.get( + ALIAS) and not updatingNym: return True def dataErrorWhileValidatingUpdate(self, data, nodeNym): From e0f7db5d8a90179c5dc23b8c45f171e1cf3e8555 Mon Sep 17 00:00:00 2001 From: toktar Date: Tue, 6 Mar 2018 14:03:06 +0300 Subject: [PATCH 4/5] INDY-1148 : change test for validation nodes -deleted test_node_adding.py -test test_add_node_with_not_unique_alias move to test_nodes_with_pool_txns.py Signed-off-by: toktar --- .../pool_transactions/test_node_adding.py | 26 ------- .../test_nodes_with_pool_txns.py | 73 +++++++++++++++---- 2 files changed, 57 insertions(+), 42 deletions(-) delete mode 100644 plenum/test/pool_transactions/test_node_adding.py diff --git a/plenum/test/pool_transactions/test_node_adding.py b/plenum/test/pool_transactions/test_node_adding.py deleted file mode 100644 index 7c41c67aa6..0000000000 --- a/plenum/test/pool_transactions/test_node_adding.py +++ /dev/null @@ -1,26 +0,0 @@ -from plenum.common.constants import STEWARD, DATA -from plenum.common.request import Request -from plenum.test.pool_transactions.helper import sendAddNewNode, addNewClient -from plenum.test.helper import waitRejectWithReason - - -def test_add_node_with_not_unique_alias(looper, - tdir, - tconf, - txnPoolNodeSet, - steward1, - stewardWallet): - newNodeName = "Alpha" - newSteward = addNewClient(STEWARD, looper, steward1, stewardWallet, - "testStewardMy") - newNode = sendAddNewNode(tdir, tconf, newNodeName, steward1, newSteward) - data = {} - for item in newNode: - if isinstance(item, Request): - data = item.operation.get(DATA) - - for node in txnPoolNodeSet: - waitRejectWithReason(looper, steward1, - "existing data has conflicts with " + - "request data {}".format(data), - node.clientstack.name) diff --git a/plenum/test/pool_transactions/test_nodes_with_pool_txns.py b/plenum/test/pool_transactions/test_nodes_with_pool_txns.py index 64fff8c604..cc37b53827 100644 --- a/plenum/test/pool_transactions/test_nodes_with_pool_txns.py +++ b/plenum/test/pool_transactions/test_nodes_with_pool_txns.py @@ -4,7 +4,8 @@ import base58 import pytest -from plenum.common.exceptions import RequestRejectedException, RequestNackedException +from plenum.common.exceptions import RequestRejectedException, \ + RequestNackedException from plenum.test.node_request.helper import sdk_ensure_pool_functional from plenum.common.constants import DATA, TARGET_NYM, \ @@ -12,7 +13,8 @@ from plenum.common.signer_simple import SimpleSigner from plenum.common.util import getMaxFailures, randomString from plenum.test import waits -from plenum.test.helper import sdk_send_random_and_check, sdk_get_and_check_replies +from plenum.test.helper import sdk_send_random_and_check, \ + sdk_get_and_check_replies from plenum.test.pool_transactions.helper import sdk_add_new_node, \ sdk_add_2_nodes, sdk_pool_refresh, sdk_add_new_nym, prepare_new_node_data, \ prepare_node_request, sdk_sign_and_send_prepared_request @@ -27,7 +29,33 @@ # initialised a connection for a new node by the time the new node's message # reaches it -def testStewardCannotAddMoreThanOneNode(looper, txnPoolNodeSet, sdk_pool_handle, +def test_add_node_with_not_unique_alias(looper, + tdir, + tconf, + sdk_pool_handle, + sdk_wallet_steward, + allPluginsPath): + new_node_name = "Alpha" + new_steward_wallet, steward_did = sdk_add_new_nym(looper, + sdk_pool_handle, + sdk_wallet_steward, + alias="TEST_STEWARD1", + role='STEWARD') + with pytest.raises(RequestRejectedException) as e: + sdk_add_new_node(looper, + sdk_pool_handle, + (new_steward_wallet, steward_did), + new_node_name, + tdir, + tconf, + allPluginsPath) + assert 'existing data has conflicts with request data' in \ + e._excinfo[1].args[0] + sdk_pool_refresh(looper, sdk_pool_handle) + + +def testStewardCannotAddMoreThanOneNode(looper, txnPoolNodeSet, + sdk_pool_handle, sdk_wallet_steward, tdir, tconf, allPluginsPath): new_node_name = "Epsilon" @@ -69,7 +97,8 @@ def testClientConnectsToNewNode(looper, """ _, new_node = sdk_node_theta_added logger.debug("{} connected to the pool".format(new_node)) - sdk_send_random_and_check(looper, txnPoolNodeSet, sdk_pool_handle, sdk_wallet_client, 1) + sdk_send_random_and_check(looper, txnPoolNodeSet, sdk_pool_handle, + sdk_wallet_client, 1) def testAdd2NewNodes(looper, txnPoolNodeSet, @@ -112,7 +141,8 @@ def testStewardCannotAddNodeWithOutFullFieldsSet(looper, tdir, tconf, new_steward_wallet_handle = sdk_add_new_nym(looper, sdk_pool_handle, sdk_wallet_steward, - alias='New steward' + randomString(3), + alias='New steward' + randomString( + 3), role=STEWARD_STRING) sigseed, verkey, bls_key, nodeIp, nodePort, clientIp, clientPort = \ prepare_new_node_data(tconf, tdir, new_node_name) @@ -129,12 +159,15 @@ def testStewardCannotAddNodeWithOutFullFieldsSet(looper, tdir, tconf, # case from the ticket request_json = json.loads(node_request) - request_json['operation'][DATA][NODE_PORT + ' '] = request_json['operation'][DATA][NODE_PORT] + request_json['operation'][DATA][NODE_PORT + ' '] = \ + request_json['operation'][DATA][NODE_PORT] del request_json['operation'][DATA][NODE_PORT] node_request1 = json.dumps(request_json) - request_couple = sdk_sign_and_send_prepared_request(looper, new_steward_wallet_handle, - sdk_pool_handle, node_request1) + request_couple = sdk_sign_and_send_prepared_request(looper, + new_steward_wallet_handle, + sdk_pool_handle, + node_request1) with pytest.raises(RequestNackedException) as e: sdk_get_and_check_replies(looper, [request_couple]) assert 'unknown field' in e._excinfo[1].args[0] @@ -143,8 +176,10 @@ def testStewardCannotAddNodeWithOutFullFieldsSet(looper, tdir, tconf, request_json = json.loads(node_request) del request_json['operation'][DATA][fn] node_request2 = json.dumps(request_json) - request_couple = sdk_sign_and_send_prepared_request(looper, new_steward_wallet_handle, - sdk_pool_handle, node_request2) + request_couple = sdk_sign_and_send_prepared_request(looper, + new_steward_wallet_handle, + sdk_pool_handle, + node_request2) # wait NAcks with exact message. it does not works for just 'is missed' # because the 'is missed' will check only first few cases with pytest.raises(RequestNackedException) as e: @@ -156,8 +191,10 @@ def testNodesConnect(txnPoolNodeSet): pass -def testNodesReceiveClientMsgs(looper, txnPoolNodeSet, sdk_wallet_client, sdk_pool_handle): - sdk_ensure_pool_functional(looper, txnPoolNodeSet, sdk_wallet_client, sdk_pool_handle) +def testNodesReceiveClientMsgs(looper, txnPoolNodeSet, sdk_wallet_client, + sdk_pool_handle): + sdk_ensure_pool_functional(looper, txnPoolNodeSet, sdk_wallet_client, + sdk_pool_handle) def testAddNewClient(looper, txnPoolNodeSet, sdk_wallet_new_client): @@ -207,8 +244,10 @@ def testStewardCannotAddNodeWithNonBase58VerKey(looper, tdir, tconf, request_json['operation'][TARGET_NYM] = hexVerKey node_request = json.dumps(request_json) - request_couple = sdk_sign_and_send_prepared_request(looper, sdk_wallet_new_steward, - sdk_pool_handle, node_request) + request_couple = sdk_sign_and_send_prepared_request(looper, + sdk_wallet_new_steward, + sdk_pool_handle, + node_request) with pytest.raises(RequestNackedException) as e: sdk_get_and_check_replies(looper, [request_couple]) assert 'should not contain the following chars' in e._excinfo[1].args[0] @@ -254,8 +293,10 @@ def testStewardCannotAddNodeWithInvalidHa(looper, tdir, tconf, request_json = json.loads(node_request) request_json['operation'][DATA][field] = value node_request1 = json.dumps(request_json) - request_couple = sdk_sign_and_send_prepared_request(looper, sdk_wallet_new_steward, - sdk_pool_handle, node_request1) + request_couple = sdk_sign_and_send_prepared_request(looper, + sdk_wallet_new_steward, + sdk_pool_handle, + node_request1) # wait NAcks with exact message. it does not works for just 'is invalid' # because the 'is invalid' will check only first few cases with pytest.raises(RequestNackedException) as e: From 4cf503b194ae095bed4b582be82eb5707688cd1f Mon Sep 17 00:00:00 2001 From: toktar Date: Tue, 6 Mar 2018 14:53:06 +0300 Subject: [PATCH 5/5] INDY-1148 : move test for validate node alias -move test test_add_node_with_not_unique_alias to the end of test_nodes_with_pool_txns.py Signed-off-by: toktar --- .../test_nodes_with_pool_txns.py | 48 ++++++++++--------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/plenum/test/pool_transactions/test_nodes_with_pool_txns.py b/plenum/test/pool_transactions/test_nodes_with_pool_txns.py index cc37b53827..4409b53431 100644 --- a/plenum/test/pool_transactions/test_nodes_with_pool_txns.py +++ b/plenum/test/pool_transactions/test_nodes_with_pool_txns.py @@ -29,29 +29,6 @@ # initialised a connection for a new node by the time the new node's message # reaches it -def test_add_node_with_not_unique_alias(looper, - tdir, - tconf, - sdk_pool_handle, - sdk_wallet_steward, - allPluginsPath): - new_node_name = "Alpha" - new_steward_wallet, steward_did = sdk_add_new_nym(looper, - sdk_pool_handle, - sdk_wallet_steward, - alias="TEST_STEWARD1", - role='STEWARD') - with pytest.raises(RequestRejectedException) as e: - sdk_add_new_node(looper, - sdk_pool_handle, - (new_steward_wallet, steward_did), - new_node_name, - tdir, - tconf, - allPluginsPath) - assert 'existing data has conflicts with request data' in \ - e._excinfo[1].args[0] - sdk_pool_refresh(looper, sdk_pool_handle) def testStewardCannotAddMoreThanOneNode(looper, txnPoolNodeSet, @@ -304,3 +281,28 @@ def testStewardCannotAddNodeWithInvalidHa(looper, tdir, tconf, assert 'invalid network ip address' in e._excinfo[1].args[0] or \ 'expected types' in e._excinfo[1].args[0] or \ 'network port out of the range' in e._excinfo[1].args[0] + + +def test_add_node_with_not_unique_alias(looper, + tdir, + tconf, + sdk_pool_handle, + sdk_wallet_steward, + allPluginsPath): + new_node_name = "Alpha" + new_steward_wallet, steward_did = sdk_add_new_nym(looper, + sdk_pool_handle, + sdk_wallet_steward, + alias="TEST_STEWARD1", + role='STEWARD') + with pytest.raises(RequestRejectedException) as e: + sdk_add_new_node(looper, + sdk_pool_handle, + (new_steward_wallet, steward_did), + new_node_name, + tdir, + tconf, + allPluginsPath) + assert 'existing data has conflicts with request data' in \ + e._excinfo[1].args[0] + sdk_pool_refresh(looper, sdk_pool_handle) \ No newline at end of file