PLT-2600/PLT-2770 Added Get Public Link modal and added new API for public file links (#2892)

* Switched public file links to use a GetLinkModal

* Separated getFile and the new getPublicFile api calls

api/api.go

@@ -43,6 +43,8 @@ type Routes struct {
 	Preferences *mux.Router // 'api/v3/preferences'
 
 	License *mux.Router // 'api/v3/license'
+
+	Public *mux.Router // 'api/v3/public'
 }
 
 var BaseRoutes *Routes
@@ -67,6 +69,7 @@ func InitApi() {
 	BaseRoutes.Admin = BaseRoutes.ApiRoot.PathPrefix("/admin").Subrouter()
 	BaseRoutes.Preferences = BaseRoutes.ApiRoot.PathPrefix("/preferences").Subrouter()
 	BaseRoutes.License = BaseRoutes.ApiRoot.PathPrefix("/license").Subrouter()
+	BaseRoutes.Public = BaseRoutes.ApiRoot.PathPrefix("/public").Subrouter()
 
 	InitUser()
 	InitTeam()

api/context.go

@@ -80,6 +80,10 @@ func ApiUserRequiredTrustRequester(h func(*Context, http.ResponseWriter, *http.R
 	return &handler{h, true, false, true, true, false, true}
 }
 
+func ApiAppHandlerTrustRequesterIndependent(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler {
+	return &handler{h, false, false, true, false, true, true}
+}
+
 type handler struct {
 	handleFunc         func(*Context, http.ResponseWriter, *http.Request)
 	requireUser        bool
@@ -187,7 +191,7 @@ func (h handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		c.SystemAdminRequired()
 	}
 
-	if c.Err == nil && len(c.TeamId) > 0 {
+	if c.Err == nil && len(c.TeamId) > 0 && !h.isTeamIndependent {
 		c.HasPermissionsToTeam(c.TeamId, "TeamRoute")
 	}
 
@@ -389,8 +393,13 @@ func (c *Context) RemoveSessionCookie(w http.ResponseWriter, r *http.Request) {
 }
 
 func (c *Context) SetInvalidParam(where string, name string) {
-	c.Err = model.NewLocAppError(where, "api.context.invalid_param.app_error", map[string]interface{}{"Name": name}, "")
+	c.Err = NewInvalidParamError(where, name)
-	c.Err.StatusCode = http.StatusBadRequest
+}
+
+func NewInvalidParamError(where string, name string) *model.AppError {
+	err := model.NewLocAppError(where, "api.context.invalid_param.app_error", map[string]interface{}{"Name": name}, "")
+	err.StatusCode = http.StatusBadRequest
+	return err
 }
 
 func (c *Context) SetUnknownError(where string, details string) {

api/file.go

@@ -61,10 +61,12 @@ func InitFile() {
 	l4g.Debug(utils.T("api.file.init.debug"))
 
 	BaseRoutes.Files.Handle("/upload", ApiUserRequired(uploadFile)).Methods("POST")
-	BaseRoutes.Files.Handle("/get/{channel_id:[A-Za-z0-9]+}/{user_id:[A-Za-z0-9]+}/{filename:([A-Za-z0-9]+/)?.+(\\.[A-Za-z0-9]{3,})?}", ApiAppHandlerTrustRequester(getFile)).Methods("GET")
+	BaseRoutes.Files.Handle("/get/{channel_id:[A-Za-z0-9]+}/{user_id:[A-Za-z0-9]+}/{filename:([A-Za-z0-9]+/)?.+(\\.[A-Za-z0-9]{3,})?}", ApiUserRequiredTrustRequester(getFile)).Methods("GET")
-	BaseRoutes.Files.Handle("/get_info/{channel_id:[A-Za-z0-9]+}/{user_id:[A-Za-z0-9]+}/{filename:([A-Za-z0-9]+/)?.+(\\.[A-Za-z0-9]{3,})?}", ApiAppHandler(getFileInfo)).Methods("GET")
+	BaseRoutes.Files.Handle("/get_info/{channel_id:[A-Za-z0-9]+}/{user_id:[A-Za-z0-9]+}/{filename:([A-Za-z0-9]+/)?.+(\\.[A-Za-z0-9]{3,})?}", ApiUserRequired(getFileInfo)).Methods("GET")
 	BaseRoutes.Files.Handle("/get_public_link", ApiUserRequired(getPublicLink)).Methods("POST")
 	BaseRoutes.Files.Handle("/get_export", ApiUserRequired(getExport)).Methods("GET")
+
+	BaseRoutes.Public.Handle("/files/get/{team_id:[A-Za-z0-9]+}/{channel_id:[A-Za-z0-9]+}/{user_id:[A-Za-z0-9]+}/{filename:([A-Za-z0-9]+/)?.+(\\.[A-Za-z0-9]{3,})?}", ApiAppHandlerTrustRequesterIndependent(getPublicFile)).Methods("GET")
 }
 
 func uploadFile(c *Context, w http.ResponseWriter, r *http.Request) {
@@ -349,72 +351,104 @@ func getFileInfo(c *Context, w http.ResponseWriter, r *http.Request) {
 }
 
 func getFile(c *Context, w http.ResponseWriter, r *http.Request) {
-	if len(utils.Cfg.FileSettings.DriverName) == 0 {
+	params := mux.Vars(r)
-		c.Err = model.NewLocAppError("uploadFile", "api.file.upload_file.storage.app_error", nil, "")
+
+	teamId := c.TeamId
+	channelId := params["channel_id"]
+	userId := params["user_id"]
+	filename := params["filename"]
+
+	if !c.HasPermissionsToChannel(Srv.Store.Channel().CheckPermissionsTo(teamId, channelId, userId), "getFile") {
+		return
+	}
+
+	if err, bytes := getFileData(teamId, channelId, userId, filename); err != nil {
+		c.Err = err
+		return
+	} else if err := writeFileResponse(filename, bytes, w, r); err != nil {
+		c.Err = err
+		return
+	}
+}
+
+func getPublicFile(c *Context, w http.ResponseWriter, r *http.Request) {
+	params := mux.Vars(r)
+
+	teamId := params["team_id"]
+	channelId := params["channel_id"]
+	userId := params["user_id"]
+	filename := params["filename"]
+
+	hash := r.URL.Query().Get("h")
+	data := r.URL.Query().Get("d")
+
+	if !utils.Cfg.FileSettings.EnablePublicLink {
+		c.Err = model.NewLocAppError("getPublicFile", "api.file.get_file.public_disabled.app_error", nil, "")
 		c.Err.StatusCode = http.StatusNotImplemented
 		return
 	}
 
-	params := mux.Vars(r)
+	if len(hash) > 0 && len(data) > 0 {
-
-	channelId := params["channel_id"]
-	if len(channelId) != 26 {
-		c.SetInvalidParam("getFile", "channel_id")
-		return
-	}
-
-	userId := params["user_id"]
-	if len(userId) != 26 {
-		c.SetInvalidParam("getFile", "user_id")
-		return
-	}
-
-	filename := params["filename"]
-	if len(filename) == 0 {
-		c.SetInvalidParam("getFile", "filename")
-		return
-	}
-
-	hash := r.URL.Query().Get("h")
-	data := r.URL.Query().Get("d")
-	teamId := r.URL.Query().Get("t")
-
-	cchan := Srv.Store.Channel().CheckPermissionsTo(c.TeamId, channelId, c.Session.UserId)
-
-	path := ""
-	if len(teamId) == 26 {
-		path = "teams/" + teamId + "/channels/" + channelId + "/users/" + userId + "/" + filename
-	} else {
-		path = "teams/" + c.TeamId + "/channels/" + channelId + "/users/" + userId + "/" + filename
-	}
-
-	fileData := make(chan []byte)
-	getFileAndForget(path, fileData)
-
-	if len(hash) > 0 && len(data) > 0 && len(teamId) == 26 {
-		if !utils.Cfg.FileSettings.EnablePublicLink {
-			c.Err = model.NewLocAppError("getFile", "api.file.get_file.public_disabled.app_error", nil, "")
-			return
-		}
-
 		if !model.ComparePassword(hash, fmt.Sprintf("%v:%v", data, utils.Cfg.FileSettings.PublicLinkSalt)) {
-			c.Err = model.NewLocAppError("getFile", "api.file.get_file.public_invalid.app_error", nil, "")
+			c.Err = model.NewLocAppError("getPublicFile", "api.file.get_file.public_invalid.app_error", nil, "")
+			c.Err.StatusCode = http.StatusBadRequest
 			return
 		}
-	} else if !c.HasPermissionsToChannel(cchan, "getFile") {
+	} else {
+		c.Err = model.NewLocAppError("getPublicFile", "api.file.get_file.public_invalid.app_error", nil, "")
+		c.Err.StatusCode = http.StatusBadRequest
 		return
 	}
 
-	f := <-fileData
+	if err, bytes := getFileData(teamId, channelId, userId, filename); err != nil {
-
+		c.Err = err
-	if f == nil {
+		return
-		c.Err = model.NewLocAppError("getFile", "api.file.get_file.not_found.app_error", nil, "path="+path)
+	} else if err := writeFileResponse(filename, bytes, w, r); err != nil {
-		c.Err.StatusCode = http.StatusNotFound
+		c.Err = err
 		return
 	}
+}
 
+func getFileData(teamId string, channelId string, userId string, filename string) (*model.AppError, []byte) {
+	if len(utils.Cfg.FileSettings.DriverName) == 0 {
+		err := model.NewLocAppError("getFileData", "api.file.upload_file.storage.app_error", nil, "")
+		err.StatusCode = http.StatusNotImplemented
+		return err, nil
+	}
+
+	if len(teamId) != 26 {
+		return NewInvalidParamError("getFileData", "team_id"), nil
+	}
+
+	if len(channelId) != 26 {
+		return NewInvalidParamError("getFileData", "channel_id"), nil
+	}
+
+	if len(userId) != 26 {
+		return NewInvalidParamError("getFileData", "user_id"), nil
+	}
+
+	if len(filename) == 0 {
+		return NewInvalidParamError("getFileData", "filename"), nil
+	}
+
+	path := "teams/" + teamId + "/channels/" + channelId + "/users/" + userId + "/" + filename
+
+	fileChan := make(chan []byte)
+	getFileAndForget(path, fileChan)
+
+	if bytes := <-fileChan; bytes == nil {
+		err := model.NewLocAppError("writeFileResponse", "api.file.get_file.not_found.app_error", nil, "path="+path)
+		err.StatusCode = http.StatusNotFound
+		return err, nil
+	} else {
+		return nil, bytes
+	}
+}
+
+func writeFileResponse(filename string, bytes []byte, w http.ResponseWriter, r *http.Request) *model.AppError {
 	w.Header().Set("Cache-Control", "max-age=2592000, public")
-	w.Header().Set("Content-Length", strconv.Itoa(len(f)))
+	w.Header().Set("Content-Length", strconv.Itoa(len(bytes)))
 	w.Header().Del("Content-Type") // Content-Type will be set automatically by the http writer
 
 	// attach extra headers to trigger a download on IE, Edge, and Safari
@@ -426,7 +460,6 @@ func getFile(c *Context, w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Content-Disposition", "attachment;filename=\""+filePart+"\"")
 
 	if bname == "Edge" || bname == "Internet Explorer" || bname == "Safari" {
-		// trim off anything before the final / so we just get the file's name
 		w.Header().Set("Content-Type", "application/octet-stream")
 	}
 
@@ -434,7 +467,9 @@ func getFile(c *Context, w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("X-Frame-Options", "DENY")
 	w.Header().Set("Content-Security-Policy", "Frame-ancestors 'none'")
 
-	w.Write(f)
+	w.Write(bytes)
+
+	return nil
 }
 
 func getFileAndForget(path string, fileData chan []byte) {
@@ -458,7 +493,7 @@ func getPublicLink(c *Context, w http.ResponseWriter, r *http.Request) {
 
 	if !utils.Cfg.FileSettings.EnablePublicLink {
 		c.Err = model.NewLocAppError("getPublicLink", "api.file.get_public_link.disabled.app_error", nil, "")
-		c.Err.StatusCode = http.StatusForbidden
+		c.Err.StatusCode = http.StatusNotImplemented
 		return
 	}
 
@@ -488,16 +523,13 @@ func getPublicLink(c *Context, w http.ResponseWriter, r *http.Request) {
 	data := model.MapToJson(newProps)
 	hash := model.HashPassword(fmt.Sprintf("%v:%v", data, utils.Cfg.FileSettings.PublicLinkSalt))
 
-	url := fmt.Sprintf("%s/files/get/%s/%s/%s?d=%s&h=%s&t=%s", c.GetSiteURL()+model.API_URL_SUFFIX, channelId, userId, filename, url.QueryEscape(data), url.QueryEscape(hash), c.TeamId)
+	url := fmt.Sprintf("%s/public/files/get/%s/%s/%s/%s?d=%s&h=%s", c.GetSiteURL()+model.API_URL_SUFFIX, c.TeamId, channelId, userId, filename, url.QueryEscape(data), url.QueryEscape(hash))
 
 	if !c.HasPermissionsToChannel(cchan, "getPublicLink") {
 		return
 	}
 
-	rData := make(map[string]string)
+	w.Write([]byte(url))
-	rData["public_link"] = url
-
-	w.Write([]byte(model.MapToJson(rData)))
 }
 
 func getExport(c *Context, w http.ResponseWriter, r *http.Request) {

api/file_benchmark_test.go

@@ -68,16 +68,13 @@ func BenchmarkGetPublicLink(b *testing.B) {
 		b.Fatal("Unable to upload file for benchmark")
 	}
 
-	data := make(map[string]string)
-	data["filename"] = filenames[0]
-
 	// wait a bit for files to ready
 	time.Sleep(5 * time.Second)
 
 	// Benchmark Start
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		if _, downErr := Client.GetPublicLink(data); downErr != nil {
+		if _, downErr := Client.GetPublicLink(filenames[0]); downErr != nil {
 			b.Fatal(downErr)
 		}
 	}

api/file_test.go

@@ -5,16 +5,15 @@ package api
 
 import (
 	"bytes"
+	"encoding/base64"
 	"fmt"
 	"github.com/goamz/goamz/aws"
 	"github.com/goamz/goamz/s3"
 	"github.com/mattermost/platform/model"
-	"github.com/mattermost/platform/store"
 	"github.com/mattermost/platform/utils"
 	"io"
 	"mime/multipart"
 	"net/http"
-	"net/url"
 	"os"
 	"strings"
 	"testing"
@@ -138,12 +137,6 @@ func TestGetFile(t *testing.T) {
 	user := th.BasicUser
 	channel := th.BasicChannel
 
-	enablePublicLink := utils.Cfg.FileSettings.EnablePublicLink
-	defer func() {
-		utils.Cfg.FileSettings.EnablePublicLink = enablePublicLink
-	}()
-	utils.Cfg.FileSettings.EnablePublicLink = true
-
 	if utils.Cfg.FileSettings.DriverName != "" {
 		body := &bytes.Buffer{}
 		writer := multipart.NewWriter(body)
@@ -202,60 +195,6 @@ func TestGetFile(t *testing.T) {
 			}
 		}
 
-		team2 := &model.Team{DisplayName: "Name", Name: "z-z-" + model.NewId() + "a", Email: "test@nowhere.com", Type: model.TEAM_OPEN}
-		team2 = Client.Must(Client.CreateTeam(team2)).Data.(*model.Team)
-
-		user2 := &model.User{Email: model.NewId() + "success+test@simulator.amazonses.com", Nickname: "Corey Hulen", Password: "pwd"}
-		user2 = Client.Must(Client.CreateUser(user2, "")).Data.(*model.User)
-		LinkUserToTeam(user2, team2)
-		store.Must(Srv.Store.User().VerifyEmail(user2.Id))
-
-		newProps := make(map[string]string)
-		newProps["filename"] = filenames[0]
-		newProps["time"] = fmt.Sprintf("%v", model.GetMillis())
-
-		data := model.MapToJson(newProps)
-		hash := model.HashPassword(fmt.Sprintf("%v:%v", data, utils.Cfg.FileSettings.PublicLinkSalt))
-
-		Client.Login(user2.Email, "pwd")
-		Client.SetTeamId(team2.Id)
-
-		if _, downErr := Client.GetFile(filenames[0]+"?d="+url.QueryEscape(data)+"&h="+url.QueryEscape(hash)+"&t="+team.Id, false); downErr != nil {
-			t.Fatal(downErr)
-		}
-
-		if _, downErr := Client.GetFile(filenames[0]+"?d="+url.QueryEscape(data)+"&h="+url.QueryEscape(hash), false); downErr == nil {
-			t.Fatal("Should have errored - missing team id")
-		}
-
-		if _, downErr := Client.GetFile(filenames[0]+"?d="+url.QueryEscape(data)+"&h="+url.QueryEscape(hash)+"&t=junk", false); downErr == nil {
-			t.Fatal("Should have errored - bad team id")
-		}
-
-		if _, downErr := Client.GetFile(filenames[0]+"?d="+url.QueryEscape(data)+"&h="+url.QueryEscape(hash)+"&t=12345678901234567890123456", false); downErr == nil {
-			t.Fatal("Should have errored - bad team id")
-		}
-
-		if _, downErr := Client.GetFile(filenames[0]+"?d="+url.QueryEscape(data)+"&t="+team.Id, false); downErr == nil {
-			t.Fatal("Should have errored - missing hash")
-		}
-
-		if _, downErr := Client.GetFile(filenames[0]+"?d="+url.QueryEscape(data)+"&h=junk&t="+team.Id, false); downErr == nil {
-			t.Fatal("Should have errored - bad hash")
-		}
-
-		if _, downErr := Client.GetFile(filenames[0]+"?h="+url.QueryEscape(hash)+"&t="+team.Id, false); downErr == nil {
-			t.Fatal("Should have errored - missing data")
-		}
-
-		if _, downErr := Client.GetFile(filenames[0]+"?d=junk&h="+url.QueryEscape(hash)+"&t="+team.Id, false); downErr == nil {
-			t.Fatal("Should have errored - bad data")
-		}
-
-		if _, downErr := Client.GetFile(filenames[0], true); downErr == nil {
-			t.Fatal("Should have errored - user not logged in and link not public")
-		}
-
 		if utils.Cfg.FileSettings.DriverName == model.IMAGE_DRIVER_S3 {
 			var auth aws.Auth
 			auth.AccessKey = utils.Cfg.FileSettings.AmazonS3AccessKeyId
@@ -309,95 +248,206 @@ func TestGetFile(t *testing.T) {
 	}
 }
 
-func TestGetPublicLink(t *testing.T) {
+func TestGetPublicFile(t *testing.T) {
 	th := Setup().InitBasic()
 	Client := th.BasicClient
-	team := th.BasicTeam
-	user := th.BasicUser
 	channel := th.BasicChannel
 
-	if utils.Cfg.FileSettings.DriverName != "" {
 	enablePublicLink := utils.Cfg.FileSettings.EnablePublicLink
+	driverName := utils.Cfg.FileSettings.DriverName
 	defer func() {
 		utils.Cfg.FileSettings.EnablePublicLink = enablePublicLink
+		utils.Cfg.FileSettings.DriverName = driverName
 	}()
 	utils.Cfg.FileSettings.EnablePublicLink = true
+	if driverName == "" {
+		driverName = model.IMAGE_DRIVER_LOCAL
+	}
 
-		body := &bytes.Buffer{}
+	filenames, err := uploadTestFile(Client, channel.Id)
-		writer := multipart.NewWriter(body)
-		part, err := writer.CreateFormFile("files", "test.png")
 	if err != nil {
-			t.Fatal(err)
+		t.Fatal("failed to upload test file", err)
 	}
 
-		path := utils.FindDir("tests")
-		file, err := os.Open(path + "/test.png")
-		if err != nil {
-			t.Fatal(err)
-		}
-		defer file.Close()
-
-		_, err = io.Copy(part, file)
-		if err != nil {
-			t.Fatal(err)
-		}
-
-		field, err := writer.CreateFormField("channel_id")
-		if err != nil {
-			t.Fatal(err)
-		}
-
-		_, err = field.Write([]byte(channel.Id))
-		if err != nil {
-			t.Fatal(err)
-		}
-
-		err = writer.Close()
-		if err != nil {
-			t.Fatal(err)
-		}
-
-		resp, upErr := Client.UploadPostAttachment(body.Bytes(), writer.FormDataContentType())
-		if upErr != nil {
-			t.Fatal(upErr)
-		}
-
-		filenames := resp.Data.(*model.FileUploadResponse).Filenames
-
 	post1 := &model.Post{ChannelId: channel.Id, Message: "a" + model.NewId() + "a", Filenames: filenames}
 
-		rpost1, postErr := Client.CreatePost(post1)
+	if rpost1, postErr := Client.CreatePost(post1); postErr != nil {
-		if postErr != nil {
 		t.Fatal(postErr)
+	} else {
+		post1 = rpost1.Data.(*model.Post)
 	}
 
-		if rpost1.Data.(*model.Post).Filenames[0] != filenames[0] {
+	var link string
-			t.Fatal("filenames don't match")
+	if result, err := Client.GetPublicLink(filenames[0]); err != nil {
+		t.Fatal("failed to get public link")
+	} else {
+		link = result.Data.(string)
 	}
 
-		// wait a bit for files to ready
+	// test a user that's logged in
-		time.Sleep(5 * time.Second)
+	if resp, err := http.Get(link); err != nil && resp.StatusCode != http.StatusOK {
-
+		t.Fatal("failed to get image with public link while logged in", err)
-		data := make(map[string]string)
-		data["filename"] = filenames[0]
-
-		if _, err := Client.GetPublicLink(data); err != nil {
-			t.Fatal(err)
 	}
 
-		data["filename"] = "junk"
+	if resp, err := http.Get(link[:strings.LastIndex(link, "?")]); err == nil && resp.StatusCode != http.StatusBadRequest {
+		t.Fatal("should've failed to get image with public link while logged in without query params", resp.Status)
+	}
 
-		if _, err := Client.GetPublicLink(data); err == nil {
+	if resp, err := http.Get(link[:strings.LastIndex(link, "&")]); err == nil && resp.StatusCode != http.StatusBadRequest {
-			t.Fatal("Should have errored - bad file path")
+		t.Fatal("should've failed to get image with public link while logged in without second query param")
+	}
+
+	if resp, err := http.Get(link[:strings.LastIndex(link, "?")] + "?" + link[strings.LastIndex(link, "&"):]); err == nil && resp.StatusCode != http.StatusBadRequest {
+		t.Fatal("should've failed to get image with public link while logged in without first query param")
+	}
+
+	utils.Cfg.FileSettings.EnablePublicLink = false
+	if resp, err := http.Get(link); err == nil && resp.StatusCode != http.StatusNotImplemented {
+		t.Fatal("should've failed to get image with disabled public link while logged in")
+	}
+
+	utils.Cfg.FileSettings.EnablePublicLink = true
+
+	// test a user that's logged out
+	Client.Must(Client.Logout())
+
+	if resp, err := http.Get(link); err != nil && resp.StatusCode != http.StatusOK {
+		t.Fatal("failed to get image with public link while not logged in", err)
+	}
+
+	if resp, err := http.Get(link[:strings.LastIndex(link, "?")]); err == nil && resp.StatusCode != http.StatusBadRequest {
+		t.Fatal("should've failed to get image with public link while not logged in without query params")
+	}
+
+	if resp, err := http.Get(link[:strings.LastIndex(link, "&")]); err == nil && resp.StatusCode != http.StatusBadRequest {
+		t.Fatal("should've failed to get image with public link while not logged in without second query param")
+	}
+
+	if resp, err := http.Get(link[:strings.LastIndex(link, "?")] + "?" + link[strings.LastIndex(link, "&"):]); err == nil && resp.StatusCode != http.StatusBadRequest {
+		t.Fatal("should've failed to get image with public link while not logged in without first query param")
+	}
+
+	utils.Cfg.FileSettings.EnablePublicLink = false
+	if resp, err := http.Get(link); err == nil && resp.StatusCode != http.StatusNotImplemented {
+		t.Fatal("should've failed to get image with disabled public link while not logged in")
+	}
+
+	utils.Cfg.FileSettings.EnablePublicLink = true
+
+	// test a user that's logged in after the salt has changed
+	utils.Cfg.FileSettings.PublicLinkSalt = model.NewId()
+
+	th.LoginBasic()
+	if resp, err := http.Get(link); err == nil && resp.StatusCode != http.StatusBadRequest {
+		t.Fatal("should've failed to get image with public link while logged in after salt changed")
+	}
+
+	Client.Must(Client.Logout())
+	if resp, err := http.Get(link); err == nil && resp.StatusCode != http.StatusBadRequest {
+		t.Fatal("should've failed to get image with public link while not logged in after salt changed")
+	}
+
+	if err := cleanupTestFile(filenames[0], th.BasicTeam.Id, channel.Id, th.BasicUser.Id); err != nil {
+		t.Fatal("failed to cleanup test file", err)
+	}
+}
+
+func TestGetPublicLink(t *testing.T) {
+	th := Setup().InitBasic()
+	Client := th.BasicClient
+	channel := th.BasicChannel
+
+	enablePublicLink := utils.Cfg.FileSettings.EnablePublicLink
+	driverName := utils.Cfg.FileSettings.DriverName
+	defer func() {
+		utils.Cfg.FileSettings.EnablePublicLink = enablePublicLink
+		utils.Cfg.FileSettings.DriverName = driverName
+	}()
+	if driverName == "" {
+		driverName = model.IMAGE_DRIVER_LOCAL
+	}
+
+	filenames, err := uploadTestFile(Client, channel.Id)
+	if err != nil {
+		t.Fatal("failed to upload test file", err)
+	}
+
+	post1 := &model.Post{ChannelId: channel.Id, Message: "a" + model.NewId() + "a", Filenames: filenames}
+
+	if rpost1, postErr := Client.CreatePost(post1); postErr != nil {
+		t.Fatal(postErr)
+	} else {
+		post1 = rpost1.Data.(*model.Post)
+	}
+
+	utils.Cfg.FileSettings.EnablePublicLink = false
+	if _, err := Client.GetPublicLink(filenames[0]); err == nil || err.StatusCode != http.StatusNotImplemented {
+		t.Fatal("should've failed when public links are disabled", err)
+	}
+
+	utils.Cfg.FileSettings.EnablePublicLink = true
+
+	if _, err := Client.GetPublicLink("garbage"); err == nil {
+		t.Fatal("should've failed for invalid link")
+	}
+
+	if _, err := Client.GetPublicLink(filenames[0]); err != nil {
+		t.Fatal("should've gotten link for file", err)
 	}
 
 	th.LoginBasic2()
 
-		data["filename"] = filenames[0]
+	if _, err := Client.GetPublicLink(filenames[0]); err == nil {
-		if _, err := Client.GetPublicLink(data); err == nil {
+		t.Fatal("should've failed, user not member of channel")
-			t.Fatal("should have errored, user not member of channel")
 	}
 
+	th.LoginBasic()
+
+	if err := cleanupTestFile(filenames[0], th.BasicTeam.Id, channel.Id, th.BasicUser.Id); err != nil {
+		t.Fatal("failed to cleanup test file", err)
+	}
+}
+
+func uploadTestFile(Client *model.Client, channelId string) ([]string, error) {
+	body := &bytes.Buffer{}
+	writer := multipart.NewWriter(body)
+	part, err := writer.CreateFormFile("files", "test.png")
+	if err != nil {
+		return nil, err
+	}
+
+	// base 64 encoded version of handtinywhite.gif from http://probablyprogramming.com/2009/03/15/the-tiniest-gif-ever
+	file, _ := base64.StdEncoding.DecodeString("R0lGODlhAQABAIABAP///wAAACwAAAAAAQABAAACAkQBADs=")
+
+	if _, err := io.Copy(part, bytes.NewReader(file)); err != nil {
+		return nil, err
+	}
+
+	field, err := writer.CreateFormField("channel_id")
+	if err != nil {
+		return nil, err
+	}
+
+	if _, err := field.Write([]byte(channelId)); err != nil {
+		return nil, err
+	}
+
+	if err := writer.Close(); err != nil {
+		return nil, err
+	}
+
+	if resp, err := Client.UploadPostAttachment(body.Bytes(), writer.FormDataContentType()); err != nil {
+		return nil, err
+	} else {
+		return resp.Data.(*model.FileUploadResponse).Filenames, nil
+	}
+}
+
+func cleanupTestFile(fullFilename, teamId, channelId, userId string) error {
+	filenames := strings.Split(fullFilename, "/")
+	filename := filenames[len(filenames)-2] + "/" + filenames[len(filenames)-1]
+	fileId := strings.Split(filename, ".")[0]
+
 	if utils.Cfg.FileSettings.DriverName == model.IMAGE_DRIVER_S3 {
 		// perform clean-up on s3
 		var auth aws.Auth
@@ -407,48 +457,33 @@ func TestGetPublicLink(t *testing.T) {
 		s := s3.New(auth, aws.Regions[utils.Cfg.FileSettings.AmazonS3Region])
 		bucket := s.Bucket(utils.Cfg.FileSettings.AmazonS3Bucket)
 
-			filenames := strings.Split(resp.Data.(*model.FileUploadResponse).Filenames[0], "/")
+		if err := bucket.Del("teams/" + teamId + "/channels/" + channelId + "/users/" + userId + "/" + filename); err != nil {
-			filename := filenames[len(filenames)-2] + "/" + filenames[len(filenames)-1]
+			return err
-			fileId := strings.Split(filename, ".")[0]
-
-			err = bucket.Del("teams/" + team.Id + "/channels/" + channel.Id + "/users/" + user.Id + "/" + filename)
-			if err != nil {
-				t.Fatal(err)
 		}
 
-			err = bucket.Del("teams/" + team.Id + "/channels/" + channel.Id + "/users/" + user.Id + "/" + fileId + "_thumb.jpg")
+		if err := bucket.Del("teams/" + teamId + "/channels/" + channelId + "/users/" + userId + "/" + fileId + "_thumb.jpg"); err != nil {
-			if err != nil {
+			return err
-				t.Fatal(err)
 		}
 
-			err = bucket.Del("teams/" + team.Id + "/channels/" + channel.Id + "/users/" + user.Id + "/" + fileId + "_preview.jpg")
+		if err := bucket.Del("teams/" + teamId + "/channels/" + channelId + "/users/" + userId + "/" + fileId + "_preview.jpg"); err != nil {
-			if err != nil {
+			return err
-				t.Fatal(err)
 		}
 	} else {
-			filenames := strings.Split(resp.Data.(*model.FileUploadResponse).Filenames[0], "/")
+		path := utils.Cfg.FileSettings.Directory + "teams/" + teamId + "/channels/" + channelId + "/users/" + userId + "/" + filename
-			filename := filenames[len(filenames)-2] + "/" + filenames[len(filenames)-1]
-			fileId := strings.Split(filename, ".")[0]
-
-			path := utils.Cfg.FileSettings.Directory + "teams/" + team.Id + "/channels/" + channel.Id + "/users/" + user.Id + "/" + filename
 		if err := os.Remove(path); err != nil {
-				t.Fatal("Couldn't remove file at " + path)
+			return fmt.Errorf("Couldn't remove file at " + path)
 		}
 
-			path = utils.Cfg.FileSettings.Directory + "teams/" + team.Id + "/channels/" + channel.Id + "/users/" + user.Id + "/" + fileId + "_thumb.jpg"
+		path = utils.Cfg.FileSettings.Directory + "teams/" + teamId + "/channels/" + channelId + "/users/" + userId + "/" + fileId + "_thumb.jpg"
 		if err := os.Remove(path); err != nil {
-				t.Fatal("Couldn't remove file at " + path)
+			return fmt.Errorf("Couldn't remove file at " + path)
 		}
 
-			path = utils.Cfg.FileSettings.Directory + "teams/" + team.Id + "/channels/" + channel.Id + "/users/" + user.Id + "/" + fileId + "_preview.jpg"
+		path = utils.Cfg.FileSettings.Directory + "teams/" + teamId + "/channels/" + channelId + "/users/" + userId + "/" + fileId + "_preview.jpg"
 		if err := os.Remove(path); err != nil {
-				t.Fatal("Couldn't remove file at " + path)
+			return fmt.Errorf("Couldn't remove file at " + path)
-			}
-		}
-	} else {
-		data := make(map[string]string)
-		if _, err := Client.GetPublicLink(data); err.StatusCode != http.StatusNotImplemented {
-			t.Fatal("Status code should have been 501 - Not Implemented")
 		}
 	}
+
+	return nil
 }

model/client.go

@@ -993,12 +993,19 @@ func (c *Client) GetFileInfo(url string) (*Result, *AppError) {
 	}
 }
 
-func (c *Client) GetPublicLink(data map[string]string) (*Result, *AppError) {
+func (c *Client) GetPublicLink(filename string) (*Result, *AppError) {
-	if r, err := c.DoApiPost(c.GetTeamRoute()+"/files/get_public_link", MapToJson(data)); err != nil {
+	if r, err := c.DoApiPost(c.GetTeamRoute()+"/files/get_public_link", MapToJson(map[string]string{"filename": filename})); err != nil {
 		return nil, err
 	} else {
+		var link string
+		if body, err := ioutil.ReadAll(r.Body); err == nil {
+			link = string(body)
+		} else {
+			// all the other Client methods return an empty string on invalid json, so we can too
+		}
+
 		return &Result{r.Header.Get(HEADER_REQUEST_ID),
-			r.Header.Get(HEADER_ETAG_SERVER), MapFromJson(r.Body)}, nil
+			r.Header.Get(HEADER_ETAG_SERVER), link}, nil
 	}
 }
 

webapp/action_creators/global_actions.jsx

@@ -281,6 +281,16 @@ export function showGetPostLinkModal(post) {
     });
 }
 
+export function showGetPublicLinkModal(channelId, userId, filename) {
+    AppDispatcher.handleViewAction({
+        type: ActionTypes.TOGGLE_GET_PUBLIC_LINK_MODAL,
+        value: true,
+        channelId,
+        userId,
+        filename
+    });
+}
+
 export function showGetTeamInviteLinkModal() {
     AppDispatcher.handleViewAction({
         type: Constants.ActionTypes.TOGGLE_GET_TEAM_INVITE_LINK_MODAL,

webapp/client/client.jsx

@@ -1325,7 +1325,13 @@ export default class Client {
             end(this.handleResponse.bind(this, 'getFileInfo', success, error));
     }
 
-    getPublicLink = (data, success, error) => {
+    getPublicLink = (channelId, userId, filename, success, error) => {
+        const data = {
+            channel_id: channelId,
+            user_id: userId,
+            filename
+        };
+
         request.
             post(`${this.getFilesRoute()}/get_public_link`).
             set(this.defaultHeaders).

webapp/components/get_public_link_modal.jsx

@@ -0,0 +1,80 @@
+// Copyright (c) 2015 Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+import React from 'react';
+
+import * as AsyncClient from 'utils/async_client.jsx';
+import Constants from 'utils/constants.jsx';
+import ModalStore from 'stores/modal_store.jsx';
+import PureRenderMixin from 'react-addons-pure-render-mixin';
+import * as Utils from 'utils/utils.jsx';
+
+import GetLinkModal from './get_link_modal.jsx';
+
+export default class GetPublicLinkModal extends React.Component {
+    constructor(props) {
+        super(props);
+
+        this.handlePublicLink = this.handlePublicLink.bind(this);
+        this.handleToggle = this.handleToggle.bind(this);
+        this.hide = this.hide.bind(this);
+
+        this.shouldComponentUpdate = PureRenderMixin.shouldComponentUpdate.bind(this);
+
+        this.state = {
+            show: false,
+            channelId: '',
+            userId: '',
+            filename: '',
+            link: ''
+        };
+    }
+
+    componentDidMount() {
+        ModalStore.addModalListener(Constants.ActionTypes.TOGGLE_GET_PUBLIC_LINK_MODAL, this.handleToggle);
+    }
+
+    componentDidUpdate(prevProps, prevState) {
+        if (this.state.show && !prevState.show) {
+            AsyncClient.getPublicLink(this.state.channelId, this.state.userId, this.state.filename, this.handlePublicLink);
+        }
+    }
+
+    componentWillUnmount() {
+        ModalStore.removeModalListener(Constants.ActionTypes.TOGGLE_GET_PUBLIC_LINK_MODAL, this.handleToggle);
+    }
+
+    handlePublicLink(link) {
+        this.setState({
+            link
+        });
+    }
+
+    handleToggle(value, args) {
+        this.setState({
+            show: value,
+            channelId: args.channelId,
+            userId: args.userId,
+            filename: args.filename,
+            link: ''
+        });
+    }
+
+    hide() {
+        this.setState({
+            show: false
+        });
+    }
+
+    render() {
+        return (
+            <GetLinkModal
+                show={this.state.show}
+                onHide={this.hide}
+                title={Utils.localizeMessage('get_public_link_modal.title', 'Copy Public Link')}
+                helpText={Utils.localizeMessage('get_public_link_modal.help', 'The link below allows anyone to see this file without being registered on this server.')}
+                link={this.state.link}
+            />
+        );
+    }
+}

webapp/components/needs_team.jsx

@@ -24,6 +24,7 @@ import Navbar from 'components/navbar.jsx';
 
 // Modals
 import GetPostLinkModal from 'components/get_post_link_modal.jsx';
+import GetPublicLinkModal from 'components/get_public_link_modal.jsx';
 import GetTeamInviteLinkModal from 'components/get_team_invite_link_modal.jsx';
 import EditPostModal from 'components/edit_post_modal.jsx';
 import DeletePostModal from 'components/delete_post_modal.jsx';
@@ -125,6 +126,7 @@ export default class NeedsTeam extends React.Component {
                     {content}
 
                     <GetPostLinkModal/>
+                    <GetPublicLinkModal/>
                     <GetTeamInviteLinkModal/>
                     <InviteMemberModal/>
                     <ImportThemeModal/>

webapp/components/view_image.jsx

@@ -3,7 +3,7 @@
 
 import $ from 'jquery';
 import * as AsyncClient from 'utils/async_client.jsx';
-import Client from 'utils/web_client.jsx';
+import * as GlobalActions from 'action_creators/global_actions.jsx';
 import * as Utils from 'utils/utils.jsx';
 import AudioVideoPreview from './audio_video_preview.jsx';
 import Constants from 'utils/constants.jsx';
@@ -43,7 +43,7 @@ class ViewImageModal extends React.Component {
 
         this.onFileStoreChange = this.onFileStoreChange.bind(this);
 
-        this.getPublicLink = this.getPublicLink.bind(this);
+        this.handleGetPublicLink = this.handleGetPublicLink.bind(this);
         this.onMouseEnterImage = this.onMouseEnterImage.bind(this);
         this.onMouseLeaveImage = this.onMouseLeaveImage.bind(this);
 
@@ -194,24 +194,10 @@ class ViewImageModal extends React.Component {
         }
     }
 
-    getPublicLink() {
+    handleGetPublicLink() {
-        var data = {};
+        this.props.onModalDismissed();
-        data.channel_id = this.props.channelId;
+
-        data.user_id = this.props.userId;
+        GlobalActions.showGetPublicLinkModal(this.props.channelId, this.props.userId, this.props.filenames[this.state.imgId]);
-        data.filename = this.props.filenames[this.state.imgId];
-        Client.getPublicLink(
-            data,
-            (serverData) => {
-                if (Utils.isMobile()) {
-                    window.location.href = serverData.public_link;
-                } else {
-                    window.open(serverData.public_link);
-                }
-            },
-            () => {
-                //Do Nothing on error
-            }
-        );
     }
 
     onMouseEnterImage() {
@@ -349,7 +335,7 @@ class ViewImageModal extends React.Component {
                                 totalFiles={this.props.filenames.length}
                                 filename={name}
                                 fileURL={fileUrl}
-                                getPublicLink={this.getPublicLink}
+                                onGetPublicLink={this.handleGetPublicLink}
                             />
                         </div>
                     </div>

webapp/components/view_image_popover_bar.jsx

@@ -15,7 +15,7 @@ export default class ViewImagePopoverBar extends React.Component {
                         href='#'
                         className='public-link text'
                         data-title='Public Image'
-                        onClick={this.props.getPublicLink}
+                        onClick={this.props.onGetPublicLink}
                     >
                         <FormattedMessage
                             id='view_image_popover.publicLink'
@@ -79,5 +79,5 @@ ViewImagePopoverBar.propTypes = {
     totalFiles: React.PropTypes.number.isRequired,
     filename: React.PropTypes.string.isRequired,
     fileURL: React.PropTypes.string.isRequired,
-    getPublicLink: React.PropTypes.func.isRequired
+    onGetPublicLink: React.PropTypes.func.isRequired
 };

webapp/stores/modal_store.jsx

@@ -37,6 +37,7 @@ class ModalStoreClass extends EventEmitter {
         case ActionTypes.TOGGLE_GET_POST_LINK_MODAL:
         case ActionTypes.TOGGLE_GET_TEAM_INVITE_LINK_MODAL:
         case ActionTypes.TOGGLE_REGISTER_APP_MODAL:
+        case ActionTypes.TOGGLE_GET_PUBLIC_LINK_MODAL:
             this.emit(type, value, args);
             break;
         }

webapp/utils/async_client.jsx

@@ -1343,3 +1343,33 @@ export function regenCommandToken(id) {
         }
     );
 }
+
+export function getPublicLink(channelId, userId, filename, success, error) {
+    const callName = 'getPublicLink' + channelId + userId + filename;
+
+    if (isCallInProgress(callName)) {
+        return;
+    }
+
+    callTracker[callName] = utils.getTimestamp();
+
+    Client.getPublicLink(
+        channelId,
+        userId,
+        filename,
+        (link) => {
+            callTracker[callName] = 0;
+
+            success(link);
+        },
+        (err) => {
+            callTracker[callName] = 0;
+
+            if (error) {
+                error(err);
+            } else {
+                dispatchError(err, 'getPublicLink');
+            }
+        }
+    );
+}

webapp/utils/constants.jsx

@@ -110,6 +110,7 @@ export default {
         TOGGLE_GET_POST_LINK_MODAL: null,
         TOGGLE_GET_TEAM_INVITE_LINK_MODAL: null,
         TOGGLE_REGISTER_APP_MODAL: null,
+        TOGGLE_GET_PUBLIC_LINK_MODAL: null,
 
         SUGGESTION_PRETEXT_CHANGED: null,
         SUGGESTION_RECEIVED_SUGGESTIONS: null,