From 6a38f44c9aa93bcafec90f857cf611c285c1872e Mon Sep 17 00:00:00 2001 From: Patrick Deziel <42919891+pdeziel@users.noreply.github.com> Date: Fri, 5 Aug 2022 14:41:27 -0500 Subject: [PATCH] Improve beneficiary identity check (#109) --- pkg/rvasp/db/fixtures_test.go | 11 ++++++ pkg/rvasp/rvasp.go | 7 ++-- pkg/rvasp/transfer.go | 40 ++++++++++++++++++-- pkg/rvasp/trisa.go | 3 +- pkg/rvasp/trisa_test.go | 71 +++++++++++++++++++++++++++-------- 5 files changed, 108 insertions(+), 24 deletions(-) diff --git a/pkg/rvasp/db/fixtures_test.go b/pkg/rvasp/db/fixtures_test.go index 73adcc0..0c9304e 100644 --- a/pkg/rvasp/db/fixtures_test.go +++ b/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) + } } diff --git a/pkg/rvasp/rvasp.go b/pkg/rvasp/rvasp.go index ae5ed8e..3cfd33c 100644 --- a/pkg/rvasp/rvasp.go +++ b/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 { diff --git a/pkg/rvasp/transfer.go b/pkg/rvasp/transfer.go index d8f64a4..a7434ab 100644 --- a/pkg/rvasp/transfer.go +++ b/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 } diff --git a/pkg/rvasp/trisa.go b/pkg/rvasp/trisa.go index 2d1eb0a..cbaae95 100644 --- a/pkg/rvasp/trisa.go +++ b/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) } diff --git a/pkg/rvasp/trisa_test.go b/pkg/rvasp/trisa_test.go index ce18978..7c72252 100644 --- a/pkg/rvasp/trisa_test.go +++ b/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)