[MM-7968] restrict creation of direct channels to team members (#17222)

* restrict creation of direct channels to team members

* run make i18n-extract

* add suggestions from hahmadia

* place common-team-check logic in app layer

* use flat SQL query

* show more specific error message to user

* MM-7968: Fmt file.

* MM-7968: Fix for moved session field.

Co-authored-by: Max Erenberg <max.erenberg@mattermost.com>
Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
Co-authored-by: Martin Kraft <martinkraft@gmail.com>
Co-authored-by: Martin Kraft <martin@upspin.org>

api4/channel_test.go

@@ -418,12 +418,24 @@ func TestCreateDirectChannel(t *testing.T) {
 	require.NotNil(t, err)
 	require.Equal(t, http.StatusBadRequest, r.StatusCode)
 
+	_, resp = th.SystemAdminClient.CreateDirectChannel(user3.Id, user2.Id)
+	CheckNoError(t, resp)
+
+	// Normal client should not be allowed to create a direct channel if users are
+	// restricted to messaging members of their own team
+	th.App.UpdateConfig(func(cfg *model.Config) {
+		*cfg.TeamSettings.RestrictDirectMessage = model.DIRECT_MESSAGE_TEAM
+	})
+	user4 := th.CreateUser()
+	_, resp = th.Client.CreateDirectChannel(user1.Id, user4.Id)
+	CheckForbiddenStatus(t, resp)
+	th.LinkUserToTeam(user4, th.BasicTeam)
+	_, resp = th.Client.CreateDirectChannel(user1.Id, user4.Id)
+	CheckNoError(t, resp)
+
 	Client.Logout()
 	_, resp = Client.CreateDirectChannel(model.NewId(), user2.Id)
 	CheckUnauthorizedStatus(t, resp)
-
-	_, resp = th.SystemAdminClient.CreateDirectChannel(user3.Id, user2.Id)
-	CheckNoError(t, resp)
 }
 
 func TestCreateDirectChannelAsGuest(t *testing.T) {

app/app_iface.go

@@ -601,6 +601,7 @@ type AppIface interface {
 	GetClusterId() string
 	GetClusterStatus() []*model.ClusterInfo
 	GetCommand(commandID string) (*model.Command, *model.AppError)
+	GetCommonTeamIDsForTwoUsers(userID, otherUserID string) ([]string, *model.AppError)
 	GetComplianceFile(job *model.Compliance) ([]byte, *model.AppError)
 	GetComplianceReport(reportId string) (*model.Compliance, *model.AppError)
 	GetComplianceReports(page, perPage int) (model.Compliances, *model.AppError)

app/channel.go

@@ -332,6 +332,17 @@ func (a *App) GetOrCreateDirectChannel(c *request.Context, userID, otherUserID s
 		return channel, nil
 	}
 
+	if *a.Config().TeamSettings.RestrictDirectMessage == model.DIRECT_MESSAGE_TEAM &&
+		!a.SessionHasPermissionTo(*c.Session(), model.PERMISSION_MANAGE_SYSTEM) {
+		commonTeamIDs, err := a.GetCommonTeamIDsForTwoUsers(userID, otherUserID)
+		if err != nil {
+			return nil, err
+		}
+		if len(commonTeamIDs) == 0 {
+			return nil, model.NewAppError("createDirectChannel", "api.channel.create_channel.direct_channel.team_restricted_error", nil, "", http.StatusForbidden)
+		}
+	}
+
 	channel, err := a.createDirectChannel(userID, otherUserID, channelOptions...)
 	if err != nil {
 		if err.Id == store.ChannelExistsError {

app/opentracing/opentracing_layer.go

@@ -5313,6 +5313,28 @@ func (a *OpenTracingAppLayer) GetCommand(commandID string) (*model.Command, *mod
 	return resultVar0, resultVar1
 }
 
+func (a *OpenTracingAppLayer) GetCommonTeamIDsForTwoUsers(userID string, otherUserID string) ([]string, *model.AppError) {
+	origCtx := a.ctx
+	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetCommonTeamIDsForTwoUsers")
+
+	a.ctx = newCtx
+	a.app.Srv().Store.SetContext(newCtx)
+	defer func() {
+		a.app.Srv().Store.SetContext(origCtx)
+		a.ctx = origCtx
+	}()
+
+	defer span.Finish()
+	resultVar0, resultVar1 := a.app.GetCommonTeamIDsForTwoUsers(userID, otherUserID)
+
+	if resultVar1 != nil {
+		span.LogFields(spanlog.Error(resultVar1))
+		ext.Error.Set(span, true)
+	}
+
+	return resultVar0, resultVar1
+}
+
 func (a *OpenTracingAppLayer) GetComplianceFile(job *model.Compliance) ([]byte, *model.AppError) {
 	origCtx := a.ctx
 	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetComplianceFile")

app/slashcommands/command_msg.go

@@ -85,7 +85,7 @@ func (*msgProvider) DoCommand(a *app.App, c *request.Context, args *model.Comman
 			var directChannel *model.Channel
 			if directChannel, err = a.GetOrCreateDirectChannel(c, args.UserId, userProfile.Id); err != nil {
 				mlog.Error(err.Error())
-				return &model.CommandResponse{Text: args.T("api.command_msg.dm_fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
+				return &model.CommandResponse{Text: args.T(err.Id), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
 			}
 			targetChannelId = directChannel.Id
 		} else {

app/team.go

@@ -1016,6 +1016,14 @@ func (a *App) GetTeamMembersByIds(teamID string, userIDs []string, restrictions
 	return teamMembers, nil
 }
 
+func (a *App) GetCommonTeamIDsForTwoUsers(userID, otherUserID string) ([]string, *model.AppError) {
+	teamIDs, err := a.Srv().Store.Team().GetCommonTeamIDsForTwoUsers(userID, otherUserID)
+	if err != nil {
+		return nil, model.NewAppError("GetCommonTeamIDsForUsers", "app.team.get_common_team_ids_for_users.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}
+	return teamIDs, nil
+}
+
 func (a *App) AddTeamMember(c *request.Context, teamID, userID string) (*model.TeamMember, *model.AppError) {
 	_, teamMember, err := a.AddUserToTeam(c, teamID, userID, "")
 	if err != nil {

i18n/en.json

@@ -271,6 +271,10 @@
     "id": "api.channel.create_channel.direct_channel.app_error",
     "translation": "Must use createDirectChannel API service for direct message channel creation."
   },
+  {
+    "id": "api.channel.create_channel.direct_channel.team_restricted_error",
+    "translation": "A direct channel cannot be created between these users because they do not share a team in common."
+  },
   {
     "id": "api.channel.create_channel.max_channel_limit.app_error",
     "translation": "Unable to create more than {{.MaxChannelsPerTeam}} channels for current team."
@@ -6070,6 +6074,10 @@
     "id": "app.team.get_by_scheme.app_error",
     "translation": "Unable to get the channels for the provided scheme."
   },
+  {
+    "id": "app.team.get_common_team_ids_for_users.app_error",
+    "translation": "Unable to get the common team IDs."
+  },
   {
     "id": "app.team.get_member.app_error",
     "translation": "Unable to get the team member."

store/opentracinglayer/opentracinglayer.go

@@ -8061,6 +8061,24 @@ func (s *OpenTracingLayerTeamStore) GetChannelUnreadsForTeam(teamID string, user
 	return result, err
 }
 
+func (s *OpenTracingLayerTeamStore) GetCommonTeamIDsForTwoUsers(userID string, otherUserID string) ([]string, error) {
+	origCtx := s.Root.Store.Context()
+	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "TeamStore.GetCommonTeamIDsForTwoUsers")
+	s.Root.Store.SetContext(newCtx)
+	defer func() {
+		s.Root.Store.SetContext(origCtx)
+	}()
+
+	defer span.Finish()
+	result, err := s.TeamStore.GetCommonTeamIDsForTwoUsers(userID, otherUserID)
+	if err != nil {
+		span.LogFields(spanlog.Error(err))
+		ext.Error.Set(span, true)
+	}
+
+	return result, err
+}
+
 func (s *OpenTracingLayerTeamStore) GetMember(ctx context.Context, teamID string, userID string) (*model.TeamMember, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "TeamStore.GetMember")

store/retrylayer/retrylayer.go

@@ -8762,6 +8762,26 @@ func (s *RetryLayerTeamStore) GetChannelUnreadsForTeam(teamID string, userID str
 
 }
 
+func (s *RetryLayerTeamStore) GetCommonTeamIDsForTwoUsers(userID string, otherUserID string) ([]string, error) {
+
+	tries := 0
+	for {
+		result, err := s.TeamStore.GetCommonTeamIDsForTwoUsers(userID, otherUserID)
+		if err == nil {
+			return result, nil
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+	}
+
+}
+
 func (s *RetryLayerTeamStore) GetMember(ctx context.Context, teamID string, userID string) (*model.TeamMember, error) {
 
 	tries := 0

store/sqlstore/team_store.go

@@ -1423,6 +1423,34 @@ func (s SqlTeamStore) GetUserTeamIds(userId string, allowFromCache bool) ([]stri
 	return teamIds, nil
 }
 
+// GetCommonTeamIDsForTwoUsers returns the intersection of all the teams to which the specified
+// users belong.
+func (s SqlTeamStore) GetCommonTeamIDsForTwoUsers(userID, otherUserID string) ([]string, error) {
+	var teamIDs []string
+	query, args, err := s.getQueryBuilder().
+		Select("TM1.TeamId").
+		From("TeamMembers AS TM1").
+		InnerJoin("TeamMembers AS TM2 ON TM1.TeamId = TM2.TeamId").
+		InnerJoin("Teams ON TM1.TeamId = Teams.Id").
+		Where(sq.And{
+			sq.Eq{"TM1.UserId": userID},
+			sq.Eq{"TM1.DeleteAt": 0},
+			sq.Eq{"TM2.UserId": otherUserID},
+			sq.Eq{"TM2.DeleteAt": 0},
+			sq.Eq{"Teams.DeleteAt": 0},
+		}).
+		ToSql()
+	if err != nil {
+		return nil, errors.Wrap(err, "team_tosql")
+	}
+	_, err = s.GetReplica().Select(&teamIDs, query, args...)
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to find TeamMembers with user IDs %s and %s", userID, otherUserID)
+	}
+
+	return teamIDs, nil
+}
+
 // GetTeamMembersForExport gets the various teams for which a user, denoted by userId, is a part of.
 func (s SqlTeamStore) GetTeamMembersForExport(userId string) ([]*model.TeamMemberForExport, error) {
 	var members []*model.TeamMemberForExport

store/store.go

@@ -150,6 +150,10 @@ type TeamStore interface {
 
 	// GroupSyncedTeamCount returns the count of non-deleted group-constrained teams.
 	GroupSyncedTeamCount() (int64, error)
+
+	// GetCommonTeamIDsForTwoUsers returns the intersection of all the teams to which the specified
+	// users belong.
+	GetCommonTeamIDsForTwoUsers(userID, otherUserID string) ([]string, error)
 }
 
 type ChannelStore interface {

store/storetest/mocks/TeamStore.go

@@ -351,6 +351,29 @@ func (_m *TeamStore) GetChannelUnreadsForTeam(teamID string, userID string) ([]*
 	return r0, r1
 }
 
+// GetCommonTeamIDsForTwoUsers provides a mock function with given fields: userID, otherUserID
+func (_m *TeamStore) GetCommonTeamIDsForTwoUsers(userID string, otherUserID string) ([]string, error) {
+	ret := _m.Called(userID, otherUserID)
+
+	var r0 []string
+	if rf, ok := ret.Get(0).(func(string, string) []string); ok {
+		r0 = rf(userID, otherUserID)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).([]string)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(string, string) error); ok {
+		r1 = rf(userID, otherUserID)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
 // GetMember provides a mock function with given fields: ctx, teamID, userID
 func (_m *TeamStore) GetMember(ctx context.Context, teamID string, userID string) (*model.TeamMember, error) {
 	ret := _m.Called(ctx, teamID, userID)

store/timerlayer/timerlayer.go

@@ -7271,6 +7271,22 @@ func (s *TimerLayerTeamStore) GetChannelUnreadsForTeam(teamID string, userID str
 	return result, err
 }
 
+func (s *TimerLayerTeamStore) GetCommonTeamIDsForTwoUsers(userID string, otherUserID string) ([]string, error) {
+	start := timemodule.Now()
+
+	result, err := s.TeamStore.GetCommonTeamIDsForTwoUsers(userID, otherUserID)
+
+	elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second)
+	if s.Root.Metrics != nil {
+		success := "false"
+		if err == nil {
+			success = "true"
+		}
+		s.Root.Metrics.ObserveStoreMethodDuration("TeamStore.GetCommonTeamIDsForTwoUsers", success, elapsed)
+	}
+	return result, err
+}
+
 func (s *TimerLayerTeamStore) GetMember(ctx context.Context, teamID string, userID string) (*model.TeamMember, error) {
 	start := timemodule.Now()