Merge branch 'release-4.7-t4merge'

api4/team_test.go

@@ -1244,7 +1244,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()
 
@@ -1343,6 +1343,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))

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))

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,14 +894,10 @@ func (a *App) ImageProxyAdder() func(string) string {
 	}
 
 	return func(url string) string {
-		if url == "" || strings.HasPrefix(url, proxyURL) {
+		if url == "" || url[0] == '/' || strings.HasPrefix(url, siteURL) || strings.HasPrefix(url, proxyURL) {
 			return url
 		}
 
-		if url[0] == '/' {
-			url = siteURL + url
-		}
-
 		switch proxyType {
 		case "atmos/camo":
 			mac := hmac.New(sha1.New, []byte(options))

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,18 @@ 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_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",

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

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)
+}

i18n/en.json

@@ -2198,10 +2198,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."
@@ -2702,6 +2698,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"
@@ -7298,10 +7298,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"
@@ -7318,10 +7314,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"