Repository: moov-io/customers
Pull request #45 (2019-10-21 20:35:06)

cmd/server: allow email, phones, and addresses to be optional on a Customer
Creating a customer might involve adding data in several steps. This is the case with paygate where initially there isn't enough information to always create a Customer. In the case of a Receiver there's no SSN to attach (and one isn't needed to push money at a Receiver).

Let's loosen the requirements for a Customer and verify the data when status transitions happen. 

Issue: https://github.com/moov-io/paygate/issues/98


Patch:
diff --git a/cmd/server/approval.go b/cmd/server/approval.go
index 5534438..5e95da9 100644
--- a/cmd/server/approval.go
+++ b/cmd/server/approval.go
@@ -38,8 +38,8 @@ var (
 // addApprovalRoutes contains "back office" admin endpoints used to validate (or reject) a Customer
 // TODO(adam): We need to hide these behind an admin level auth, but we'll write them for now.
 // What about a header like x-admin-id ??
-func addApprovalRoutes(logger log.Logger, svc *admin.Server, repo customerRepository, ofac *ofacSearcher) {
-	svc.AddHandler("/customers/{customerID}/status", updateCustomerStatus(logger, repo, ofac))
+func addApprovalRoutes(logger log.Logger, svc *admin.Server, repo customerRepository, customerSSNRepo customerSSNRepository, ofac *ofacSearcher) {
+	svc.AddHandler("/customers/{customerID}/status", updateCustomerStatus(logger, repo, customerSSNRepo, ofac))
 	svc.AddHandler("/customers/{customerID}/addresses/{addressId}", updateCustomerAddress(logger, repo))
 }
 
@@ -54,7 +54,7 @@ type updateCustomerStatusRequest struct {
 //  - KYC is only valid if the Customer has first, last, address, and date of birth
 //  - OFAC can only be after an OFAC search has been performed (and search info recorded)
 //  - CIP can only be if the SSN has been set
-func validCustomerStatusTransition(existing *client.Customer, futureStatus CustomerStatus, repo customerRepository, ofac *ofacSearcher, requestID string) error {
+func validCustomerStatusTransition(existing *client.Customer, ssn *SSN, futureStatus CustomerStatus, repo customerRepository, ofac *ofacSearcher, requestID string) error {
 	eql := func(s string, status CustomerStatus) bool {
 		return strings.EqualFold(s, string(status))
 	}
@@ -70,6 +70,9 @@ func validCustomerStatusTransition(existing *client.Customer, futureStatus Custo
 		if existing.BirthDate.IsZero() {
 			return fmt.Errorf("customer=%s is missing date of birth", existing.ID)
 		}
+		if existing.Email == "" {
+			return fmt.Errorf("customer=%s is missing an email address", existing.ID)
+		}
 		if !containsValidPrimaryAddress(existing.Addresses) {
 			return fmt.Errorf("customer=%s is missing a valid primary Address", existing.ID)
 		}
@@ -94,7 +97,10 @@ func validCustomerStatusTransition(existing *client.Customer, futureStatus Custo
 	case CustomerStatusCIP: // TODO(adam): need to impl lookup
 		// What can we do to validate an SSN?
 		// https://www.ssa.gov/employer/randomization.html (not much)
-		return fmt.Errorf("customers=%s %s to CIP transition needs to lookup encrypted SSN", existing.ID, existing.Status)
+		if ssn == nil || len(ssn.encrypted) == 0 {
+			return fmt.Errorf("customer=%s is missing SSN", existing.ID)
+		}
+		return fmt.Errorf("customer=%s %s to CIP transition needs to lookup encrypted SSN", existing.ID, existing.Status)
 	}
 	return nil
 }
@@ -108,7 +114,7 @@ func containsValidPrimaryAddress(addrs []client.Address) bool {
 	return false
 }
 
-func updateCustomerStatus(logger log.Logger, repo customerRepository, ofac *ofacSearcher) http.HandlerFunc {
+func updateCustomerStatus(logger log.Logger, repo customerRepository, customerSSNRepo customerSSNRepository, ofac *ofacSearcher) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		w = wrapResponseWriter(logger, w, r)
 
@@ -133,7 +139,13 @@ func updateCustomerStatus(logger log.Logger, repo customerRepository, ofac *ofac
 			moovhttp.Problem(w, err)
 			return
 		}
-		if err := validCustomerStatusTransition(cust, req.Status, repo, ofac, requestID); err != nil {
+
+		ssn, err := customerSSNRepo.getCustomerSSN(customerID)
+		if err != nil {
+			moovhttp.Problem(w, err)
+			return
+		}
+		if err := validCustomerStatusTransition(cust, ssn, req.Status, repo, ofac, requestID); err != nil {
 			moovhttp.Problem(w, err)
 			return
 		}
diff --git a/cmd/server/customers.go b/cmd/server/customers.go
index 4ceeab2..8192f4d 100644
--- a/cmd/server/customers.go
+++ b/cmd/server/customers.go
@@ -157,15 +157,6 @@ func (req customerRequest) validate() error {
 	if req.FirstName == "" || req.LastName == "" {
 		return errors.New("create customer: empty name field(s)")
 	}
-	if req.Email == "" {
-		return errors.New("create customer: empty email")
-	}
-	if len(req.Phones) == 0 {
-		return errors.New("create customer: phone array is required")
-	}
-	if len(req.Addresses) == 0 {
-		return errors.New("create customer: address array is required")
-	}
 	if err := validateMetadata(req.Metadata); err != nil {
 		return fmt.Errorf("create customer: %v", err)
 	}
@@ -216,8 +207,11 @@ func (req customerRequest) asCustomer(storage *ssnStorage) (*client.Customer, *S
 			Active:     true,
 		})
 	}
-	ssn, err := storage.encryptRaw(customer.ID, req.SSN)
-	return customer, ssn, err
+	if req.SSN != "" {
+		ssn, err := storage.encryptRaw(customer.ID, req.SSN)
+		return customer, ssn, err
+	}
+	return customer, nil, nil
 }
 
 func createCustomer(logger log.Logger, repo customerRepository, customerSSNStorage *ssnStorage, ofac *ofacSearcher) http.HandlerFunc {
@@ -244,10 +238,13 @@ func createCustomer(logger log.Logger, repo customerRepository, customerSSNStora
 			moovhttp.Problem(w, err)
 			return
 		}
-		if err := customerSSNStorage.repo.saveCustomerSSN(ssn); err != nil {
-			logger.Log("customers", fmt.Sprintf("problem saving SSN for Customer=%s: %v", cust.ID, err), "requestID", requestID)
-			moovhttp.Problem(w, fmt.Errorf("saveCustomerSSN: %v", err))
-			return
+		if ssn != nil {
+			err := customerSSNStorage.repo.saveCustomerSSN(ssn)
+			if err != nil {
+				logger.Log("customers", fmt.Sprintf("problem saving SSN for Customer=%s: %v", cust.ID, err), "requestID", requestID)
+				moovhttp.Problem(w, fmt.Errorf("saveCustomerSSN: %v", err))
+				return
+			}
 		}
 		if err := repo.createCustomer(cust); err != nil {
 			if requestID != "" {
diff --git a/cmd/server/main.go b/cmd/server/main.go
index 2aaabab..2d2e9ed 100644
--- a/cmd/server/main.go
+++ b/cmd/server/main.go
@@ -113,7 +113,7 @@ func main() {
 	}
 
 	// Register our admin routes
-	addApprovalRoutes(logger, adminServer, customerRepo, ofac)
+	addApprovalRoutes(logger, adminServer, customerRepo, customerSSNRepo, ofac)
 	addDisclaimerAdminRoutes(logger, adminServer, disclaimerRepo, documentRepo)
 
 	// Setup Customer SSN storage wrapper


Test patch:
diff --git a/cmd/server/approval_test.go b/cmd/server/approval_test.go
index 7a14984..8decb0c 100644
--- a/cmd/server/approval_test.go
+++ b/cmd/server/approval_test.go
@@ -31,10 +31,11 @@ func TestCustomers__updateCustomerStatus(t *testing.T) {
 		},
 	}
 	searcher := createTestOFACSearcher(repo, nil)
+	ssnRepo := &testCustomerSSNRepository{}
 
 	svc := admin.NewServer(":10001")
 	defer svc.Shutdown()
-	addApprovalRoutes(log.NewNopLogger(), svc, repo, searcher)
+	addApprovalRoutes(log.NewNopLogger(), svc, repo, ssnRepo, searcher)
 	go svc.Listen()
 
 	body := strings.NewReader(`{"status": "ReviewRequired", "comment": "test comment"}`)
@@ -92,10 +93,11 @@ func TestCustomers__getAddressId(t *testing.T) {
 func TestCustomers__updateCustomerStatusFailure(t *testing.T) {
 	repo := &testCustomerRepository{}
 	searcher := createTestOFACSearcher(repo, nil)
+	ssnRepo := &testCustomerSSNRepository{}
 
 	w := httptest.NewRecorder()
 	req := httptest.NewRequest("GET", "/customers/foo/status", nil)
-	updateCustomerStatus(log.NewNopLogger(), repo, searcher)(w, req)
+	updateCustomerStatus(log.NewNopLogger(), repo, ssnRepo, searcher)(w, req)
 	w.Flush()
 
 	if w.Code != http.StatusBadRequest {
@@ -105,7 +107,7 @@ func TestCustomers__updateCustomerStatusFailure(t *testing.T) {
 	// try the proper HTTP verb
 	w = httptest.NewRecorder()
 	req = httptest.NewRequest("PUT", "/customers/foo/status", nil)
-	updateCustomerStatus(log.NewNopLogger(), repo, searcher)(w, req)
+	updateCustomerStatus(log.NewNopLogger(), repo, ssnRepo, searcher)(w, req)
 	w.Flush()
 
 	if w.Code != http.StatusBadRequest {
@@ -144,28 +146,30 @@ func TestCustomers__validCustomerStatusTransition(t *testing.T) {
 	repo := &testCustomerRepository{}
 	searcher := createTestOFACSearcher(repo, nil)
 
-	if err := validCustomerStatusTransition(cust, CustomerStatusDeceased, repo, searcher, "requestID"); err != nil {
+	ssn := &SSN{customerID: cust.ID, encrypted: []byte("secret")}
+
+	if err := validCustomerStatusTransition(cust, ssn, CustomerStatusDeceased, repo, searcher, "requestID"); err != nil {
 		t.Errorf("expected no error: %v", err)
 	}
 
 	// block Deceased and Rejected customers
 	cust.Status = CustomerStatusDeceased
-	if err := validCustomerStatusTransition(cust, CustomerStatusKYC, repo, searcher, "requestID"); err == nil {
+	if err := validCustomerStatusTransition(cust, ssn, CustomerStatusKYC, repo, searcher, "requestID"); err == nil {
 		t.Error("expected error")
 	}
 	cust.Status = CustomerStatusRejected
-	if err := validCustomerStatusTransition(cust, CustomerStatusKYC, repo, searcher, "requestID"); err == nil {
+	if err := validCustomerStatusTransition(cust, ssn, CustomerStatusKYC, repo, searcher, "requestID"); err == nil {
 		t.Error("expected error")
 	}
 
 	// normal KYC approval (rejected due to missing info)
 	cust.FirstName, cust.LastName = "Jane", "Doe"
 	cust.Status = CustomerStatusReviewRequired
-	if err := validCustomerStatusTransition(cust, CustomerStatusKYC, repo, searcher, "requestID"); err == nil {
+	if err := validCustomerStatusTransition(cust, ssn, CustomerStatusKYC, repo, searcher, "requestID"); err == nil {
 		t.Error("expected error")
 	}
 	cust.BirthDate = time.Now()
-	if err := validCustomerStatusTransition(cust, CustomerStatusKYC, repo, searcher, "requestID"); err == nil {
+	if err := validCustomerStatusTransition(cust, ssn, CustomerStatusKYC, repo, searcher, "requestID"); err == nil {
 		t.Error("expected error")
 	}
 	cust.Addresses = append(cust.Addresses, client.Address{
@@ -175,7 +179,14 @@ func TestCustomers__validCustomerStatusTransition(t *testing.T) {
 
 	// CIP transistions are WIP // TODO(adam):
 	cust.Status = CustomerStatusReviewRequired
-	if err := validCustomerStatusTransition(cust, CustomerStatusCIP, repo, searcher, "requestID"); err == nil {
+	if err := validCustomerStatusTransition(cust, nil, CustomerStatusCIP, repo, searcher, "requestID"); err != nil {
+		if !strings.Contains(err.Error(), "is missing SSN") {
+			t.Errorf("CIP: unexpected error: %v", err)
+		}
+	} else {
+		t.Error("CIP transition is WIP")
+	}
+	if err := validCustomerStatusTransition(cust, ssn, CustomerStatusCIP, repo, searcher, "requestID"); err == nil {
 		t.Error("CIP transition is WIP")
 	}
 }
@@ -189,14 +200,16 @@ func TestCustomers__validCustomerStatusTransitionError(t *testing.T) {
 	ofacClient := &testOFACClient{}
 	searcher := createTestOFACSearcher(repo, ofacClient)
 
+	ssn := &SSN{customerID: cust.ID, encrypted: []byte("secret")}
+
 	repo.err = errors.New("bad error")
-	if err := validCustomerStatusTransition(cust, CustomerStatusOFAC, repo, searcher, ""); err == nil {
+	if err := validCustomerStatusTransition(cust, ssn, CustomerStatusOFAC, repo, searcher, ""); err == nil {
 		t.Error("expected error, but got none")
 	}
 	repo.err = nil
 
 	ofacClient.err = errors.New("bad error")
-	if err := validCustomerStatusTransition(cust, CustomerStatusOFAC, repo, searcher, ""); err == nil {
+	if err := validCustomerStatusTransition(cust, ssn, CustomerStatusOFAC, repo, searcher, ""); err == nil {
 		t.Error("expected error, but got none")
 	}
 }
@@ -209,17 +222,19 @@ func TestCustomers__validCustomerStatusTransitionOFAC(t *testing.T) {
 	repo := &testCustomerRepository{}
 	searcher := createTestOFACSearcher(repo, nil)
 
+	ssn := &SSN{customerID: cust.ID, encrypted: []byte("secret")}
+
 	repo.ofacSearchResult = &ofacSearchResult{
 		sdnName: "Jane Doe",
 		match:   0.10,
 	}
-	if err := validCustomerStatusTransition(cust, CustomerStatusOFAC, repo, searcher, "requestID"); err != nil {
+	if err := validCustomerStatusTransition(cust, ssn, CustomerStatusOFAC, repo, searcher, "requestID"); err != nil {
 		t.Errorf("unexpected error in OFAC transition: %v", err)
 	}
 
 	// OFAC transition with positive match
 	repo.ofacSearchResult.match = 0.99
-	if err := validCustomerStatusTransition(cust, CustomerStatusOFAC, repo, searcher, "requestID"); err != nil {
+	if err := validCustomerStatusTransition(cust, ssn, CustomerStatusOFAC, repo, searcher, "requestID"); err != nil {
 		if !strings.Contains(err.Error(), "positive OFAC match") {
 			t.Errorf("unexpected error in OFAC transition: %v", err)
 		}
@@ -232,7 +247,7 @@ func TestCustomers__validCustomerStatusTransitionOFAC(t *testing.T) {
 			EntityID: "12124",
 		}
 	}
-	if err := validCustomerStatusTransition(cust, CustomerStatusOFAC, repo, searcher, "requestID"); err != nil {
+	if err := validCustomerStatusTransition(cust, ssn, CustomerStatusOFAC, repo, searcher, "requestID"); err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
 	if repo.savedOFACSearchResult.entityId != "12124" {
@@ -247,10 +262,11 @@ func TestCustomers__updateCustomerAddress(t *testing.T) {
 		},
 	}
 	searcher := createTestOFACSearcher(repo, nil)
+	ssnRepo := &testCustomerSSNRepository{}
 
 	svc := admin.NewServer(":10002")
 	defer svc.Shutdown()
-	addApprovalRoutes(log.NewNopLogger(), svc, repo, searcher)
+	addApprovalRoutes(log.NewNopLogger(), svc, repo, ssnRepo, searcher)
 	go svc.Listen()
 
 	body := strings.NewReader(`{"type": "primary", "validated": true}`)
diff --git a/cmd/server/customers_test.go b/cmd/server/customers_test.go
index 0d11099..567ff07 100644
--- a/cmd/server/customers_test.go
+++ b/cmd/server/customers_test.go
@@ -9,13 +9,14 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"github.com/moov-io/customers/internal/database"
 	"net/http"
 	"net/http/httptest"
 	"strconv"
 	"strings"
 	"testing"
 
+	"github.com/moov-io/customers/internal/database"
+
 	"github.com/moov-io/base"
 	client "github.com/moov-io/customers/client"
 
@@ -205,21 +206,28 @@ func TestCustomers__customerRequest(t *testing.T) {
 		t.Error("expected error")
 	}
 	req.FirstName = "jane"
-	req.LastName = "doe"
 	if err := req.validate(); err == nil {
 		t.Error("expected error")
 	}
+
+	req.LastName = "doe"
+	if err := req.validate(); err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
 	req.Email = "jane.doe@example.com"
-	if err := req.validate(); err == nil {
-		t.Error("expected error")
+	if err := req.validate(); err != nil {
+		t.Errorf("unexpected error: %v", err)
 	}
+
 	req.Phones = append(req.Phones, phone{
 		Number: "123.456.7890",
 		Type:   "Checking",
 	})
-	if err := req.validate(); err == nil {
-		t.Error("expected error")
+	if err := req.validate(); err != nil {
+		t.Errorf("unexpected error: %v", err)
 	}
+
 	req.Addresses = append(req.Addresses, address{
 		Address1:   "123 1st st",
 		City:       "fake city",
@@ -734,11 +742,9 @@ func TestCustomers__MetaDataValidate(t *testing.T) {
 	}
 }
 
-func TestCustomers__NoSSN(t *testing.T) {
+func TestCustomers__minimumFields(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"}`
-	body := fmt.Sprintf(`{"firstName": "jane", "lastName": "doe", "email": "jane@example.com", "ssn": "", "phones": [%s], "addresses": [%s]}`, phone, address)
+	body := `{"firstName": "jane", "lastName": "doe"}`
 	req := httptest.NewRequest("POST", "/customers", strings.NewReader(body))
 	req.Header.Set("x-user-id", "test")
 	req.Header.Set("x-request-id", "test")
@@ -753,13 +759,9 @@ func TestCustomers__NoSSN(t *testing.T) {
 	router.ServeHTTP(w, req)
 	w.Flush()
 
-	if w.Code != http.StatusBadRequest {
+	if w.Code != http.StatusOK {
 		t.Errorf("bogus status code: %d: %v", w.Code, w.Body.String())
 	}
-
-	if !strings.Contains(w.Body.String(), "SSN") {
-		t.Errorf("Expected SSN error received %s", w.Body.String())
-	}
 }
 
 func TestCustomers__BadReq(t *testing.T) {