From 3d1db91426171448e428a012306be3884b87d224 Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Mon, 15 Jul 2019 16:32:52 -0500 Subject: [PATCH 01/26] internal/filetransfer: create a new package from file transfer code --- common.go | 17 -- common_test.go | 22 --- file_transfer_async.go | 133 +++++-------- file_transfer_async_test.go | 147 +++++++------- http_test.go | 42 +--- .../filetransfer/config.go | 114 +++++------ .../filetransfer/config_test.go | 84 ++++---- internal/filetransfer/file_transfer.go | 92 +++++++++ .../filetransfer/file_transfer_test.go | 187 +++++++++++------- .../filetransfer/ftp.go | 137 ++++++------- internal/httptest/certs.go | 48 +++++ main.go | 7 +- 12 files changed, 548 insertions(+), 482 deletions(-) rename file_transfer_configs.go => internal/filetransfer/config.go (67%) rename file_transfer_configs_test.go => internal/filetransfer/config_test.go (72%) create mode 100644 internal/filetransfer/file_transfer.go rename file_transfer_test.go => internal/filetransfer/file_transfer_test.go (56%) rename file_transfer.go => internal/filetransfer/ftp.go (54%) create mode 100644 internal/httptest/certs.go diff --git a/common.go b/common.go index 9d1351ba9..4feb83d35 100644 --- a/common.go +++ b/common.go @@ -166,23 +166,6 @@ func (a *Amount) UnmarshalJSON(b []byte) error { return a.FromString(s) } -var errTimeout = errors.New("timeout exceeded") - -// try will attempt to call f, but only for as long as t. If the function is still -// processing after t has elapsed then errTimeout will be returned. -func try(f func() error, t time.Duration) error { - answer := make(chan error) - go func() { - answer <- f() - }() - select { - case err := <-answer: - return err - case <-time.After(t): - return errTimeout - } -} - // startOfDayAndTomorrow returns two time.Time values from a given time.Time value. // The first is at the start of the same day as provided and the second is exactly 24 hours // after the first. diff --git a/common_test.go b/common_test.go index 3b69ff044..4c0cbeea8 100644 --- a/common_test.go +++ b/common_test.go @@ -171,28 +171,6 @@ func TestAmount__json(t *testing.T) { } } -func TestTry(t *testing.T) { - start := time.Now() - - err := try(func() error { - time.Sleep(50 * time.Millisecond) - return nil - }, 1*time.Second) - - if err != nil { - t.Fatal(err) - } - - diff := time.Since(start) - - if diff < 50*time.Millisecond { - t.Errorf("%v was under 50ms", diff) - } - if limit := 2 * 100 * time.Millisecond; diff > limit { - t.Errorf("%v was over %v", diff, limit) - } -} - func TestStartOfDayAndTomorrow(t *testing.T) { now := time.Now() min, max := startOfDayAndTomorrow(now) diff --git a/file_transfer_async.go b/file_transfer_async.go index 7dcc959d3..04e24be9e 100644 --- a/file_transfer_async.go +++ b/file_transfer_async.go @@ -8,7 +8,6 @@ import ( "bufio" "bytes" "context" - "encoding/json" "fmt" "io" "io/ioutil" @@ -21,6 +20,7 @@ import ( "time" "github.com/moov-io/ach" + "github.com/moov-io/paygate/internal/filetransfer" "github.com/moov-io/paygate/pkg/achclient" "github.com/go-kit/kit/log" @@ -68,36 +68,6 @@ var ( }, []string{"destination", "origin"}) ) -// cutoffTime represents the time of a banking day when all ACH files need to be uploaded in order -// to be processed for that day. Files which miss the cutoff time won't be processed until the next day. -// -// TODO(adam): How to handle multiple cutoffTime's for Same Day ACH? -type cutoffTime struct { - routingNumber string - cutoff int // 24-hour time value (0000 to 2400) - loc *time.Location // timezone cutoff is in (usually America/New_York) -} - -// diff returns the time.Duration between when and the cutoffTime -// A negative value will be returned if the cutoff has already passed -func (c *cutoffTime) diff(when time.Time) time.Duration { - now := time.Now() - cutoffTime := time.Date(now.Year(), now.Month(), now.Day(), c.cutoff/100, c.cutoff%100, 0, 0, c.loc) - return cutoffTime.Sub(when) -} - -func (c cutoffTime) MarshalJSON() ([]byte, error) { - return json.Marshal(struct { - RoutingNumber string - Cutoff int - Location string - }{ - RoutingNumber: c.routingNumber, - Cutoff: c.cutoff, - Location: c.loc.String(), // *time.Location doesn't marshal to JSON, so just write the IANA name - }) -} - // fileTransferController is a controller which is responsible for periodic sync'ing of ACH files // with their remote FTP destination. The ACH network operates on uploading and downloding files // from hosts during the business day. @@ -106,10 +76,11 @@ type fileTransferController struct { batchSize int interval time.Duration - cutoffTimes []*cutoffTime + cutoffTimes []*filetransfer.CutoffTime - ftpConfigs []*ftpConfig - fileTransferConfigs []*fileTransferConfig + repo filetransfer.Repository + ftpConfigs []*filetransfer.FTPConfig + fileTransferConfigs []*filetransfer.Config ach *achclient.ACH accountsClient AccountsClient @@ -121,7 +92,7 @@ type fileTransferController struct { // to their FTP host for processing. // // To change the refresh duration set ACH_FILE_TRANSFER_INTERVAL with a Go time.Duration value. (i.e. 10m for 10 minutes) -func newFileTransferController(logger log.Logger, dir string, repo fileTransferRepository, achClient *achclient.ACH, accountsClient AccountsClient, accountsCallsDisabled bool) (*fileTransferController, error) { +func newFileTransferController(logger log.Logger, dir string, repo filetransfer.Repository, achClient *achclient.ACH, accountsClient AccountsClient, accountsCallsDisabled bool) (*fileTransferController, error) { if _, err := os.Stat(dir); dir == "" || err != nil { return nil, fmt.Errorf("file-transfer-controller: problem with storage directory %q: %v", dir, err) } @@ -146,15 +117,15 @@ func newFileTransferController(logger log.Logger, dir string, repo fileTransferR } logger.Log("newFileTransferController", fmt.Sprintf("starting ACH file transfer controller: interval=%v batchSize=%d", interval, batchSize)) - cutoffTimes, err := repo.getCutoffTimes() + cutoffTimes, err := repo.GetCutoffTimes() if err != nil { return nil, fmt.Errorf("file-transfer-controller: error reading cutoffTimes: %v", err) } - ftpConfigs, err := repo.getFTPConfigs() + ftpConfigs, err := repo.GetFTPConfigs() if err != nil { return nil, fmt.Errorf("file-transfer-controller: error reading ftpConfigs: %v", err) } - fileTransferConfigs, err := repo.getFileTransferConfigs() + fileTransferConfigs, err := repo.GetConfigs() if err != nil { return nil, fmt.Errorf("file-transfer-controller: error reading ftpConfigs: %v", err) } @@ -171,6 +142,7 @@ func newFileTransferController(logger log.Logger, dir string, repo fileTransferR interval: interval, batchSize: batchSize, cutoffTimes: cutoffTimes, + repo: repo, ftpConfigs: ftpConfigs, fileTransferConfigs: fileTransferConfigs, ach: achClient, @@ -182,16 +154,16 @@ func newFileTransferController(logger log.Logger, dir string, repo fileTransferR return controller, nil } -func (c *fileTransferController) getDetails(cutoff *cutoffTime) (*ftpConfig, *fileTransferConfig) { - var ftp *ftpConfig +func (c *fileTransferController) getDetails(cutoff *filetransfer.CutoffTime) (*filetransfer.FTPConfig, *filetransfer.Config) { + var ftp *filetransfer.FTPConfig for i := range c.ftpConfigs { - if cutoff.routingNumber == c.ftpConfigs[i].RoutingNumber { + if cutoff.RoutingNumber == c.ftpConfigs[i].RoutingNumber { ftp = c.ftpConfigs[i] break } } for i := range c.fileTransferConfigs { - if cutoff.routingNumber == c.fileTransferConfigs[i].RoutingNumber { + if cutoff.RoutingNumber == c.fileTransferConfigs[i].RoutingNumber { return ftp, c.fileTransferConfigs[i] } } @@ -263,8 +235,8 @@ func (c *fileTransferController) downloadAndProcessIncomingFiles(depRepo deposit for i := range c.cutoffTimes { ftpConf, fileTransferConf := c.getDetails(c.cutoffTimes[i]) if ftpConf == nil || fileTransferConf == nil { - c.logger.Log("downloadAndProcessIncomingFiles", fmt.Sprintf("missing ftp and/or file transfer config for %s", c.cutoffTimes[i].routingNumber)) - missingFileUploadConfigs.With("routing_number", c.cutoffTimes[i].routingNumber).Add(1) + c.logger.Log("downloadAndProcessIncomingFiles", fmt.Sprintf("missing ftp and/or file transfer config for %s", c.cutoffTimes[i].RoutingNumber)) + missingFileUploadConfigs.With("routing_number", c.cutoffTimes[i].RoutingNumber).Add(1) continue } if err := c.downloadAllFiles(dir, ftpConf, fileTransferConf); err != nil { @@ -287,12 +259,12 @@ func (c *fileTransferController) downloadAndProcessIncomingFiles(depRepo deposit } // downloadAllFiles will setup directories for each routing number and initiate downloading and writing the files to sub-directories. -func (c *fileTransferController) downloadAllFiles(dir string, ftpConf *ftpConfig, fileTransferConf *fileTransferConfig) error { - agent, err := newFileTransferAgent(ftpConf, fileTransferConf) +func (c *fileTransferController) downloadAllFiles(dir string, ftpConf *filetransfer.FTPConfig, fileTransferConf *filetransfer.Config) error { + agent, err := filetransfer.New("", fileTransferConf, c.repo) // TODO(adam): pass through _type if err != nil { return fmt.Errorf("downloadAllFiles: problem with %s file transfer agent init: %v", ftpConf.RoutingNumber, err) } - defer agent.close() + defer agent.Close() // Setup file downloads if err := c.saveRemoteFiles(agent, dir); err != nil { @@ -459,23 +431,23 @@ func updateTransferFromReturnCode(logger log.Logger, code *ach.ReturnCode, origD // writeFiles will create files in dir for each file object provided // The contents of each file struct will always be closed. -func (c *fileTransferController) writeFiles(files []file, dir string) error { +func (c *fileTransferController) writeFiles(files []filetransfer.File, dir string) error { var errordFilenames []string os.Mkdir(dir, 0777) // ignore errors for i := range files { - f, err := os.Create(filepath.Join(dir, files[i].filename)) + f, err := os.Create(filepath.Join(dir, files[i].Filename)) if err != nil { - errordFilenames = append(errordFilenames, files[i].filename) + errordFilenames = append(errordFilenames, files[i].Filename) continue } - if _, err = io.Copy(f, files[i].contents); err != nil { - errordFilenames = append(errordFilenames, files[i].filename) + if _, err = io.Copy(f, files[i].Contents); err != nil { + errordFilenames = append(errordFilenames, files[i].Filename) continue } f.Sync() f.Close() - files[i].contents.Close() + files[i].Contents.Close() } if len(errordFilenames) != 0 { return fmt.Errorf("writeFiles problem on: %s", strings.Join(errordFilenames, ", ")) @@ -484,7 +456,7 @@ func (c *fileTransferController) writeFiles(files []file, dir string) error { } // saveRemoteFiles will write all inbound and return ACH files for a given routing number to the specified directory -func (c *fileTransferController) saveRemoteFiles(agent fileTransferAgent, dir string) error { +func (c *fileTransferController) saveRemoteFiles(agent filetransfer.Agent, dir string) error { errs := make(chan error, 10) var wg sync.WaitGroup @@ -492,21 +464,21 @@ func (c *fileTransferController) saveRemoteFiles(agent fileTransferAgent, dir st wg.Add(1) go func() { defer wg.Done() - files, err := agent.getInboundFiles() + files, err := agent.GetInboundFiles() if err != nil { errs <- err return } - if err := c.writeFiles(files, filepath.Join(dir, agent.inboundPath())); err != nil { + if err := c.writeFiles(files, filepath.Join(dir, agent.InboundPath())); err != nil { errs <- err return } for i := range files { - c.logger.Log("saveRemoteFiles", fmt.Sprintf("ACH: copied down inbound file %s", files[i].filename)) + c.logger.Log("saveRemoteFiles", fmt.Sprintf("ACH: copied down inbound file %s", files[i].Filename)) // Delete inbound file from FTP server - if err := agent.delete(filepath.Join(agent.inboundPath(), files[i].filename)); err != nil { - c.logger.Log("saveRemoteFiles", fmt.Sprintf("ACH: problem deleting inbound file %s", files[i].filename), "error", err) + if err := agent.Delete(filepath.Join(agent.InboundPath(), files[i].Filename)); err != nil { + c.logger.Log("saveRemoteFiles", fmt.Sprintf("ACH: problem deleting inbound file %s", files[i].Filename), "error", err) } } }() @@ -515,21 +487,21 @@ func (c *fileTransferController) saveRemoteFiles(agent fileTransferAgent, dir st wg.Add(1) go func() { defer wg.Done() - files, err := agent.getReturnFiles() + files, err := agent.GetReturnFiles() if err != nil { errs <- err return } - if err := c.writeFiles(files, filepath.Join(dir, agent.returnPath())); err != nil { + if err := c.writeFiles(files, filepath.Join(dir, agent.ReturnPath())); err != nil { errs <- err return } for i := range files { - c.logger.Log("saveRemoteFiles", fmt.Sprintf("ACH: copied down return file %s", files[i].filename)) + c.logger.Log("saveRemoteFiles", fmt.Sprintf("ACH: copied down return file %s", files[i].Filename)) // Delete return file from FTP server - if err := agent.delete(filepath.Join(agent.returnPath(), files[i].filename)); err != nil { - c.logger.Log("saveRemoteFiles", fmt.Sprintf("ACH: problem deleting return file %s", files[i].filename), "error", err) + if err := agent.Delete(filepath.Join(agent.ReturnPath(), files[i].Filename)); err != nil { + c.logger.Log("saveRemoteFiles", fmt.Sprintf("ACH: problem deleting return file %s", files[i].Filename), "error", err) } } }() @@ -663,12 +635,12 @@ func (c *fileTransferController) mergeAndUploadFiles(cur *transferCursor, transf // Find files close to their cutoff to enqueue for i := range c.cutoffTimes { - matches, err := filepath.Glob(fmt.Sprintf("*-%s-*.ach", c.cutoffTimes[i].routingNumber)) + matches, err := filepath.Glob(fmt.Sprintf("*-%s-*.ach", c.cutoffTimes[i].RoutingNumber)) if err != nil || len(matches) == 0 { continue } // If we're close to the cutoffTime then enqueue for upload - diff := c.cutoffTimes[i].diff(time.Now()) + diff := c.cutoffTimes[i].Diff(time.Now()) if diff > 0*time.Second && diff <= forcedCutoffUploadDelta { for j := range matches { file, err := parseACHFilepath(filepath.Join(mergedDir, matches[j])) @@ -686,7 +658,7 @@ func (c *fileTransferController) mergeAndUploadFiles(cur *transferCursor, transf // Upload files for i := range filesToUpload { for j := range c.cutoffTimes { - if filesToUpload[i].Header.ImmediateDestination == c.cutoffTimes[j].routingNumber { + if filesToUpload[i].Header.ImmediateDestination == c.cutoffTimes[j].RoutingNumber { if err := c.maybeUploadFile(filesToUpload[i], c.cutoffTimes[j]); err != nil { c.logger.Log("mergeAndUploadFiles", fmt.Sprintf("problem uploading %s", filesToUpload[i].filepath), "error", err.Error()) continue // skip, don't rename if we failed the upload @@ -742,36 +714,31 @@ func (c *fileTransferController) mergeGroupableTransfer(mergedDir string, xfer * return nil } -// maybeUploadFile will grab the needed configs and upload an given file to the FTP server for cutoffTime's routingNumber -func (c *fileTransferController) maybeUploadFile(fileToUpload *achFile, cutoffTime *cutoffTime) error { - // Grab configs for setting up FTP uploader - ftpConf, fileTransferConf := c.getDetails(cutoffTime) - if ftpConf == nil { - return fmt.Errorf("missing ftp config for %s", cutoffTime.routingNumber) +// maybeUploadFile will grab the needed configs and upload an given file to the FTP server for CutoffTime's RoutingNumber +func (c *fileTransferController) maybeUploadFile(fileToUpload *achFile, cutoffTime *filetransfer.CutoffTime) error { + _, cfg := c.getDetails(cutoffTime) + if cfg == nil { + return fmt.Errorf("missing file transfer config for %s", cutoffTime.RoutingNumber) } - if fileTransferConf == nil { - return fmt.Errorf("missing file transfer config for %s", cutoffTime.routingNumber) - } - - agent, err := newFileTransferAgent(ftpConf, fileTransferConf) + agent, err := filetransfer.New("", cfg, c.repo) if err != nil { - return fmt.Errorf("problem creating fileTransferAgent for %s: %v", ftpConf.RoutingNumber, err) + return fmt.Errorf("problem creating fileTransferAgent for %s: %v", cfg.RoutingNumber, err) } - defer agent.close() + defer agent.Close() - c.logger.Log("maybeUploadFile", fmt.Sprintf("uploading %s for routing number %s", fileToUpload.filepath, cutoffTime.routingNumber)) + c.logger.Log("maybeUploadFile", fmt.Sprintf("uploading %s for routing number %s", fileToUpload.filepath, cutoffTime.RoutingNumber)) return c.uploadFile(agent, fileToUpload) } -func (c *fileTransferController) uploadFile(agent fileTransferAgent, f *achFile) error { +func (c *fileTransferController) uploadFile(agent filetransfer.Agent, f *achFile) error { fd, err := os.Open(f.filepath) if err != nil { return fmt.Errorf("problem opening %s for upload: %v", f.filepath, err) } defer fd.Close() - if err := agent.uploadFile(file{filename: filepath.Base(f.filepath), contents: fd}); err != nil { + if err := agent.UploadFile(filetransfer.File{Filename: filepath.Base(f.filepath), Contents: fd}); err != nil { return fmt.Errorf("problem uploading %s: %v", f.filepath, err) } c.logger.Log("uploadFile", fmt.Sprintf("merged: uploaded file %s", f.filepath)) diff --git a/file_transfer_async_test.go b/file_transfer_async_test.go index 721b78647..4bcf48cb8 100644 --- a/file_transfer_async_test.go +++ b/file_transfer_async_test.go @@ -6,7 +6,6 @@ package main import ( "bytes" - "encoding/json" "errors" "fmt" "io" @@ -15,69 +14,20 @@ import ( "os" "path/filepath" "strings" + "sync" "testing" "time" "github.com/moov-io/ach" "github.com/moov-io/base" "github.com/moov-io/paygate/internal/database" + "github.com/moov-io/paygate/internal/filetransfer" "github.com/moov-io/paygate/pkg/achclient" "github.com/go-kit/kit/log" "github.com/gorilla/mux" ) -func TestCutoffTime(t *testing.T) { - now := time.Now() - loc, _ := time.LoadLocation("America/New_York") - ct := &cutoffTime{routingNumber: "123456789", cutoff: 1700, loc: loc} - - // before - when := time.Date(now.Year(), now.Month(), now.Day(), 12, 34, 0, 0, loc) - if d := ct.diff(when); d != (4*time.Hour)+(26*time.Minute) { // written at 4:37PM - t.Errorf("got %v", d) - } - - // 1min before - when = time.Date(now.Year(), now.Month(), now.Day(), 16, 59, 0, 0, loc) - if d := ct.diff(when); d != 1*time.Minute { // written at 4:38PM - t.Errorf("got %v", d) - } - - // 1min after - when = time.Date(now.Year(), now.Month(), now.Day(), 17, 01, 0, 0, loc) - if d := ct.diff(when); d != -1*time.Minute { // written at 4:38PM - t.Errorf("got %v", d) - } - - // after - when = time.Date(now.Year(), now.Month(), now.Day(), 18, 21, 0, 0, loc) - if d := ct.diff(when); d != (-1*time.Hour)-(21*time.Minute) { // written at 4:40PM - t.Errorf("got %v", d) - } -} - -func TestCutoffTime__JSON(t *testing.T) { - loc, _ := time.LoadLocation("America/New_York") - ct := &cutoffTime{routingNumber: "123456789", cutoff: 1700, loc: loc} - - var buf bytes.Buffer - if err := json.NewEncoder(&buf).Encode(ct); err != nil { - t.Fatal(err) - } - - // Crude check of JSON properties - if !strings.Contains(buf.String(), `"RoutingNumber":"123456789"`) { - t.Error(buf.String()) - } - if !strings.Contains(buf.String(), `"Cutoff":1700`) { - t.Error(buf.String()) - } - if !strings.Contains(buf.String(), `"Location":"America/New_York"`) { - t.Error(buf.String()) - } -} - func TestFileTransferController__newFileTransferController(t *testing.T) { dir, err := ioutil.TempDir("", "fileTransferController") if err != nil { @@ -85,7 +35,7 @@ func TestFileTransferController__newFileTransferController(t *testing.T) { } defer os.RemoveAll(dir) - repo := &localFileTransferRepository{} + repo := filetransfer.NewRepository(nil, "local") // filetransfer.localFileTransferRepository controller, err := newFileTransferController(log.NewNopLogger(), dir, repo, nil, nil, true) if err != nil { t.Fatal(err) @@ -108,13 +58,13 @@ func TestFileTransferController__newFileTransferController(t *testing.T) { } func TestFileTransferController__getDetails(t *testing.T) { - cutoff := &cutoffTime{ - routingNumber: "123", - cutoff: 1700, - loc: time.UTC, + cutoff := &filetransfer.CutoffTime{ + RoutingNumber: "123", + Cutoff: 1700, + Loc: time.UTC, } controller := &fileTransferController{ - ftpConfigs: []*ftpConfig{ + ftpConfigs: []*filetransfer.FTPConfig{ { RoutingNumber: "123", Hostname: "ftp.foo.com", @@ -124,7 +74,7 @@ func TestFileTransferController__getDetails(t *testing.T) { Hostname: "ftp.bar.com", }, }, - fileTransferConfigs: []*fileTransferConfig{ + fileTransferConfigs: []*filetransfer.Config{ { RoutingNumber: "123", InboundPath: "inbound/", @@ -149,7 +99,7 @@ func TestFileTransferController__getDetails(t *testing.T) { } // not found - ftpConf, fileTransferConf = controller.getDetails(&cutoffTime{routingNumber: "456"}) + ftpConf, fileTransferConf = controller.getDetails(&filetransfer.CutoffTime{RoutingNumber: "456"}) if ftpConf != nil || fileTransferConf != nil { t.Fatalf("ftpConf=%v fileTransferConf=%v", ftpConf, fileTransferConf) } @@ -160,10 +110,10 @@ func TestFileTransferController__writeFiles(t *testing.T) { defer os.RemoveAll(dir) controller := &fileTransferController{} - files := []file{ + files := []filetransfer.File{ { - filename: "write-test", - contents: ioutil.NopCloser(strings.NewReader("test conents")), + Filename: "write-test", + Contents: ioutil.NopCloser(strings.NewReader("test conents")), }, } if err := controller.writeFiles(files, dir); err != nil { @@ -189,18 +139,65 @@ func readFileAsCloser(path string) io.ReadCloser { return ioutil.NopCloser(bytes.NewReader(bs)) } +type mockFileTransferAgent struct { + inboundFiles []filetransfer.File + returnFiles []filetransfer.File + uploadedFile *filetransfer.File // non-nil on file upload + deletedFile string // filepath of last deleted file + mu sync.RWMutex // protects all fields +} + +func (a *mockFileTransferAgent) GetInboundFiles() ([]filetransfer.File, error) { + a.mu.RLock() + defer a.mu.RUnlock() + + return a.inboundFiles, nil +} + +func (a *mockFileTransferAgent) GetReturnFiles() ([]filetransfer.File, error) { + a.mu.RLock() + defer a.mu.RUnlock() + + return a.returnFiles, nil +} + +func (a *mockFileTransferAgent) UploadFile(f filetransfer.File) error { + a.mu.Lock() + defer a.mu.Unlock() + + // read f.contents before callers close the underlying os.Open file descriptor + bs, _ := ioutil.ReadAll(f.Contents) + a.uploadedFile = &f + a.uploadedFile.Contents = ioutil.NopCloser(bytes.NewReader(bs)) + return nil +} + +func (a *mockFileTransferAgent) Delete(path string) error { + a.mu.Lock() + defer a.mu.Unlock() + + a.deletedFile = path + return nil +} + +func (a *mockFileTransferAgent) InboundPath() string { return "inbound/" } +func (a *mockFileTransferAgent) OutboundPath() string { return "outbound/" } +func (a *mockFileTransferAgent) ReturnPath() string { return "return/" } + +func (a *mockFileTransferAgent) Close() error { return nil } + func TestFileTransferController__saveRemoteFiles(t *testing.T) { agent := &mockFileTransferAgent{ - inboundFiles: []file{ + inboundFiles: []filetransfer.File{ { - filename: "ppd-debit.ach", - contents: readFileAsCloser(filepath.Join("testdata", "ppd-debit.ach")), + Filename: "ppd-debit.ach", + Contents: readFileAsCloser(filepath.Join("testdata", "ppd-debit.ach")), }, }, - returnFiles: []file{ + returnFiles: []filetransfer.File{ { - filename: "return-WEB.ach", - contents: readFileAsCloser(filepath.Join("testdata", "return-WEB.ach")), + Filename: "return-WEB.ach", + Contents: readFileAsCloser(filepath.Join("testdata", "return-WEB.ach")), }, }, } @@ -218,14 +215,14 @@ func TestFileTransferController__saveRemoteFiles(t *testing.T) { } // read written files - file, err := parseACHFilepath(filepath.Join(dir, agent.inboundPath(), "ppd-debit.ach")) + file, err := parseACHFilepath(filepath.Join(dir, agent.InboundPath(), "ppd-debit.ach")) if err != nil { t.Error(err) } if v := file.Batches[0].GetHeader().StandardEntryClassCode; v != "PPD" { t.Errorf("SEC code found is %s", v) } - file, err = parseACHFilepath(filepath.Join(dir, agent.returnPath(), "return-WEB.ach")) + file, err = parseACHFilepath(filepath.Join(dir, agent.ReturnPath(), "return-WEB.ach")) if err != nil { t.Error(err) } @@ -383,11 +380,10 @@ func TestFileTransferController__uploadFile(t *testing.T) { if agent.uploadedFile == nil { t.Fatal("nil agent.uploadedFile") } - if v := agent.uploadedFile.filename; v != "ppd-debit.ach" { + if v := agent.uploadedFile.Filename; v != "ppd-debit.ach" { t.Errorf("got %v", v) } - - if bs, err := ioutil.ReadAll(agent.uploadedFile.contents); len(bs) == 0 || err != nil { + if bs, err := ioutil.ReadAll(agent.uploadedFile.Contents); len(bs) == 0 || err != nil { t.Errorf("copied empty file: %v", err) } } @@ -633,7 +629,8 @@ func TestFileTransferController__processReturnEntry(t *testing.T) { dir, _ := ioutil.TempDir("", "processReturnEntry") defer os.RemoveAll(dir) - controller, err := newFileTransferController(log.NewNopLogger(), dir, &localFileTransferRepository{}, nil, nil, true) + repo := filetransfer.NewRepository(nil, "local") // filetransfer.localFileTransferRepository + controller, err := newFileTransferController(log.NewNopLogger(), dir, repo, nil, nil, true) if err != nil { t.Fatal(err) } diff --git a/http_test.go b/http_test.go index 9c0a96337..8c58cbc58 100644 --- a/http_test.go +++ b/http_test.go @@ -5,19 +5,14 @@ package main import ( - "bytes" - "crypto/tls" - "encoding/pem" "errors" - "io/ioutil" - "net" "net/http" "net/http/httptest" "os" "testing" - "time" "github.com/moov-io/base" + mhttptest "github.com/moov-io/paygate/internal/httptest" "github.com/go-kit/kit/log" "github.com/gorilla/mux" @@ -114,7 +109,7 @@ func TestHTTP__tlsHttpClient(t *testing.T) { return // skip network calls } - cafile, err := grabConnectionCertificates(t, "google.com:443") + cafile, err := mhttptest.GrabConnectionCertificates(t, "google.com:443") if err != nil { t.Fatal(err) } @@ -128,36 +123,3 @@ func TestHTTP__tlsHttpClient(t *testing.T) { t.Error("empty http.Client") } } - -// grabConnectionCertificates returns a filepath of certificate chain from a given address's -// server. This is useful for adding extra root CA's to network clients -func grabConnectionCertificates(t *testing.T, addr string) (string, error) { - dialer := &net.Dialer{Timeout: 10 * time.Second} - conn, err := tls.DialWithDialer(dialer, "tcp", addr, nil) - if err != nil { - t.Error(err) - } - defer conn.Close() - - fd, err := ioutil.TempFile("", "conn-certs") - if err != nil { - t.Fatal(err) - } - - // Write x509 certs to disk - certs := conn.ConnectionState().PeerCertificates - var buf bytes.Buffer - for i := range certs { - b := &pem.Block{ - Type: "CERTIFICATE", - Bytes: certs[i].Raw, - } - if err := pem.Encode(&buf, b); err != nil { - t.Fatal(err) - } - } - if err := ioutil.WriteFile(fd.Name(), buf.Bytes(), 0644); err != nil { - t.Fatal(err) - } - return fd.Name(), nil -} diff --git a/file_transfer_configs.go b/internal/filetransfer/config.go similarity index 67% rename from file_transfer_configs.go rename to internal/filetransfer/config.go index e94f07739..7925dfd94 100644 --- a/file_transfer_configs.go +++ b/internal/filetransfer/config.go @@ -2,7 +2,7 @@ // Use of this source code is governed by an Apache License // license that can be found in the LICENSE file. -package main +package filetransfer import ( "database/sql" @@ -20,26 +20,26 @@ import ( "github.com/gorilla/mux" ) -type fileTransferRepository interface { - getCutoffTimes() ([]*cutoffTime, error) - getFTPConfigs() ([]*ftpConfig, error) - getFileTransferConfigs() ([]*fileTransferConfig, error) +type Repository interface { + GetCutoffTimes() ([]*CutoffTime, error) + GetFTPConfigs() ([]*FTPConfig, error) + GetConfigs() ([]*Config, error) - close() error + Close() error } -func newFileTransferRepository(db *sql.DB, dbType string) fileTransferRepository { +func NewRepository(db *sql.DB, dbType string) Repository { if db == nil { return &localFileTransferRepository{} } - sqliteRepo := &sqliteFileTransferRepository{db} + sqliteRepo := &sqliteRepository{db} if strings.EqualFold(dbType, "mysql") { // On 'mysql' database setups return that over the local (hardcoded) values. return sqliteRepo } - cutoffCount, ftpCount, fileTransferCount := sqliteRepo.getCounts() + cutoffCount, ftpCount, fileTransferCount := sqliteRepo.GetCounts() if (cutoffCount + ftpCount + fileTransferCount) == 0 { return &localFileTransferRepository{} } @@ -47,19 +47,19 @@ func newFileTransferRepository(db *sql.DB, dbType string) fileTransferRepository return sqliteRepo } -type sqliteFileTransferRepository struct { +type sqliteRepository struct { // TODO(adam): admin endpoints to read and write these configs? (w/ masked passwords) db *sql.DB } -func (r *sqliteFileTransferRepository) close() error { +func (r *sqliteRepository) Close() error { return r.db.Close() } -// getCounts returns the count of cutoffTime's, ftpConfig's, and fileTransferConfig's in the sqlite database. +// GetCounts returns the count of CutoffTime's, FTPConfig's, and Config's in the sqlite database. // // This is used to return localFileTransferRepository if the counts are empty (so local dev "just works"). -func (r *sqliteFileTransferRepository) getCounts() (int, int, int) { +func (r *sqliteRepository) GetCounts() (int, int, int) { count := func(table string) int { query := fmt.Sprintf(`select count(*) from %s`, table) stmt, err := r.db.Prepare(query) @@ -76,7 +76,7 @@ func (r *sqliteFileTransferRepository) getCounts() (int, int, int) { return count("cutoff_times"), count("ftp_configs"), count("file_transfer_configs") } -func (r *sqliteFileTransferRepository) getCutoffTimes() ([]*cutoffTime, error) { +func (r *sqliteRepository) GetCutoffTimes() ([]*CutoffTime, error) { query := `select routing_number, cutoff, location from cutoff_times;` stmt, err := r.db.Prepare(query) if err != nil { @@ -84,29 +84,29 @@ func (r *sqliteFileTransferRepository) getCutoffTimes() ([]*cutoffTime, error) { } defer stmt.Close() - var times []*cutoffTime + var times []*CutoffTime rows, err := stmt.Query() if err != nil { return nil, err } defer rows.Close() for rows.Next() { - var cutoff cutoffTime + var cutoff CutoffTime var loc string - if err := rows.Scan(&cutoff.routingNumber, &cutoff.cutoff, &loc); err != nil { - return nil, fmt.Errorf("getCutoffTimes: scan: %v", err) + if err := rows.Scan(&cutoff.RoutingNumber, &cutoff.Cutoff, &loc); err != nil { + return nil, fmt.Errorf("GetCutoffTimes: scan: %v", err) } if l, err := time.LoadLocation(loc); err != nil { - return nil, fmt.Errorf("getCutoffTimes: parsing %q failed: %v", loc, err) + return nil, fmt.Errorf("GetCutoffTimes: parsing %q failed: %v", loc, err) } else { - cutoff.loc = l + cutoff.Loc = l } times = append(times, &cutoff) } return times, rows.Err() } -func (r *sqliteFileTransferRepository) getFTPConfigs() ([]*ftpConfig, error) { +func (r *sqliteRepository) GetFTPConfigs() ([]*FTPConfig, error) { query := `select routing_number, hostname, username, password from ftp_configs;` stmt, err := r.db.Prepare(query) if err != nil { @@ -114,23 +114,23 @@ func (r *sqliteFileTransferRepository) getFTPConfigs() ([]*ftpConfig, error) { } defer stmt.Close() - var configs []*ftpConfig + var configs []*FTPConfig rows, err := stmt.Query() if err != nil { return nil, err } defer rows.Close() for rows.Next() { - var cfg ftpConfig + var cfg FTPConfig if err := rows.Scan(&cfg.RoutingNumber, &cfg.Hostname, &cfg.Username, &cfg.Password); err != nil { - return nil, fmt.Errorf("getFTPConfigs: scan: %v", err) + return nil, fmt.Errorf("GetFTPConfigs: scan: %v", err) } configs = append(configs, &cfg) } return configs, rows.Err() } -func (r *sqliteFileTransferRepository) getFileTransferConfigs() ([]*fileTransferConfig, error) { +func (r *sqliteRepository) GetConfigs() ([]*Config, error) { query := `select routing_number, inbound_path, outbound_path, return_path from file_transfer_configs;` stmt, err := r.db.Prepare(query) if err != nil { @@ -138,41 +138,41 @@ func (r *sqliteFileTransferRepository) getFileTransferConfigs() ([]*fileTransfer } defer stmt.Close() - var configs []*fileTransferConfig + var configs []*Config rows, err := stmt.Query() if err != nil { return nil, err } defer rows.Close() for rows.Next() { - var cfg fileTransferConfig + var cfg Config if err := rows.Scan(&cfg.RoutingNumber, &cfg.InboundPath, &cfg.OutboundPath, &cfg.ReturnPath); err != nil { - return nil, fmt.Errorf("getFileTransferConfigs: scan: %v", err) + return nil, fmt.Errorf("GetConfigs: scan: %v", err) } configs = append(configs, &cfg) } return configs, rows.Err() } -// localFileTransferRepository is a fileTransferRepository for local dev with values that match +// localFileTransferRepository is a Repository for local dev with values that match // apitest, testdata/ftp-server/ paths, files and FTP (fsftp) defaults. type localFileTransferRepository struct{} -func (r *localFileTransferRepository) close() error { return nil } +func (r *localFileTransferRepository) Close() error { return nil } -func (r *localFileTransferRepository) getCutoffTimes() ([]*cutoffTime, error) { +func (r *localFileTransferRepository) GetCutoffTimes() ([]*CutoffTime, error) { nyc, _ := time.LoadLocation("America/New_York") - return []*cutoffTime{ + return []*CutoffTime{ { - routingNumber: "121042882", - cutoff: 1700, - loc: nyc, + RoutingNumber: "121042882", + Cutoff: 1700, + Loc: nyc, }, }, nil } -func (r *localFileTransferRepository) getFTPConfigs() ([]*ftpConfig, error) { - return []*ftpConfig{ +func (r *localFileTransferRepository) GetFTPConfigs() ([]*FTPConfig, error) { + return []*FTPConfig{ { RoutingNumber: "121042882", // from 'go run ./cmd/server' in Accounts Hostname: "localhost:2121", // below configs for moov/fftp:v0.1.0 @@ -182,8 +182,8 @@ func (r *localFileTransferRepository) getFTPConfigs() ([]*ftpConfig, error) { }, nil } -func (r *localFileTransferRepository) getFileTransferConfigs() ([]*fileTransferConfig, error) { - return []*fileTransferConfig{ +func (r *localFileTransferRepository) GetConfigs() ([]*Config, error) { + return []*Config{ { RoutingNumber: "121042882", InboundPath: "inbound/", // below configs match paygate's testdata/ftp-server/ @@ -193,9 +193,9 @@ func (r *localFileTransferRepository) getFileTransferConfigs() ([]*fileTransferC }, nil } -// addFileTransferConfigRoutes registers the admin HTTP routes for modifying file-transfer (uploading) configs. -func addFileTransferConfigRoutes(logger log.Logger, svc *admin.Server, repo fileTransferRepository) { - svc.AddHandler("/configs/uploads", getFileTransferConfigs(logger, repo)) +// AddFileTransferConfigRoutes registers the admin HTTP routes for modifying file-transfer (uploading) configs. +func AddFileTransferConfigRoutes(logger log.Logger, svc *admin.Server, repo Repository) { + svc.AddHandler("/configs/uploads", GetConfigs(logger, repo)) svc.AddHandler("/configs/uploads/cutoff-times/{routingNumber}", upsertCutoffTimeConfig(logger, repo)) svc.AddHandler("/configs/uploads/cutoff-times/{routingNumber}", deleteCutoffTimeConfig(logger, repo)) @@ -216,13 +216,13 @@ func getRoutingNumber(r *http.Request) string { } type adminConfigResponse struct { - CutoffTimes []*cutoffTime `json:"cutoffTimes"` - FTPConfigs []*ftpConfig `json:"ftpConfigs"` - FileTransferConfigs []*fileTransferConfig `json:"fileTransferConfigs"` + CutoffTimes []*CutoffTime `json:"CutoffTimes"` + FTPConfigs []*FTPConfig `json:"FTPConfigs"` + FileTransferConfigs []*Config `json:"Configs"` } -// getFileTransferConfigs returns all configurations (i.e. FTP, cutoff times, file-transfer configs with passwords masked. (e.g. 'p******d') -func getFileTransferConfigs(logger log.Logger, repo fileTransferRepository) http.HandlerFunc { +// GetConfigs returns all configurations (i.e. FTP, cutoff times, file-transfer configs with passwords masked. (e.g. 'p******d') +func GetConfigs(logger log.Logger, repo Repository) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { if r.Method != "GET" { moovhttp.Problem(w, fmt.Errorf("unsupported HTTP verb %s", r.Method)) @@ -230,19 +230,19 @@ func getFileTransferConfigs(logger log.Logger, repo fileTransferRepository) http } resp := &adminConfigResponse{} - if v, err := repo.getCutoffTimes(); err != nil { + if v, err := repo.GetCutoffTimes(); err != nil { moovhttp.Problem(w, err) return } else { resp.CutoffTimes = v } - if v, err := repo.getFTPConfigs(); err != nil { + if v, err := repo.GetFTPConfigs(); err != nil { moovhttp.Problem(w, err) return } else { resp.FTPConfigs = maskPasswords(v) } - if v, err := repo.getFileTransferConfigs(); err != nil { + if v, err := repo.GetConfigs(); err != nil { moovhttp.Problem(w, err) return } else { @@ -265,14 +265,14 @@ func maskPassword(s string) string { } } -func maskPasswords(cfgs []*ftpConfig) []*ftpConfig { +func maskPasswords(cfgs []*FTPConfig) []*FTPConfig { for i := range cfgs { cfgs[i].Password = maskPassword(cfgs[i].Password) } return cfgs } -func upsertCutoffTimeConfig(logger log.Logger, repo fileTransferRepository) http.HandlerFunc { +func upsertCutoffTimeConfig(logger log.Logger, repo Repository) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { if r.Method != "PUT" { moovhttp.Problem(w, fmt.Errorf("unsupported HTTP verb %s", r.Method)) @@ -285,7 +285,7 @@ func upsertCutoffTimeConfig(logger log.Logger, repo fileTransferRepository) http } } -func deleteCutoffTimeConfig(logger log.Logger, repo fileTransferRepository) http.HandlerFunc { +func deleteCutoffTimeConfig(logger log.Logger, repo Repository) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { if r.Method != "DELETE" { moovhttp.Problem(w, fmt.Errorf("unsupported HTTP verb %s", r.Method)) @@ -298,7 +298,7 @@ func deleteCutoffTimeConfig(logger log.Logger, repo fileTransferRepository) http } } -func upsertFileTransferConfig(logger log.Logger, repo fileTransferRepository) http.HandlerFunc { +func upsertFileTransferConfig(logger log.Logger, repo Repository) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { if r.Method != "PUT" { moovhttp.Problem(w, fmt.Errorf("unsupported HTTP verb %s", r.Method)) @@ -311,7 +311,7 @@ func upsertFileTransferConfig(logger log.Logger, repo fileTransferRepository) ht } } -func deleteFileTransferConfig(logger log.Logger, repo fileTransferRepository) http.HandlerFunc { +func deleteFileTransferConfig(logger log.Logger, repo Repository) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { if r.Method != "DELETE" { moovhttp.Problem(w, fmt.Errorf("unsupported HTTP verb %s", r.Method)) @@ -324,7 +324,7 @@ func deleteFileTransferConfig(logger log.Logger, repo fileTransferRepository) ht } } -func upsertFTPConfig(logger log.Logger, repo fileTransferRepository) http.HandlerFunc { +func upsertFTPConfig(logger log.Logger, repo Repository) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { if r.Method != "PUT" { moovhttp.Problem(w, fmt.Errorf("unsupported HTTP verb %s", r.Method)) @@ -337,7 +337,7 @@ func upsertFTPConfig(logger log.Logger, repo fileTransferRepository) http.Handle } } -func deleteFTPConfig(logger log.Logger, repo fileTransferRepository) http.HandlerFunc { +func deleteFTPConfig(logger log.Logger, repo Repository) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { if r.Method != "DELETE" { moovhttp.Problem(w, fmt.Errorf("unsupported HTTP verb %s", r.Method)) diff --git a/file_transfer_configs_test.go b/internal/filetransfer/config_test.go similarity index 72% rename from file_transfer_configs_test.go rename to internal/filetransfer/config_test.go index 190d96d81..ce774554d 100644 --- a/file_transfer_configs_test.go +++ b/internal/filetransfer/config_test.go @@ -2,7 +2,7 @@ // Use of this source code is governed by an Apache License // license that can be found in the LICENSE file. -package main +package filetransfer import ( "database/sql" @@ -16,34 +16,34 @@ import ( "github.com/go-kit/kit/log" ) -type testSqliteFileTransferRepository struct { - *sqliteFileTransferRepository +type testSqliteRepository struct { + *sqliteRepository testDB *database.TestSQLiteDB } -func (r *testSqliteFileTransferRepository) close() error { - r.sqliteFileTransferRepository.close() +func (r *testSqliteRepository) Close() error { + r.sqliteRepository.Close() return r.testDB.Close() } -func createTestSqliteFileTransferRepository(t *testing.T) *testSqliteFileTransferRepository { +func createTestsqliteRepository(t *testing.T) *testSqliteRepository { t.Helper() db := database.CreateTestSqliteDB(t) - repo := &sqliteFileTransferRepository{db: db.DB} - return &testSqliteFileTransferRepository{repo, db} + repo := &sqliteRepository{db: db.DB} + return &testSqliteRepository{repo, db} } -func TestSqliteFileTransferRepository__getCounts(t *testing.T) { - repo := createTestSqliteFileTransferRepository(t) - defer repo.close() +func TestsqliteRepository__getCounts(t *testing.T) { + repo := createTestsqliteRepository(t) + defer repo.Close() writeCutoffTime(t, repo) writeFTPConfig(t, repo) writeFileTransferConfig(t, repo.db) - cutoffs, ftps, filexfers := repo.getCounts() + cutoffs, ftps, filexfers := repo.GetCounts() if cutoffs != 1 { t.Errorf("got %d", cutoffs) } @@ -54,15 +54,15 @@ func TestSqliteFileTransferRepository__getCounts(t *testing.T) { t.Errorf("got %d", filexfers) } - // If we read at least one row from each config table we need to make sure newFileTransferRepository - // returns sqliteFileTransferRepository (rather than localFileTransferRepository) - r := newFileTransferRepository(repo.db, "") - if _, ok := r.(*sqliteFileTransferRepository); !ok { + // If we read at least one row from each config table we need to make sure NewRepository + // returns sqliteRepository (rather than localFileTransferRepository) + r := NewRepository(repo.db, "") + if _, ok := r.(*sqliteRepository); !ok { t.Errorf("got %T", r) } } -func writeCutoffTime(t *testing.T, repo *testSqliteFileTransferRepository) { +func writeCutoffTime(t *testing.T, repo *testSqliteRepository) { t.Helper() query := `insert into cutoff_times (routing_number, cutoff, location) values ('123456789', 1700, 'America/New_York');` @@ -76,31 +76,31 @@ func writeCutoffTime(t *testing.T, repo *testSqliteFileTransferRepository) { } } -func TestSqliteFileTransferRepository__getCutoffTimes(t *testing.T) { - repo := createTestSqliteFileTransferRepository(t) - defer repo.close() +func TestsqliteRepository__GetCutoffTimes(t *testing.T) { + repo := createTestsqliteRepository(t) + defer repo.Close() writeCutoffTime(t, repo) - cutoffTimes, err := repo.getCutoffTimes() + cutoffTimes, err := repo.GetCutoffTimes() if err != nil { t.Fatal(err) } if len(cutoffTimes) != 1 { t.Errorf("len(cutoffTimes)=%d", len(cutoffTimes)) } - if cutoffTimes[0].routingNumber != "123456789" { - t.Errorf("cutoffTimes[0].routingNumber=%s", cutoffTimes[0].routingNumber) + if cutoffTimes[0].RoutingNumber != "123456789" { + t.Errorf("cutoffTimes[0].RoutingNumber=%s", cutoffTimes[0].RoutingNumber) } - if cutoffTimes[0].cutoff != 1700 { - t.Errorf("cutoffTimes[0].cutoff=%d", cutoffTimes[0].cutoff) + if cutoffTimes[0].Cutoff != 1700 { + t.Errorf("cutoffTimes[0].Cutoff=%d", cutoffTimes[0].Cutoff) } - if v := cutoffTimes[0].loc.String(); v != "America/New_York" { - t.Errorf("cutoffTimes[0].loc=%v", v) + if v := cutoffTimes[0].Loc.String(); v != "America/New_York" { + t.Errorf("cutoffTimes[0].Loc=%v", v) } } -func writeFTPConfig(t *testing.T, repo *testSqliteFileTransferRepository) { +func writeFTPConfig(t *testing.T, repo *testSqliteRepository) { t.Helper() query := `insert into ftp_configs (routing_number, hostname, username, password) values ('123456789', 'ftp.moov.io', 'moov', 'secret');` @@ -114,14 +114,14 @@ func writeFTPConfig(t *testing.T, repo *testSqliteFileTransferRepository) { } } -func TestSqliteFileTransferRepository__getFTPConfigs(t *testing.T) { - repo := createTestSqliteFileTransferRepository(t) - defer repo.close() +func TestsqliteRepository__GetFTPConfigs(t *testing.T) { + repo := createTestsqliteRepository(t) + defer repo.Close() writeFTPConfig(t, repo) // now read - configs, err := repo.getFTPConfigs() + configs, err := repo.GetFTPConfigs() if err != nil { t.Fatal(err) } @@ -156,14 +156,14 @@ func writeFileTransferConfig(t *testing.T, db *sql.DB) { } } -func TestSqliteFileTransferRepository__getFileTransferConfigs(t *testing.T) { - repo := createTestSqliteFileTransferRepository(t) - defer repo.close() +func TestsqliteRepository__GetConfigs(t *testing.T) { + repo := createTestsqliteRepository(t) + defer repo.Close() writeFileTransferConfig(t, repo.db) // now read - configs, err := repo.getFileTransferConfigs() + configs, err := repo.GetConfigs() if err != nil { t.Fatal(err) } @@ -187,13 +187,13 @@ func TestSqliteFileTransferRepository__getFileTransferConfigs(t *testing.T) { func TestMySQLFileTransferRepository(t *testing.T) { testdb := database.CreateTestMySQLDB(t) - repo := newFileTransferRepository(testdb.DB, "mysql") - if _, ok := repo.(*sqliteFileTransferRepository); !ok { + repo := NewRepository(testdb.DB, "mysql") + if _, ok := repo.(*sqliteRepository); !ok { t.Fatalf("got %T", repo) } writeFileTransferConfig(t, testdb.DB) - configs, err := repo.getFileTransferConfigs() + configs, err := repo.GetConfigs() if err != nil { t.Fatal(err) } @@ -230,7 +230,7 @@ func TestFileTransferConfigs__maskPassword(t *testing.T) { t.Errorf("got %q", v) } - out := maskPasswords([]*ftpConfig{{Password: "password"}}) + out := maskPasswords([]*FTPConfig{{Password: "password"}}) if len(out) != 1 { t.Errorf("got %d ftpConfigs: %v", len(out), out) } @@ -239,14 +239,14 @@ func TestFileTransferConfigs__maskPassword(t *testing.T) { } } -func TestFileTransferConfigsHTTP__getFileTransferConfigs(t *testing.T) { +func TestFileTransferConfigsHTTP__GetConfigs(t *testing.T) { svc := admin.NewServer(":0") go svc.Listen() defer svc.Shutdown() repo := &localFileTransferRepository{} - addFileTransferConfigRoutes(log.NewNopLogger(), svc, repo) + AddFileTransferConfigRoutes(log.NewNopLogger(), svc, repo) req, err := http.NewRequest("GET", "http://localhost"+svc.BindAddr()+"/configs/uploads", nil) // need moov-io/base update if err != nil { diff --git a/internal/filetransfer/file_transfer.go b/internal/filetransfer/file_transfer.go new file mode 100644 index 000000000..9bb15c4ca --- /dev/null +++ b/internal/filetransfer/file_transfer.go @@ -0,0 +1,92 @@ +// Copyright 2019 The Moov Authors +// Use of this source code is governed by an Apache License +// license that can be found in the LICENSE file. + +package filetransfer + +import ( + "encoding/json" + "fmt" + "io" + "strings" + "time" +) + +type Config struct { + RoutingNumber string + + InboundPath string + OutboundPath string + ReturnPath string +} + +type File struct { + Filename string + Contents io.ReadCloser +} + +func (f File) Close() error { + return f.Contents.Close() +} + +// Agent represents an interface for uploading and retrieving ACH files from a remote service. +type Agent interface { + GetInboundFiles() ([]File, error) + GetReturnFiles() ([]File, error) + UploadFile(f File) error + Delete(path string) error + + InboundPath() string + OutboundPath() string + ReturnPath() string + + Close() error +} + +// New returns an implementation of a Agent which is used to upload files to a remote server. +// +// This function reads ACH_FILE_TRANSFERS_ROOT_CAFILE for a file with root certificates to be used in all secured connections. +func New(_type string, cfg *Config, repo Repository) (Agent, error) { + switch strings.ToLower(_type) { + case "ftp": + ftpConfigs, err := repo.GetFTPConfigs() + if err != nil { + return nil, fmt.Errorf("filetransfer: error creating new FTP client: %v", err) + } + return newFTPTransferAgent(cfg, ftpConfigs) + case "sftp": + return nil, nil // TODO(adam): impl + default: + return nil, fmt.Errorf("filetransfer: unknown type %s", _type) + } +} + +// CutoffTime represents the time of a banking day when all ACH files need to be uploaded in order +// to be processed for that day. Files which miss the cutoff time won't be processed until the next day. +// +// TODO(adam): How to handle multiple CutoffTime's for Same Day ACH? +type CutoffTime struct { + RoutingNumber string + Cutoff int // 24-hour time value (0000 to 2400) + Loc *time.Location // timezone cutoff is in (usually America/New_York) +} + +// diff returns the time.Duration between when and the CutoffTime +// A negative value will be returned if the cutoff has already passed +func (c *CutoffTime) Diff(when time.Time) time.Duration { + now := time.Now() + ct := time.Date(now.Year(), now.Month(), now.Day(), c.Cutoff/100, c.Cutoff%100, 0, 0, c.Loc) + return ct.Sub(when) +} + +func (c CutoffTime) MarshalJSON() ([]byte, error) { + return json.Marshal(struct { + RoutingNumber string + Cutoff int + Location string + }{ + RoutingNumber: c.RoutingNumber, + Cutoff: c.Cutoff, + Location: c.Loc.String(), // *time.Location doesn't marshal to JSON, so just write the IANA name + }) +} diff --git a/file_transfer_test.go b/internal/filetransfer/file_transfer_test.go similarity index 56% rename from file_transfer_test.go rename to internal/filetransfer/file_transfer_test.go index d2bf10f12..9dfa6e503 100644 --- a/file_transfer_test.go +++ b/internal/filetransfer/file_transfer_test.go @@ -2,10 +2,11 @@ // Use of this source code is governed by an Apache License // license that can be found in the LICENSE file. -package main +package filetransfer import ( "bytes" + "encoding/json" "errors" "fmt" "io/ioutil" @@ -13,11 +14,11 @@ import ( "os" "path/filepath" "strings" - "sync" "testing" "time" "github.com/moov-io/base" + mhttptest "github.com/moov-io/paygate/internal/httptest" filedriver "github.com/goftp/file-driver" "github.com/goftp/server" @@ -28,52 +29,57 @@ var ( portSource = rand.NewSource(time.Now().Unix()) ) -type mockFileTransferAgent struct { - inboundFiles []file - returnFiles []file - uploadedFile *file // non-nil on file upload - deletedFile string // filepath of last deleted file - mu sync.RWMutex // protects all fields -} +func TestCutoffTime(t *testing.T) { + now := time.Now() + loc, _ := time.LoadLocation("America/New_York") + ct := &CutoffTime{RoutingNumber: "123456789", Cutoff: 1700, Loc: loc} -func (a *mockFileTransferAgent) getInboundFiles() ([]file, error) { - a.mu.RLock() - defer a.mu.RUnlock() + // before + when := time.Date(now.Year(), now.Month(), now.Day(), 12, 34, 0, 0, loc) + if d := ct.Diff(when); d != (4*time.Hour)+(26*time.Minute) { // written at 4:37PM + t.Errorf("got %v", d) + } - return a.inboundFiles, nil -} + // 1min before + when = time.Date(now.Year(), now.Month(), now.Day(), 16, 59, 0, 0, loc) + if d := ct.Diff(when); d != 1*time.Minute { // written at 4:38PM + t.Errorf("got %v", d) + } -func (a *mockFileTransferAgent) getReturnFiles() ([]file, error) { - a.mu.RLock() - defer a.mu.RUnlock() + // 1min after + when = time.Date(now.Year(), now.Month(), now.Day(), 17, 01, 0, 0, loc) + if d := ct.Diff(when); d != -1*time.Minute { // written at 4:38PM + t.Errorf("got %v", d) + } - return a.returnFiles, nil + // after + when = time.Date(now.Year(), now.Month(), now.Day(), 18, 21, 0, 0, loc) + if d := ct.Diff(when); d != (-1*time.Hour)-(21*time.Minute) { // written at 4:40PM + t.Errorf("got %v", d) + } } -func (a *mockFileTransferAgent) uploadFile(f file) error { - a.mu.Lock() - defer a.mu.Unlock() - - // read f.contents before callers close the underlying os.Open file descriptor - bs, _ := ioutil.ReadAll(f.contents) - a.uploadedFile = &f - a.uploadedFile.contents = ioutil.NopCloser(bytes.NewReader(bs)) - return nil -} +func TestCutoffTime__JSON(t *testing.T) { + loc, _ := time.LoadLocation("America/New_York") + ct := &CutoffTime{RoutingNumber: "123456789", Cutoff: 1700, Loc: loc} -func (a *mockFileTransferAgent) delete(path string) error { - a.mu.Lock() - defer a.mu.Unlock() + var buf bytes.Buffer + if err := json.NewEncoder(&buf).Encode(ct); err != nil { + t.Fatal(err) + } - a.deletedFile = path - return nil + // Crude check of JSON properties + if !strings.Contains(buf.String(), `"RoutingNumber":"123456789"`) { + t.Error(buf.String()) + } + if !strings.Contains(buf.String(), `"Cutoff":1700`) { + t.Error(buf.String()) + } + if !strings.Contains(buf.String(), `"Location":"America/New_York"`) { + t.Error(buf.String()) + } } -func (a *mockFileTransferAgent) inboundPath() string { return "inbound/" } -func (a *mockFileTransferAgent) outboundPath() string { return "outbound/" } -func (a *mockFileTransferAgent) returnPath() string { return "return/" } -func (a *mockFileTransferAgent) close() error { return nil } - func port() int { return int(30000 + (portSource.Int63() % 9999)) } @@ -89,7 +95,7 @@ func createTestFTPServer(t *testing.T) (*server.Server, error) { Password: "password", }, Factory: &filedriver.FileDriverFactory{ - RootPath: filepath.Join("testdata", "ftp-server"), + RootPath: filepath.Join("..", "..", "testdata", "ftp-server"), Perm: server.NewSimplePerm("test", "test"), }, Hostname: "localhost", @@ -109,6 +115,45 @@ func createTestFTPServer(t *testing.T) (*server.Server, error) { return svc, nil } +var errTimeout = errors.New("timeout exceeded") + +// try will attempt to call f, but only for as long as t. If the function is still +// processing after t has elapsed then errTimeout will be returned. +func try(f func() error, t time.Duration) error { + answer := make(chan error) + go func() { + answer <- f() + }() + select { + case err := <-answer: + return err + case <-time.After(t): + return errTimeout + } +} + +func TestTry(t *testing.T) { + start := time.Now() + + err := try(func() error { + time.Sleep(50 * time.Millisecond) + return nil + }, 1*time.Second) + + if err != nil { + t.Fatal(err) + } + + diff := time.Since(start) + + if diff < 50*time.Millisecond { + t.Errorf("%v was under 50ms", diff) + } + if limit := 2 * 100 * time.Millisecond; diff > limit { + t.Errorf("%v was over %v", diff, limit) + } +} + func createTestFTPConnection(t *testing.T, svc *server.Server) (*ftp.ServerConn, error) { t.Helper() conn, err := ftp.DialTimeout(fmt.Sprintf("localhost:%d", svc.Port), 10*time.Second) @@ -164,7 +209,7 @@ func TestFTP__tlsDialOption(t *testing.T) { return // skip network calls } - cafile, err := grabConnectionCertificates(t, "google.com:443") + cafile, err := mhttptest.GrabConnectionCertificates(t, "google.com:443") if err != nil { t.Fatal(err) } @@ -179,7 +224,7 @@ func TestFTP__tlsDialOption(t *testing.T) { } } -func createTestFileTransferAgent(t *testing.T) (*server.Server, fileTransferAgent) { +func createTestFileTransferAgent(t *testing.T) (*server.Server, Agent) { svc, err := createTestFTPServer(t) if err != nil { return nil, nil @@ -189,17 +234,19 @@ func createTestFileTransferAgent(t *testing.T) (*server.Server, fileTransferAgen if !ok { t.Errorf("unknown svc.Auth: %T", svc.Auth) } - ftpConf := &ftpConfig{ - Hostname: fmt.Sprintf("%s:%d", svc.Hostname, svc.Port), - Username: auth.Name, - Password: auth.Password, - } - conf := &fileTransferConfig{ // these need to match paths at testdata/ftp-srever/ + conf := &Config{ // these need to match paths at testdata/ftp-srever/ InboundPath: "inbound", OutboundPath: "outbound", ReturnPath: "returned", } - agent, err := newFileTransferAgent(ftpConf, conf) + ftpConfigs := []*FTPConfig{ + { + Hostname: fmt.Sprintf("%s:%d", svc.Hostname, svc.Port), + Username: auth.Name, + Password: auth.Password, + }, + } + agent, err := newFTPTransferAgent(conf, ftpConfigs) if err != nil { svc.Shutdown() t.Fatalf("problem creating FileTransferAgent: %v", err) @@ -210,96 +257,96 @@ func createTestFileTransferAgent(t *testing.T) (*server.Server, fileTransferAgen func TestFTP__getInboundFiles(t *testing.T) { svc, agent := createTestFileTransferAgent(t) - defer agent.close() + defer agent.Close() defer svc.Shutdown() - files, err := agent.getInboundFiles() + files, err := agent.GetInboundFiles() if err != nil { t.Fatal(err) } if len(files) != 1 { t.Errorf("got %d files", len(files)) } - if files[0].filename != "iat-credit.ach" { + if files[0].Filename != "iat-credit.ach" { t.Errorf("files[0]=%s", files[0]) } - bs, _ := ioutil.ReadAll(files[0].contents) + bs, _ := ioutil.ReadAll(files[0].Contents) bs = bytes.TrimSpace(bs) if !strings.HasPrefix(string(bs), "101 121042882 2313801041812180000A094101Bank My Bank Name ") { t.Errorf("got %v", string(bs)) } // make sure we perform the same call and get the same result - files, err = agent.getInboundFiles() + files, err = agent.GetInboundFiles() if err != nil { t.Fatal(err) } if len(files) != 1 { t.Errorf("got %d files", len(files)) } - if files[0].filename != "iat-credit.ach" { + if files[0].Filename != "iat-credit.ach" { t.Errorf("files[0]=%s", files[0]) } } func TestFTP__getReturnFiles(t *testing.T) { svc, agent := createTestFileTransferAgent(t) - defer agent.close() + defer agent.Close() defer svc.Shutdown() - files, err := agent.getReturnFiles() + files, err := agent.GetReturnFiles() if err != nil { t.Fatal(err) } if len(files) != 1 { t.Errorf("got %d files", len(files)) } - if files[0].filename != "return-WEB.ach" { + if files[0].Filename != "return-WEB.ach" { t.Errorf("files[0]=%s", files[0]) } - bs, _ := ioutil.ReadAll(files[0].contents) + bs, _ := ioutil.ReadAll(files[0].Contents) bs = bytes.TrimSpace(bs) if !strings.HasPrefix(string(bs), "101 091400606 6910001341810170306A094101FIRST BANK & TRUST ASF APPLICATION SUPERVI ") { t.Errorf("got %v", string(bs)) } // make sure we perform the same call and get the same result - files, err = agent.getReturnFiles() + files, err = agent.GetReturnFiles() if err != nil { t.Fatal(err) } if len(files) != 1 { t.Errorf("got %d files", len(files)) } - if files[0].filename != "return-WEB.ach" { + if files[0].Filename != "return-WEB.ach" { t.Errorf("files[0]=%s", files[0]) } } func TestFTP__uploadFile(t *testing.T) { svc, agent := createTestFileTransferAgent(t) - defer agent.close() + defer agent.Close() defer svc.Shutdown() content := base.ID() - f := file{ - filename: base.ID(), - contents: ioutil.NopCloser(strings.NewReader(content)), // random file contents + f := File{ + Filename: base.ID(), + Contents: ioutil.NopCloser(strings.NewReader(content)), // random file contents } // Create outbound directory - parent := filepath.Join("testdata", "ftp-server", agent.outboundPath()) + parent := filepath.Join("..", "..", "testdata", "ftp-server", agent.OutboundPath()) os.Mkdir(parent, 0777) - if err := agent.uploadFile(f); err != nil { + if err := agent.UploadFile(f); err != nil { t.Fatal(err) } - ftpAgent, _ := agent.(*ftpFileTransferAgent) + ftpAgent, _ := agent.(*FTPTransferAgent) // manually read file contents - ftpAgent.conn.ChangeDir(agent.outboundPath()) - resp, _ := ftpAgent.conn.Retr(f.filename) + ftpAgent.conn.ChangeDir(agent.OutboundPath()) + resp, _ := ftpAgent.conn.Retr(f.Filename) if resp == nil { t.Fatal("nil File response") } @@ -313,7 +360,7 @@ func TestFTP__uploadFile(t *testing.T) { } // delete the file - if err := agent.delete(f.filename); err != nil { + if err := agent.Delete(f.Filename); err != nil { t.Fatal(err) } } diff --git a/file_transfer.go b/internal/filetransfer/ftp.go similarity index 54% rename from file_transfer.go rename to internal/filetransfer/ftp.go index 0f6dcb12a..2a14430f3 100644 --- a/file_transfer.go +++ b/internal/filetransfer/ftp.go @@ -2,7 +2,7 @@ // Use of this source code is governed by an Apache License // license that can be found in the LICENSE file. -package main +package filetransfer import ( "bytes" @@ -19,64 +19,40 @@ import ( "github.com/jlaffaye/ftp" ) -type ftpConfig struct { +type FTPConfig struct { RoutingNumber string - Hostname string - Username, Password string + Hostname string + Username string + Password string } -type fileTransferConfig struct { - RoutingNumber string - - InboundPath string - OutboundPath string - ReturnPath string -} +// FTPTransferAgent is an FTP implementation of a Agent +type FTPTransferAgent struct { + conn *ftp.ServerConn -// fileTransferAgent represents an interface for uploading and retrieving ACH files from a remote service. -type fileTransferAgent interface { - getInboundFiles() ([]file, error) - getReturnFiles() ([]file, error) - uploadFile(f file) error - delete(path string) error - - inboundPath() string - outboundPath() string - returnPath() string - - close() error -} - -// ftpFileTransferAgent is an FTP implementation of a fileTransferAgent -type ftpFileTransferAgent struct { - config *fileTransferConfig - conn *ftp.ServerConn + cfg *Config + ftpConfigs []*FTPConfig mu sync.Mutex // protects all read/write methods } -func (agent *ftpFileTransferAgent) inboundPath() string { - return agent.config.InboundPath -} - -func (agent *ftpFileTransferAgent) outboundPath() string { - return agent.config.OutboundPath -} - -func (agent *ftpFileTransferAgent) returnPath() string { - return agent.config.ReturnPath +func (a *FTPTransferAgent) findConfig() *FTPConfig { + for i := range a.ftpConfigs { + if a.ftpConfigs[i].RoutingNumber == a.cfg.RoutingNumber { + return a.ftpConfigs[i] + } + } + return nil } -func (agent *ftpFileTransferAgent) close() error { - return agent.conn.Quit() -} +func newFTPTransferAgent(cfg *Config, ftpConfigs []*FTPConfig) (*FTPTransferAgent, error) { + agent := &FTPTransferAgent{cfg: cfg, ftpConfigs: ftpConfigs} + ftpConf := agent.findConfig() + if ftpConf == nil { + return nil, fmt.Errorf("ftp: unable to find config for %s", cfg.RoutingNumber) + } -// newFileTransferAgent returns an FTP implementation of a fileTransferAgent -// -// This function reads ACH_FILE_TRANSFERS_ROOT_CAFILE for a file with root certificates to be used -// in all secured connections. -func newFileTransferAgent(ftpConf *ftpConfig, conf *fileTransferConfig) (fileTransferAgent, error) { opts := []ftp.DialOption{ ftp.DialWithTimeout(30 * time.Second), ftp.DialWithDisabledEPSV(false), @@ -88,19 +64,17 @@ func newFileTransferAgent(ftpConf *ftpConfig, conf *fileTransferConfig) (fileTra if tlsOpt != nil { opts = append(opts, *tlsOpt) } + // Make the first connection conn, err := ftp.Dial(ftpConf.Hostname, opts...) if err != nil { return nil, err } - if err := conn.Login(ftpConf.Username, ftpConf.Password); err != nil { return nil, err } - return &ftpFileTransferAgent{ - config: conf, - conn: conn, - }, nil + agent.conn = conn + return agent, nil } func tlsDialOption(caFilePath string) (*ftp.DialOption, error) { @@ -126,9 +100,25 @@ func tlsDialOption(caFilePath string) (*ftp.DialOption, error) { return &opt, nil } -func (agent *ftpFileTransferAgent) delete(path string) error { +func (agent *FTPTransferAgent) Close() error { + return agent.conn.Quit() +} + +func (agent *FTPTransferAgent) InboundPath() string { + return agent.cfg.InboundPath +} + +func (agent *FTPTransferAgent) OutboundPath() string { + return agent.cfg.OutboundPath +} + +func (agent *FTPTransferAgent) ReturnPath() string { + return agent.cfg.ReturnPath +} + +func (agent *FTPTransferAgent) Delete(path string) error { if path == "" || strings.HasSuffix(path, "/") { - return fmt.Errorf("ftpFileTransferAgent: invalid path %v", path) + return fmt.Errorf("FTPTransferAgent: invalid path %v", path) } return agent.conn.Delete(path) } @@ -136,35 +126,36 @@ func (agent *ftpFileTransferAgent) delete(path string) error { // uploadFile saves the content of File at the given filename in the OutboundPath directory // // The File's contents will always be closed -func (agent *ftpFileTransferAgent) uploadFile(f file) error { +func (agent *FTPTransferAgent) UploadFile(f File) error { + defer f.Close() + agent.mu.Lock() defer agent.mu.Unlock() - defer f.contents.Close() // close File + + ftpConf := agent.findConfig() + if ftpConf == nil { + return fmt.Errorf("ftp.uploadFile: unable to find config for %s", agent.cfg.RoutingNumber) + } // move into inbound directory and set a trigger to undo - if err := agent.conn.ChangeDir(agent.config.OutboundPath); err != nil { + if err := agent.conn.ChangeDir(agent.cfg.OutboundPath); err != nil { return err } defer agent.conn.ChangeDirToParent() // Write file contents into path - return agent.conn.Stor(f.filename, f.contents) -} - -type file struct { - filename string - contents io.ReadCloser + return agent.conn.Stor(f.Filename, f.Contents) } -func (agent *ftpFileTransferAgent) getInboundFiles() ([]file, error) { - return agent.readFiles(agent.config.InboundPath) +func (agent *FTPTransferAgent) GetInboundFiles() ([]File, error) { + return agent.readFiles(agent.cfg.InboundPath) } -func (agent *ftpFileTransferAgent) getReturnFiles() ([]file, error) { - return agent.readFiles(agent.config.ReturnPath) +func (agent *FTPTransferAgent) GetReturnFiles() ([]File, error) { + return agent.readFiles(agent.cfg.ReturnPath) } -func (agent *ftpFileTransferAgent) readFiles(path string) ([]file, error) { +func (agent *FTPTransferAgent) readFiles(path string) ([]File, error) { agent.mu.Lock() defer agent.mu.Unlock() @@ -179,7 +170,7 @@ func (agent *ftpFileTransferAgent) readFiles(path string) ([]file, error) { if err != nil { return nil, err } - var files []file + var files []File for i := range items { resp, err := agent.conn.Retr(items[i]) if err != nil { @@ -189,15 +180,15 @@ func (agent *ftpFileTransferAgent) readFiles(path string) ([]file, error) { if err != nil { return nil, fmt.Errorf("problem reading %s: %v", items[i], err) } - files = append(files, file{ - filename: items[i], - contents: r, + files = append(files, File{ + Filename: items[i], + Contents: r, }) } return files, nil } -func (agent *ftpFileTransferAgent) readResponse(resp *ftp.Response) (io.ReadCloser, error) { +func (*FTPTransferAgent) readResponse(resp *ftp.Response) (io.ReadCloser, error) { defer resp.Close() var buf bytes.Buffer diff --git a/internal/httptest/certs.go b/internal/httptest/certs.go new file mode 100644 index 000000000..05904374f --- /dev/null +++ b/internal/httptest/certs.go @@ -0,0 +1,48 @@ +// Copyright 2018 The Moov Authors +// Use of this source code is governed by an Apache License +// license that can be found in the LICENSE file. + +package httptest + +import ( + "bytes" + "crypto/tls" + "encoding/pem" + "io/ioutil" + "net" + "testing" + "time" +) + +// GrabConnectionCertificates returns a filepath of certificate chain from a given address's +// server. This is useful for adding extra root CA's to network clients +func GrabConnectionCertificates(t *testing.T, addr string) (string, error) { + dialer := &net.Dialer{Timeout: 10 * time.Second} + conn, err := tls.DialWithDialer(dialer, "tcp", addr, nil) + if err != nil { + t.Error(err) + } + defer conn.Close() + + fd, err := ioutil.TempFile("", "conn-certs") + if err != nil { + t.Fatal(err) + } + + // Write x509 certs to disk + certs := conn.ConnectionState().PeerCertificates + var buf bytes.Buffer + for i := range certs { + b := &pem.Block{ + Type: "CERTIFICATE", + Bytes: certs[i].Raw, + } + if err := pem.Encode(&buf, b); err != nil { + t.Fatal(err) + } + } + if err := ioutil.WriteFile(fd.Name(), buf.Bytes(), 0644); err != nil { + t.Fatal(err) + } + return fd.Name(), nil +} diff --git a/main.go b/main.go index 0d4e033e9..c50dade16 100644 --- a/main.go +++ b/main.go @@ -19,6 +19,7 @@ import ( "github.com/moov-io/base/admin" "github.com/moov-io/base/http/bind" "github.com/moov-io/paygate/internal/database" + "github.com/moov-io/paygate/internal/filetransfer" "github.com/moov-io/paygate/internal/version" "github.com/moov-io/paygate/pkg/achclient" @@ -141,8 +142,8 @@ func main() { achStorageDir = "./storage/" os.Mkdir(achStorageDir, 0777) } - fileTransferRepo := newFileTransferRepository(db, os.Getenv("DATABASE_TYPE")) - defer fileTransferRepo.close() + fileTransferRepo := filetransfer.NewRepository(db, os.Getenv("DATABASE_TYPE")) + defer fileTransferRepo.Close() fileTransferController, err := newFileTransferController(logger, achStorageDir, fileTransferRepo, achClient, accountsClient, accountsCallsDisabled) if err != nil { panic(fmt.Sprintf("ERROR: creating ACH file transfer controller: %v", err)) @@ -155,7 +156,7 @@ func main() { go fileTransferController.startPeriodicFileOperations(ctx, depositoryRepo, transferRepo) // side-effect register HTTP routes - addFileTransferConfigRoutes(logger, adminServer, fileTransferRepo) + filetransfer.AddFileTransferConfigRoutes(logger, adminServer, fileTransferRepo) } // Create HTTP handler From 9265406723449bd3227e8c0689848c901922b14f Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Mon, 15 Jul 2019 19:03:55 -0500 Subject: [PATCH 02/26] internal/filetransfer: initial addition of the SFTP Agent --- go.mod | 3 + go.sum | 5 + internal/filetransfer/config.go | 68 ++++++-- internal/filetransfer/file_transfer.go | 6 +- internal/filetransfer/sftp.go | 206 +++++++++++++++++++++++++ 5 files changed, 272 insertions(+), 16 deletions(-) create mode 100644 internal/filetransfer/sftp.go diff --git a/go.mod b/go.mod index 82271b969..282118f88 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/goftp/server v0.0.0-20190304020633-eabccc535b5a github.com/gorilla/mux v1.7.2 github.com/jlaffaye/ftp v0.0.0-20190522102603-9284a88df536 + github.com/kr/fs v0.1.0 // indirect github.com/mattn/go-sqlite3 v1.10.0 github.com/moov-io/accounts v0.3.1-0.20190522191929-18df7fa8fa58 github.com/moov-io/ach v0.6.0-rc7.0.20190627161251-130df29fdfbc @@ -15,7 +16,9 @@ require ( github.com/moov-io/fed v0.1.4 github.com/moov-io/ofac v0.7.1-0.20190506160459-5cf50ac71326 github.com/ory/dockertest v3.3.4+incompatible + github.com/pkg/sftp v1.10.0 github.com/prometheus/client_golang v1.0.0 + golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 golang.org/x/net v0.0.0-20190517184700-3ec191127204 // indirect golang.org/x/oauth2 v0.0.0-20190517184700-950ef44c6e07 // indirect golang.org/x/sys v0.0.0-20190520161452-ad400b127469 // indirect diff --git a/go.sum b/go.sum index 027981b35..0a11bd971 100644 --- a/go.sum +++ b/go.sum @@ -64,6 +64,8 @@ github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7V github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= github.com/konsorten/go-windows-terminal-sequences v1.0.2 h1:DB17ag19krx9CFsz4o3enTrPXyIXCl+2iCXH/aMAp9s= github.com/konsorten/go-windows-terminal-sequences v1.0.2/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= +github.com/kr/fs v0.1.0 h1:Jskdu9ieNAYnjxsi0LbQp1ulIKZV1LAFgK1tWhpZgl8= +github.com/kr/fs v0.1.0/go.mod h1:FFnZGqtBN9Gxj7eW1uZ42v5BccTP0vu6NEaFoC2HwRg= github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515 h1:T+h1c/A9Gawja4Y9mFVWj2vyii2bbUNDw3kt9VxK2EY= github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= @@ -104,6 +106,8 @@ github.com/ory/dockertest v3.3.4+incompatible/go.mod h1:1vX4m9wsvi00u5bseYwXaSnh github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/pkg/sftp v1.10.0 h1:DGA1KlA9esU6WcicH+P8PxFZOl15O6GYtab1cIJdOlE= +github.com/pkg/sftp v1.10.0/go.mod h1:NxmoDg/QLVWluQDUYG7XBZTLUpKeFa8e3aMf1BfjyHk= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/prometheus/client_golang v0.9.1/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw= @@ -145,6 +149,7 @@ github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UV github.com/xrash/smetrics v0.0.0-20170218160415-a3153f7040e9/go.mod h1:N3UwUGtsrSj3ccvlPHLoLsHnpR27oXr4ZE984MbSER8= go4.org v0.0.0-20190430205326-94abd6928b1d/go.mod h1:MkTOUMDaeVYJUOUsaDXIhWPZYa1yOyC1qaOBpL57BhE= golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 h1:VklqNMn3ovrHsnt90PveolxSbWFaJdECFbxSq0Mqo2M= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20181114220301-adae6a3d119a/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= diff --git a/internal/filetransfer/config.go b/internal/filetransfer/config.go index 7925dfd94..1e4296aea 100644 --- a/internal/filetransfer/config.go +++ b/internal/filetransfer/config.go @@ -21,9 +21,11 @@ import ( ) type Repository interface { + GetConfigs() ([]*Config, error) GetCutoffTimes() ([]*CutoffTime, error) + GetFTPConfigs() ([]*FTPConfig, error) - GetConfigs() ([]*Config, error) + GetSFTPConfigs() ([]*SFTPConfig, error) Close() error } @@ -76,6 +78,30 @@ func (r *sqliteRepository) GetCounts() (int, int, int) { return count("cutoff_times"), count("ftp_configs"), count("file_transfer_configs") } +func (r *sqliteRepository) GetConfigs() ([]*Config, error) { + query := `select routing_number, inbound_path, outbound_path, return_path from file_transfer_configs;` + stmt, err := r.db.Prepare(query) + if err != nil { + return nil, err + } + defer stmt.Close() + + var configs []*Config + rows, err := stmt.Query() + if err != nil { + return nil, err + } + defer rows.Close() + for rows.Next() { + var cfg Config + if err := rows.Scan(&cfg.RoutingNumber, &cfg.InboundPath, &cfg.OutboundPath, &cfg.ReturnPath); err != nil { + return nil, fmt.Errorf("GetConfigs: scan: %v", err) + } + configs = append(configs, &cfg) + } + return configs, rows.Err() +} + func (r *sqliteRepository) GetCutoffTimes() ([]*CutoffTime, error) { query := `select routing_number, cutoff, location from cutoff_times;` stmt, err := r.db.Prepare(query) @@ -130,24 +156,24 @@ func (r *sqliteRepository) GetFTPConfigs() ([]*FTPConfig, error) { return configs, rows.Err() } -func (r *sqliteRepository) GetConfigs() ([]*Config, error) { - query := `select routing_number, inbound_path, outbound_path, return_path from file_transfer_configs;` +func (r *sqliteRepository) GetSFTPConfigs() ([]*SFTPConfig, error) { + query := `select routing_number, hostname, username, password, client_private_key from sftp_configs;` stmt, err := r.db.Prepare(query) if err != nil { return nil, err } defer stmt.Close() - var configs []*Config + var configs []*SFTPConfig rows, err := stmt.Query() if err != nil { return nil, err } defer rows.Close() for rows.Next() { - var cfg Config - if err := rows.Scan(&cfg.RoutingNumber, &cfg.InboundPath, &cfg.OutboundPath, &cfg.ReturnPath); err != nil { - return nil, fmt.Errorf("GetConfigs: scan: %v", err) + var cfg SFTPConfig + if err := rows.Scan(&cfg.RoutingNumber, &cfg.Hostname, &cfg.Username, &cfg.Password, &cfg.ClientPrivateKey); err != nil { + return nil, fmt.Errorf("GetSFTPConfigs: scan: %v", err) } configs = append(configs, &cfg) } @@ -160,6 +186,17 @@ type localFileTransferRepository struct{} func (r *localFileTransferRepository) Close() error { return nil } +func (r *localFileTransferRepository) GetConfigs() ([]*Config, error) { + return []*Config{ + { + RoutingNumber: "121042882", + InboundPath: "inbound/", // below configs match paygate's testdata/ftp-server/ + OutboundPath: "outbound/", + ReturnPath: "returned/", + }, + }, nil +} + func (r *localFileTransferRepository) GetCutoffTimes() ([]*CutoffTime, error) { nyc, _ := time.LoadLocation("America/New_York") return []*CutoffTime{ @@ -182,14 +219,15 @@ func (r *localFileTransferRepository) GetFTPConfigs() ([]*FTPConfig, error) { }, nil } -func (r *localFileTransferRepository) GetConfigs() ([]*Config, error) { - return []*Config{ - { - RoutingNumber: "121042882", - InboundPath: "inbound/", // below configs match paygate's testdata/ftp-server/ - OutboundPath: "outbound/", - ReturnPath: "returned/", - }, +func (r *localFileTransferRepository) GetSFTPConfigs() ([]*SFTPConfig, error) { + return []*SFTPConfig{ + // { + // RoutingNumber: "121042882", // from 'go run ./cmd/server' in Accounts + // Hostname: "localhost:2121", // below configs for moov/fftp:v0.1.0 + // Username: "admin", + // // Password: "123456", + // // ClientPrivateKey: "...", // Base64 encoded or PEM format + // }, }, nil } diff --git a/internal/filetransfer/file_transfer.go b/internal/filetransfer/file_transfer.go index 9bb15c4ca..c5a655c90 100644 --- a/internal/filetransfer/file_transfer.go +++ b/internal/filetransfer/file_transfer.go @@ -55,7 +55,11 @@ func New(_type string, cfg *Config, repo Repository) (Agent, error) { } return newFTPTransferAgent(cfg, ftpConfigs) case "sftp": - return nil, nil // TODO(adam): impl + sftpConfigs, err := repo.GetSFTPConfigs() + if err != nil { + return nil, fmt.Errorf("filetransfer: error creating new SFTP client: %v", err) + } + return newSFTPTransferAgent(cfg, sftpConfigs) default: return nil, fmt.Errorf("filetransfer: unknown type %s", _type) } diff --git a/internal/filetransfer/sftp.go b/internal/filetransfer/sftp.go new file mode 100644 index 000000000..5638b1753 --- /dev/null +++ b/internal/filetransfer/sftp.go @@ -0,0 +1,206 @@ +// Copyright 2019 The Moov Authors +// Use of this source code is governed by an Apache License +// license that can be found in the LICENSE file. + +package filetransfer + +import ( + "bytes" + "fmt" + "io" + "io/ioutil" + "sync" + "time" + + "github.com/pkg/sftp" + "golang.org/x/crypto/ssh" +) + +type SFTPConfig struct { + RoutingNumber string + + Hostname string + Username string + + Password string + ClientPrivateKey string +} + +type SFTPTransferAgent struct { + conn *ssh.Client + client *sftp.Client + + cfg *Config + sftpConfigs []*SFTPConfig + + mu sync.Mutex // protects all read/write methods +} + +func (a *SFTPTransferAgent) findConfig() *SFTPConfig { + for i := range a.sftpConfigs { + if a.sftpConfigs[i].RoutingNumber == a.cfg.RoutingNumber { + return a.sftpConfigs[i] + } + } + return nil +} + +func newSFTPTransferAgent(cfg *Config, sftpConfigs []*SFTPConfig) (*SFTPTransferAgent, error) { + agent := &SFTPTransferAgent{cfg: cfg} + sftpConf := agent.findConfig() + if sftpConf == nil { + return nil, fmt.Errorf("sftp: unable to find config for %s", cfg.RoutingNumber) + } + + conn, err := sshConnect(sftpConf) + if err != nil { + return nil, fmt.Errorf("filetransfer: %v", err) + } + agent.conn = conn + + // Setup our SFTP client + var opts = []sftp.ClientOption{ + // TODO(adam): Thoughts on these defaults? + // // Q(adam): Would we ever have multiple requests to the same file? + // // See: https://godoc.org/github.com/pkg/sftp#MaxConcurrentRequestsPerFile + // sftp.MaxConcurrentRequestsPerFile(64), + // // The docs suggest lowering this on "failed to send packet header: EOF" errors, + // // so we're going to lower it by default (which is 32768). + // sftp.MaxPacket(29999), + } + client, err := sftp.NewClient(agent.conn, opts...) + if err != nil { + return nil, fmt.Errorf("filetransfer: sftp connect: %v", err) + } + agent.client = client + + return agent, nil +} + +func sshConnect(sftpConf *SFTPConfig) (*ssh.Client, error) { + conf := &ssh.ClientConfig{ + User: sftpConf.Username, + Timeout: 30 * time.Second, + // TODO(adam): How to read this per-host? + // var hostKey ssh.PublicKey + // HostKeyCallback: ssh.FixedHostKey(hostKey) + HostKeyCallback: ssh.InsecureIgnoreHostKey(), // TODO(adam): fix + } + switch { + case sftpConf.Password != "": + conf.Auth = append(conf.Auth, ssh.Password(sftpConf.Password)) + case sftpConf.ClientPrivateKey != "": + // TODO(adam): attempt base64 decode also + signer, err := ssh.ParsePrivateKey([]byte(sftpConf.ClientPrivateKey)) + if err != nil { + return nil, fmt.Errorf("sshConnect: failed to read client private key: %v", err) + } + conf.Auth = append(conf.Auth, ssh.PublicKeys(signer)) + default: + return nil, fmt.Errorf("sshConnect: no auth method provided for routingNumber=%s", sftpConf.RoutingNumber) + } + + // Connect to the remote server + client, err := ssh.Dial("tcp", sftpConf.Hostname, conf) + if err != nil { + return nil, fmt.Errorf("sshConnect: error with routingNumber=%s: %v", sftpConf.RoutingNumber, err) + } + return client, nil +} + +func (a *SFTPTransferAgent) Close() error { + if a == nil { + return nil + } + e1 := a.client.Close() + e2 := a.conn.Close() + if e1 != nil || e2 != nil { + return fmt.Errorf("sftp: agent close e1=%v e2=%v", e1, e2) + } + return nil +} + +func (agent *SFTPTransferAgent) InboundPath() string { + return agent.cfg.InboundPath +} + +func (agent *SFTPTransferAgent) OutboundPath() string { + return agent.cfg.OutboundPath +} + +func (agent *SFTPTransferAgent) ReturnPath() string { + return agent.cfg.ReturnPath +} + +func (agent *SFTPTransferAgent) Delete(path string) error { + info, err := agent.client.Stat(path) + if err != nil { + return fmt.Errorf("sftp: delete stat: %v", err) + } + if info != nil { + if err := agent.client.Remove(path); err != nil { + return fmt.Errorf("sftp: delete: %v", err) + } + } + return nil // not found +} + +// uploadFile saves the content of File at the given filename in the OutboundPath directory +// +// The File's contents will always be closed +func (agent *SFTPTransferAgent) UploadFile(f File) error { + defer f.Close() + + agent.mu.Lock() + defer agent.mu.Unlock() + + fd, err := agent.client.Create(f.Filename) + if err != nil { + return fmt.Errorf("sftp: problem creating %s: %v", f.Filename, err) + } + n, err := io.Copy(fd, f.Contents) + if n == 0 || err != nil { + return fmt.Errorf("sftp: problem copying (n=%d) %s: %v", n, f.Filename, err) + } + if err := fd.Close(); err != nil { + return fmt.Errorf("sftp: problem closing %s: %v", f.Filename, err) + } + if err := fd.Chmod(0600); err != nil { + return fmt.Errorf("sftp: problem chmod %s: %v", f.Filename, err) + } + return nil +} + +func (agent *SFTPTransferAgent) GetInboundFiles() ([]File, error) { + return agent.readFiles(agent.cfg.InboundPath) +} + +func (agent *SFTPTransferAgent) GetReturnFiles() ([]File, error) { + return agent.readFiles(agent.cfg.ReturnPath) +} + +func (agent *SFTPTransferAgent) readFiles(dir string) ([]File, error) { + infos, err := agent.client.ReadDir(dir) + if err != nil { + return nil, fmt.Errorf("sftp: readdir %s: %v", dir, err) + } + + var files []File + for i := range infos { + fd, err := agent.client.Open(infos[i].Name()) + if err != nil { + return nil, fmt.Errorf("sftp: open %s: %v", infos[i].Name(), err) + } + var buf bytes.Buffer + if n, err := io.Copy(&buf, fd); n == 0 || err != nil { + fd.Close() + return nil, fmt.Errorf("sftp: read (n=%d) %s: %v", n, infos[i].Name(), err) + } + fd.Close() + files = append(files, File{ + Filename: infos[i].Name(), + Contents: ioutil.NopCloser(&buf), + }) + } + return files, nil +} From 4e0292e7fb7a3063b41aa87e90d452497a35d90f Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Mon, 15 Jul 2019 19:04:21 -0500 Subject: [PATCH 03/26] internal/filetransfer: split out FTP tests from file_transfer_test.go --- internal/filetransfer/file_transfer_test.go | 268 ------------------ internal/filetransfer/ftp_test.go | 283 ++++++++++++++++++++ 2 files changed, 283 insertions(+), 268 deletions(-) create mode 100644 internal/filetransfer/ftp_test.go diff --git a/internal/filetransfer/file_transfer_test.go b/internal/filetransfer/file_transfer_test.go index 9dfa6e503..b57cd7971 100644 --- a/internal/filetransfer/file_transfer_test.go +++ b/internal/filetransfer/file_transfer_test.go @@ -7,26 +7,12 @@ package filetransfer import ( "bytes" "encoding/json" - "errors" "fmt" - "io/ioutil" - "math/rand" - "os" - "path/filepath" "strings" "testing" "time" - "github.com/moov-io/base" - mhttptest "github.com/moov-io/paygate/internal/httptest" - - filedriver "github.com/goftp/file-driver" "github.com/goftp/server" - "github.com/jlaffaye/ftp" -) - -var ( - portSource = rand.NewSource(time.Now().Unix()) ) func TestCutoffTime(t *testing.T) { @@ -80,150 +66,6 @@ func TestCutoffTime__JSON(t *testing.T) { } } -func port() int { - return int(30000 + (portSource.Int63() % 9999)) -} - -func createTestFTPServer(t *testing.T) (*server.Server, error) { - t.Helper() - if testing.Short() { - t.Skip("skipping due to -short") - } - opts := &server.ServerOpts{ - Auth: &server.SimpleAuth{ - Name: "moov", - Password: "password", - }, - Factory: &filedriver.FileDriverFactory{ - RootPath: filepath.Join("..", "..", "testdata", "ftp-server"), - Perm: server.NewSimplePerm("test", "test"), - }, - Hostname: "localhost", - Port: port(), - Logger: &server.DiscardLogger{}, - } - svc := server.NewServer(opts) - if svc == nil { - return nil, errors.New("nil FTP server") - } - if err := try(func() error { return svc.ListenAndServe() }, 50*time.Millisecond); err != nil { - if err == errTimeout { - return svc, nil - } - return nil, err - } - return svc, nil -} - -var errTimeout = errors.New("timeout exceeded") - -// try will attempt to call f, but only for as long as t. If the function is still -// processing after t has elapsed then errTimeout will be returned. -func try(f func() error, t time.Duration) error { - answer := make(chan error) - go func() { - answer <- f() - }() - select { - case err := <-answer: - return err - case <-time.After(t): - return errTimeout - } -} - -func TestTry(t *testing.T) { - start := time.Now() - - err := try(func() error { - time.Sleep(50 * time.Millisecond) - return nil - }, 1*time.Second) - - if err != nil { - t.Fatal(err) - } - - diff := time.Since(start) - - if diff < 50*time.Millisecond { - t.Errorf("%v was under 50ms", diff) - } - if limit := 2 * 100 * time.Millisecond; diff > limit { - t.Errorf("%v was over %v", diff, limit) - } -} - -func createTestFTPConnection(t *testing.T, svc *server.Server) (*ftp.ServerConn, error) { - t.Helper() - conn, err := ftp.DialTimeout(fmt.Sprintf("localhost:%d", svc.Port), 10*time.Second) - if err != nil { - t.Fatal(err) - } - if err := conn.Login("moov", "password"); err != nil { - t.Fatal(err) - } - return conn, nil -} - -func TestFTP(t *testing.T) { - svc, err := createTestFTPServer(t) - if err != nil { - t.Fatal(err) - } - defer svc.Shutdown() - - conn, err := createTestFTPConnection(t, svc) - if err != nil { - t.Fatal(err) - } - defer conn.Quit() - - dir, err := conn.CurrentDir() - if err != nil { - t.Fatal(err) - } - if dir == "" { - t.Error("empty current dir?!") - } - - // Change directory - if err := conn.ChangeDir("scratch"); err != nil { - t.Error(err) - } - - // Read a file we know should exist - resp, err := conn.RetrFrom("existing-file", 0) // offset of 0 - if err != nil { - t.Error(err) - } - bs, _ := ioutil.ReadAll(resp) - bs = bytes.TrimSpace(bs) - if !bytes.Equal(bs, []byte("Hello, World!")) { - t.Errorf("got %q", string(bs)) - } -} - -func TestFTP__tlsDialOption(t *testing.T) { - if testing.Short() { - return // skip network calls - } - - cafile, err := mhttptest.GrabConnectionCertificates(t, "google.com:443") - if err != nil { - t.Fatal(err) - } - defer os.Remove(cafile) - - opt, err := tlsDialOption(cafile) - if err != nil { - t.Fatal(err) - } - if opt == nil { - t.Fatal("nil tls DialOption") - } -} - func createTestFileTransferAgent(t *testing.T) (*server.Server, Agent) { svc, err := createTestFTPServer(t) if err != nil { @@ -254,113 +96,3 @@ func createTestFileTransferAgent(t *testing.T) (*server.Server, Agent) { } return svc, agent } - -func TestFTP__getInboundFiles(t *testing.T) { - svc, agent := createTestFileTransferAgent(t) - defer agent.Close() - defer svc.Shutdown() - - files, err := agent.GetInboundFiles() - if err != nil { - t.Fatal(err) - } - if len(files) != 1 { - t.Errorf("got %d files", len(files)) - } - if files[0].Filename != "iat-credit.ach" { - t.Errorf("files[0]=%s", files[0]) - } - bs, _ := ioutil.ReadAll(files[0].Contents) - bs = bytes.TrimSpace(bs) - if !strings.HasPrefix(string(bs), "101 121042882 2313801041812180000A094101Bank My Bank Name ") { - t.Errorf("got %v", string(bs)) - } - - // make sure we perform the same call and get the same result - files, err = agent.GetInboundFiles() - if err != nil { - t.Fatal(err) - } - if len(files) != 1 { - t.Errorf("got %d files", len(files)) - } - if files[0].Filename != "iat-credit.ach" { - t.Errorf("files[0]=%s", files[0]) - } -} - -func TestFTP__getReturnFiles(t *testing.T) { - svc, agent := createTestFileTransferAgent(t) - defer agent.Close() - defer svc.Shutdown() - - files, err := agent.GetReturnFiles() - if err != nil { - t.Fatal(err) - } - if len(files) != 1 { - t.Errorf("got %d files", len(files)) - } - if files[0].Filename != "return-WEB.ach" { - t.Errorf("files[0]=%s", files[0]) - } - bs, _ := ioutil.ReadAll(files[0].Contents) - bs = bytes.TrimSpace(bs) - if !strings.HasPrefix(string(bs), "101 091400606 6910001341810170306A094101FIRST BANK & TRUST ASF APPLICATION SUPERVI ") { - t.Errorf("got %v", string(bs)) - } - - // make sure we perform the same call and get the same result - files, err = agent.GetReturnFiles() - if err != nil { - t.Fatal(err) - } - if len(files) != 1 { - t.Errorf("got %d files", len(files)) - } - if files[0].Filename != "return-WEB.ach" { - t.Errorf("files[0]=%s", files[0]) - } -} - -func TestFTP__uploadFile(t *testing.T) { - svc, agent := createTestFileTransferAgent(t) - defer agent.Close() - defer svc.Shutdown() - - content := base.ID() - f := File{ - Filename: base.ID(), - Contents: ioutil.NopCloser(strings.NewReader(content)), // random file contents - } - - // Create outbound directory - parent := filepath.Join("..", "..", "testdata", "ftp-server", agent.OutboundPath()) - os.Mkdir(parent, 0777) - - if err := agent.UploadFile(f); err != nil { - t.Fatal(err) - } - - ftpAgent, _ := agent.(*FTPTransferAgent) - - // manually read file contents - ftpAgent.conn.ChangeDir(agent.OutboundPath()) - resp, _ := ftpAgent.conn.Retr(f.Filename) - if resp == nil { - t.Fatal("nil File response") - } - r, _ := ftpAgent.readResponse(resp) - if r == nil { - t.Fatal("failed to read file") - } - bs, _ := ioutil.ReadAll(r) - if !bytes.Equal(bs, []byte(content)) { - t.Errorf("got %q", string(bs)) - } - - // delete the file - if err := agent.Delete(f.Filename); err != nil { - t.Fatal(err) - } -} diff --git a/internal/filetransfer/ftp_test.go b/internal/filetransfer/ftp_test.go new file mode 100644 index 000000000..18946c166 --- /dev/null +++ b/internal/filetransfer/ftp_test.go @@ -0,0 +1,283 @@ +// Copyright 2019 The Moov Authors +// Use of this source code is governed by an Apache License +// license that can be found in the LICENSE file. + +package filetransfer + +import ( + "bytes" + "errors" + "fmt" + "io/ioutil" + "math/rand" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/moov-io/base" + mhttptest "github.com/moov-io/paygate/internal/httptest" + + filedriver "github.com/goftp/file-driver" + "github.com/goftp/server" + "github.com/jlaffaye/ftp" +) + +var ( + portSource = rand.NewSource(time.Now().Unix()) +) + +func port() int { + return int(30000 + (portSource.Int63() % 9999)) +} + +func createTestFTPServer(t *testing.T) (*server.Server, error) { + t.Helper() + if testing.Short() { + t.Skip("skipping due to -short") + } + opts := &server.ServerOpts{ + Auth: &server.SimpleAuth{ + Name: "moov", + Password: "password", + }, + Factory: &filedriver.FileDriverFactory{ + RootPath: filepath.Join("..", "..", "testdata", "ftp-server"), + Perm: server.NewSimplePerm("test", "test"), + }, + Hostname: "localhost", + Port: port(), + Logger: &server.DiscardLogger{}, + } + svc := server.NewServer(opts) + if svc == nil { + return nil, errors.New("nil FTP server") + } + if err := try(func() error { return svc.ListenAndServe() }, 50*time.Millisecond); err != nil { + if err == errTimeout { + return svc, nil + } + return nil, err + } + return svc, nil +} + +var errTimeout = errors.New("timeout exceeded") + +// try will attempt to call f, but only for as long as t. If the function is still +// processing after t has elapsed then errTimeout will be returned. +func try(f func() error, t time.Duration) error { + answer := make(chan error) + go func() { + answer <- f() + }() + select { + case err := <-answer: + return err + case <-time.After(t): + return errTimeout + } +} + +func TestTry(t *testing.T) { + start := time.Now() + + err := try(func() error { + time.Sleep(50 * time.Millisecond) + return nil + }, 1*time.Second) + + if err != nil { + t.Fatal(err) + } + + diff := time.Since(start) + + if diff < 50*time.Millisecond { + t.Errorf("%v was under 50ms", diff) + } + if limit := 2 * 100 * time.Millisecond; diff > limit { + t.Errorf("%v was over %v", diff, limit) + } +} + +func createTestFTPConnection(t *testing.T, svc *server.Server) (*ftp.ServerConn, error) { + t.Helper() + conn, err := ftp.DialTimeout(fmt.Sprintf("localhost:%d", svc.Port), 10*time.Second) + if err != nil { + t.Fatal(err) + } + if err := conn.Login("moov", "password"); err != nil { + t.Fatal(err) + } + return conn, nil +} + +func TestFTP(t *testing.T) { + svc, err := createTestFTPServer(t) + if err != nil { + t.Fatal(err) + } + defer svc.Shutdown() + + conn, err := createTestFTPConnection(t, svc) + if err != nil { + t.Fatal(err) + } + defer conn.Quit() + + dir, err := conn.CurrentDir() + if err != nil { + t.Fatal(err) + } + if dir == "" { + t.Error("empty current dir?!") + } + + // Change directory + if err := conn.ChangeDir("scratch"); err != nil { + t.Error(err) + } + + // Read a file we know should exist + resp, err := conn.RetrFrom("existing-file", 0) // offset of 0 + if err != nil { + t.Error(err) + } + bs, _ := ioutil.ReadAll(resp) + bs = bytes.TrimSpace(bs) + if !bytes.Equal(bs, []byte("Hello, World!")) { + t.Errorf("got %q", string(bs)) + } +} + +func TestFTP__tlsDialOption(t *testing.T) { + if testing.Short() { + return // skip network calls + } + + cafile, err := mhttptest.GrabConnectionCertificates(t, "google.com:443") + if err != nil { + t.Fatal(err) + } + defer os.Remove(cafile) + + opt, err := tlsDialOption(cafile) + if err != nil { + t.Fatal(err) + } + if opt == nil { + t.Fatal("nil tls DialOption") + } +} + +func TestFTP__getInboundFiles(t *testing.T) { + svc, agent := createTestFileTransferAgent(t) + defer agent.Close() + defer svc.Shutdown() + + files, err := agent.GetInboundFiles() + if err != nil { + t.Fatal(err) + } + if len(files) != 1 { + t.Errorf("got %d files", len(files)) + } + if files[0].Filename != "iat-credit.ach" { + t.Errorf("files[0]=%s", files[0]) + } + bs, _ := ioutil.ReadAll(files[0].Contents) + bs = bytes.TrimSpace(bs) + if !strings.HasPrefix(string(bs), "101 121042882 2313801041812180000A094101Bank My Bank Name ") { + t.Errorf("got %v", string(bs)) + } + + // make sure we perform the same call and get the same result + files, err = agent.GetInboundFiles() + if err != nil { + t.Fatal(err) + } + if len(files) != 1 { + t.Errorf("got %d files", len(files)) + } + if files[0].Filename != "iat-credit.ach" { + t.Errorf("files[0]=%s", files[0]) + } +} + +func TestFTP__getReturnFiles(t *testing.T) { + svc, agent := createTestFileTransferAgent(t) + defer agent.Close() + defer svc.Shutdown() + + files, err := agent.GetReturnFiles() + if err != nil { + t.Fatal(err) + } + if len(files) != 1 { + t.Errorf("got %d files", len(files)) + } + if files[0].Filename != "return-WEB.ach" { + t.Errorf("files[0]=%s", files[0]) + } + bs, _ := ioutil.ReadAll(files[0].Contents) + bs = bytes.TrimSpace(bs) + if !strings.HasPrefix(string(bs), "101 091400606 6910001341810170306A094101FIRST BANK & TRUST ASF APPLICATION SUPERVI ") { + t.Errorf("got %v", string(bs)) + } + + // make sure we perform the same call and get the same result + files, err = agent.GetReturnFiles() + if err != nil { + t.Fatal(err) + } + if len(files) != 1 { + t.Errorf("got %d files", len(files)) + } + if files[0].Filename != "return-WEB.ach" { + t.Errorf("files[0]=%s", files[0]) + } +} + +func TestFTP__uploadFile(t *testing.T) { + svc, agent := createTestFileTransferAgent(t) + defer agent.Close() + defer svc.Shutdown() + + content := base.ID() + f := File{ + Filename: base.ID(), + Contents: ioutil.NopCloser(strings.NewReader(content)), // random file contents + } + + // Create outbound directory + parent := filepath.Join("..", "..", "testdata", "ftp-server", agent.OutboundPath()) + os.Mkdir(parent, 0777) + + if err := agent.UploadFile(f); err != nil { + t.Fatal(err) + } + + ftpAgent, _ := agent.(*FTPTransferAgent) + + // manually read file contents + ftpAgent.conn.ChangeDir(agent.OutboundPath()) + resp, _ := ftpAgent.conn.Retr(f.Filename) + if resp == nil { + t.Fatal("nil File response") + } + r, _ := ftpAgent.readResponse(resp) + if r == nil { + t.Fatal("failed to read file") + } + bs, _ := ioutil.ReadAll(r) + if !bytes.Equal(bs, []byte(content)) { + t.Errorf("got %q", string(bs)) + } + + // delete the file + if err := agent.Delete(f.Filename); err != nil { + t.Fatal(err) + } +} From cbd7aa9c5b67fa9ef747a386bd17dc03889eba59 Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Wed, 17 Jul 2019 15:34:33 -0500 Subject: [PATCH 04/26] internal/filetransfer: add sftp_configs table with tests --- README.md | 2 +- internal/database/mysql.go | 4 +-- internal/database/sqlite.go | 4 +-- internal/filetransfer/config_test.go | 52 ++++++++++++++++++++++++---- 4 files changed, 49 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 10f2c6274..e9b36e094 100644 --- a/README.md +++ b/README.md @@ -99,7 +99,7 @@ Based on `DATABASE_TYPE` the following environment variables will be read to con ##### MySQL -- `MYSQL_ADDRESS`: TCP address for connecting to the mysql server. (example: `localhost:3306`) +- `MYSQL_ADDRESS`: TCP address for connecting to the mysql server. (example: `tcp(hostname:3306)`) - `MYSQL_DATABASE`: Name of database to connect into. - `MYSQL_PASSWORD`: Password of user account for authentication. - `MYSQL_USER`: Username used for authentication, diff --git a/internal/database/mysql.go b/internal/database/mysql.go index c6cc79c21..36b33fc64 100644 --- a/internal/database/mysql.go +++ b/internal/database/mysql.go @@ -96,10 +96,8 @@ func mysqlConnection(logger log.Logger, user, pass string, address string, datab // File Merging and Uploading `create table if not exists cutoff_times(routing_number varchar(10), cutoff varchar(10), location varchar(25));`, `create table if not exists file_transfer_configs(routing_number varchar(10), inbound_path varchar(100), outbound_path varchar(100), return_path varchar(100));`, - - // TODO(adam): We need to rename sftp_configs to ftp_configs (and create a new sftp_configs table for ssh-based file transfer) - // TODO(adam): ftp_configs needs the password encrypted? (or stored in vault) `create table if not exists ftp_configs(routing_number varchar(10), hostname varchar(100), username varchar(25), password varchar(25));`, + `create table if not exists sftp_configs(routing_number varchar(10), hostname varchar(100), username varchar(25), password varchar(25), client_private_key varchar(2100));`, }, } } diff --git a/internal/database/sqlite.go b/internal/database/sqlite.go index 1d92b6add..795961ced 100644 --- a/internal/database/sqlite.go +++ b/internal/database/sqlite.go @@ -112,10 +112,8 @@ func sqliteConnection(logger log.Logger, path string) *sqlite { // File Merging and Uploading `create table if not exists cutoff_times(routing_number, cutoff, location);`, `create table if not exists file_transfer_configs(routing_number, inbound_path, outbound_path, return_path);`, - - // TODO(adam): We need to rename sftp_configs to ftp_configs (and create a new sftp_configs table for ssh-based file transfer) - // TODO(adam): ftp_configs needs the password encrypted? (or stored in vault) `create table if not exists ftp_configs(routing_number, hostname, username, password);`, + `create table if not exists sftp_configs(routing_number, hostname, username, password, client_private_key);`, }, connections: sqliteConnections, } diff --git a/internal/filetransfer/config_test.go b/internal/filetransfer/config_test.go index ce774554d..9899aa8cd 100644 --- a/internal/filetransfer/config_test.go +++ b/internal/filetransfer/config_test.go @@ -27,7 +27,7 @@ func (r *testSqliteRepository) Close() error { return r.testDB.Close() } -func createTestsqliteRepository(t *testing.T) *testSqliteRepository { +func createTestSQLiteRepository(t *testing.T) *testSqliteRepository { t.Helper() db := database.CreateTestSqliteDB(t) @@ -36,12 +36,12 @@ func createTestsqliteRepository(t *testing.T) *testSqliteRepository { } func TestsqliteRepository__getCounts(t *testing.T) { - repo := createTestsqliteRepository(t) + repo := createTestSQLiteRepository(t) defer repo.Close() writeCutoffTime(t, repo) - writeFTPConfig(t, repo) writeFileTransferConfig(t, repo.db) + writeFTPConfig(t, repo) cutoffs, ftps, filexfers := repo.GetCounts() if cutoffs != 1 { @@ -77,7 +77,7 @@ func writeCutoffTime(t *testing.T, repo *testSqliteRepository) { } func TestsqliteRepository__GetCutoffTimes(t *testing.T) { - repo := createTestsqliteRepository(t) + repo := createTestSQLiteRepository(t) defer repo.Close() writeCutoffTime(t, repo) @@ -115,7 +115,7 @@ func writeFTPConfig(t *testing.T, repo *testSqliteRepository) { } func TestsqliteRepository__GetFTPConfigs(t *testing.T) { - repo := createTestsqliteRepository(t) + repo := createTestSQLiteRepository(t) defer repo.Close() writeFTPConfig(t, repo) @@ -157,7 +157,7 @@ func writeFileTransferConfig(t *testing.T, db *sql.DB) { } func TestsqliteRepository__GetConfigs(t *testing.T) { - repo := createTestsqliteRepository(t) + repo := createTestSQLiteRepository(t) defer repo.Close() writeFileTransferConfig(t, repo.db) @@ -272,6 +272,46 @@ func TestFileTransferConfigsHTTP__GetConfigs(t *testing.T) { } } +func writeSFTPConfig(t *testing.T, repo *testSqliteRepository) { + t.Helper() + + query := `insert into sftp_configs (routing_number, hostname, username, password, client_private_key) values ('123456789', 'ftp.moov.io', 'moov', '', '==secret==');` + stmt, err := repo.db.Prepare(query) + if err != nil { + t.Fatal(err) + } + defer stmt.Close() + if _, err := stmt.Exec(); err != nil { + t.Fatal(err) + } +} + +func TestConfigs__GetSFTPConfigs(t *testing.T) { + t.Helper() + + check := func(t *testing.T, repo *testSqliteRepository) { + writeSFTPConfig(t, repo) + + configs, err := repo.GetSFTPConfigs() + if err != nil { + t.Fatal(err) + } + if len(configs) != 1 { + t.Errorf("got %d SFTP configs: %#v", len(configs), configs) + } + } + + // SQLite tests + sqliteDB := database.CreateTestSqliteDB(t) + defer sqliteDB.Close() + check(t, &testSqliteRepository{&sqliteRepository{sqliteDB.DB}, sqliteDB}) + + // MySQL tests + mysqlDB := database.CreateTestMySQLDB(t) + defer mysqlDB.Close() + check(t, &testSqliteRepository{sqliteRepository: &sqliteRepository{mysqlDB.DB}}) +} + // svc.AddHandler("/configs/uploads/cutoff-times/{routingNumber}", upsertCutoffTimeConfig(logger, repo)) // svc.AddHandler("/configs/uploads/cutoff-times/{routingNumber}", deleteCutoffTimeConfig(logger, repo)) From 5095a328551451bf82c161ab0dc82acc1e2ff799 Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Thu, 18 Jul 2019 13:50:13 -0500 Subject: [PATCH 05/26] internal/filetransfer: pass along sftp configs to our SFTPTransferAgent This was missed before, whoops. --- internal/filetransfer/sftp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/filetransfer/sftp.go b/internal/filetransfer/sftp.go index 5638b1753..dc53fdf6a 100644 --- a/internal/filetransfer/sftp.go +++ b/internal/filetransfer/sftp.go @@ -46,7 +46,7 @@ func (a *SFTPTransferAgent) findConfig() *SFTPConfig { } func newSFTPTransferAgent(cfg *Config, sftpConfigs []*SFTPConfig) (*SFTPTransferAgent, error) { - agent := &SFTPTransferAgent{cfg: cfg} + agent := &SFTPTransferAgent{cfg: cfg, sftpConfigs: sftpConfigs} sftpConf := agent.findConfig() if sftpConf == nil { return nil, fmt.Errorf("sftp: unable to find config for %s", cfg.RoutingNumber) From 5675898831d08d9216faf4875cfad0ae43e5d3de Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Thu, 18 Jul 2019 13:51:36 -0500 Subject: [PATCH 06/26] internal/filetransfer: experiment with SFTP's NewClientPipe and a test setup --- internal/filetransfer/sftp.go | 44 +++++++-- internal/filetransfer/sftp_test.go | 148 +++++++++++++++++++++++++++++ 2 files changed, 186 insertions(+), 6 deletions(-) create mode 100644 internal/filetransfer/sftp_test.go diff --git a/internal/filetransfer/sftp.go b/internal/filetransfer/sftp.go index dc53fdf6a..dd54b71d5 100644 --- a/internal/filetransfer/sftp.go +++ b/internal/filetransfer/sftp.go @@ -52,7 +52,8 @@ func newSFTPTransferAgent(cfg *Config, sftpConfigs []*SFTPConfig) (*SFTPTransfer return nil, fmt.Errorf("sftp: unable to find config for %s", cfg.RoutingNumber) } - conn, err := sshConnect(sftpConf) + // conn, _, _, err := sftpConnect(sftpConf) + conn, stdin, stdout, err := sftpConnect(sftpConf) if err != nil { return nil, fmt.Errorf("filetransfer: %v", err) } @@ -69,6 +70,8 @@ func newSFTPTransferAgent(cfg *Config, sftpConfigs []*SFTPConfig) (*SFTPTransfer // sftp.MaxPacket(29999), } client, err := sftp.NewClient(agent.conn, opts...) + // client, err := sftp.NewClient(conn, opts...) + client, err := sftp.NewClientPipe(stdout, stdin, opts...) if err != nil { return nil, fmt.Errorf("filetransfer: sftp connect: %v", err) } @@ -77,7 +80,7 @@ func newSFTPTransferAgent(cfg *Config, sftpConfigs []*SFTPConfig) (*SFTPTransfer return agent, nil } -func sshConnect(sftpConf *SFTPConfig) (*ssh.Client, error) { +func sftpConnect(sftpConf *SFTPConfig) (*ssh.Client, io.WriteCloser, io.Reader, error) { conf := &ssh.ClientConfig{ User: sftpConf.Username, Timeout: 30 * time.Second, @@ -93,19 +96,48 @@ func sshConnect(sftpConf *SFTPConfig) (*ssh.Client, error) { // TODO(adam): attempt base64 decode also signer, err := ssh.ParsePrivateKey([]byte(sftpConf.ClientPrivateKey)) if err != nil { - return nil, fmt.Errorf("sshConnect: failed to read client private key: %v", err) + return nil, nil, nil, fmt.Errorf("sftpConnect: failed to read client private key: %v", err) } conf.Auth = append(conf.Auth, ssh.PublicKeys(signer)) default: - return nil, fmt.Errorf("sshConnect: no auth method provided for routingNumber=%s", sftpConf.RoutingNumber) + return nil, nil, nil, fmt.Errorf("sftpConnect: no auth method provided for routingNumber=%s", sftpConf.RoutingNumber) } // Connect to the remote server client, err := ssh.Dial("tcp", sftpConf.Hostname, conf) if err != nil { - return nil, fmt.Errorf("sshConnect: error with routingNumber=%s: %v", sftpConf.RoutingNumber, err) + return nil, nil, nil, fmt.Errorf("sftpConnect: error with routingNumber=%s: %v", sftpConf.RoutingNumber, err) } - return client, nil + + session, err := client.NewSession() + if err != nil { + go client.Close() + return nil, nil, nil, err + } + if err = session.RequestSubsystem("sftp"); err != nil { + go client.Close() + return nil, nil, nil, err + } + pw, err := session.StdinPipe() + if err != nil { + go client.Close() + return nil, nil, nil, err + } + pr, err := session.StdoutPipe() + if err != nil { + go client.Close() + return nil, nil, nil, err + } + + return client, pw, pr, nil +} + +func (a *SFTPTransferAgent) Ping() error { + _, err := a.client.ReadDir(".") + if err != nil { + return fmt.Errorf("sftp: ping %v", err) + } + return nil } func (a *SFTPTransferAgent) Close() error { diff --git a/internal/filetransfer/sftp_test.go b/internal/filetransfer/sftp_test.go new file mode 100644 index 000000000..59f813386 --- /dev/null +++ b/internal/filetransfer/sftp_test.go @@ -0,0 +1,148 @@ +// Copyright 2019 The Moov Authors +// Use of this source code is governed by an Apache License +// license that can be found in the LICENSE file. + +package filetransfer + +import ( + "fmt" + "io/ioutil" + "strings" + "testing" + + // "github.com/pkg/sftp" + + "github.com/moov-io/base/docker" + + "github.com/ory/dockertest" +) + +type sftpDeployment struct { + res *dockertest.Resource + agent *SFTPTransferAgent +} + +func (s *sftpDeployment) close(t *testing.T) { + if err := s.res.Close(); err != nil { + t.Error(err) + } +} + +// spawnSFTP launches an SFTP Docker image +// +// You can verify this container launches with an ssh command like: +// $ ssh ssh://demo@127.0.0.1:33138 -s sftp +func spawnSFTP(t *testing.T, containerArgs string) *sftpDeployment { + if testing.Short() { + t.Skip("-short flag enabled") + } + if !docker.Enabled() { + t.Skip("Docker not enabled") + } + + // Start our Docker image + pool, err := dockertest.NewPool("") + if err != nil { + t.Fatal(err) + } + resource, err := pool.RunWithOptions(&dockertest.RunOptions{ + Repository: "atmoz/sftp", + Tag: "latest", + Cmd: []string{"demo:password:::upload"}, + }) + if err != nil { + t.Fatal(err) + } + addr := fmt.Sprintf("localhost:%s", resource.GetPort("22/tcp")) + agent, err := newAgent(addr, "demo", "password", "") + if err != nil { + t.Fatal(err) + } + err = pool.Retry(func() error { + return agent.Ping() + }) + if err != nil { + t.Fatal(err) + } + return &sftpDeployment{res: resource, agent: agent} +} + +func newAgent(host, user, pass, passFile string) (*SFTPTransferAgent, error) { + cfg := &Config{ + RoutingNumber: "121042882", // arbitrary routing number + InboundPath: "upload/inbound/", + OutboundPath: "upload/outbound/", + ReturnPath: "upload/returns/", + } + sftpConfigs := []*SFTPConfig{ + { + RoutingNumber: "121042882", + Hostname: host, + Username: user, + }, + } + if pass != "" { + sftpConfigs[0].Password = pass + } else { + sftpConfigs[0].ClientPrivateKey = passFile + } + return newSFTPTransferAgent(cfg, sftpConfigs) +} + +func TestSFTP(t *testing.T) { + if testing.Short() { + return + } + + // This test server is available for lots of various protocols + // and services. We can only list files as the server is read-only. + // See: https://test.rebex.net/ + agent, err := newAgent("test.rebex.net:22", "demo", "password", "") + if err != nil { + t.Fatal(err) + } + fds, err := agent.client.ReadDir(".") + if err != nil { + t.Fatal(err) + } + for i := range fds { + t.Logf(" #%d %s", i, fds[i].Name()) + } + if len(fds) == 0 { + t.Errorf("expected to find files, we've found them before") + } +} + +// docker run -p 22:22 -d atmoz/sftp foo:pass:::upload + +func TestSFTP__password(t *testing.T) { + t.Skip("can't connect to the Docker image for some reason...") + + deployment := spawnSFTP(t, "foo:pass:::upload") + defer deployment.close(t) + + if err := deployment.agent.Ping(); err != nil { + t.Fatal(err) + } + + err := deployment.agent.UploadFile(File{ + Filename: "upload.ach", + Contents: ioutil.NopCloser(strings.NewReader("test data")), + }) + if err != nil { + t.Fatal(err) + } +} + +// Generate keys (in Go) and mount them into our test container +// +// docker run \ +// -v /host/id_rsa.pub:/home/foo/.ssh/keys/id_rsa.pub:ro \ +// -v /host/id_other.pub:/home/foo/.ssh/keys/id_other.pub:ro \ +// -v /host/share:/home/foo/share \ +// -p 2222:22 -d atmoz/sftp \ +// foo::1001 + +func TestSFTP__ClientPrivateKey(t *testing.T) { + +} From d0e75516739ee508ba4445de3629df1849bb5791 Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Thu, 18 Jul 2019 13:52:05 -0500 Subject: [PATCH 07/26] internal/filetransfer: close ssh.Client if we fail our SFTP connection --- internal/filetransfer/sftp.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/filetransfer/sftp.go b/internal/filetransfer/sftp.go index dd54b71d5..1c6e695f8 100644 --- a/internal/filetransfer/sftp.go +++ b/internal/filetransfer/sftp.go @@ -73,6 +73,7 @@ func newSFTPTransferAgent(cfg *Config, sftpConfigs []*SFTPConfig) (*SFTPTransfer // client, err := sftp.NewClient(conn, opts...) client, err := sftp.NewClientPipe(stdout, stdin, opts...) if err != nil { + go conn.Close() return nil, fmt.Errorf("filetransfer: sftp connect: %v", err) } agent.client = client From 7600dd2041e6d2cf76d3868de30dfc77216d61a7 Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Thu, 18 Jul 2019 13:52:24 -0500 Subject: [PATCH 08/26] internal/filetransfer: override defaults for our SFTP client --- internal/filetransfer/sftp.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/internal/filetransfer/sftp.go b/internal/filetransfer/sftp.go index 1c6e695f8..18faae24e 100644 --- a/internal/filetransfer/sftp.go +++ b/internal/filetransfer/sftp.go @@ -61,15 +61,14 @@ func newSFTPTransferAgent(cfg *Config, sftpConfigs []*SFTPConfig) (*SFTPTransfer // Setup our SFTP client var opts = []sftp.ClientOption{ - // TODO(adam): Thoughts on these defaults? - // // Q(adam): Would we ever have multiple requests to the same file? - // // See: https://godoc.org/github.com/pkg/sftp#MaxConcurrentRequestsPerFile - // sftp.MaxConcurrentRequestsPerFile(64), - // // The docs suggest lowering this on "failed to send packet header: EOF" errors, - // // so we're going to lower it by default (which is 32768). - // sftp.MaxPacket(29999), - } - client, err := sftp.NewClient(agent.conn, opts...) + // Q(adam): Would we ever have multiple requests to the same file? + // See: https://godoc.org/github.com/pkg/sftp#MaxConcurrentRequestsPerFile + sftp.MaxConcurrentRequestsPerFile(8), + + // The docs suggest lowering this on "failed to send packet header: EOF" errors, + // so we're going to lower it by default (which is 32768). + sftp.MaxPacket(29999), + } // client, err := sftp.NewClient(conn, opts...) client, err := sftp.NewClientPipe(stdout, stdin, opts...) if err != nil { From 7437b6e75bea301f4f53ee21ac9a340c53c34844 Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Thu, 18 Jul 2019 13:52:38 -0500 Subject: [PATCH 09/26] internal/filetransfer: misc improvements to docs --- README.md | 2 ++ internal/filetransfer/file_transfer.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index e9b36e094..7635ee0a3 100644 --- a/README.md +++ b/README.md @@ -130,6 +130,8 @@ Twitter [@moov_io](https://twitter.com/moov_io) | You can follow Moov.IO's Twitt Yes please! Please review our [Contributing guide](CONTRIBUTING.md) and [Code of Conduct](https://github.com/moov-io/ach/blob/master/CODE_OF_CONDUCT.md) to get started! +Paygate includes several "long" tests which spawn Docker containers, make requests over the internet, and perform more complicated tests. To skip these long tests add the `-short` flag to `go test`. + Note: This project uses Go Modules, which requires Go 1.11 or higher, but we ship the vendor directory in our repository. ### Test Coverage diff --git a/internal/filetransfer/file_transfer.go b/internal/filetransfer/file_transfer.go index c5a655c90..889526be6 100644 --- a/internal/filetransfer/file_transfer.go +++ b/internal/filetransfer/file_transfer.go @@ -45,7 +45,7 @@ type Agent interface { // New returns an implementation of a Agent which is used to upload files to a remote server. // -// This function reads ACH_FILE_TRANSFERS_ROOT_CAFILE for a file with root certificates to be used in all secured connections. +// This function reads ACH_FILE_TRANSFERS_ROOT_CAFILE for a file with additional root certificates to be used in all secured connections. func New(_type string, cfg *Config, repo Repository) (Agent, error) { switch strings.ToLower(_type) { case "ftp": From 15002cc430e874c6f1b06b9d838585c18fff178b Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Fri, 19 Jul 2019 10:15:11 -0500 Subject: [PATCH 10/26] internal/filetransfer: close SFTP agent in deployment.close() --- internal/filetransfer/sftp_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/filetransfer/sftp_test.go b/internal/filetransfer/sftp_test.go index 59f813386..7373fc8ed 100644 --- a/internal/filetransfer/sftp_test.go +++ b/internal/filetransfer/sftp_test.go @@ -23,6 +23,9 @@ type sftpDeployment struct { } func (s *sftpDeployment) close(t *testing.T) { + if err := s.agent.Close(); err != nil { + t.Error(err) + } if err := s.res.Close(); err != nil { t.Error(err) } From b1aa38e68f19247e7be3ba88cc4ab839716e445a Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Fri, 19 Jul 2019 10:15:38 -0500 Subject: [PATCH 11/26] internal/filetransfer: close agent and check ping in remote SFTP test --- internal/filetransfer/sftp_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/filetransfer/sftp_test.go b/internal/filetransfer/sftp_test.go index 7373fc8ed..72b22cfbc 100644 --- a/internal/filetransfer/sftp_test.go +++ b/internal/filetransfer/sftp_test.go @@ -104,6 +104,8 @@ func TestSFTP(t *testing.T) { if err != nil { t.Fatal(err) } + defer agent.Close() + fds, err := agent.client.ReadDir(".") if err != nil { t.Fatal(err) @@ -114,6 +116,10 @@ func TestSFTP(t *testing.T) { if len(fds) == 0 { t.Errorf("expected to find files, we've found them before") } + + if err := agent.Ping(); err != nil { + t.Fatal(err) + } } // docker run -p 22:22 -d atmoz/sftp foo:pass:::upload From c4aebbdca5e3f75ef85e309d078db847fb8a0202 Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Fri, 19 Jul 2019 12:12:39 -0500 Subject: [PATCH 12/26] internal/filetransfer: retry on SFTP client create Our tests are a bit slow to launch, but we should allow a (crude) retry here anyway. --- internal/filetransfer/sftp.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/internal/filetransfer/sftp.go b/internal/filetransfer/sftp.go index 18faae24e..525679702 100644 --- a/internal/filetransfer/sftp.go +++ b/internal/filetransfer/sftp.go @@ -104,8 +104,15 @@ func sftpConnect(sftpConf *SFTPConfig) (*ssh.Client, io.WriteCloser, io.Reader, } // Connect to the remote server - client, err := ssh.Dial("tcp", sftpConf.Hostname, conf) - if err != nil { + var client *ssh.Client + var err error + for i := 0; i < 3; i++ { + if client == nil { + client, err = ssh.Dial("tcp", sftpConf.Hostname, conf) // retry connection + time.Sleep(250 * time.Millisecond) + } + } + if client == nil && err != nil { return nil, nil, nil, fmt.Errorf("sftpConnect: error with routingNumber=%s: %v", sftpConf.RoutingNumber, err) } From 6b728e01312e0347ecf34992fc3c0d29df07da4e Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Fri, 19 Jul 2019 12:13:13 -0500 Subject: [PATCH 13/26] internal/filetransfer: lower SFTP MaxPacket This was a suggested example in some EOF related issues, however setting it this low might be too strict for a default. --- internal/filetransfer/sftp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/filetransfer/sftp.go b/internal/filetransfer/sftp.go index 525679702..7772bd4a5 100644 --- a/internal/filetransfer/sftp.go +++ b/internal/filetransfer/sftp.go @@ -67,7 +67,7 @@ func newSFTPTransferAgent(cfg *Config, sftpConfigs []*SFTPConfig) (*SFTPTransfer // The docs suggest lowering this on "failed to send packet header: EOF" errors, // so we're going to lower it by default (which is 32768). - sftp.MaxPacket(29999), + sftp.MaxPacket(20480), } // client, err := sftp.NewClient(conn, opts...) client, err := sftp.NewClientPipe(stdout, stdin, opts...) From 1a9798f8c14dc9066a33e6f763e7f406a602c770 Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Fri, 19 Jul 2019 12:14:35 -0500 Subject: [PATCH 14/26] internal/filetransfer: get our test working with a Docker image --- internal/filetransfer/sftp_test.go | 80 ++++++++++++++++++++++++------ 1 file changed, 66 insertions(+), 14 deletions(-) diff --git a/internal/filetransfer/sftp_test.go b/internal/filetransfer/sftp_test.go index 72b22cfbc..b3b1a6c4e 100644 --- a/internal/filetransfer/sftp_test.go +++ b/internal/filetransfer/sftp_test.go @@ -7,10 +7,12 @@ package filetransfer import ( "fmt" "io/ioutil" + "os" + "runtime" "strings" + "syscall" "testing" - - // "github.com/pkg/sftp" + "time" "github.com/moov-io/base/docker" @@ -20,9 +22,18 @@ import ( type sftpDeployment struct { res *dockertest.Resource agent *SFTPTransferAgent + + dir string // temporary directory } func (s *sftpDeployment) close(t *testing.T) { + defer func() { + // Always try and cleanup our scratch dir + if err := os.RemoveAll(s.dir); err != nil { + t.Error(err) + } + }() + if err := s.agent.Close(); err != nil { t.Error(err) } @@ -35,13 +46,22 @@ func (s *sftpDeployment) close(t *testing.T) { // // You can verify this container launches with an ssh command like: // $ ssh ssh://demo@127.0.0.1:33138 -s sftp -func spawnSFTP(t *testing.T, containerArgs string) *sftpDeployment { +func spawnSFTP(t *testing.T) *sftpDeployment { if testing.Short() { t.Skip("-short flag enabled") } if !docker.Enabled() { t.Skip("Docker not enabled") } + switch runtime.GOOS { + case "darwin", "linux": + // continue on with our test + default: + t.Skipf("we haven't coded test support for uid/gid extraction on %s", runtime.GOOS) + } + + // Setup a temp directory for our SFTP instance + dir, uid, gid := mkdir(t) // Start our Docker image pool, err := dockertest.NewPool("") @@ -51,14 +71,27 @@ func spawnSFTP(t *testing.T, containerArgs string) *sftpDeployment { resource, err := pool.RunWithOptions(&dockertest.RunOptions{ Repository: "atmoz/sftp", Tag: "latest", - Cmd: []string{"demo:password:::upload"}, + // set user and group to grant write permissions + Cmd: []string{ + fmt.Sprintf("demo:password:%d:%d:upload", uid, gid), + }, + Mounts: []string{ + fmt.Sprintf("%s:/home/demo/upload", dir), + }, }) if err != nil { t.Fatal(err) } addr := fmt.Sprintf("localhost:%s", resource.GetPort("22/tcp")) - agent, err := newAgent(addr, "demo", "password", "") - if err != nil { + + var agent *SFTPTransferAgent + for i := 0; i < 10; i++ { + if agent == nil { + agent, err = newAgent(addr, "demo", "password", "") + time.Sleep(250 * time.Millisecond) + } + } + if agent == nil && err != nil { t.Fatal(err) } err = pool.Retry(func() error { @@ -67,15 +100,36 @@ func spawnSFTP(t *testing.T, containerArgs string) *sftpDeployment { if err != nil { t.Fatal(err) } - return &sftpDeployment{res: resource, agent: agent} + return &sftpDeployment{res: resource, agent: agent, dir: dir} +} + +func mkdir(t *testing.T) (string, uint32, uint32) { + wd, _ := os.Getwd() + dir, err := ioutil.TempDir(wd, "sftp") + if err != nil { + t.Fatal(err) + } + fd, err := os.Stat(dir) + if err != nil { + t.Fatal(err) + } + stat, ok := fd.Sys().(*syscall.Stat_t) + if !ok { + t.Fatalf("unable to stat %s", fd.Name()) + } + return dir, stat.Uid, stat.Gid } func newAgent(host, user, pass, passFile string) (*SFTPTransferAgent, error) { cfg := &Config{ RoutingNumber: "121042882", // arbitrary routing number - InboundPath: "upload/inbound/", - OutboundPath: "upload/outbound/", - ReturnPath: "upload/returns/", + // Our SFTP client inits into '/' with one folder, 'upload', so we need to + // put files into /upload/ (as an absolute path). + // + // Currently it's assumed sub-directories would exist for inbound vs outbound files. + InboundPath: "/upload/", + OutboundPath: "/upload/", + ReturnPath: "/upload/", } sftpConfigs := []*SFTPConfig{ { @@ -125,9 +179,7 @@ func TestSFTP(t *testing.T) { // docker run -p 22:22 -d atmoz/sftp foo:pass:::upload func TestSFTP__password(t *testing.T) { - t.Skip("can't connect to the Docker image for some reason...") - - deployment := spawnSFTP(t, "foo:pass:::upload") + deployment := spawnSFTP(t) defer deployment.close(t) if err := deployment.agent.Ping(); err != nil { @@ -135,7 +187,7 @@ func TestSFTP__password(t *testing.T) { } err := deployment.agent.UploadFile(File{ - Filename: "upload.ach", + Filename: deployment.agent.OutboundPath() + "upload.ach", Contents: ioutil.NopCloser(strings.NewReader("test data")), }) if err != nil { From ca4154afdbcd0b30798e92617a33194e9641d73e Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Fri, 19 Jul 2019 13:32:39 -0500 Subject: [PATCH 15/26] internal/filetransfer: include a test SFTP config for atmoz/sftp:latest --- internal/filetransfer/config.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/filetransfer/config.go b/internal/filetransfer/config.go index 1e4296aea..9a7cf3e9d 100644 --- a/internal/filetransfer/config.go +++ b/internal/filetransfer/config.go @@ -211,7 +211,7 @@ func (r *localFileTransferRepository) GetCutoffTimes() ([]*CutoffTime, error) { func (r *localFileTransferRepository) GetFTPConfigs() ([]*FTPConfig, error) { return []*FTPConfig{ { - RoutingNumber: "121042882", // from 'go run ./cmd/server' in Accounts + RoutingNumber: "121042882", Hostname: "localhost:2121", // below configs for moov/fftp:v0.1.0 Username: "admin", Password: "123456", @@ -221,13 +221,13 @@ func (r *localFileTransferRepository) GetFTPConfigs() ([]*FTPConfig, error) { func (r *localFileTransferRepository) GetSFTPConfigs() ([]*SFTPConfig, error) { return []*SFTPConfig{ - // { - // RoutingNumber: "121042882", // from 'go run ./cmd/server' in Accounts - // Hostname: "localhost:2121", // below configs for moov/fftp:v0.1.0 - // Username: "admin", - // // Password: "123456", - // // ClientPrivateKey: "...", // Base64 encoded or PEM format - // }, + { + RoutingNumber: "121042882", + Hostname: "localhost:22", // below configs for atmoz/sftp:latest + Username: "demo", + Password: "password", + // ClientPrivateKey: "...", // Base64 encoded or PEM format + }, }, nil } From 4072b1adb567f51bf60ef42a213b8ef4da1804f8 Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Fri, 19 Jul 2019 13:33:33 -0500 Subject: [PATCH 16/26] internal/filetransfer: bump up test coverage --- internal/filetransfer/config_test.go | 25 ++++---- internal/filetransfer/file_transfer_test.go | 34 ----------- internal/filetransfer/ftp_test.go | 68 ++++++++++++++++++--- 3 files changed, 72 insertions(+), 55 deletions(-) diff --git a/internal/filetransfer/config_test.go b/internal/filetransfer/config_test.go index 9899aa8cd..00bf33cd2 100644 --- a/internal/filetransfer/config_test.go +++ b/internal/filetransfer/config_test.go @@ -35,23 +35,22 @@ func createTestSQLiteRepository(t *testing.T) *testSqliteRepository { return &testSqliteRepository{repo, db} } -func TestsqliteRepository__getCounts(t *testing.T) { +func TestSQLiteRepository__getCounts(t *testing.T) { repo := createTestSQLiteRepository(t) defer repo.Close() + cutoffs, ftps, filexfers := repo.GetCounts() + if cutoffs != 0 || ftps != 0 || filexfers != 0 { + t.Errorf("cutoffs=%d ftps=%d filexfers=%d", cutoffs, ftps, filexfers) + } + writeCutoffTime(t, repo) writeFileTransferConfig(t, repo.db) writeFTPConfig(t, repo) - cutoffs, ftps, filexfers := repo.GetCounts() - if cutoffs != 1 { - t.Errorf("got %d", cutoffs) - } - if ftps != 1 { - t.Errorf("got %d", ftps) - } - if filexfers != 1 { - t.Errorf("got %d", filexfers) + cutoffs, ftps, filexfers = repo.GetCounts() + if cutoffs != 1 || ftps != 1 || filexfers != 1 { + t.Errorf("cutoffs=%d ftps=%d filexfers=%d", cutoffs, ftps, filexfers) } // If we read at least one row from each config table we need to make sure NewRepository @@ -76,7 +75,7 @@ func writeCutoffTime(t *testing.T, repo *testSqliteRepository) { } } -func TestsqliteRepository__GetCutoffTimes(t *testing.T) { +func TestSQLiteRepository__GetCutoffTimes(t *testing.T) { repo := createTestSQLiteRepository(t) defer repo.Close() @@ -114,7 +113,7 @@ func writeFTPConfig(t *testing.T, repo *testSqliteRepository) { } } -func TestsqliteRepository__GetFTPConfigs(t *testing.T) { +func TestSQLiteRepository__GetFTPConfigs(t *testing.T) { repo := createTestSQLiteRepository(t) defer repo.Close() @@ -156,7 +155,7 @@ func writeFileTransferConfig(t *testing.T, db *sql.DB) { } } -func TestsqliteRepository__GetConfigs(t *testing.T) { +func TestSQLiteRepository__GetConfigs(t *testing.T) { repo := createTestSQLiteRepository(t) defer repo.Close() diff --git a/internal/filetransfer/file_transfer_test.go b/internal/filetransfer/file_transfer_test.go index b57cd7971..7be545071 100644 --- a/internal/filetransfer/file_transfer_test.go +++ b/internal/filetransfer/file_transfer_test.go @@ -7,12 +7,9 @@ package filetransfer import ( "bytes" "encoding/json" - "fmt" "strings" "testing" "time" - - "github.com/goftp/server" ) func TestCutoffTime(t *testing.T) { @@ -65,34 +62,3 @@ func TestCutoffTime__JSON(t *testing.T) { t.Error(buf.String()) } } - -func createTestFileTransferAgent(t *testing.T) (*server.Server, Agent) { - svc, err := createTestFTPServer(t) - if err != nil { - return nil, nil - } - - auth, ok := svc.Auth.(*server.SimpleAuth) - if !ok { - t.Errorf("unknown svc.Auth: %T", svc.Auth) - } - conf := &Config{ // these need to match paths at testdata/ftp-srever/ - InboundPath: "inbound", - OutboundPath: "outbound", - ReturnPath: "returned", - } - ftpConfigs := []*FTPConfig{ - { - Hostname: fmt.Sprintf("%s:%d", svc.Hostname, svc.Port), - Username: auth.Name, - Password: auth.Password, - }, - } - agent, err := newFTPTransferAgent(conf, ftpConfigs) - if err != nil { - svc.Shutdown() - t.Fatalf("problem creating FileTransferAgent: %v", err) - return nil, nil - } - return svc, agent -} diff --git a/internal/filetransfer/ftp_test.go b/internal/filetransfer/ftp_test.go index 18946c166..2fac252d0 100644 --- a/internal/filetransfer/ftp_test.go +++ b/internal/filetransfer/ftp_test.go @@ -152,6 +152,54 @@ func TestFTP(t *testing.T) { } } +func createTestFTPAgent(t *testing.T) (*server.Server, *FTPTransferAgent) { + svc, err := createTestFTPServer(t) + if err != nil { + return nil, nil + } + + auth, ok := svc.Auth.(*server.SimpleAuth) + if !ok { + t.Errorf("unknown svc.Auth: %T", svc.Auth) + } + conf := &Config{ // these need to match paths at testdata/ftp-srever/ + InboundPath: "inbound", + OutboundPath: "outbound", + ReturnPath: "returned", + } + ftpConfigs := []*FTPConfig{ + { + Hostname: fmt.Sprintf("%s:%d", svc.Hostname, svc.Port), + Username: auth.Name, + Password: auth.Password, + }, + } + agent, err := newFTPTransferAgent(conf, ftpConfigs) + if err != nil { + svc.Shutdown() + t.Fatalf("problem creating FileTransferAgent: %v", err) + return nil, nil + } + return svc, agent +} + +func TestFTPAgent(t *testing.T) { + svc, agent := createTestFTPAgent(t) + defer agent.Close() + defer svc.Shutdown() + + // Verify directories aren setup as expected + if v := agent.InboundPath(); v != "inbound" { + t.Errorf("got %s", v) + } + if v := agent.OutboundPath(); v != "outbound" { + t.Errorf("got %s", v) + } + if v := agent.ReturnPath(); v != "returned" { + t.Errorf("got %s", v) + } +} + func TestFTP__tlsDialOption(t *testing.T) { if testing.Short() { return // skip network calls @@ -173,7 +221,7 @@ func TestFTP__tlsDialOption(t *testing.T) { } func TestFTP__getInboundFiles(t *testing.T) { - svc, agent := createTestFileTransferAgent(t) + svc, agent := createTestFTPAgent(t) defer agent.Close() defer svc.Shutdown() @@ -207,7 +255,7 @@ func TestFTP__getInboundFiles(t *testing.T) { } func TestFTP__getReturnFiles(t *testing.T) { - svc, agent := createTestFileTransferAgent(t) + svc, agent := createTestFTPAgent(t) defer agent.Close() defer svc.Shutdown() @@ -241,7 +289,7 @@ func TestFTP__getReturnFiles(t *testing.T) { } func TestFTP__uploadFile(t *testing.T) { - svc, agent := createTestFileTransferAgent(t) + svc, agent := createTestFTPAgent(t) defer agent.Close() defer svc.Shutdown() @@ -259,15 +307,13 @@ func TestFTP__uploadFile(t *testing.T) { t.Fatal(err) } - ftpAgent, _ := agent.(*FTPTransferAgent) - // manually read file contents - ftpAgent.conn.ChangeDir(agent.OutboundPath()) - resp, _ := ftpAgent.conn.Retr(f.Filename) + agent.conn.ChangeDir(agent.OutboundPath()) + resp, _ := agent.conn.Retr(f.Filename) if resp == nil { t.Fatal("nil File response") } - r, _ := ftpAgent.readResponse(resp) + r, _ := agent.readResponse(resp) if r == nil { t.Fatal("failed to read file") } @@ -280,4 +326,10 @@ func TestFTP__uploadFile(t *testing.T) { if err := agent.Delete(f.Filename); err != nil { t.Fatal(err) } + + // get an error with no FTP configs + agent.ftpConfigs = nil + if err := agent.UploadFile(f); err == nil { + t.Error("expected error") + } } From 70a263cf6c0bb4d88dda75600954180000235317 Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Fri, 19 Jul 2019 13:33:43 -0500 Subject: [PATCH 17/26] internal/filetransfer: add stub HTTP endpoints for SFTP configs --- internal/filetransfer/config.go | 56 ++++++++++++++++++++-------- internal/filetransfer/config_test.go | 17 +++++++-- 2 files changed, 55 insertions(+), 18 deletions(-) diff --git a/internal/filetransfer/config.go b/internal/filetransfer/config.go index 9a7cf3e9d..431ea519e 100644 --- a/internal/filetransfer/config.go +++ b/internal/filetransfer/config.go @@ -243,6 +243,8 @@ func AddFileTransferConfigRoutes(logger log.Logger, svc *admin.Server, repo Repo svc.AddHandler("/configs/uploads/ftp/{routingNumber}", upsertFTPConfig(logger, repo)) svc.AddHandler("/configs/uploads/ftp/{routingNumber}", deleteFTPConfig(logger, repo)) + // svc.AddHandler("/configs/uploads/sftp/{routingNumber}", upsertSFTPConfig(logger, repo)) // TODO(adam): impl + // svc.AddHandler("/configs/uploads/sftp/{routingNumber}", deleteSFTPConfig(logger, repo)) } func getRoutingNumber(r *http.Request) string { @@ -255,8 +257,9 @@ func getRoutingNumber(r *http.Request) string { type adminConfigResponse struct { CutoffTimes []*CutoffTime `json:"CutoffTimes"` - FTPConfigs []*FTPConfig `json:"FTPConfigs"` FileTransferConfigs []*Config `json:"Configs"` + FTPConfigs []*FTPConfig `json:"FTPConfigs"` + SFTPConfigs []*SFTPConfig `json:"SFTPConfigs"` } // GetConfigs returns all configurations (i.e. FTP, cutoff times, file-transfer configs with passwords masked. (e.g. 'p******d') @@ -274,17 +277,23 @@ func GetConfigs(logger log.Logger, repo Repository) http.HandlerFunc { } else { resp.CutoffTimes = v } + if v, err := repo.GetConfigs(); err != nil { + moovhttp.Problem(w, err) + return + } else { + resp.FileTransferConfigs = v + } if v, err := repo.GetFTPConfigs(); err != nil { moovhttp.Problem(w, err) return } else { - resp.FTPConfigs = maskPasswords(v) + resp.FTPConfigs = maskFTPPasswords(v) } - if v, err := repo.GetConfigs(); err != nil { + if v, err := repo.GetSFTPConfigs(); err != nil { moovhttp.Problem(w, err) return } else { - resp.FileTransferConfigs = v + resp.SFTPConfigs = maskSFTPPasswords(v) } w.Header().Set("Content-Type", "application/json; charset=utf-8") @@ -303,7 +312,14 @@ func maskPassword(s string) string { } } -func maskPasswords(cfgs []*FTPConfig) []*FTPConfig { +func maskFTPPasswords(cfgs []*FTPConfig) []*FTPConfig { + for i := range cfgs { + cfgs[i].Password = maskPassword(cfgs[i].Password) + } + return cfgs +} + +func maskSFTPPasswords(cfgs []*SFTPConfig) []*SFTPConfig { for i := range cfgs { cfgs[i].Password = maskPassword(cfgs[i].Password) } @@ -316,9 +332,7 @@ func upsertCutoffTimeConfig(logger log.Logger, repo Repository) http.HandlerFunc moovhttp.Problem(w, fmt.Errorf("unsupported HTTP verb %s", r.Method)) return } - logger.Log("file-transfer-configs", "", "requestId", moovhttp.GetRequestId(r)) - w.WriteHeader(http.StatusOK) } } @@ -329,9 +343,7 @@ func deleteCutoffTimeConfig(logger log.Logger, repo Repository) http.HandlerFunc moovhttp.Problem(w, fmt.Errorf("unsupported HTTP verb %s", r.Method)) return } - logger.Log("file-transfer-configs", "", "requestId", moovhttp.GetRequestId(r)) - w.WriteHeader(http.StatusOK) } } @@ -342,9 +354,7 @@ func upsertFileTransferConfig(logger log.Logger, repo Repository) http.HandlerFu moovhttp.Problem(w, fmt.Errorf("unsupported HTTP verb %s", r.Method)) return } - logger.Log("file-transfer-configs", "", "requestId", moovhttp.GetRequestId(r)) - w.WriteHeader(http.StatusOK) } } @@ -355,9 +365,7 @@ func deleteFileTransferConfig(logger log.Logger, repo Repository) http.HandlerFu moovhttp.Problem(w, fmt.Errorf("unsupported HTTP verb %s", r.Method)) return } - logger.Log("file-transfer-configs", "", "requestId", moovhttp.GetRequestId(r)) - w.WriteHeader(http.StatusOK) } } @@ -368,9 +376,7 @@ func upsertFTPConfig(logger log.Logger, repo Repository) http.HandlerFunc { moovhttp.Problem(w, fmt.Errorf("unsupported HTTP verb %s", r.Method)) return } - logger.Log("file-transfer-configs", "", "requestId", moovhttp.GetRequestId(r)) - w.WriteHeader(http.StatusOK) } } @@ -381,9 +387,29 @@ func deleteFTPConfig(logger log.Logger, repo Repository) http.HandlerFunc { moovhttp.Problem(w, fmt.Errorf("unsupported HTTP verb %s", r.Method)) return } + logger.Log("file-transfer-configs", "", "requestId", moovhttp.GetRequestId(r)) + w.WriteHeader(http.StatusOK) + } +} +func upsertSFTPConfig(logger log.Logger, repo Repository) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + if r.Method != "PUT" { + moovhttp.Problem(w, fmt.Errorf("unsupported HTTP verb %s", r.Method)) + return + } logger.Log("file-transfer-configs", "", "requestId", moovhttp.GetRequestId(r)) + w.WriteHeader(http.StatusOK) + } +} +func deleteSFTPConfig(logger log.Logger, repo Repository) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + if r.Method != "DELETE" { + moovhttp.Problem(w, fmt.Errorf("unsupported HTTP verb %s", r.Method)) + return + } + logger.Log("file-transfer-configs", "", "requestId", moovhttp.GetRequestId(r)) w.WriteHeader(http.StatusOK) } } diff --git a/internal/filetransfer/config_test.go b/internal/filetransfer/config_test.go index 00bf33cd2..fc85f729d 100644 --- a/internal/filetransfer/config_test.go +++ b/internal/filetransfer/config_test.go @@ -229,13 +229,21 @@ func TestFileTransferConfigs__maskPassword(t *testing.T) { t.Errorf("got %q", v) } - out := maskPasswords([]*FTPConfig{{Password: "password"}}) + out := maskFTPPasswords([]*FTPConfig{{Password: "password"}}) if len(out) != 1 { t.Errorf("got %d ftpConfigs: %v", len(out), out) } if out[0].Password != "p******d" { t.Errorf("got %q", out[0].Password) } + + out2 := maskSFTPPasswords([]*SFTPConfig{{Password: "drowssap"}}) + if len(out2) != 1 { + t.Errorf("got %d sftpConfigs: %v", len(out2), out2) + } + if out2[0].Password != "d******p" { + t.Errorf("got %q", out2[0].Password) + } } func TestFileTransferConfigsHTTP__GetConfigs(t *testing.T) { @@ -266,8 +274,11 @@ func TestFileTransferConfigsHTTP__GetConfigs(t *testing.T) { if err := json.NewDecoder(resp.Body).Decode(&response); err != nil { t.Fatal(err) } - if len(response.CutoffTimes) == 0 || len(response.FTPConfigs) == 0 || len(response.FileTransferConfigs) == 0 { - t.Errorf("response.CutoffTimes=%d response.FTPConfigs=%d response.FileTransferConfigs=%d", len(response.CutoffTimes), len(response.FTPConfigs), len(response.FileTransferConfigs)) + if len(response.CutoffTimes) == 0 || len(response.FileTransferConfigs) == 0 { + t.Errorf("response.CutoffTimes=%d response.FileTransferConfigs=%d", len(response.CutoffTimes), len(response.FileTransferConfigs)) + } + if len(response.FTPConfigs) == 0 || len(response.SFTPConfigs) == 0 { + t.Errorf("response.FTPConfigs=%d response.SFTPConfigs=%d", len(response.FTPConfigs), len(response.SFTPConfigs)) } } From d95f810e027053362c8c244a60a0adc6a05b7349 Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Mon, 22 Jul 2019 13:55:09 -0500 Subject: [PATCH 18/26] files: minify fileTransferController.getDetails type surface area --- file_transfer_async.go | 28 +++++++++++----------------- file_transfer_async_test.go | 15 ++++++--------- 2 files changed, 17 insertions(+), 26 deletions(-) diff --git a/file_transfer_async.go b/file_transfer_async.go index 2fd999713..23ad21b64 100644 --- a/file_transfer_async.go +++ b/file_transfer_async.go @@ -160,19 +160,13 @@ func newFileTransferController(logger log.Logger, dir string, repo filetransfer. return controller, nil } -func (c *fileTransferController) getDetails(cutoff *filetransfer.CutoffTime) (*filetransfer.FTPConfig, *filetransfer.Config) { - var ftp *filetransfer.FTPConfig - for i := range c.ftpConfigs { - if cutoff.RoutingNumber == c.ftpConfigs[i].RoutingNumber { - ftp = c.ftpConfigs[i] - } - } +func (c *fileTransferController) findFileTransferConfig(cutoff *filetransfer.CutoffTime) *filetransfer.Config { for i := range c.fileTransferConfigs { if cutoff.RoutingNumber == c.fileTransferConfigs[i].RoutingNumber { - return ftp, c.fileTransferConfigs[i] + return c.fileTransferConfigs[i] } } - return nil, nil + return nil } // startPeriodicFileOperations will block forever to periodically download incoming and returned ACH files while also merging @@ -238,13 +232,13 @@ func (c *fileTransferController) downloadAndProcessIncomingFiles(depRepo deposit defer os.RemoveAll(dir) for i := range c.cutoffTimes { - ftpConf, fileTransferConf := c.getDetails(c.cutoffTimes[i]) - if ftpConf == nil || fileTransferConf == nil { - c.logger.Log("downloadAndProcessIncomingFiles", fmt.Sprintf("missing ftp and/or file transfer config for %s", c.cutoffTimes[i].RoutingNumber)) + fileTransferConf := c.findFileTransferConfig(c.cutoffTimes[i]) + if fileTransferConf == nil { + c.logger.Log("downloadAndProcessIncomingFiles", fmt.Sprintf("missing file transfer config for %s", c.cutoffTimes[i].RoutingNumber)) missingFileUploadConfigs.With("routing_number", c.cutoffTimes[i].RoutingNumber).Add(1) continue } - if err := c.downloadAllFiles(dir, ftpConf, fileTransferConf); err != nil { + if err := c.downloadAllFiles(dir, fileTransferConf); err != nil { c.logger.Log("downloadAndProcessIncomingFiles", fmt.Sprintf("error downloading files into %s", dir), "error", err) continue } @@ -264,16 +258,16 @@ func (c *fileTransferController) downloadAndProcessIncomingFiles(depRepo deposit } // downloadAllFiles will setup directories for each routing number and initiate downloading and writing the files to sub-directories. -func (c *fileTransferController) downloadAllFiles(dir string, ftpConf *filetransfer.FTPConfig, fileTransferConf *filetransfer.Config) error { +func (c *fileTransferController) downloadAllFiles(dir string, fileTransferConf *filetransfer.Config) error { agent, err := filetransfer.New("", fileTransferConf, c.repo) // TODO(adam): pass through _type if err != nil { - return fmt.Errorf("downloadAllFiles: problem with %s file transfer agent init: %v", ftpConf.RoutingNumber, err) + return fmt.Errorf("downloadAllFiles: problem with %s file transfer agent init: %v", fileTransferConf.RoutingNumber, err) } defer agent.Close() // Setup file downloads if err := c.saveRemoteFiles(agent, dir); err != nil { - c.logger.Log("downloadAllFiles", fmt.Sprintf("ERROR downloading files (ABA: %s)", ftpConf.RoutingNumber), "error", err) + c.logger.Log("downloadAllFiles", fmt.Sprintf("ERROR downloading files (ABA: %s)", fileTransferConf.RoutingNumber), "error", err) } return nil } @@ -719,7 +713,7 @@ func (c *fileTransferController) mergeGroupableTransfer(mergedDir string, xfer * // maybeUploadFile will grab the needed configs and upload an given file to the FTP server for CutoffTime's RoutingNumber func (c *fileTransferController) maybeUploadFile(fileToUpload *achFile, cutoffTime *filetransfer.CutoffTime) error { - _, cfg := c.getDetails(cutoffTime) + cfg := c.findFileTransferConfig(cutoffTime) if cfg == nil { return fmt.Errorf("missing file transfer config for %s", cutoffTime.RoutingNumber) } diff --git a/file_transfer_async_test.go b/file_transfer_async_test.go index ce5ea29cb..c46421aa9 100644 --- a/file_transfer_async_test.go +++ b/file_transfer_async_test.go @@ -87,21 +87,18 @@ func TestFileTransferController__getDetails(t *testing.T) { } // happy path - found - ftpConf, fileTransferConf := controller.getDetails(cutoff) - if ftpConf == nil || fileTransferConf == nil { - t.Fatalf("ftpConf=%v fileTransferConf=%v", ftpConf, fileTransferConf) - } - if ftpConf.Hostname != "ftp.foo.com" { - t.Errorf("ftpConf=%#v", ftpConf) + fileTransferConf := controller.findFileTransferConfig(cutoff) + if fileTransferConf == nil { + t.Fatalf("fileTransferConf=%v", fileTransferConf) } if fileTransferConf.InboundPath != "inbound/" { t.Errorf("fileTransferConf=%#v", fileTransferConf) } // not found - ftpConf, fileTransferConf = controller.getDetails(&filetransfer.CutoffTime{RoutingNumber: "456"}) - if ftpConf != nil || fileTransferConf != nil { - t.Fatalf("ftpConf=%v fileTransferConf=%v", ftpConf, fileTransferConf) + fileTransferConf = controller.findFileTransferConfig(&filetransfer.CutoffTime{RoutingNumber: "456"}) + if fileTransferConf != nil { + t.Fatalf("fileTransferConf=%v", fileTransferConf) } } From ea6eda1d33ebb2396ebc39ccf5bcd51c56387f23 Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Mon, 22 Jul 2019 14:40:38 -0500 Subject: [PATCH 19/26] files: pass through filetransfer type from routing numbers --- file_transfer_async.go | 20 ++++++++++++++++-- file_transfer_async_test.go | 29 +++++++++++++++++++++++++- internal/filetransfer/file_transfer.go | 2 +- 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/file_transfer_async.go b/file_transfer_async.go index 23ad21b64..1368b35bd 100644 --- a/file_transfer_async.go +++ b/file_transfer_async.go @@ -169,6 +169,22 @@ func (c *fileTransferController) findFileTransferConfig(cutoff *filetransfer.Cut return nil } +// findTransferType will return a string from matching the provided routingNumber against +// FTP, SFTP (and future) file transport protocols. This string needs to match filetransfer.New. +func (c *fileTransferController) findTransferType(routingNumber string) string { + for i := range c.ftpConfigs { + if routingNumber == c.ftpConfigs[i].RoutingNumber { + return "ftp" + } + } + for i := range c.sftpConfigs { + if routingNumber == c.sftpConfigs[i].RoutingNumber { + return "sftp" + } + } + return "unknown" +} + // startPeriodicFileOperations will block forever to periodically download incoming and returned ACH files while also merging // and uploading ACH files to their remote SFTP server. // @@ -259,7 +275,7 @@ func (c *fileTransferController) downloadAndProcessIncomingFiles(depRepo deposit // downloadAllFiles will setup directories for each routing number and initiate downloading and writing the files to sub-directories. func (c *fileTransferController) downloadAllFiles(dir string, fileTransferConf *filetransfer.Config) error { - agent, err := filetransfer.New("", fileTransferConf, c.repo) // TODO(adam): pass through _type + agent, err := filetransfer.New(c.findTransferType(fileTransferConf.RoutingNumber), fileTransferConf, c.repo) // TODO(adam): pass through _type if err != nil { return fmt.Errorf("downloadAllFiles: problem with %s file transfer agent init: %v", fileTransferConf.RoutingNumber, err) } @@ -717,7 +733,7 @@ func (c *fileTransferController) maybeUploadFile(fileToUpload *achFile, cutoffTi if cfg == nil { return fmt.Errorf("missing file transfer config for %s", cutoffTime.RoutingNumber) } - agent, err := filetransfer.New("", cfg, c.repo) + agent, err := filetransfer.New(c.findTransferType(cutoffTime.RoutingNumber), cfg, c.repo) if err != nil { return fmt.Errorf("problem creating fileTransferAgent for %s: %v", cfg.RoutingNumber, err) } diff --git a/file_transfer_async_test.go b/file_transfer_async_test.go index c46421aa9..aebff0927 100644 --- a/file_transfer_async_test.go +++ b/file_transfer_async_test.go @@ -57,7 +57,7 @@ func TestFileTransferController__newFileTransferController(t *testing.T) { } } -func TestFileTransferController__getDetails(t *testing.T) { +func TestFileTransferController__findFileTransferConfig(t *testing.T) { cutoff := &filetransfer.CutoffTime{ RoutingNumber: "123", Cutoff: 1700, @@ -102,6 +102,33 @@ func TestFileTransferController__getDetails(t *testing.T) { } } +func TestFileTransferController__findTransferType(t *testing.T) { + controller := &fileTransferController{} + + if v := controller.findTransferType(""); v != "unknown" { + t.Errorf("got %s", v) + } + if v := controller.findTransferType("987654320"); v != "unknown" { + t.Errorf("got %s", v) + } + + // Get 'sftp' as type + controller.sftpConfigs = append(controller.sftpConfigs, &filetransfer.SFTPConfig{ + RoutingNumber: "987654320", + }) + if v := controller.findTransferType("987654320"); v != "sftp" { + t.Errorf("got %s", v) + } + + // 'ftp' is checked first, so let's override that now + controller.ftpConfigs = append(controller.ftpConfigs, &filetransfer.FTPConfig{ + RoutingNumber: "987654320", + }) + if v := controller.findTransferType("987654320"); v != "ftp" { + t.Errorf("got %s", v) + } +} + func TestFileTransferController__writeFiles(t *testing.T) { dir, _ := ioutil.TempDir("", "file-transfer-async") defer os.RemoveAll(dir) diff --git a/internal/filetransfer/file_transfer.go b/internal/filetransfer/file_transfer.go index 889526be6..c9e2299a5 100644 --- a/internal/filetransfer/file_transfer.go +++ b/internal/filetransfer/file_transfer.go @@ -61,7 +61,7 @@ func New(_type string, cfg *Config, repo Repository) (Agent, error) { } return newSFTPTransferAgent(cfg, sftpConfigs) default: - return nil, fmt.Errorf("filetransfer: unknown type %s", _type) + return nil, fmt.Errorf("filetransfer: unknown type '%s'", _type) } } From 8a2e03ceeeec798597d2f4916f762346945d85b7 Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Mon, 22 Jul 2019 16:22:56 -0500 Subject: [PATCH 20/26] internal/httptest: test GrabConnectionCertificates --- internal/filetransfer/sftp_test.go | 2 -- internal/httptest/certs_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 internal/httptest/certs_test.go diff --git a/internal/filetransfer/sftp_test.go b/internal/filetransfer/sftp_test.go index b3b1a6c4e..355018a7b 100644 --- a/internal/filetransfer/sftp_test.go +++ b/internal/filetransfer/sftp_test.go @@ -176,8 +176,6 @@ func TestSFTP(t *testing.T) { } } -// docker run -p 22:22 -d atmoz/sftp foo:pass:::upload - func TestSFTP__password(t *testing.T) { deployment := spawnSFTP(t) defer deployment.close(t) diff --git a/internal/httptest/certs_test.go b/internal/httptest/certs_test.go new file mode 100644 index 000000000..6e96b2896 --- /dev/null +++ b/internal/httptest/certs_test.go @@ -0,0 +1,30 @@ +// Copyright 2018 The Moov Authors +// Use of this source code is governed by an Apache License +// license that can be found in the LICENSE file. + +package httptest + +import ( + "os" + "testing" +) + +func TestGrabConnectionCertificates(t *testing.T) { + if testing.Short() { + return + } + + path, err := GrabConnectionCertificates(t, "google.com:443") + if err != nil { + t.Fatal(err) + } + defer os.Remove(path) + + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if info.Size() == 0 { + t.Fatalf("%s is an empty file", info.Name()) + } +} From 4ec37831c8cd26e27d3d6f9819e236c1ce264735 Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Mon, 22 Jul 2019 19:20:01 -0500 Subject: [PATCH 21/26] files: capture first error in writeFiles --- file_transfer_async.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/file_transfer_async.go b/file_transfer_async.go index 1368b35bd..b8c8ba756 100644 --- a/file_transfer_async.go +++ b/file_transfer_async.go @@ -447,16 +447,23 @@ func updateTransferFromReturnCode(logger log.Logger, code *ach.ReturnCode, origD // writeFiles will create files in dir for each file object provided // The contents of each file struct will always be closed. func (c *fileTransferController) writeFiles(files []filetransfer.File, dir string) error { + var firstErr error var errordFilenames []string os.Mkdir(dir, 0777) // ignore errors for i := range files { f, err := os.Create(filepath.Join(dir, files[i].Filename)) if err != nil { + if firstErr == nil { + firstErr = err + } errordFilenames = append(errordFilenames, files[i].Filename) continue } if _, err = io.Copy(f, files[i].Contents); err != nil { + if firstErr == nil { + firstErr = err + } errordFilenames = append(errordFilenames, files[i].Filename) continue } @@ -465,7 +472,7 @@ func (c *fileTransferController) writeFiles(files []filetransfer.File, dir strin files[i].Contents.Close() } if len(errordFilenames) != 0 { - return fmt.Errorf("writeFiles problem on: %s", strings.Join(errordFilenames, ", ")) + return fmt.Errorf("writeFiles problem on: %s: %v", strings.Join(errordFilenames, ", "), firstErr) } return nil } From 5c65e2982dbc0291d5d180a3194078c8039061bc Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Mon, 22 Jul 2019 19:21:34 -0500 Subject: [PATCH 22/26] files: 'mkdir -p' when writing files This helps fixup for SFTP where the paths are slightly different. --- file_transfer_async.go | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/file_transfer_async.go b/file_transfer_async.go index b8c8ba756..97d55b0f1 100644 --- a/file_transfer_async.go +++ b/file_transfer_async.go @@ -450,7 +450,7 @@ func (c *fileTransferController) writeFiles(files []filetransfer.File, dir strin var firstErr error var errordFilenames []string - os.Mkdir(dir, 0777) // ignore errors + os.MkdirAll(dir, 0777) // ignore errors for i := range files { f, err := os.Create(filepath.Join(dir, files[i].Filename)) if err != nil { @@ -491,15 +491,19 @@ func (c *fileTransferController) saveRemoteFiles(agent filetransfer.Agent, dir s errs <- err return } + if err := os.MkdirAll(filepath.Base(filepath.Join(dir, agent.InboundPath())), 0777); err != nil { + errs <- err + return + } if err := c.writeFiles(files, filepath.Join(dir, agent.InboundPath())); err != nil { errs <- err return } for i := range files { - c.logger.Log("saveRemoteFiles", fmt.Sprintf("ACH: copied down inbound file %s", files[i].Filename)) - // Delete inbound file from FTP server + c.logger.Log("saveRemoteFiles", fmt.Sprintf("%T: copied down inbound file %s", agent, files[i].Filename)) + if err := agent.Delete(filepath.Join(agent.InboundPath(), files[i].Filename)); err != nil { - c.logger.Log("saveRemoteFiles", fmt.Sprintf("ACH: problem deleting inbound file %s", files[i].Filename), "error", err) + c.logger.Log("saveRemoteFiles", fmt.Sprintf("%T: problem deleting inbound file %s", agent, files[i].Filename), "error", err) } } }() @@ -513,15 +517,19 @@ func (c *fileTransferController) saveRemoteFiles(agent filetransfer.Agent, dir s errs <- err return } + if err := os.MkdirAll(filepath.Base(filepath.Join(dir, agent.ReturnPath())), 0777); err != nil { + errs <- err + return + } if err := c.writeFiles(files, filepath.Join(dir, agent.ReturnPath())); err != nil { errs <- err return } for i := range files { - c.logger.Log("saveRemoteFiles", fmt.Sprintf("ACH: copied down return file %s", files[i].Filename)) - // Delete return file from FTP server + c.logger.Log("saveRemoteFiles", fmt.Sprintf("%T: copied down return file %s", agent, files[i].Filename)) + if err := agent.Delete(filepath.Join(agent.ReturnPath(), files[i].Filename)); err != nil { - c.logger.Log("saveRemoteFiles", fmt.Sprintf("ACH: problem deleting return file %s", files[i].Filename), "error", err) + c.logger.Log("saveRemoteFiles", fmt.Sprintf("%T problem deleting return file %s", agent, files[i].Filename), "error", err) } } }() From c10091fe5fc8aaa2b4dc6cde50c221a481844d07 Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Mon, 22 Jul 2019 19:22:21 -0500 Subject: [PATCH 23/26] internal/filetransfer: add an example SFTP server with test files --- internal/filetransfer/config.go | 17 ++++++++++++----- internal/filetransfer/sftp.go | 3 ++- makefile | 4 ++++ testdata/sftp-server/inbound/iat-credit.ach | 20 ++++++++++++++++++++ testdata/sftp-server/returned/return-WEB.ach | 10 ++++++++++ 5 files changed, 48 insertions(+), 6 deletions(-) create mode 100644 testdata/sftp-server/inbound/iat-credit.ach create mode 100644 testdata/sftp-server/returned/return-WEB.ach diff --git a/internal/filetransfer/config.go b/internal/filetransfer/config.go index 1f9b0f46b..42f5d0ccc 100644 --- a/internal/filetransfer/config.go +++ b/internal/filetransfer/config.go @@ -189,10 +189,17 @@ func (r *localFileTransferRepository) Close() error { return nil } func (r *localFileTransferRepository) GetConfigs() ([]*Config, error) { return []*Config{ { - RoutingNumber: "121042882", - InboundPath: "inbound/", // below configs match paygate's testdata/ftp-server/ - OutboundPath: "outbound/", - ReturnPath: "returned/", + RoutingNumber: "121042882", // example + + // For 'make start-ftp-server' + InboundPath: "inbound/", // below configs match paygate's testdata/ftp-server/ + OutboundPath: "outbound/", + ReturnPath: "returned/", + + // For 'make start-sftp-server' + // InboundPath: "/upload/inbound/", // below configs match paygate's testdata/ftp-server/ + // OutboundPath: "/upload/outbound/", + // ReturnPath: "/upload/returned/", }, }, nil } @@ -223,7 +230,7 @@ func (r *localFileTransferRepository) GetSFTPConfigs() ([]*SFTPConfig, error) { return []*SFTPConfig{ { RoutingNumber: "121042882", - Hostname: "localhost:22", // below configs for atmoz/sftp:latest + Hostname: "localhost:2222", // below configs for atmoz/sftp:latest Username: "demo", Password: "password", // ClientPrivateKey: "...", // Base64 encoded or PEM format diff --git a/internal/filetransfer/sftp.go b/internal/filetransfer/sftp.go index 7772bd4a5..ffc22e05c 100644 --- a/internal/filetransfer/sftp.go +++ b/internal/filetransfer/sftp.go @@ -9,6 +9,7 @@ import ( "fmt" "io" "io/ioutil" + "path/filepath" "sync" "time" @@ -226,7 +227,7 @@ func (agent *SFTPTransferAgent) readFiles(dir string) ([]File, error) { var files []File for i := range infos { - fd, err := agent.client.Open(infos[i].Name()) + fd, err := agent.client.Open(filepath.Join(dir, infos[i].Name())) if err != nil { return nil, fmt.Errorf("sftp: open %s: %v", infos[i].Name(), err) } diff --git a/makefile b/makefile index c9288deb8..e25e43ef4 100644 --- a/makefile +++ b/makefile @@ -51,6 +51,10 @@ start-ftp-server: @echo Using ACH files in testdata/ftp-server for FTP server @docker run -p 2121:2121 -p 30000-30009:30000-30009 -v $(shell pwd)/testdata/ftp-server:/data moov/fsftp:v0.2.0 -host 0.0.0.0 -root /data -user admin -pass 123456 -passive-ports 30000-30009 +start-sftp-server: + @echo Using ACH files in testdata/sftp-server for SFTP server + @docker run -p 2222:22 -v $(shell pwd)/testdata/sftp-server:/home/demo/upload atmoz/sftp:latest demo:password:::upload + # From https://github.com/genuinetools/img .PHONY: AUTHORS AUTHORS: diff --git a/testdata/sftp-server/inbound/iat-credit.ach b/testdata/sftp-server/inbound/iat-credit.ach new file mode 100644 index 000000000..536bd005e --- /dev/null +++ b/testdata/sftp-server/inbound/iat-credit.ach @@ -0,0 +1,20 @@ +101 121042882 2313801041812180000A094101Bank My Bank Name +5225 FF3 US123456789 IATTRADEPAYMTCADUSD181219 1231380100000001 +6221210428820007 0000100000123456789 1231380100000001 +710ANN000000000000100000928383-23938 BEK Enterprises 0000001 +711BEK Solutions 15 West Place Street 0000001 +712JacobsTown*PA\ US*19305\ 0000001 +713Wells Fargo 01231380104 US 0000001 +714Citadel Bank 01121042882 CA 0000001 +7159874654932139872121 Front Street 0000001 +716LetterTown*AB\ CA*80014\ 0000001 +717This is an international payment 00010000001 +718Bank of France 01456456456987987 FR 00010000001 +82250000100012104288000000000000000000100000 231380100000001 +9000001000002000000100012104288000000000000000000100000 +9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999 +9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999 +9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999 +9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999 +9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999 +9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999 diff --git a/testdata/sftp-server/returned/return-WEB.ach b/testdata/sftp-server/returned/return-WEB.ach new file mode 100644 index 000000000..d81130c86 --- /dev/null +++ b/testdata/sftp-server/returned/return-WEB.ach @@ -0,0 +1,10 @@ +101 091400606 6910001341810170306A094101FIRST BANK & TRUST ASF APPLICATION SUPERVI +5200CoinLion 123456789 WEBTRANSFER 000101 1091000010000001 +626091400606123456789 0000012354MjMxNDAwMjAtOGQPaul Jones S 1091000017611242 +799R01091400600000001 09100001 091000017611242 +82000000020009140060000000012354000000000000 123456789 091000010000001 +5200CoinLion 123456789 WEBTRANSFER 000101 1021000020000002 +621091400606867530999999 0000004565NmRjZTJmMzItMGNBob Marley S 1021000029461242 +799R03091400600000003 02100002 021000029461242 +82000000020009140060000000000000000000004565 123456789 021000020000002 +9000002000001000000040018280120000000012354000000004565 \ No newline at end of file From b8fc8c759c508fc3f3c4fe94377f8d9c613e87b5 Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Mon, 22 Jul 2019 20:07:30 -0500 Subject: [PATCH 24/26] internal/filetransfer: add more tests --- internal/filetransfer/sftp_test.go | 36 ++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/internal/filetransfer/sftp_test.go b/internal/filetransfer/sftp_test.go index 355018a7b..1d955cfb4 100644 --- a/internal/filetransfer/sftp_test.go +++ b/internal/filetransfer/sftp_test.go @@ -205,3 +205,39 @@ func TestSFTP__password(t *testing.T) { func TestSFTP__ClientPrivateKey(t *testing.T) { } + +func TestSFTP__sftpConnect(t *testing.T) { + client, _, _, err := sftpConnect(&SFTPConfig{ + Username: "foo", + }) + if client != nil || err == nil { + t.Errorf("client=%v err=%v", client, err) + } +} + +func TestSFTPAgent(t *testing.T) { + agent := &SFTPTransferAgent{ + cfg: &Config{ + InboundPath: "inbound", + }, + } + if v := agent.InboundPath(); v != "inbound" { + t.Errorf("agent.InboundPath()=%s", agent.InboundPath()) + } + + agent.cfg.ReturnPath = "return" + if v := agent.ReturnPath(); v != "return" { + t.Errorf("agent.ReturnPath()=%s", agent.ReturnPath()) + } +} + +func TestSFTPAgent__findConfig(t *testing.T) { + agent := &SFTPTransferAgent{ + cfg: &Config{ + RoutingNumber: "987654320", + }, + } + if conf := agent.findConfig(); conf != nil { + t.Error("expected nil") + } +} From 47982defd36e8edbd095cd56a87b4519efef808c Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Tue, 23 Jul 2019 10:42:32 -0500 Subject: [PATCH 25/26] internal/filetransfer: test sftp Delete --- internal/filetransfer/sftp_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/filetransfer/sftp_test.go b/internal/filetransfer/sftp_test.go index 1d955cfb4..d45ce5eeb 100644 --- a/internal/filetransfer/sftp_test.go +++ b/internal/filetransfer/sftp_test.go @@ -191,6 +191,10 @@ func TestSFTP__password(t *testing.T) { if err != nil { t.Fatal(err) } + + if err := deployment.agent.Delete(deployment.agent.OutboundPath() + "upload.ach"); err != nil { + t.Fatal(err) + } } // Generate keys (in Go) and mount them into our test container From 4b6f9bb0c6a3fd4a050b9f51e853de93c226f9d9 Mon Sep 17 00:00:00 2001 From: Adam Shannon Date: Tue, 23 Jul 2019 10:42:47 -0500 Subject: [PATCH 26/26] internal/filetransfer: sftp test GetInboundFiles() and GetReturnFiles() --- internal/filetransfer/sftp_test.go | 53 ++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/internal/filetransfer/sftp_test.go b/internal/filetransfer/sftp_test.go index d45ce5eeb..aad4955b7 100644 --- a/internal/filetransfer/sftp_test.go +++ b/internal/filetransfer/sftp_test.go @@ -6,8 +6,10 @@ package filetransfer import ( "fmt" + "io" "io/ioutil" "os" + "path/filepath" "runtime" "strings" "syscall" @@ -127,9 +129,9 @@ func newAgent(host, user, pass, passFile string) (*SFTPTransferAgent, error) { // put files into /upload/ (as an absolute path). // // Currently it's assumed sub-directories would exist for inbound vs outbound files. - InboundPath: "/upload/", + InboundPath: "/upload/inbound/", OutboundPath: "/upload/", - ReturnPath: "/upload/", + ReturnPath: "/upload/returned/", } sftpConfigs := []*SFTPConfig{ { @@ -176,6 +178,19 @@ func TestSFTP(t *testing.T) { } } +func cp(from, to string) error { + f, err := os.Open(from) + if err != nil { + return err + } + t, err := os.Create(to) + if err != nil { + return err + } + _, err = io.Copy(t, f) + return err +} + func TestSFTP__password(t *testing.T) { deployment := spawnSFTP(t) defer deployment.close(t) @@ -195,6 +210,40 @@ func TestSFTP__password(t *testing.T) { if err := deployment.agent.Delete(deployment.agent.OutboundPath() + "upload.ach"); err != nil { t.Fatal(err) } + + // Inbound files (IAT in our testdata/sftp-server/) + os.MkdirAll(filepath.Join(deployment.dir, "inbound"), 0777) + err = cp( + filepath.Join("..", "..", "testdata", "sftp-server", "inbound", "iat-credit.ach"), + filepath.Join(deployment.dir, "inbound", "iat-credit.ach"), + ) + if err != nil { + t.Fatal(err) + } + files, err := deployment.agent.GetInboundFiles() + if err != nil { + t.Fatal(err) + } + if len(files) != 1 || files[0].Filename != "iat-credit.ach" { + t.Errorf("%d of files: %#v", len(files), files) + } + + // Return files (WEB in our testdata/sftp-server/) + os.MkdirAll(filepath.Join(deployment.dir, "returned"), 0777) + err = cp( + filepath.Join("..", "..", "testdata", "sftp-server", "returned", "return-WEB.ach"), + filepath.Join(deployment.dir, "returned", "return-WEB.ach"), + ) + if err != nil { + t.Fatal(err) + } + files, err = deployment.agent.GetReturnFiles() + if err != nil { + t.Fatal(err) + } + if len(files) != 1 || files[0].Filename != "return-WEB.ach" { + t.Errorf("%d of files: %#v", len(files), files) + } } // Generate keys (in Go) and mount them into our test container