diff --git a/server/Makefile b/server/Makefile index c1b65eee32..3049ca7bf6 100644 --- a/server/Makefile +++ b/server/Makefile @@ -755,7 +755,7 @@ vet: ## Run mattermost go vet specific checks ## Note that it is pinned to a specific commit, rather than a branch. This is to prevent ## having to backport the fix to multiple release branches for any new change. $(GO) install github.com/mattermost/mattermost-govet/v2@72e7752bcf59b57688fcb41173a6a1583c405adf - $(GO) vet -vettool=$(GOBIN)/mattermost-govet -structuredLogging -inconsistentReceiverName -emptyStrCmp -tFatal -configtelemetry -errorAssertions -license -inconsistentReceiverName.ignore=session_serial_gen.go,team_member_serial_gen.go,user_serial_gen.go ./... + $(GO) vet -vettool=$(GOBIN)/mattermost-govet -structuredLogging -inconsistentReceiverName -emptyStrCmp -tFatal -configtelemetry -errorAssertions -license -inconsistentReceiverName.ignore=session_serial_gen.go,team_member_serial_gen.go,user_serial_gen.go,utils_serial_gen.go ./... ifeq ($(BUILD_ENTERPRISE_READY),true) ifneq ($(MM_NO_ENTERPRISE_LINT),true) $(GO) vet -vettool=$(GOBIN)/mattermost-govet -structuredLogging -inconsistentReceiverName -emptyStrCmp -tFatal -configtelemetry -errorAssertions -enterpriseLicense $(BUILD_ENTERPRISE_DIR)/... @@ -780,6 +780,10 @@ gen-serialized: ## Generates serialization methods for hot structs # would be to temporarily move all the structs to the same file, # but that involves a lot of manual work. $(GO) install github.com/tinylib/msgp@v1.1.6 + $(GOBIN)/msgp -file=./public/model/utils.go -tests=false -o=./public/model/utils_serial_gen.go + @echo "$$LICENSE_HEADER" > tmp.go + @cat ./public/model/utils_serial_gen.go >> tmp.go + @mv tmp.go ./public/model/utils_serial_gen.go $(GOBIN)/msgp -file=./public/model/session.go -tests=false -o=./public/model/session_serial_gen.go @echo "$$LICENSE_HEADER" > tmp.go @cat ./public/model/session_serial_gen.go >> tmp.go diff --git a/server/channels/app/authentication.go b/server/channels/app/authentication.go index 86d7a0bd4f..d113e25fde 100644 --- a/server/channels/app/authentication.go +++ b/server/channels/app/authentication.go @@ -212,7 +212,7 @@ func (a *App) CheckUserMfa(rctx request.CTX, user *model.User, token string) *mo return model.NewAppError("CheckUserMfa", "mfa.mfa_disabled.app_error", nil, "", http.StatusNotImplemented) } - ok, err := mfa.New(a.Srv().Store().User()).ValidateToken(user.MfaSecret, token) + ok, err := mfa.New(a.Srv().Store().User()).ValidateToken(user, token) if err != nil { return model.NewAppError("CheckUserMfa", "mfa.validate_token.authenticate.app_error", nil, "", http.StatusBadRequest).Wrap(err) } diff --git a/server/channels/db/migrations/migrations.list b/server/channels/db/migrations/migrations.list index d17b261f32..9c1ebdd5b0 100644 --- a/server/channels/db/migrations/migrations.list +++ b/server/channels/db/migrations/migrations.list @@ -249,6 +249,8 @@ channels/db/migrations/mysql/000125_remoteclusters_add_default_team_id.down.sql channels/db/migrations/mysql/000125_remoteclusters_add_default_team_id.up.sql channels/db/migrations/mysql/000126_sharedchannels_remotes_add_deleteat.down.sql channels/db/migrations/mysql/000126_sharedchannels_remotes_add_deleteat.up.sql +channels/db/migrations/mysql/000127_add_mfa_used_ts_to_users.down.sql +channels/db/migrations/mysql/000127_add_mfa_used_ts_to_users.up.sql channels/db/migrations/postgres/000001_create_teams.down.sql channels/db/migrations/postgres/000001_create_teams.up.sql channels/db/migrations/postgres/000002_create_team_members.down.sql @@ -499,3 +501,5 @@ channels/db/migrations/postgres/000125_remoteclusters_add_default_team_id.down.s channels/db/migrations/postgres/000125_remoteclusters_add_default_team_id.up.sql channels/db/migrations/postgres/000126_sharedchannels_remotes_add_deleteat.down.sql channels/db/migrations/postgres/000126_sharedchannels_remotes_add_deleteat.up.sql +channels/db/migrations/postgres/000127_add_mfa_used_ts_to_users.down.sql +channels/db/migrations/postgres/000127_add_mfa_used_ts_to_users.up.sql diff --git a/server/channels/db/migrations/mysql/000127_add_mfa_used_ts_to_users.down.sql b/server/channels/db/migrations/mysql/000127_add_mfa_used_ts_to_users.down.sql new file mode 100644 index 0000000000..2a339e7852 --- /dev/null +++ b/server/channels/db/migrations/mysql/000127_add_mfa_used_ts_to_users.down.sql @@ -0,0 +1 @@ +ALTER TABLE Users DROP COLUMN MfaUsedTimestamps; diff --git a/server/channels/db/migrations/mysql/000127_add_mfa_used_ts_to_users.up.sql b/server/channels/db/migrations/mysql/000127_add_mfa_used_ts_to_users.up.sql new file mode 100644 index 0000000000..8c02b4fdf4 --- /dev/null +++ b/server/channels/db/migrations/mysql/000127_add_mfa_used_ts_to_users.up.sql @@ -0,0 +1 @@ +ALTER TABLE Users ADD COLUMN MfaUsedTimestamps json NULL; diff --git a/server/channels/db/migrations/postgres/000127_add_mfa_used_ts_to_users.down.sql b/server/channels/db/migrations/postgres/000127_add_mfa_used_ts_to_users.down.sql new file mode 100644 index 0000000000..57429e5f70 --- /dev/null +++ b/server/channels/db/migrations/postgres/000127_add_mfa_used_ts_to_users.down.sql @@ -0,0 +1,2 @@ + +ALTER TABLE Users DROP COLUMN IF EXISTS MfaUsedTimestamps; diff --git a/server/channels/db/migrations/postgres/000127_add_mfa_used_ts_to_users.up.sql b/server/channels/db/migrations/postgres/000127_add_mfa_used_ts_to_users.up.sql new file mode 100644 index 0000000000..06f7c2911d --- /dev/null +++ b/server/channels/db/migrations/postgres/000127_add_mfa_used_ts_to_users.up.sql @@ -0,0 +1 @@ +ALTER TABLE Users ADD COLUMN IF NOT EXISTS MfaUsedTimestamps jsonb NULL; diff --git a/server/channels/store/opentracinglayer/opentracinglayer.go b/server/channels/store/opentracinglayer/opentracinglayer.go index eca1a651f7..26d196f101 100644 --- a/server/channels/store/opentracinglayer/opentracinglayer.go +++ b/server/channels/store/opentracinglayer/opentracinglayer.go @@ -11843,6 +11843,24 @@ func (s *OpenTracingLayerUserStore) GetMany(ctx context.Context, ids []string) ( return result, err } +func (s *OpenTracingLayerUserStore) GetMfaUsedTimestamps(userID string) ([]int, error) { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserStore.GetMfaUsedTimestamps") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + result, err := s.UserStore.GetMfaUsedTimestamps(userID) + if err != nil { + span.LogFields(spanlog.Error(err)) + ext.Error.Set(span, true) + } + + return result, err +} + func (s *OpenTracingLayerUserStore) GetNewUsersForTeam(teamID string, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserStore.GetNewUsersForTeam") @@ -12530,6 +12548,24 @@ func (s *OpenTracingLayerUserStore) SearchWithoutTeam(term string, options *mode return result, err } +func (s *OpenTracingLayerUserStore) StoreMfaUsedTimestamps(userID string, ts []int) error { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserStore.StoreMfaUsedTimestamps") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + err := s.UserStore.StoreMfaUsedTimestamps(userID, ts) + if err != nil { + span.LogFields(spanlog.Error(err)) + ext.Error.Set(span, true) + } + + return err +} + func (s *OpenTracingLayerUserStore) Update(rctx request.CTX, user *model.User, allowRoleUpdate bool) (*model.UserUpdate, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserStore.Update") diff --git a/server/channels/store/retrylayer/retrylayer.go b/server/channels/store/retrylayer/retrylayer.go index 55c04111d6..7419ac72c0 100644 --- a/server/channels/store/retrylayer/retrylayer.go +++ b/server/channels/store/retrylayer/retrylayer.go @@ -13529,6 +13529,27 @@ func (s *RetryLayerUserStore) GetMany(ctx context.Context, ids []string) ([]*mod } +func (s *RetryLayerUserStore) GetMfaUsedTimestamps(userID string) ([]int, error) { + + tries := 0 + for { + result, err := s.UserStore.GetMfaUsedTimestamps(userID) + 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) GetNewUsersForTeam(teamID string, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) { tries := 0 @@ -14303,6 +14324,27 @@ func (s *RetryLayerUserStore) SearchWithoutTeam(term string, options *model.User } +func (s *RetryLayerUserStore) StoreMfaUsedTimestamps(userID string, ts []int) error { + + tries := 0 + for { + err := s.UserStore.StoreMfaUsedTimestamps(userID, ts) + if err == nil { + return nil + } + if !isRepeatableError(err) { + return err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + func (s *RetryLayerUserStore) Update(rctx request.CTX, user *model.User, allowRoleUpdate bool) (*model.UserUpdate, error) { tries := 0 diff --git a/server/channels/store/sqlstore/user_store.go b/server/channels/store/sqlstore/user_store.go index eea7b64997..98a9e2970e 100644 --- a/server/channels/store/sqlstore/user_store.go +++ b/server/channels/store/sqlstore/user_store.go @@ -9,6 +9,7 @@ import ( "encoding/json" "fmt" "sort" + "strconv" "strings" "time" "unicode/utf8" @@ -55,7 +56,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", + 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", "u.MfaUsedTimestamps", "b.UserId IS NOT NULL AS IsBot", "COALESCE(b.Description, '') AS BotDescription", "COALESCE(b.LastIconUpdate, 0) AS BotLastIconUpdate", "u.RemoteId", "u.LastLogin"). From("Users u"). LeftJoin("Bots b ON ( b.UserId = u.Id )") @@ -85,12 +86,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, MfaUsedTimestamps) 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, :MfaUsedTimestamps)` user.Props = wrapBinaryParamStringMap(us.IsBinaryParamEnabled(), user.Props) return us.GetMasterX().NamedExec(query, user) @@ -197,6 +198,7 @@ func (us SqlUserStore) Update(rctx request.CTX, user *model.User, trustedUpdateD user.FailedAttempts = oldUser.FailedAttempts user.MfaSecret = oldUser.MfaSecret user.MfaActive = oldUser.MfaActive + user.MfaUsedTimestamps = oldUser.MfaUsedTimestamps user.LastLogin = oldUser.LastLogin if !trustedUpdateData { @@ -227,7 +229,7 @@ func (us SqlUserStore) Update(rctx request.CTX, user *model.User, trustedUpdateD AllowMarketing=:AllowMarketing, Props=:Props, NotifyProps=:NotifyProps, LastPasswordUpdate=:LastPasswordUpdate, LastPictureUpdate=:LastPictureUpdate, FailedAttempts=:FailedAttempts,Locale=:Locale, Timezone=:Timezone, MfaActive=:MfaActive, - MfaSecret=:MfaSecret, RemoteId=:RemoteId, LastLogin=:LastLogin + MfaSecret=:MfaSecret, RemoteId=:RemoteId, LastLogin=:LastLogin, MfaUsedTimestamps=:MfaUsedTimestamps WHERE Id=:Id` user.Props = wrapBinaryParamStringMap(us.IsBinaryParamEnabled(), user.Props) @@ -343,7 +345,8 @@ func (us SqlUserStore) UpdateAuthData(userId string, service string, authData *s if resetMfa { updateQuery = updateQuery.Set("MfaActive", false). - Set("MfaSecret", "") + Set("MfaSecret", ""). + Set("MfaUsedTimestamps", model.StringArray{}) } queryString, args, err := updateQuery.ToSql() @@ -427,7 +430,7 @@ func (us SqlUserStore) ResetAuthDataToEmailForUsers(service string, userIDs []st func (us SqlUserStore) UpdateMfaSecret(userId, secret string) error { updateAt := model.GetMillis() - if _, err := us.GetMasterX().Exec("UPDATE Users SET MfaSecret = ?, UpdateAt = ? WHERE Id = ?", secret, updateAt, userId); err != nil { + if _, err := us.GetMasterX().Exec("UPDATE Users SET MfaSecret = ?, MfaUsedTimestamps = ?, UpdateAt = ? WHERE Id = ?", secret, model.StringArray{}, updateAt, userId); err != nil { return errors.Wrapf(err, "failed to update User with userId=%s", userId) } @@ -444,6 +447,37 @@ func (us SqlUserStore) UpdateMfaActive(userId string, active bool) error { return nil } +func (us SqlUserStore) StoreMfaUsedTimestamps(userId string, ts []int) error { + tSStrArray := model.StringArray{} + for _, t := range ts { + tSStrArray = append(tSStrArray, fmt.Sprintf("%d", t)) + } + + updateAt := model.GetMillis() + if _, err := us.GetMasterX().Exec("UPDATE Users SET MfaUsedTimestamps = ?, UpdateAt = ? WHERE Id = ?", tSStrArray, updateAt, userId); err != nil { + return errors.Wrapf(err, "failed to update User with userId=%s", userId) + } + return nil +} + +func (us SqlUserStore) GetMfaUsedTimestamps(userId string) ([]int, error) { + tsStrArray := model.StringArray{} + err := us.GetReplicaX().Get(&tsStrArray, "SELECT MfaUsedTimestamps FROM Users WHERE Id = ?", userId) + if err != nil { + return nil, errors.Wrapf(err, "failed to get MFA used timestamps for user with ID %s", userId) + } + + ts := make([]int, len(tsStrArray)) + for i, t := range tsStrArray { + ts[i], err = strconv.Atoi(t) + if err != nil { + return nil, errors.Wrapf(err, "failed to parse MFA used timestamp %s for user with ID %s", t, userId) + } + } + + return ts, nil +} + // GetMany returns a list of users for the provided list of ids func (us SqlUserStore) GetMany(ctx context.Context, ids []string) ([]*model.User, error) { query := us.usersQuery.Where(sq.Eq{"Id": ids}) @@ -474,7 +508,7 @@ func (us SqlUserStore) Get(ctx context.Context, id string) (*model.User, error) &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.FailedAttempts, &user.Locale, &timezone, &user.MfaActive, &user.MfaSecret, &user.MfaUsedTimestamps, &user.IsBot, &user.BotDescription, &user.BotLastIconUpdate, &user.RemoteId, &user.LastLogin) if err != nil { if err == sql.ErrNoRows { @@ -877,7 +911,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, &user.LastLogin); 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.MfaUsedTimestamps, &user.IsBot, &user.BotDescription, &user.BotLastIconUpdate, &user.RemoteId, &user.LastLogin); 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 { diff --git a/server/channels/store/store.go b/server/channels/store/store.go index 21f11d7da5..2c33e7b316 100644 --- a/server/channels/store/store.go +++ b/server/channels/store/store.go @@ -417,6 +417,8 @@ type UserStore interface { ResetAuthDataToEmailForUsers(service string, userIDs []string, includeDeleted bool, dryRun bool) (int, error) UpdateMfaSecret(userID, secret string) error UpdateMfaActive(userID string, active bool) error + StoreMfaUsedTimestamps(userID string, ts []int) error + GetMfaUsedTimestamps(userID string) ([]int, error) Get(ctx context.Context, id string) (*model.User, error) GetMany(ctx context.Context, ids []string) ([]*model.User, error) GetAll() ([]*model.User, error) diff --git a/server/channels/store/storetest/mocks/UserStore.go b/server/channels/store/storetest/mocks/UserStore.go index 4d9c43d41b..8bf8cbcfc3 100644 --- a/server/channels/store/storetest/mocks/UserStore.go +++ b/server/channels/store/storetest/mocks/UserStore.go @@ -861,6 +861,36 @@ func (_m *UserStore) GetMany(ctx context.Context, ids []string) ([]*model.User, return r0, r1 } +// GetMfaUsedTimestamps provides a mock function with given fields: userID +func (_m *UserStore) GetMfaUsedTimestamps(userID string) ([]int, error) { + ret := _m.Called(userID) + + if len(ret) == 0 { + panic("no return value specified for GetMfaUsedTimestamps") + } + + var r0 []int + var r1 error + if rf, ok := ret.Get(0).(func(string) ([]int, error)); ok { + return rf(userID) + } + if rf, ok := ret.Get(0).(func(string) []int); ok { + r0 = rf(userID) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]int) + } + } + + if rf, ok := ret.Get(1).(func(string) error); ok { + r1 = rf(userID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetNewUsersForTeam provides a mock function with given fields: teamID, offset, limit, viewRestrictions func (_m *UserStore) GetNewUsersForTeam(teamID string, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) { ret := _m.Called(teamID, offset, limit, viewRestrictions) @@ -1884,6 +1914,24 @@ func (_m *UserStore) SearchWithoutTeam(term string, options *model.UserSearchOpt return r0, r1 } +// StoreMfaUsedTimestamps provides a mock function with given fields: userID, ts +func (_m *UserStore) StoreMfaUsedTimestamps(userID string, ts []int) error { + ret := _m.Called(userID, ts) + + if len(ret) == 0 { + panic("no return value specified for StoreMfaUsedTimestamps") + } + + var r0 error + if rf, ok := ret.Get(0).(func(string, []int) error); ok { + r0 = rf(userID, ts) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // Update provides a mock function with given fields: rctx, user, allowRoleUpdate func (_m *UserStore) Update(rctx request.CTX, user *model.User, allowRoleUpdate bool) (*model.UserUpdate, error) { ret := _m.Called(rctx, user, allowRoleUpdate) diff --git a/server/channels/store/storetest/user_store.go b/server/channels/store/storetest/user_store.go index 65013b41ee..f0b4b95294 100644 --- a/server/channels/store/storetest/user_store.go +++ b/server/channels/store/storetest/user_store.go @@ -101,6 +101,7 @@ func TestUserStore(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) { t.Run("GetUsersWithInvalidEmails", func(t *testing.T) { testGetUsersWithInvalidEmails(t, rctx, ss) }) t.Run("UpdateLastLogin", func(t *testing.T) { testUpdateLastLogin(t, rctx, ss) }) t.Run("GetUserReport", func(t *testing.T) { testGetUserReport(t, rctx, ss, s) }) + t.Run("MfaUsedTimestamps", func(t *testing.T) { testMfaUsedTimestamps(t, rctx, ss) }) } func testUserStoreSave(t *testing.T, rctx request.CTX, ss store.Store) { @@ -6613,3 +6614,24 @@ func testGetUserReport(t *testing.T, rctx request.CTX, ss store.Store, s SqlStor require.Len(t, userReport, 11) }) } + +func testMfaUsedTimestamps(t *testing.T, rctx request.CTX, ss store.Store) { + u1, err := ss.User().Save(rctx, &model.User{ + Email: "ben@invalid.mattermost.com", + Username: "u1" + model.NewId(), + }) + + require.NoError(t, err) + defer func() { require.NoError(t, ss.User().PermanentDelete(rctx, u1.Id)) }() + + tss, err := ss.User().GetMfaUsedTimestamps(u1.Id) + require.NoError(t, err) + require.Empty(t, tss) + + err = ss.User().StoreMfaUsedTimestamps(u1.Id, []int{1, 2, 3}) + require.NoError(t, err) + + tss, err = ss.User().GetMfaUsedTimestamps(u1.Id) + require.NoError(t, err) + require.Equal(t, []int{1, 2, 3}, tss) +} diff --git a/server/channels/store/timerlayer/timerlayer.go b/server/channels/store/timerlayer/timerlayer.go index fc79c0b5a0..48854c712a 100644 --- a/server/channels/store/timerlayer/timerlayer.go +++ b/server/channels/store/timerlayer/timerlayer.go @@ -10661,6 +10661,22 @@ func (s *TimerLayerUserStore) GetMany(ctx context.Context, ids []string) ([]*mod return result, err } +func (s *TimerLayerUserStore) GetMfaUsedTimestamps(userID string) ([]int, error) { + start := time.Now() + + result, err := s.UserStore.GetMfaUsedTimestamps(userID) + + 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.GetMfaUsedTimestamps", success, elapsed) + } + return result, err +} + func (s *TimerLayerUserStore) GetNewUsersForTeam(teamID string, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) { start := time.Now() @@ -11282,6 +11298,22 @@ func (s *TimerLayerUserStore) SearchWithoutTeam(term string, options *model.User return result, err } +func (s *TimerLayerUserStore) StoreMfaUsedTimestamps(userID string, ts []int) error { + start := time.Now() + + err := s.UserStore.StoreMfaUsedTimestamps(userID, ts) + + 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.StoreMfaUsedTimestamps", success, elapsed) + } + return err +} + func (s *TimerLayerUserStore) Update(rctx request.CTX, user *model.User, allowRoleUpdate bool) (*model.UserUpdate, error) { start := time.Now() diff --git a/server/platform/shared/mfa/mfa.go b/server/platform/shared/mfa/mfa.go index a5c73dc296..7f138b47f4 100644 --- a/server/platform/shared/mfa/mfa.go +++ b/server/platform/shared/mfa/mfa.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/dgryski/dgoogauth" + "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/rsc/qr" "github.com/pkg/errors" ) @@ -26,6 +27,8 @@ const ( type Store interface { UpdateMfaActive(userId string, active bool) error UpdateMfaSecret(userId, secret string) error + StoreMfaUsedTimestamps(userId string, ts []int) error + GetMfaUsedTimestamps(userId string) ([]int, error) } type MFA struct { @@ -120,11 +123,17 @@ func (m *MFA) Deactivate(userId string) error { } // Validate the provide token using the secret provided -func (m *MFA) ValidateToken(secret, token string) (bool, error) { +func (m *MFA) ValidateToken(user *model.User, token string) (bool, error) { + usedTs, err := m.store.GetMfaUsedTimestamps(user.Id) + if err != nil { + return false, errors.Wrap(err, "unable to retrieve the DisallowReuse slice") + } + otpConfig := &dgoogauth.OTPConfig{ - Secret: secret, - WindowSize: 3, - HotpCounter: 0, + Secret: user.MfaSecret, + WindowSize: 3, + HotpCounter: 0, + DisallowReuse: usedTs, } trimmedToken := strings.TrimSpace(token) @@ -132,6 +141,14 @@ func (m *MFA) ValidateToken(secret, token string) (bool, error) { if err != nil { return false, errors.Wrap(err, "unable to parse the token") } + if !ok { + return false, nil + } - return ok, nil + err = m.store.StoreMfaUsedTimestamps(user.Id, otpConfig.DisallowReuse) + if err != nil { + return true, errors.Wrap(err, "unable to store the DisallowReuse slice") + } + + return true, nil } diff --git a/server/platform/shared/mfa/mfa_test.go b/server/platform/shared/mfa/mfa_test.go index b771d79f22..49c9386daf 100644 --- a/server/platform/shared/mfa/mfa_test.go +++ b/server/platform/shared/mfa/mfa_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/public/plugin/plugintest/mock" "github.com/mattermost/mattermost/server/v8/channels/store/storetest/mocks" ) @@ -156,12 +157,49 @@ func TestDeactivate(t *testing.T) { func TestValidateToken(t *testing.T) { t.Run("fail on wrongly formatted token", func(t *testing.T) { + id := model.NewId() secret := newRandomBase32String(mfaSecretSize) - ok, err := New(nil).ValidateToken(secret, "invalid-token") + u := &model.User{Id: id, MfaSecret: secret} + + usMock := mocks.UserStore{} + usMock.On("GetMfaUsedTimestamps", u.Id).Return([]int{}, nil).Once() + ok, err := New(&usMock).ValidateToken(u, "invalid-token") require.Error(t, err) require.False(t, ok) require.Contains(t, err.Error(), "unable to parse the token") }) + + t.Run("successful validation", func(t *testing.T) { + id := model.NewId() + secret := newRandomBase32String(mfaSecretSize) + u := &model.User{Id: id, MfaSecret: secret} + + code := fmt.Sprintf("%06d", dgoogauth.ComputeCode(secret, time.Now().UTC().Unix()/30)) + + usMock := mocks.UserStore{} + usMock.On("GetMfaUsedTimestamps", u.Id).Return([]int{}, nil).Once() + usMock.On("StoreMfaUsedTimestamps", u.Id, mock.AnythingOfType("[]int")).Return(nil).Once() + + ok, err := New(&usMock).ValidateToken(u, code) + require.NoError(t, err) + require.True(t, ok) + }) + + t.Run("disallow reuse of totp", func(t *testing.T) { + id := model.NewId() + secret := newRandomBase32String(mfaSecretSize) + u := &model.User{Id: id, MfaSecret: secret} + + t0 := time.Now().UTC().Unix() / 30 + code := fmt.Sprintf("%06d", dgoogauth.ComputeCode(secret, t0)) + + usMock := mocks.UserStore{} + usMock.On("GetMfaUsedTimestamps", u.Id).Return([]int{int(t0)}, nil).Once() + + ok, err := New(&usMock).ValidateToken(u, code) + require.False(t, ok) + require.NoError(t, err) + }) } func TestRandomBase32String(t *testing.T) { diff --git a/server/public/model/user.go b/server/public/model/user.go index a461c32246..b01e667c5d 100644 --- a/server/public/model/user.go +++ b/server/public/model/user.go @@ -75,40 +75,41 @@ const ( // This struct's serializer methods are auto-generated. If a new field is added/removed, // please run make gen-serialized. type User struct { - Id string `json:"id"` - CreateAt int64 `json:"create_at,omitempty"` - UpdateAt int64 `json:"update_at,omitempty"` - DeleteAt int64 `json:"delete_at"` - Username string `json:"username"` - Password string `json:"password,omitempty"` - AuthData *string `json:"auth_data,omitempty"` - AuthService string `json:"auth_service"` - Email string `json:"email"` - EmailVerified bool `json:"email_verified,omitempty"` - Nickname string `json:"nickname"` - FirstName string `json:"first_name"` - LastName string `json:"last_name"` - Position string `json:"position"` - Roles string `json:"roles"` - AllowMarketing bool `json:"allow_marketing,omitempty"` - Props StringMap `json:"props,omitempty"` - NotifyProps StringMap `json:"notify_props,omitempty"` - LastPasswordUpdate int64 `json:"last_password_update,omitempty"` - LastPictureUpdate int64 `json:"last_picture_update,omitempty"` - FailedAttempts int `json:"failed_attempts,omitempty"` - Locale string `json:"locale"` - Timezone StringMap `json:"timezone"` - MfaActive bool `json:"mfa_active,omitempty"` - MfaSecret string `json:"mfa_secret,omitempty"` - RemoteId *string `json:"remote_id,omitempty"` - LastActivityAt int64 `json:"last_activity_at,omitempty"` - IsBot bool `json:"is_bot,omitempty"` - BotDescription string `json:"bot_description,omitempty"` - BotLastIconUpdate int64 `json:"bot_last_icon_update,omitempty"` - TermsOfServiceId string `json:"terms_of_service_id,omitempty"` - TermsOfServiceCreateAt int64 `json:"terms_of_service_create_at,omitempty"` - DisableWelcomeEmail bool `json:"disable_welcome_email"` - LastLogin int64 `json:"last_login,omitempty"` + Id string `json:"id"` + CreateAt int64 `json:"create_at,omitempty"` + UpdateAt int64 `json:"update_at,omitempty"` + DeleteAt int64 `json:"delete_at"` + Username string `json:"username"` + Password string `json:"password,omitempty"` + AuthData *string `json:"auth_data,omitempty"` + AuthService string `json:"auth_service"` + Email string `json:"email"` + EmailVerified bool `json:"email_verified,omitempty"` + Nickname string `json:"nickname"` + FirstName string `json:"first_name"` + LastName string `json:"last_name"` + Position string `json:"position"` + Roles string `json:"roles"` + AllowMarketing bool `json:"allow_marketing,omitempty"` + Props StringMap `json:"props,omitempty"` + NotifyProps StringMap `json:"notify_props,omitempty"` + LastPasswordUpdate int64 `json:"last_password_update,omitempty"` + LastPictureUpdate int64 `json:"last_picture_update,omitempty"` + FailedAttempts int `json:"failed_attempts,omitempty"` + Locale string `json:"locale"` + Timezone StringMap `json:"timezone"` + MfaActive bool `json:"mfa_active,omitempty"` + MfaSecret string `json:"mfa_secret,omitempty"` + RemoteId *string `json:"remote_id,omitempty"` + LastActivityAt int64 `json:"last_activity_at,omitempty"` + IsBot bool `json:"is_bot,omitempty"` + BotDescription string `json:"bot_description,omitempty"` + BotLastIconUpdate int64 `json:"bot_last_icon_update,omitempty"` + TermsOfServiceId string `json:"terms_of_service_id,omitempty"` + TermsOfServiceCreateAt int64 `json:"terms_of_service_create_at,omitempty"` + DisableWelcomeEmail bool `json:"disable_welcome_email"` + LastLogin int64 `json:"last_login,omitempty"` + MfaUsedTimestamps StringArray `json:"mfa_used_timestamps,omitempty"` } func (u *User) Auditable() map[string]interface{} { @@ -660,6 +661,7 @@ func (u *User) Sanitize(options map[string]bool) { u.Password = "" u.AuthData = NewPointer("") u.MfaSecret = "" + u.MfaUsedTimestamps = nil u.LastLogin = 0 if len(options) != 0 && !options["email"] { @@ -694,6 +696,7 @@ func (u *User) SanitizeInput(isAdmin bool) { u.FailedAttempts = 0 u.MfaActive = false u.MfaSecret = "" + u.MfaUsedTimestamps = StringArray{} u.Email = strings.TrimSpace(u.Email) u.LastActivityAt = 0 } @@ -702,6 +705,7 @@ func (u *User) ClearNonProfileFields(asAdmin bool) { u.Password = "" u.AuthData = NewPointer("") u.MfaSecret = "" + u.MfaUsedTimestamps = nil u.EmailVerified = false u.AllowMarketing = false u.LastPasswordUpdate = 0 diff --git a/server/public/model/user_serial_gen.go b/server/public/model/user_serial_gen.go index e33b4e2f60..410698e05b 100644 --- a/server/public/model/user_serial_gen.go +++ b/server/public/model/user_serial_gen.go @@ -17,8 +17,8 @@ func (z *User) DecodeMsg(dc *msgp.Reader) (err error) { err = msgp.WrapError(err) return } - if zb0001 != 34 { - err = msgp.ArrayError{Wanted: 34, Got: zb0001} + if zb0001 != 35 { + err = msgp.ArrayError{Wanted: 35, Got: zb0001} return } z.Id, err = dc.ReadString() @@ -215,13 +215,18 @@ func (z *User) DecodeMsg(dc *msgp.Reader) (err error) { err = msgp.WrapError(err, "LastLogin") return } + err = z.MfaUsedTimestamps.DecodeMsg(dc) + if err != nil { + err = msgp.WrapError(err, "MfaUsedTimestamps") + return + } return } // EncodeMsg implements msgp.Encodable func (z *User) EncodeMsg(en *msgp.Writer) (err error) { - // array header, size 34 - err = en.Append(0xdc, 0x0, 0x22) + // array header, size 35 + err = en.Append(0xdc, 0x0, 0x23) if err != nil { return } @@ -409,14 +414,19 @@ func (z *User) EncodeMsg(en *msgp.Writer) (err error) { err = msgp.WrapError(err, "LastLogin") return } + err = z.MfaUsedTimestamps.EncodeMsg(en) + if err != nil { + err = msgp.WrapError(err, "MfaUsedTimestamps") + return + } return } // MarshalMsg implements msgp.Marshaler func (z *User) MarshalMsg(b []byte) (o []byte, err error) { o = msgp.Require(b, z.Msgsize()) - // array header, size 34 - o = append(o, 0xdc, 0x0, 0x22) + // array header, size 35 + o = append(o, 0xdc, 0x0, 0x23) o = msgp.AppendString(o, z.Id) o = msgp.AppendInt64(o, z.CreateAt) o = msgp.AppendInt64(o, z.UpdateAt) @@ -471,6 +481,11 @@ func (z *User) MarshalMsg(b []byte) (o []byte, err error) { o = msgp.AppendInt64(o, z.TermsOfServiceCreateAt) o = msgp.AppendBool(o, z.DisableWelcomeEmail) o = msgp.AppendInt64(o, z.LastLogin) + o, err = z.MfaUsedTimestamps.MarshalMsg(o) + if err != nil { + err = msgp.WrapError(err, "MfaUsedTimestamps") + return + } return } @@ -482,8 +497,8 @@ func (z *User) UnmarshalMsg(bts []byte) (o []byte, err error) { err = msgp.WrapError(err) return } - if zb0001 != 34 { - err = msgp.ArrayError{Wanted: 34, Got: zb0001} + if zb0001 != 35 { + err = msgp.ArrayError{Wanted: 35, Got: zb0001} return } z.Id, bts, err = msgp.ReadStringBytes(bts) @@ -678,6 +693,11 @@ func (z *User) UnmarshalMsg(bts []byte) (o []byte, err error) { err = msgp.WrapError(err, "LastLogin") return } + bts, err = z.MfaUsedTimestamps.UnmarshalMsg(bts) + if err != nil { + err = msgp.WrapError(err, "MfaUsedTimestamps") + return + } o = bts return } @@ -696,7 +716,7 @@ func (z *User) Msgsize() (s int) { } else { s += msgp.StringPrefixSize + len(*z.RemoteId) } - s += msgp.Int64Size + msgp.BoolSize + msgp.StringPrefixSize + len(z.BotDescription) + msgp.Int64Size + msgp.StringPrefixSize + len(z.TermsOfServiceId) + msgp.Int64Size + msgp.BoolSize + msgp.Int64Size + s += msgp.Int64Size + msgp.BoolSize + msgp.StringPrefixSize + len(z.BotDescription) + msgp.Int64Size + msgp.StringPrefixSize + len(z.TermsOfServiceId) + msgp.Int64Size + msgp.BoolSize + msgp.Int64Size + z.MfaUsedTimestamps.Msgsize() return } diff --git a/server/public/model/user_test.go b/server/public/model/user_test.go index 5310eecbc6..927b8f2242 100644 --- a/server/public/model/user_test.go +++ b/server/public/model/user_test.go @@ -332,6 +332,7 @@ func TestUserSanitizeInput(t *testing.T) { user.EmailVerified = true user.FailedAttempts = 10 user.LastActivityAt = GetMillis() + user.MfaUsedTimestamps = StringArray{"1234", "4566"} user.SanitizeInput(false) @@ -347,6 +348,7 @@ func TestUserSanitizeInput(t *testing.T) { require.Equal(t, int64(0), user.LastPictureUpdate) require.Equal(t, int64(0), user.LastActivityAt) require.Equal(t, 0, user.FailedAttempts) + require.Equal(t, StringArray{}, user.MfaUsedTimestamps) // these fields should remain intact require.Equal(t, "user@example.com", user.Email) diff --git a/server/public/model/utils.go b/server/public/model/utils.go index 4ac8b4f1ba..7ae5f5eff8 100644 --- a/server/public/model/utils.go +++ b/server/public/model/utils.go @@ -41,8 +41,11 @@ const ( var ErrMaxPropSizeExceeded = fmt.Errorf("max prop size of %d exceeded", maxPropSizeBytes) +//msgp:ignore StringInterface StringSet type StringInterface map[string]any type StringSet map[string]struct{} + +//msgp:tuple StringArray type StringArray []string func (ss StringSet) Has(val string) bool { @@ -228,6 +231,7 @@ func AppErrorInit(t i18n.TranslateFunc) { }) } +//msgp:ignore AppError type AppError struct { Id string `json:"id"` Message string `json:"message"` // Message to be display to the end user without debugging information diff --git a/server/public/model/utils_serial_gen.go b/server/public/model/utils_serial_gen.go new file mode 100644 index 0000000000..b14631b390 --- /dev/null +++ b/server/public/model/utils_serial_gen.go @@ -0,0 +1,93 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package model + +// Code generated by github.com/tinylib/msgp DO NOT EDIT. + +import ( + "github.com/tinylib/msgp/msgp" +) + +// DecodeMsg implements msgp.Decodable +func (z *StringArray) DecodeMsg(dc *msgp.Reader) (err error) { + var zb0002 uint32 + zb0002, err = dc.ReadArrayHeader() + if err != nil { + err = msgp.WrapError(err) + return + } + if cap((*z)) >= int(zb0002) { + (*z) = (*z)[:zb0002] + } else { + (*z) = make(StringArray, zb0002) + } + for zb0001 := range *z { + (*z)[zb0001], err = dc.ReadString() + if err != nil { + err = msgp.WrapError(err, zb0001) + return + } + } + return +} + +// EncodeMsg implements msgp.Encodable +func (z StringArray) EncodeMsg(en *msgp.Writer) (err error) { + err = en.WriteArrayHeader(uint32(len(z))) + if err != nil { + err = msgp.WrapError(err) + return + } + for zb0003 := range z { + err = en.WriteString(z[zb0003]) + if err != nil { + err = msgp.WrapError(err, zb0003) + return + } + } + return +} + +// MarshalMsg implements msgp.Marshaler +func (z StringArray) MarshalMsg(b []byte) (o []byte, err error) { + o = msgp.Require(b, z.Msgsize()) + o = msgp.AppendArrayHeader(o, uint32(len(z))) + for zb0003 := range z { + o = msgp.AppendString(o, z[zb0003]) + } + return +} + +// UnmarshalMsg implements msgp.Unmarshaler +func (z *StringArray) UnmarshalMsg(bts []byte) (o []byte, err error) { + var zb0002 uint32 + zb0002, bts, err = msgp.ReadArrayHeaderBytes(bts) + if err != nil { + err = msgp.WrapError(err) + return + } + if cap((*z)) >= int(zb0002) { + (*z) = (*z)[:zb0002] + } else { + (*z) = make(StringArray, zb0002) + } + for zb0001 := range *z { + (*z)[zb0001], bts, err = msgp.ReadStringBytes(bts) + if err != nil { + err = msgp.WrapError(err, zb0001) + return + } + } + o = bts + return +} + +// Msgsize returns an upper bound estimate of the number of bytes occupied by the serialized message +func (z StringArray) Msgsize() (s int) { + s = msgp.ArrayHeaderSize + for zb0003 := range z { + s += msgp.StringPrefixSize + len(z[zb0003]) + } + return +}