From ac1cd2d52e447da3b35a766ebcd49e06844c7952 Mon Sep 17 00:00:00 2001 From: Sergey Shilov <31342446+sergey-shilov@users.noreply.github.com> Date: Fri, 26 Jan 2018 20:22:14 +0300 Subject: [PATCH] INDY-1069: move state-depended checks from static to dynamic validation. (#531) * INDY-1069: move state-depended checks from static to dynamic validation. Signed-off-by: Sergey Shilov * Add test that emulates NYM resend. Signed-off-by: Sergey Shilov * Fix testTrusteeSuspendingTrustee test. Signed-off-by: Sergey Shilov * Move resend NYM test into separate file. Signed-off-by: Sergey Shilov * Remove duplicated authorised check. Signed-off-by: Sergey Shilov * Fix flake8 warning. Signed-off-by: Sergey Shilov * Get back checking for owner or trustee for existing NYM. Also implement corresponding test. Signed-off-by: Sergey Shilov * Fix flake8 warning. Signed-off-by: Sergey Shilov * Fix testTrusteeSuspensionByTrustee test. Signed-off-by: Sergey Shilov * Add ugly hack in test_nym_send_twice() test to deal with old libindy. Signed-off-by: Sergey Shilov --- indy_client/test/cli/test_nym.py | 2 + indy_client/test/cli/test_nym_suspension.py | 2 +- indy_node/server/domain_req_handler.py | 62 ++++++++++++--------- indy_node/test/conftest.py | 4 ++ indy_node/test/nym_txn/__init__.py | 0 indy_node/test/nym_txn/test_nym_resend.py | 41 ++++++++++++++ 6 files changed, 83 insertions(+), 28 deletions(-) create mode 100644 indy_node/test/nym_txn/__init__.py create mode 100644 indy_node/test/nym_txn/test_nym_resend.py diff --git a/indy_client/test/cli/test_nym.py b/indy_client/test/cli/test_nym.py index 7c0258db8..48fb92fea 100644 --- 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 index 0b5f349f8..870d049c0 100644 --- 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 index e3d5c7344..7a2be3aa3 100644 --- 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 index f9db01f83..37e80678e 100644 --- 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 new file mode 100644 index 000000000..e69de29bb diff --git a/indy_node/test/nym_txn/test_nym_resend.py b/indy_node/test/nym_txn/test_nym_resend.py new file mode 100644 index 000000000..a235737f7 --- /dev/null +++ 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 \ No newline at end of file