From a752d7de5a0a1aa1679790f76f2f8dc35e322c41 Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Wed, 9 May 2018 12:25:03 -0400 Subject: [PATCH 01/10] MM-10495 Updated user agent detection to match output of previous library (#8748) * MM-10495 Updated user agent detection to match output of previous library * Fixed missing license header --- app/login.go | 25 ++----- app/user_agent.go | 132 +++++++++++++++++++++++++++++++++++ app/user_agent_test.go | 151 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 287 insertions(+), 21 deletions(-) create mode 100644 app/user_agent.go create mode 100644 app/user_agent_test.go diff --git a/app/login.go b/app/login.go index 43b0227494..a2f06dbc31 100644 --- a/app/login.go +++ b/app/login.go @@ -6,7 +6,6 @@ package app import ( "fmt" "net/http" - "strings" "time" "github.com/avct/uasurfer" @@ -73,26 +72,10 @@ func (a *App) DoLogin(w http.ResponseWriter, r *http.Request, user *model.User, ua := uasurfer.Parse(r.UserAgent()) - plat := ua.OS.Platform.String() - if plat == "" { - plat = "unknown" - } - - os := ua.OS.Name.String() - if os == "" { - os = "unknown" - } - - bname := ua.Browser.Name.String() - if bname == "" { - bname = "unknown" - } - - if strings.Contains(r.UserAgent(), "Mattermost") { - bname = "Desktop App" - } - - bversion := ua.Browser.Version + plat := getPlatformName(ua) + os := getOSName(ua) + bname := getBrowserName(ua, r.UserAgent()) + bversion := getBrowserVersion(ua, r.UserAgent()) session.AddProp(model.SESSION_PROP_PLATFORM, plat) session.AddProp(model.SESSION_PROP_OS, os) diff --git a/app/user_agent.go b/app/user_agent.go new file mode 100644 index 0000000000..d731fd2a8f --- /dev/null +++ b/app/user_agent.go @@ -0,0 +1,132 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "fmt" + "strings" + + "github.com/avct/uasurfer" +) + +var platformNames = map[uasurfer.Platform]string{ + uasurfer.PlatformUnknown: "Windows", + uasurfer.PlatformWindows: "Windows", + uasurfer.PlatformMac: "Macintosh", + uasurfer.PlatformLinux: "Linux", + uasurfer.PlatformiPad: "iPad", + uasurfer.PlatformiPhone: "iPhone", + uasurfer.PlatformiPod: "iPod", + uasurfer.PlatformBlackberry: "BlackBerry", + uasurfer.PlatformWindowsPhone: "Windows Phone", +} + +func getPlatformName(ua *uasurfer.UserAgent) string { + platform := ua.OS.Platform + + if name, ok := platformNames[platform]; !ok { + return platformNames[uasurfer.PlatformUnknown] + } else { + return name + } +} + +var osNames = map[uasurfer.OSName]string{ + uasurfer.OSUnknown: "", + uasurfer.OSWindowsPhone: "Windows Phone", + uasurfer.OSWindows: "Windows", + uasurfer.OSMacOSX: "Mac OS", + uasurfer.OSiOS: "iOS", + uasurfer.OSAndroid: "Android", + uasurfer.OSBlackberry: "BlackBerry", + uasurfer.OSChromeOS: "Chrome OS", + uasurfer.OSKindle: "Kindle", + uasurfer.OSWebOS: "webOS", + uasurfer.OSLinux: "Linux", +} + +func getOSName(ua *uasurfer.UserAgent) string { + os := ua.OS + + if os.Name == uasurfer.OSWindows { + major := os.Version.Major + minor := os.Version.Minor + + name := "Windows" + + // Adapted from https://github.com/mssola/user_agent/blob/master/operating_systems.go#L26 + if major == 5 { + if minor == 0 { + name = "Windows 2000" + } else if minor == 1 { + name = "Windows XP" + } else if minor == 2 { + name = "Windows XP x64 Edition" + } + } else if major == 6 { + if minor == 0 { + name = "Windows Vista" + } else if minor == 1 { + name = "Windows 7" + } else if minor == 2 { + name = "Windows 8" + } else if minor == 3 { + name = "Windows 8.1" + } + } else if major == 10 { + name = "Windows 10" + } + + return name + } else if name, ok := osNames[os.Name]; ok { + return name + } else { + return osNames[uasurfer.OSUnknown] + } +} + +func getBrowserVersion(ua *uasurfer.UserAgent, userAgentString string) string { + if index := strings.Index(userAgentString, "Mattermost/"); index != -1 { + afterVersion := userAgentString[index+len("Mattermost/"):] + return strings.Fields(afterVersion)[0] + } else if index := strings.Index(userAgentString, "Franz/"); index != -1 { + afterVersion := userAgentString[index+len("Franz/"):] + return strings.Fields(afterVersion)[0] + } else { + return getUAVersion(ua.Browser.Version) + } +} + +func getUAVersion(version uasurfer.Version) string { + if version.Patch == 0 { + return fmt.Sprintf("%v.%v", version.Major, version.Minor) + } else { + return fmt.Sprintf("%v.%v.%v", version.Major, version.Minor, version.Patch) + } +} + +var browserNames = map[uasurfer.BrowserName]string{ + uasurfer.BrowserUnknown: "Unknown", + uasurfer.BrowserChrome: "Chrome", + uasurfer.BrowserIE: "Internet Explorer", + uasurfer.BrowserSafari: "Safari", + uasurfer.BrowserFirefox: "Firefox", + uasurfer.BrowserAndroid: "Android", + uasurfer.BrowserOpera: "Opera", + uasurfer.BrowserBlackberry: "BlackBerry", +} + +func getBrowserName(ua *uasurfer.UserAgent, userAgentString string) string { + browser := ua.Browser.Name + + if strings.Contains(userAgentString, "Mattermost") { + return "Desktop App" + } else if browser == uasurfer.BrowserIE && ua.Browser.Version.Major > 11 { + return "Edge" + } else if name, ok := browserNames[browser]; ok { + return name + } else { + return browserNames[uasurfer.BrowserUnknown] + } +} diff --git a/app/user_agent_test.go b/app/user_agent_test.go new file mode 100644 index 0000000000..e4680bfc93 --- /dev/null +++ b/app/user_agent_test.go @@ -0,0 +1,151 @@ +package app + +import ( + "fmt" + "testing" + + "github.com/avct/uasurfer" +) + +type testUserAgent struct { + Name string + UserAgent string +} + +var testUserAgents = []testUserAgent{ + {"Mozilla 40.1", "Mozilla/5.0 (Windows NT 6.1; WOW64; rv:40.0) Gecko/20100101 Firefox/40.1"}, + {"Chrome 60", "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.90 Safari/537.36"}, + {"Chrome Mobile", "Mozilla/5.0 (Linux; Android 6.0; Nexus 5 Build/MRA58N) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.113 Mobile Safari/537.36"}, + {"MM Classic App", "Mozilla/5.0 (Linux; Android 8.0.0; Nexus 5X Build/OPR6.170623.013; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/61.0.3163.81 Mobile Safari/537.36 Web-Atoms-Mobile-WebView"}, + {"MM App 3.7.1", "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Mattermost/3.7.1 Chrome/56.0.2924.87 Electron/1.6.11 Safari/537.36"}, + {"Franz 4.0.4", "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Franz/4.0.4 Chrome/52.0.2743.82 Electron/1.3.1 Safari/537.36"}, + {"Edge 14", "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.79 Safari/537.36 Edge/14.14393"}, + {"Internet Explorer 9", "Mozilla/5.0 (compatible; MSIE 9.0; Windows NT 7.1; Trident/5.0"}, + {"Internet Explorer 11", "Mozilla/5.0 (Windows NT 10.0; WOW64; Trident/7.0; rv:11.0) like Gecko"}, + {"Internet Explorer 11 2", "Mozilla/5.0 (Windows NT 10.0; WOW64; Trident/7.0; .NET4.0C; .NET4.0E; .NET CLR 2.0.50727; .NET CLR 3.0.30729; .NET CLR 3.5.30729; Zoom 3.6.0; rv:11.0) like Gecko"}, + {"Internet Explorer 11 (Compatibility Mode) 1", "Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 10.0; WOW64; Trident/7.0; .NET4.0C; .NET4.0E; .NET CLR 2.0.50727; .NET CLR 3.0.30729; .NET CLR 3.5.30729; .NET CLR 1.1.4322; InfoPath.3; Zoom 3.6.0)"}, + {"Internet Explorer 11 (Compatibility Mode) 2", "Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 10.0; WOW64; Trident/7.0; .NET4.0C; .NET4.0E; .NET CLR 2.0.50727; .NET CLR 3.0.30729; .NET CLR 3.5.30729; Zoom 3.6.0)"}, + {"Safari 9", "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/604.1.38 (KHTML, like Gecko) Version/11.0 Safari/604.1.38"}, + {"Safari 8", "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_4) AppleWebKit/600.7.12 (KHTML, like Gecko) Version/8.0.7 Safari/600.7.12"}, + {"Safari Mobile", "Mozilla/5.0 (iPhone; CPU iPhone OS 9_1 like Mac OS X) AppleWebKit/601.1.46 (KHTML, like Gecko) Version/9.0 Mobile/13B137 Safari/601.1"}, +} + +func TestGetPlatformName(t *testing.T) { + expected := []string{ + "Windows", + "Macintosh", + "Linux", + "Linux", + "Macintosh", + "Macintosh", + "Windows", + "Windows", + "Windows", + "Windows", + "Windows", + "Windows", + "Macintosh", + "Macintosh", + "iPhone", + } + + for i, userAgent := range testUserAgents { + t.Run(fmt.Sprintf("GetPlatformName_%v", i), func(t *testing.T) { + ua := uasurfer.Parse(userAgent.UserAgent) + + if actual := getPlatformName(ua); actual != expected[i] { + t.Fatalf("%v Got %v, expected %v", userAgent.Name, actual, expected[i]) + } + }) + } +} + +func TestGetOSName(t *testing.T) { + expected := []string{ + "Windows 7", + "Mac OS", + "Android", + "Android", + "Mac OS", + "Mac OS", + "Windows 10", + "Windows", + "Windows 10", + "Windows 10", + "Windows 10", + "Windows 10", + "Mac OS", + "Mac OS", + "iOS", + } + + for i, userAgent := range testUserAgents { + t.Run(fmt.Sprintf("GetOSName_%v", i), func(t *testing.T) { + ua := uasurfer.Parse(userAgent.UserAgent) + + if actual := getOSName(ua); actual != expected[i] { + t.Fatalf("Got %v, expected %v", actual, expected[i]) + } + }) + } +} + +func TestGetBrowserName(t *testing.T) { + expected := []string{ + "Firefox", + "Chrome", + "Chrome", + "Chrome", + "Desktop App", + "Chrome", + "Edge", + "Internet Explorer", + "Internet Explorer", + "Internet Explorer", + "Internet Explorer", + "Internet Explorer", + "Safari", + "Safari", + "Safari", + } + + for i, userAgent := range testUserAgents { + t.Run(fmt.Sprintf("GetBrowserName_%v", i), func(t *testing.T) { + ua := uasurfer.Parse(userAgent.UserAgent) + + if actual := getBrowserName(ua, userAgent.UserAgent); actual != expected[i] { + t.Fatalf("Got %v, expected %v", actual, expected[i]) + } + }) + } +} + +func TestGetBrowserVersion(t *testing.T) { + expected := []string{ + "40.1", + "60.0.3112", // Doesn't report the fourth part of the version + "60.0.3112", // Doesn't report the fourth part of the version + "61.0.3163", + "3.7.1", + "4.0.4", + "14.14393", + "9.0", + "11.0", + "11.0", + "7.0", + "7.0", + "11.0", + "8.0.7", + "9.0", + } + + for i, userAgent := range testUserAgents { + t.Run(fmt.Sprintf("GetBrowserVersion_%v", i), func(t *testing.T) { + ua := uasurfer.Parse(userAgent.UserAgent) + + if actual := getBrowserVersion(ua, userAgent.UserAgent); actual != expected[i] { + t.Fatalf("Got %v, expected %v", actual, expected[i]) + } + }) + } +} From 823b22c403510a52d56bc96428cf9977b80a9dfc Mon Sep 17 00:00:00 2001 From: Carlos Tadeu Panato Junior Date: Thu, 10 May 2018 14:43:49 +0200 Subject: [PATCH 02/10] fix test and add new (#8758) --- api4/system_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/api4/system_test.go b/api4/system_test.go index d4134f8e2b..f46ae74361 100644 --- a/api4/system_test.go +++ b/api4/system_test.go @@ -618,9 +618,14 @@ func TestS3TestConnection(t *testing.T) { config.FileSettings.AmazonS3Bucket = "Wrong_bucket" _, resp = th.SystemAdminClient.TestS3Connection(&config) CheckInternalErrorStatus(t, resp) - if resp.Error.Message != "Error checking if bucket exists." { + if resp.Error.Message != "Unable to create bucket" { t.Fatal("should return error ") } + + config.FileSettings.AmazonS3Bucket = "shouldcreatenewbucket" + _, resp = th.SystemAdminClient.TestS3Connection(&config) + CheckOKStatus(t, resp) + } func TestSupportedTimezones(t *testing.T) { From 68340d4715ba470462c4e870f824533f6559c6f1 Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Thu, 10 May 2018 11:22:10 -0400 Subject: [PATCH 03/10] Prevent divide by zero if there are no hubs (#8763) --- app/web_hub.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/app/web_hub.go b/app/web_hub.go index 18eb97c8ef..c9ca2f4f58 100644 --- a/app/web_hub.go +++ b/app/web_hub.go @@ -125,6 +125,10 @@ func (a *App) HubStop() { } func (a *App) GetHubForUserId(userId string) *Hub { + if len(a.Hubs) == 0 { + return nil + } + hash := fnv.New32a() hash.Write([]byte(userId)) index := hash.Sum32() % uint32(len(a.Hubs)) @@ -132,11 +136,17 @@ func (a *App) GetHubForUserId(userId string) *Hub { } func (a *App) HubRegister(webConn *WebConn) { - a.GetHubForUserId(webConn.UserId).Register(webConn) + hub := a.GetHubForUserId(webConn.UserId) + if hub != nil { + hub.Register(webConn) + } } func (a *App) HubUnregister(webConn *WebConn) { - a.GetHubForUserId(webConn.UserId).Unregister(webConn) + hub := a.GetHubForUserId(webConn.UserId) + if hub != nil { + hub.Unregister(webConn) + } } func (a *App) Publish(message *model.WebSocketEvent) { From 84499825763b32dad3b6c8a3a50290246c895a71 Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Thu, 10 May 2018 14:17:01 -0400 Subject: [PATCH 04/10] Attempt to fix hub nil panic (#8766) --- app/web_hub.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/web_hub.go b/app/web_hub.go index c9ca2f4f58..f69645f501 100644 --- a/app/web_hub.go +++ b/app/web_hub.go @@ -343,7 +343,7 @@ func (h *Hub) Unregister(webConn *WebConn) { } func (h *Hub) Broadcast(message *model.WebSocketEvent) { - if message != nil { + if h != nil && h.broadcast != nil && message != nil { h.broadcast <- message } } From 21d3b247d9645efa9471877cc36b46de92dc1d09 Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Thu, 10 May 2018 15:31:23 -0400 Subject: [PATCH 05/10] Comment out problematic TestS3TestConnection test (#8770) --- api4/system_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api4/system_test.go b/api4/system_test.go index f46ae74361..8e15f6ed27 100644 --- a/api4/system_test.go +++ b/api4/system_test.go @@ -570,7 +570,7 @@ func TestGetAnalyticsOld(t *testing.T) { CheckUnauthorizedStatus(t, resp) } -func TestS3TestConnection(t *testing.T) { +/*func TestS3TestConnection(t *testing.T) { th := Setup().InitBasic().InitSystemAdmin() defer th.TearDown() Client := th.Client @@ -626,7 +626,7 @@ func TestS3TestConnection(t *testing.T) { _, resp = th.SystemAdminClient.TestS3Connection(&config) CheckOKStatus(t, resp) -} +}*/ func TestSupportedTimezones(t *testing.T) { th := Setup().InitBasic() From 52674eb4c70865fadcde6dbd3d294ffbbbcb844c Mon Sep 17 00:00:00 2001 From: Derrick Anderson Date: Fri, 11 May 2018 10:44:42 -0400 Subject: [PATCH 06/10] add s3 test back in for master --- api4/system_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api4/system_test.go b/api4/system_test.go index 8e15f6ed27..f46ae74361 100644 --- a/api4/system_test.go +++ b/api4/system_test.go @@ -570,7 +570,7 @@ func TestGetAnalyticsOld(t *testing.T) { CheckUnauthorizedStatus(t, resp) } -/*func TestS3TestConnection(t *testing.T) { +func TestS3TestConnection(t *testing.T) { th := Setup().InitBasic().InitSystemAdmin() defer th.TearDown() Client := th.Client @@ -626,7 +626,7 @@ func TestGetAnalyticsOld(t *testing.T) { _, resp = th.SystemAdminClient.TestS3Connection(&config) CheckOKStatus(t, resp) -}*/ +} func TestSupportedTimezones(t *testing.T) { th := Setup().InitBasic() From 7e7c55198719337e7cb39b07c0d5a48c0a6908de Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Mon, 14 May 2018 09:43:06 -0400 Subject: [PATCH 07/10] MM-9739: deprecate the now unused Channels.ExtraUpdateAt (#8762) --- i18n/en.json | 4 ---- model/channel.go | 6 ----- store/sqlstore/channel_store.go | 41 +++----------------------------- store/storetest/channel_store.go | 33 ------------------------- 4 files changed, 3 insertions(+), 81 deletions(-) diff --git a/i18n/en.json b/i18n/en.json index c0b1d0558a..d42c6a544d 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -5926,10 +5926,6 @@ "id": "store.sql_channel.delete.channel.app_error", "translation": "We couldn't delete the channel" }, - { - "id": "store.sql_channel.extra_updated.app_error", - "translation": "Problem updating members last updated time" - }, { "id": "store.sql_channel.get.existing.app_error", "translation": "We couldn't find the existing channel" diff --git a/model/channel.go b/model/channel.go index df68202d6b..749b8dc94e 100644 --- a/model/channel.go +++ b/model/channel.go @@ -44,7 +44,6 @@ type Channel struct { Purpose string `json:"purpose"` LastPostAt int64 `json:"last_post_at"` TotalMsgCount int64 `json:"total_msg_count"` - ExtraUpdateAt int64 `json:"extra_update_at"` CreatorId string `json:"creator_id"` } @@ -133,17 +132,12 @@ func (o *Channel) PreSave() { o.CreateAt = GetMillis() o.UpdateAt = o.CreateAt - o.ExtraUpdateAt = o.CreateAt } func (o *Channel) PreUpdate() { o.UpdateAt = GetMillis() } -func (o *Channel) ExtraUpdated() { - o.ExtraUpdateAt = GetMillis() -} - func (o *Channel) IsGroupOrDirect() bool { return o.Type == CHANNEL_DIRECT || o.Type == CHANNEL_GROUP } diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index 3bd87961a9..eebc3ad697 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -270,25 +270,6 @@ func (s SqlChannelStore) Update(channel *model.Channel) store.StoreChannel { }) } -func (s SqlChannelStore) extraUpdated(channel *model.Channel) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - channel.ExtraUpdated() - - _, err := s.GetMaster().Exec( - `UPDATE - Channels - SET - ExtraUpdateAt = :Time - WHERE - Id = :Id`, - map[string]interface{}{"Id": channel.Id, "Time": channel.ExtraUpdateAt}) - - if err != nil { - result.Err = model.NewAppError("SqlChannelStore.extraUpdated", "store.sql_channel.extra_updated.app_error", nil, "id="+channel.Id+", "+err.Error(), http.StatusInternalServerError) - } - }) -} - func (s SqlChannelStore) GetChannelUnread(channelId, userId string) store.StoreChannel { return store.Do(func(result *store.StoreResult) { var unreadChannel model.ChannelUnread @@ -750,10 +731,6 @@ func (s SqlChannelStore) SaveMember(member *model.ChannelMember) store.StoreChan if err := transaction.Commit(); err != nil { result.Err = model.NewAppError("SqlChannelStore.SaveMember", "store.sql_channel.save_member.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) } - // If successfull record members have changed in channel - if mu := <-s.extraUpdated(channel); mu.Err != nil { - result.Err = mu.Err - } } } } @@ -1055,21 +1032,9 @@ func (s SqlChannelStore) GetMemberCount(channelId string, allowFromCache bool) s func (s SqlChannelStore) RemoveMember(channelId string, userId string) store.StoreChannel { return store.Do(func(result *store.StoreResult) { - // Grab the channel we are saving this member to - if cr := <-s.Get(channelId, true); cr.Err != nil { - result.Err = cr.Err - } else { - channel := cr.Data.(*model.Channel) - - _, err := s.GetMaster().Exec("DELETE FROM ChannelMembers WHERE ChannelId = :ChannelId AND UserId = :UserId", map[string]interface{}{"ChannelId": channelId, "UserId": userId}) - if err != nil { - result.Err = model.NewAppError("SqlChannelStore.RemoveMember", "store.sql_channel.remove_member.app_error", nil, "channel_id="+channelId+", user_id="+userId+", "+err.Error(), http.StatusInternalServerError) - } else { - // If successfull record members have changed in channel - if mu := <-s.extraUpdated(channel); mu.Err != nil { - result.Err = mu.Err - } - } + _, err := s.GetMaster().Exec("DELETE FROM ChannelMembers WHERE ChannelId = :ChannelId AND UserId = :UserId", map[string]interface{}{"ChannelId": channelId, "UserId": userId}) + if err != nil { + result.Err = model.NewAppError("SqlChannelStore.RemoveMember", "store.sql_channel.remove_member.app_error", nil, "channel_id="+channelId+", user_id="+userId+", "+err.Error(), http.StatusInternalServerError) } }) } diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index 481631783c..ccb7b87b8f 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -713,9 +713,6 @@ func testChannelMemberStore(t *testing.T, ss store.Store) { c1.Type = model.CHANNEL_OPEN c1 = *store.Must(ss.Channel().Save(&c1, -1)).(*model.Channel) - c1t1 := (<-ss.Channel().Get(c1.Id, false)).Data.(*model.Channel) - t1 := c1t1.ExtraUpdateAt - u1 := model.User{} u1.Email = model.NewId() u1.Nickname = model.NewId() @@ -740,13 +737,6 @@ func testChannelMemberStore(t *testing.T, ss store.Store) { o2.NotifyProps = model.GetDefaultChannelNotifyProps() store.Must(ss.Channel().SaveMember(&o2)) - c1t2 := (<-ss.Channel().Get(c1.Id, false)).Data.(*model.Channel) - t2 := c1t2.ExtraUpdateAt - - if t2 <= t1 { - t.Fatal("Member update time incorrect") - } - count := (<-ss.Channel().GetMemberCount(o1.ChannelId, true)).Data.(int64) if count != 2 { t.Fatal("should have saved 2 members") @@ -777,13 +767,6 @@ func testChannelMemberStore(t *testing.T, ss store.Store) { t.Fatal("should have removed 1 member") } - c1t3 := (<-ss.Channel().Get(c1.Id, false)).Data.(*model.Channel) - t3 := c1t3.ExtraUpdateAt - - if t3 <= t2 || t3 <= t1 { - t.Fatal("Member update time incorrect on delete") - } - member := (<-ss.Channel().GetMember(o1.ChannelId, o1.UserId)).Data.(*model.ChannelMember) if member.ChannelId != o1.ChannelId { t.Fatal("should have go member") @@ -792,12 +775,6 @@ func testChannelMemberStore(t *testing.T, ss store.Store) { if err := (<-ss.Channel().SaveMember(&o1)).Err; err == nil { t.Fatal("Should have been a duplicate") } - - c1t4 := (<-ss.Channel().Get(c1.Id, false)).Data.(*model.Channel) - t4 := c1t4.ExtraUpdateAt - if t4 != t3 { - t.Fatal("Should not update time upon failure") - } } func testChannelDeleteMemberStore(t *testing.T, ss store.Store) { @@ -808,9 +785,6 @@ func testChannelDeleteMemberStore(t *testing.T, ss store.Store) { c1.Type = model.CHANNEL_OPEN c1 = *store.Must(ss.Channel().Save(&c1, -1)).(*model.Channel) - c1t1 := (<-ss.Channel().Get(c1.Id, false)).Data.(*model.Channel) - t1 := c1t1.ExtraUpdateAt - u1 := model.User{} u1.Email = model.NewId() u1.Nickname = model.NewId() @@ -835,13 +809,6 @@ func testChannelDeleteMemberStore(t *testing.T, ss store.Store) { o2.NotifyProps = model.GetDefaultChannelNotifyProps() store.Must(ss.Channel().SaveMember(&o2)) - c1t2 := (<-ss.Channel().Get(c1.Id, false)).Data.(*model.Channel) - t2 := c1t2.ExtraUpdateAt - - if t2 <= t1 { - t.Fatal("Member update time incorrect") - } - count := (<-ss.Channel().GetMemberCount(o1.ChannelId, false)).Data.(int64) if count != 2 { t.Fatal("should have saved 2 members") From 47250c6629416b628a19e5571ac89f7b4646418c Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Mon, 14 May 2018 10:24:58 -0400 Subject: [PATCH 08/10] Refactor context out of API packages (#8755) * Refactor context out of API packages * Update function names per feedback * Move webhook handlers to web and fix web tests * Move more webhook tests out of api package * Fix static handler --- api/webhook_test.go | 154 ------------------------ api4/api.go | 16 +-- api4/emoji.go | 3 +- api4/handlers.go | 67 +++++++++++ api4/webhook.go | 91 -------------- api4/webhook_test.go | 45 ------- cmd/commands/server.go | 2 +- {api4 => web}/context.go | 170 +------------------------- {api4 => web}/context_test.go | 6 +- web/handlers.go | 158 +++++++++++++++++++++++++ {api4 => web}/params.go | 8 +- web/static.go | 84 +++++++++++++ web/web.go | 99 +++++----------- web/web_test.go | 99 ++++++---------- web/webhook.go | 101 ++++++++++++++++ web/webhook_test.go | 216 ++++++++++++++++++++++++++++++++++ 16 files changed, 708 insertions(+), 611 deletions(-) create mode 100644 api4/handlers.go rename {api4 => web}/context.go (70%) rename {api4 => web}/context_test.go (81%) create mode 100644 web/handlers.go rename {api4 => web}/params.go (96%) create mode 100644 web/static.go create mode 100644 web/webhook.go create mode 100644 web/webhook_test.go diff --git a/api/webhook_test.go b/api/webhook_test.go index 0b3073f833..c9ca7d783b 100644 --- a/api/webhook_test.go +++ b/api/webhook_test.go @@ -4,8 +4,6 @@ package api import ( - "fmt" - "net/http" "testing" "github.com/mattermost/mattermost-server/model" @@ -968,155 +966,3 @@ func TestRegenOutgoingHookToken(t *testing.T) { t.Fatal("should have errored - webhooks turned off") } } - -func TestIncomingWebhooks(t *testing.T) { - th := Setup().InitBasic().InitSystemAdmin() - defer th.TearDown() - - Client := th.SystemAdminClient - team := th.SystemAdminTeam - channel1 := th.CreateChannel(Client, team) - user2 := th.CreateUser(Client) - th.LinkUserToTeam(user2, team) - - th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableIncomingWebhooks = true }) - - hook := &model.IncomingWebhook{ChannelId: channel1.Id} - hook = Client.Must(Client.CreateIncomingWebhook(hook)).Data.(*model.IncomingWebhook) - - url := "/hooks/" + hook.Id - text := `this is a \"test\" - that contains a newline and a tab` - - if _, err := Client.DoPost(url, "{\"text\":\"this is a test\"}", "application/json"); err != nil { - t.Fatal(err) - } - - if _, err := Client.DoPost(url, "{\"text\":\""+text+"\"}", "application/json"); err != nil { - t.Fatal(err) - } - - if _, err := Client.DoPost(url, fmt.Sprintf("{\"text\":\"this is a test\", \"channel\":\"%s\"}", channel1.Name), "application/json"); err != nil { - t.Fatal(err) - } - - if _, err := Client.DoPost(url, fmt.Sprintf("{\"text\":\"this is a test\", \"channel\":\"#%s\"}", channel1.Name), "application/json"); err != nil { - t.Fatal(err) - } - - if _, err := Client.DoPost(url, fmt.Sprintf("{\"text\":\"this is a test\", \"channel\":\"@%s\"}", user2.Username), "application/json"); err != nil { - t.Fatal(err) - } - - if _, err := Client.DoPost(url, "payload={\"text\":\"this is a test\"}", "application/x-www-form-urlencoded"); err != nil { - t.Fatal(err) - } - - if _, err := Client.DoPost(url, "payload={\"text\":\""+text+"\"}", "application/x-www-form-urlencoded"); err != nil { - t.Fatal(err) - } - - if _, err := th.BasicClient.DoPost(url, fmt.Sprintf("{\"text\":\"this is a test\", \"channel\":\"%s\"}", model.DEFAULT_CHANNEL), "application/json"); err != nil { - t.Fatal("should not have failed -- ExperimentalTownSquareIsReadOnly is false and it's not a read only channel") - } - - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.ExperimentalTownSquareIsReadOnly = true }) - th.App.SetLicense(model.NewTestLicense()) - - if _, err := th.BasicClient.DoPost(url, fmt.Sprintf("{\"text\":\"this is a test\", \"channel\":\"%s\"}", model.DEFAULT_CHANNEL), "application/json"); err == nil { - t.Fatal("should have failed -- ExperimentalTownSquareIsReadOnly is true and it's a read only channel") - } - - attachmentPayload := `{ - "text": "this is a test", - "attachments": [ - { - "fallback": "Required plain-text summary of the attachment.", - - "color": "#36a64f", - - "pretext": "Optional text that appears above the attachment block", - - "author_name": "Bobby Tables", - "author_link": "http://flickr.com/bobby/", - "author_icon": "http://flickr.com/icons/bobby.jpg", - - "title": "Slack API Documentation", - "title_link": "https://api.slack.com/", - - "text": "Optional text that appears within the attachment", - - "fields": [ - { - "title": "Priority", - "value": "High", - "short": false - } - ], - - "image_url": "http://my-website.com/path/to/image.jpg", - "thumb_url": "http://example.com/path/to/thumb.png" - } - ] - }` - - if _, err := Client.DoPost(url, attachmentPayload, "application/json"); err != nil { - t.Fatal(err) - } - - if _, err := Client.DoPost(url, "{\"text\":\"\"}", "application/json"); err == nil || err.StatusCode != http.StatusBadRequest { - t.Fatal("should have failed - no text") - } - - tooLongText := "" - for i := 0; i < 8200; i++ { - tooLongText += "a" - } - - if _, err := Client.DoPost(url, "{\"text\":\""+tooLongText+"\"}", "application/json"); err != nil { - t.Fatal(err) - } - - attachmentPayload = `{ - "text": "this is a test", - "attachments": [ - { - "fallback": "Required plain-text summary of the attachment.", - - "color": "#36a64f", - - "pretext": "Optional text that appears above the attachment block", - - "author_name": "Bobby Tables", - "author_link": "http://flickr.com/bobby/", - "author_icon": "http://flickr.com/icons/bobby.jpg", - - "title": "Slack API Documentation", - "title_link": "https://api.slack.com/", - - "text": "` + tooLongText + `", - - "fields": [ - { - "title": "Priority", - "value": "High", - "short": false - } - ], - - "image_url": "http://my-website.com/path/to/image.jpg", - "thumb_url": "http://example.com/path/to/thumb.png" - } - ] - }` - - if _, err := Client.DoPost(url, attachmentPayload, "application/json"); err != nil { - t.Fatal(err) - } - - th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableIncomingWebhooks = false }) - - if _, err := Client.DoPost(url, "{\"text\":\"this is a test\"}", "application/json"); err == nil { - t.Fatal("should have failed - webhooks turned off") - } -} diff --git a/api4/api.go b/api4/api.go index d36c3e3eeb..acce923c05 100644 --- a/api4/api.go +++ b/api4/api.go @@ -4,14 +4,12 @@ package api4 import ( - "fmt" "net/http" "github.com/gorilla/mux" "github.com/mattermost/mattermost-server/app" - "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" - "github.com/mattermost/mattermost-server/utils" + "github.com/mattermost/mattermost-server/web" _ "github.com/nicksnyder/go-i18n/i18n" ) @@ -231,7 +229,7 @@ func Init(a *app.App, root *mux.Router, full bool) *API { api.InitRole() api.InitImage() - root.Handle("/api/v4/{anything:.*}", http.HandlerFunc(Handle404)) + root.Handle("/api/v4/{anything:.*}", http.HandlerFunc(api.Handle404)) // REMOVE CONDITION WHEN APIv3 REMOVED if full { @@ -241,14 +239,8 @@ func Init(a *app.App, root *mux.Router, full bool) *API { return api } -func Handle404(w http.ResponseWriter, r *http.Request) { - err := model.NewAppError("Handle404", "api.context.404.app_error", nil, "", http.StatusNotFound) - - mlog.Debug(fmt.Sprintf("%v: code=404 ip=%v", r.URL.Path, utils.GetIpAddress(r))) - - w.WriteHeader(err.StatusCode) - err.DetailedError = "There doesn't appear to be an api call for the url='" + r.URL.Path + "'." - w.Write([]byte(err.ToJson())) +func (api *API) Handle404(w http.ResponseWriter, r *http.Request) { + web.Handle404(api.App, w, r) } func ReturnStatusOK(w http.ResponseWriter) { diff --git a/api4/emoji.go b/api4/emoji.go index ae4a35dd56..cfb5dd6abc 100644 --- a/api4/emoji.go +++ b/api4/emoji.go @@ -9,6 +9,7 @@ import ( "github.com/mattermost/mattermost-server/app" "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/web" ) const ( @@ -197,7 +198,7 @@ func searchEmojis(c *Context, w http.ResponseWriter, r *http.Request) { return } - emojis, err := c.App.SearchEmoji(emojiSearch.Term, emojiSearch.PrefixOnly, PER_PAGE_MAXIMUM) + emojis, err := c.App.SearchEmoji(emojiSearch.Term, emojiSearch.PrefixOnly, web.PER_PAGE_MAXIMUM) if err != nil { c.Err = err return diff --git a/api4/handlers.go b/api4/handlers.go new file mode 100644 index 0000000000..74e2fc88d3 --- /dev/null +++ b/api4/handlers.go @@ -0,0 +1,67 @@ +// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package api4 + +import ( + "net/http" + + "github.com/mattermost/mattermost-server/web" +) + +type Context = web.Context + +func (api *API) ApiHandler(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { + return &web.Handler{ + App: api.App, + HandleFunc: h, + RequireSession: false, + TrustRequester: false, + RequireMfa: false, + IsStatic: false, + } +} + +func (api *API) ApiSessionRequired(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { + return &web.Handler{ + App: api.App, + HandleFunc: h, + RequireSession: true, + TrustRequester: false, + RequireMfa: true, + IsStatic: false, + } +} + +func (api *API) ApiSessionRequiredMfa(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { + return &web.Handler{ + App: api.App, + HandleFunc: h, + RequireSession: true, + TrustRequester: false, + RequireMfa: false, + IsStatic: false, + } +} + +func (api *API) ApiHandlerTrustRequester(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { + return &web.Handler{ + App: api.App, + HandleFunc: h, + RequireSession: false, + TrustRequester: true, + RequireMfa: false, + IsStatic: false, + } +} + +func (api *API) ApiSessionRequiredTrustRequester(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { + return &web.Handler{ + App: api.App, + HandleFunc: h, + RequireSession: true, + TrustRequester: true, + RequireMfa: true, + IsStatic: false, + } +} diff --git a/api4/webhook.go b/api4/webhook.go index fadc3fbf39..ec90c6f3a1 100644 --- a/api4/webhook.go +++ b/api4/webhook.go @@ -4,14 +4,8 @@ package api4 import ( - "fmt" - "io" "net/http" - "strings" - "github.com/gorilla/mux" - "github.com/gorilla/schema" - "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" ) @@ -28,12 +22,6 @@ func (api *API) InitWebhook() { api.BaseRoutes.OutgoingHook.Handle("", api.ApiSessionRequired(updateOutgoingHook)).Methods("PUT") api.BaseRoutes.OutgoingHook.Handle("", api.ApiSessionRequired(deleteOutgoingHook)).Methods("DELETE") api.BaseRoutes.OutgoingHook.Handle("/regen_token", api.ApiSessionRequired(regenOutgoingHookToken)).Methods("POST") - - api.BaseRoutes.Root.Handle("/hooks/commands/{id:[A-Za-z0-9]+}", api.ApiHandler(commandWebhook)).Methods("POST") - api.BaseRoutes.Root.Handle("/hooks/{id:[A-Za-z0-9]+}", api.ApiHandler(incomingWebhook)).Methods("POST") - - // Old endpoint for backwards compatibility - api.BaseRoutes.Root.Handle("/api/v3/teams/{team_id:[A-Za-z0-9]+}/hooks/{id:[A-Za-z0-9]+}", api.ApiHandler(incomingWebhook)).Methods("POST") } func createIncomingHook(c *Context, w http.ResponseWriter, r *http.Request) { @@ -454,82 +442,3 @@ func deleteOutgoingHook(c *Context, w http.ResponseWriter, r *http.Request) { c.LogAudit("success") ReturnStatusOK(w) } - -func incomingWebhook(c *Context, w http.ResponseWriter, r *http.Request) { - params := mux.Vars(r) - id := params["id"] - - r.ParseForm() - - var err *model.AppError - incomingWebhookPayload := &model.IncomingWebhookRequest{} - contentType := r.Header.Get("Content-Type") - if strings.Split(contentType, "; ")[0] == "application/x-www-form-urlencoded" { - payload := strings.NewReader(r.FormValue("payload")) - - incomingWebhookPayload, err = decodePayload(payload) - if err != nil { - c.Err = err - return - } - } else if strings.HasPrefix(contentType, "multipart/form-data") { - r.ParseMultipartForm(0) - - decoder := schema.NewDecoder() - err := decoder.Decode(incomingWebhookPayload, r.PostForm) - - if err != nil { - c.Err = model.NewAppError("incomingWebhook", "api.webhook.incoming.error", nil, err.Error(), http.StatusBadRequest) - return - } - } else { - incomingWebhookPayload, err = decodePayload(r.Body) - if err != nil { - c.Err = err - return - } - } - - if c.App.Config().LogSettings.EnableWebhookDebugging { - mlog.Debug(fmt.Sprint("Incoming webhook received. Content=", incomingWebhookPayload.ToJson())) - } - - err = c.App.HandleIncomingWebhook(id, incomingWebhookPayload) - if err != nil { - c.Err = err - return - } - - w.Header().Set("Content-Type", "text/plain") - w.Write([]byte("ok")) -} - -func commandWebhook(c *Context, w http.ResponseWriter, r *http.Request) { - params := mux.Vars(r) - id := params["id"] - - response, err := model.CommandResponseFromHTTPBody(r.Header.Get("Content-Type"), r.Body) - if err != nil { - c.Err = model.NewAppError("commandWebhook", "web.command_webhook.parse.app_error", nil, err.Error(), http.StatusBadRequest) - return - } - - appErr := c.App.HandleCommandWebhook(id, response) - if appErr != nil { - c.Err = appErr - return - } - - w.Header().Set("Content-Type", "text/plain") - w.Write([]byte("ok")) -} - -func decodePayload(payload io.Reader) (*model.IncomingWebhookRequest, *model.AppError) { - incomingWebhookPayload, decodeError := model.IncomingWebhookRequestFromJson(payload) - - if decodeError != nil { - return nil, decodeError - } - - return incomingWebhookPayload, nil -} diff --git a/api4/webhook_test.go b/api4/webhook_test.go index e983b6461c..441fb8bb78 100644 --- a/api4/webhook_test.go +++ b/api4/webhook_test.go @@ -4,8 +4,6 @@ package api4 import ( - "bytes" - "net/http" "testing" "github.com/stretchr/testify/assert" @@ -892,46 +890,3 @@ func TestDeleteOutgoingHook(t *testing.T) { CheckForbiddenStatus(t, resp) }) } - -func TestCommandWebhooks(t *testing.T) { - th := Setup().InitBasic().InitSystemAdmin() - defer th.TearDown() - - Client := th.SystemAdminClient - - cmd := &model.Command{ - CreatorId: th.BasicUser.Id, - TeamId: th.BasicTeam.Id, - URL: "http://nowhere.com", - Method: model.COMMAND_METHOD_POST, - Trigger: "delayed"} - - cmd, _ = Client.CreateCommand(cmd) - args := &model.CommandArgs{ - TeamId: th.BasicTeam.Id, - UserId: th.BasicUser.Id, - ChannelId: th.BasicChannel.Id, - } - hook, err := th.App.CreateCommandWebhook(cmd.Id, args) - if err != nil { - t.Fatal(err) - } - - if resp, _ := http.Post(Client.Url+"/hooks/commands/123123123123", "application/json", bytes.NewBufferString(`{"text":"this is a test"}`)); resp.StatusCode != http.StatusNotFound { - t.Fatal("expected not-found for non-existent hook") - } - - if resp, err := http.Post(Client.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString(`{"text":"invalid`)); err != nil || resp.StatusCode != http.StatusBadRequest { - t.Fatal(err) - } - - for i := 0; i < 5; i++ { - if resp, err := http.Post(Client.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString(`{"text":"this is a test"}`)); err != nil || resp.StatusCode != http.StatusOK { - t.Fatal(err) - } - } - - if resp, _ := http.Post(Client.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString(`{"text":"this is a test"}`)); resp.StatusCode != http.StatusBadRequest { - t.Fatal("expected error for sixth usage") - } -} diff --git a/cmd/commands/server.go b/cmd/commands/server.go index 77f195f4bd..441eb82bcf 100644 --- a/cmd/commands/server.go +++ b/cmd/commands/server.go @@ -107,7 +107,7 @@ func runServer(configFileLocation string, disableConfigWatch bool, interruptChan api4.Init(a, a.Srv.Router, false) api3 := api.Init(a, a.Srv.Router) wsapi.Init(a, a.Srv.WebSocketRouter) - web.Init(api3) + web.NewWeb(a, a.Srv.Router) license := a.License() diff --git a/api4/context.go b/web/context.go similarity index 70% rename from api4/context.go rename to web/context.go index c965e1d804..bf7cfcb8d8 100644 --- a/api4/context.go +++ b/web/context.go @@ -1,14 +1,13 @@ -// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See License.txt for license information. -package api4 +package web import ( "fmt" "net/http" "regexp" "strings" - "time" goi18n "github.com/nicksnyder/go-i18n/i18n" @@ -21,7 +20,7 @@ import ( type Context struct { App *app.App Session model.Session - Params *ApiParams + Params *Params Err *model.AppError T goi18n.TranslateFunc RequestId string @@ -30,169 +29,6 @@ type Context struct { siteURLHeader string } -func (api *API) ApiHandler(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { - return &handler{ - app: api.App, - handleFunc: h, - requireSession: false, - trustRequester: false, - requireMfa: false, - } -} - -func (api *API) ApiSessionRequired(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { - return &handler{ - app: api.App, - handleFunc: h, - requireSession: true, - trustRequester: false, - requireMfa: true, - } -} - -func (api *API) ApiSessionRequiredMfa(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { - return &handler{ - app: api.App, - handleFunc: h, - requireSession: true, - trustRequester: false, - requireMfa: false, - } -} - -func (api *API) ApiHandlerTrustRequester(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { - return &handler{ - app: api.App, - handleFunc: h, - requireSession: false, - trustRequester: true, - requireMfa: false, - } -} - -func (api *API) ApiSessionRequiredTrustRequester(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { - return &handler{ - app: api.App, - handleFunc: h, - requireSession: true, - trustRequester: true, - requireMfa: true, - } -} - -type handler struct { - app *app.App - handleFunc func(*Context, http.ResponseWriter, *http.Request) - requireSession bool - trustRequester bool - requireMfa bool -} - -func (h handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - now := time.Now() - mlog.Debug(fmt.Sprintf("%v - %v", r.Method, r.URL.Path)) - - c := &Context{} - c.App = h.app - c.T, _ = utils.GetTranslationsAndLocale(w, r) - c.RequestId = model.NewId() - c.IpAddress = utils.GetIpAddress(r) - c.Params = ApiParamsFromRequest(r) - - token, tokenLocation := app.ParseAuthTokenFromRequest(r) - - // CSRF Check - if tokenLocation == app.TokenLocationCookie && h.requireSession && !h.trustRequester { - if r.Header.Get(model.HEADER_REQUESTED_WITH) != model.HEADER_REQUESTED_WITH_XML { - c.Err = model.NewAppError("ServeHTTP", "api.context.session_expired.app_error", nil, "token="+token+" Appears to be a CSRF attempt", http.StatusUnauthorized) - token = "" - } - } - - c.SetSiteURLHeader(app.GetProtocol(r) + "://" + r.Host) - - w.Header().Set(model.HEADER_REQUEST_ID, c.RequestId) - w.Header().Set(model.HEADER_VERSION_ID, fmt.Sprintf("%v.%v.%v.%v", model.CurrentVersion, model.BuildNumber, c.App.ClientConfigHash(), c.App.License() != nil)) - - w.Header().Set("Content-Type", "application/json") - - if r.Method == "GET" { - w.Header().Set("Expires", "0") - } - - if len(token) != 0 { - session, err := c.App.GetSession(token) - - if err != nil { - mlog.Info(fmt.Sprintf("Invalid session err=%v", err.Error())) - if err.StatusCode == http.StatusInternalServerError { - c.Err = err - } else if h.requireSession { - c.RemoveSessionCookie(w, r) - c.Err = model.NewAppError("ServeHTTP", "api.context.session_expired.app_error", nil, "token="+token, http.StatusUnauthorized) - } - } else if !session.IsOAuth && tokenLocation == app.TokenLocationQueryString { - c.Err = model.NewAppError("ServeHTTP", "api.context.token_provided.app_error", nil, "token="+token, http.StatusUnauthorized) - } else { - c.Session = *session - } - - // Rate limit by UserID - if c.App.Srv.RateLimiter != nil && c.App.Srv.RateLimiter.UserIdRateLimit(c.Session.UserId, w) { - return - } - } - - c.Path = r.URL.Path - - if c.Err == nil && h.requireSession { - c.SessionRequired() - } - - if c.Err == nil && h.requireMfa { - c.MfaRequired() - } - - if c.Err == nil { - h.handleFunc(c, w, r) - } - - // Handle errors that have occurred - if c.Err != nil { - c.Err.Translate(c.T) - c.Err.RequestId = c.RequestId - - if c.Err.Id == "api.context.session_expired.app_error" { - c.LogInfo(c.Err) - } else { - c.LogError(c.Err) - } - - c.Err.Where = r.URL.Path - - // Block out detailed error when not in developer mode - if !*c.App.Config().ServiceSettings.EnableDeveloper { - c.Err.DetailedError = "" - } - - w.WriteHeader(c.Err.StatusCode) - w.Write([]byte(c.Err.ToJson())) - - if c.App.Metrics != nil { - c.App.Metrics.IncrementHttpError() - } - } - - if c.App.Metrics != nil { - c.App.Metrics.IncrementHttpRequest() - - if r.URL.Path != model.API_URL_SUFFIX+"/websocket" { - elapsed := float64(time.Since(now)) / float64(time.Second) - c.App.Metrics.ObserveHttpRequestDuration(elapsed) - } - } -} - func (c *Context) LogAudit(extraInfo string) { audit := &model.Audit{UserId: c.Session.UserId, IpAddress: c.IpAddress, Action: c.Path, ExtraInfo: extraInfo, SessionId: c.Session.Id} if r := <-c.App.Srv.Store.Audit().Save(audit); r.Err != nil { diff --git a/api4/context_test.go b/web/context_test.go similarity index 81% rename from api4/context_test.go rename to web/context_test.go index 302b7b24bf..3fa6ebf22d 100644 --- a/api4/context_test.go +++ b/web/context_test.go @@ -1,4 +1,4 @@ -package api4 +package web import ( "net/http" @@ -8,7 +8,7 @@ import ( func TestRequireHookId(t *testing.T) { c := &Context{} t.Run("WhenHookIdIsValid", func(t *testing.T) { - c.Params = &ApiParams{HookId: "abcdefghijklmnopqrstuvwxyz"} + c.Params = &Params{HookId: "abcdefghijklmnopqrstuvwxyz"} c.RequireHookId() if c.Err != nil { @@ -17,7 +17,7 @@ func TestRequireHookId(t *testing.T) { }) t.Run("WhenHookIdIsInvalid", func(t *testing.T) { - c.Params = &ApiParams{HookId: "abc"} + c.Params = &Params{HookId: "abc"} c.RequireHookId() if c.Err == nil { diff --git a/web/handlers.go b/web/handlers.go new file mode 100644 index 0000000000..e2521674aa --- /dev/null +++ b/web/handlers.go @@ -0,0 +1,158 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package web + +import ( + "fmt" + "net/http" + "time" + + "github.com/mattermost/mattermost-server/app" + "github.com/mattermost/mattermost-server/mlog" + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/utils" +) + +func (w *Web) NewHandler(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { + return &Handler{ + App: w.App, + HandleFunc: h, + RequireSession: false, + TrustRequester: false, + RequireMfa: false, + IsStatic: false, + } +} + +func (w *Web) NewStaticHandler(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { + return &Handler{ + App: w.App, + HandleFunc: h, + RequireSession: false, + TrustRequester: false, + RequireMfa: false, + IsStatic: true, + } +} + +type Handler struct { + App *app.App + HandleFunc func(*Context, http.ResponseWriter, *http.Request) + RequireSession bool + TrustRequester bool + RequireMfa bool + IsStatic bool +} + +func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + now := time.Now() + mlog.Debug(fmt.Sprintf("%v - %v", r.Method, r.URL.Path)) + + c := &Context{} + c.App = h.App + c.T, _ = utils.GetTranslationsAndLocale(w, r) + c.RequestId = model.NewId() + c.IpAddress = utils.GetIpAddress(r) + c.Params = ParamsFromRequest(r) + + token, tokenLocation := app.ParseAuthTokenFromRequest(r) + + // CSRF Check + if tokenLocation == app.TokenLocationCookie && h.RequireSession && !h.TrustRequester { + if r.Header.Get(model.HEADER_REQUESTED_WITH) != model.HEADER_REQUESTED_WITH_XML { + c.Err = model.NewAppError("ServeHTTP", "api.context.session_expired.app_error", nil, "token="+token+" Appears to be a CSRF attempt", http.StatusUnauthorized) + token = "" + } + } + + c.SetSiteURLHeader(app.GetProtocol(r) + "://" + r.Host) + + w.Header().Set(model.HEADER_REQUEST_ID, c.RequestId) + w.Header().Set(model.HEADER_VERSION_ID, fmt.Sprintf("%v.%v.%v.%v", model.CurrentVersion, model.BuildNumber, c.App.ClientConfigHash(), c.App.License() != nil)) + + if h.IsStatic { + // Instruct the browser not to display us in an iframe unless is the same origin for anti-clickjacking + w.Header().Set("X-Frame-Options", "SAMEORIGIN") + w.Header().Set("Content-Security-Policy", "frame-ancestors 'self'") + } else { + // All api response bodies will be JSON formatted by default + w.Header().Set("Content-Type", "application/json") + + if r.Method == "GET" { + w.Header().Set("Expires", "0") + } + } + + if len(token) != 0 { + session, err := c.App.GetSession(token) + + if err != nil { + mlog.Info(fmt.Sprintf("Invalid session err=%v", err.Error())) + if err.StatusCode == http.StatusInternalServerError { + c.Err = err + } else if h.RequireSession { + c.RemoveSessionCookie(w, r) + c.Err = model.NewAppError("ServeHTTP", "api.context.session_expired.app_error", nil, "token="+token, http.StatusUnauthorized) + } + } else if !session.IsOAuth && tokenLocation == app.TokenLocationQueryString { + c.Err = model.NewAppError("ServeHTTP", "api.context.token_provided.app_error", nil, "token="+token, http.StatusUnauthorized) + } else { + c.Session = *session + } + + // Rate limit by UserID + if c.App.Srv.RateLimiter != nil && c.App.Srv.RateLimiter.UserIdRateLimit(c.Session.UserId, w) { + return + } + } + + c.Path = r.URL.Path + + if c.Err == nil && h.RequireSession { + c.SessionRequired() + } + + if c.Err == nil && h.RequireMfa { + c.MfaRequired() + } + + if c.Err == nil { + h.HandleFunc(c, w, r) + } + + // Handle errors that have occurred + if c.Err != nil { + c.Err.Translate(c.T) + c.Err.RequestId = c.RequestId + + if c.Err.Id == "api.context.session_expired.app_error" { + c.LogInfo(c.Err) + } else { + c.LogError(c.Err) + } + + c.Err.Where = r.URL.Path + + // Block out detailed error when not in developer mode + if !*c.App.Config().ServiceSettings.EnableDeveloper { + c.Err.DetailedError = "" + } + + w.WriteHeader(c.Err.StatusCode) + w.Write([]byte(c.Err.ToJson())) + + if c.App.Metrics != nil { + c.App.Metrics.IncrementHttpError() + } + } + + if c.App.Metrics != nil { + c.App.Metrics.IncrementHttpRequest() + + if r.URL.Path != model.API_URL_SUFFIX+"/websocket" { + elapsed := float64(time.Since(now)) / float64(time.Second) + c.App.Metrics.ObserveHttpRequestDuration(elapsed) + } + } +} diff --git a/api4/params.go b/web/params.go similarity index 96% rename from api4/params.go rename to web/params.go index e8e3f25e73..286c3f44f1 100644 --- a/api4/params.go +++ b/web/params.go @@ -1,7 +1,7 @@ // Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. // See License.txt for license information. -package api4 +package web import ( "net/http" @@ -19,7 +19,7 @@ const ( LOGS_PER_PAGE_MAXIMUM = 10000 ) -type ApiParams struct { +type Params struct { UserId string TeamId string InviteId string @@ -53,8 +53,8 @@ type ApiParams struct { Permanent bool } -func ApiParamsFromRequest(r *http.Request) *ApiParams { - params := &ApiParams{} +func ParamsFromRequest(r *http.Request) *Params { + params := &Params{} props := mux.Vars(r) query := r.URL.Query() diff --git a/web/static.go b/web/static.go new file mode 100644 index 0000000000..487526fdf1 --- /dev/null +++ b/web/static.go @@ -0,0 +1,84 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package web + +import ( + "fmt" + "net/http" + "path/filepath" + "strings" + + "github.com/NYTimes/gziphandler" + + "github.com/mattermost/mattermost-server/mlog" + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/utils" +) + +func (w *Web) InitStatic() { + if *w.App.Config().ServiceSettings.WebserverMode != "disabled" { + staticDir, _ := utils.FindDir(model.CLIENT_DIR) + mlog.Debug(fmt.Sprintf("Using client directory at %v", staticDir)) + + staticHandler := staticHandler(http.StripPrefix("/static/", http.FileServer(http.Dir(staticDir)))) + pluginHandler := pluginHandler(w.App.Config, http.StripPrefix("/static/plugins/", http.FileServer(http.Dir(*w.App.Config().PluginSettings.ClientDirectory)))) + + if *w.App.Config().ServiceSettings.WebserverMode == "gzip" { + staticHandler = gziphandler.GzipHandler(staticHandler) + pluginHandler = gziphandler.GzipHandler(pluginHandler) + } + + w.MainRouter.PathPrefix("/static/plugins/").Handler(pluginHandler) + w.MainRouter.PathPrefix("/static/").Handler(staticHandler) + w.MainRouter.Handle("/{anything:.*}", w.NewStaticHandler(root)).Methods("GET") + } +} + +func root(c *Context, w http.ResponseWriter, r *http.Request) { + + if !CheckClientCompatability(r.UserAgent()) { + w.Header().Set("Cache-Control", "no-store") + page := utils.NewHTMLTemplate(c.App.HTMLTemplates(), "unsupported_browser") + page.Props["Title"] = c.T("web.error.unsupported_browser.title") + page.Props["Message"] = c.T("web.error.unsupported_browser.message") + page.RenderToWriter(w) + return + } + + if IsApiCall(r) { + Handle404(c.App, w, r) + return + } + + w.Header().Set("Cache-Control", "no-cache, max-age=31556926, public") + + staticDir, _ := utils.FindDir(model.CLIENT_DIR) + http.ServeFile(w, r, filepath.Join(staticDir, "root.html")) +} + +func staticHandler(handler http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Cache-Control", "max-age=31556926, public") + if strings.HasSuffix(r.URL.Path, "/") { + http.NotFound(w, r) + return + } + handler.ServeHTTP(w, r) + }) +} + +func pluginHandler(config model.ConfigFunc, handler http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if *config().ServiceSettings.EnableDeveloper { + w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate") + } else { + w.Header().Set("Cache-Control", "max-age=31556926, public") + } + if strings.HasSuffix(r.URL.Path, "/") { + http.NotFound(w, r) + return + } + handler.ServeHTTP(w, r) + }) +} diff --git a/web/web.go b/web/web.go index 56a5ab6ac1..94363cfdee 100644 --- a/web/web.go +++ b/web/web.go @@ -6,69 +6,38 @@ package web import ( "fmt" "net/http" - "path/filepath" "strings" - "github.com/NYTimes/gziphandler" "github.com/avct/uasurfer" + "github.com/gorilla/mux" - "github.com/mattermost/mattermost-server/api" + "github.com/mattermost/mattermost-server/app" "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/utils" ) -func Init(api3 *api.API) { +type Web struct { + App *app.App + MainRouter *mux.Router +} + +func NewWeb(a *app.App, root *mux.Router) *Web { mlog.Debug("Initializing web routes") - mainrouter := api3.BaseRoutes.Root - - if *api3.App.Config().ServiceSettings.WebserverMode != "disabled" { - staticDir, _ := utils.FindDir(model.CLIENT_DIR) - mlog.Debug(fmt.Sprintf("Using client directory at %v", staticDir)) - - staticHandler := staticHandler(http.StripPrefix("/static/", http.FileServer(http.Dir(staticDir)))) - pluginHandler := pluginHandler(api3.App.Config, http.StripPrefix("/static/plugins/", http.FileServer(http.Dir(*api3.App.Config().PluginSettings.ClientDirectory)))) - - if *api3.App.Config().ServiceSettings.WebserverMode == "gzip" { - staticHandler = gziphandler.GzipHandler(staticHandler) - pluginHandler = gziphandler.GzipHandler(pluginHandler) - } - - mainrouter.PathPrefix("/static/plugins/").Handler(pluginHandler) - mainrouter.PathPrefix("/static/").Handler(staticHandler) - mainrouter.Handle("/{anything:.*}", api3.AppHandlerIndependent(root)).Methods("GET") + web := &Web{ + App: a, + MainRouter: root, } + + web.InitStatic() + web.InitWebhooks() + + return web } -func staticHandler(handler http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Cache-Control", "max-age=31556926, public") - if strings.HasSuffix(r.URL.Path, "/") { - http.NotFound(w, r) - return - } - handler.ServeHTTP(w, r) - }) -} - -func pluginHandler(config model.ConfigFunc, handler http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if *config().ServiceSettings.EnableDeveloper { - w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate") - } else { - w.Header().Set("Cache-Control", "max-age=31556926, public") - } - if strings.HasSuffix(r.URL.Path, "/") { - http.NotFound(w, r) - return - } - handler.ServeHTTP(w, r) - }) -} - -// Due to the complexities of UA detection and the ramifications of a misdetection only older Safari and IE browsers throw incompatibility errors. - +// Due to the complexities of UA detection and the ramifications of a misdetection +// only older Safari and IE browsers throw incompatibility errors. // Map should be of minimum required browser version. var browserMinimumSupported = map[string]int{ "BrowserIE": 11, @@ -85,24 +54,20 @@ func CheckClientCompatability(agentString string) bool { return true } -func root(c *api.Context, w http.ResponseWriter, r *http.Request) { +func Handle404(a *app.App, w http.ResponseWriter, r *http.Request) { + err := model.NewAppError("Handle404", "api.context.404.app_error", nil, "", http.StatusNotFound) - if !CheckClientCompatability(r.UserAgent()) { - w.Header().Set("Cache-Control", "no-store") - page := utils.NewHTMLTemplate(c.App.HTMLTemplates(), "unsupported_browser") - page.Props["Title"] = c.T("web.error.unsupported_browser.title") - page.Props["Message"] = c.T("web.error.unsupported_browser.message") - page.RenderToWriter(w) - return + mlog.Debug(fmt.Sprintf("%v: code=404 ip=%v", r.URL.Path, utils.GetIpAddress(r))) + + if IsApiCall(r) { + w.WriteHeader(err.StatusCode) + err.DetailedError = "There doesn't appear to be an api call for the url='" + r.URL.Path + "'. Typo? are you missing a team_id or user_id as part of the url?" + w.Write([]byte(err.ToJson())) + } else { + utils.RenderWebAppError(w, r, err, a.AsymmetricSigningKey()) } - - if api.IsApiCall(r) { - api.Handle404(c.App, w, r) - return - } - - w.Header().Set("Cache-Control", "no-cache, max-age=31556926, public") - - staticDir, _ := utils.FindDir(model.CLIENT_DIR) - http.ServeFile(w, r, filepath.Join(staticDir, "root.html")) +} + +func IsApiCall(r *http.Request) bool { + return strings.Index(r.URL.Path, "/api/") == 0 } diff --git a/web/web_test.go b/web/web_test.go index 12099709ec..4497f00ccb 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -8,8 +8,6 @@ import ( "os" "testing" - "github.com/mattermost/mattermost-server/api" - "github.com/mattermost/mattermost-server/api4" "github.com/mattermost/mattermost-server/app" "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" @@ -38,7 +36,14 @@ func StopTestStore() { } } -func Setup() *app.App { +type TestHelper struct { + App *app.App + BasicUser *model.User + BasicChannel *model.Channel + BasicTeam *model.Team +} + +func Setup() *TestHelper { a, err := app.New(app.StoreOverride(testStore), app.DisableConfigWatch) if err != nil { panic(err) @@ -50,9 +55,8 @@ func Setup() *app.App { panic(serverErr) } a.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ListenAddress = prevListenAddress }) - api4.Init(a, a.Srv.Router, false) - api3 := api.Init(a, a.Srv.Router) - Init(api3) + + NewWeb(a, a.Srv.Router) URL = fmt.Sprintf("http://localhost:%v", a.Srv.ListenAddr.Port) ApiClient = model.NewClient(URL) @@ -65,11 +69,31 @@ func Setup() *app.App { *cfg.ServiceSettings.EnableAPIv3 = true }) - return a + th := &TestHelper{ + App: a, + } + + return th } -func TearDown(a *app.App) { - a.Shutdown() +func (th *TestHelper) InitBasic() *TestHelper { + user, _ := th.App.CreateUser(&model.User{Email: model.NewId() + "success+test@simulator.amazonses.com", Nickname: "Corey Hulen", Password: "passwd1", EmailVerified: true, Roles: model.SYSTEM_ADMIN_ROLE_ID}) + + team, _ := th.App.CreateTeam(&model.Team{DisplayName: "Name", Name: "z-z-" + model.NewId() + "a", Email: user.Email, Type: model.TEAM_OPEN}) + + th.App.JoinUserToTeam(team, user, "") + + channel, _ := th.App.CreateChannel(&model.Channel{DisplayName: "Test API Name", Name: "zz" + model.NewId() + "a", Type: model.CHANNEL_OPEN, TeamId: team.Id, CreatorId: user.Id}, true) + + th.BasicUser = user + th.BasicChannel = channel + th.BasicTeam = team + + return th +} + +func (th *TestHelper) TearDown() { + th.App.Shutdown() if err := recover(); err != nil { StopTestStore() panic(err) @@ -93,63 +117,6 @@ func TestStatic(t *testing.T) { } */ -func TestIncomingWebhook(t *testing.T) { - a := Setup() - defer TearDown(a) - - user := &model.User{Email: model.NewId() + "success+test@simulator.amazonses.com", Nickname: "Corey Hulen", Password: "passwd1"} - user = ApiClient.Must(ApiClient.CreateUser(user, "")).Data.(*model.User) - store.Must(a.Srv.Store.User().VerifyEmail(user.Id)) - - ApiClient.Login(user.Email, "passwd1") - - team := &model.Team{DisplayName: "Name", Name: "z-z-" + model.NewId() + "a", Email: "test@nowhere.com", Type: model.TEAM_OPEN} - team = ApiClient.Must(ApiClient.CreateTeam(team)).Data.(*model.Team) - - a.JoinUserToTeam(team, user, "") - - a.UpdateUserRoles(user.Id, model.SYSTEM_ADMIN_ROLE_ID, false) - ApiClient.SetTeamId(team.Id) - - channel1 := &model.Channel{DisplayName: "Test API Name", Name: "zz" + model.NewId() + "a", Type: model.CHANNEL_OPEN, TeamId: team.Id} - channel1 = ApiClient.Must(ApiClient.CreateChannel(channel1)).Data.(*model.Channel) - - if a.Config().ServiceSettings.EnableIncomingWebhooks { - hook1 := &model.IncomingWebhook{ChannelId: channel1.Id} - hook1 = ApiClient.Must(ApiClient.CreateIncomingWebhook(hook1)).Data.(*model.IncomingWebhook) - - payload := "payload={\"text\": \"test text\"}" - if _, err := ApiClient.PostToWebhook(hook1.Id, payload); err != nil { - t.Fatal(err) - } - - payload = "payload={\"text\": \"\"}" - if _, err := ApiClient.PostToWebhook(hook1.Id, payload); err == nil { - t.Fatal("should have errored - no text to post") - } - - payload = "payload={\"text\": \"test text\", \"channel\": \"junk\"}" - if _, err := ApiClient.PostToWebhook(hook1.Id, payload); err == nil { - t.Fatal("should have errored - bad channel") - } - - payload = "payload={\"text\": \"test text\"}" - if _, err := ApiClient.PostToWebhook("abc123", payload); err == nil { - t.Fatal("should have errored - bad hook") - } - - payloadMultiPart := "------WebKitFormBoundary7MA4YWxkTrZu0gW\r\nContent-Disposition: form-data; name=\"username\"\r\n\r\nwebhook-bot\r\n------WebKitFormBoundary7MA4YWxkTrZu0gW\r\nContent-Disposition: form-data; name=\"text\"\r\n\r\nthis is a test :tada:\r\n------WebKitFormBoundary7MA4YWxkTrZu0gW--" - if _, err := ApiClient.DoPost("/hooks/"+hook1.Id, payloadMultiPart, "multipart/form-data; boundary=----WebKitFormBoundary7MA4YWxkTrZu0gW"); err != nil { - t.Fatal("should have errored - bad hook") - } - - } else { - if _, err := ApiClient.PostToWebhook("123", "123"); err == nil { - t.Fatal("should have failed - webhooks turned off") - } - } -} - func TestMain(m *testing.M) { // Setup a global logger to catch tests logging outside of app context // The global logger will be stomped by apps initalizing but that's fine for testing. Ideally this won't happen. diff --git a/web/webhook.go b/web/webhook.go new file mode 100644 index 0000000000..55cdeb9b5e --- /dev/null +++ b/web/webhook.go @@ -0,0 +1,101 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package web + +import ( + "fmt" + "io" + "net/http" + "strings" + + "github.com/gorilla/mux" + "github.com/gorilla/schema" + + "github.com/mattermost/mattermost-server/mlog" + "github.com/mattermost/mattermost-server/model" +) + +func (w *Web) InitWebhooks() { + w.MainRouter.Handle("/hooks/commands/{id:[A-Za-z0-9]+}", w.NewHandler(commandWebhook)).Methods("POST") + w.MainRouter.Handle("/hooks/{id:[A-Za-z0-9]+}", w.NewHandler(incomingWebhook)).Methods("POST") +} + +func incomingWebhook(c *Context, w http.ResponseWriter, r *http.Request) { + params := mux.Vars(r) + id := params["id"] + + r.ParseForm() + + var err *model.AppError + incomingWebhookPayload := &model.IncomingWebhookRequest{} + contentType := r.Header.Get("Content-Type") + if strings.Split(contentType, "; ")[0] == "application/x-www-form-urlencoded" { + payload := strings.NewReader(r.FormValue("payload")) + + incomingWebhookPayload, err = decodePayload(payload) + if err != nil { + c.Err = err + return + } + } else if strings.HasPrefix(contentType, "multipart/form-data") { + r.ParseMultipartForm(0) + + decoder := schema.NewDecoder() + err := decoder.Decode(incomingWebhookPayload, r.PostForm) + + if err != nil { + c.Err = model.NewAppError("incomingWebhook", "api.webhook.incoming.error", nil, err.Error(), http.StatusBadRequest) + return + } + } else { + incomingWebhookPayload, err = decodePayload(r.Body) + if err != nil { + c.Err = err + return + } + } + + if c.App.Config().LogSettings.EnableWebhookDebugging { + mlog.Debug(fmt.Sprint("Incoming webhook received. Content=", incomingWebhookPayload.ToJson())) + } + + err = c.App.HandleIncomingWebhook(id, incomingWebhookPayload) + if err != nil { + c.Err = err + return + } + + w.Header().Set("Content-Type", "text/plain") + w.Write([]byte("ok")) +} + +func commandWebhook(c *Context, w http.ResponseWriter, r *http.Request) { + params := mux.Vars(r) + id := params["id"] + + response, err := model.CommandResponseFromHTTPBody(r.Header.Get("Content-Type"), r.Body) + if err != nil { + c.Err = model.NewAppError("commandWebhook", "web.command_webhook.parse.app_error", nil, err.Error(), http.StatusBadRequest) + return + } + + appErr := c.App.HandleCommandWebhook(id, response) + if appErr != nil { + c.Err = appErr + return + } + + w.Header().Set("Content-Type", "text/plain") + w.Write([]byte("ok")) +} + +func decodePayload(payload io.Reader) (*model.IncomingWebhookRequest, *model.AppError) { + incomingWebhookPayload, decodeError := model.IncomingWebhookRequestFromJson(payload) + + if decodeError != nil { + return nil, decodeError + } + + return incomingWebhookPayload, nil +} diff --git a/web/webhook_test.go b/web/webhook_test.go new file mode 100644 index 0000000000..e625e55bbb --- /dev/null +++ b/web/webhook_test.go @@ -0,0 +1,216 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package web + +import ( + "bytes" + "fmt" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/mattermost/mattermost-server/model" +) + +func TestIncomingWebhook(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + if !th.App.Config().ServiceSettings.EnableIncomingWebhooks { + _, err := ApiClient.PostToWebhook("123", "123") + assert.NotNil(t, err, "should have errored - webhooks turned off") + return + } + + hook, err := th.App.CreateIncomingWebhookForChannel(th.BasicUser.Id, th.BasicChannel, &model.IncomingWebhook{ChannelId: th.BasicChannel.Id}) + require.Nil(t, err) + + url := "/hooks/" + hook.Id + + tooLongText := "" + for i := 0; i < 8200; i++ { + tooLongText += "a" + } + + t.Run("WebhookBasics", func(t *testing.T) { + payload := "payload={\"text\": \"test text\"}" + _, err := ApiClient.DoPost(url, payload, "application/x-www-form-urlencoded") + assert.Nil(t, err) + + payload = "payload={\"text\": \"\"}" + _, err = ApiClient.DoPost(url, payload, "application/x-www-form-urlencoded") + assert.NotNil(t, err, "should have errored - no text to post") + + payload = "payload={\"text\": \"test text\", \"channel\": \"junk\"}" + _, err = ApiClient.DoPost(url, payload, "application/x-www-form-urlencoded") + assert.NotNil(t, err, "should have errored - bad channel") + + payload = "payload={\"text\": \"test text\"}" + _, err = ApiClient.DoPost("/hooks/abc123", payload, "application/x-www-form-urlencoded") + assert.NotNil(t, err, "should have errored - bad hook") + + _, err = ApiClient.DoPost(url, "{\"text\":\"this is a test\"}", "application/json") + assert.Nil(t, err) + + text := `this is a \"test\" + that contains a newline and a tab` + _, err = ApiClient.DoPost(url, "{\"text\":\""+text+"\"}", "application/json") + assert.Nil(t, err) + + _, err = ApiClient.DoPost(url, fmt.Sprintf("{\"text\":\"this is a test\", \"channel\":\"%s\"}", th.BasicChannel.Name), "application/json") + assert.Nil(t, err) + + _, err = ApiClient.DoPost(url, fmt.Sprintf("{\"text\":\"this is a test\", \"channel\":\"#%s\"}", th.BasicChannel.Name), "application/json") + assert.Nil(t, err) + + _, err = ApiClient.DoPost(url, fmt.Sprintf("{\"text\":\"this is a test\", \"channel\":\"@%s\"}", th.BasicUser.Username), "application/json") + assert.Nil(t, err) + + _, err = ApiClient.DoPost(url, "payload={\"text\":\"this is a test\"}", "application/x-www-form-urlencoded") + assert.Nil(t, err) + + _, err = ApiClient.DoPost(url, "payload={\"text\":\""+text+"\"}", "application/x-www-form-urlencoded") + assert.Nil(t, err) + + _, err = ApiClient.DoPost(url, "{\"text\":\""+tooLongText+"\"}", "application/json") + assert.Nil(t, err) + + payloadMultiPart := "------WebKitFormBoundary7MA4YWxkTrZu0gW\r\nContent-Disposition: form-data; name=\"username\"\r\n\r\nwebhook-bot\r\n------WebKitFormBoundary7MA4YWxkTrZu0gW\r\nContent-Disposition: form-data; name=\"text\"\r\n\r\nthis is a test :tada:\r\n------WebKitFormBoundary7MA4YWxkTrZu0gW--" + _, err = ApiClient.DoPost("/hooks/"+hook.Id, payloadMultiPart, "multipart/form-data; boundary=----WebKitFormBoundary7MA4YWxkTrZu0gW") + assert.Nil(t, err) + }) + + t.Run("WebhookExperimentReadOnly", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.ExperimentalTownSquareIsReadOnly = false }) + _, err := ApiClient.DoPost(url, fmt.Sprintf("{\"text\":\"this is a test\", \"channel\":\"%s\"}", model.DEFAULT_CHANNEL), "application/json") + assert.Nil(t, err, "Not read only") + + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.ExperimentalTownSquareIsReadOnly = true }) + th.App.SetLicense(model.NewTestLicense()) + }) + + t.Run("WebhookAttachments", func(t *testing.T) { + attachmentPayload := `{ + "text": "this is a test", + "attachments": [ + { + "fallback": "Required plain-text summary of the attachment.", + + "color": "#36a64f", + + "pretext": "Optional text that appears above the attachment block", + + "author_name": "Bobby Tables", + "author_link": "http://flickr.com/bobby/", + "author_icon": "http://flickr.com/icons/bobby.jpg", + + "title": "Slack API Documentation", + "title_link": "https://api.slack.com/", + + "text": "Optional text that appears within the attachment", + + "fields": [ + { + "title": "Priority", + "value": "High", + "short": false + } + ], + + "image_url": "http://my-website.com/path/to/image.jpg", + "thumb_url": "http://example.com/path/to/thumb.png" + } + ] + }` + + _, err := ApiClient.DoPost(url, attachmentPayload, "application/json") + assert.Nil(t, err) + + attachmentPayload = `{ + "text": "this is a test", + "attachments": [ + { + "fallback": "Required plain-text summary of the attachment.", + + "color": "#36a64f", + + "pretext": "Optional text that appears above the attachment block", + + "author_name": "Bobby Tables", + "author_link": "http://flickr.com/bobby/", + "author_icon": "http://flickr.com/icons/bobby.jpg", + + "title": "Slack API Documentation", + "title_link": "https://api.slack.com/", + + "text": "` + tooLongText + `", + + "fields": [ + { + "title": "Priority", + "value": "High", + "short": false + } + ], + + "image_url": "http://my-website.com/path/to/image.jpg", + "thumb_url": "http://example.com/path/to/thumb.png" + } + ] + }` + + _, err = ApiClient.DoPost(url, attachmentPayload, "application/json") + assert.Nil(t, err) + }) + + t.Run("DisableWebhooks", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableIncomingWebhooks = false }) + _, err := ApiClient.DoPost(url, "{\"text\":\"this is a test\"}", "application/json") + assert.NotNil(t, err) + }) +} + +func TestCommandWebhooks(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + cmd, err := th.App.CreateCommand(&model.Command{ + CreatorId: th.BasicUser.Id, + TeamId: th.BasicTeam.Id, + URL: "http://nowhere.com", + Method: model.COMMAND_METHOD_POST, + Trigger: "delayed"}) + require.Nil(t, err) + + args := &model.CommandArgs{ + TeamId: th.BasicTeam.Id, + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + } + + hook, err := th.App.CreateCommandWebhook(cmd.Id, args) + if err != nil { + t.Fatal(err) + } + + if resp, _ := http.Post(ApiClient.Url+"/hooks/commands/123123123123", "application/json", bytes.NewBufferString(`{"text":"this is a test"}`)); resp.StatusCode != http.StatusNotFound { + t.Fatal("expected not-found for non-existent hook") + } + + if resp, err := http.Post(ApiClient.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString(`{"text":"invalid`)); err != nil || resp.StatusCode != http.StatusBadRequest { + t.Fatal(err) + } + + for i := 0; i < 5; i++ { + if resp, err := http.Post(ApiClient.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString(`{"text":"this is a test"}`)); err != nil || resp.StatusCode != http.StatusOK { + t.Fatal(err) + } + } + + if resp, _ := http.Post(ApiClient.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString(`{"text":"this is a test"}`)); resp.StatusCode != http.StatusBadRequest { + t.Fatal("expected error for sixth usage") + } +} From 6a9aa855d1c862e4d39f8c00c6b7425405e7a612 Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Mon, 14 May 2018 11:27:30 -0400 Subject: [PATCH 09/10] Move SAML endpoints out of api package (#8780) --- api/user.go | 131 --------------------------------------- api4/api.go | 6 +- api4/apitestlib.go | 2 + web/saml.go | 149 +++++++++++++++++++++++++++++++++++++++++++++ web/web.go | 7 +++ 5 files changed, 159 insertions(+), 136 deletions(-) create mode 100644 web/saml.go diff --git a/api/user.go b/api/user.go index 5931eac1ef..7592d1119e 100644 --- a/api/user.go +++ b/api/user.go @@ -4,11 +4,9 @@ package api import ( - b64 "encoding/base64" "fmt" "net/http" "strconv" - "strings" "time" "github.com/gorilla/mux" @@ -62,9 +60,6 @@ func (api *API) InitUser() { api.BaseRoutes.NeedUser.Handle("/audits", api.ApiUserRequired(getAudits)).Methods("GET") api.BaseRoutes.NeedUser.Handle("/image", api.ApiUserRequiredTrustRequester(getProfileImage)).Methods("GET") api.BaseRoutes.NeedUser.Handle("/update_roles", api.ApiUserRequired(updateRoles)).Methods("POST") - - api.BaseRoutes.Root.Handle("/login/sso/saml", api.AppHandlerIndependent(loginWithSaml)).Methods("GET") - api.BaseRoutes.Root.Handle("/login/sso/saml", api.AppHandlerIndependent(completeSaml)).Methods("POST") } func createUser(c *Context, w http.ResponseWriter, r *http.Request) { @@ -1080,132 +1075,6 @@ func checkMfa(c *Context, w http.ResponseWriter, r *http.Request) { w.Write([]byte(model.MapToJson(rdata))) } -func loginWithSaml(c *Context, w http.ResponseWriter, r *http.Request) { - samlInterface := c.App.Saml - - if samlInterface == nil { - c.Err = model.NewAppError("loginWithSaml", "api.user.saml.not_available.app_error", nil, "", http.StatusFound) - return - } - - teamId, err := c.App.GetTeamIdFromQuery(r.URL.Query()) - if err != nil { - c.Err = err - return - } - action := r.URL.Query().Get("action") - redirectTo := r.URL.Query().Get("redirect_to") - relayProps := map[string]string{} - relayState := "" - - if len(action) != 0 { - relayProps["team_id"] = teamId - relayProps["action"] = action - if action == model.OAUTH_ACTION_EMAIL_TO_SSO { - relayProps["email"] = r.URL.Query().Get("email") - } - } - - if len(redirectTo) != 0 { - relayProps["redirect_to"] = redirectTo - } - - if len(relayProps) > 0 { - relayState = b64.StdEncoding.EncodeToString([]byte(model.MapToJson(relayProps))) - } - - if data, err := samlInterface.BuildRequest(relayState); err != nil { - c.Err = err - return - } else { - w.Header().Set("Content-Type", "application/x-www-form-urlencoded") - http.Redirect(w, r, data.URL, http.StatusFound) - } -} - -func completeSaml(c *Context, w http.ResponseWriter, r *http.Request) { - samlInterface := c.App.Saml - - if samlInterface == nil { - c.Err = model.NewAppError("completeSaml", "api.user.saml.not_available.app_error", nil, "", http.StatusFound) - return - } - - //Validate that the user is with SAML and all that - encodedXML := r.FormValue("SAMLResponse") - relayState := r.FormValue("RelayState") - - relayProps := make(map[string]string) - if len(relayState) > 0 { - stateStr := "" - if b, err := b64.StdEncoding.DecodeString(relayState); err != nil { - c.Err = model.NewAppError("completeSaml", "api.user.authorize_oauth_user.invalid_state.app_error", nil, err.Error(), http.StatusFound) - return - } else { - stateStr = string(b) - } - relayProps = model.MapFromJson(strings.NewReader(stateStr)) - } - - action := relayProps["action"] - if user, err := samlInterface.DoLogin(encodedXML, relayProps); err != nil { - if action == model.OAUTH_ACTION_MOBILE { - err.Translate(c.T) - w.Write([]byte(err.ToJson())) - } else { - c.Err = err - c.Err.StatusCode = http.StatusFound - } - return - } else { - if err := c.App.CheckUserAllAuthenticationCriteria(user, ""); err != nil { - c.Err = err - c.Err.StatusCode = http.StatusFound - return - } - - switch action { - case model.OAUTH_ACTION_SIGNUP: - teamId := relayProps["team_id"] - if len(teamId) > 0 { - c.App.Go(func() { - if err := c.App.AddUserToTeamByTeamId(teamId, user); err != nil { - mlog.Error(err.Error()) - } else { - c.App.AddDirectChannels(teamId, user) - } - }) - } - case model.OAUTH_ACTION_EMAIL_TO_SSO: - if err := c.App.RevokeAllSessions(user.Id); err != nil { - c.Err = err - return - } - c.LogAuditWithUserId(user.Id, "Revoked all sessions for user") - c.App.Go(func() { - if err := c.App.SendSignInChangeEmail(user.Email, strings.Title(model.USER_AUTH_SERVICE_SAML)+" SSO", user.Locale, c.App.GetSiteURL()); err != nil { - mlog.Error(err.Error()) - } - }) - } - doLogin(c, w, r, user, "") - if c.Err != nil { - return - } - - if val, ok := relayProps["redirect_to"]; ok { - http.Redirect(w, r, c.GetSiteURLHeader()+val, http.StatusFound) - return - } - - if action == model.OAUTH_ACTION_MOBILE { - ReturnStatusOK(w) - } else { - http.Redirect(w, r, app.GetProtocol(r)+"://"+r.Host, http.StatusFound) - } - } -} - func sanitizeProfile(c *Context, user *model.User) *model.User { options := c.App.Config().GetSanitizeOptions() diff --git a/api4/api.go b/api4/api.go index acce923c05..9172391dd7 100644 --- a/api4/api.go +++ b/api4/api.go @@ -243,8 +243,4 @@ func (api *API) Handle404(w http.ResponseWriter, r *http.Request) { web.Handle404(api.App, w, r) } -func ReturnStatusOK(w http.ResponseWriter) { - m := make(map[string]string) - m[model.STATUS] = model.STATUS_OK - w.Write([]byte(model.MapToJson(m))) -} +var ReturnStatusOK = web.ReturnStatusOK diff --git a/api4/apitestlib.go b/api4/apitestlib.go index 48765687ac..0ce3341548 100644 --- a/api4/apitestlib.go +++ b/api4/apitestlib.go @@ -26,6 +26,7 @@ import ( "github.com/mattermost/mattermost-server/store/sqlstore" "github.com/mattermost/mattermost-server/store/storetest" "github.com/mattermost/mattermost-server/utils" + "github.com/mattermost/mattermost-server/web" "github.com/mattermost/mattermost-server/wsapi" s3 "github.com/minio/minio-go" @@ -120,6 +121,7 @@ func setupTestHelper(enterprise bool) *TestHelper { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ListenAddress = prevListenAddress }) Init(th.App, th.App.Srv.Router, true) + web.NewWeb(th.App, th.App.Srv.Router) wsapi.Init(th.App, th.App.Srv.WebSocketRouter) th.App.Srv.Store.MarkSystemRanUnitTests() th.App.DoAdvancedPermissionsMigration() diff --git a/web/saml.go b/web/saml.go new file mode 100644 index 0000000000..f3e5a12e81 --- /dev/null +++ b/web/saml.go @@ -0,0 +1,149 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package web + +import ( + b64 "encoding/base64" + "net/http" + "strings" + + "github.com/mattermost/mattermost-server/app" + "github.com/mattermost/mattermost-server/mlog" + "github.com/mattermost/mattermost-server/model" +) + +func (w *Web) InitSaml() { + w.MainRouter.Handle("/login/sso/saml", w.NewHandler(loginWithSaml)).Methods("GET") + w.MainRouter.Handle("/login/sso/saml", w.NewHandler(completeSaml)).Methods("POST") +} + +func loginWithSaml(c *Context, w http.ResponseWriter, r *http.Request) { + samlInterface := c.App.Saml + + if samlInterface == nil { + c.Err = model.NewAppError("loginWithSaml", "api.user.saml.not_available.app_error", nil, "", http.StatusFound) + return + } + + teamId, err := c.App.GetTeamIdFromQuery(r.URL.Query()) + if err != nil { + c.Err = err + return + } + action := r.URL.Query().Get("action") + redirectTo := r.URL.Query().Get("redirect_to") + relayProps := map[string]string{} + relayState := "" + + if len(action) != 0 { + relayProps["team_id"] = teamId + relayProps["action"] = action + if action == model.OAUTH_ACTION_EMAIL_TO_SSO { + relayProps["email"] = r.URL.Query().Get("email") + } + } + + if len(redirectTo) != 0 { + relayProps["redirect_to"] = redirectTo + } + + if len(relayProps) > 0 { + relayState = b64.StdEncoding.EncodeToString([]byte(model.MapToJson(relayProps))) + } + + if data, err := samlInterface.BuildRequest(relayState); err != nil { + c.Err = err + return + } else { + w.Header().Set("Content-Type", "application/x-www-form-urlencoded") + http.Redirect(w, r, data.URL, http.StatusFound) + } +} + +func completeSaml(c *Context, w http.ResponseWriter, r *http.Request) { + samlInterface := c.App.Saml + + if samlInterface == nil { + c.Err = model.NewAppError("completeSaml", "api.user.saml.not_available.app_error", nil, "", http.StatusFound) + return + } + + //Validate that the user is with SAML and all that + encodedXML := r.FormValue("SAMLResponse") + relayState := r.FormValue("RelayState") + + relayProps := make(map[string]string) + if len(relayState) > 0 { + stateStr := "" + if b, err := b64.StdEncoding.DecodeString(relayState); err != nil { + c.Err = model.NewAppError("completeSaml", "api.user.authorize_oauth_user.invalid_state.app_error", nil, err.Error(), http.StatusFound) + return + } else { + stateStr = string(b) + } + relayProps = model.MapFromJson(strings.NewReader(stateStr)) + } + + action := relayProps["action"] + if user, err := samlInterface.DoLogin(encodedXML, relayProps); err != nil { + if action == model.OAUTH_ACTION_MOBILE { + err.Translate(c.T) + w.Write([]byte(err.ToJson())) + } else { + c.Err = err + c.Err.StatusCode = http.StatusFound + } + return + } else { + if err := c.App.CheckUserAllAuthenticationCriteria(user, ""); err != nil { + c.Err = err + c.Err.StatusCode = http.StatusFound + return + } + + switch action { + case model.OAUTH_ACTION_SIGNUP: + teamId := relayProps["team_id"] + if len(teamId) > 0 { + c.App.Go(func() { + if err := c.App.AddUserToTeamByTeamId(teamId, user); err != nil { + mlog.Error(err.Error()) + } else { + c.App.AddDirectChannels(teamId, user) + } + }) + } + case model.OAUTH_ACTION_EMAIL_TO_SSO: + if err := c.App.RevokeAllSessions(user.Id); err != nil { + c.Err = err + return + } + c.LogAuditWithUserId(user.Id, "Revoked all sessions for user") + c.App.Go(func() { + if err := c.App.SendSignInChangeEmail(user.Email, strings.Title(model.USER_AUTH_SERVICE_SAML)+" SSO", user.Locale, c.App.GetSiteURL()); err != nil { + mlog.Error(err.Error()) + } + }) + } + + session, err := c.App.DoLogin(w, r, user, "") + if err != nil { + c.Err = err + return + } + + c.Session = *session + + if val, ok := relayProps["redirect_to"]; ok { + http.Redirect(w, r, c.GetSiteURLHeader()+val, http.StatusFound) + return + } + + if action == model.OAUTH_ACTION_MOBILE { + ReturnStatusOK(w) + } else { + http.Redirect(w, r, app.GetProtocol(r)+"://"+r.Host, http.StatusFound) + } + } +} diff --git a/web/web.go b/web/web.go index 94363cfdee..53276953ed 100644 --- a/web/web.go +++ b/web/web.go @@ -32,6 +32,7 @@ func NewWeb(a *app.App, root *mux.Router) *Web { web.InitStatic() web.InitWebhooks() + web.InitSaml() return web } @@ -71,3 +72,9 @@ func Handle404(a *app.App, w http.ResponseWriter, r *http.Request) { func IsApiCall(r *http.Request) bool { return strings.Index(r.URL.Path, "/api/") == 0 } + +func ReturnStatusOK(w http.ResponseWriter) { + m := make(map[string]string) + m[model.STATUS] = model.STATUS_OK + w.Write([]byte(model.MapToJson(m))) +} From a1656dffa98fbc8865e476b214e4e0c562547d39 Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Mon, 14 May 2018 13:24:22 -0400 Subject: [PATCH 10/10] MM-10201: querystring for get slash commands (#8779) * pass GET slash command payloads through query string Previously, both GET and POST requests received the payload via the body, but this was incorrect for GET requests. Now, the payloads for GET requests is sent via the query string. * reorder tests for clarity * switch command tests to use httptest servers * restore original test command endpoints --- api4/command_test.go | 191 ++++++++++++++++++++++++++++++------------- app/command.go | 8 +- 2 files changed, 137 insertions(+), 62 deletions(-) diff --git a/api4/command_test.go b/api4/command_test.go index 8c4ce5d504..0d37d74405 100644 --- a/api4/command_test.go +++ b/api4/command_test.go @@ -5,9 +5,13 @@ package api4 import ( "fmt" - "strings" + "net/http" + "net/http/httptest" + "net/url" "testing" + "github.com/stretchr/testify/require" + "github.com/mattermost/mattermost-server/model" ) @@ -392,7 +396,7 @@ func TestRegenToken(t *testing.T) { } } -func TestExecuteCommand(t *testing.T) { +func TestExecuteInvalidCommand(t *testing.T) { th := Setup().InitBasic().InitSystemAdmin() defer th.TearDown() Client := th.Client @@ -407,56 +411,19 @@ func TestExecuteCommand(t *testing.T) { }) }() th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCommands = true }) - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost" }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "127.0.0.0/8" }) - postCmd := &model.Command{ - CreatorId: th.BasicUser.Id, - TeamId: th.BasicTeam.Id, - URL: fmt.Sprintf("http://localhost:%v", th.App.Srv.ListenAddr.Port) + model.API_URL_SUFFIX_V4 + "/teams/command_test", - Method: model.COMMAND_METHOD_POST, - Trigger: "postcommand", - } + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + rc := &model.CommandResponse{} - if _, err := th.App.CreateCommand(postCmd); err != nil { - t.Fatal("failed to create post command") - } - - commandResponse, resp := Client.ExecuteCommand(channel.Id, "/postcommand") - CheckNoError(t, resp) - - if commandResponse == nil { - t.Fatal("command response should have returned") - } - - posts, err := th.App.GetPostsPage(channel.Id, 0, 10) - if err != nil || posts == nil || len(posts.Order) != 3 { - t.Fatal("Test command failed to send") - } - - cmdPosted := false - for _, post := range posts.Posts { - if strings.Contains(post.Message, "test command response") { - if post.Type != "custom_test" { - t.Fatal("wrong type set in slash command post") - } - - if post.Props["someprop"] != "somevalue" { - t.Fatal("wrong prop set in slash command post") - } - - cmdPosted = true - break - } - } - - if !cmdPosted { - t.Fatal("Test command response failed to post") - } + w.Write([]byte(rc.ToJson())) + })) + defer ts.Close() getCmd := &model.Command{ CreatorId: th.BasicUser.Id, TeamId: th.BasicTeam.Id, - URL: fmt.Sprintf("http://localhost:%v", th.App.Srv.ListenAddr.Port) + model.API_URL_SUFFIX_V4 + "/teams/command_test", + URL: fmt.Sprintf("%s/%s/teams/command_test", ts.URL, model.API_URL_SUFFIX_V4), Method: model.COMMAND_METHOD_GET, Trigger: "getcommand", } @@ -465,19 +432,7 @@ func TestExecuteCommand(t *testing.T) { t.Fatal("failed to create get command") } - commandResponse, resp = Client.ExecuteCommand(channel.Id, "/getcommand") - CheckNoError(t, resp) - - if commandResponse == nil { - t.Fatal("command response should have returned") - } - - posts, err = th.App.GetPostsPage(channel.Id, 0, 10) - if err != nil || posts == nil || len(posts.Order) != 4 { - t.Fatal("Test command failed to send") - } - - _, resp = Client.ExecuteCommand(channel.Id, "") + _, resp := Client.ExecuteCommand(channel.Id, "") CheckBadRequestStatus(t, resp) _, resp = Client.ExecuteCommand(channel.Id, "/") @@ -504,6 +459,124 @@ func TestExecuteCommand(t *testing.T) { CheckNoError(t, resp) } +func TestExecuteGetCommand(t *testing.T) { + th := Setup().InitBasic().InitSystemAdmin() + defer th.TearDown() + Client := th.Client + channel := th.BasicChannel + + enableCommands := *th.App.Config().ServiceSettings.EnableCommands + allowedInternalConnections := *th.App.Config().ServiceSettings.AllowedUntrustedInternalConnections + defer func() { + th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableCommands = &enableCommands }) + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.ServiceSettings.AllowedUntrustedInternalConnections = &allowedInternalConnections + }) + }() + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCommands = true }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "127.0.0.0/8" }) + + token := model.NewId() + expectedCommandResponse := &model.CommandResponse{ + Text: "test get command response", + ResponseType: model.COMMAND_RESPONSE_TYPE_IN_CHANNEL, + Type: "custom_test", + Props: map[string]interface{}{"someprop": "somevalue"}, + } + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodGet, r.Method) + + values, err := url.ParseQuery(r.URL.RawQuery) + require.NoError(t, err) + + require.Equal(t, token, values.Get("token")) + require.Equal(t, th.BasicTeam.Name, values.Get("team_domain")) + + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(expectedCommandResponse.ToJson())) + })) + defer ts.Close() + + getCmd := &model.Command{ + CreatorId: th.BasicUser.Id, + TeamId: th.BasicTeam.Id, + URL: fmt.Sprintf("%s/%s/teams/command_test", ts.URL, model.API_URL_SUFFIX_V4), + Method: model.COMMAND_METHOD_GET, + Trigger: "getcommand", + Token: token, + } + + if _, err := th.App.CreateCommand(getCmd); err != nil { + t.Fatal("failed to create get command") + } + + commandResponse, resp := Client.ExecuteCommand(channel.Id, "/getcommand") + CheckNoError(t, resp) + + expectedCommandResponse.Props["from_webhook"] = "true" + require.Equal(t, expectedCommandResponse, commandResponse) +} + +func TestExecutePostCommand(t *testing.T) { + th := Setup().InitBasic().InitSystemAdmin() + defer th.TearDown() + Client := th.Client + channel := th.BasicChannel + + enableCommands := *th.App.Config().ServiceSettings.EnableCommands + allowedInternalConnections := *th.App.Config().ServiceSettings.AllowedUntrustedInternalConnections + defer func() { + th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableCommands = &enableCommands }) + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.ServiceSettings.AllowedUntrustedInternalConnections = &allowedInternalConnections + }) + }() + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCommands = true }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "127.0.0.0/8" }) + + token := model.NewId() + expectedCommandResponse := &model.CommandResponse{ + Text: "test post command response", + ResponseType: model.COMMAND_RESPONSE_TYPE_IN_CHANNEL, + Type: "custom_test", + Props: map[string]interface{}{"someprop": "somevalue"}, + } + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodPost, r.Method) + + r.ParseForm() + + require.Equal(t, token, r.FormValue("token")) + require.Equal(t, th.BasicTeam.Name, r.FormValue("team_domain")) + + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(expectedCommandResponse.ToJson())) + })) + defer ts.Close() + + getCmd := &model.Command{ + CreatorId: th.BasicUser.Id, + TeamId: th.BasicTeam.Id, + URL: fmt.Sprintf("%s/%s/teams/command_test", ts.URL, model.API_URL_SUFFIX_V4), + Method: model.COMMAND_METHOD_POST, + Trigger: "postcommand", + Token: token, + } + + if _, err := th.App.CreateCommand(getCmd); err != nil { + t.Fatal("failed to create get command") + } + + commandResponse, resp := Client.ExecuteCommand(channel.Id, "/postcommand") + CheckNoError(t, resp) + + expectedCommandResponse.Props["from_webhook"] = "true" + require.Equal(t, expectedCommandResponse, commandResponse) + +} + func TestExecuteCommandAgainstChannelOnAnotherTeam(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() diff --git a/app/command.go b/app/command.go index 796d656a7f..92c35865ab 100644 --- a/app/command.go +++ b/app/command.go @@ -230,12 +230,14 @@ func (a *App) ExecuteCommand(args *model.CommandArgs) (*model.CommandResponse, * p.Set("response_url", args.SiteURL+"/hooks/commands/"+hook.Id) } - method := "POST" + var req *http.Request if cmd.Method == model.COMMAND_METHOD_GET { - method = "GET" + req, _ = http.NewRequest(http.MethodGet, cmd.URL, nil) + req.URL.RawQuery = p.Encode() + } else { + req, _ = http.NewRequest(http.MethodPost, cmd.URL, strings.NewReader(p.Encode())) } - req, _ := http.NewRequest(method, cmd.URL, strings.NewReader(p.Encode())) req.Header.Set("Accept", "application/json") req.Header.Set("Authorization", "Token "+cmd.Token) if cmd.Method == model.COMMAND_METHOD_POST {