diff --git a/api/v4/source/users.yaml b/api/v4/source/users.yaml index 139a62e410..504adc820b 100644 --- a/api/v4/source/users.yaml +++ b/api/v4/source/users.yaml @@ -3266,3 +3266,25 @@ $ref: "#/components/responses/Unauthorized" "403": $ref: "#/components/responses/Forbidden" + "/api/v4/users/{user_id}/reset_failed_attempts": + post: + tags: + - users + summary: Reset the failed password attempts for a user + description: | + Reset the FailedAttempts field for a user to 0. This will only work for ldap and email/password users. + + ##### Permissions + + Requires `sysconsole_write_user_management_users` permission. + + operationId: resetPasswordFailedAttempts + responses: + "200": + description: User's thread update successful + "400": + $ref: "#/components/responses/BadRequest" + "401": + $ref: "#/components/responses/Unauthorized" + "404": + $ref: "#/components/responses/NotFound" diff --git a/e2e-tests/playwright/support/server/default_config.ts b/e2e-tests/playwright/support/server/default_config.ts index 55d06e1336..77e386eed7 100644 --- a/e2e-tests/playwright/support/server/default_config.ts +++ b/e2e-tests/playwright/support/server/default_config.ts @@ -469,6 +469,7 @@ const defaultServerConfig: AdminConfig = { EnableSync: false, LdapServer: '', LdapPort: 389, + MaximumLoginAttempts: 10, ConnectionSecurity: '', BaseDN: '', BindUsername: '', diff --git a/server/channels/api4/user.go b/server/channels/api4/user.go index ec57306c2f..dc107fe0f4 100644 --- a/server/channels/api4/user.go +++ b/server/channels/api4/user.go @@ -56,6 +56,7 @@ func (api *API) InitUser() { api.BaseRoutes.User.Handle("/email/verify/member", api.APISessionRequired(verifyUserEmailWithoutToken)).Methods(http.MethodPost) api.BaseRoutes.User.Handle("/terms_of_service", api.APISessionRequired(saveUserTermsOfService)).Methods(http.MethodPost) api.BaseRoutes.User.Handle("/terms_of_service", api.APISessionRequired(getUserTermsOfService)).Methods(http.MethodGet) + api.BaseRoutes.User.Handle("/reset_failed_attempts", api.APISessionRequired(resetPasswordFailedAttempts)).Methods(http.MethodPost) api.BaseRoutes.User.Handle("/auth", api.APISessionRequiredTrustRequester(updateUserAuth)).Methods(http.MethodPut) @@ -1861,6 +1862,7 @@ func login(c *Context, w http.ResponseWriter, r *http.Request) { "api.user.check_user_login_attempts.too_many.app_error", "app.team.join_user_to_team.max_accounts.app_error", "store.sql_user.save.max_accounts.app_error", + "api.user.check_user_login_attempts.too_many_ldap.app_error", } maskError := true @@ -3525,3 +3527,46 @@ func getUsersWithInvalidEmails(c *Context, w http.ResponseWriter, r *http.Reques c.Logger.Warn("Error writing response", mlog.Err(err)) } } + +func resetPasswordFailedAttempts(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequireUserId() + if c.Err != nil { + return + } + errParams := map[string]any{"userID": c.Params.UserId} + + auditRec := c.MakeAuditRecord("resetPasswordFailedAttempts", audit.Fail) + defer c.LogAuditRec(auditRec) + + if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionSysconsoleWriteUserManagementUsers) { + c.Err = model.NewAppError("resetPasswordFailedAttempts", "api.user.reset_password_failed_attempts.permissions.app_error", errParams, "", http.StatusForbidden) + return + } + + user, err := c.App.GetUser(c.Params.UserId) + if err != nil { + c.Err = err + return + } + auditRec.AddEventPriorState(user) + auditRec.AddEventObjectType("user") + + if user.IsSystemAdmin() && !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) { + c.SetPermissionError(model.PermissionManageSystem) + return + } + + if user.AuthService != model.UserAuthServiceLdap && user.AuthService != "" { + c.Err = model.NewAppError("resetPasswordFailedAttempts", "api.user.reset_password_failed_attempts.ldap_and_email_only.app_error", errParams, "", http.StatusBadRequest) + return + } + + if err := c.App.ResetPasswordFailedAttempts(c.AppContext, user); err != nil { + c.Err = err + return + } + + auditRec.Success() + + ReturnStatusOK(w) +} diff --git a/server/channels/api4/user_test.go b/server/channels/api4/user_test.go index 4a4d458931..7d62b2543a 100644 --- a/server/channels/api4/user_test.go +++ b/server/channels/api4/user_test.go @@ -19,6 +19,7 @@ import ( "time" "github.com/dgryski/dgoogauth" + "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -8973,6 +8974,221 @@ func TestRevokeAllSessionsForUser(t *testing.T) { }) } +func TestResetPasswordFailedAttempts(t *testing.T) { + th := SetupEnterprise(t).InitBasic() + defer th.TearDown() + th.SetupLdapConfig() + + th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + + t.Run("Reset password failed attempts for regular user", func(t *testing.T) { + client := th.CreateClient() + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.MaximumLoginAttempts = 10 + }) + maxAttempts := th.App.Config().ServiceSettings.MaximumLoginAttempts + + user := th.CreateUser() + + for i := 0; i < *maxAttempts; i++ { + _, _, err := client.Login(context.Background(), user.Email, "wrongpassword") + require.Error(t, err) + } + + user, resp, err := th.SystemAdminClient.GetUser(context.Background(), user.Id, "") + require.NoError(t, err) + CheckOKStatus(t, resp) + require.Equal(t, *maxAttempts, user.FailedAttempts) + + resp, err = th.SystemAdminClient.ResetFailedAttempts(context.Background(), user.Id) + require.NoError(t, err) + CheckOKStatus(t, resp) + + user, resp, err = th.SystemAdminClient.GetUser(context.Background(), user.Id, "") + require.NoError(t, err) + CheckOKStatus(t, resp) + require.Equal(t, int(0), user.FailedAttempts) + }) + + t.Run("Reset password failed attempts for ldap user", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.LdapSettings.MaximumLoginAttempts = 5 + }) + + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockLdap := &mocks.LdapInterface{} + + username := GenerateTestUsername() + + ldapUser := &model.User{ + Email: "foobar+testdomainrestriction@mattermost.org", + Username: username, + AuthService: "ldap", + AuthData: &username, + EmailVerified: true, + } + ldapUser, appErr := th.App.CreateUser(th.Context, ldapUser) + require.Nil(t, appErr) + + client := th.CreateClient() + mockLdap.Mock.On("GetUser", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("string")).Return(ldapUser, nil).Times(5) + + th.App.Channels().Ldap = mockLdap + + for i := 0; i < 5; i++ { + mockedLdapUser := ldapUser + mockedLdapUser.FailedAttempts = i + mockLdap.Mock.On("DoLogin", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return(mockedLdapUser, &model.AppError{Id: "ent.ldap.do_login.invalid_password.app_error"}) + _, _, err := client.LoginByLdap(context.Background(), *ldapUser.AuthData, "wrongpassword") + require.Error(t, err) + } + + user, resp, err := th.SystemAdminClient.GetUser(context.Background(), ldapUser.Id, "") + require.NoError(t, err) + CheckOKStatus(t, resp) + require.Equal(t, int(5), user.FailedAttempts) + + resp, err = th.SystemAdminClient.ResetFailedAttempts(context.Background(), ldapUser.Id) + require.NoError(t, err) + CheckOKStatus(t, resp) + + user, resp, err = th.SystemAdminClient.GetUser(context.Background(), ldapUser.Id, "") + require.NoError(t, err) + CheckOKStatus(t, resp) + require.Equal(t, int(0), user.FailedAttempts) + }) + + t.Run("Regular user unable to reset failed attempts", func(t *testing.T) { + client := th.CreateClient() + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.MaximumLoginAttempts = 10 + }) + maxAttempts := th.App.Config().ServiceSettings.MaximumLoginAttempts + + user := th.CreateUser() + + for i := 0; i < *maxAttempts; i++ { + _, _, err := client.Login(context.Background(), user.Email, "wrongpassword") + require.Error(t, err) + } + + user, resp, err := th.SystemAdminClient.GetUser(context.Background(), user.Id, "") + require.NoError(t, err) + CheckOKStatus(t, resp) + require.Equal(t, *maxAttempts, user.FailedAttempts) + + resp, err = th.Client.ResetFailedAttempts(context.Background(), user.Id) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + + user, resp, err = th.SystemAdminClient.GetUser(context.Background(), user.Id, "") + require.NoError(t, err) + CheckOKStatus(t, resp) + require.Equal(t, *maxAttempts, user.FailedAttempts) + }) + + t.Run("Reset password failed attempts when user has PermissionSysconsoleWriteUserManagementUsers", func(t *testing.T) { + th.AddPermissionToRole(model.PermissionSysconsoleWriteUserManagementUsers.Id, model.SystemUserRoleId) + defer th.RemovePermissionFromRole(model.PermissionSysconsoleWriteUserManagementUsers.Id, model.SystemUserRoleId) + + client := th.CreateClient() + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.MaximumLoginAttempts = 10 + }) + maxAttempts := th.App.Config().ServiceSettings.MaximumLoginAttempts + + user := th.CreateUser() + + for i := 0; i < *maxAttempts; i++ { + _, _, err := client.Login(context.Background(), user.Email, "wrongpassword") + require.Error(t, err) + } + + fetchedUser, resp, err := th.SystemAdminClient.GetUser(context.Background(), user.Id, "") + require.NoError(t, err) + CheckOKStatus(t, resp) + require.Equal(t, *maxAttempts, fetchedUser.FailedAttempts) + + resp, err = th.Client.ResetFailedAttempts(context.Background(), user.Id) + require.NoError(t, err) + CheckOKStatus(t, resp) + + fetchedUser, resp, err = th.SystemAdminClient.GetUser(context.Background(), user.Id, "") + require.NoError(t, err) + CheckOKStatus(t, resp) + require.Equal(t, int(0), fetchedUser.FailedAttempts) + }) + + t.Run("Unable to reset password failed attempts for sysadmin when user has PermissionSysconsoleWriteUserManagementUsers", func(t *testing.T) { + th.AddPermissionToRole(model.PermissionSysconsoleWriteUserManagementUsers.Id, model.SystemUserRoleId) + defer th.RemovePermissionFromRole(model.PermissionSysconsoleWriteUserManagementUsers.Id, model.SystemUserRoleId) + + client := th.CreateClient() + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.MaximumLoginAttempts = 10 + }) + maxAttempts := th.App.Config().ServiceSettings.MaximumLoginAttempts + + // create sysadmin user + sysadmin := th.CreateUser() + _, appErr := th.App.UpdateUserRoles(th.Context, sysadmin.Id, model.SystemUserRoleId+" "+model.SystemAdminRoleId, false) + require.Nil(t, appErr) + + for i := 0; i < *maxAttempts; i++ { + _, _, err := client.Login(context.Background(), sysadmin.Email, "wrongpassword") + require.Error(t, err) + } + + sysadminUser, resp, err := th.SystemAdminClient.GetUser(context.Background(), sysadmin.Id, "") + require.NoError(t, err) + CheckOKStatus(t, resp) + require.Equal(t, *maxAttempts, sysadminUser.FailedAttempts) + + resp, err = th.Client.ResetFailedAttempts(context.Background(), sysadminUser.Id) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + + sysadminUser, resp, err = th.SystemAdminClient.GetUser(context.Background(), sysadminUser.Id, "") + require.NoError(t, err) + CheckOKStatus(t, resp) + require.Equal(t, int(10), sysadminUser.FailedAttempts) + }) + + t.Run("Reset password failed attempts for sysadmin", func(t *testing.T) { + client := th.CreateClient() + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.MaximumLoginAttempts = 10 + }) + maxAttempts := th.App.Config().ServiceSettings.MaximumLoginAttempts + + sysadmin := th.CreateUser() + _, appErr := th.App.UpdateUserRoles(th.Context, sysadmin.Id, model.SystemUserRoleId+" "+model.SystemAdminRoleId, false) + require.Nil(t, appErr) + + for i := 0; i < *maxAttempts; i++ { + _, _, err := client.Login(context.Background(), sysadmin.Email, "wrongpassword") + require.Error(t, err) + } + + sysadminUser, resp, err := th.SystemAdminClient.GetUser(context.Background(), sysadmin.Id, "") + require.NoError(t, err) + CheckOKStatus(t, resp) + require.Equal(t, *maxAttempts, sysadminUser.FailedAttempts) + + resp, err = th.SystemAdminClient.ResetFailedAttempts(context.Background(), sysadminUser.Id) + require.NoError(t, err) + CheckOKStatus(t, resp) + + sysadminUser, resp, err = th.SystemAdminClient.GetUser(context.Background(), sysadminUser.Id, "") + require.NoError(t, err) + CheckOKStatus(t, resp) + require.Equal(t, int(0), sysadminUser.FailedAttempts) + }) +} func TestSearchUsersWithMfaEnforced(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() diff --git a/server/channels/app/authentication.go b/server/channels/app/authentication.go index 532beb8fd4..3b8354f55c 100644 --- a/server/channels/app/authentication.go +++ b/server/channels/app/authentication.go @@ -64,8 +64,8 @@ func (a *App) IsPasswordValid(rctx request.CTX, password string) *model.AppError func (a *App) CheckPasswordAndAllCriteria(rctx request.CTX, userID string, password string, mfaToken string) *model.AppError { // MM-37585 // Use locks to avoid concurrently checking AND updating the failed login attempts. - a.ch.loginAttemptsMut.Lock() - defer a.ch.loginAttemptsMut.Unlock() + a.ch.emailLoginAttemptsMut.Lock() + defer a.ch.emailLoginAttemptsMut.Unlock() user, err := a.GetUser(userID) if err != nil { @@ -149,31 +149,86 @@ func (a *App) DoubleCheckPassword(rctx request.CTX, user *model.User, password s return nil } -func (a *App) checkLdapUserPasswordAndAllCriteria(rctx request.CTX, ldapId *string, password string, mfaToken string) (*model.User, *model.AppError) { - if a.Ldap() == nil || ldapId == nil { +func (a *App) checkLdapUserPasswordAndAllCriteria(rctx request.CTX, user *model.User, password, mfaToken string) (*model.User, *model.AppError) { + // MM-37585: Use locks to avoid concurrently checking AND updating the failed login attempts. + a.ch.ldapLoginAttemptsMut.Lock() + defer a.ch.ldapLoginAttemptsMut.Unlock() + + // We need to get the latest value of the user from the database after we acquire the lock. user is nil for first-time LDAP users. + if user.Id != "" { + var err *model.AppError + user, err = a.GetUser(user.Id) + if err != nil { + if err.Id != MissingAccountError { + err.StatusCode = http.StatusInternalServerError + return nil, err + } + err.StatusCode = http.StatusBadRequest + return nil, err + } + } + + ldapID := user.AuthData + + if a.Ldap() == nil || ldapID == nil { err := model.NewAppError("doLdapAuthentication", "api.user.login_ldap.not_available.app_error", nil, "", http.StatusNotImplemented) return nil, err } - ldapUser, err := a.Ldap().DoLogin(rctx, *ldapId, password) + // First time LDAP users will not have a userID + if user.Id != "" { + if err := checkUserLoginAttempts(user, *a.Config().LdapSettings.MaximumLoginAttempts); err != nil { + return nil, err + } + } + + ldapUser, err := a.Ldap().DoLogin(rctx, *ldapID, password) if err != nil { + // If this is a new LDAP user, we need to get the user from the database because DoLogin will have created the user. + if user.Id == "" { + var getUserByAuthErr *model.AppError + ldapUser, getUserByAuthErr = a.GetUserByAuth(ldapID, model.UserAuthServiceLdap) + if getUserByAuthErr != nil { + return nil, getUserByAuthErr + } + } else { + ldapUser = user + } + // Log a info to make it easier to admin to spot that a user tried to log in with a legitimate user name. if err.Id == "ent.ldap.do_login.invalid_password.app_error" { - rctx.Logger().LogM(mlog.MlvlLDAPInfo, "A user tried to sign in, which matched an LDAP account, but the password was incorrect.", mlog.String("ldap_id", *ldapId)) + rctx.Logger().LogM(mlog.MlvlLDAPInfo, "A user tried to sign in, which matched an LDAP account, but the password was incorrect.", mlog.String("ldap_id", *ldapID)) + + if passErr := a.Srv().Store().User().UpdateFailedPasswordAttempts(ldapUser.Id, ldapUser.FailedAttempts+1); passErr != nil { + return nil, model.NewAppError("CheckPasswordAndAllCriteria", "app.user.update_failed_pwd_attempts.app_error", nil, "", http.StatusInternalServerError).Wrap(passErr) + } } err.StatusCode = http.StatusUnauthorized return nil, err } - if err := a.CheckUserMfa(rctx, ldapUser, mfaToken); err != nil { + if err = a.CheckUserMfa(rctx, ldapUser, mfaToken); err != nil { + // If the mfaToken is not set, we assume the client used this as a pre-flight request to query the server + // about the MFA state of the user in question + if mfaToken != "" && ldapUser.Id != "" { + if passErr := a.Srv().Store().User().UpdateFailedPasswordAttempts(ldapUser.Id, ldapUser.FailedAttempts+1); passErr != nil { + return nil, model.NewAppError("CheckPasswordAndAllCriteria", "app.user.update_failed_pwd_attempts.app_error", nil, "", http.StatusInternalServerError).Wrap(passErr) + } + } return nil, err } - if err := checkUserNotDisabled(ldapUser); err != nil { + if err = checkUserNotDisabled(ldapUser); err != nil { return nil, err } + if ldapUser.FailedAttempts > 0 { + if passErr := a.Srv().Store().User().UpdateFailedPasswordAttempts(ldapUser.Id, 0); passErr != nil { + return nil, model.NewAppError("CheckPasswordAndAllCriteria", "app.user.update_failed_pwd_attempts.app_error", nil, "", http.StatusInternalServerError).Wrap(passErr) + } + } + // user successfully authenticated return ldapUser, nil } @@ -286,6 +341,9 @@ func (a *App) MFARequired(rctx request.CTX) *model.AppError { func checkUserLoginAttempts(user *model.User, max int) *model.AppError { if user.FailedAttempts >= max { + if user.AuthService == model.UserAuthServiceLdap { + return model.NewAppError("checkUserLoginAttempts", "api.user.check_user_login_attempts.too_many_ldap.app_error", nil, "user_id="+user.Id, http.StatusUnauthorized) + } return model.NewAppError("checkUserLoginAttempts", "api.user.check_user_login_attempts.too_many.app_error", nil, "user_id="+user.Id, http.StatusUnauthorized) } @@ -316,7 +374,7 @@ func (a *App) authenticateUser(rctx request.CTX, user *model.User, password, mfa return user, err } - ldapUser, err := a.checkLdapUserPasswordAndAllCriteria(rctx, user.AuthData, password, mfaToken) + ldapUser, err := a.checkLdapUserPasswordAndAllCriteria(rctx, user, password, mfaToken) if err != nil { err.StatusCode = http.StatusUnauthorized return user, err diff --git a/server/channels/app/authentication_test.go b/server/channels/app/authentication_test.go index 586550669e..a91d63473d 100644 --- a/server/channels/app/authentication_test.go +++ b/server/channels/app/authentication_test.go @@ -13,9 +13,11 @@ import ( "time" "github.com/dgryski/dgoogauth" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/v8/einterfaces/mocks" ) func TestParseAuthTokenFromRequest(t *testing.T) { @@ -153,3 +155,211 @@ func TestCheckPasswordAndAllCriteria(t *testing.T) { } }) } + +func TestCheckLdapUserPasswordAndAllCriteria(t *testing.T) { + th := SetupEnterprise(t).InitBasic() + defer th.TearDown() + + // update config + const maxFailedLoginAttempts = 3 + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.LdapSettings.MaximumLoginAttempts = maxFailedLoginAttempts + *cfg.ServiceSettings.EnableMultifactorAuthentication = true + }) + + mockLdap := &mocks.LdapInterface{} + th.App.Channels().Ldap = mockLdap + + authData := model.NewRandomString(32) + + // create an ldap user by calling createUser + ldapUser := &model.User{ + Email: "ldapuser@mattermost-customer.com", + Username: "ldapuser", + AuthService: model.UserAuthServiceLdap, + AuthData: &authData, + EmailVerified: true, + } + user, appErr := th.App.CreateUser(th.Context, ldapUser) + require.Nil(t, appErr) + user.AuthData = &authData + + testCases := []struct { + name string + password string + expectedErrID string + mockDoLogin func() + }{ + { + name: "valid password", + password: "password", + expectedErrID: "", + mockDoLogin: func() { + mockLdap.Mock.On("DoLogin", th.Context, authData, "password").Return(user, nil) + }, + }, + { + name: "invalid password", + password: "wrongpassword", + expectedErrID: "api.user.check_user_password.invalid.app_error", + mockDoLogin: func() { + mockLdap.Mock.On("DoLogin", th.Context, authData, "wrongpassword").Return(nil, &model.AppError{Id: "ent.ldap.do_login.invalid_password.app_error"}) + }, + }, + { + name: "too many login attempts", + password: "wrongpassword", + expectedErrID: "api.user.check_user_login_attempts.too_many_ldap.app_error", + mockDoLogin: func() { + mockLdap.Mock.On("DoLogin", th.Context, authData, "wrongpassword").Return(nil, &model.AppError{Id: "ent.ldap.do_login.invalid_password.app_error"}).Once() + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Reset login attempts + err := th.App.Srv().Store().User().UpdateFailedPasswordAttempts(user.Id, 0) + require.NoError(t, err) + + tc.mockDoLogin() + + ldapUser := user + + // Simulate failed login attempts if necessary + if tc.expectedErrID == "api.user.check_user_login_attempts.too_many_ldap.app_error" { + for i := 0; i < maxFailedLoginAttempts-1; i++ { + _, appErr = th.App.checkLdapUserPasswordAndAllCriteria(th.Context, ldapUser, "wrongpassword", "") + require.NotNil(t, appErr) + require.Equal(t, "ent.ldap.do_login.invalid_password.app_error", appErr.Id) + } + } + // Call the method with the test case parameters + _, appErr := th.App.checkLdapUserPasswordAndAllCriteria(th.Context, ldapUser, tc.password, "") + + // Verify the returned error matches the expected error + if tc.expectedErrID == "" { + require.Nil(t, appErr) + } else { + require.NotNil(t, appErr) + } + + if tc.expectedErrID == "api.user.check_user_login_attempts.too_many_ldap.app_error" { + updatedUser, err := th.App.GetUser(ldapUser.Id) + require.Nil(t, err) + require.Equal(t, maxFailedLoginAttempts, updatedUser.FailedAttempts) + } + }) + } +} + +func TestCheckLdapUserPasswordConcurrency(t *testing.T) { + th := SetupEnterprise(t).InitBasic() + defer th.TearDown() + + // update config + const maxFailedLoginAttempts = 1 + const concurrentAttempts = 10 + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.LdapSettings.MaximumLoginAttempts = maxFailedLoginAttempts + *cfg.ServiceSettings.EnableMultifactorAuthentication = true + }) + + authData := model.NewRandomString(32) + + // create an ldap user by calling createUser + ldapUser := &model.User{ + Email: "ldapuser@mattermost-customer.com", + Username: "ldapuser", + AuthService: model.UserAuthServiceLdap, + AuthData: &authData, + EmailVerified: true, + } + user, appErr := th.App.CreateUser(th.Context, ldapUser) + require.Nil(t, appErr) + + // setup MFA + secret, appErr := th.App.GenerateMfaSecret(user.Id) + require.Nil(t, appErr) + err := th.Server.Store().User().UpdateMfaActive(user.Id, true) + require.NoError(t, err) + err = th.Server.Store().User().UpdateMfaSecret(user.Id, secret.Secret) + require.NoError(t, err) + + user, appErr = th.App.GetUser(user.Id) + require.Nil(t, appErr) + user.AuthData = &authData + + t.Run("validate concurrent failed attempts to bypass checks", func(t *testing.T) { + testCases := []struct { + name string + password string + mfaToken string + expectedErrID string + doLoginExpectedErrID string + }{ + { + name: "should not breach max. login attempts when password is wrong", + password: "wrong password", + mfaToken: "", + doLoginExpectedErrID: "ent.ldap.do_login.invalid_password.app_error", + expectedErrID: "ent.ldap.do_login.invalid_password.app_error", + }, + { + name: "should not breach max. login attempts when MFA is wrong", + password: "password", + mfaToken: "123456", + doLoginExpectedErrID: "", + expectedErrID: "api.user.check_user_mfa.bad_code.app_error", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + mockLdap := &mocks.LdapInterface{} + th.App.Channels().Ldap = mockLdap + // Reset login attempts + err := th.App.Srv().Store().User().UpdateFailedPasswordAttempts(user.Id, 0) + require.NoError(t, err) + + // Capture all concurrent errors + appErrs := make([]*model.AppError, concurrentAttempts) + + // Wait to complete the test + var completeWG sync.WaitGroup + completeWG.Add(concurrentAttempts) + + for i := 0; i < concurrentAttempts; i++ { + go func(i int) { + defer completeWG.Done() + + if tc.doLoginExpectedErrID == "ent.ldap.do_login.invalid_password.app_error" { + mockLdap.Mock.On("DoLogin", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return(nil, &model.AppError{Id: tc.doLoginExpectedErrID}) + } else { + mockLdap.Mock.On("DoLogin", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("string"), tc.password).Return(user, nil) + } + _, appErrs[i] = th.App.checkLdapUserPasswordAndAllCriteria(th.Context, user, tc.password, tc.mfaToken) + }(i) + } + + completeWG.Wait() + + expectedErrsCount := 0 + for i := 0; i < concurrentAttempts; i++ { + if appErrs[i].Id == tc.expectedErrID { + expectedErrsCount++ + continue + } + + if appErrs[i] != nil { + require.Equal(t, "api.user.check_user_login_attempts.too_many_ldap.app_error", appErrs[i].Id, "All other errors should be of too many login attempts only.") + } + } + + // Password/MFA failure attempts should not breach the maxFailedAttempts + // even during concurrent access by the same user. + require.Equal(t, maxFailedLoginAttempts, expectedErrsCount) + }) + } + }) +} diff --git a/server/channels/app/channels.go b/server/channels/app/channels.go index cc10f3643b..953de1747e 100644 --- a/server/channels/app/channels.go +++ b/server/channels/app/channels.go @@ -80,10 +80,11 @@ type Channels struct { postReminderMut sync.Mutex postReminderTask *model.ScheduledTask - interruptQuitChan chan struct{} - scheduledPostMut sync.Mutex - scheduledPostTask *model.ScheduledTask - loginAttemptsMut sync.Mutex + interruptQuitChan chan struct{} + scheduledPostMut sync.Mutex + scheduledPostTask *model.ScheduledTask + emailLoginAttemptsMut sync.Mutex + ldapLoginAttemptsMut sync.Mutex } func NewChannels(s *Server) (*Channels, error) { diff --git a/server/channels/app/ldap.go b/server/channels/app/ldap.go index 25a09d9bec..e2a26ed104 100644 --- a/server/channels/app/ldap.go +++ b/server/channels/app/ldap.go @@ -153,11 +153,8 @@ func (a *App) SwitchLdapToEmail(c request.CTX, ldapPassword, code, email, newPas return "", model.NewAppError("SwitchLdapToEmail", "api.user.ldap_to_email.not_available.app_error", nil, "", http.StatusNotImplemented) } - if err := ldapInterface.CheckPasswordAuthData(c, *user.AuthData, ldapPassword); err != nil { - return "", err - } - - if err := a.CheckUserMfa(c, user, code); err != nil { + user, err = a.checkLdapUserPasswordAndAllCriteria(c, user, ldapPassword, code) + if err != nil { return "", err } diff --git a/server/channels/app/user.go b/server/channels/app/user.go index c429df5dde..ed2f09231e 100644 --- a/server/channels/app/user.go +++ b/server/channels/app/user.go @@ -2922,3 +2922,12 @@ func (a *App) UserIsFirstAdmin(rctx request.CTX, user *model.User) bool { return true } + +func (a *App) ResetPasswordFailedAttempts(c request.CTX, user *model.User) *model.AppError { + err := a.Srv().Store().User().UpdateFailedPasswordAttempts(user.Id, 0) + if err != nil { + return model.NewAppError("ResetPasswordFailedAttempts", "app.user.reset_password_failed_attempts.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + + return nil +} diff --git a/server/i18n/en.json b/server/i18n/en.json index 3d77afe6cb..b1f9fdb9d0 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -3930,6 +3930,10 @@ "id": "api.user.check_user_login_attempts.too_many.app_error", "translation": "Your account is locked because of too many failed password attempts. Please reset your password." }, + { + "id": "api.user.check_user_login_attempts.too_many_ldap.app_error", + "translation": "Your account is locked because of too many failed password attempts. Please contact your System Administrator." + }, { "id": "api.user.check_user_mfa.bad_code.app_error", "translation": "Invalid MFA token." @@ -4218,6 +4222,14 @@ "id": "api.user.reset_password.token_parse.error", "translation": "Unable to parse the reset password token" }, + { + "id": "api.user.reset_password_failed_attempts.ldap_and_email_only.app_error", + "translation": "User auth service must be LDAP or Email." + }, + { + "id": "api.user.reset_password_failed_attempts.permissions.app_error", + "translation": "You do not have permission to update this resource." + }, { "id": "api.user.saml.not_available.app_error", "translation": "SAML 2.0 is not configured or supported on this server." @@ -7264,6 +7276,10 @@ "id": "app.user.promote_guest.user_update.app_error", "translation": "Failed to update the user." }, + { + "id": "app.user.reset_password_failed_attempts.app_error", + "translation": "Failed to reset login attempts." + }, { "id": "app.user.save.app_error", "translation": "Unable to save the account." @@ -8048,6 +8064,10 @@ "id": "ent.ldap.do_login.x509.app_error", "translation": "Error creating key pair" }, + { + "id": "ent.ldap.get_user_by_auth.app_error", + "translation": "Failed to get user." + }, { "id": "ent.ldap.no.users.checkcertificate", "translation": "No LDAP users found, check your user filter and certificates." @@ -8980,6 +9000,10 @@ "id": "model.config.is_valid.ldap_login_id", "translation": "AD/LDAP field \"Login ID Attribute\" is required." }, + { + "id": "model.config.is_valid.ldap_max_login_attempts.app_error", + "translation": "Invalid maximum login attempts for ldap settings. Must be a positive number." + }, { "id": "model.config.is_valid.ldap_max_page_size.app_error", "translation": "Invalid max page size value." diff --git a/server/public/model/client4.go b/server/public/model/client4.go index 2a6bb06d7b..118a59e524 100644 --- a/server/public/model/client4.go +++ b/server/public/model/client4.go @@ -1684,6 +1684,16 @@ func (c *Client4) UpdateUserActive(ctx context.Context, userId string, active bo return BuildResponse(r), nil } +// ResetFailedAttempts resets the number of failed attempts for a user. +func (c *Client4) ResetFailedAttempts(ctx context.Context, userId string) (*Response, error) { + r, err := c.DoAPIPost(ctx, c.userRoute(userId)+"/reset_failed_attempts", "") + if err != nil { + return BuildResponse(r), err + } + defer closeBody(r) + return BuildResponse(r), nil +} + // DeleteUser deactivates a user in the system based on the provided user id string. func (c *Client4) DeleteUser(ctx context.Context, userId string) (*Response, error) { r, err := c.DoAPIDelete(ctx, c.userRoute(userId)) diff --git a/server/public/model/config.go b/server/public/model/config.go index 2de87a3a56..04c77c5c0c 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -158,6 +158,7 @@ const ( LdapSettingsDefaultGroupDisplayNameAttribute = "" LdapSettingsDefaultGroupIdAttribute = "" LdapSettingsDefaultPictureAttribute = "" + LdapSettingsDefaultMaximumLoginAttempts = 10 SamlSettingsDefaultIdAttribute = "" SamlSettingsDefaultGuestAttribute = "" @@ -2415,14 +2416,15 @@ type ClientRequirements struct { type LdapSettings struct { // Basic - Enable *bool `access:"authentication_ldap"` - EnableSync *bool `access:"authentication_ldap"` - LdapServer *string `access:"authentication_ldap"` // telemetry: none - LdapPort *int `access:"authentication_ldap"` // telemetry: none - ConnectionSecurity *string `access:"authentication_ldap"` - BaseDN *string `access:"authentication_ldap"` // telemetry: none - BindUsername *string `access:"authentication_ldap"` // telemetry: none - BindPassword *string `access:"authentication_ldap"` // telemetry: none + Enable *bool `access:"authentication_ldap"` + EnableSync *bool `access:"authentication_ldap"` + LdapServer *string `access:"authentication_ldap"` // telemetry: none + LdapPort *int `access:"authentication_ldap"` // telemetry: none + ConnectionSecurity *string `access:"authentication_ldap"` + BaseDN *string `access:"authentication_ldap"` // telemetry: none + BindUsername *string `access:"authentication_ldap"` // telemetry: none + BindPassword *string `access:"authentication_ldap"` // telemetry: none + MaximumLoginAttempts *int `access:"authentication_ldap"` // telemetry: none // Filtering UserFilter *string `access:"authentication_ldap"` // telemetry: none @@ -2510,6 +2512,10 @@ func (s *LdapSettings) SetDefaults() { s.BindPassword = NewPointer("") } + if s.MaximumLoginAttempts == nil { + s.MaximumLoginAttempts = NewPointer(LdapSettingsDefaultMaximumLoginAttempts) + } + if s.UserFilter == nil { s.UserFilter = NewPointer("") } @@ -4100,6 +4106,10 @@ func (s *LdapSettings) isValid() *AppError { return NewAppError("Config.IsValid", "model.config.is_valid.ldap_server", nil, "", http.StatusBadRequest) } + if *s.MaximumLoginAttempts <= 0 { + return NewAppError("Config.IsValid", "model.config.is_valid.ldap_max_login_attempts.app_error", nil, "", http.StatusBadRequest) + } + if *s.BaseDN == "" { return NewAppError("Config.IsValid", "model.config.is_valid.ldap_basedn", nil, "", http.StatusBadRequest) } diff --git a/server/public/model/report.go b/server/public/model/report.go index 387f390ee7..48824d6ea1 100644 --- a/server/public/model/report.go +++ b/server/public/model/report.go @@ -150,7 +150,7 @@ func (u *UserReportOptions) IsValid() *AppError { } func (u *UserReportQuery) ToReport() *UserReport { - u.ClearNonProfileFields(false) + u.ClearNonProfileFields(true) return &UserReport{ User: u.User, UserPostStats: u.UserPostStats, diff --git a/server/public/model/user.go b/server/public/model/user.go index 7b7c6b7021..1e6ec1ba1d 100644 --- a/server/public/model/user.go +++ b/server/public/model/user.go @@ -709,10 +709,10 @@ func (u *User) ClearNonProfileFields(asAdmin bool) { u.EmailVerified = false u.AllowMarketing = false u.LastPasswordUpdate = 0 - u.FailedAttempts = 0 if !asAdmin { u.NotifyProps = StringMap{} + u.FailedAttempts = 0 } } diff --git a/webapp/channels/src/components/admin_console/admin_definition.tsx b/webapp/channels/src/components/admin_console/admin_definition.tsx index 8178c41fbb..5f8866b927 100644 --- a/webapp/channels/src/components/admin_console/admin_definition.tsx +++ b/webapp/channels/src/components/admin_console/admin_definition.tsx @@ -3469,6 +3469,19 @@ const AdminDefinition: AdminDefinitionType = { ), ), }, + { + type: 'number', + key: 'LdapSettings.MaximumLoginAttempts', + label: defineMessage({id: 'admin.ldap.maximumLoginAttemptsTitle', defaultMessage: 'Maximum Login Attempts:'}), + help_text: defineMessage({id: 'admin.ldap.maximumLoginAttemptsDesc', defaultMessage: 'The maximum number of login attempts before the Mattermost account is locked. You can unlock the account in system console on the users page. Setting this value lower than your LDAP maximum login attempts ensures that the users won\'t be locked out of your LDAP server because of failed login attempts in Mattermost.'}), + isDisabled: it.any( + it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.LDAP)), + it.all( + it.stateIsFalse('LdapSettings.Enable'), + it.stateIsFalse('LdapSettings.EnableSync'), + ), + ), + }, { type: 'text', key: 'LdapSettings.LdapServer', diff --git a/webapp/channels/src/components/admin_console/system_users/system_users_list_actions/confirm_reset_failed_attempts_modal.tsx b/webapp/channels/src/components/admin_console/system_users/system_users_list_actions/confirm_reset_failed_attempts_modal.tsx new file mode 100644 index 0000000000..c1a5e9b1b9 --- /dev/null +++ b/webapp/channels/src/components/admin_console/system_users/system_users_list_actions/confirm_reset_failed_attempts_modal.tsx @@ -0,0 +1,78 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React from 'react'; +import {FormattedMessage} from 'react-intl'; +import {useDispatch} from 'react-redux'; + +import type {ServerError} from '@mattermost/types/errors'; +import type {UserProfile} from '@mattermost/types/users'; + +import {resetFailedAttempts} from 'mattermost-redux/actions/users'; + +import ConfirmModalRedux from 'components/confirm_modal_redux'; + +type Props = { + user: UserProfile; + onError: (error: ServerError) => void; + onSuccess: () => void; + onExited: () => void; +} + +export default function ConfirmResetFailedAttemptsModal({user, onSuccess, onError, onExited}: Props) { + const dispatch = useDispatch(); + + async function confirm() { + const {error} = await dispatch(resetFailedAttempts(user.id)); + if (error) { + onError(error); + } + onSuccess(); + } + + const title = ( + + ); + + const message = ( + + ); + + const createGroupMembershipsButton = ( + + ); + + const cancelGroupMembershipsButton = ( + + ); + + return ( + + ); +} diff --git a/webapp/channels/src/components/admin_console/system_users/system_users_list_actions/index.tsx b/webapp/channels/src/components/admin_console/system_users/system_users_list_actions/index.tsx index 4b15eab766..58c3b3c033 100644 --- a/webapp/channels/src/components/admin_console/system_users/system_users_list_actions/index.tsx +++ b/webapp/channels/src/components/admin_console/system_users/system_users_list_actions/index.tsx @@ -35,6 +35,7 @@ import Constants, {ModalIdentifiers} from 'utils/constants'; import type {GlobalState} from 'types/store'; import ConfirmManageUserSettingsModal from './confirm_manage_user_settings_modal'; +import ConfirmResetFailedAttemptsModal from './confirm_reset_failed_attempts_modal'; import CreateGroupSyncablesMembershipsModal from './create_group_syncables_membership_modal'; import DeactivateMemberModal from './deactivate_member_modal'; import DemoteToGuestModal from './demote_to_guest_modal'; @@ -303,6 +304,24 @@ export function SystemUsersListAction({user, currentUser, tableId, rowIndex, onE ); }, [user, updateUser, onError]); + const handleResetAttemptsClick = useCallback(() => { + function onResetAttemptsSuccess() { + updateUser({failed_attempts: 0}); + } + + dispatch( + openModal({ + modalId: ModalIdentifiers.CONFIRM_RESET_FAILED_ATTEMPTS_MODAL, + dialogType: ConfirmResetFailedAttemptsModal, + dialogProps: { + user, + onError, + onSuccess: onResetAttemptsSuccess, + }, + }), + ); + }, [user, updateUser, onError]); + const disableActivationToggle = user.auth_service === Constants.LDAP_SERVICE; const getManagedByLDAPText = (managedByLDAP: boolean) => { @@ -314,6 +333,18 @@ export function SystemUsersListAction({user, currentUser, tableId, rowIndex, onE } : {}; }; + const showResetFailedAttempts = useCallback(() => { + if (user.failed_attempts === undefined) { + return false; + } + + if (user.auth_service !== Constants.LDAP_SERVICE && user.auth_service !== '') { + return false; + } + + return true; + }, [user]); + return ( } + {showResetFailedAttempts() && ( + + } + onClick={handleResetAttemptsClick} + /> + )} {user.mfa_active && config.ServiceSettings?.EnableMultifactorAuthentication && { + return async (dispatch, getState) => { + try { + await Client4.resetFailedAttempts(userId); + } catch (error) { + dispatch(logError(error)); + return {error}; + } + + const profile = getState().entities.users.profiles[userId]; + if (profile) { + dispatch({type: UserTypes.RECEIVED_PROFILE, data: {...profile, failed_attempts: 0}}); + } + + return {data: true}; + }; +} + export function updateUserActive(userId: string, active: boolean): ActionFuncAsync { return async (dispatch, getState) => { try { diff --git a/webapp/channels/src/utils/constants.tsx b/webapp/channels/src/utils/constants.tsx index e65716527c..0cbde004d9 100644 --- a/webapp/channels/src/utils/constants.tsx +++ b/webapp/channels/src/utils/constants.tsx @@ -467,6 +467,7 @@ export const ModalIdentifiers = { SECURE_CONNECTION_ACCEPT_INVITE: 'secure_connection_accept_invite', SHARED_CHANNEL_REMOTE_INVITE: 'shared_channel_remote_invite', SHARED_CHANNEL_REMOTE_UNINVITE: 'shared_channel_remote_uninvite', + CONFIRM_RESET_FAILED_ATTEMPTS_MODAL: 'confirm_reset_failed_attempts_modal', USER_PROPERTY_FIELD_DELETE: 'user_property_field_delete', }; diff --git a/webapp/platform/client/src/client4.ts b/webapp/platform/client/src/client4.ts index 65357d336b..605241cbfa 100644 --- a/webapp/platform/client/src/client4.ts +++ b/webapp/platform/client/src/client4.ts @@ -674,6 +674,13 @@ export default class Client4 { ); }; + resetFailedAttempts = (userId: string) => { + return this.doFetch( + `${this.getUserRoute(userId)}/reset_failed_attempts`, + {method: 'post'}, + ); + }; + getKnownUsers = () => { return this.doFetch>( `${this.getUsersRoute()}/known`, diff --git a/webapp/platform/types/src/config.ts b/webapp/platform/types/src/config.ts index e39369c44a..af8601fa7a 100644 --- a/webapp/platform/types/src/config.ts +++ b/webapp/platform/types/src/config.ts @@ -709,6 +709,7 @@ export type LdapSettings = { LoginButtonColor: string; LoginButtonBorderColor: string; LoginButtonTextColor: string; + MaximumLoginAttempts: number; }; export type ComplianceSettings = { diff --git a/webapp/platform/types/src/users.ts b/webapp/platform/types/src/users.ts index 16bdf590e3..4248ff2214 100644 --- a/webapp/platform/types/src/users.ts +++ b/webapp/platform/types/src/users.ts @@ -60,6 +60,7 @@ export type UserProfile = { terms_of_service_create_at: number; remote_id?: string; status?: string; + failed_attempts?: number; custom_profile_attributes?: Record; };