Fix API Get channels for a user returns users' dm channels with blank teamid (#4748)

* fix API Get channels for a user returns users' dm channels with blank team ID

add check in the context.go

add suggestion

made adjustment per review and support from @joram

* update tests

* add check if needd user or admin permissions

* update per review

4 files changed, 41 insertions, 11 deletions

diff --git a/api/channel.go b/api/channel.go
index dcc4ed563..941692ac3 100644
--- a/api/channel.go
+++ b/api/channel.go
@@ -431,10 +431,13 @@ func updateChannelPurpose(c *Context, w http.ResponseWriter, r *http.Request) {
 }
 
 func getChannels(c *Context, w http.ResponseWriter, r *http.Request) {
-
+	if c.TeamId == "" {
+		c.Err = model.NewLocAppError("", "api.context.missing_teamid.app_error", nil, "TeamIdRequired")
+		c.Err.StatusCode = http.StatusBadRequest
+		return
+	}
 	// user is already in the team
 	// Get's all channels the user is a member of
-
 	if result := <-Srv.Store.Channel().GetChannels(c.TeamId, c.Session.UserId); result.Err != nil {
 		if result.Err.Id == "store.sql_channel.get_channels.not_found.app_error" {
 			// lets make sure the user is valid
diff --git a/api/channel_test.go b/api/channel_test.go
index 683deb8a9..c916a27cf 100644
--- a/api/channel_test.go
+++ b/api/channel_test.go
@@ -745,10 +745,21 @@ func TestGetChannel(t *testing.T) {
 		t.Fatal("should have failed - bad channel id")
 	}
 
-	Client.SetTeamId(team2.Id)
+	th.BasicClient.SetTeamId(team2.Id)
 	if _, err := Client.GetChannel(channel2.Id, ""); err == nil {
 		t.Fatal("should have failed - wrong team")
 	}
+
+	//Test if a wrong team id is supplied should return error
+	if _, err := Client.CreateDirectChannel(th.BasicUser2.Id); err != nil {
+		t.Fatal(err)
+	}
+
+	th.BasicClient.SetTeamId("nonexitingteamid")
+	if _, err := Client.GetChannels(""); err == nil {
+		t.Fatal("should have failed - wrong team id")
+	}
+
 }
 
 func TestGetMoreChannelsPage(t *testing.T) {
diff --git a/api/context.go b/api/context.go
index 4042a7b0f..765bb502a 100644
--- a/api/context.go
+++ b/api/context.go
@@ -221,6 +221,11 @@ func (h handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		SetStatusOnline(c.Session.UserId, c.Session.Id, false)
 	}
 
+	if c.Err == nil && (h.requireUser || h.requireSystemAdmin) {
+		//check if teamId exist
+		c.CheckTeamId()
+	}
+
 	if c.Err == nil {
 		h.handleFunc(c, w, r)
 	}
@@ -575,3 +580,18 @@ func InvalidateAllCaches() {
 	store.ClearUserCaches()
 	store.ClearPostCaches()
 }
+
+func (c *Context) CheckTeamId() {
+	if c.TeamId != "" && c.Session.GetTeamByTeamId(c.TeamId) == nil {
+		if HasPermissionToContext(c, model.PERMISSION_MANAGE_SYSTEM) {
+			if result := <-Srv.Store.Team().Get(c.TeamId); result.Err != nil {
+				c.Err = result.Err
+				c.Err.StatusCode = http.StatusBadRequest
+				return
+			}
+		} else {
+			// just return because it fail on the HasPermissionToContext and the error is already on the Context c.Err
+			return
+		}
+	}
+}
diff --git a/api/team_test.go b/api/team_test.go
index c4bcb1868..52b23e1ba 100644
--- a/api/team_test.go
+++ b/api/team_test.go
@@ -766,15 +766,11 @@ func TestGetTeamStats(t *testing.T) {
 		}
 	}
 
-	if result, err := th.SystemAdminClient.GetTeamStats("junk"); err != nil {
-		t.Fatal(err)
+	if _, err := th.SystemAdminClient.GetTeamStats("junk"); err == nil {
+		t.Fatal("should fail invalid teamid")
 	} else {
-		if result.Data.(*model.TeamStats).TotalMemberCount != 0 {
-			t.Fatal("wrong count")
-		}
-
-		if result.Data.(*model.TeamStats).ActiveMemberCount != 0 {
-			t.Fatal("wrong count")
+		if err.Id != "store.sql_team.get.find.app_error" {
+			t.Fatal("wrong error. Got: " + err.Id)
 		}
 	}