diff --git a/api4/user_test.go b/api4/user_test.go index 58aa461926..456b93efc0 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -3643,7 +3643,6 @@ func TestLoginCookies(t *testing.T) { t.Run("should return cookie with MMCLOUDURL for cloud installations", func(t *testing.T) { updateConfig := func(cfg *model.Config) { - *cfg.ServiceSettings.AllowCookiesForSubdomains = true *cfg.ServiceSettings.SiteURL = "https://testchips.cloud.mattermost.com" } th := SetupAndApplyConfigBeforeLogin(t, updateConfig).InitBasic() @@ -3661,9 +3660,29 @@ func TestLoginCookies(t *testing.T) { assert.Equal(t, "mattermost.com", domain) }) + t.Run("should NOT return cookie with MMCLOUDURL for cloud installations without expected format of cloud URL", func(t *testing.T) { + updateConfig := func(cfg *model.Config) { + *cfg.ServiceSettings.SiteURL = "https://testchips.com" // correct cloud URL would be https://testchips.cloud.mattermost.com + } + th := SetupAndApplyConfigBeforeLogin(t, updateConfig).InitBasic() + defer th.TearDown() + + th.App.Srv().SetLicense(model.NewTestLicense("cloud")) + + _, resp, _ := th.Client.Login(th.BasicUser.Email, th.BasicUser.Password) + + cloudSessionCookie := "" + for _, cookie := range resp.Header["Set-Cookie"] { + if match := regexp.MustCompile("^" + model.SessionCookieCloudUrl + "=([a-z0-9]+)").FindStringSubmatch(cookie); match != nil { + cloudSessionCookie = match[1] + } + } + // no cookie set + assert.Equal(t, "", cloudSessionCookie) + }) + t.Run("should NOT return cookie with MMCLOUDURL for NON cloud installations", func(t *testing.T) { updateConfig := func(cfg *model.Config) { - *cfg.ServiceSettings.AllowCookiesForSubdomains = true *cfg.ServiceSettings.SiteURL = "https://testchips.com" } th := SetupAndApplyConfigBeforeLogin(t, updateConfig).InitBasic() diff --git a/app/login.go b/app/login.go index fd13f3a999..c3816f0fd5 100644 --- a/app/login.go +++ b/app/login.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "net/http" + "net/url" "os" "strconv" "strings" @@ -245,21 +246,35 @@ func (a *App) AttachCloudSessionCookie(c *request.Context, w http.ResponseWriter } maxAge := *a.Config().ServiceSettings.SessionLengthWebInDays * 60 * 60 * 24 - domain := a.GetCookieDomain() subpath, _ := utils.GetSubpathFromConfig(a.Config()) expiresAt := time.Unix(model.GetMillis()/1000+int64(maxAge), 0) - var val string + domain := "" + if siteURL, err := url.Parse(a.GetSiteURL()); err == nil { + domain = siteURL.Hostname() + } + + if domain == "" { + return + } + + var workspaceName string if strings.Contains(domain, "localhost") { - val = "localhost" + workspaceName = "localhost" } else { - val = strings.SplitN(domain, ".", 2)[0] + + // ensure we have a format for a cloud workspace url i.e. example.cloud.mattermost.com + if len(strings.Split(domain, ".")) != 4 { + return + } + workspaceName = strings.SplitN(domain, ".", 2)[0] domain = strings.SplitN(domain, ".", 3)[2] + domain = "." + domain } cookie := &http.Cookie{ Name: model.SessionCookieCloudUrl, - Value: val, + Value: workspaceName, Path: subpath, MaxAge: maxAge, Expires: expiresAt, @@ -269,6 +284,7 @@ func (a *App) AttachCloudSessionCookie(c *request.Context, w http.ResponseWriter } http.SetCookie(w, cookie) + } func (a *App) AttachSessionCookies(c *request.Context, w http.ResponseWriter, r *http.Request) {