IntenseWebs/mattermost
3
0
Fork 0
mirror of https://github.com/mattermost/mattermost.git synced 2025-02-25 18:55:24 -06:00
Code Issues Packages Projects Releases Wiki Activity

MM-58275 Ensure image proxy site URL is updated when that changes (#27214)

Browse Source 
* MM-58275 Ensure image proxy site URL is updated when that changes

* Check if proxy settings changed using reflection
This commit is contained in:
Harrison Healey 2024-06-04 14:06:37 -04:00 committed by GitHub
parent 206ff6e697
commit 2bcaa42dc0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 127 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
server/channels/utils/testutils/static_config_service.go
View File

@ -11,23 +11,41 @@ import (
type StaticConfigService struct {
Cfg *model.Config
listeners map[string]func(old, current *model.Config)
}
func (s StaticConfigService) Config() *model.Config {
func (s *StaticConfigService) Config() *model.Config {
return s.Cfg
}
func (StaticConfigService) AddConfigListener(func(old, current *model.Config)) string {
return ""
func (s *StaticConfigService) AddConfigListener(listener func(old, current *model.Config)) string {
if s.listeners == nil {
s.listeners = make(map[string]func(old, current *model.Config))
}
listenerID := model.NewId()
s.listeners[listenerID] = listener
return listenerID
}
func (StaticConfigService) RemoveConfigListener(string) {
func (s *StaticConfigService) RemoveConfigListener(listenerID string) {
delete(s.listeners, listenerID)
}
func (StaticConfigService) AsymmetricSigningKey() *ecdsa.PrivateKey {
func (s *StaticConfigService) AsymmetricSigningKey() *ecdsa.PrivateKey {
return &ecdsa.PrivateKey{}
}
func (StaticConfigService) PostActionCookieSecret() []byte {
func (s *StaticConfigService) PostActionCookieSecret() []byte {
return make([]byte, 32)
}
func (s *StaticConfigService) UpdateConfig(newConfig *model.Config) {
oldConfig := s.Config()
s.Cfg = newConfig
for _, listener := range s.listeners {
listener(oldConfig, newConfig)
}
}

28
server/platform/services/imageproxy/atmos_camo.go
View File

@ -10,26 +10,27 @@ import (
"io"
"net/http"
"net/url"
"github.com/mattermost/mattermost/server/public/model"
)
type AtmosCamoBackend struct {
proxy *ImageProxy
siteURL *url.URL
remoteURL *url.URL
client *http.Client
siteURL *url.URL
remoteOptions string
remoteURL *url.URL
client *http.Client
}
func makeAtmosCamoBackend(proxy *ImageProxy) *AtmosCamoBackend {
func makeAtmosCamoBackend(proxy *ImageProxy, proxySettings model.ImageProxySettings) *AtmosCamoBackend {
// We deliberately ignore the error because it's from config.json.
// The function returns a nil pointer in case of error, and we handle it when it's used.
siteURL, _ := url.Parse(*proxy.ConfigService.Config().ServiceSettings.SiteURL)
remoteURL, _ := url.Parse(*proxy.ConfigService.Config().ImageProxySettings.RemoteImageProxyURL)
remoteURL, _ := url.Parse(*proxySettings.RemoteImageProxyURL)
return &AtmosCamoBackend{
proxy: proxy,
siteURL: siteURL,
remoteURL: remoteURL,
client: proxy.HTTPService.MakeClient(false),
siteURL: proxy.siteURL,
remoteURL: remoteURL,
remoteOptions: *proxySettings.RemoteImageProxyOptions,
client: proxy.HTTPService.MakeClient(false),
}
}
@ -53,9 +54,6 @@ func (backend *AtmosCamoBackend) GetImageDirect(imageURL string) (io.ReadCloser,
}
func (backend *AtmosCamoBackend) getAtmosCamoImageURL(imageURL string) string {
cfg := *backend.proxy.ConfigService.Config()
options := *cfg.ImageProxySettings.RemoteImageProxyOptions
if imageURL == "" || backend.siteURL == nil {
return imageURL
}
@ -84,7 +82,7 @@ func (backend *AtmosCamoBackend) getAtmosCamoImageURL(imageURL string) string {
}
urlBytes := []byte(parsedURL.String())
mac := hmac.New(sha1.New, []byte(options))
mac := hmac.New(sha1.New, []byte(backend.remoteOptions))
mac.Write(urlBytes)
digest := hex.EncodeToString(mac.Sum(nil))

9
server/platform/services/imageproxy/atmos_camo_test.go
View File

@ -66,14 +66,13 @@ func TestAtmosCamoBackend_GetImageDirect(t *testing.T) {
defer mock.Close()
proxy := makeTestAtmosCamoProxy()
parsedURL, err := url.Parse(*proxy.ConfigService.Config().ServiceSettings.SiteURL)
parsedURL, err := url.Parse("https://mattermost.example.com")
require.NoError(t, err)
remoteURL, err := url.Parse(mock.URL)
require.NoError(t, err)
backend := &AtmosCamoBackend{
proxy: proxy,
siteURL: parsedURL,
remoteURL: remoteURL,
client: proxy.HTTPService.MakeClient(false),
@ -177,9 +176,9 @@ func TestGetAtmosCamoImageURL(t *testing.T) {
require.NoError(t, err)
backend := &AtmosCamoBackend{
proxy: makeTestAtmosCamoProxy(),
siteURL: parsedURL,
remoteURL: remoteURL,
siteURL: parsedURL,
remoteURL: remoteURL,
remoteOptions: *makeTestAtmosCamoProxy().ConfigService.Config().ImageProxySettings.RemoteImageProxyOptions,
}
assert.Equal(t, test.Expected, backend.getAtmosCamoImageURL(test.Input))

20
server/platform/services/imageproxy/imageproxy.go
View File

@ -8,6 +8,7 @@ import (
"io"
"net/http"
"net/url"
"reflect"
"strings"
"sync"
@ -59,21 +60,21 @@ func MakeImageProxy(configService configservice.ConfigService, httpService https
proxy.configListenerID = proxy.ConfigService.AddConfigListener(proxy.OnConfigChange)
config := proxy.ConfigService.Config()
proxy.backend = proxy.makeBackend(*config.ImageProxySettings.Enable, *config.ImageProxySettings.ImageProxyType)
proxy.backend = proxy.makeBackend(config.ImageProxySettings)
return proxy
}
func (proxy *ImageProxy) makeBackend(enable bool, proxyType string) ImageProxyBackend {
if !enable {
func (proxy *ImageProxy) makeBackend(proxySettings model.ImageProxySettings) ImageProxyBackend {
if !*proxySettings.Enable {
return nil
}
switch proxyType {
switch *proxySettings.ImageProxyType {
case model.ImageProxyTypeLocal:
return makeLocalBackend(proxy)
case model.ImageProxyTypeAtmosCamo:
return makeAtmosCamoBackend(proxy)
return makeAtmosCamoBackend(proxy, proxySettings)
default:
return nil
}
@ -87,12 +88,15 @@ func (proxy *ImageProxy) Close() {
}
func (proxy *ImageProxy) OnConfigChange(oldConfig, newConfig *model.Config) {
if *oldConfig.ImageProxySettings.Enable != *newConfig.ImageProxySettings.Enable ||
*oldConfig.ImageProxySettings.ImageProxyType != *newConfig.ImageProxySettings.ImageProxyType {
if *oldConfig.ServiceSettings.SiteURL != *newConfig.ServiceSettings.SiteURL ||
!reflect.DeepEqual(oldConfig.ImageProxySettings, newConfig.ImageProxySettings) {
proxy.lock.Lock()
defer proxy.lock.Unlock()
proxy.backend = proxy.makeBackend(*newConfig.ImageProxySettings.Enable, *newConfig.ImageProxySettings.ImageProxyType)
siteURL, _ := url.Parse(*newConfig.ServiceSettings.SiteURL)
proxy.siteURL = siteURL
proxy.backend = proxy.makeBackend(newConfig.ImageProxySettings)
}
}

70
server/platform/services/imageproxy/imageproxy_test.go
View File

@ -7,6 +7,8 @@ import (
"net/url"
"testing"
"github.com/mattermost/mattermost/server/public/model"
"github.com/mattermost/mattermost/server/v8/channels/utils/testutils"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@ -115,3 +117,71 @@ func TestGetUnproxiedImageURL(t *testing.T) {
})
}
}
func TestOnConfigChange(t *testing.T) {
t.Run("should switch between backends", func(t *testing.T) {
proxy := makeTestAtmosCamoProxy()
require.Equal(t, "https://mattermost.example.com", proxy.backend.(*AtmosCamoBackend).siteURL.String())
newConfig := proxy.ConfigService.Config().Clone()
newConfig.ImageProxySettings.ImageProxyType = model.NewString(model.ImageProxyTypeLocal)
proxy.ConfigService.(*testutils.StaticConfigService).UpdateConfig(newConfig)
require.Equal(t, "https://mattermost.example.com", proxy.backend.(*LocalBackend).baseURL.String())
newConfig = proxy.ConfigService.Config().Clone()
newConfig.ImageProxySettings.ImageProxyType = model.NewString(model.ImageProxyTypeAtmosCamo)
proxy.ConfigService.(*testutils.StaticConfigService).UpdateConfig(newConfig)
require.Equal(t, "https://mattermost.example.com", proxy.backend.(*AtmosCamoBackend).siteURL.String())
})
t.Run("for local proxy, should update site URL when that changes", func(t *testing.T) {
proxy := makeTestLocalProxy()
require.Equal(t, "https://mattermost.example.com", proxy.siteURL.String())
require.Equal(t, "https://mattermost.example.com", proxy.backend.(*LocalBackend).baseURL.String())
newConfig := proxy.ConfigService.Config().Clone()
newConfig.ServiceSettings.SiteURL = model.NewString("https://new.example.com")
proxy.ConfigService.(*testutils.StaticConfigService).UpdateConfig(newConfig)
require.Equal(t, "https://new.example.com", proxy.siteURL.String())
require.Equal(t, "https://new.example.com", proxy.backend.(*LocalBackend).baseURL.String())
})
t.Run("for atmos/camo proxy, should update site URL when that changes", func(t *testing.T) {
proxy := makeTestAtmosCamoProxy()
require.Equal(t, "https://mattermost.example.com", proxy.siteURL.String())
require.Equal(t, "https://mattermost.example.com", proxy.backend.(*AtmosCamoBackend).siteURL.String())
newConfig := proxy.ConfigService.Config().Clone()
newConfig.ServiceSettings.SiteURL = model.NewString("https://new.example.com")
proxy.ConfigService.(*testutils.StaticConfigService).UpdateConfig(newConfig)
require.Equal(t, "https://new.example.com", proxy.siteURL.String())
require.Equal(t, "https://new.example.com", proxy.backend.(*AtmosCamoBackend).siteURL.String())
})
t.Run("for atmos/camo proxy, should update additional options when those change", func(t *testing.T) {
proxy := makeTestAtmosCamoProxy()
require.Equal(t, "http://images.example.com", proxy.backend.(*AtmosCamoBackend).remoteURL.String())
// require.Equal(t, "7e5f3fab20b94782b43cdb022a66985ef28ba355df2c5d5da3c9a05e4b697bac", proxy.backend.(*AtmosCamoBackend).remoteOptions)
newConfig := proxy.ConfigService.Config().Clone()
newConfig.ImageProxySettings.RemoteImageProxyURL = model.NewString("https://new.example.com")
newConfig.ImageProxySettings.RemoteImageProxyOptions = model.NewString("some other random hash")
proxy.ConfigService.(*testutils.StaticConfigService).UpdateConfig(newConfig)
require.Equal(t, "https://new.example.com", proxy.backend.(*AtmosCamoBackend).remoteURL.String())
// require.Equal(t, "some other random hash", proxy.backend.(*AtmosCamoBackend).remoteOptions)
})
}

9
server/platform/services/imageproxy/local.go
View File

@ -40,8 +40,6 @@ var msgNotAllowed = "requested URL is not allowed"
var ErrLocalRequestFailed = Error{errors.New("imageproxy.LocalBackend: failed to request proxied image")}
type LocalBackend struct {
proxy *ImageProxy
client *http.Client
baseURL *url.URL
}
@ -57,15 +55,14 @@ func (e URLError) Error() string {
}
func makeLocalBackend(proxy *ImageProxy) *LocalBackend {
baseURL, err := url.Parse(*proxy.ConfigService.Config().ServiceSettings.SiteURL)
if err != nil {
mlog.Warn("Failed to set base URL for image proxy. Relative image links may not work.", mlog.Err(err))
baseURL := proxy.siteURL
if baseURL == nil {
mlog.Warn("Failed to set base URL for image proxy. Relative image links may not work.")
}
client := proxy.HTTPService.MakeClient(false)
return &LocalBackend{
proxy: proxy,
client: client,
baseURL: baseURL,
}