MM-60115, MM-62436, MM-62493: Compliance export downloads from local and s3; e2e tests (#29806)

* add ZipReader method to filestore and file/s3 backends

to merge

to merge

* add WriteStreamResponse as an alternative to WriteFileResponse

* add generated layers

* enable download link; download job endpoint now streams export dir zips

* fix MM-62493

* re-enable e2e tests--we have download links, folks

* Add tests for ZipReader in filestore and s3store

* remove unnecessary error return on ZipReader

* little cleanup

* improve tests; some refactoring: s.Nil(err) -> s.NoError(err)

* blank commit

* backwards compatability for pre-10.5 job downloads

* compress file response; better errors; better comments; PR comments

* update generated app layers

* improve/widen tests; improve comments; simplify localstore ZipReader

* regenerate layers

* follow GoDoc conventions

* update generated layers

* remove unnecessary comment

* in jobs/job-id/download, clean exportDir before sending to ZipReader

* better comments; add an error return on ZipReader

* improve file permissions

* adjust tests for new error returns

* linting

* i18n

server/channels/api4/job.go

@@ -6,10 +6,13 @@ package api4
 import (
 	"encoding/json"
 	"net/http"
+	"path"
 	"path/filepath"
 	"strconv"
 	"time"
 
+	"github.com/mattermost/mattermost/server/v8/platform/shared/filestore"
+
 	"github.com/mattermost/mattermost/server/public/model"
 	"github.com/mattermost/mattermost/server/public/shared/mlog"
 	"github.com/mattermost/mattermost/server/v8/channels/audit"
@@ -55,7 +58,7 @@ func getJob(c *Context, w http.ResponseWriter, r *http.Request) {
 
 func downloadJob(c *Context, w http.ResponseWriter, r *http.Request) {
 	config := c.App.Config()
-	const FilePath = "export"
+	const oldFilePath = "export"
 	const FileMime = "application/zip"
 
 	c.RequireJobId()
@@ -90,19 +93,53 @@ func downloadJob(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	fileName := job.Id + ".zip"
-	filePath := filepath.Join(FilePath, fileName)
-	fileReader, err := c.App.FileReader(filePath)
-	if err != nil {
-		c.Err = err
-		c.Err.StatusCode = http.StatusNotFound
+	exportDir, ok := job.Data["export_dir"]
+	fileName := path.Base(exportDir)
+	if !ok || exportDir == "" || fileName == "/" || fileName == "." {
+		// Could be a pre-overhaul job. Try the old method:
+		fileName = job.Id + ".zip"
+		filePath := filepath.Join(oldFilePath, fileName)
+		var fileReader filestore.ReadCloseSeeker
+		fileReader, err = c.App.FileReader(filePath)
+		if err != nil {
+			c.Err = model.NewAppError("unableToDownloadJob", "api.job.unable_to_download_job", nil,
+				"job.Data did not include export_dir, export_dir was malformed, or jobId.zip wasn't found",
+				http.StatusNotFound).Wrap(err)
+			return
+		}
+		defer fileReader.Close()
+
+		// We are able to pass 0 for content size due to the fact that Golang's serveContent (https://golang.org/src/net/http/fs.go)
+		// already sets that for us
+		web.WriteFileResponse(fileName, FileMime, 0, time.UnixMilli(job.LastActivityAt), *c.App.Config().ServiceSettings.WebserverMode, fileReader, true, w, r)
 		return
 	}
-	defer fileReader.Close()
 
-	// We are able to pass 0 for content size due to the fact that Golang's serveContent (https://golang.org/src/net/http/fs.go)
-	// already sets that for us
-	web.WriteFileResponse(fileName, FileMime, 0, time.Unix(0, job.LastActivityAt*int64(1000*1000)), *c.App.Config().ServiceSettings.WebserverMode, fileReader, true, w, r)
+	// We have a base directory, we're using that as the exported filename:
+	fileName += ".zip"
+
+	cleanedExportDir := filepath.Clean(exportDir)
+	if !filepath.IsLocal(cleanedExportDir) {
+		c.Err = model.NewAppError("unableToDownloadJob", "api.job.unable_to_download_job", nil,
+			"job.Data did not include export_dir, export_dir was malformed, or jobId.zip wasn't found",
+			http.StatusNotFound).Wrap(err)
+		return
+	}
+
+	zipReader, err := c.App.ZipReader(cleanedExportDir, false)
+	if err != nil {
+		c.Err = model.NewAppError("unableToDownloadJob", "api.job.unable_to_download_job", nil,
+			"error creating zip reader", http.StatusNotFound).Wrap(err)
+		return
+	}
+	defer zipReader.Close()
+
+	if err := web.WriteStreamResponse(w, zipReader, fileName, FileMime, true); err != nil {
+		c.Err = model.NewAppError("unableToDownloadJob", "api.job.unable_to_download_job", nil,
+			"failure to WriteStreamResponse", http.StatusInternalServerError).
+			Wrap(err)
+		return
+	}
 }
 
 func createJob(c *Context, w http.ResponseWriter, r *http.Request) {

server/channels/app/app_iface.go

@@ -54,10 +54,6 @@ type AppIface interface {
 	AddPublicKey(name string, key io.Reader) *model.AppError
 	// AddUserToChannel adds a user to a given channel.
 	AddUserToChannel(c request.CTX, user *model.User, channel *model.Channel, skipTeamMemberIntegrityCheck bool) (*model.ChannelMember, *model.AppError)
-	// Caller must close the first return value
-	ExportFileReader(path string) (filestore.ReadCloseSeeker, *model.AppError)
-	// Caller must close the first return value
-	FileReader(path string) (filestore.ReadCloseSeeker, *model.AppError)
 	// ChannelMembersMinusGroupMembers returns the set of users in the given channel minus the set of users in the given
 	// groups.
 	//
@@ -105,8 +101,6 @@ type AppIface interface {
 	// CreateUser creates a user and sets several fields of the returned User struct to
 	// their zero values.
 	CreateUser(c request.CTX, user *model.User) (*model.User, *model.AppError)
-	// Creates and stores FileInfos for a post created before the FileInfos table existed.
-	MigrateFilenamesToFileInfos(rctx request.CTX, post *model.Post) []*model.FileInfo
 	// DefaultChannelNames returns the list of system-wide default channel names.
 	//
 	// By default the list will be (not necessarily in this order):
@@ -153,10 +147,18 @@ type AppIface interface {
 	// attributes of the attachment structure. The Slack attachment structure is
 	// documented here: https://api.slack.com/docs/attachments
 	ProcessSlackAttachments(attachments []*model.SlackAttachment) []*model.SlackAttachment
+	// ExportFileReader returns a ReadCloseSeeker for path from the ExportFileBackend.
+	//
+	// The caller is responsible for closing the returned ReadCloseSeeker.
+	ExportFileReader(path string) (filestore.ReadCloseSeeker, *model.AppError)
 	// ExtendSessionExpiryIfNeeded extends Session.ExpiresAt based on session lengths in config.
 	// A new ExpiresAt is only written if enough time has elapsed since last update.
 	// Returns true only if the session was extended.
 	ExtendSessionExpiryIfNeeded(rctx request.CTX, session *model.Session) bool
+	// FileReader returns a ReadCloseSeeker for path from the FileBackend.
+	//
+	// The caller is responsible for closing the returned ReadCloseSeeker.
+	FileReader(path string) (filestore.ReadCloseSeeker, *model.AppError)
 	// FillInPostProps should be invoked before saving posts to fill in properties such as
 	// channel_mentions.
 	//
@@ -270,6 +272,8 @@ type AppIface interface {
 	// MentionsToTeamMembers returns all the @ mentions found in message that
 	// belong to users in the specified team, linking them to their users
 	MentionsToTeamMembers(c request.CTX, message, teamID string) model.UserMentionMap
+	// MigrateFilenamesToFileInfos creates and stores FileInfos for a post created before the FileInfos table existed.
+	MigrateFilenamesToFileInfos(rctx request.CTX, post *model.Post) []*model.FileInfo
 	// MoveChannel method is prone to data races if someone joins to channel during the move process. However this
 	// function is only exposed to sysadmins and the possibility of this edge case is relatively small.
 	MoveChannel(c request.CTX, team *model.Team, channel *model.Channel, user *model.User) *model.AppError
@@ -422,6 +426,10 @@ type AppIface interface {
 	ValidateUserPermissionsOnChannels(c request.CTX, userId string, channelIds []string) []string
 	// VerifyPlugin checks that the given signature corresponds to the given plugin and matches a trusted certificate.
 	VerifyPlugin(plugin, signature io.ReadSeeker) *model.AppError
+	// ZipReader returns a ReadCloser for path. If deflate is true, the zip will use compression.
+	//
+	// The caller is responsible for closing the returned ReadCloser.
+	ZipReader(path string, deflate bool) (io.ReadCloser, *model.AppError)
 	// validateMoveOrCopy performs validation on a provided post list to determine
 	// if all permissions are in place to allow the for the posts to be moved or
 	// copied.

server/channels/app/file.go

@@ -124,20 +124,43 @@ func fileReader(backend filestore.FileBackend, path string) (filestore.ReadClose
 	return result, nil
 }
 
+func zipReader(backend filestore.FileBackend, path string, deflate bool) (io.ReadCloser, *model.AppError) {
+	result, err := backend.ZipReader(path, deflate)
+	if err != nil {
+		return nil, model.NewAppError("ZipReader", "api.file.zip_file_reader.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
+	}
+	return result, nil
+}
+
 func (s *Server) fileReader(path string) (filestore.ReadCloseSeeker, *model.AppError) {
 	return fileReader(s.FileBackend(), path)
 }
 
+func (s *Server) zipReader(path string, deflate bool) (io.ReadCloser, *model.AppError) {
+	return zipReader(s.FileBackend(), path, deflate)
+}
+
 func (s *Server) exportFileReader(path string) (filestore.ReadCloseSeeker, *model.AppError) {
 	return fileReader(s.ExportFileBackend(), path)
 }
 
-// Caller must close the first return value
+// FileReader returns a ReadCloseSeeker for path from the FileBackend.
+//
+// The caller is responsible for closing the returned ReadCloseSeeker.
 func (a *App) FileReader(path string) (filestore.ReadCloseSeeker, *model.AppError) {
 	return a.Srv().fileReader(path)
 }
 
-// Caller must close the first return value
+// ZipReader returns a ReadCloser for path. If deflate is true, the zip will use compression.
+//
+// The caller is responsible for closing the returned ReadCloser.
+func (a *App) ZipReader(path string, deflate bool) (io.ReadCloser, *model.AppError) {
+	return a.Srv().zipReader(path, deflate)
+}
+
+// ExportFileReader returns a ReadCloseSeeker for path from the ExportFileBackend.
+//
+// The caller is responsible for closing the returned ReadCloseSeeker.
 func (a *App) ExportFileReader(path string) (filestore.ReadCloseSeeker, *model.AppError) {
 	return a.Srv().exportFileReader(path)
 }
@@ -424,7 +447,7 @@ func parseOldFilenames(rctx request.CTX, filenames []string, channelID, userID s
 	return parsed
 }
 
-// Creates and stores FileInfos for a post created before the FileInfos table existed.
+// MigrateFilenamesToFileInfos creates and stores FileInfos for a post created before the FileInfos table existed.
 func (a *App) MigrateFilenamesToFileInfos(rctx request.CTX, post *model.Post) []*model.FileInfo {
 	if len(post.Filenames) == 0 {
 		rctx.Logger().Warn("Unable to migrate post to use FileInfos with an empty Filenames field", mlog.String("post_id", post.Id))

server/channels/app/opentracing/opentracing_layer.go

@@ -20061,6 +20061,28 @@ func (a *OpenTracingAppLayer) WriteFileContext(ctx context.Context, fr io.Reader
 	return resultVar0, resultVar1
 }
 
+func (a *OpenTracingAppLayer) ZipReader(path string, deflate bool) (io.ReadCloser, *model.AppError) {
+	origCtx := a.ctx
+	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.ZipReader")
+
+	a.ctx = newCtx
+	a.app.Srv().Store().SetContext(newCtx)
+	defer func() {
+		a.app.Srv().Store().SetContext(origCtx)
+		a.ctx = origCtx
+	}()
+
+	defer span.Finish()
+	resultVar0, resultVar1 := a.app.ZipReader(path, deflate)
+
+	if resultVar1 != nil {
+		span.LogFields(spanlog.Error(resultVar1))
+		ext.Error.Set(span, true)
+	}
+
+	return resultVar0, resultVar1
+}
+
 func NewOpenTracingAppLayer(childApp app.AppIface, ctx context.Context) *OpenTracingAppLayer {
 	newApp := OpenTracingAppLayer{
 		app: childApp,