From 1c498996a4a32ec44bb3c4a35a4a5d3711887013 Mon Sep 17 00:00:00 2001
From: Agniva De Sarker <agnivade@yahoo.co.in>
Date: Mon, 16 Mar 2020 22:58:59 +0530
Subject: [PATCH] MM-23221: Fix getAllTeams handler returning null (#14045)

Automatic Merge
---
 api4/team.go      | 12 ++++++++----
 api4/team_test.go | 44 ++++++++++++++++++++++++++++++++------------
 i18n/en.json      |  4 ++++
 3 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/api4/team.go b/api4/team.go
index fab1b234cc..0fef9f0978 100644
--- a/api4/team.go
+++ b/api4/team.go
@@ -853,26 +853,30 @@ func getAllTeams(c *Context, w http.ResponseWriter, r *http.Request) {
 	var err *model.AppError
 	var teamsWithCount *model.TeamsWithCount
 
-	if c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_LIST_PRIVATE_TEAMS) && c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_LIST_PUBLIC_TEAMS) {
+	listPrivate := c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_LIST_PRIVATE_TEAMS)
+	listPublic := c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_LIST_PUBLIC_TEAMS)
+	if listPrivate && listPublic {
 		if c.Params.IncludeTotalCount {
 			teamsWithCount, err = c.App.GetAllTeamsPageWithCount(c.Params.Page*c.Params.PerPage, c.Params.PerPage)
 		} else {
 			teams, err = c.App.GetAllTeamsPage(c.Params.Page*c.Params.PerPage, c.Params.PerPage)
 		}
-	} else if c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_LIST_PRIVATE_TEAMS) {
+	} else if listPrivate {
 		if c.Params.IncludeTotalCount {
 			teamsWithCount, err = c.App.GetAllPrivateTeamsPageWithCount(c.Params.Page*c.Params.PerPage, c.Params.PerPage)
 		} else {
 			teams, err = c.App.GetAllPrivateTeamsPage(c.Params.Page*c.Params.PerPage, c.Params.PerPage)
 		}
-	} else if c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_LIST_PUBLIC_TEAMS) {
+	} else if listPublic {
 		if c.Params.IncludeTotalCount {
 			teamsWithCount, err = c.App.GetAllPublicTeamsPageWithCount(c.Params.Page*c.Params.PerPage, c.Params.PerPage)
 		} else {
 			teams, err = c.App.GetAllPublicTeamsPage(c.Params.Page*c.Params.PerPage, c.Params.PerPage)
 		}
+	} else {
+		// The user doesn't have permissions to list private as well as public teams.
+		err = model.NewAppError("getAllTeams", "api.team.get_all_teams.insufficient_permissions", nil, "", http.StatusForbidden)
 	}
-
 	if err != nil {
 		c.Err = err
 		return
diff --git a/api4/team_test.go b/api4/team_test.go
index a40e49e1e4..cb83bc41ac 100644
--- a/api4/team_test.go
+++ b/api4/team_test.go
@@ -572,13 +572,16 @@ func TestGetAllTeams(t *testing.T) {
 	CheckNoError(t, resp)
 
 	testCases := []struct {
-		Name          string
-		Page          int
-		PerPage       int
-		Permissions   []string
-		ExpectedTeams []string
-		WithCount     bool
-		ExpectedCount int64
+		Name               string
+		Page               int
+		PerPage            int
+		Permissions        []string
+		ExpectedTeams      []string
+		WithCount          bool
+		ExpectedCount      int64
+		ExpectedError      bool
+		ErrorId            string
+		ExpectedStatusCode int
 	}{
 		{
 			Name:          "Get 1 team per page",
@@ -623,11 +626,23 @@ func TestGetAllTeams(t *testing.T) {
 			ExpectedTeams: []string{th.BasicTeam.Id, team1.Id, team2.Id, team3.Id, team4.Id},
 		},
 		{
-			Name:          "Get no teams because permissions",
-			Page:          0,
-			PerPage:       10,
-			Permissions:   []string{},
-			ExpectedTeams: []string{},
+			Name:               "Get no teams because permissions",
+			Page:               0,
+			PerPage:            10,
+			Permissions:        []string{},
+			ExpectedError:      true,
+			ExpectedStatusCode: http.StatusForbidden,
+			ErrorId:            "api.team.get_all_teams.insufficient_permissions",
+		},
+		{
+			Name:               "Get no teams because permissions with count",
+			Page:               0,
+			PerPage:            10,
+			Permissions:        []string{},
+			WithCount:          true,
+			ExpectedError:      true,
+			ExpectedStatusCode: http.StatusForbidden,
+			ErrorId:            "api.team.get_all_teams.insufficient_permissions",
 		},
 		{
 			Name:          "Get all teams with count",
@@ -679,6 +694,11 @@ func TestGetAllTeams(t *testing.T) {
 			} else {
 				teams, resp = Client.GetAllTeams("", tc.Page, tc.PerPage)
 			}
+			if tc.ExpectedError {
+				CheckErrorMessage(t, resp, tc.ErrorId)
+				checkHTTPStatus(t, resp, tc.ExpectedStatusCode, true)
+				return
+			}
 			CheckNoError(t, resp)
 			require.Equal(t, len(tc.ExpectedTeams), len(teams))
 			for idx, team := range teams {
diff --git a/i18n/en.json b/i18n/en.json
index 2f1bf7f2ab..1ddfa5079b 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -1966,6 +1966,10 @@
     "id": "api.team.demote_user_to_guest.license.error",
     "translation": "Your license does not support guest accounts"
   },
+  {
+    "id": "api.team.get_all_teams.insufficient_permissions",
+    "translation": "You don't have the appropriate permissions to list all teams"
+  },
   {
     "id": "api.team.get_invite_info.not_open_team",
     "translation": "Invite is invalid because this is not an open team."