Add backend file size limit for uploads (#2658)

* Add fileSizeLimit parameter to Uploader. Uploads greater than the fileSizeLimit limit will be rejected.
* Add MaxFileSizeLimit for all Uploaders of 350MB

cmd/generate-test-data/main.go

@@ -158,7 +158,10 @@ func main() {
 		zap.L().Info("Using memory storage backend")
 		fsParams := storage.NewMemoryParams("tmp", "testdata", logger)
 		storer := storage.NewMemory(fsParams)
-		loader := uploader.NewUploader(dbConnection, logger, storer)
+		loader, uploaderErr := uploader.NewUploader(dbConnection, logger, storer, 25*uploader.MB)
+		if uploaderErr != nil {
+			logger.Fatal("could not instantiate uploader", zap.Error(err))
+		}
 
 		tdgs.E2eBasicScenario.Run(dbConnection, loader, logger, storer)
 		logger.Info("Success! Created e2e test data.")

cmd/image-to-pdf/main.go

@@ -35,7 +35,10 @@ func main() {
 		log.Fatalf("Failed to initialize Zap logging due to %v", err)
 	}
 	storer := storage.NewMemory(storage.NewMemoryParams("", "", logger))
-	uploader := uploader.NewUploader(nil, logger, storer)
+	uploader, err := uploader.NewUploader(nil, logger, storer, 25*uploader.MB)
+	if err != nil {
+		log.Fatalf("could not instantiate uploader due to %v", err)
+	}
 	generator, err := paperwork.NewGenerator(nil, logger, uploader)
 	if err != nil {
 		log.Fatal(err.Error())

pkg/handlers/internalapi/personally_procured_move_attachments.go

@@ -41,7 +41,11 @@ func (h CreatePersonallyProcuredMoveAttachmentsHandler) Handle(params ppmop.Crea
 	}
 
 	// Init our tools
-	loader := uploader.NewUploader(h.DB(), logger, h.FileStorer())
+	loader, err := uploader.NewUploader(h.DB(), logger, h.FileStorer(), 100*uploader.MB)
+	if err != nil {
+		logger.Error("could not instantiate uploader", zap.Error(err))
+		return ppmop.NewCreatePPMAttachmentsInternalServerError()
+	}
 	generator, err := paperwork.NewGenerator(h.DB(), logger, loader)
 	if err != nil {
 		logger.Error("failed to initialize generator", zap.Error(err))
@@ -74,7 +78,12 @@ func (h CreatePersonallyProcuredMoveAttachmentsHandler) Handle(params ppmop.Crea
 	// Upload merged PDF to S3 and return Upload object
 	pdfUpload, verrs, err := loader.CreateUpload(session.UserID, &mergedPdf, uploader.AllowedTypesPDF)
 	if verrs.HasAny() || err != nil {
-		return handlers.ResponseForVErrors(logger, verrs, err)
+		switch err.(type) {
+		case uploader.ErrTooLarge:
+			return ppmop.NewCreatePPMAttachmentsRequestEntityTooLarge()
+		default:
+			return handlers.ResponseForVErrors(logger, verrs, err)
+		}
 	}
 
 	url, err := loader.PresignedURL(pdfUpload)

pkg/handlers/internalapi/personally_procured_move_attachments_test.go

@@ -82,7 +82,8 @@ func (suite *HandlerSuite) TestCreatePPMAttachmentsHandlerTests() {
 			suite.NoError(err)
 
 			// Create upload for expense document model
-			loader := uploader.NewUploader(suite.DB(), suite.TestLogger(), context.FileStorer())
+			loader, err := uploader.NewUploader(suite.DB(), suite.TestLogger(), context.FileStorer(), 100*uploader.MB)
+			suite.NoError(err)
 			loader.CreateUploadForDocument(&expDoc.MoveDocument.DocumentID, *officeUser.UserID, f, uploader.AllowedTypesServiceMember)
 
 			request := httptest.NewRequest("POST", "/fake/path", nil)

pkg/handlers/internalapi/uploads.go

@@ -81,10 +81,18 @@ func (h CreateUploadHandler) Handle(params uploadop.CreateUploadParams) middlewa
 		return uploadop.NewCreateUploadInternalServerError()
 	}
 
-	uploader := uploaderpkg.NewUploader(h.DB(), logger, h.FileStorer())
+	uploader, err := uploaderpkg.NewUploader(h.DB(), logger, h.FileStorer(), 25*uploaderpkg.MB)
+	if err != nil {
+		logger.Fatal("could not instantiate uploader", zap.Error(err))
+	}
 	newUpload, verrs, err := uploader.CreateUploadForDocument(docID, session.UserID, aFile, uploaderpkg.AllowedTypesServiceMember)
-	if err != nil || verrs.HasAny() {
-		return handlers.ResponseForVErrors(logger, verrs, err)
+	if verrs.HasAny() || err != nil {
+		switch err.(type) {
+		case uploaderpkg.ErrTooLarge:
+			return uploadop.NewCreateUploadRequestEntityTooLarge()
+		default:
+			return handlers.ResponseForVErrors(logger, verrs, err)
+		}
 	}
 
 	url, err := uploader.PresignedURL(newUpload)
@@ -114,7 +122,10 @@ func (h DeleteUploadHandler) Handle(params uploadop.DeleteUploadParams) middlewa
 		return handlers.ResponseForError(logger, err)
 	}
 
-	uploader := uploaderpkg.NewUploader(h.DB(), logger, h.FileStorer())
+	uploader, err := uploaderpkg.NewUploader(h.DB(), logger, h.FileStorer(), 25*uploaderpkg.MB)
+	if err != nil {
+		logger.Fatal("could not instantiate uploader", zap.Error(err))
+	}
 	if err = uploader.DeleteUpload(&upload); err != nil {
 		return handlers.ResponseForError(logger, err)
 	}
@@ -134,7 +145,10 @@ func (h DeleteUploadsHandler) Handle(params uploadop.DeleteUploadsParams) middle
 
 	// User should always be populated by middleware
 	session, logger := h.SessionAndLoggerFromRequest(params.HTTPRequest)
-	uploader := uploaderpkg.NewUploader(h.DB(), logger, h.FileStorer())
+	uploader, err := uploaderpkg.NewUploader(h.DB(), logger, h.FileStorer(), 25*uploaderpkg.MB)
+	if err != nil {
+		logger.Fatal("could not instantiate uploader", zap.Error(err))
+	}
 
 	for _, uploadID := range params.UploadIds {
 		uuid, _ := uuid.FromString(uploadID.String())

pkg/paperwork/paperwork_test.go

@@ -66,10 +66,14 @@ func TestPaperworkSuite(t *testing.T) {
 	storer := storageTest.NewFakeS3Storage(true)
 
 	popSuite := testingsuite.NewPopTestSuite(testingsuite.CurrentPackage())
+	newUploader, err := uploader.NewUploader(popSuite.DB(), logger, storer, 25*uploader.MB)
+	if err != nil {
+		log.Panic(err)
+	}
 	hs := &PaperworkSuite{
 		PopTestSuite: popSuite,
 		logger:       logger,
-		uploader:     uploader.NewUploader(popSuite.DB(), logger, storer),
+		uploader:     newUploader,
 	}
 
 	suite.Run(t, hs)

pkg/services/invoice/store_invoice.go

@@ -54,7 +54,10 @@ func (s StoreInvoice858C) Call(edi string, invoice *models.Invoice, userID uuid.
 	}
 
 	// Create Upload'r
-	loader := uploader.NewUploader(s.DB, s.Logger, *s.Storer)
+	loader, err := uploader.NewUploader(s.DB, s.Logger, *s.Storer, 25*uploader.MB)
+	if err != nil {
+		s.Logger.Fatal("could not instantiate uploader", zap.Error(err))
+	}
 	// Set Storagekey path for S3
 	loader.SetUploadStorageKey(ediTmpFile)
 

pkg/services/invoice/update_invoice_upload_test.go

@@ -64,14 +64,15 @@ func (suite *InvoiceServiceSuite) fixture(name string) afero.File {
 func (suite *InvoiceServiceSuite) helperCreateUpload(storer *storage.FileStorer) *models.Upload {
 	document := testdatagen.MakeDefaultDocument(suite.DB())
 	userID := document.ServiceMember.UserID
-	up := uploader.NewUploader(suite.DB(), suite.logger, *storer)
+	up, err := uploader.NewUploader(suite.DB(), suite.logger, *storer, 25*uploader.MB)
+	suite.NoError(err)
 
 	// Create file to use for upload
 	file := suite.fixture("test.pdf")
 	if file == nil {
 		suite.T().Fatal("test.pdf is missing")
 	}
-	_, err := file.Stat()
+	_, err = file.Stat()
 	if err != nil {
 		suite.T().Fatalf("file.Stat() err: %s", err.Error())
 	}

pkg/uploader/uploader.go

@@ -1,6 +1,7 @@
 package uploader
 
 import (
+	"fmt"
 	"io"
 
 	"github.com/gobuffalo/pop"
@@ -17,23 +18,53 @@ import (
 // ErrZeroLengthFile represents an error caused by a file with no content
 var ErrZeroLengthFile = errors.New("File has length of 0")
 
+type ErrTooLarge struct {
+	FileSize      int64
+	FileSizeLimit ByteSize
+}
+
+var ErrFileSizeLimitExceedsMax = errors.Errorf("FileSizeLimit exceeds max of %d bytes", MaxFileSizeLimit)
+
+const MaxFileSizeLimit = 350 * MB
+
+func (e ErrTooLarge) Error() string {
+	return fmt.Sprintf("file is too large: %d > %d filesize limit", e.FileSize, e.FileSizeLimit)
+}
+
+type ByteSize int64
+
+const (
+	B  ByteSize = 1
+	KB          = 1000
+	MB          = 1000 * 1000
+)
+
+func (b ByteSize) Int64() int64 {
+	return int64(b)
+}
+
 // Uploader encapsulates a few common processes: creating Uploads for a Document,
 // generating pre-signed URLs for file access, and deleting Uploads.
 type Uploader struct {
 	db               *pop.Connection
 	logger           Logger
 	Storer           storage.FileStorer
 	UploadStorageKey string
+	FileSizeLimit    ByteSize
 }
 
 // NewUploader creates and returns a new uploader
-func NewUploader(db *pop.Connection, logger Logger, storer storage.FileStorer) *Uploader {
+func NewUploader(db *pop.Connection, logger Logger, storer storage.FileStorer, fileSizeLimit ByteSize) (*Uploader, error) {
+	if fileSizeLimit > MaxFileSizeLimit {
+		return nil, ErrFileSizeLimitExceedsMax
+	}
 	return &Uploader{
 		db:               db,
 		logger:           logger,
 		Storer:           storer,
 		UploadStorageKey: "",
-	}
+		FileSizeLimit:    fileSizeLimit,
+	}, nil
 }
 
 // SetUploadStorageKey set the Upload.StorageKey member
@@ -56,6 +87,14 @@ func (u *Uploader) CreateUploadForDocument(documentID *uuid.UUID, userID uuid.UU
 		return nil, responseVErrors, ErrZeroLengthFile
 	}
 
+	if info.Size() > u.FileSizeLimit.Int64() {
+		u.logger.Error("upload exceeds file size limit",
+			zap.String("Filename", file.Name()),
+			zap.Int64("FileSize", info.Size()),
+			zap.Int64("FileSizeLimit", u.FileSizeLimit.Int64()))
+		return nil, responseVErrors, ErrTooLarge{info.Size(), u.FileSizeLimit}
+	}
+
 	contentType, detectContentTypeErr := storage.DetectContentType(file)
 	if detectContentTypeErr != nil {
 		u.logger.Error("Could not detect content type", zap.Error(detectContentTypeErr))

pkg/uploader/uploader_test.go

@@ -1,6 +1,7 @@
 package uploader_test
 
 import (
+	"fmt"
 	"io"
 	"log"
 	"os"
@@ -94,10 +95,17 @@ func TestUploaderSuite(t *testing.T) {
 	suite.Run(t, hs)
 }
 
+func (suite *UploaderSuite) TestUploaderExceedsFileSizeLimit() {
+	_, err := uploader.NewUploader(suite.DB(), suite.logger, suite.storer, 361*uploader.MB)
+	suite.Error(err)
+	suite.Equal(uploader.ErrFileSizeLimitExceedsMax, err)
+}
+
 func (suite *UploaderSuite) TestUploadFromLocalFile() {
 	document := testdatagen.MakeDefaultDocument(suite.DB())
 
-	up := uploader.NewUploader(suite.DB(), suite.logger, suite.storer)
+	up, err := uploader.NewUploader(suite.DB(), suite.logger, suite.storer, 25*uploader.MB)
+	suite.NoError(err)
 	file := suite.fixture("test.pdf")
 
 	upload, verrs, err := up.CreateUploadForDocument(&document.ID, document.ServiceMember.UserID, file, uploader.AllowedTypesPDF)
@@ -110,25 +118,68 @@ func (suite *UploaderSuite) TestUploadFromLocalFile() {
 func (suite *UploaderSuite) TestUploadFromLocalFileZeroLength() {
 	document := testdatagen.MakeDefaultDocument(suite.DB())
 
-	up := uploader.NewUploader(suite.DB(), suite.logger, suite.storer)
-	file := suite.fixture("empty.pdf")
+	up, err := uploader.NewUploader(suite.DB(), suite.logger, suite.storer, 25*uploader.MB)
+	suite.NoError(err)
+	file, cleanup, err := suite.createFileOfArbitrarySize(uint64(0 * uploader.MB))
+	suite.Nil(err, "failed to create upload")
+	defer cleanup()
 
-	upload, verrs, err := up.CreateUploadForDocument(&document.ID, document.ServiceMember.UserID, file, uploader.AllowedTypesPDF)
-	suite.Equal(err, uploader.ErrZeroLengthFile)
+	upload, verrs, err := up.CreateUploadForDocument(&document.ID, document.ServiceMember.UserID, file, uploader.AllowedTypesAny)
+	suite.Equal(uploader.ErrZeroLengthFile, err)
 	suite.False(verrs.HasAny(), "failed to validate upload")
 	suite.Nil(upload, "returned an upload when erroring")
 }
 
+func (suite *UploaderSuite) TestUploadFromLocalFileWrongContentType() {
+	document := testdatagen.MakeDefaultDocument(suite.DB())
+
+	up, err := uploader.NewUploader(suite.DB(), suite.logger, suite.storer, 25*uploader.MB)
+	suite.NoError(err)
+	file, cleanup, err := suite.createFileOfArbitrarySize(uint64(1 * uploader.MB))
+	suite.Nil(err, "failed to create upload")
+	defer cleanup()
+
+	upload, verrs, err := up.CreateUploadForDocument(&document.ID, document.ServiceMember.UserID, file, uploader.AllowedTypesPDF)
+	suite.NoError(err)
+	suite.True(verrs.HasAny(), "invalid content type for upload")
+	suite.Nil(upload, "returned an upload when erroring")
+}
+
 func (suite *UploaderSuite) TestTooLargeUploadFromLocalFile() {
 	document := testdatagen.MakeDefaultDocument(suite.DB())
 
-	up := uploader.NewUploader(suite.DB(), suite.logger, suite.storer)
-	file := suite.fixture("largejpeg.jpg")
+	up, err := uploader.NewUploader(suite.DB(), suite.logger, suite.storer, 25*uploader.MB)
+	suite.NoError(err)
+	f, cleanup, err := suite.createFileOfArbitrarySize(uint64(26 * uploader.MB))
+	suite.NoError(err)
+	defer cleanup()
 
-	upload, verrs, err := up.CreateUploadForDocument(&document.ID, document.ServiceMember.UserID, file, uploader.AllowedTypesServiceMember)
-	suite.Nil(err)
+	_, verrs, err := up.CreateUploadForDocument(&document.ID, document.ServiceMember.UserID, f, uploader.AllowedTypesAny)
+	suite.Error(err)
+	suite.IsType(uploader.ErrTooLarge{}, err)
 	suite.False(verrs.HasAny(), "failed to validate upload")
-	suite.NotNil(upload)
+}
+
+func (suite *UploaderSuite) createFileOfArbitrarySize(size uint64) (afero.File, func(), error) {
+	data := make([]byte, size, size)
+	tmpFileName := "tmpfile"
+	f, err := suite.fs.Create(tmpFileName)
+	if err != nil {
+		fmt.Printf("Error: %s", err)
+	}
+	_, err = f.Write(data)
+	if err != nil {
+		fmt.Printf("Error: %s", err)
+	}
+	cleanup := func() {
+		if closeErr := f.Close(); closeErr != nil {
+			log.Println("error closing file")
+		}
+		if removeErr := suite.fs.Remove(tmpFileName); removeErr != nil {
+			log.Println("error removing file")
+		}
+	}
+	return f, cleanup, err
 }
 
 func (suite *UploaderSuite) helperNewTempFile() (afero.File, error) {
@@ -143,7 +194,8 @@ func (suite *UploaderSuite) TestCreateUploadNoDocument() {
 	document := testdatagen.MakeDefaultDocument(suite.DB())
 	userID := document.ServiceMember.UserID
 
-	up := uploader.NewUploader(suite.DB(), suite.logger, suite.storer)
+	up, err := uploader.NewUploader(suite.DB(), suite.logger, suite.storer, 25*uploader.MB)
+	suite.NoError(err)
 	file := suite.fixture("test.pdf")
 	fixtureFileInfo, err := file.Stat()
 	suite.NoError(err)

swagger/internal.yaml

@@ -3400,6 +3400,8 @@ paths:
           description: must be authenticated to use this endpoint
         403:
           description: not authorized to perform this action
+        413:
+          description: payload is too large
         422:
           description: malformed PDF contained in uploads
         424:
@@ -3599,6 +3601,8 @@ paths:
           description: not authorized
         404:
           description: not found
+        413:
+          description: payload is too large
         500:
           description: server error
     delete: