Check team member instead of session for team admin role when updating/deleting channels (#3007)

api/channel.go

@@ -188,6 +188,7 @@ func updateChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 
 	sc := Srv.Store.Channel().Get(channel.Id)
 	cmc := Srv.Store.Channel().GetMember(channel.Id, c.Session.UserId)
+	tmc := Srv.Store.Team().GetMember(c.TeamId, c.Session.UserId)
 
 	if cresult := <-sc; cresult.Err != nil {
 		c.Err = cresult.Err
@@ -195,14 +196,19 @@ func updateChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 	} else if cmcresult := <-cmc; cmcresult.Err != nil {
 		c.Err = cmcresult.Err
 		return
+	} else if tmcresult := <-tmc; cmcresult.Err != nil {
+		c.Err = tmcresult.Err
+		return
 	} else {
 		oldChannel := cresult.Data.(*model.Channel)
 		channelMember := cmcresult.Data.(model.ChannelMember)
+		teamMember := tmcresult.Data.(model.TeamMember)
+
 		if !c.HasPermissionsToTeam(oldChannel.TeamId, "updateChannel") {
 			return
 		}
 
-		if !strings.Contains(channelMember.Roles, model.CHANNEL_ROLE_ADMIN) && !strings.Contains(c.Session.Roles, model.ROLE_TEAM_ADMIN) {
+		if !strings.Contains(channelMember.Roles, model.CHANNEL_ROLE_ADMIN) && !strings.Contains(teamMember.Roles, model.ROLE_TEAM_ADMIN) {
 			c.Err = model.NewLocAppError("updateChannel", "api.channel.update_channel.permission.app_error", nil, "")
 			c.Err.StatusCode = http.StatusForbidden
 			return
@@ -636,6 +642,7 @@ func deleteChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 
 	sc := Srv.Store.Channel().Get(id)
 	scm := Srv.Store.Channel().GetMember(id, c.Session.UserId)
+	tmc := Srv.Store.Team().GetMember(c.TeamId, c.Session.UserId)
 	uc := Srv.Store.User().Get(c.Session.UserId)
 	ihc := Srv.Store.Webhook().GetIncomingByChannel(id)
 	ohc := Srv.Store.Webhook().GetOutgoingByChannel(id)
@@ -649,6 +656,9 @@ func deleteChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 	} else if scmresult := <-scm; scmresult.Err != nil {
 		c.Err = scmresult.Err
 		return
+	} else if tmcresult := <-tmc; tmcresult.Err != nil {
+		c.Err = tmcresult.Err
+		return
 	} else if ihcresult := <-ihc; ihcresult.Err != nil {
 		c.Err = ihcresult.Err
 		return
@@ -659,6 +669,7 @@ func deleteChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 		channel := cresult.Data.(*model.Channel)
 		user := uresult.Data.(*model.User)
 		channelMember := scmresult.Data.(model.ChannelMember)
+		teamMember := tmcresult.Data.(model.TeamMember)
 		incomingHooks := ihcresult.Data.([]*model.IncomingWebhook)
 		outgoingHooks := ohcresult.Data.([]*model.OutgoingWebhook)
 
@@ -666,7 +677,7 @@ func deleteChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 			return
 		}
 
-		if !strings.Contains(channelMember.Roles, model.CHANNEL_ROLE_ADMIN) && !strings.Contains(c.Session.Roles, model.ROLE_TEAM_ADMIN) {
+		if !strings.Contains(channelMember.Roles, model.CHANNEL_ROLE_ADMIN) && !strings.Contains(teamMember.Roles, model.ROLE_TEAM_ADMIN) {
 			c.Err = model.NewLocAppError("deleteChannel", "api.channel.delete_channel.permissions.app_error", nil, "")
 			c.Err.StatusCode = http.StatusForbidden
 			return

api/channel_test.go

@@ -134,6 +134,7 @@ func TestUpdateChannel(t *testing.T) {
 	team := th.BasicTeam
 	user := th.BasicUser
 	user2 := th.CreateUser(th.BasicClient)
+	LinkUserToTeam(user2, team)
 
 	channel1 := &model.Channel{DisplayName: "A Test API Name", Name: "a" + model.NewId() + "a", Type: model.CHANNEL_OPEN, TeamId: team.Id}
 	channel1 = Client.Must(Client.CreateChannel(channel1)).Data.(*model.Channel)
@@ -175,6 +176,13 @@ func TestUpdateChannel(t *testing.T) {
 	if _, err := Client.UpdateChannel(upChannel1); err == nil {
 		t.Fatal("Standard User should have failed to update")
 	}
+
+	Client.Must(Client.JoinChannel(channel1.Id))
+	UpdateUserToTeamAdmin(user2, team)
+
+	if _, err := Client.UpdateChannel(upChannel1); err != nil {
+		t.Fatal(err)
+	}
 }
 
 func TestUpdateChannelHeader(t *testing.T) {
@@ -566,6 +574,12 @@ func TestDeleteChannel(t *testing.T) {
 			break
 		}
 	}
+
+	UpdateUserToTeamAdmin(userStd, team)
+
+	if _, err := Client.DeleteChannel(channel2.Id); err != nil {
+		t.Fatal(err)
+	}
 }
 
 func TestGetChannelExtraInfo(t *testing.T) {

i18n/en.json

@@ -3379,6 +3379,10 @@
     "id": "store.sql_team.get_by_name.app_error",
     "translation": "We couldn't find the existing team"
   },
+  {
+    "id": "store.sql_team.get_member.app_error",
+    "translation": "We couldn't get the team member"
+  },
   {
     "id": "store.sql_team.get_members.app_error",
     "translation": "We couldn't get the team members"

store/sql_team_store.go

@@ -411,6 +411,27 @@ func (s SqlTeamStore) UpdateMember(member *model.TeamMember) StoreChannel {
 	return storeChannel
 }
 
+func (s SqlTeamStore) GetMember(teamId string, userId string) StoreChannel {
+	storeChannel := make(StoreChannel)
+
+	go func() {
+		result := StoreResult{}
+
+		var member model.TeamMember
+		err := s.GetReplica().SelectOne(&member, "SELECT * FROM TeamMembers WHERE TeamId = :TeamId AND UserId = :UserId", map[string]interface{}{"TeamId": teamId, "UserId": userId})
+		if err != nil {
+			result.Err = model.NewLocAppError("SqlTeamStore.GetMember", "store.sql_team.get_member.app_error", nil, "teamId="+teamId+" userId="+userId+" "+err.Error())
+		} else {
+			result.Data = member
+		}
+
+		storeChannel <- result
+		close(storeChannel)
+	}()
+
+	return storeChannel
+}
+
 func (s SqlTeamStore) GetMembers(teamId string) StoreChannel {
 	storeChannel := make(StoreChannel)
 

store/sql_team_store_test.go

@@ -403,3 +403,34 @@ func TestTeamMembers(t *testing.T) {
 		}
 	}
 }
+
+func TestGetTeamMember(t *testing.T) {
+	Setup()
+
+	teamId1 := model.NewId()
+
+	m1 := &model.TeamMember{TeamId: teamId1, UserId: model.NewId()}
+	Must(store.Team().SaveMember(m1))
+
+	if r := <-store.Team().GetMember(m1.TeamId, m1.UserId); r.Err != nil {
+		t.Fatal(r.Err)
+	} else {
+		rm1 := r.Data.(model.TeamMember)
+
+		if rm1.TeamId != m1.TeamId {
+			t.Fatal("bad team id")
+		}
+
+		if rm1.UserId != m1.UserId {
+			t.Fatal("bad user id")
+		}
+	}
+
+	if r := <-store.Team().GetMember(m1.TeamId, ""); r.Err == nil {
+		t.Fatal("empty user id - should have failed")
+	}
+
+	if r := <-store.Team().GetMember("", m1.UserId); r.Err == nil {
+		t.Fatal("empty team id - should have failed")
+	}
+}

store/store.go

@@ -61,6 +61,7 @@ type TeamStore interface {
 	AnalyticsTeamCount() StoreChannel
 	SaveMember(member *model.TeamMember) StoreChannel
 	UpdateMember(member *model.TeamMember) StoreChannel
+	GetMember(teamId string, userId string) StoreChannel
 	GetMembers(teamId string) StoreChannel
 	GetTeamsForUser(userId string) StoreChannel
 	RemoveMember(teamId string, userId string) StoreChannel