From 12e5fd518962a16cdbdb8964179b6cd8e915f230 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Fri, 5 May 2023 18:56:23 +0200 Subject: [PATCH] Adding the new ExternaUserId field to users --- server/channels/api4/user.go | 24 +++---- server/channels/api4/user_test.go | 6 +- server/channels/app/plugin_api.go | 4 ++ server/channels/app/user.go | 14 ++++ server/channels/app/users/users.go | 4 ++ .../mysql/000109_external_user_id.down.sql | 29 ++++++++ .../mysql/000109_external_user_id.up.sql | 29 ++++++++ .../postgres/000109_external_user_id.down.sql | 2 + .../postgres/000109_external_user_id.up.sql | 2 + server/channels/product/api.go | 1 + .../opentracinglayer/opentracinglayer.go | 18 +++++ .../channels/store/retrylayer/retrylayer.go | 21 ++++++ server/channels/store/sqlstore/user_store.go | 48 +++++++++---- server/channels/store/store.go | 1 + .../store/storetest/mocks/UserStore.go | 26 +++++++ server/channels/store/storetest/user_store.go | 72 +++++++++---------- .../channels/store/timerlayer/timerlayer.go | 16 +++++ server/model/user.go | 9 ++- server/model/user_count.go | 4 +- server/plugin/api.go | 6 ++ server/plugin/api_timer_layer_generated.go | 7 ++ server/plugin/client_rpc_generated.go | 29 ++++++++ server/plugin/plugintest/api.go | 28 ++++++++ .../user_list_row_with_error.tsx | 6 +- webapp/platform/types/src/users.ts | 3 +- 25 files changed, 337 insertions(+), 72 deletions(-) create mode 100644 server/channels/db/migrations/mysql/000109_external_user_id.down.sql create mode 100644 server/channels/db/migrations/mysql/000109_external_user_id.up.sql create mode 100644 server/channels/db/migrations/postgres/000109_external_user_id.down.sql create mode 100644 server/channels/db/migrations/postgres/000109_external_user_id.up.sql diff --git a/server/channels/api4/user.go b/server/channels/api4/user.go index 62ac9d28ce..bf87c730b1 100644 --- a/server/channels/api4/user.go +++ b/server/channels/api4/user.go @@ -558,14 +558,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") + includeExternalUsers := r.URL.Query().Get("include_external_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) + includeExternalUsersBool, _ := strconv.ParseBool(includeExternalUsers) roles := []string{} var rolesValid bool @@ -594,14 +594,14 @@ 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, - ChannelRoles: channelRoles, - TeamRoles: teamRoles, + IncludeDeleted: includeDeletedBool, + IncludeBotAccounts: includeBotAccountsBool, + IncludeExternalUsers: includeExternalUsersBool, + TeamId: teamID, + ChannelId: channelID, + Roles: roles, + ChannelRoles: channelRoles, + TeamRoles: teamRoles, } if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionSysconsoleReadUserManagementUsers) { @@ -1834,7 +1834,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.external_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", @@ -1935,7 +1935,7 @@ func login(c *Context, w http.ResponseWriter, r *http.Request) { } } - if user.IsRemote() { + if user.IsExternal() { c.Err = model.NewAppError("login", "api.user.login.remote_users.login.error", nil, "", http.StatusUnauthorized) return } diff --git a/server/channels/api4/user_test.go b/server/channels/api4/user_test.go index 0f1947ad39..9ea7297d3f 100644 --- a/server/channels/api4/user_test.go +++ b/server/channels/api4/user_test.go @@ -3746,16 +3746,16 @@ 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) { + t.Run("external 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")} + user := model.User{Email: email, Nickname: "Darth Vader", Password: "hello1", Username: GenerateTestUsername(), Roles: model.SystemAdminRoleId + " " + model.SystemUserRoleId, ExternalUserId: model.NewString("external-user-id")} ruser, _, _ := th.Client.CreateUser(&user) _, err := th.SystemAdminClient.UpdateUserPassword(ruser.Id, "", "password") require.NoError(t, err) _, _, err = th.Client.Login(ruser.Email, "password") - CheckErrorID(t, err, "api.user.login.remote_users.login.error") + CheckErrorID(t, err, "api.user.login.external_users.login.error") }) t.Run("login with terms_of_service set", func(t *testing.T) { diff --git a/server/channels/app/plugin_api.go b/server/channels/app/plugin_api.go index 71fefaa2aa..5c09dadc49 100644 --- a/server/channels/app/plugin_api.go +++ b/server/channels/app/plugin_api.go @@ -253,6 +253,10 @@ func (api *PluginAPI) GetUserByEmail(email string) (*model.User, *model.AppError return api.app.GetUserByEmail(email) } +func (api *PluginAPI) GetUserByExternalUserId(externalUserId string) (*model.User, *model.AppError) { + return api.app.GetUserByExternalUserId(externalUserId) +} + func (api *PluginAPI) GetUserByUsername(name string) (*model.User, *model.AppError) { return api.app.GetUserByUsername(name) } diff --git a/server/channels/app/user.go b/server/channels/app/user.go index 8787402a50..2d0be11849 100644 --- a/server/channels/app/user.go +++ b/server/channels/app/user.go @@ -461,6 +461,20 @@ func (a *App) GetUserByEmail(email string) (*model.User, *model.AppError) { return user, nil } +func (a *App) GetUserByExternalUserId(externalUserId string) (*model.User, *model.AppError) { + user, err := a.ch.srv.userService.GetUserByExternalUserId(externalUserId) + if err != nil { + var nfErr *store.ErrNotFound + switch { + case errors.As(err, &nfErr): + return nil, model.NewAppError("GetUserByExternalUserId", MissingAccountError, nil, "", http.StatusNotFound).Wrap(err) + default: + return nil, model.NewAppError("GetUserByExternalUserId", 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 { diff --git a/server/channels/app/users/users.go b/server/channels/app/users/users.go index 543be78956..e339292982 100644 --- a/server/channels/app/users/users.go +++ b/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) GetUserByExternalUserId(externalUserId string) (*model.User, error) { + return us.store.GetByExternalUserId(externalUserId) +} + func (us *UserService) GetUserByAuth(authData *string, authService string) (*model.User, error) { return us.store.GetByAuth(authData, authService) } diff --git a/server/channels/db/migrations/mysql/000109_external_user_id.down.sql b/server/channels/db/migrations/mysql/000109_external_user_id.down.sql new file mode 100644 index 0000000000..e361f64a4f --- /dev/null +++ b/server/channels/db/migrations/mysql/000109_external_user_id.down.sql @@ -0,0 +1,29 @@ +SET @preparedStatement = (SELECT IF( + EXISTS( + SELECT 1 FROM INFORMATION_SCHEMA.STATISTICS + WHERE table_name = 'Users' + AND table_schema = DATABASE() + AND index_name = 'idx_users_externaluserid' + ) > 0, + 'DROP INDEX idx_users_externaluserid ON Users;', + 'SELECT 1' +)); + +PREPARE removeIndexIfExists FROM @preparedStatement; +EXECUTE removeIndexIfExists; +DEALLOCATE PREPARE removeIndexIfExists; + +SET @preparedStatement = (SELECT IF( + EXISTS( + SELECT 1 FROM INFORMATION_SCHEMA.STATISTICS + WHERE table_name = 'Users' + AND table_schema = DATABASE() + AND column_name = 'ExternalUserId' + ) > 0, + 'ALTER TABLE Users DROP COLUMN ExternalUserId;', + 'SELECT 1;' +)); + +PREPARE removeColumnIfExists FROM @preparedStatement; +EXECUTE removeColumnIfExists; +DEALLOCATE PREPARE removeColumnIfExists; diff --git a/server/channels/db/migrations/mysql/000109_external_user_id.up.sql b/server/channels/db/migrations/mysql/000109_external_user_id.up.sql new file mode 100644 index 0000000000..2853ef8783 --- /dev/null +++ b/server/channels/db/migrations/mysql/000109_external_user_id.up.sql @@ -0,0 +1,29 @@ +SET @preparedStatement = (SELECT IF( + NOT EXISTS( + SELECT 1 FROM INFORMATION_SCHEMA.COLUMNS + WHERE table_name = 'Users' + AND table_schema = DATABASE() + AND column_name = 'ExternalUserId' + ), + 'ALTER TABLE Users ADD COLUMN ExternalUserId varchar(26);', + 'SELECT 1;' +)); + +PREPARE addColumnIfNotExists FROM @preparedStatement; +EXECUTE addColumnIfNotExists; +DEALLOCATE PREPARE addColumnIfNotExists; + +SET @preparedStatement = (SELECT IF( + NOT EXISTS( + SELECT 1 FROM INFORMATION_SCHEMA.STATISTICS + WHERE table_name = 'Users' + AND table_schema = DATABASE() + AND index_name = 'idx_users_externaluserid' + ), + 'CREATE INDEX idx_users_externaluserid ON Users(ExternalUserId);', + 'SELECT 1' +)); + +PREPARE createIndexIfNotExists FROM @preparedStatement; +EXECUTE createIndexIfNotExists; +DEALLOCATE PREPARE createIndexIfNotExists; diff --git a/server/channels/db/migrations/postgres/000109_external_user_id.down.sql b/server/channels/db/migrations/postgres/000109_external_user_id.down.sql new file mode 100644 index 0000000000..7559130d08 --- /dev/null +++ b/server/channels/db/migrations/postgres/000109_external_user_id.down.sql @@ -0,0 +1,2 @@ +DROP INDEX IF EXISTS idx_users_externaluserid; +ALTER TABLE fileinfo DROP COLUMN IF EXISTS externaluserid; diff --git a/server/channels/db/migrations/postgres/000109_external_user_id.up.sql b/server/channels/db/migrations/postgres/000109_external_user_id.up.sql new file mode 100644 index 0000000000..bfb4e8c09d --- /dev/null +++ b/server/channels/db/migrations/postgres/000109_external_user_id.up.sql @@ -0,0 +1,2 @@ +ALTER TABLE users ADD COLUMN IF NOT EXISTS externaluserid varchar(255); +CREATE INDEX IF NOT EXISTS idx_users_externaluserid ON users(externaluserid); diff --git a/server/channels/product/api.go b/server/channels/product/api.go index 73cff0a451..759812d1fc 100644 --- a/server/channels/product/api.go +++ b/server/channels/product/api.go @@ -101,6 +101,7 @@ type UserService interface { GetUser(userID string) (*model.User, *model.AppError) UpdateUser(c request.CTX, user *model.User, sendNotifications bool) (*model.User, *model.AppError) GetUserByEmail(email string) (*model.User, *model.AppError) + GetUserByExternalUserId(externalUserId string) (*model.User, *model.AppError) GetUserByUsername(username string) (*model.User, *model.AppError) GetUsersFromProfiles(options *model.UserGetOptions) ([]*model.User, *model.AppError) } diff --git a/server/channels/store/opentracinglayer/opentracinglayer.go b/server/channels/store/opentracinglayer/opentracinglayer.go index f70d14eddc..6e4fc1bf71 100644 --- a/server/channels/store/opentracinglayer/opentracinglayer.go +++ b/server/channels/store/opentracinglayer/opentracinglayer.go @@ -11211,6 +11211,24 @@ func (s *OpenTracingLayerUserStore) GetByEmail(email string) (*model.User, error return result, err } +func (s *OpenTracingLayerUserStore) GetByExternalUserId(externalUserId string) (*model.User, error) { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserStore.GetByExternalUserId") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + result, err := s.UserStore.GetByExternalUserId(externalUserId) + 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") diff --git a/server/channels/store/retrylayer/retrylayer.go b/server/channels/store/retrylayer/retrylayer.go index 37d71bd2d6..2372478cf6 100644 --- a/server/channels/store/retrylayer/retrylayer.go +++ b/server/channels/store/retrylayer/retrylayer.go @@ -12814,6 +12814,27 @@ func (s *RetryLayerUserStore) GetByEmail(email string) (*model.User, error) { } +func (s *RetryLayerUserStore) GetByExternalUserId(externalUserId string) (*model.User, error) { + + tries := 0 + for { + result, err := s.UserStore.GetByExternalUserId(externalUserId) + 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 diff --git a/server/channels/store/sqlstore/user_store.go b/server/channels/store/sqlstore/user_store.go index c915bc332f..86920f5e22 100644 --- a/server/channels/store/sqlstore/user_store.go +++ b/server/channels/store/sqlstore/user_store.go @@ -54,7 +54,7 @@ func newSqlUserStore(sqlStore *SqlStore, metrics einterfaces.MetricsInterface) s // note: we are providing field names explicitly here to maintain order of columns (needed when using raw queries) us.usersQuery = us.getQueryBuilder(). Select("u.Id", "u.CreateAt", "u.UpdateAt", "u.DeleteAt", "u.Username", "u.Password", "u.AuthData", "u.AuthService", "u.Email", "u.EmailVerified", "u.Nickname", "u.FirstName", "u.LastName", "u.Position", "u.Roles", "u.AllowMarketing", "u.Props", "u.NotifyProps", "u.LastPasswordUpdate", "u.LastPictureUpdate", "u.FailedAttempts", "u.Locale", "u.Timezone", "u.MfaActive", "u.MfaSecret", - "b.UserId IS NOT NULL AS IsBot", "COALESCE(b.Description, '') AS BotDescription", "COALESCE(b.LastIconUpdate, 0) AS BotLastIconUpdate", "u.RemoteId"). + "b.UserId IS NOT NULL AS IsBot", "COALESCE(b.Description, '') AS BotDescription", "COALESCE(b.LastIconUpdate, 0) AS BotLastIconUpdate", "u.RemoteId", "u.ExternalUserId"). From("Users u"). LeftJoin("Bots b ON ( b.UserId = u.Id )") @@ -83,12 +83,12 @@ func (us SqlUserStore) insert(user *model.User) (sql.Result, error) { (Id, CreateAt, UpdateAt, DeleteAt, Username, Password, AuthData, AuthService, Email, EmailVerified, Nickname, FirstName, LastName, Position, Roles, AllowMarketing, Props, NotifyProps, LastPasswordUpdate, LastPictureUpdate, FailedAttempts, - Locale, Timezone, MfaActive, MfaSecret, RemoteId) + Locale, Timezone, MfaActive, MfaSecret, RemoteId, ExternalUserId) VALUES (:Id, :CreateAt, :UpdateAt, :DeleteAt, :Username, :Password, :AuthData, :AuthService, :Email, :EmailVerified, :Nickname, :FirstName, :LastName, :Position, :Roles, :AllowMarketing, :Props, :NotifyProps, :LastPasswordUpdate, :LastPictureUpdate, :FailedAttempts, - :Locale, :Timezone, :MfaActive, :MfaSecret, :RemoteId)` + :Locale, :Timezone, :MfaActive, :MfaSecret, :RemoteId, :ExternalUserId)` user.Props = wrapBinaryParamStringMap(us.IsBinaryParamEnabled(), user.Props) return us.GetMasterX().NamedExec(query, user) @@ -222,7 +222,7 @@ func (us SqlUserStore) Update(user *model.User, trustedUpdateData bool) (*model. AllowMarketing=:AllowMarketing, Props=:Props, NotifyProps=:NotifyProps, LastPasswordUpdate=:LastPasswordUpdate, LastPictureUpdate=:LastPictureUpdate, FailedAttempts=:FailedAttempts,Locale=:Locale, Timezone=:Timezone, MfaActive=:MfaActive, - MfaSecret=:MfaSecret, RemoteId=:RemoteId + MfaSecret=:MfaSecret, RemoteId=:RemoteId, ExternalUserId=:ExternalUserId WHERE Id=:Id` user.Props = wrapBinaryParamStringMap(us.IsBinaryParamEnabled(), user.Props) @@ -449,7 +449,7 @@ func (us SqlUserStore) Get(ctx context.Context, id string) (*model.User, error) &user.Nickname, &user.FirstName, &user.LastName, &user.Position, &user.Roles, &user.AllowMarketing, &props, ¬ifyProps, &user.LastPasswordUpdate, &user.LastPictureUpdate, &user.FailedAttempts, &user.Locale, &timezone, &user.MfaActive, &user.MfaSecret, - &user.IsBot, &user.BotDescription, &user.BotLastIconUpdate, &user.RemoteId) + &user.IsBot, &user.BotDescription, &user.BotLastIconUpdate, &user.RemoteId, &user.ExternalUserId) if err != nil { if err == sql.ErrNoRows { return nil, store.NewErrNotFound("User", id) @@ -853,7 +853,7 @@ func (us SqlUserStore) GetAllProfilesInChannel(ctx context.Context, channelID st for rows.Next() { var user model.User var props, notifyProps, timezone []byte - if err = rows.Scan(&user.Id, &user.CreateAt, &user.UpdateAt, &user.DeleteAt, &user.Username, &user.Password, &user.AuthData, &user.AuthService, &user.Email, &user.EmailVerified, &user.Nickname, &user.FirstName, &user.LastName, &user.Position, &user.Roles, &user.AllowMarketing, &props, ¬ifyProps, &user.LastPasswordUpdate, &user.LastPictureUpdate, &user.FailedAttempts, &user.Locale, &timezone, &user.MfaActive, &user.MfaSecret, &user.IsBot, &user.BotDescription, &user.BotLastIconUpdate, &user.RemoteId); err != nil { + if err = rows.Scan(&user.Id, &user.CreateAt, &user.UpdateAt, &user.DeleteAt, &user.Username, &user.Password, &user.AuthData, &user.AuthService, &user.Email, &user.EmailVerified, &user.Nickname, &user.FirstName, &user.LastName, &user.Position, &user.Roles, &user.AllowMarketing, &props, ¬ifyProps, &user.LastPasswordUpdate, &user.LastPictureUpdate, &user.FailedAttempts, &user.Locale, &timezone, &user.MfaActive, &user.MfaSecret, &user.IsBot, &user.BotDescription, &user.BotLastIconUpdate, &user.RemoteId, &user.ExternalUserId); err != nil { return nil, errors.Wrap(err, "failed to scan values from rows into User entity") } if err = json.Unmarshal(props, &user.Props); err != nil { @@ -1174,6 +1174,26 @@ func (us SqlUserStore) GetByEmail(email string) (*model.User, error) { return &user, nil } +func (us SqlUserStore) GetByExternalUserId(externalUserId string) (*model.User, error) { + query := us.usersQuery.Where(sq.Eq{"ExternalUserId": externalUserId}) + + queryString, args, err := query.ToSql() + if err != nil { + return nil, errors.Wrap(err, "get_by_external_user_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("externaluserid=%s", externalUserId)), "failed to find User") + } + + return nil, errors.Wrapf(err, "failed to get User with ExternalUserId=%s", externalUserId) + } + + return &user, nil +} + func (us SqlUserStore) GetByAuth(authData *string, authService string) (*model.User, error) { if authData == nil || *authData == "" { return nil, store.NewErrInvalidInput("User", "", "empty or nil") @@ -1311,8 +1331,8 @@ 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}}) + if !options.IncludeExternalUsers { + query = query.Where(sq.Or{sq.Eq{"u.ExternalUserId": ""}, sq.Eq{"u.ExternalUserId": nil}}) } if options.IncludeBotAccounts { @@ -1361,12 +1381,12 @@ func (us SqlUserStore) AnalyticsActiveCount(timePeriod int64, options model.User query = query.LeftJoin("Bots ON s.UserId = Bots.UserId").Where("Bots.UserId IS NULL") } - if !options.IncludeRemoteUsers || !options.IncludeDeleted { + if !options.IncludeExternalUsers || !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.IncludeExternalUsers { + query = query.Where(sq.Or{sq.Eq{"Users.ExternalUserId": ""}, sq.Eq{"Users.ExternalUserId": nil}}) } if !options.IncludeDeleted { @@ -1394,12 +1414,12 @@ func (us SqlUserStore) AnalyticsActiveCountForPeriod(startTime int64, endTime in query = query.LeftJoin("Bots ON s.UserId = Bots.UserId").Where("Bots.UserId IS NULL") } - if !options.IncludeRemoteUsers || !options.IncludeDeleted { + if !options.IncludeExternalUsers || !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.IncludeExternalUsers { + query = query.Where(sq.Or{sq.Eq{"Users.ExternalUserId": ""}, sq.Eq{"Users.ExternalUserId": nil}}) } if !options.IncludeDeleted { diff --git a/server/channels/store/store.go b/server/channels/store/store.go index f9738bcf44..600284b939 100644 --- a/server/channels/store/store.go +++ b/server/channels/store/store.go @@ -441,6 +441,7 @@ type UserStore interface { GetProfileByGroupChannelIdsForUser(userID string, channelIds []string) (map[string][]*model.User, error) InvalidateProfileCacheForUser(userID string) GetByEmail(email string) (*model.User, error) + GetByExternalUserId(externalUserId string) (*model.User, error) GetByAuth(authData *string, authService string) (*model.User, error) GetAllUsingAuthService(authService string) ([]*model.User, error) GetAllNotInAuthService(authServices []string) ([]*model.User, error) diff --git a/server/channels/store/storetest/mocks/UserStore.go b/server/channels/store/storetest/mocks/UserStore.go index dd1e851456..9532c1acce 100644 --- a/server/channels/store/storetest/mocks/UserStore.go +++ b/server/channels/store/storetest/mocks/UserStore.go @@ -541,6 +541,32 @@ func (_m *UserStore) GetByEmail(email string) (*model.User, error) { return r0, r1 } +// GetByExternalUserId provides a mock function with given fields: externalUserId +func (_m *UserStore) GetByExternalUserId(externalUserId string) (*model.User, error) { + ret := _m.Called(externalUserId) + + var r0 *model.User + var r1 error + if rf, ok := ret.Get(0).(func(string) (*model.User, error)); ok { + return rf(externalUserId) + } + if rf, ok := ret.Get(0).(func(string) *model.User); ok { + r0 = rf(externalUserId) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.User) + } + } + + if rf, ok := ret.Get(1).(func(string) error); ok { + r1 = rf(externalUserId) + } 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) diff --git a/server/channels/store/storetest/user_store.go b/server/channels/store/storetest/user_store.go index 3ea5410eb3..4272bf88fa 100644 --- a/server/channels/store/storetest/user_store.go +++ b/server/channels/store/storetest/user_store.go @@ -3964,14 +3964,14 @@ 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, + // External User + externalId := "external-id" + externalUser, err := ss.User().Save(&model.User{ + Email: MakeEmail(), + ExternalUserId: &externalId, }) require.NoError(t, err) - defer func() { require.NoError(t, ss.User().PermanentDelete(remoteUser.Id)) }() + defer func() { require.NoError(t, ss.User().PermanentDelete(externalUser.Id)) }() // Bot botUser, err := ss.User().Save(&model.User{ @@ -4078,67 +4078,67 @@ func testCount(t *testing.T, ss store.Store) { 0, }, { - "Include remote accounts no deleted accounts and no team id", + "Include external accounts no deleted accounts and no team id", model.UserCountOptions{ - IncludeRemoteUsers: true, - IncludeDeleted: false, - TeamId: "", + IncludeExternalUsers: true, + IncludeDeleted: false, + TeamId: "", }, 5, }, { - "Include delete accounts no remote accounts and no team id", + "Include delete accounts no external accounts and no team id", model.UserCountOptions{ - IncludeRemoteUsers: false, - IncludeDeleted: true, - TeamId: "", + IncludeExternalUsers: false, + IncludeDeleted: true, + TeamId: "", }, 5, }, { - "Include remote accounts and deleted accounts and no team id", + "Include external accounts and deleted accounts and no team id", model.UserCountOptions{ - IncludeRemoteUsers: true, - IncludeDeleted: true, - TeamId: "", + IncludeExternalUsers: true, + IncludeDeleted: true, + TeamId: "", }, 6, }, { - "Include remote accounts and deleted accounts with existing team id", + "Include external accounts and deleted accounts with existing team id", model.UserCountOptions{ - IncludeRemoteUsers: true, - IncludeDeleted: true, - TeamId: teamId, + IncludeExternalUsers: true, + IncludeDeleted: true, + TeamId: teamId, }, 4, }, { - "Include remote accounts and deleted accounts with fake team id", + "Include external accounts and deleted accounts with fake team id", model.UserCountOptions{ - IncludeRemoteUsers: true, - IncludeDeleted: true, - TeamId: model.NewId(), + IncludeExternalUsers: true, + IncludeDeleted: true, + TeamId: model.NewId(), }, 0, }, { - "Include remote accounts and deleted accounts with existing team id and view restrictions allowing team", + "Include external 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}}, + IncludeExternalUsers: 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", + "Include external 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()}}, + IncludeExternalUsers: true, + IncludeDeleted: true, + TeamId: teamId, + ViewRestrictions: &model.ViewUsersRestrictions{Teams: []string{model.NewId()}}, }, 0, }, diff --git a/server/channels/store/timerlayer/timerlayer.go b/server/channels/store/timerlayer/timerlayer.go index 815b215305..b1ba96fc1b 100644 --- a/server/channels/store/timerlayer/timerlayer.go +++ b/server/channels/store/timerlayer/timerlayer.go @@ -10086,6 +10086,22 @@ func (s *TimerLayerUserStore) GetByEmail(email string) (*model.User, error) { return result, err } +func (s *TimerLayerUserStore) GetByExternalUserId(externalUserId string) (*model.User, error) { + start := time.Now() + + result, err := s.UserStore.GetByExternalUserId(externalUserId) + + 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.GetByExternalUserId", success, elapsed) + } + return result, err +} + func (s *TimerLayerUserStore) GetByUsername(username string) (*model.User, error) { start := time.Now() diff --git a/server/model/user.go b/server/model/user.go index 5fbbd47d6f..e0d36019da 100644 --- a/server/model/user.go +++ b/server/model/user.go @@ -96,6 +96,7 @@ type User struct { MfaActive bool `json:"mfa_active,omitempty"` MfaSecret string `json:"mfa_secret,omitempty"` RemoteId *string `json:"remote_id,omitempty"` + ExternalUserId *string `json:"external_user_id,omitempty"` LastActivityAt int64 `json:"last_activity_at,omitempty"` IsBot bool `json:"is_bot,omitempty"` BotDescription string `json:"bot_description,omitempty"` @@ -127,6 +128,7 @@ func (u *User) Auditable() map[string]interface{} { "timezone": u.Timezone, "mfa_active": u.MfaActive, "remote_id": u.RemoteId, + "external_user_id": u.ExternalUserId, "last_activity_at": u.LastActivityAt, "is_bot": u.IsBot, "bot_description": u.BotDescription, @@ -329,7 +331,7 @@ func (u *User) IsValid() *AppError { return InvalidUserError("update_at", u.Id, u.UpdateAt) } - if u.IsRemote() { + if u.IsRemote() || u.IsExternal() { if !IsValidUsernameAllowRemote(u.Username) { return InvalidUserError("username", u.Id, u.Username) } @@ -877,6 +879,11 @@ func (u *User) IsRemote() bool { return u.RemoteId != nil && *u.RemoteId != "" } +// IsExternal returns true if the user is a synthetic external user (has ExternalUserId). +func (u *User) IsExternal() bool { + return u.ExternalUserId != nil && *u.ExternalUserId != "" +} + // GetRemoteID returns the remote id for this user or "" if not a remote user. func (u *User) GetRemoteID() string { if u.RemoteId != nil { diff --git a/server/model/user_count.go b/server/model/user_count.go index 950a670044..8df96b9813 100644 --- a/server/model/user_count.go +++ b/server/model/user_count.go @@ -9,8 +9,8 @@ type UserCountOptions struct { IncludeBotAccounts bool // Should include deleted users (of any type) IncludeDeleted bool - // Include remote users - IncludeRemoteUsers bool + // Include external users + IncludeExternalUsers bool // Exclude regular users ExcludeRegularUsers bool // Only include users on a specific team. "" for any team. diff --git a/server/plugin/api.go b/server/plugin/api.go index 63be22ce43..e61270c748 100644 --- a/server/plugin/api.go +++ b/server/plugin/api.go @@ -147,6 +147,12 @@ type API interface { // Minimum server version: 5.2 GetUserByEmail(email string) (*model.User, *model.AppError) + // GetUserByExternalUserId gets a user by their external user id. + // + // @tag User + // Minimum server version: 7.12 + GetUserByExternalUserId(externalUserId string) (*model.User, *model.AppError) + // GetUserByUsername gets a user by their username. // // @tag User diff --git a/server/plugin/api_timer_layer_generated.go b/server/plugin/api_timer_layer_generated.go index 0f1ab22ff7..d04b58a10b 100644 --- a/server/plugin/api_timer_layer_generated.go +++ b/server/plugin/api_timer_layer_generated.go @@ -175,6 +175,13 @@ func (api *apiTimerLayer) GetUserByEmail(email string) (*model.User, *model.AppE return _returnsA, _returnsB } +func (api *apiTimerLayer) GetUserByExternalUserId(externalUserId string) (*model.User, *model.AppError) { + startTime := timePkg.Now() + _returnsA, _returnsB := api.apiImpl.GetUserByExternalUserId(externalUserId) + api.recordTime(startTime, "GetUserByExternalUserId", _returnsB == nil) + return _returnsA, _returnsB +} + func (api *apiTimerLayer) GetUserByUsername(name string) (*model.User, *model.AppError) { startTime := timePkg.Now() _returnsA, _returnsB := api.apiImpl.GetUserByUsername(name) diff --git a/server/plugin/client_rpc_generated.go b/server/plugin/client_rpc_generated.go index 3453a35578..add6d5133a 100644 --- a/server/plugin/client_rpc_generated.go +++ b/server/plugin/client_rpc_generated.go @@ -1599,6 +1599,35 @@ func (s *apiRPCServer) GetUserByEmail(args *Z_GetUserByEmailArgs, returns *Z_Get return nil } +type Z_GetUserByExternalUserIdArgs struct { + A string +} + +type Z_GetUserByExternalUserIdReturns struct { + A *model.User + B *model.AppError +} + +func (g *apiRPCClient) GetUserByExternalUserId(externalUserId string) (*model.User, *model.AppError) { + _args := &Z_GetUserByExternalUserIdArgs{externalUserId} + _returns := &Z_GetUserByExternalUserIdReturns{} + if err := g.client.Call("Plugin.GetUserByExternalUserId", _args, _returns); err != nil { + log.Printf("RPC call to GetUserByExternalUserId API failed: %s", err.Error()) + } + return _returns.A, _returns.B +} + +func (s *apiRPCServer) GetUserByExternalUserId(args *Z_GetUserByExternalUserIdArgs, returns *Z_GetUserByExternalUserIdReturns) error { + if hook, ok := s.impl.(interface { + GetUserByExternalUserId(externalUserId string) (*model.User, *model.AppError) + }); ok { + returns.A, returns.B = hook.GetUserByExternalUserId(args.A) + } else { + return encodableError(fmt.Errorf("API GetUserByExternalUserId called but not implemented.")) + } + return nil +} + type Z_GetUserByUsernameArgs struct { A string } diff --git a/server/plugin/plugintest/api.go b/server/plugin/plugintest/api.go index 9fa1420457..476961f075 100644 --- a/server/plugin/plugintest/api.go +++ b/server/plugin/plugintest/api.go @@ -2514,6 +2514,34 @@ func (_m *API) GetUserByEmail(email string) (*model.User, *model.AppError) { return r0, r1 } +// GetUserByExternalUserId provides a mock function with given fields: externalUserId +func (_m *API) GetUserByExternalUserId(externalUserId string) (*model.User, *model.AppError) { + ret := _m.Called(externalUserId) + + var r0 *model.User + var r1 *model.AppError + if rf, ok := ret.Get(0).(func(string) (*model.User, *model.AppError)); ok { + return rf(externalUserId) + } + if rf, ok := ret.Get(0).(func(string) *model.User); ok { + r0 = rf(externalUserId) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.User) + } + } + + if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + r1 = rf(externalUserId) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 +} + // GetUserByUsername provides a mock function with given fields: name func (_m *API) GetUserByUsername(name string) (*model.User, *model.AppError) { ret := _m.Called(name) diff --git a/webapp/channels/src/components/user_list_row_with_error/user_list_row_with_error.tsx b/webapp/channels/src/components/user_list_row_with_error/user_list_row_with_error.tsx index 90c4cb3655..76815c52bd 100644 --- a/webapp/channels/src/components/user_list_row_with_error/user_list_row_with_error.tsx +++ b/webapp/channels/src/components/user_list_row_with_error/user_list_row_with_error.tsx @@ -156,12 +156,12 @@ export default class UserListRowWithError extends React.PureComponent {this.props.user.is_bot && } - {this.props.user.remote_id && ( + {this.props.user.external_user_id && ( } /> diff --git a/webapp/platform/types/src/users.ts b/webapp/platform/types/src/users.ts index 842424cb89..a71aefe5db 100644 --- a/webapp/platform/types/src/users.ts +++ b/webapp/platform/types/src/users.ts @@ -54,6 +54,7 @@ export type UserProfile = { terms_of_service_id: string; terms_of_service_create_at: number; remote_id?: string; + external_user_id?: string; status?: string; }; @@ -131,7 +132,7 @@ export type GetFilteredUsersStatsOpts = { in_channel?: string; include_deleted?: boolean; include_bots?: boolean; - include_remote_users?: boolean; + include_external_users?: boolean; roles?: string[]; channel_roles?: string[]; team_roles?: string[];