From 1ed1a6be0b727d8273246b040e3d670ef3ec5fd7 Mon Sep 17 00:00:00 2001
From: Hossein Ahmadian-Yazdi <hahmadia@users.noreply.github.com>
Date: Thu, 14 Nov 2019 09:21:23 -0500
Subject: [PATCH] [MM-19740] Return error for non-system-admins/team
 admins/channel admins from the GET /api/v4/groups API endpoint response
 (#12961)

* [MM-19740] Differentiate between admin and non-admin

* [MM-19740] added testing admin and non admin

* [MM-19740] Address PR comments

* [MM-19740] Addressed PR comments

* [MM-19740] Updated en.json

* [MM-19740] Addressed PR comments

* Address PR comments

* Address PR comments
---
 api4/group.go      | 29 +++++++++++++++++++++++------
 api4/group_test.go | 22 +++++++++++++++++-----
 i18n/en.json       |  4 ++++
 3 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/api4/group.go b/api4/group.go
index 4f799c3b58..a0434e0ed8 100644
--- a/api4/group.go
+++ b/api4/group.go
@@ -575,23 +575,40 @@ func getGroups(c *Context, w http.ResponseWriter, r *http.Request) {
 		c.Err = model.NewAppError("Api4.getGroups", "api.ldap_groups.license_error", nil, "", http.StatusNotImplemented)
 		return
 	}
+	var teamID, channelID string
+
+	if id := c.Params.NotAssociatedToTeam; model.IsValidId(id) {
+		teamID = id
+	}
+
+	if id := c.Params.NotAssociatedToChannel; model.IsValidId(id) {
+		channelID = id
+	}
+
+	if teamID == "" && channelID == "" {
+		c.Err = model.NewAppError("Api4.getGroups", "api.getGroups.invalid_or_missing_channel_or_team_id", nil, "", http.StatusBadRequest)
+		return
+	}
 
 	opts := model.GroupSearchOpts{
 		Q:                  c.Params.Q,
 		IncludeMemberCount: c.Params.IncludeMemberCount,
 	}
 
-	teamID := c.Params.NotAssociatedToTeam
-	if len(teamID) == 26 {
-		if !c.App.SessionHasPermissionToTeam(c.App.Session, teamID, model.PERMISSION_VIEW_TEAM) {
-			c.SetPermissionError(model.PERMISSION_VIEW_TEAM)
+	if teamID != "" {
+		_, err := c.App.GetTeam(teamID)
+		if err != nil {
+			c.Err = err
+			return
+		}
+		if !c.App.SessionHasPermissionToTeam(c.App.Session, teamID, model.PERMISSION_MANAGE_TEAM) {
+			c.SetPermissionError(model.PERMISSION_MANAGE_TEAM)
 			return
 		}
 		opts.NotAssociatedToTeam = teamID
 	}
 
-	channelID := c.Params.NotAssociatedToChannel
-	if len(channelID) == 26 {
+	if channelID != "" {
 		channel, err := c.App.GetChannel(channelID)
 		if err != nil {
 			c.Err = err
diff --git a/api4/group_test.go b/api4/group_test.go
index a94066efdc..b6cb0361ae 100644
--- a/api4/group_test.go
+++ b/api4/group_test.go
@@ -8,11 +8,9 @@ import (
 	"net/http"
 	"testing"
 
-	"github.com/stretchr/testify/require"
-
-	"github.com/stretchr/testify/assert"
-
 	"github.com/mattermost/mattermost-server/model"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestGetGroup(t *testing.T) {
@@ -765,6 +763,20 @@ func TestGetGroups(t *testing.T) {
 
 	th.App.SetLicense(model.NewTestLicense("ldap"))
 
+	_, response = th.SystemAdminClient.GetGroups(opts)
+	CheckBadRequestStatus(t, response)
+
+	_, response = th.SystemAdminClient.UpdateChannelRoles(th.BasicChannel.Id, th.BasicUser.Id, "")
+	require.Nil(t, response.Error)
+
+	opts.NotAssociatedToChannel = th.BasicChannel.Id
+
+	_, response = th.Client.GetGroups(opts)
+	CheckForbiddenStatus(t, response)
+
+	_, response = th.SystemAdminClient.UpdateChannelRoles(th.BasicChannel.Id, th.BasicUser.Id, "channel_user channel_admin")
+	require.Nil(t, response.Error)
+
 	groups, response := th.SystemAdminClient.GetGroups(opts)
 	assert.Nil(t, response.Error)
 	assert.ElementsMatch(t, []*model.Group{group, th.Group}, groups)
@@ -787,7 +799,7 @@ func TestGetGroups(t *testing.T) {
 	_, response = th.Client.GetGroups(opts)
 	CheckForbiddenStatus(t, response)
 
-	_, response = th.SystemAdminClient.UpdateTeamMemberRoles(th.BasicTeam.Id, th.BasicUser.Id, "team_user")
+	_, response = th.SystemAdminClient.UpdateTeamMemberRoles(th.BasicTeam.Id, th.BasicUser.Id, "team_user team_admin")
 	require.Nil(t, response.Error)
 
 	_, response = th.Client.GetGroups(opts)
diff --git a/i18n/en.json b/i18n/en.json
index d8a2796d00..6631c537a2 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -1340,6 +1340,10 @@
     "id": "api.file.write_file_locally.writing.app_error",
     "translation": "Encountered an error writing to local server storage"
   },
+  {
+    "id": "api.getGroups.invalid_or_missing_channel_or_team_id",
+    "translation": "Invalid/Missing channel ID or Team ID."
+  },
   {
     "id": "api.incoming_webhook.disabled.app_error",
     "translation": "Incoming webhooks have been disabled by the system admin."