INDY-1069: move state-depended checks from static to dynamic validati…

…on. (hyperledger#531) * INDY-1069: move state-depended checks from static to dynamic validation. Signed-off-by: Sergey Shilov <[email protected]> * Add test that emulates NYM resend. Signed-off-by: Sergey Shilov <[email protected]> * Fix testTrusteeSuspendingTrustee test. Signed-off-by: Sergey Shilov <[email protected]> * Move resend NYM test into separate file. Signed-off-by: Sergey Shilov <[email protected]> * Remove duplicated authorised check. Signed-off-by: Sergey Shilov <[email protected]> * Fix flake8 warning. Signed-off-by: Sergey Shilov <[email protected]> * Get back checking for owner or trustee for existing NYM. Also implement corresponding test. Signed-off-by: Sergey Shilov <[email protected]> * Fix flake8 warning. Signed-off-by: Sergey Shilov <[email protected]> * Fix testTrusteeSuspensionByTrustee test. Signed-off-by: Sergey Shilov <[email protected]> * Add ugly hack in test_nym_send_twice() test to deal with old libindy. Signed-off-by: Sergey Shilov <[email protected]>
lucianoacsilva · Jan 26, 2018 · ac1cd2d · ac1cd2d
1 parent ecde3c0
commit ac1cd2d
Show file tree

Hide file tree

Showing 6 changed files with 83 additions and 28 deletions.
diff --git a/indy_client/test/cli/test_nym.py b/indy_client/test/cli/test_nym.py
@@ -9,6 +9,7 @@
 from indy_node.test.did.conftest import wallet, abbrevVerkey
 from indy_client.test.cli.helper import connect_and_check_output
 
+
 TRUST_ANCHOR_SEED = b'TRUST0NO0ONE00000000000000000000'
 
 NYM_ADDED = 'Nym {dest} added'
@@ -361,3 +362,4 @@ def test_send_different_nyms_succeeds_when_batched(
 
     do('send GET_NYM dest={dest}',
         mapper=parameters, expect=CURRENT_VERKEY_FOR_NYM, within=2)
+
diff --git a/indy_client/test/cli/test_nym_suspension.py b/indy_client/test/cli/test_nym_suspension.py
@@ -127,7 +127,7 @@ def testTrusteeSuspendingTrustee(be, do, trusteeCli, anotherTrusteeAdded,
        within=5,
        expect=nymAddedOut, mapper=anotherTrusteeAdded)
     be(anotherTrusteeCli)
-    errorMsg = 'InvalidClientRequest'
+    errorMsg = 'CouldNotAuthenticate'
     do('send NYM dest={remote} role=',
        within=5,
        expect=[errorMsg], mapper=stewardAdded)

diff --git a/indy_node/server/domain_req_handler.py b/indy_node/server/domain_req_handler.py
@@ -15,6 +15,7 @@
 from plenum.common.exceptions import InvalidClientRequest, \
     UnauthorizedClientRequest, UnknownIdentifier, InvalidClientMessageException
 from plenum.common.types import f
+from plenum.common.constants import TRUSTEE
 from plenum.server.domain_req_handler import DomainRequestHandler as PHandler
 from stp_core.common.log import getlogger
 
@@ -73,16 +74,6 @@ def commit(self, txnCount, stateRoot, txnRoot) -> List:
         self.idrCache.onBatchCommitted(stateRoot)
         return r
 
-    def canNymRequestBeProcessed(self, identifier, msg) -> (bool, str):
-        nym = msg.get(TARGET_NYM)
-        if self.hasNym(nym, isCommitted=False):
-            if not self.idrCache.hasTrustee(identifier, isCommitted=False) \
-                    and self.idrCache.getOwnerFor(nym, isCommitted=False) != identifier:
-                reason = '{} is neither Trustee nor owner of {}' \
-                    .format(identifier, nym)
-                return False, reason
-        return True, ''
-
     def doStaticValidation(self, request: Request):
         identifier, req_id, operation = request.identifier, request.reqId, request.operation
         if operation[TXN_TYPE] == NYM:
@@ -101,10 +92,6 @@ def _doStaticValidationNym(self, identifier, reqId, operation):
             raise InvalidClientRequest(identifier, reqId,
                                        "{} not a valid role".
                                        format(role))
-        # TODO: This is not static validation as it involves state
-        s, reason = self.canNymRequestBeProcessed(identifier, operation)
-        if not s:
-            raise InvalidClientRequest(identifier, reqId, reason)
 
     @staticmethod
     def _validate_attrib_keys(operation):
@@ -171,26 +158,47 @@ def _validateNewNym(self, req: Request, op, originRole):
             )
 
     def _validateExistingNym(self, req: Request, op, originRole, nymData):
+        unauthorized = False
+        reason = None
         origin = req.identifier
         owner = self.idrCache.getOwnerFor(op[TARGET_NYM], isCommitted=False)
         isOwner = origin == owner
-        updateKeys = [ROLE, VERKEY]
-        for key in updateKeys:
-            if key in op:
-                newVal = op[key]
-                oldVal = nymData.get(key)
-                if oldVal != newVal:
-                    r, msg = Authoriser.authorised(NYM, key, originRole,
-                                                   oldVal=oldVal, newVal=newVal,
-                                                   isActorOwnerOfSubject=isOwner)
-                    if not r:
-                        raise UnauthorizedClientRequest(
-                            req.identifier, req.reqId, "{} cannot update {}".
-                            format(Roles.nameFromValue(originRole), key))
+
+        if not originRole == TRUSTEE and not isOwner:
+            reason = '{} is neither Trustee nor owner of {}' \
+                .format(origin, op[TARGET_NYM])
+            unauthorized = True
+
+        if not unauthorized:
+            updateKeys = [ROLE, VERKEY]
+            for key in updateKeys:
+                if key in op:
+                    newVal = op[key]
+                    oldVal = nymData.get(key)
+                    if oldVal != newVal:
+                        r, msg = Authoriser.authorised(NYM, key, originRole,
+                                                       oldVal=oldVal, newVal=newVal,
+                                                       isActorOwnerOfSubject=isOwner)
+                        if not r:
+                            unauthorized = True
+                            reason = "{} cannot update {}".\
+                                format(Roles.nameFromValue(originRole), key)
+                            break
+        if unauthorized:
+            raise UnauthorizedClientRequest(
+                req.identifier, req.reqId, reason)
 
     def _validateAttrib(self, req: Request):
         origin = req.identifier
         op = req.operation
+
+        if not (not op.get(TARGET_NYM) or
+                self.hasNym(op[TARGET_NYM], isCommitted=False)):
+            raise InvalidClientRequest(origin, req.reqId,
+                                       '{} should be added before adding '
+                                       'attribute for it'.
+                                       format(TARGET_NYM))
+
         if op.get(TARGET_NYM) and op[TARGET_NYM] != req.identifier and \
                 not self.idrCache.getOwnerFor(op[TARGET_NYM],
                                               isCommitted=False) == origin:

diff --git a/indy_node/test/conftest.py b/indy_node/test/conftest.py
@@ -47,6 +47,10 @@
 from indy_common.test.conftest import general_conf_tdir, tconf, poolTxnTrusteeNames, \
     domainTxnOrderedFields, looper, setTestLogLevel, node_config_helper_class, config_helper_class
 
+from plenum.test.conftest import sdk_pool_handle, sdk_pool_name, sdk_wallet_steward, sdk_wallet_handle, \
+    sdk_wallet_name, sdk_steward_seed
+
+
 Logger.setLogLevel(logging.NOTSET)
 
 

diff --git a/indy_node/test/nym_txn/__init__.py b/indy_node/test/nym_txn/__init__.py
diff --git a/indy_node/test/nym_txn/test_nym_resend.py b/indy_node/test/nym_txn/test_nym_resend.py
@@ -0,0 +1,41 @@
+import json
+
+from indy_client.test.cli.helper import createHalfKeyIdentifierAndAbbrevVerkey
+from indy.ledger import sign_request, submit_request, build_nym_request
+from indy.error import IndyError, ErrorCode
+from plenum.common.constants import REPLY, REJECT
+
+
+def test_nym_send_twice(looper, sdk_pool_handle, sdk_wallet_steward):
+    idr, verkey = createHalfKeyIdentifierAndAbbrevVerkey()
+
+    wallet_handle, identifier = sdk_wallet_steward
+
+    for i in range(2):
+        request = looper.loop.run_until_complete(build_nym_request(identifier, idr, verkey, None, None))
+        req_signed = looper.loop.run_until_complete(sign_request(wallet_handle, identifier, request))
+
+        if i == 0:
+            result = json.loads(looper.loop.run_until_complete(submit_request(sdk_pool_handle, req_signed)))
+            assert result['op'] == REPLY
+        else:
+            # TODO(INDY-1069): Ugly hack to deal with old libindy which raises exception on REJECT,
+            # in fact it should be simple:
+            # assert result['op'] == REJECT
+            try:
+                json.loads(looper.loop.run_until_complete(submit_request(sdk_pool_handle, req_signed)))
+                assert False
+            except IndyError as ex:
+                assert ex.error_code == ErrorCode.LedgerInvalidTransaction
+
+def test_nym_resend(looper, sdk_pool_handle, sdk_wallet_steward):
+    idr, verkey = createHalfKeyIdentifierAndAbbrevVerkey()
+
+    wallet_handle, identifier = sdk_wallet_steward
+
+    request = looper.loop.run_until_complete(build_nym_request(identifier, idr, verkey, None, None))
+    req_signed = looper.loop.run_until_complete(sign_request(wallet_handle, identifier, request))
+
+    for i in range(2):
+        result = json.loads(looper.loop.run_until_complete(submit_request(sdk_pool_handle, req_signed)))
+        assert result['op'] == REPLY