changes to account access logic

cmd/geth/main.go

@@ -363,20 +363,20 @@ func startNode(ctx *cli.Context, stack *node.Node) {
 func startQuorumPermissionService(ctx *cli.Context, stack *node.Node) {
 
 	var quorumApis []string
-	if ctx.GlobalBool(utils.EnableNodePermissionFlag.Name) {
-		//default quorum api list
-		quorumApis = []string{"quorumNodeMgmt", "quorumAcctMgmt", "quorumKeyMgmt"}
-	} else {
-		quorumApis = []string{"quorumKeyMgmt"}
-	}
+
 	// start the permissions management service
 	pc, err := permission.NewQuorumPermissionCtrl(stack, ctx.GlobalBool(utils.EnableNodePermissionFlag.Name), ctx.GlobalBool(utils.RaftModeFlag.Name))
 	if err != nil {
-		utils.Fatalf("Failed to start Quorum Permission contract service: %v", err)
-	}
+		log.Error("Failed to start Quorum Permission contract service: %v", err)
+	} else {
+		err = pc.Start()
+		if err ==nil {
+			quorumApis = []string{"quorumNodeMgmt", "quorumAcctMgmt"}
 
-	err = pc.Start()
-	log.Info("Node Permission service started")
+		} else {
+			log.Error("Failed to start Quorum Permission contract service: %v", err)
+		}
+	}
 
 	// Changes for managing org level cluster keys for privateFor txns
 	kc, err := cluster.NewOrgKeyCtrl(stack)
@@ -385,6 +385,7 @@ func startQuorumPermissionService(ctx *cli.Context, stack *node.Node) {
 	} else {
 		kc.Start()
 		log.Trace("Key management service started")
+		quorumApis = append(quorumApis, "quorumKeyMgmt")
 	}
 
 	for _, apiName := range quorumApis {

controls/permission/Permission.sol

@@ -346,11 +346,37 @@ contract Permissions {
   }
 
 
+  // validate if the sender can set the access.
+  // Accounts with Full Access can assign any access
+  // Accounts with Contract access can assign contract or transact
+  // Account with transact can assign transact only
+  function checkOpAllowed(address _from, AccountAccess _accountAccess) internal view returns (bool) {
+    if (!(networkBoot)){
+      return true;
+    }
+    uint acctIndex = getAcctIndex(_from);
+    if (acctToIndex[_from] == 0){
+      return false;
+    }
+    AccountAccess senderAccess = acctAccessList[acctIndex].acctAccess;
+    if (senderAccess == AccountAccess.FullAccess){
+      return true;
+    }
+    else if ((senderAccess == AccountAccess.ContractDeploy) && (_accountAccess != AccountAccess.FullAccess)){
+      return true;
+    }
+    else if ((senderAccess == AccountAccess.Transact) && (_accountAccess == AccountAccess.Transact || _accountAccess == AccountAccess.ReadOnly)){
+      return true;
+    }
+    else {
+      return false;
+    }
+  }
   // Checks if the Node is already added. If yes then returns true
   function updateAccountAccess(address _address, AccountAccess _accountAccess) external
   {
+    require(checkOpAllowed(msg.sender, _accountAccess) == true, "Sender account does not have appropriate access");
     // Check if account already exists
-    emit AccountAccessModified(_address, _accountAccess);
     uint acctIndex = getAcctIndex(_address);
     if (acctToIndex[_address] != 0){
       acctAccessList[acctIndex].acctAccess = _accountAccess;
@@ -360,6 +386,7 @@ contract Permissions {
       acctToIndex[_address] = numberOfAccts;
       acctAccessList.push(AccountAccessDetails(_address, _accountAccess));
     }
+    emit AccountAccessModified(_address, _accountAccess);
   }
 
   // Add voting account

controls/permission/permission.go

@@ -14,6 +14,7 @@ import (
 	"github.com/ethereum/go-ethereum/accounts/abi/bind"
 	"github.com/ethereum/go-ethereum/controls"
 	pbind "github.com/ethereum/go-ethereum/controls/bind"
+	"github.com/ethereum/go-ethereum/cmd/utils"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/eth"
 	"github.com/ethereum/go-ethereum/ethclient"

core/quorum/api.go

@@ -81,6 +81,16 @@ type ExecStatus struct {
 	Status bool
 	Msg    string
 }
+var (
+	ErrNoVoterAccount = ExecStatus{false, "No voter account registered. Add voter first"}
+	ErrInvalidNode = ExecStatus{false, "Invalid node id"}
+	ErrAccountNotAVoter = ExecStatus{false, "Account is not a voter. Action cannot be approved"}
+	ErrInvalidAccount = ExecStatus{false, "Invalid account id"}
+	ErrInvalidAccountAccess = ExecStatus{false, "Invalid account access"}
+	ErrFailedExecution = ExecStatus{false, "Failed to execute permission action"}
+	ExecSuccess = ExecStatus{true, "Action completed successfully"}
+)
+
 
 var nodeApproveStatus = map[uint8]string{
 	0: "Unknown",
@@ -140,21 +150,22 @@ func (s *PermissionAPI) PermissionNodeList() []nodeStatus {
 	}
 	// get the total number of nodes on the contract
 	nodeCnt, err := ps.GetNumberOfNodes()
+	if err != nil {
+		return nil
+	}
 	nodeCntI := nodeCnt.Int64()
 	nodeStatArr := make([]nodeStatus, nodeCntI)
 	// loop for each index and get the node details from the contract
-	if err == nil {
-		i := int64(0)
-		for i < nodeCntI {
-			nodeDtls, _ := ps.GetNodeDetails(big.NewInt(i))
-			nodeStatArr[i].Name = "enode://" + nodeDtls.EnodeId + "@" + nodeDtls.IpAddrPort
-			nodeStatArr[i].Name += "?discport=" + nodeDtls.DiscPort
-			if len(nodeDtls.RaftPort) > 0 {
-				nodeStatArr[i].Name += "&raftport" + nodeDtls.RaftPort
-			}
-			nodeStatArr[i].Status = decodeNodeStatus(nodeDtls.NodeStatus)
-			i++
+	i := int64(0)
+	for i < nodeCntI {
+		nodeDtls, _ := ps.GetNodeDetails(big.NewInt(i))
+		nodeStatArr[i].Name = "enode://" + nodeDtls.EnodeId + "@" + nodeDtls.IpAddrPort
+		nodeStatArr[i].Name += "?discport=" + nodeDtls.DiscPort
+		if len(nodeDtls.RaftPort) > 0 {
+			nodeStatArr[i].Name += "&raftport=" + nodeDtls.RaftPort
 		}
+		nodeStatArr[i].Status = decodeNodeStatus(nodeDtls.NodeStatus)
+		i++
 	}
 	return nodeStatArr
 }
@@ -227,12 +238,10 @@ func (s *PermissionAPI) SetAccountAccess(acct common.Address, access string, txa
 func (s *PermissionAPI) executePermAction(action PermAction, args txArgs) ExecStatus {
 	var err error
 	var w accounts.Wallet
-	voterErr := ExecStatus{false, "add voter first"}
-	invalidNodeErr := ExecStatus{false, "invalid node id"}
 
 	w, err = s.validateAccount(args.txa.From)
 	if err != nil {
-		return ExecStatus{false, err.Error()}
+		return ErrInvalidAccount
 	}
 	ps := s.newPermSession(w, args.txa)
 	var tx *types.Transaction
@@ -247,12 +256,12 @@ func (s *PermissionAPI) executePermAction(action PermAction, args txArgs) ExecSt
 
 	case ProposeNode:
 		if !checkVoterExists(ps) {
-			return voterErr
+			return ErrNoVoterAccount
 		}
 		node, err = discover.ParseNode(args.nodeId)
 		if err != nil {
 			log.Error("invalid node id: %v", err)
-			return invalidNodeErr
+			return ErrInvalidNode
 		}
 		enodeID := node.ID.String()
 		ipAddr := node.IP.String()
@@ -265,72 +274,72 @@ func (s *PermissionAPI) executePermAction(action PermAction, args txArgs) ExecSt
 
 	case ApproveNode:
 		if !checkIsVoter(ps, args.txa.From) {
-			return voterErr
+			return ErrAccountNotAVoter
 		}
 		node, err = discover.ParseNode(args.nodeId)
 		if err != nil {
 			log.Error("invalid node id: %v", err)
-			return invalidNodeErr
+			return ErrInvalidNode
 		}
 		enodeID := node.ID.String()
 		tx, err = ps.ApproveNode(enodeID)
 
 	case ProposeNodeDeactivation:
 		if !checkVoterExists(ps) {
-			return voterErr
+			return ErrNoVoterAccount
 		}
 		node, err = discover.ParseNode(args.nodeId)
 		if err != nil {
 			log.Error("invalid node id: %v", err)
-			return invalidNodeErr
+			return ErrInvalidNode
 		}
 		enodeID := node.ID.String()
 		tx, err = ps.ProposeDeactivation(enodeID)
 
 	case ApproveNodeDeactivation:
 		if !checkIsVoter(ps, args.txa.From) {
-			return voterErr
+			return ErrAccountNotAVoter
 		}
 		node, err = discover.ParseNode(args.nodeId)
 		if err != nil {
 			log.Error("invalid node id: %v", err)
-			return invalidNodeErr
+			return ErrInvalidNode
 		}
 		enodeID := node.ID.String()
 		tx, err = ps.DeactivateNode(enodeID)
 
 	case ProposeNodeActivation:
 		if !checkVoterExists(ps) {
-			return voterErr
+			return ErrNoVoterAccount
 		}
 		node, err = discover.ParseNode(args.nodeId)
 		if err != nil {
 			log.Error("invalid node id: %v", err)
-			return invalidNodeErr
+			return ErrInvalidNode
 		}
 		enodeID := node.ID.String()
 		tx, err = ps.ProposeNodeActivation(enodeID)
 
 	case ApproveNodeActivation:
 		if !checkIsVoter(ps, args.txa.From) {
-			return voterErr
+			return ErrAccountNotAVoter
 		}
 		node, err = discover.ParseNode(args.nodeId)
 		if err != nil {
 			log.Error("invalid node id: %v", err)
-			return invalidNodeErr
+			return ErrInvalidNode
 		}
 		enodeID := node.ID.String()
 		tx, err = ps.ActivateNode(enodeID)
 
 	case ProposeNodeBlacklisting:
 		if !checkVoterExists(ps) {
-			return voterErr
+			return ErrNoVoterAccount
 		}
 		node, err = discover.ParseNode(args.nodeId)
 		if err != nil {
 			log.Error("invalid node id: %v", err)
-			return invalidNodeErr
+			return ErrInvalidNode
 		}
 		enodeID := node.ID.String()
 		ipAddr := node.IP.String()
@@ -342,12 +351,12 @@ func (s *PermissionAPI) executePermAction(action PermAction, args txArgs) ExecSt
 		tx, err = ps.ProposeNodeBlacklisting(enodeID, ipAddrPort, discPort, raftPort)
 	case ApproveNodeBlacklisting:
 		if !checkIsVoter(ps, args.txa.From) {
-			return voterErr
+			return ErrAccountNotAVoter
 		}
 		node, err = discover.ParseNode(args.nodeId)
 		if err != nil {
 			log.Error("invalid node id: %v", err)
-			return invalidNodeErr
+			return ErrInvalidNode
 		}
 		enodeID := node.ID.String()
 		tx, err = ps.BlacklistNode(enodeID)
@@ -356,17 +365,17 @@ func (s *PermissionAPI) executePermAction(action PermAction, args txArgs) ExecSt
 		var access uint64
 		access, err = strconv.ParseUint(args.accessType, 10, 8)
 		if err != nil {
-			return ExecStatus{false, "invalid access type"}
+			return ErrInvalidAccountAccess
 		}
 		tx, err = ps.UpdateAccountAccess(args.acctId, uint8(access))
 	}
 
 	if err != nil {
 		log.Error("Failed to execute permission action", "action", action, "err", err)
-		return ExecStatus{false, err.Error()}
+		return ErrFailedExecution
 	}
 	log.Debug("executed permission action", "action", action, "tx", tx)
-	return ExecStatus{true, ""}
+	return ExecSuccess
 }
 
 // executeOrgKeyAction helps to execute an action in cluster contract

core/tx_pool.go

@@ -1283,6 +1283,7 @@ func checkAccount(fromAcct common.Address, toAcct *common.Address) error {
 
 	switch access {
 	case types.FullAccess:
+	case types.ContractDeploy:
 		return nil
 
 	case types.ReadOnly:
@@ -1296,13 +1297,13 @@ func checkAccount(fromAcct common.Address, toAcct *common.Address) error {
 		} else {
 			return nil
 		}
-	case types.ContractDeploy:
-		if toAcct != nil {
-			err := errors.New("Account Does not have transacte permissions")
-			return err
-		} else {
-			return nil
-		}
+	// case types.ContractDeploy:
+	// 	if toAcct != nil {
+	// 		err := errors.New("Account Does not have transacte permissions")
+	// 		return err
+	// 	} else {
+	// 		return nil
+	// 	}
 	}
 	return nil
 }