Remove remote users from the license counting and explicitly dissallow them to log in (#22582)

* Making all the counts aware of Remote users

* Disable login for remote users

* Adding tests for login remote_users error

* Adding tests for the store

* Adding frontend part of not counting remote users in the license

* Addressing PR review comment

* Adding the new ExternaUserId field to users

* Running make migrations-extract

* Running make app-layers and make gen-serialized

* Revert "Adding the new ExternaUserId field to users"

This reverts commit 12e5fd5189.

* Adding GetUserByRemoteID methods

* Adding needed migration for users

* i18n-extract

* Fixing postgres increase remote user id field size migration up and down

* run make gen-serialized

* Removing migration code

* Not count remote users as part of the cloud pricing

* Add the cloud subscription when a user gets promote from remote to not-remote

* Fixing merge problems

---------

Co-authored-by: Mattermost Build <build@mattermost.com>

server/channels/api4/user.go

@@ -557,12 +557,14 @@ func getFilteredUsersStats(c *Context, w http.ResponseWriter, r *http.Request) {
 	channelID := r.URL.Query().Get("in_channel")
 	includeDeleted := r.URL.Query().Get("include_deleted")
 	includeBotAccounts := r.URL.Query().Get("include_bots")
+	includeRemoteUsers := r.URL.Query().Get("include_remote_users")
 	rolesString := r.URL.Query().Get("roles")
 	channelRolesString := r.URL.Query().Get("channel_roles")
 	teamRolesString := r.URL.Query().Get("team_roles")
 
 	includeDeletedBool, _ := strconv.ParseBool(includeDeleted)
 	includeBotAccountsBool, _ := strconv.ParseBool(includeBotAccounts)
+	includeRemoteUsersBool, _ := strconv.ParseBool(includeRemoteUsers)
 
 	roles := []string{}
 	var rolesValid bool
@@ -593,6 +595,7 @@ func getFilteredUsersStats(c *Context, w http.ResponseWriter, r *http.Request) {
 	options := &model.UserCountOptions{
 		IncludeDeleted:     includeDeletedBool,
 		IncludeBotAccounts: includeBotAccountsBool,
+		IncludeRemoteUsers: includeRemoteUsersBool,
 		TeamId:             teamID,
 		ChannelId:          channelID,
 		Roles:              roles,
@@ -1832,6 +1835,7 @@ func login(c *Context, w http.ResponseWriter, r *http.Request) {
 			"api.user.check_user_mfa.bad_code.app_error",
 			"api.user.login.blank_pwd.app_error",
 			"api.user.login.bot_login_forbidden.app_error",
+			"api.user.login.remote_users.login.error",
 			"api.user.login.client_side_cert.certificate.app_error",
 			"api.user.login.inactive.app_error",
 			"api.user.login.not_verified.app_error",
@@ -1932,6 +1936,11 @@ func login(c *Context, w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
+	if user.IsRemote() {
+		c.Err = model.NewAppError("login", "api.user.login.remote_users.login.error", nil, "", http.StatusUnauthorized)
+		return
+	}
+
 	c.LogAuditWithUserId(user.Id, "authenticated")
 
 	err = c.App.DoLogin(c.AppContext, w, r, user, deviceId, false, false, false)

server/channels/api4/user_test.go

@@ -3842,6 +3842,18 @@ func TestLogin(t *testing.T) {
 		CheckErrorID(t, err, "api.user.login.bot_login_forbidden.app_error")
 	})
 
+	t.Run("remote user login rejected", func(t *testing.T) {
+		email := th.GenerateTestEmail()
+		user := model.User{Email: email, Nickname: "Darth Vader", Password: "hello1", Username: GenerateTestUsername(), Roles: model.SystemAdminRoleId + " " + model.SystemUserRoleId, RemoteId: model.NewString("remote-id")}
+		ruser, _, _ := th.Client.CreateUser(context.Background(), &user)
+
+		_, err := th.SystemAdminClient.UpdateUserPassword(context.Background(), ruser.Id, "", "password")
+		require.NoError(t, err)
+
+		_, _, err = th.Client.Login(context.Background(), ruser.Email, "password")
+		CheckErrorID(t, err, "api.user.login.remote_users.login.error")
+	})
+
 	t.Run("login with terms_of_service set", func(t *testing.T) {
 		termsOfService, appErr := th.App.CreateTermsOfService("terms of service", th.BasicUser.Id)
 		require.Nil(t, appErr)

server/channels/app/app_iface.go

@@ -820,6 +820,7 @@ type AppIface interface {
 	GetUserAccessTokensForUser(userID string, page, perPage int) ([]*model.UserAccessToken, *model.AppError)
 	GetUserByAuth(authData *string, authService string) (*model.User, *model.AppError)
 	GetUserByEmail(email string) (*model.User, *model.AppError)
+	GetUserByRemoteID(remoteID string) (*model.User, *model.AppError)
 	GetUserByUsername(username string) (*model.User, *model.AppError)
 	GetUserForLogin(id, loginId string) (*model.User, *model.AppError)
 	GetUserTermsOfService(userID string) (*model.UserTermsOfService, *model.AppError)

server/channels/app/opentracing/opentracing_layer.go

@@ -10489,6 +10489,28 @@ func (a *OpenTracingAppLayer) GetUserByEmail(email string) (*model.User, *model.
 	return resultVar0, resultVar1
 }
 
+func (a *OpenTracingAppLayer) GetUserByRemoteID(remoteID string) (*model.User, *model.AppError) {
+	origCtx := a.ctx
+	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetUserByRemoteID")
+
+	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.GetUserByRemoteID(remoteID)
+
+	if resultVar1 != nil {
+		span.LogFields(spanlog.Error(resultVar1))
+		ext.Error.Set(span, true)
+	}
+
+	return resultVar0, resultVar1
+}
+
 func (a *OpenTracingAppLayer) GetUserByUsername(username string) (*model.User, *model.AppError) {
 	origCtx := a.ctx
 	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetUserByUsername")

server/channels/app/plugin_api.go

@@ -257,6 +257,10 @@ func (api *PluginAPI) GetUserByUsername(name string) (*model.User, *model.AppErr
 	return api.app.GetUserByUsername(name)
 }
 
+func (api *PluginAPI) GetUserByRemoteID(remoteID string) (*model.User, *model.AppError) {
+	return api.app.GetUserByRemoteID(remoteID)
+}
+
 func (api *PluginAPI) GetUsersByUsernames(usernames []string) ([]*model.User, *model.AppError) {
 	return api.app.GetUsersByUsernames(usernames, true, nil)
 }

server/channels/app/user.go

@@ -308,7 +308,7 @@ func (a *App) createUserOrGuest(c request.CTX, user *model.User, guest bool) (*m
 	// table in CWS. This is then used to calculate how much the customers have to pay in addition for the extra users. If the
 	// workspace is currently on a monthly plan, then this function will not do anything.
 
-	if a.Channels().License().IsCloud() {
+	if a.Channels().License().IsCloud() && !ruser.IsRemote() {
 		go func(userId string) {
 			_, err := a.SendSubscriptionHistoryEvent(userId)
 			if err != nil {
@@ -440,6 +440,20 @@ func (a *App) GetUserByEmail(email string) (*model.User, *model.AppError) {
 	return user, nil
 }
 
+func (a *App) GetUserByRemoteID(remoteID string) (*model.User, *model.AppError) {
+	user, err := a.ch.srv.userService.GetUserByRemoteID(remoteID)
+	if err != nil {
+		var nfErr *store.ErrNotFound
+		switch {
+		case errors.As(err, &nfErr):
+			return nil, model.NewAppError("GetUserByRemoteID", MissingAccountError, nil, "", http.StatusNotFound).Wrap(err)
+		default:
+			return nil, model.NewAppError("GetUserByRemoteID", MissingAccountError, nil, "", http.StatusInternalServerError).Wrap(err)
+		}
+	}
+	return user, nil
+}
+
 func (a *App) GetUserByAuth(authData *string, authService string) (*model.User, *model.AppError) {
 	user, err := a.ch.srv.userService.GetUserByAuth(authData, authService)
 	if err != nil {
@@ -1262,6 +1276,15 @@ func (a *App) UpdateUser(c request.CTX, user *model.User, sendNotifications bool
 	a.InvalidateCacheForUser(user.Id)
 	a.onUserProfileChange(user.Id)
 
+	if a.Channels().License().IsCloud() && prev.IsRemote() && !user.IsRemote() {
+		go func(userId string) {
+			_, err := a.SendSubscriptionHistoryEvent(userId)
+			if err != nil {
+				c.Logger().Error("Failed to create/update the SubscriptionHistoryEvent", mlog.Err(err))
+			}
+		}(user.Id)
+	}
+
 	return newUser, nil
 }
 

server/channels/app/users/users.go

@@ -105,6 +105,10 @@ func (us *UserService) GetUserByEmail(email string) (*model.User, error) {
 	return us.store.GetByEmail(email)
 }
 
+func (us *UserService) GetUserByRemoteID(remoteID string) (*model.User, error) {
+	return us.store.GetByRemoteID(remoteID)
+}
+
 func (us *UserService) GetUserByAuth(authData *string, authService string) (*model.User, error) {
 	return us.store.GetByAuth(authData, authService)
 }

server/channels/store/opentracinglayer/opentracinglayer.go

@@ -11148,6 +11148,24 @@ func (s *OpenTracingLayerUserStore) GetByEmail(email string) (*model.User, error
 	return result, err
 }
 
+func (s *OpenTracingLayerUserStore) GetByRemoteID(remoteID string) (*model.User, error) {
+	origCtx := s.Root.Store.Context()
+	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserStore.GetByRemoteID")
+	s.Root.Store.SetContext(newCtx)
+	defer func() {
+		s.Root.Store.SetContext(origCtx)
+	}()
+
+	defer span.Finish()
+	result, err := s.UserStore.GetByRemoteID(remoteID)
+	if err != nil {
+		span.LogFields(spanlog.Error(err))
+		ext.Error.Set(span, true)
+	}
+
+	return result, err
+}
+
 func (s *OpenTracingLayerUserStore) GetByUsername(username string) (*model.User, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserStore.GetByUsername")

server/channels/store/retrylayer/retrylayer.go

@@ -12739,6 +12739,27 @@ func (s *RetryLayerUserStore) GetByEmail(email string) (*model.User, error) {
 
 }
 
+func (s *RetryLayerUserStore) GetByRemoteID(remoteID string) (*model.User, error) {
+
+	tries := 0
+	for {
+		result, err := s.UserStore.GetByRemoteID(remoteID)
+		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
+		}
+		timepkg.Sleep(100 * timepkg.Millisecond)
+	}
+
+}
+
 func (s *RetryLayerUserStore) GetByUsername(username string) (*model.User, error) {
 
 	tries := 0

server/channels/store/sqlstore/user_store.go

@@ -1174,6 +1174,26 @@ func (us SqlUserStore) GetByEmail(email string) (*model.User, error) {
 	return &user, nil
 }
 
+func (us SqlUserStore) GetByRemoteID(remoteID string) (*model.User, error) {
+	query := us.usersQuery.Where(sq.Eq{"RemoteId": remoteID})
+
+	queryString, args, err := query.ToSql()
+	if err != nil {
+		return nil, errors.Wrap(err, "get_by_remote_id_tosql")
+	}
+
+	user := model.User{}
+	if err := us.GetReplicaX().Get(&user, queryString, args...); err != nil {
+		if err == sql.ErrNoRows {
+			return nil, errors.Wrap(store.NewErrNotFound("User", fmt.Sprintf("remoteid=%s", remoteID)), "failed to find User")
+		}
+
+		return nil, errors.Wrapf(err, "failed to get User with RemoteId=%s", remoteID)
+	}
+
+	return &user, nil
+}
+
 func (us SqlUserStore) GetByAuth(authData *string, authService string) (*model.User, error) {
 	if authData == nil || *authData == "" {
 		return nil, store.NewErrInvalidInput("User", "<authData>", "empty or nil")
@@ -1310,6 +1330,10 @@ func (us SqlUserStore) Count(options model.UserCountOptions) (int64, error) {
 		query = query.Where("u.DeleteAt = 0")
 	}
 
+	if !options.IncludeRemoteUsers {
+		query = query.Where(sq.Or{sq.Eq{"u.RemoteId": ""}, sq.Eq{"u.RemoteId": nil}})
+	}
+
 	isPostgreSQL := us.DriverName() == model.DatabaseDriverPostgres
 	if options.IncludeBotAccounts {
 		if options.ExcludeRegularUsers {
@@ -1365,8 +1389,17 @@ func (us SqlUserStore) AnalyticsActiveCount(timePeriod int64, options model.User
 			query = query.Where(sq.Expr("UserId NOT IN (SELECT UserId FROM Bots)"))
 		}
 	}
+
+	if !options.IncludeRemoteUsers || !options.IncludeDeleted {
+		query = query.LeftJoin("Users ON s.UserId = Users.Id")
+	}
+
+	if !options.IncludeRemoteUsers {
+		query = query.Where(sq.Or{sq.Eq{"Users.RemoteId": ""}, sq.Eq{"Users.RemoteId": nil}})
+	}
+
 	if !options.IncludeDeleted {
-		query = query.LeftJoin("Users ON s.UserId = Users.Id").Where("Users.DeleteAt = 0")
+		query = query.Where("Users.DeleteAt = 0")
 	}
 
 	queryStr, args, err := query.ToSql()
@@ -1393,8 +1426,16 @@ func (us SqlUserStore) AnalyticsActiveCountForPeriod(startTime int64, endTime in
 		}
 	}
 
+	if !options.IncludeRemoteUsers || !options.IncludeDeleted {
+		query = query.LeftJoin("Users ON s.UserId = Users.Id")
+	}
+
+	if !options.IncludeRemoteUsers {
+		query = query.Where(sq.Or{sq.Eq{"Users.RemoteId": ""}, sq.Eq{"Users.RemoteId": nil}})
+	}
+
 	if !options.IncludeDeleted {
-		query = query.LeftJoin("Users ON s.UserId = Users.Id").Where("Users.DeleteAt = 0")
+		query = query.Where("Users.DeleteAt = 0")
 	}
 
 	queryStr, args, err := query.ToSql()

server/channels/store/store.go

@@ -424,6 +424,7 @@ type UserStore interface {
 	GetProfileByGroupChannelIdsForUser(userID string, channelIds []string) (map[string][]*model.User, error)
 	InvalidateProfileCacheForUser(userID string)
 	GetByEmail(email string) (*model.User, error)
+	GetByRemoteID(remoteID string) (*model.User, error)
 	GetByAuth(authData *string, authService string) (*model.User, error)
 	GetAllUsingAuthService(authService string) ([]*model.User, error)
 	GetAllNotInAuthService(authServices []string) ([]*model.User, error)

server/channels/store/storetest/mocks/UserStore.go

@@ -541,6 +541,32 @@ func (_m *UserStore) GetByEmail(email string) (*model.User, error) {
 	return r0, r1
 }
 
+// GetByRemoteID provides a mock function with given fields: remoteID
+func (_m *UserStore) GetByRemoteID(remoteID string) (*model.User, error) {
+	ret := _m.Called(remoteID)
+
+	var r0 *model.User
+	var r1 error
+	if rf, ok := ret.Get(0).(func(string) (*model.User, error)); ok {
+		return rf(remoteID)
+	}
+	if rf, ok := ret.Get(0).(func(string) *model.User); ok {
+		r0 = rf(remoteID)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*model.User)
+		}
+	}
+
+	if rf, ok := ret.Get(1).(func(string) error); ok {
+		r1 = rf(remoteID)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
 // GetByUsername provides a mock function with given fields: username
 func (_m *UserStore) GetByUsername(username string) (*model.User, error) {
 	ret := _m.Called(username)

server/channels/store/storetest/user_store.go

@@ -3963,6 +3963,15 @@ func testCount(t *testing.T, ss store.Store) {
 	require.NoError(t, err)
 	defer func() { require.NoError(t, ss.User().PermanentDelete(deletedUser.Id)) }()
 
+	// Remote User
+	remoteId := "remote-id"
+	remoteUser, err := ss.User().Save(&model.User{
+		Email:    MakeEmail(),
+		RemoteId: &remoteId,
+	})
+	require.NoError(t, err)
+	defer func() { require.NoError(t, ss.User().PermanentDelete(remoteUser.Id)) }()
+
 	// Bot
 	botUser, err := ss.User().Save(&model.User{
 		Email: MakeEmail(),
@@ -4067,6 +4076,71 @@ func testCount(t *testing.T, ss store.Store) {
 			},
 			0,
 		},
+		{
+			"Include remote accounts no deleted accounts and no team id",
+			model.UserCountOptions{
+				IncludeRemoteUsers: true,
+				IncludeDeleted:     false,
+				TeamId:             "",
+			},
+			5,
+		},
+		{
+			"Include delete accounts no remote accounts and no team id",
+			model.UserCountOptions{
+				IncludeRemoteUsers: false,
+				IncludeDeleted:     true,
+				TeamId:             "",
+			},
+			5,
+		},
+		{
+			"Include remote accounts and deleted accounts and no team id",
+			model.UserCountOptions{
+				IncludeRemoteUsers: true,
+				IncludeDeleted:     true,
+				TeamId:             "",
+			},
+			6,
+		},
+		{
+			"Include remote accounts and deleted accounts with existing team id",
+			model.UserCountOptions{
+				IncludeRemoteUsers: true,
+				IncludeDeleted:     true,
+				TeamId:             teamId,
+			},
+			4,
+		},
+		{
+			"Include remote accounts and deleted accounts with fake team id",
+			model.UserCountOptions{
+				IncludeRemoteUsers: true,
+				IncludeDeleted:     true,
+				TeamId:             model.NewId(),
+			},
+			0,
+		},
+		{
+			"Include remote accounts and deleted accounts with existing team id and view restrictions allowing team",
+			model.UserCountOptions{
+				IncludeRemoteUsers: true,
+				IncludeDeleted:     true,
+				TeamId:             teamId,
+				ViewRestrictions:   &model.ViewUsersRestrictions{Teams: []string{teamId}},
+			},
+			4,
+		},
+		{
+			"Include remote accounts and deleted accounts with existing team id and view restrictions not allowing current team",
+			model.UserCountOptions{
+				IncludeRemoteUsers: true,
+				IncludeDeleted:     true,
+				TeamId:             teamId,
+				ViewRestrictions:   &model.ViewUsersRestrictions{Teams: []string{model.NewId()}},
+			},
+			0,
+		},
 		{
 			"Filter by system admins only",
 			model.UserCountOptions{

server/channels/store/timerlayer/timerlayer.go

@@ -10032,6 +10032,22 @@ func (s *TimerLayerUserStore) GetByEmail(email string) (*model.User, error) {
 	return result, err
 }
 
+func (s *TimerLayerUserStore) GetByRemoteID(remoteID string) (*model.User, error) {
+	start := time.Now()
+
+	result, err := s.UserStore.GetByRemoteID(remoteID)
+
+	elapsed := float64(time.Since(start)) / float64(time.Second)
+	if s.Root.Metrics != nil {
+		success := "false"
+		if err == nil {
+			success = "true"
+		}
+		s.Root.Metrics.ObserveStoreMethodDuration("UserStore.GetByRemoteID", success, elapsed)
+	}
+	return result, err
+}
+
 func (s *TimerLayerUserStore) GetByUsername(username string) (*model.User, error) {
 	start := time.Now()
 

server/i18n/en.json

@@ -4291,6 +4291,10 @@
     "id": "api.user.login.not_verified.app_error",
     "translation": "Login failed because email address has not been verified."
   },
+  {
+    "id": "api.user.login.remote_users.login.error",
+    "translation": "Login failed because remote users are not allow to log in."
+  },
   {
     "id": "api.user.login.use_auth_service.app_error",
     "translation": "Please sign in using {{.AuthService}}."

server/public/model/user_count.go

@@ -9,6 +9,8 @@ type UserCountOptions struct {
 	IncludeBotAccounts bool
 	// Should include deleted users (of any type)
 	IncludeDeleted bool
+	// Include remote users
+	IncludeRemoteUsers bool
 	// Exclude regular users
 	ExcludeRegularUsers bool
 	// Only include users on a specific team. "" for any team.

webapp/channels/src/components/user_list_row_with_error/user_list_row_with_error.tsx

@@ -2,11 +2,13 @@
 // See LICENSE.txt for license information.
 
 import React from 'react';
+import {FormattedMessage} from 'react-intl';
 import {Link} from 'react-router-dom';
 
 import {ConnectedComponent} from 'react-redux';
 
 import BotTag from 'components/widgets/tag/bot_tag';
+import Tag from 'components/widgets/tag/tag';
 
 import {Client4} from 'mattermost-redux/client';
 
@@ -154,6 +156,16 @@ export default class UserListRowWithError extends React.PureComponent<Props, Sta
                                 />
 
                                 {this.props.user.is_bot && <BotTag/>}
+                                {this.props.user.remote_id && (
+                                    <Tag
+                                        text={
+                                            <FormattedMessage
+                                                id='admin.user_item.remoteUser'
+                                                defaultMessage='Remote user'
+                                            />
+                                        }
+                                    />
+                                )}
                             </div>
                             <div
                                 id={userCountEmail || undefined}

webapp/channels/src/i18n/en.json

@@ -2561,6 +2561,7 @@
   "admin.user_item.mfaNo": "**MFA**: No",
   "admin.user_item.mfaYes": "**MFA**: Yes",
   "admin.user_item.promoteToMember": "Promote to Member",
+  "admin.user_item.remoteUser": "Remote user",
   "admin.user_item.resetEmail": "Update Email",
   "admin.user_item.resetMfa": "Remove MFA",
   "admin.user_item.resetPwd": "Reset Password",

webapp/platform/types/src/users.ts

@@ -133,6 +133,7 @@ export type GetFilteredUsersStatsOpts = {
     in_channel?: string;
     include_deleted?: boolean;
     include_bots?: boolean;
+    include_remote_users?: boolean;
     roles?: string[];
     channel_roles?: string[];
     team_roles?: string[];