Improve beneficiary identity check (#109)

pkg/rvasp/db/fixtures_test.go

@@ -13,6 +13,11 @@ func TestLoadVASPs(t *testing.T) {
 	require.NoError(t, err)
 
 	require.Equal(t, 4, len(vasps))
+	for _, vasp := range vasps {
+		identity, err := vasp.LoadIdentity()
+		require.NoError(t, err, "failed to load identity for vasp fixture %s", vasp.Name)
+		require.NoError(t, identity.GetLegalPerson().Validate(), "identity for vasp fixture %s must be a valid ivms101.LegalPerson", vasp.Name)
+	}
 }
 
 func TestLoadWallets(t *testing.T) {
@@ -21,4 +26,10 @@ func TestLoadWallets(t *testing.T) {
 
 	require.Equal(t, 16, len(wallets))
 	require.Equal(t, 16, len(accounts))
+	for _, account := range accounts {
+		identity, err := account.LoadIdentity()
+		require.NoError(t, err, "failed to load identity for account fixture %s", account.Name)
+		require.NoError(t, identity.GetNaturalPerson().Validate(), "identity for account fixture %s must be a valid ivms101.NaturalPerson", account.Name)
+		require.NotEmpty(t, account.WalletAddress, "account fixture %s must have a wallet address", account.Name)
+	}
 }

pkg/rvasp/rvasp.go

@@ -227,9 +227,7 @@ func (s *Server) Transfer(ctx context.Context, req *pb.TransferRequest) (reply *
 	}
 
 	// Build the transfer response
-	reply = &pb.TransferReply{
-		Transaction: xfer.Proto(),
-	}
+	reply = &pb.TransferReply{}
 
 	// Handle rVASP errors and TRISA protocol errors
 	if transferError != nil {
@@ -248,6 +246,9 @@ func (s *Server) Transfer(ctx context.Context, req *pb.TransferRequest) (reply *
 		}
 	}
 
+	// Populate the transfer response with the transaction details
+	reply.Transaction = xfer.Proto()
+
 	// Save the updated transaction
 	// TODO: Clean up completed transactions in the database
 	if err = s.db.Save(xfer).Error; err != nil {

pkg/rvasp/transfer.go

@@ -59,7 +59,7 @@ func (s *Server) createIdentityPayload(originatorAccount db.Account, beneficiary
 	// Create the beneficiary account information if provided
 	if beneficiaryAccount.WalletAddress != "" {
 		identity.Beneficiary = &ivms101.Beneficiary{
-			BeneficiaryPersons: make([]*ivms101.Person, 0, 1),
+			BeneficiaryPersons: make([]*ivms101.Person, 0),
 			AccountNumbers:     []string{beneficiaryAccount.WalletAddress},
 		}
 	}
@@ -187,7 +187,7 @@ func parsePayload(payload *protocol.Payload, response bool) (identity *ivms101.I
 }
 
 // Validate an identity payload, returning an error if the payload is not valid.
-func validateIdentityPayload(identity *ivms101.IdentityPayload, requireBeneficiary bool) (err *protocol.Error) {
+func validateIdentityPayload(identity *ivms101.IdentityPayload, requireBeneficiary bool) *protocol.Error {
 	// Verify the identity payload is not nil
 	if identity == nil {
 		log.Warn().Msg("identity payload is nil")
@@ -207,17 +207,49 @@ func validateIdentityPayload(identity *ivms101.IdentityPayload, requireBeneficia
 	}
 
 	if requireBeneficiary {
-		// Validate that the beneficiary is present
+		var err error
+
+		// Check that the beneficiary is present
 		if identity.Beneficiary == nil {
 			log.Warn().Msg("identity payload missing beneficiary")
 			return protocol.Errorf(protocol.IncompleteIdentity, "missing beneficiary")
 		}
 
-		// Validate that the beneficiary vasp is present
+		// Check that the beneficiary person is present
+		if len(identity.Beneficiary.BeneficiaryPersons) == 0 {
+			log.Warn().Msg("identity payload missing beneficiary person")
+			return protocol.Errorf(protocol.IncompleteIdentity, "missing beneficiary person")
+		}
+
+		// Validate the beneficiary person
+		if err = identity.Beneficiary.BeneficiaryPersons[0].GetNaturalPerson().Validate(); err != nil {
+			log.Warn().Err(err).Msg("beneficiary person validation error")
+			return protocol.Errorf(protocol.ValidationError, "beneficiary person validation error: %s", err)
+		}
+
+		// Check that the account number is present
+		if len(identity.Beneficiary.AccountNumbers) == 0 || identity.Beneficiary.AccountNumbers[0] == "" {
+			log.Warn().Msg("identity payload missing account number")
+			return protocol.Errorf(protocol.IncompleteIdentity, "missing beneficiary account number")
+		}
+
+		// Check that the beneficiary vasp is present
 		if identity.BeneficiaryVasp == nil {
 			log.Warn().Msg("identity payload missing beneficiary vasp")
 			return protocol.Errorf(protocol.IncompleteIdentity, "missing beneficiary vasp")
 		}
+
+		// Check that the beneficiary vasp entity is present
+		if identity.BeneficiaryVasp.BeneficiaryVasp == nil {
+			log.Warn().Msg("identity payload missing beneficiary vasp entity")
+			return protocol.Errorf(protocol.IncompleteIdentity, "missing beneficiary vasp entity")
+		}
+
+		// Validate the beneficiary vasp entity
+		if err = identity.BeneficiaryVasp.BeneficiaryVasp.GetLegalPerson().Validate(); err != nil {
+			log.Warn().Err(err).Msg("beneficiary vasp entity validation error")
+			return protocol.Errorf(protocol.ValidationError, "beneficiary vasp entity validation error: %s", err)
+		}
 	}
 	return nil
 }

pkg/rvasp/trisa.go

@@ -420,7 +420,8 @@ func (s *TRISA) handleTransaction(ctx context.Context, peer *peers.Peer, in *pro
 		return nil, protocol.Errorf(protocol.InternalError, "unknown policy '%s' for wallet '%s'", policy, account.WalletAddress)
 	}
 
-	if transferError != nil {
+	// Mark transaction as failed if it was not rejected but an error occurred
+	if xfer.State != pb.TransactionState_REJECTED && transferError != nil {
 		log.Debug().Err(transferError).Msg("transfer failed")
 		xfer.SetState(pb.TransactionState_FAILED)
 	}

pkg/rvasp/trisa_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/trisacrypto/testnet/pkg/rvasp"
 	"github.com/trisacrypto/testnet/pkg/rvasp/bufconn"
 	"github.com/trisacrypto/testnet/pkg/rvasp/config"
+	"github.com/trisacrypto/testnet/pkg/rvasp/db"
 	"github.com/trisacrypto/trisa/pkg/ivms101"
 	protocol "github.com/trisacrypto/trisa/pkg/trisa/api/v1beta1"
 	generic "github.com/trisacrypto/trisa/pkg/trisa/data/generic/v1beta1"
@@ -21,19 +22,25 @@ import (
 	"google.golang.org/protobuf/types/known/anypb"
 )
 
-const bufSize = 1024 * 1024
+const (
+	bufSize      = 1024 * 1024
+	fixturesPath = "fixtures"
+)
 
 // rVASPTestSuite tests interactions with the rvasp servers to ensure that they return
 // the expected error codes and messages.
 type rVASPTestSuite struct {
 	suite.Suite
-	grpc  *bufconn.GRPCListener
-	db    sqlmock.Sqlmock
-	trisa *rvasp.TRISA
-	certs *trust.Provider
-	chain trust.ProviderPool
-	peers *peers.Peers
-	conf  *config.Config
+	grpc     *bufconn.GRPCListener
+	db       sqlmock.Sqlmock
+	trisa    *rvasp.TRISA
+	certs    *trust.Provider
+	chain    trust.ProviderPool
+	peers    *peers.Peers
+	conf     *config.Config
+	vasps    []db.VASP
+	wallets  []db.Wallet
+	accounts []db.Account
 }
 
 func TestRVASP(t *testing.T) {
@@ -49,6 +56,15 @@ func (s *rVASPTestSuite) SetupSuite() {
 	certPath := filepath.Join("testdata", "cert.pem")
 	s.conf.CertPath = certPath
 	s.conf.TrustChainPath = certPath
+
+	s.vasps, err = db.LoadVASPs(fixturesPath)
+	require.NoError(err, "could not load VASP fixtures")
+	require.Greater(len(s.vasps), 0, "no VASPs loaded")
+
+	s.wallets, s.accounts, err = db.LoadWallets(fixturesPath)
+	require.NoError(err, "could not load wallet fixtures")
+	require.Greater(len(s.wallets), 0, "no wallets loaded")
+	require.Greater(len(s.accounts), 0, "no accounts loaded")
 }
 
 func (s *rVASPTestSuite) BeforeTest(suiteName, testName string) {
@@ -74,6 +90,36 @@ func expectStandardQuery(db sqlmock.Sqlmock, kind string) {
 	db.ExpectQuery(kind).WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(1))
 }
 
+// createIdentityPayload which has a valid beneficiary identity and can be used for
+// SyncRequire transfers.
+func (s *rVASPTestSuite) createIdentityPayload() (identity *ivms101.IdentityPayload) {
+	require := s.Require()
+
+	identity = &ivms101.IdentityPayload{
+		Originator:      &ivms101.Originator{},
+		OriginatingVasp: &ivms101.OriginatingVasp{},
+		Beneficiary:     &ivms101.Beneficiary{},
+		BeneficiaryVasp: &ivms101.BeneficiaryVasp{},
+	}
+
+	// For unit testing it does not matter which VASP fixture is used although it must
+	// have a valid ivms101.LegalPerson
+	var err error
+	identity.BeneficiaryVasp.BeneficiaryVasp, err = s.vasps[0].LoadIdentity()
+	require.NoError(err, "could not load beneficiary VASP identity")
+	require.NoError(identity.BeneficiaryVasp.BeneficiaryVasp.GetLegalPerson().Validate(), "VASP ivms101.LegalPerson fixture failed validation")
+
+	// The account fixture must have a valid ivms101.NaturalPerson
+	beneficiary, err := s.accounts[0].LoadIdentity()
+	require.NoError(err, "could not load beneficiary account identity")
+	require.NoError(beneficiary.GetNaturalPerson().Validate(), "account ivms101.NaturalPerson fixture failed validation")
+
+	identity.Beneficiary.BeneficiaryPersons = []*ivms101.Person{beneficiary}
+	identity.Beneficiary.AccountNumbers = []string{s.accounts[0].WalletAddress}
+
+	return identity
+}
+
 // Test that the TRISA server returns a valid envelope when a valid request is sent for
 // SyncRequire.
 func (s *rVASPTestSuite) TestValidTransfer() {
@@ -88,14 +134,7 @@ func (s *rVASPTestSuite) TestValidTransfer() {
 		SentAt: time.Now().Format(time.RFC3339),
 	}
 
-	identity := &ivms101.IdentityPayload{
-		Originator:      &ivms101.Originator{},
-		OriginatingVasp: &ivms101.OriginatingVasp{},
-		Beneficiary:     &ivms101.Beneficiary{},
-		BeneficiaryVasp: &ivms101.BeneficiaryVasp{
-			BeneficiaryVasp: &ivms101.Person{},
-		},
-	}
+	identity := s.createIdentityPayload()
 	payload.Identity, err = anypb.New(identity)
 	require.NoError(err)