Repository: moov-io/customers (go)

Issue:
accounts: GET /accounts/:id does not validate organization
<!-- Please fill out the following questions, thanks! -->

Customers Version: `v0.5.0-dev23`

**What were you trying to do?**
When loading `GET /customers/{customerID}/accounts` the `X-Organization` header is not checked such that Customer (for the Account(s)) belongs to the Organization.

The same applies on other Account related endpoints. 

There's no check in the endpoint currently

https://github.com/moov-io/customers/blob/v0.5.0-dev23/pkg/accounts/router.go#L46-L62
https://github.com/moov-io/customers/blob/v0.5.0-dev23/pkg/accounts/router.go#L168-L199

**What did you expect to see?**
The `X-Organization` header is used to filter Accounts returned - often by authentication systems. 

Pull request:
validate that customer belongs to a ceratin org when fetching accounts
Fixes #249 

Patch:
diff --git a/pkg/accounts/mock_repository.go b/pkg/accounts/mock_repository.go
index 33adb63..ad6361f 100644
--- a/pkg/accounts/mock_repository.go
+++ b/pkg/accounts/mock_repository.go
@@ -33,7 +33,7 @@ func (r *mockRepository) GetCustomerAccountsByIDs(accountIDs []string) ([]*clien
 	return r.Accounts, nil
 }
 
-func (r *mockRepository) getAccountsByCustomerID(customerID string) ([]*client.Account, error) {
+func (r *mockRepository) getAccounts(customerID string, organization string) ([]*client.Account, error) {
 	if r.Err != nil {
 		return nil, r.Err
 	}
diff --git a/pkg/accounts/repository.go b/pkg/accounts/repository.go
index de63dc9..0033bf0 100644
--- a/pkg/accounts/repository.go
+++ b/pkg/accounts/repository.go
@@ -21,7 +21,7 @@ import (
 type Repository interface {
 	getCustomerAccount(customerID, accountID string) (*client.Account, error)
 	GetCustomerAccountsByIDs(accountIDs []string) ([]*client.Account, error)
-	getAccountsByCustomerID(customerID string) ([]*client.Account, error)
+	getAccounts(customerID string, organization string) ([]*client.Account, error)
 
 	CreateCustomerAccount(customerID, userID string, req *CreateAccountRequest) (*client.Account, error)
 	deactivateCustomerAccount(accountID string) error
@@ -111,15 +111,15 @@ func (r *sqlAccountRepository) getCustomerAccount(customerID, accountID string)
 	return &a, nil
 }
 
-func (r *sqlAccountRepository) getAccountsByCustomerID(customerID string) ([]*client.Account, error) {
-	query := `select account_id from accounts where customer_id = ? and deleted_at is null;`
+func (r *sqlAccountRepository) getAccounts(customerID string, organization string) ([]*client.Account, error) {
+	query := `select a.account_id from accounts as a left outer join customers as c on a.customer_id = c.customer_id where a.customer_id = ? and c.organization = ? and a.deleted_at is null;`
 	stmt, err := r.db.Prepare(query)
 	if err != nil {
 		return nil, err
 	}
 	defer stmt.Close()
 
-	rows, err := stmt.Query(customerID)
+	rows, err := stmt.Query(customerID, organization)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/accounts/router.go b/pkg/accounts/router.go
index 8f878ac..ea849d4 100644
--- a/pkg/accounts/router.go
+++ b/pkg/accounts/router.go
@@ -50,9 +50,18 @@ func getCustomerAccounts(logger log.Logger, repo Repository, fedClient fed.Clien
 		w = route.Responder(logger, w, r)
 
 		customerID := route.GetCustomerID(w, r)
-		accounts, err := repo.getAccountsByCustomerID(customerID)
+		if customerID == "" {
+			return
+		}
+
+		organization := route.GetOrganization(w, r)
+		if organization == "" {
+			return
+		}
+
+		accounts, err := repo.getAccounts(customerID, organization)
 		if err != nil {
-			moovhttp.Problem(w, err)
+			moovhttp.Problem(w, fmt.Errorf("getting accounts: %v", err))
 			return
 		}
 		accounts = decorateInstitutionDetails(accounts, fedClient)


Test patch:
diff --git a/pkg/accounts/repository_test.go b/pkg/accounts/repository_test.go
index 79bcc8c..2482072 100644
--- a/pkg/accounts/repository_test.go
+++ b/pkg/accounts/repository_test.go
@@ -36,21 +36,34 @@ func setupTestAccountRepository(t *testing.T) *testAccountRepository {
 }
 
 func TestRepository(t *testing.T) {
-	customerID, userID := base.ID(), base.ID()
-	repo := setupTestAccountRepository(t)
+	userID := base.ID()
+	logger := log.NewNopLogger()
+	db := database.CreateTestSqliteDB(t).DB
+	customerRepo := customers.NewCustomerRepo(logger, db)
+	accountRepo := NewRepo(logger, db)
+	organization := "test"
+
+	customer := &client.Customer{
+		CustomerID: base.ID(),
+		FirstName:  "jane",
+		LastName:   "doe",
+		Type:       client.CUSTOMERTYPE_INDIVIDUAL,
+	}
+	err := customerRepo.CreateCustomer(customer, organization)
+	require.NoError(t, err)
 
 	// look for account that does not exist
-	_, err := repo.getCustomerAccount(customerID, "xxx")
+	_, err = accountRepo.getCustomerAccount(customer.CustomerID, "xxx")
 	require.Error(t, err)
 
 	// initial read, find no accounts
-	accounts, err := repo.getAccountsByCustomerID(customerID)
+	accounts, err := accountRepo.getAccounts(customer.CustomerID, organization)
 	if len(accounts) != 0 || err != nil {
 		t.Fatalf("got accounts=%#v error=%v", accounts, err)
 	}
 
 	// create account
-	acct, err := repo.CreateCustomerAccount(customerID, userID, &CreateAccountRequest{
+	acct, err := accountRepo.CreateCustomerAccount(customer.CustomerID, userID, &CreateAccountRequest{
 		AccountNumber: "123",
 		RoutingNumber: "987654320",
 		Type:          client.ACCOUNTTYPE_CHECKING,
@@ -60,7 +73,7 @@ func TestRepository(t *testing.T) {
 	}
 
 	// read after creating
-	accounts, err = repo.getAccountsByCustomerID(customerID)
+	accounts, err = accountRepo.getAccounts(customer.CustomerID, organization)
 	if len(accounts) != 1 || err != nil {
 		t.Fatalf("got accounts=%#v error=%v", accounts, err)
 	}
@@ -69,10 +82,10 @@ func TestRepository(t *testing.T) {
 	}
 
 	// delete, expect no accounts
-	if err := repo.deactivateCustomerAccount(acct.AccountID); err != nil {
+	if err := accountRepo.deactivateCustomerAccount(acct.AccountID); err != nil {
 		t.Fatal(err)
 	}
-	accounts, err = repo.getAccountsByCustomerID(customerID)
+	accounts, err = accountRepo.getAccounts(customer.CustomerID, organization)
 	if len(accounts) != 0 || err != nil {
 		t.Fatalf("got accounts=%#v error=%v", accounts, err)
 	}
diff --git a/pkg/accounts/router_test.go b/pkg/accounts/router_test.go
index 0c80f0c..31e445e 100644
--- a/pkg/accounts/router_test.go
+++ b/pkg/accounts/router_test.go
@@ -44,13 +44,14 @@ func createTestOFACSearcher(repo Repository, watchmanClient watchman.Client) *Ac
 
 func TestAccountRoutes(t *testing.T) {
 	customerID := base.ID()
+	org := "moov"
 
 	repo := setupTestAccountRepository(t)
-	setupCustomerWithOrganization(t, customerID, "moov", repo)
+	setupCustomerWithOrganization(t, customerID, org, repo)
 	handler := setupRouterWithTestAccountRepo(t, repo)
 
 	// first read, expect no accounts
-	accounts := httpReadAccounts(t, handler, customerID)
+	accounts := httpReadAccounts(t, handler, customerID, org)
 	if len(accounts) != 0 {
 		t.Errorf("got accounts: %v", accounts)
 	}
@@ -62,7 +63,7 @@ func TestAccountRoutes(t *testing.T) {
 	}
 
 	// re-read, find account
-	accounts = httpReadAccounts(t, handler, customerID)
+	accounts = httpReadAccounts(t, handler, customerID, org)
 	if len(accounts) != 1 || accounts[0].AccountID != account.AccountID {
 		t.Errorf("got accounts: %v", accounts)
 	}
@@ -74,7 +75,7 @@ func TestAccountRoutes(t *testing.T) {
 
 	// delete and expect no accounts
 	httpDeleteAccount(t, handler, customerID, account.AccountID)
-	accounts = httpReadAccounts(t, handler, customerID)
+	accounts = httpReadAccounts(t, handler, customerID, org)
 	if len(accounts) != 0 {
 		t.Errorf("got accounts: %v", accounts)
 	}
@@ -82,11 +83,12 @@ func TestAccountRoutes(t *testing.T) {
 
 func TestCreateAccountAndCheckAccountOfacSearch(t *testing.T) {
 	customerID := base.ID()
+	org := "moov"
 
 	handler := setupRouter(t)
 
 	// first read, expect no accounts
-	accounts := httpReadAccounts(t, handler, customerID)
+	accounts := httpReadAccounts(t, handler, customerID, org)
 	if len(accounts) != 0 {
 		t.Errorf("got accounts: %v", accounts)
 	}
@@ -106,11 +108,12 @@ func TestCreateAccountAndCheckAccountOfacSearch(t *testing.T) {
 
 func TestRefreshAccountAndCheckAccountOfacSearch(t *testing.T) {
 	customerID := base.ID()
+	org := "moov"
 
 	handler := setupRouter(t)
 
 	// first read, expect no accounts
-	accounts := httpReadAccounts(t, handler, customerID)
+	accounts := httpReadAccounts(t, handler, customerID, org)
 	if len(accounts) != 0 {
 		t.Errorf("got accounts: %v", accounts)
 	}
@@ -228,6 +231,7 @@ func TestRoutes__EmptyAccounts(t *testing.T) {
 
 	w := httptest.NewRecorder()
 	req := httptest.NewRequest("GET", fmt.Sprintf("/customers/%s/accounts", customerID), nil)
+	req.Header.Set("X-Organization", "test")
 	handler.ServeHTTP(w, req)
 	w.Flush()
 
@@ -348,9 +352,10 @@ func setupCustomerWithOrganization(t *testing.T, customerID, organization string
 	return cust
 }
 
-func httpReadAccounts(t *testing.T, handler *mux.Router, customerID string) []*client.Account {
+func httpReadAccounts(t *testing.T, handler *mux.Router, customerID string, organization string) []*client.Account {
 	w := httptest.NewRecorder()
 	req := httptest.NewRequest("GET", fmt.Sprintf("/customers/%s/accounts", customerID), nil)
+	req.Header.Set("X-Organization", organization)
 	handler.ServeHTTP(w, req)
 	w.Flush()