From a4e9499714999d58f26c712df02c014f1facccf7 Mon Sep 17 00:00:00 2001 From: Chris Date: Fri, 9 Feb 2018 13:56:11 -0600 Subject: Add /v4/image api (#8230) * add image api * i suppose i should add a test... * only redirect to image proxy --- api4/api.go | 5 +++++ api4/image.go | 22 ++++++++++++++++++++++ api4/image_test.go | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ utils/config.go | 3 +++ 4 files changed, 82 insertions(+) create mode 100644 api4/image.go create mode 100644 api4/image_test.go diff --git a/api4/api.go b/api4/api.go index 580bd8c58..871dca0ac 100644 --- a/api4/api.go +++ b/api4/api.go @@ -76,6 +76,8 @@ type Routes struct { Compliance *mux.Router // 'api/v4/compliance' Cluster *mux.Router // 'api/v4/cluster' + Image *mux.Router // 'api/v4/image' + LDAP *mux.Router // 'api/v4/ldap' Elasticsearch *mux.Router // 'api/v4/elasticsearch' @@ -194,6 +196,8 @@ func Init(a *app.App, root *mux.Router, full bool) *API { api.BaseRoutes.OpenGraph = api.BaseRoutes.ApiRoot.PathPrefix("/opengraph").Subrouter() + api.BaseRoutes.Image = api.BaseRoutes.ApiRoot.PathPrefix("/image").Subrouter() + api.InitUser() api.InitTeam() api.InitChannel() @@ -219,6 +223,7 @@ func Init(a *app.App, root *mux.Router, full bool) *API { api.InitWebrtc() api.InitOpenGraph() api.InitPlugin() + api.InitImage() root.Handle("/api/v4/{anything:.*}", http.HandlerFunc(Handle404)) diff --git a/api4/image.go b/api4/image.go new file mode 100644 index 000000000..4589de204 --- /dev/null +++ b/api4/image.go @@ -0,0 +1,22 @@ +// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package api4 + +import ( + "net/http" +) + +func (api *API) InitImage() { + api.BaseRoutes.Image.Handle("", api.ApiSessionRequiredTrustRequester(getImage)).Methods("GET") +} + +func getImage(c *Context, w http.ResponseWriter, r *http.Request) { + // Only redirect to our image proxy if one is enabled. Arbitrary redirects are not allowed for + // security reasons. + if transform := c.App.ImageProxyAdder(); transform != nil { + http.Redirect(w, r, transform(r.URL.Query().Get("url")), http.StatusFound) + } else { + http.NotFound(w, r) + } +} diff --git a/api4/image_test.go b/api4/image_test.go new file mode 100644 index 000000000..236d5785d --- /dev/null +++ b/api4/image_test.go @@ -0,0 +1,52 @@ +// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package api4 + +import ( + "net/http" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/mattermost/mattermost-server/model" +) + +func TestGetImage(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + th.Client.HttpClient.CheckRedirect = func(*http.Request, []*http.Request) error { + return http.ErrUseLastResponse + } + + originURL := "http://foo.bar/baz.gif" + + r, err := http.NewRequest("GET", th.Client.ApiUrl+"/image?url="+url.QueryEscape(originURL), nil) + require.NoError(t, err) + r.Header.Set(model.HEADER_AUTH, th.Client.AuthType+" "+th.Client.AuthToken) + + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.ServiceSettings.ImageProxyType = nil + }) + + resp, err := th.Client.HttpClient.Do(r) + require.NoError(t, err) + assert.Equal(t, http.StatusNotFound, resp.StatusCode) + + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.ServiceSettings.ImageProxyType = model.NewString("willnorris/imageproxy") + cfg.ServiceSettings.ImageProxyURL = model.NewString("https://proxy.foo.bar") + }) + + r, err = http.NewRequest("GET", th.Client.ApiUrl+"/image?url="+originURL, nil) + require.NoError(t, err) + r.Header.Set(model.HEADER_AUTH, th.Client.AuthType+" "+th.Client.AuthToken) + + resp, err = th.Client.HttpClient.Do(r) + require.NoError(t, err) + assert.Equal(t, http.StatusFound, resp.StatusCode) + assert.Equal(t, "https://proxy.foo.bar//"+originURL, resp.Header.Get("Location")) +} diff --git a/utils/config.go b/utils/config.go index 9e962eef4..87ebee693 100644 --- a/utils/config.go +++ b/utils/config.go @@ -456,6 +456,9 @@ func GenerateClientConfig(c *model.Config, diagnosticId string) map[string]strin props["PluginsEnabled"] = strconv.FormatBool(*c.PluginSettings.Enable) + hasImageProxy := c.ServiceSettings.ImageProxyType != nil && *c.ServiceSettings.ImageProxyType != "" && c.ServiceSettings.ImageProxyURL != nil && *c.ServiceSettings.ImageProxyURL != "" + props["HasImageProxy"] = strconv.FormatBool(hasImageProxy) + if IsLicensed() { License := License() props["ExperimentalTownSquareIsReadOnly"] = strconv.FormatBool(*c.TeamSettings.ExperimentalTownSquareIsReadOnly) -- cgit v1.2.3-1-g7c22 From 396e7513ecc7d86b04e56745586c802e56e5d763 Mon Sep 17 00:00:00 2001 From: Chris Date: Fri, 9 Feb 2018 15:41:06 -0600 Subject: Don't proxy same-site image urls (#8238) * don't proxy same-site urls * fix empty site url case --- app/post.go | 8 ++++++-- app/post_test.go | 10 ++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/app/post.go b/app/post.go index 005624605..5b0e59b23 100644 --- a/app/post.go +++ b/app/post.go @@ -876,6 +876,10 @@ func (a *App) imageProxyConfig() (proxyType, proxyURL, options, siteURL string) proxyURL += "/" } + if siteURL == "" || siteURL[len(siteURL)-1] != '/' { + siteURL += "/" + } + if cfg.ServiceSettings.ImageProxyOptions != nil { options = *cfg.ServiceSettings.ImageProxyOptions } @@ -890,12 +894,12 @@ func (a *App) ImageProxyAdder() func(string) string { } return func(url string) string { - if url == "" || strings.HasPrefix(url, proxyURL) { + if url == "" || strings.HasPrefix(url, siteURL) || strings.HasPrefix(url, proxyURL) { return url } if url[0] == '/' { - url = siteURL + url + url = siteURL + url[1:] } switch proxyType { diff --git a/app/post_test.go b/app/post_test.go index 3f3783265..e09d3a198 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -190,6 +190,10 @@ func TestImageProxy(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.SiteURL = "http://mymattermost.com" + }) + for name, tc := range map[string]struct { ProxyType string ProxyURL string @@ -211,6 +215,12 @@ func TestImageProxy(t *testing.T) { ImageURL: "http://mydomain.com/myimage", ProxiedImageURL: "https://127.0.0.1/x1000/http://mydomain.com/myimage", }, + "willnorris/imageproxy_SameSite": { + ProxyType: "willnorris/imageproxy", + ProxyURL: "https://127.0.0.1", + ImageURL: "http://mymattermost.com/myimage", + ProxiedImageURL: "http://mymattermost.com/myimage", + }, "willnorris/imageproxy_EmptyImageURL": { ProxyType: "willnorris/imageproxy", ProxyURL: "https://127.0.0.1", -- cgit v1.2.3-1-g7c22 From c1b6e8792c9f91c66c35737438c20757ef43066f Mon Sep 17 00:00:00 2001 From: Christopher Brown Date: Fri, 9 Feb 2018 20:08:39 -0600 Subject: minor addition to #8238 --- app/post.go | 6 +----- app/post_test.go | 6 ++++++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/app/post.go b/app/post.go index 5b0e59b23..be9374e10 100644 --- a/app/post.go +++ b/app/post.go @@ -894,14 +894,10 @@ func (a *App) ImageProxyAdder() func(string) string { } return func(url string) string { - if url == "" || strings.HasPrefix(url, siteURL) || strings.HasPrefix(url, proxyURL) { + if url == "" || url[0] == '/' || strings.HasPrefix(url, siteURL) || strings.HasPrefix(url, proxyURL) { return url } - if url[0] == '/' { - url = siteURL + url[1:] - } - switch proxyType { case "atmos/camo": mac := hmac.New(sha1.New, []byte(options)) diff --git a/app/post_test.go b/app/post_test.go index e09d3a198..409bc043d 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -221,6 +221,12 @@ func TestImageProxy(t *testing.T) { ImageURL: "http://mymattermost.com/myimage", ProxiedImageURL: "http://mymattermost.com/myimage", }, + "willnorris/imageproxy_PathOnly": { + ProxyType: "willnorris/imageproxy", + ProxyURL: "https://127.0.0.1", + ImageURL: "/myimage", + ProxiedImageURL: "/myimage", + }, "willnorris/imageproxy_EmptyImageURL": { ProxyType: "willnorris/imageproxy", ProxyURL: "https://127.0.0.1", -- cgit v1.2.3-1-g7c22 From 9707ac3aaf2cb4352c573aadf54b8535e237dd9e Mon Sep 17 00:00:00 2001 From: Jonathan Date: Mon, 12 Feb 2018 09:16:17 -0500 Subject: Added invite_id field to email invite url, along with validation of this field on the server (#8235) --- api4/team_test.go | 3 ++- app/email.go | 1 + app/team.go | 5 +++++ app/team_test.go | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ i18n/en.json | 16 ++++--------- 5 files changed, 79 insertions(+), 13 deletions(-) diff --git a/api4/team_test.go b/api4/team_test.go index a8696a30b..d365fd686 100644 --- a/api4/team_test.go +++ b/api4/team_test.go @@ -1250,7 +1250,7 @@ func TestAddTeamMember(t *testing.T) { tm, resp := Client.AddTeamMember(team.Id, otherUser.Id) CheckForbiddenStatus(t, resp) if resp.Error == nil { - t.Fatalf("Error is nhul") + t.Fatalf("Error is nil") } Client.Logout() @@ -1376,6 +1376,7 @@ func TestAddTeamMember(t *testing.T) { dataObject := make(map[string]string) dataObject["time"] = fmt.Sprintf("%v", model.GetMillis()) dataObject["id"] = team.Id + dataObject["invite_id"] = team.InviteId data := model.MapToJson(dataObject) hashed := utils.HashSha256(fmt.Sprintf("%v:%v", data, th.App.Config().EmailSettings.InviteSalt)) diff --git a/app/email.go b/app/email.go index b809b972d..764dc017a 100644 --- a/app/email.go +++ b/app/email.go @@ -276,6 +276,7 @@ func (a *App) SendInviteEmails(team *model.Team, senderName string, invites []st props["display_name"] = team.DisplayName props["name"] = team.Name props["time"] = fmt.Sprintf("%v", model.GetMillis()) + props["invite_id"] = team.InviteId data := model.MapToJson(props) hash := utils.HashSha256(fmt.Sprintf("%v:%v", data, a.Config().EmailSettings.InviteSalt)) bodyPage.Props["Link"] = fmt.Sprintf("%s/signup_user_complete/?d=%s&h=%s", siteURL, url.QueryEscape(data), url.QueryEscape(hash)) diff --git a/app/team.go b/app/team.go index 21b8e5879..8e8c29e2a 100644 --- a/app/team.go +++ b/app/team.go @@ -234,6 +234,11 @@ func (a *App) AddUserToTeamByHash(userId string, hash string, data string) (*mod team = result.Data.(*model.Team) } + // verify that the team's invite id hasn't been changed since the invite was sent + if team.InviteId != props["invite_id"] { + return nil, model.NewAppError("JoinUserToTeamByHash", "api.user.create_user.signup_link_mismatched_invite_id.app_error", nil, "", http.StatusBadRequest) + } + var user *model.User if result := <-uchan; result.Err != nil { return nil, result.Err diff --git a/app/team_test.go b/app/team_test.go index 084558fb4..7cb20b6f6 100644 --- a/app/team_test.go +++ b/app/team_test.go @@ -7,7 +7,15 @@ import ( "strings" "testing" + "fmt" + + "sync/atomic" + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/store" + "github.com/mattermost/mattermost-server/store/storetest" + "github.com/mattermost/mattermost-server/utils" + "github.com/stretchr/testify/assert" ) func TestCreateTeam(t *testing.T) { @@ -393,3 +401,62 @@ func TestSanitizeTeams(t *testing.T) { } }) } + +func TestAddUserToTeamByHashMismatchedInviteId(t *testing.T) { + mockStore := &storetest.Store{} + defer mockStore.AssertExpectations(t) + + teamId := model.NewId() + userId := model.NewId() + inviteSalt := model.NewId() + + inviteId := model.NewId() + teamInviteId := model.NewId() + + // generate a fake email invite - stolen from SendInviteEmails() in email.go + props := make(map[string]string) + props["email"] = model.NewId() + "@mattermost.com" + props["id"] = teamId + props["display_name"] = model.NewId() + props["name"] = model.NewId() + props["time"] = fmt.Sprintf("%v", model.GetMillis()) + props["invite_id"] = inviteId + data := model.MapToJson(props) + hash := utils.HashSha256(fmt.Sprintf("%v:%v", data, inviteSalt)) + + // when the server tries to validate the invite, it will pull the user from our mock store + // this can return nil, because we'll fail before we get to trying to use it + mockStore.UserStore.On("Get", userId).Return( + storetest.NewStoreChannel(store.StoreResult{ + Data: nil, + Err: nil, + }), + ) + + // the server will also pull the team. the one we return has a different invite id than the one in the email invite we made above + mockStore.TeamStore.On("Get", teamId).Return( + storetest.NewStoreChannel(store.StoreResult{ + Data: &model.Team{ + InviteId: teamInviteId, + }, + Err: nil, + }), + ) + + app := App{ + Srv: &Server{ + Store: mockStore, + }, + config: atomic.Value{}, + } + app.config.Store(&model.Config{ + EmailSettings: model.EmailSettings{ + InviteSalt: inviteSalt, + }, + }) + + // this should fail because the invite ids are mismatched + team, err := app.AddUserToTeamByHash(userId, hash, data) + assert.Nil(t, team) + assert.Equal(t, "api.user.create_user.signup_link_mismatched_invite_id.app_error", err.Id) +} diff --git a/i18n/en.json b/i18n/en.json index d983e8855..4365a44fb 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -2202,10 +2202,6 @@ "id": "api.team.create_team_from_signup.expired_link.app_error", "translation": "The signup link has expired" }, - { - "id": "api.team.create_team_from_signup.invalid_link.app_error", - "translation": "The signup link does not appear to be valid" - }, { "id": "api.team.create_team_from_signup.unavailable.app_error", "translation": "This URL is unavailable. Please try another." @@ -2706,6 +2702,10 @@ "id": "api.user.create_user.signup_link_expired.app_error", "translation": "The signup link has expired" }, + { + "id": "api.user.create_user.signup_link_mismatched_invite_id.app_error", + "translation": "The signup link does not appear to be valid" + }, { "id": "api.user.create_user.signup_link_invalid.app_error", "translation": "The signup link does not appear to be valid" @@ -7294,10 +7294,6 @@ "id": "web.root.singup_title", "translation": "Signup" }, - { - "id": "web.signup_team_complete.invalid_link.app_error", - "translation": "The signup link does not appear to be valid" - }, { "id": "web.signup_team_complete.link_expired.app_error", "translation": "The signup link has expired" @@ -7314,10 +7310,6 @@ "id": "web.signup_user_complete.link_expired.app_error", "translation": "The signup link has expired" }, - { - "id": "web.signup_user_complete.link_invalid.app_error", - "translation": "The signup link does not appear to be valid" - }, { "id": "web.signup_user_complete.no_invites.app_error", "translation": "The team type doesn't allow open invites" -- cgit v1.2.3-1-g7c22