Repository: moov-io/customers
Pull request #217 (2020-10-02 22:55:51)

Validate customer belongs to org when retrieveing encrypted account number

codecov-commenter: # [Codecov](https://codecov.io/gh/moov-io/customers/pull/217?src=pr&el=h1) Report
> Merging [#217](https://codecov.io/gh/moov-io/customers/pull/217?src=pr&el=desc) into [master](https://codecov.io/gh/moov-io/customers/commit/fac0d7d0c272ed2545500ba1c35efc3519484e27?el=desc) will **decrease** coverage by `2.64%`.
> The diff coverage is `90.90%`.


```diff
@@            Coverage Diff             @@
##           master     #217      +/-   ##
==========================================
- Coverage   61.31%   58.66%   -2.65%     
==========================================
  Files          49       49              
  Lines        2629     2729     +100     
==========================================
- Hits         1612     1601      -11     
- Misses        710      822     +112     
+ Partials      307      306       -1     
```

codecov-io: # [Codecov](https://codecov.io/gh/moov-io/customers/pull/217?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@f86b774`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `83.33%`.


```diff
@@            Coverage Diff            @@
##             master     #217   +/-   ##
=========================================
  Coverage          ?   58.60%           
=========================================
  Files             ?       49           
  Lines             ?     2732           
  Branches          ?        0           
=========================================
  Hits              ?     1601           
  Misses            ?      825           
  Partials          ?      306           
```


Patch:
diff --git a/go.sum b/go.sum
index 0e8df41..04f10cf 100644
--- a/go.sum
+++ b/go.sum
@@ -129,6 +129,7 @@ github.com/Nvveen/Gotty v0.0.0-20120604004816-cd527374f1e5 h1:TngWCqHvy9oXAN6lEV
 github.com/Nvveen/Gotty v0.0.0-20120604004816-cd527374f1e5/go.mod h1:lmUJ/7eu/Q8D7ML55dXQrVaamCz2vxCfdQBasLZfHKk=
 github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU=
 github.com/Pallinder/go-randomdata v1.2.0/go.mod h1:yHmJgulpD2Nfrm0cR9tI/+oAgRqCQQixsA8HyRZfV9Y=
+github.com/PuerkitoBio/goquery v1.5.1 h1:PSPBGne8NIUWw+/7vFBV+kG2J/5MOjbzc7154OaKCSE=
 github.com/PuerkitoBio/goquery v1.5.1/go.mod h1:GsLWisAFVj4WgDibEWF4pvYnkVQBpKBKeU+7zCJoLcc=
 github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWXgklEdEo=
 github.com/Shopify/sarama v1.26.1/go.mod h1:NbSGBSSndYaIhRcBtY9V0U7AyH+x71bG668AuWys/yU=
@@ -142,6 +143,7 @@ github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuy
 github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
 github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
 github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk541a8SKzHPHnH3zbiI+7dagKZ0cgpgrD7Fyho=
+github.com/andybalholm/cascadia v1.1.0 h1:BuuO6sSfQNFRu1LppgbD25Hr2vLYW25JvxHs5zzsLTo=
 github.com/andybalholm/cascadia v1.1.0/go.mod h1:GsXiBklL0woXo1j/WYWtSYYC4ouU9PqHO0sqidkEA4Y=
 github.com/antihax/optional v0.0.0-20180407024304-ca021399b1a6/go.mod h1:V8iCPQYkqmusNa815XgQio277wI47sdRh1dUOLdyC6Q=
 github.com/antihax/optional v1.0.0 h1:xK2lYat7ZLaVVcIuj82J8kIro4V6kDe0AUDFboUCwcg=
@@ -316,6 +318,7 @@ github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/me
 github.com/go-test/deep v1.0.1/go.mod h1:wGDj63lr65AM2AQyKZd/NYHGb0R+1RLqB8NKt3aSFNA=
 github.com/go-test/deep v1.0.2-0.20181118220953-042da051cf31/go.mod h1:wGDj63lr65AM2AQyKZd/NYHGb0R+1RLqB8NKt3aSFNA=
 github.com/gobuffalo/here v0.6.0/go.mod h1:wAG085dHOYqUpf+Ap+WOdrPTp5IYcDAs/x7PLa8Y5fM=
+github.com/gobuffalo/here v0.6.2 h1:ZtCqC7F9ou3moLbYfHM1Tj+gwHGgWhjyRjVjsir9BE0=
 github.com/gobuffalo/here v0.6.2/go.mod h1:D75Sq0p2BVHdgQu3vCRsXbg85rx943V19urJpqAVWjI=
 github.com/gocql/gocql v0.0.0-20190301043612-f6df8288f9b4/go.mod h1:4Fw1eo5iaEhDUs8XyuhSVCVy52Jq3L+/3GJgYkwc+/0=
 github.com/goftp/file-driver v0.0.0-20180502053751-5d604a0fc0c9/go.mod h1:GpOj6zuVBG3Inr9qjEnuVTgBlk2lZ1S9DcoFiXWyKss=
@@ -621,6 +624,7 @@ github.com/lyft/protoc-gen-validate v0.0.13/go.mod h1:XbGvPuh87YZc5TdIa2/I4pLk0Q
 github.com/magiconair/properties v1.8.1 h1:ZC2Vc7/ZFkGmsVC9KvOjumD+G5lXy2RtTKyzRKO2BQ4=
 github.com/magiconair/properties v1.8.1/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
 github.com/markbates/pkger v0.15.1/go.mod h1:0JoVlrol20BSywW79rN3kdFFsE5xYM+rSCQDXbLhiuI=
+github.com/markbates/pkger v0.17.0 h1:RFfyBPufP2V6cddUyyEVSHBpaAnM1WzaMNyqomeT+iY=
 github.com/markbates/pkger v0.17.0/go.mod h1:0JoVlrol20BSywW79rN3kdFFsE5xYM+rSCQDXbLhiuI=
 github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU=
 github.com/mattn/go-colorable v0.1.1/go.mod h1:FuOcm+DKB9mbwrcAfNl7/TZVBZ6rcnceauSikq3lYCQ=
diff --git a/pkg/accounts/mock_repository.go b/pkg/accounts/mock_repository.go
index d68fa22..33adb63 100644
--- a/pkg/accounts/mock_repository.go
+++ b/pkg/accounts/mock_repository.go
@@ -58,7 +58,7 @@ func (r *mockRepository) updateAccountStatus(accountID string, status client.Acc
 	return r.Err
 }
 
-func (r *mockRepository) getEncryptedAccountNumber(customerID, accountID string) (string, error) {
+func (r *mockRepository) getEncryptedAccountNumber(organization, customerID, accountID string) (string, error) {
 	if r.Err != nil {
 		return "", r.Err
 	}
diff --git a/pkg/accounts/repository.go b/pkg/accounts/repository.go
index beb20d4..f5b5fcf 100644
--- a/pkg/accounts/repository.go
+++ b/pkg/accounts/repository.go
@@ -28,7 +28,7 @@ type Repository interface {
 
 	updateAccountStatus(accountID string, status client.AccountStatus) error
 
-	getEncryptedAccountNumber(customerID, accountID string) (string, error)
+	getEncryptedAccountNumber(organization, customerID, accountID string) (string, error)
 
 	getLatestAccountOFACSearch(accountID string) (*client.OfacSearch, error)
 	saveAccountOFACSearch(id string, result *client.OfacSearch) error
@@ -141,6 +141,7 @@ func (r *sqlAccountRepository) getAccountsByCustomerID(customerID string) ([]*cl
 }
 
 func (r *sqlAccountRepository) CreateCustomerAccount(customerID, userID string, req *CreateAccountRequest) (*client.Account, error) {
+	// TODO: remove userID
 	account := &client.Account{
 		CustomerID:          customerID,
 		AccountID:           base.ID(),
@@ -161,6 +162,7 @@ func (r *sqlAccountRepository) CreateCustomerAccount(customerID, userID string,
 	}
 	defer stmt.Close()
 
+	// TODO: remove userID
 	_, err = stmt.Exec(
 		account.AccountID,
 		account.CustomerID,
@@ -207,8 +209,12 @@ func (r *sqlAccountRepository) updateAccountStatus(accountID string, status clie
 	return err
 }
 
-func (r *sqlAccountRepository) getEncryptedAccountNumber(customerID, accountID string) (string, error) {
-	query := `select encrypted_account_number from accounts where customer_id = ? and account_id = ? and deleted_at is null limit 1;`
+func (r *sqlAccountRepository) getEncryptedAccountNumber(organization, customerID, accountID string) (string, error) {
+	query := `select encrypted_account_number 
+from accounts as a 
+inner join customers as c on c.customer_id = a.customer_id 
+where a.customer_id = ? and a.account_id = ? and c.organization = ? and a.deleted_at is null 
+limit 1;`
 	stmt, err := r.db.Prepare(query)
 	if err != nil {
 		return "", err
@@ -216,7 +222,7 @@ func (r *sqlAccountRepository) getEncryptedAccountNumber(customerID, accountID s
 	defer stmt.Close()
 
 	var encrypted string
-	if err := stmt.QueryRow(customerID, accountID).Scan(&encrypted); err != nil {
+	if err := stmt.QueryRow(customerID, accountID, organization).Scan(&encrypted); err != nil {
 		if err == sql.ErrNoRows {
 			return "", nil
 		}
diff --git a/pkg/accounts/router.go b/pkg/accounts/router.go
index 8ca9ab4..ea64a20 100644
--- a/pkg/accounts/router.go
+++ b/pkg/accounts/router.go
@@ -216,8 +216,11 @@ func decryptAccountNumber(logger log.Logger, repo Repository, keeper *secrets.St
 			return
 		}
 
+		// get organization
+		organization := route.GetOrganization(w, r)
+
 		// grab encrypted value
-		encrypted, err := repo.getEncryptedAccountNumber(customerID, accountID)
+		encrypted, err := repo.getEncryptedAccountNumber(organization, customerID, accountID)
 		if err != nil {
 			moovhttp.Problem(w, err)
 			return
diff --git a/pkg/accounts/validate.go b/pkg/accounts/validate.go
index 6d3608f..177918d 100644
--- a/pkg/accounts/validate.go
+++ b/pkg/accounts/validate.go
@@ -192,8 +192,11 @@ func completeAccountValidation(logger log.Logger, repo Repository, validations v
 			return
 		}
 
+		// get organization
+		organization := route.GetOrganization(w, r)
+
 		// grab encrypted account number
-		encrypted, err := repo.getEncryptedAccountNumber(customerID, accountID)
+		encrypted, err := repo.getEncryptedAccountNumber(organization, customerID, accountID)
 		if err != nil {
 			moovhttp.Problem(w, err)
 			return


Test patch:
diff --git a/pkg/accounts/repository_test.go b/pkg/accounts/repository_test.go
index e51de35..91bf0c4 100644
--- a/pkg/accounts/repository_test.go
+++ b/pkg/accounts/repository_test.go
@@ -12,6 +12,7 @@ import (
 
 	"github.com/moov-io/customers/internal/database"
 	"github.com/moov-io/customers/pkg/client"
+	"github.com/moov-io/customers/pkg/customers"
 	"github.com/moov-io/customers/pkg/secrets"
 
 	"github.com/go-kit/kit/log"
@@ -79,6 +80,7 @@ func TestRepository(t *testing.T) {
 
 func TestRepository__getEncryptedAccountNumber(t *testing.T) {
 	customerID, userID := base.ID(), base.ID()
+	organization := "test-org"
 	repo := setupTestAccountRepository(t)
 
 	keeper := secrets.TestStringKeeper(t)
@@ -97,8 +99,21 @@ func TestRepository__getEncryptedAccountNumber(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	// create customer
+	customerRepo := customers.NewCustomerRepo(log.NewNopLogger(), repo.db.DB)
+	cust := &client.Customer{
+		CustomerID: customerID,
+		FirstName:  "jane",
+		LastName:   "doe",
+		Type:       client.INDIVIDUAL,
+	}
+	custErr := customerRepo.CreateCustomer(cust, organization)
+	if custErr != nil {
+		t.Fatal(custErr)
+	}
+
 	// read encrypted account number
-	encrypted, err := repo.getEncryptedAccountNumber(customerID, acct.AccountID)
+	encrypted, err := repo.getEncryptedAccountNumber(organization, customerID, acct.AccountID)
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/pkg/accounts/router_test.go b/pkg/accounts/router_test.go
index 8a8d2d9..a7eff30 100644
--- a/pkg/accounts/router_test.go
+++ b/pkg/accounts/router_test.go
@@ -19,6 +19,7 @@ import (
 
 	"github.com/moov-io/customers/internal/testclient"
 	"github.com/moov-io/customers/pkg/client"
+	"github.com/moov-io/customers/pkg/customers"
 	"github.com/moov-io/customers/pkg/fed"
 	"github.com/moov-io/customers/pkg/secrets"
 	"github.com/moov-io/customers/pkg/validator"
@@ -44,7 +45,9 @@ func createTestOFACSearcher(repo Repository, watchmanClient watchman.Client) *Ac
 func TestAccountRoutes(t *testing.T) {
 	customerID := base.ID()
 
-	handler := setupRouter(t)
+	repo := setupTestAccountRepository(t)
+	setupCustomerWithOrganization(t, customerID, "moov", repo)
+	handler := setupRouterWithTestAccountRepo(t, repo)
 
 	// first read, expect no accounts
 	accounts := httpReadAccounts(t, handler, customerID)
@@ -191,17 +194,31 @@ func TestGetAccountByID(t *testing.T) {
 }
 
 func TestRoutes__DecryptAccountNumber(t *testing.T) {
-	customerID := base.ID()
+	customerID, userID := base.ID(), base.ID()
+	organization := "test-org"
 
-	handler := setupRouter(t)
+	repo := setupTestAccountRepository(t)
+	keeper := secrets.TestStringKeeper(t)
 
-	// create an account
-	account := httpCreateAccount(t, handler, customerID)
-	if account == nil {
-		t.Fatal("missing account")
+	handler := setupRouterWithTestAccountRepo(t, repo)
+
+	// create account
+	req := &CreateAccountRequest{
+		AccountNumber: "123",
+		RoutingNumber: "987654320",
+		Type:          client.CHECKING,
+	}
+	if err := req.disfigure(keeper); err != nil {
+		t.Fatal(err)
+	}
+	account, err := repo.CreateCustomerAccount(customerID, userID, req)
+	if err != nil {
+		t.Fatal(err)
 	}
 
-	httpDecryptAccountNumber(t, handler, customerID, account.AccountID)
+	setupCustomerWithOrganization(t, customerID, organization, repo)
+
+	httpDecryptAccountNumber(t, handler, customerID, account.AccountID, organization)
 }
 
 func TestRoutes__EmptyAccounts(t *testing.T) {
@@ -285,6 +302,21 @@ func setupRouter(t *testing.T) *mux.Router {
 	return handler
 }
 
+func setupRouterWithTestAccountRepo(t *testing.T, testRepo *testAccountRepository) *mux.Router {
+	handler := mux.NewRouter()
+
+	testFedClient := &fed.MockClient{}
+	validations := &validator.MockRepository{}
+	keeper := secrets.TestStringKeeper(t)
+	validationStrategies := map[validator.StrategyKey]validator.Strategy{
+		{Strategy: "test", Vendor: "moov"}: testvalidator.NewStrategy(),
+	}
+
+	RegisterRoutes(log.NewNopLogger(), handler, testRepo, validations, testFedClient, keeper, keeper, validationStrategies, createTestOFACSearcher(testRepo, nil))
+
+	return handler
+}
+
 func setupRouterWithRepo(t *testing.T, repo *mockRepository) *mux.Router {
 	handler := mux.NewRouter()
 
@@ -300,6 +332,22 @@ func setupRouterWithRepo(t *testing.T, repo *mockRepository) *mux.Router {
 	return handler
 }
 
+func setupCustomerWithOrganization(t *testing.T, customerID, organization string, testRepo *testAccountRepository) *client.Customer {
+	customerRepo := customers.NewCustomerRepo(log.NewNopLogger(), testRepo.db.DB)
+	cust := &client.Customer{
+		CustomerID: customerID,
+		FirstName:  "jane",
+		LastName:   "doe",
+		Type:       client.INDIVIDUAL,
+	}
+	custErr := customerRepo.CreateCustomer(cust, organization)
+	if custErr != nil {
+		t.Fatal(custErr)
+	}
+
+	return cust
+}
+
 func httpReadAccounts(t *testing.T, handler *mux.Router, customerID string) []*client.Account {
 	w := httptest.NewRecorder()
 	req := httptest.NewRequest("GET", fmt.Sprintf("/customers/%s/accounts", customerID), nil)
@@ -394,6 +442,7 @@ func httpCompleteAccountValidation(t *testing.T, handler *mux.Router, customerID
 
 	w := httptest.NewRecorder()
 	req := httptest.NewRequest("PUT", fmt.Sprintf("/customers/%s/accounts/%s/validations/%s", customerID, accountID, validationID), body)
+	req.Header.Set("X-Organization", "moov")
 	handler.ServeHTTP(w, req)
 	w.Flush()
 
@@ -429,9 +478,10 @@ func httpDeleteAccount(t *testing.T, handler *mux.Router, customerID, accountID
 	}
 }
 
-func httpDecryptAccountNumber(t *testing.T, handler *mux.Router, customerID, accountID string) {
+func httpDecryptAccountNumber(t *testing.T, handler *mux.Router, customerID, accountID, organization string) {
 	w := httptest.NewRecorder()
 	req := httptest.NewRequest("POST", fmt.Sprintf("/customers/%s/accounts/%s/decrypt", customerID, accountID), nil)
+	req.Header.Set("X-Organization", organization)
 	handler.ServeHTTP(w, req)
 
 	if w.Code != http.StatusOK {
diff --git a/pkg/accounts/validate_test.go b/pkg/accounts/validate_test.go
index 8dc60cc..3f76208 100644
--- a/pkg/accounts/validate_test.go
+++ b/pkg/accounts/validate_test.go
@@ -8,6 +8,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"github.com/moov-io/customers/pkg/customers"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -30,6 +31,7 @@ import (
 
 func TestRouter__AccountValidation(t *testing.T) {
 	customerID, userID := base.ID(), base.ID()
+	organization := "moov"
 	accounts := setupTestAccountRepository(t)
 	validations := &validator.MockRepository{}
 
@@ -46,6 +48,7 @@ func TestRouter__AccountValidation(t *testing.T) {
 		req, err := http.NewRequest("GET", "/", nil)
 		require.NoError(t, err)
 		req.Header.Set("Content-Type", "application/json")
+		req.Header.Set("X-Organization", organization)
 		req = mux.SetURLVars(req, map[string]string{
 			"customerID":   customerID,
 			"accountID":    acc.AccountID,
@@ -245,6 +248,7 @@ func TestRouter__InitAccountValidation(t *testing.T) {
 
 func TestRouter__CompleteAccountValidation(t *testing.T) {
 	customerID, userID := base.ID(), base.ID()
+	organization := "moov"
 	repo := setupTestAccountRepository(t)
 	keeper := secrets.TestStringKeeper(t)
 	validations := &validator.MockRepository{}
@@ -360,6 +364,18 @@ func TestRouter__CompleteAccountValidation(t *testing.T) {
 		})
 		require.NoError(t, err)
 
+		customerRepo := customers.NewCustomerRepo(log.NewNopLogger(), repo.db.DB)
+		cust := &client.Customer{
+			CustomerID: customerID,
+			FirstName:  "mary",
+			LastName:   "doe",
+			Type:       client.INDIVIDUAL,
+		}
+		custErr := customerRepo.CreateCustomer(cust, organization)
+		if custErr != nil {
+			t.Fatal(custErr)
+		}
+
 		validation := &validator.Validation{
 			AccountID: acc.AccountID,
 			Strategy:  "test",
@@ -385,6 +401,7 @@ func TestRouter__CompleteAccountValidation(t *testing.T) {
 		req, err := http.NewRequest("POST", "/", body)
 		require.NoError(t, err)
 		req.Header.Set("Content-Type", "application/json")
+		req.Header.Set("X-Organization", organization)
 		req = mux.SetURLVars(req, map[string]string{
 			"customerID":   customerID,
 			"accountID":    acc.AccountID,
@@ -419,6 +436,8 @@ func TestRouter__CompleteAccountValidation(t *testing.T) {
 	// has own tests. The only value I see right now is that it shows how
 	// request for microdeposits strategy may look like...
 	t.Run("Test micro-deposits strategy", func(t *testing.T) {
+		// Redefine customerID and userID to use new unique values
+		customerID, userID := base.ID(), base.ID()
 		encrypted, err := keeper.EncryptString("12345")
 		require.NoError(t, err)
 
@@ -451,6 +470,18 @@ func TestRouter__CompleteAccountValidation(t *testing.T) {
 			{Strategy: "micro-deposits", Vendor: "moov"}: microdeposits.NewStrategy(paygateClient),
 		}
 
+		customerRepo := customers.NewCustomerRepo(log.NewNopLogger(), repo.db.DB)
+		cust := &client.Customer{
+			CustomerID: customerID,
+			FirstName:  "john",
+			LastName:   "doe",
+			Type:       client.INDIVIDUAL,
+		}
+		custErr := customerRepo.CreateCustomer(cust, organization)
+		if custErr != nil {
+			t.Fatal(custErr)
+		}
+
 		// build request with strategy params
 		params := &client.CompleteAccountValidationRequest{
 			VendorRequest: validator.VendorRequest{
@@ -467,6 +498,7 @@ func TestRouter__CompleteAccountValidation(t *testing.T) {
 		req, err := http.NewRequest("PUT", "/", body)
 		require.NoError(t, err)
 		req.Header.Set("Content-Type", "application/json")
+		req.Header.Set("X-Organization", organization)
 		req = mux.SetURLVars(req, map[string]string{
 			"customerID":   customerID,
 			"accountID":    acc.AccountID,
diff --git a/pkg/customers/customers_test.go b/pkg/customers/customers_test.go
index a2a06e5..0f372be 100644
--- a/pkg/customers/customers_test.go
+++ b/pkg/customers/customers_test.go
@@ -225,7 +225,7 @@ func TestCustomers__DeleteCustomer(t *testing.T) {
 	require.Nil(t, got)
 }
 
-func TestCustomerRepository__createCustomer(t *testing.T) {
+func TestCustomerRepository__CreateCustomer(t *testing.T) {
 	check := func(t *testing.T, repo *sqlCustomerRepository) {
 		cust, _, _ := (customerRequest{
 			FirstName: "Jane",
@@ -340,7 +340,7 @@ func TestCustomers__addressValidate(t *testing.T) {
 	}
 }
 
-func TestCustomers__createCustomer(t *testing.T) {
+func TestCustomers__CreateCustomer(t *testing.T) {
 	w := httptest.NewRecorder()
 	phone := `{"number": "555.555.5555", "type": "mobile"}`
 	address := `{"type": "home", "address1": "123 1st St", "city": "Denver", "state": "CO", "postalCode": "12345", "country": "USA"}`