MM-10417 Add local image proxy and enable by default (#9967)

* MM-10417 Add local image proxy and enable by default

* Remove unused function

* Add dependencies for willnorris/imageproxy

* Fixed compilation errors

* Lock to the master version of willnorris/imageproxy

* Fix atmos/camo proxy when no SiteURL is specified

* Re-add default values for deprecated settings

* Fix unit tests added by merge

* Pass imageproxy to App struct

* Remove unneeded locking when creating the image proxy

* Remove empty test file

app/app.go

@@ -14,6 +14,7 @@ import (
 	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/services/httpservice"
+	"github.com/mattermost/mattermost-server/services/imageproxy"
 	"github.com/mattermost/mattermost-server/services/timezones"
 	"github.com/mattermost/mattermost-server/utils"
 	goi18n "github.com/nicksnyder/go-i18n/i18n"
@@ -43,6 +44,7 @@ type App struct {
 	Saml             einterfaces.SamlInterface
 
 	HTTPService httpservice.HTTPService
+	ImageProxy  *imageproxy.ImageProxy
 	Timezones   *timezones.Timezones
 }
 

app/diagnostics.go

@@ -45,6 +45,7 @@ const (
 	TRACK_CONFIG_MESSAGE_EXPORT     = "config_message_export"
 	TRACK_CONFIG_DISPLAY            = "config_display"
 	TRACK_CONFIG_TIMEZONE           = "config_timezone"
+	TRACK_CONFIG_IMAGE_PROXY        = "config_image_proxy"
 	TRACK_PERMISSIONS_GENERAL       = "permissions_general"
 	TRACK_PERMISSIONS_SYSTEM_SCHEME = "permissions_system_scheme"
 	TRACK_PERMISSIONS_TEAM_SCHEMES  = "permissions_team_schemes"
@@ -255,9 +256,6 @@ func (a *App) trackConfig() {
 		"enable_tutorial":                                         *cfg.ServiceSettings.EnableTutorial,
 		"experimental_enable_default_channel_leave_join_messages": *cfg.ServiceSettings.ExperimentalEnableDefaultChannelLeaveJoinMessages,
 		"experimental_group_unread_channels":                      *cfg.ServiceSettings.ExperimentalGroupUnreadChannels,
-		"isdefault_image_proxy_type":                              isDefault(*cfg.ServiceSettings.ImageProxyType, ""),
-		"isdefault_image_proxy_url":                               isDefault(*cfg.ServiceSettings.ImageProxyURL, ""),
-		"isdefault_image_proxy_options":                           isDefault(*cfg.ServiceSettings.ImageProxyOptions, ""),
 		"websocket_url":                                           isDefault(*cfg.ServiceSettings.WebsocketURL, ""),
 		"allow_cookies_for_subdomains":                            *cfg.ServiceSettings.AllowCookiesForSubdomains,
 		"enable_api_team_deletion":                                *cfg.ServiceSettings.EnableAPITeamDeletion,
@@ -561,6 +559,13 @@ func (a *App) trackConfig() {
 	a.SendDiagnostic(TRACK_CONFIG_TIMEZONE, map[string]interface{}{
 		"isdefault_supported_timezones_path": isDefault(*cfg.TimezoneSettings.SupportedTimezonesPath, model.TIMEZONE_SETTINGS_DEFAULT_SUPPORTED_TIMEZONES_PATH),
 	})
+
+	a.SendDiagnostic(TRACK_CONFIG_IMAGE_PROXY, map[string]interface{}{
+		"enable":                               *cfg.ImageProxySettings.Enable,
+		"image_proxy_type":                     *cfg.ImageProxySettings.ImageProxyType,
+		"isdefault_remote_image_proxy_url":     isDefault(*cfg.ImageProxySettings.RemoteImageProxyURL, ""),
+		"isdefault_remote_image_proxy_options": isDefault(*cfg.ImageProxySettings.RemoteImageProxyOptions, ""),
+	})
 }
 
 func (a *App) trackLicense() {

app/options.go

@@ -76,6 +76,7 @@ func ServerConnector(s *Server) AppOption {
 		a.Saml = s.Saml
 
 		a.HTTPService = s.HTTPService
+		a.ImageProxy = s.ImageProxy
 		a.Timezones = s.timezones
 	}
 }

app/post.go

@@ -4,9 +4,6 @@
 package app
 
 import (
-	"crypto/hmac"
-	"crypto/sha1"
-	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"net/http"
@@ -916,78 +913,23 @@ func (a *App) PostPatchWithProxyRemovedFromImageURLs(patch *model.PostPatch) *mo
 	return patch
 }
 
-func (a *App) imageProxyConfig() (proxyType, proxyURL, options, siteURL string) {
-	cfg := a.Config()
-
-	if cfg.ServiceSettings.ImageProxyURL == nil || cfg.ServiceSettings.ImageProxyType == nil || cfg.ServiceSettings.SiteURL == nil {
-		return
-	}
-
-	proxyURL = *cfg.ServiceSettings.ImageProxyURL
-	proxyType = *cfg.ServiceSettings.ImageProxyType
-	siteURL = *cfg.ServiceSettings.SiteURL
-
-	if proxyURL == "" || proxyType == "" {
-		return "", "", "", ""
-	}
-
-	if proxyURL[len(proxyURL)-1] != '/' {
-		proxyURL += "/"
-	}
-
-	if siteURL == "" || siteURL[len(siteURL)-1] != '/' {
-		siteURL += "/"
-	}
-
-	if cfg.ServiceSettings.ImageProxyOptions != nil {
-		options = *cfg.ServiceSettings.ImageProxyOptions
-	}
-
-	return
-}
-
 func (a *App) ImageProxyAdder() func(string) string {
-	proxyType, proxyURL, options, siteURL := a.imageProxyConfig()
-	if proxyType == "" {
+	if !*a.Config().ImageProxySettings.Enable {
 		return nil
 	}
 
 	return func(url string) string {
-		if url == "" || url[0] == '/' || strings.HasPrefix(url, siteURL) || strings.HasPrefix(url, proxyURL) {
-			return url
-		}
-
-		switch proxyType {
-		case "atmos/camo":
-			mac := hmac.New(sha1.New, []byte(options))
-			mac.Write([]byte(url))
-			digest := hex.EncodeToString(mac.Sum(nil))
-			return proxyURL + digest + "/" + hex.EncodeToString([]byte(url))
-		}
-
-		return url
+		return a.Srv.ImageProxy.GetProxiedImageURL(url)
 	}
 }
 
 func (a *App) ImageProxyRemover() (f func(string) string) {
-	proxyType, proxyURL, _, _ := a.imageProxyConfig()
-	if proxyType == "" {
+	if !*a.Config().ImageProxySettings.Enable {
 		return nil
 	}
 
 	return func(url string) string {
-		switch proxyType {
-		case "atmos/camo":
-			if strings.HasPrefix(url, proxyURL) {
-				if slash := strings.IndexByte(url[len(proxyURL):], '/'); slash >= 0 {
-					if decoded, err := hex.DecodeString(url[len(proxyURL)+slash+1:]); err == nil {
-						return string(decoded)
-					}
-				}
-			}
-		}
-
-		return url
+		return a.Srv.ImageProxy.GetUnproxiedImageURL(url)
 	}
 }
 

app/post_metadata.go

@@ -25,8 +25,10 @@ var linkCache = utils.NewLru(LINK_CACHE_SIZE)
 func (a *App) InitPostMetadata() {
 	// Dump any cached links if the proxy settings have changed so image URLs can be updated
 	a.AddConfigListener(func(before, after *model.Config) {
-		if (before.ServiceSettings.ImageProxyType != after.ServiceSettings.ImageProxyType) ||
-			(before.ServiceSettings.ImageProxyURL != after.ServiceSettings.ImageProxyURL) {
+		if (before.ImageProxySettings.Enable != after.ImageProxySettings.Enable) ||
+			(before.ImageProxySettings.ImageProxyType != after.ImageProxySettings.ImageProxyType) ||
+			(before.ImageProxySettings.RemoteImageProxyURL != after.ImageProxySettings.RemoteImageProxyURL) ||
+			(before.ImageProxySettings.RemoteImageProxyOptions != after.ImageProxySettings.RemoteImageProxyOptions) {
 			linkCache.Purge()
 		}
 	})

app/post_metadata_test.go

@@ -58,9 +58,7 @@ func TestPreparePostForClient(t *testing.T) {
 		th := Setup().InitBasic()
 
 		th.App.UpdateConfig(func(cfg *model.Config) {
-			*cfg.ServiceSettings.ImageProxyType = ""
-			*cfg.ServiceSettings.ImageProxyURL = ""
-			*cfg.ServiceSettings.ImageProxyOptions = ""
+			*cfg.ImageProxySettings.Enable = false
 			*cfg.ExperimentalSettings.EnablePostMetadata = true
 		})
 
@@ -248,7 +246,7 @@ func TestPreparePostForClient(t *testing.T) {
 
 		t.Run("populates image dimensions", func(t *testing.T) {
 			imageDimensions := clientPost.Metadata.Images
-			assert.Len(t, imageDimensions, 2)
+			require.Len(t, imageDimensions, 2)
 			assert.Equal(t, &model.PostImage{
 				Width:  1068,
 				Height: 552,
@@ -301,7 +299,7 @@ func TestPreparePostForClient(t *testing.T) {
 
 		t.Run("populates image dimensions", func(t *testing.T) {
 			imageDimensions := clientPost.Metadata.Images
-			assert.Len(t, imageDimensions, 1)
+			require.Len(t, imageDimensions, 1)
 			assert.Equal(t, &model.PostImage{
 				Width:  1068,
 				Height: 552,
@@ -345,7 +343,7 @@ func TestPreparePostForClient(t *testing.T) {
 
 		t.Run("populates image dimensions", func(t *testing.T) {
 			imageDimensions := clientPost.Metadata.Images
-			assert.Len(t, imageDimensions, 1)
+			require.Len(t, imageDimensions, 1)
 			assert.Equal(t, &model.PostImage{
 				Width:  420,
 				Height: 420,
@@ -382,7 +380,7 @@ func TestPreparePostForClient(t *testing.T) {
 
 		t.Run("populates image dimensions", func(t *testing.T) {
 			imageDimensions := clientPost.Metadata.Images
-			assert.Len(t, imageDimensions, 1)
+			require.Len(t, imageDimensions, 1)
 			assert.Equal(t, &model.PostImage{
 				Width:  501,
 				Height: 501,
@@ -415,9 +413,10 @@ func TestPreparePostForClientWithImageProxy(t *testing.T) {
 
 		th.App.UpdateConfig(func(cfg *model.Config) {
 			*cfg.ServiceSettings.SiteURL = "http://mymattermost.com"
-			*cfg.ServiceSettings.ImageProxyType = "atmos/camo"
-			*cfg.ServiceSettings.ImageProxyURL = "https://127.0.0.1"
-			*cfg.ServiceSettings.ImageProxyOptions = "foo"
+			*cfg.ImageProxySettings.Enable = true
+			*cfg.ImageProxySettings.ImageProxyType = "atmos/camo"
+			*cfg.ImageProxySettings.RemoteImageProxyURL = "https://127.0.0.1"
+			*cfg.ImageProxySettings.RemoteImageProxyOptions = "foo"
 			*cfg.ExperimentalSettings.EnablePostMetadata = true
 		})
 
@@ -453,10 +452,10 @@ func testProxyLinkedImage(t *testing.T, th *TestHelper, shouldProxy bool) {
 	clientPost := th.App.PreparePostForClient(post)
 
 	if shouldProxy {
-		assert.Equal(t, post.Message, fmt.Sprintf(postTemplate, imageURL), "should not have mutated original post")
-		assert.Equal(t, clientPost.Message, fmt.Sprintf(postTemplate, proxiedImageURL), "should've replaced linked image URLs")
+		assert.Equal(t, fmt.Sprintf(postTemplate, imageURL), post.Message, "should not have mutated original post")
+		assert.Equal(t, fmt.Sprintf(postTemplate, proxiedImageURL), clientPost.Message, "should've replaced linked image URLs")
 	} else {
-		assert.Equal(t, clientPost.Message, fmt.Sprintf(postTemplate, imageURL), "shouldn't have replaced linked image URLs")
+		assert.Equal(t, fmt.Sprintf(postTemplate, imageURL), clientPost.Message, "shouldn't have replaced linked image URLs")
 	}
 }
 
@@ -468,29 +467,27 @@ func testProxyOpenGraphImage(t *testing.T, th *TestHelper, shouldProxy bool) {
 	}, th.BasicChannel, false)
 	require.Nil(t, err)
 
-	clientPost := th.App.PreparePostForClient(post)
+	embeds := th.App.PreparePostForClient(post).Metadata.Embeds
+	require.Len(t, embeds, 1, "should have one embed")
 
-	image := &opengraph.Image{}
+	embed := embeds[0]
+	assert.Equal(t, model.POST_EMBED_OPENGRAPH, embed.Type, "embed type should be OpenGraph")
+	assert.Equal(t, "https://github.com/hmhealey/test-files", embed.URL, "embed URL should be correct")
+
+	og, ok := embed.Data.(*opengraph.OpenGraph)
+	assert.Equal(t, true, ok, "data should be non-nil OpenGraph data")
+	assert.Equal(t, "GitHub", og.SiteName, "OpenGraph data should be correctly populated")
+
+	require.Len(t, og.Images, 1, "OpenGraph data should have one image")
+
+	image := og.Images[0]
 	if shouldProxy {
-		image.SecureURL = "https://127.0.0.1/b2ef6ef4890a0107aa80ba33b3011fd51f668303/68747470733a2f2f61766174617273312e67697468756275736572636f6e74656e742e636f6d2f752f333237373331303f733d34303026763d34"
+		assert.Equal(t, "", image.URL, "image URL should not be set with proxy")
+		assert.Equal(t, "https://127.0.0.1/b2ef6ef4890a0107aa80ba33b3011fd51f668303/68747470733a2f2f61766174617273312e67697468756275736572636f6e74656e742e636f6d2f752f333237373331303f733d34303026763d34", image.SecureURL, "secure image URL should be sent through proxy")
 	} else {
-		image.URL = "https://avatars1.githubusercontent.com/u/3277310?s=400&v=4"
+		assert.Equal(t, "https://avatars1.githubusercontent.com/u/3277310?s=400&v=4", image.URL, "image URL should be set")
+		assert.Equal(t, "", image.SecureURL, "secure image URL should not be set")
 	}
-
-	assert.ElementsMatch(t, []*model.PostEmbed{
-		{
-			Type: model.POST_EMBED_OPENGRAPH,
-			URL:  "https://github.com/hmhealey/test-files",
-			Data: &opengraph.OpenGraph{
-				Description: "Contribute to hmhealey/test-files development by creating an account on GitHub.",
-				SiteName:    "GitHub",
-				Title:       "hmhealey/test-files",
-				Type:        "object",
-				URL:         "https://github.com/hmhealey/test-files",
-				Images:      []*opengraph.Image{image},
-			},
-		},
-	}, clientPost.Metadata.Embeds)
 }
 
 func TestGetEmojiNamesForString(t *testing.T) {

app/post_test.go

@@ -406,39 +406,60 @@ func TestImageProxy(t *testing.T) {
 		ProxiedImageURL string
 	}{
 		"atmos/camo": {
-			ProxyType:       "atmos/camo",
+			ProxyType:       model.IMAGE_PROXY_TYPE_ATMOS_CAMO,
 			ProxyURL:        "https://127.0.0.1",
 			ProxyOptions:    "foo",
 			ImageURL:        "http://mydomain.com/myimage",
 			ProxiedImageURL: "https://127.0.0.1/f8dace906d23689e8d5b12c3cefbedbf7b9b72f5/687474703a2f2f6d79646f6d61696e2e636f6d2f6d79696d616765",
 		},
 		"atmos/camo_SameSite": {
-			ProxyType:       "atmos/camo",
+			ProxyType:       model.IMAGE_PROXY_TYPE_ATMOS_CAMO,
 			ProxyURL:        "https://127.0.0.1",
 			ProxyOptions:    "foo",
 			ImageURL:        "http://mymattermost.com/myimage",
 			ProxiedImageURL: "http://mymattermost.com/myimage",
 		},
 		"atmos/camo_PathOnly": {
-			ProxyType:       "atmos/camo",
+			ProxyType:       model.IMAGE_PROXY_TYPE_ATMOS_CAMO,
 			ProxyURL:        "https://127.0.0.1",
 			ProxyOptions:    "foo",
 			ImageURL:        "/myimage",
 			ProxiedImageURL: "/myimage",
 		},
 		"atmos/camo_EmptyImageURL": {
-			ProxyType:       "atmos/camo",
+			ProxyType:       model.IMAGE_PROXY_TYPE_ATMOS_CAMO,
 			ProxyURL:        "https://127.0.0.1",
 			ProxyOptions:    "foo",
 			ImageURL:        "",
 			ProxiedImageURL: "",
 		},
+		"local": {
+			ProxyType:       model.IMAGE_PROXY_TYPE_LOCAL,
+			ImageURL:        "http://mydomain.com/myimage",
+			ProxiedImageURL: "http://mymattermost.com/api/v4/image?url=http%3A%2F%2Fmydomain.com%2Fmyimage",
+		},
+		"local_SameSite": {
+			ProxyType:       model.IMAGE_PROXY_TYPE_LOCAL,
+			ImageURL:        "http://mymattermost.com/myimage",
+			ProxiedImageURL: "http://mymattermost.com/myimage",
+		},
+		"local_PathOnly": {
+			ProxyType:       model.IMAGE_PROXY_TYPE_LOCAL,
+			ImageURL:        "/myimage",
+			ProxiedImageURL: "/myimage",
+		},
+		"local_EmptyImageURL": {
+			ProxyType:       model.IMAGE_PROXY_TYPE_LOCAL,
+			ImageURL:        "",
+			ProxiedImageURL: "",
+		},
 	} {
 		t.Run(name, func(t *testing.T) {
 			th.App.UpdateConfig(func(cfg *model.Config) {
-				cfg.ServiceSettings.ImageProxyType = model.NewString(tc.ProxyType)
-				cfg.ServiceSettings.ImageProxyOptions = model.NewString(tc.ProxyOptions)
-				cfg.ServiceSettings.ImageProxyURL = model.NewString(tc.ProxyURL)
+				cfg.ImageProxySettings.Enable = model.NewBool(true)
+				cfg.ImageProxySettings.ImageProxyType = model.NewString(tc.ProxyType)
+				cfg.ImageProxySettings.RemoteImageProxyOptions = model.NewString(tc.ProxyOptions)
+				cfg.ImageProxySettings.RemoteImageProxyURL = model.NewString(tc.ProxyURL)
 			})
 
 			post := &model.Post{
@@ -574,9 +595,10 @@ func TestCreatePost(t *testing.T) {
 
 		th.App.UpdateConfig(func(cfg *model.Config) {
 			*cfg.ExperimentalSettings.EnablePostMetadata = false
-			*cfg.ServiceSettings.ImageProxyType = "atmos/camo"
-			*cfg.ServiceSettings.ImageProxyURL = "https://127.0.0.1"
-			*cfg.ServiceSettings.ImageProxyOptions = "foo"
+			*cfg.ImageProxySettings.Enable = true
+			*cfg.ImageProxySettings.ImageProxyType = "atmos/camo"
+			*cfg.ImageProxySettings.RemoteImageProxyURL = "https://127.0.0.1"
+			*cfg.ImageProxySettings.RemoteImageProxyOptions = "foo"
 		})
 
 		imageURL := "http://mydomain.com/myimage"
@@ -601,9 +623,10 @@ func TestPatchPost(t *testing.T) {
 
 		th.App.UpdateConfig(func(cfg *model.Config) {
 			*cfg.ExperimentalSettings.EnablePostMetadata = false
-			*cfg.ServiceSettings.ImageProxyType = "atmos/camo"
-			*cfg.ServiceSettings.ImageProxyURL = "https://127.0.0.1"
-			*cfg.ServiceSettings.ImageProxyOptions = "foo"
+			*cfg.ImageProxySettings.Enable = true
+			*cfg.ImageProxySettings.ImageProxyType = "atmos/camo"
+			*cfg.ImageProxySettings.RemoteImageProxyURL = "https://127.0.0.1"
+			*cfg.ImageProxySettings.RemoteImageProxyOptions = "foo"
 		})
 
 		imageURL := "http://mydomain.com/myimage"
@@ -636,9 +659,10 @@ func TestUpdatePost(t *testing.T) {
 
 		th.App.UpdateConfig(func(cfg *model.Config) {
 			*cfg.ExperimentalSettings.EnablePostMetadata = false
-			*cfg.ServiceSettings.ImageProxyType = "atmos/camo"
-			*cfg.ServiceSettings.ImageProxyURL = "https://127.0.0.1"
-			*cfg.ServiceSettings.ImageProxyOptions = "foo"
+			*cfg.ImageProxySettings.Enable = true
+			*cfg.ImageProxySettings.ImageProxyType = "atmos/camo"
+			*cfg.ImageProxySettings.RemoteImageProxyURL = "https://127.0.0.1"
+			*cfg.ImageProxySettings.RemoteImageProxyOptions = "foo"
 		})
 
 		imageURL := "http://mydomain.com/myimage"

app/server.go

@@ -30,6 +30,7 @@ import (
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/plugin"
 	"github.com/mattermost/mattermost-server/services/httpservice"
+	"github.com/mattermost/mattermost-server/services/imageproxy"
 	"github.com/mattermost/mattermost-server/services/timezones"
 	"github.com/mattermost/mattermost-server/store"
 	"github.com/mattermost/mattermost-server/utils"
@@ -110,6 +111,8 @@ type Server struct {
 
 	HTTPService httpservice.HTTPService
 
+	ImageProxy *imageproxy.ImageProxy
+
 	Log *mlog.Logger
 
 	joinCluster        bool
@@ -165,6 +168,8 @@ func NewServer(options ...Option) (*Server, error) {
 
 	s.HTTPService = httpservice.MakeHTTPService(s.FakeApp())
 
+	s.ImageProxy = imageproxy.MakeImageProxy(s, s.HTTPService)
+
 	if utils.T == nil {
 		if err := utils.TranslationsPreInit(); err != nil {
 			return nil, errors.Wrapf(err, "unable to load Mattermost translation files")