From f3801d7db517de9316a59ce3ba2771039f08314d Mon Sep 17 00:00:00 2001 From: Daniel Schalla Date: Tue, 28 May 2019 20:26:02 +0200 Subject: [PATCH] Dont expose any information about the user status on login failure (#10925) --- api4/user.go | 9 ++++++--- api4/user_test.go | 37 +++++++++++++++++++++++++------------ 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/api4/user.go b/api4/user.go index 3063a7fb13..0241b983ab 100644 --- a/api4/user.go +++ b/api4/user.go @@ -1240,10 +1240,13 @@ func sendPasswordReset(c *Context, w http.ResponseWriter, r *http.Request) { } func login(c *Context, w http.ResponseWriter, r *http.Request) { - // For hardened mode, translate all login errors to generic. MFA error being an exception, since it's required for - // the login flow itself. + // Translate all login errors to generic. MFA error being an exception, since it's required for the login flow itself defer func() { - if *c.App.Config().ServiceSettings.ExperimentalEnableHardenedMode && c.Err != nil && c.Err.Id != "mfa.validate_token.authenticate.app_error" { + if c.Err != nil && + c.Err.Id != "mfa.validate_token.authenticate.app_error" && + c.Err.Id != "api.user.login.blank_pwd.app_error" && + c.Err.Id != "api.user.login.bot_login_forbidden.app_error" && + c.Err.Id != "api.user.login.client_side_cert.certificate.app_error" { c.Err = model.NewAppError("login", "api.user.login.invalid_credentials", nil, "", http.StatusUnauthorized) } }() diff --git a/api4/user_test.go b/api4/user_test.go index 104f70f8b2..e1017c431b 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -2655,7 +2655,7 @@ func TestLogin(t *testing.T) { t.Run("unknown user", func(t *testing.T) { _, resp := th.Client.Login("unknown", th.BasicUser.Password) - CheckErrorMessage(t, resp, "store.sql_user.get_for_login.app_error") + CheckErrorMessage(t, resp, "api.user.login.invalid_credentials") }) t.Run("valid login", func(t *testing.T) { @@ -2764,7 +2764,7 @@ func TestCBALogin(t *testing.T) { th.Client.Logout() th.Client.HttpHeader["X-SSL-Client-Cert-Subject-DN"] = "C=US, ST=Maryland, L=Pasadena, O=Brent Baccala, OU=FreeSoft, CN=www.freesoft.org/emailAddress=mis_match" + th.BasicUser.Email _, resp := th.Client.Login(th.BasicUser.Email, "") - CheckBadRequestStatus(t, resp) + CheckUnauthorizedStatus(t, resp) }) t.Run("successful cba login", func(t *testing.T) { @@ -4114,28 +4114,41 @@ func TestLoginLockout(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableMultifactorAuthentication = true }) _, resp = th.Client.Login(th.BasicUser.Email, "wrong") - CheckErrorMessage(t, resp, "api.user.check_user_password.invalid.app_error") + CheckErrorMessage(t, resp, "api.user.login.invalid_credentials") _, resp = th.Client.Login(th.BasicUser.Email, "wrong") - CheckErrorMessage(t, resp, "api.user.check_user_password.invalid.app_error") + CheckErrorMessage(t, resp, "api.user.login.invalid_credentials") _, resp = th.Client.Login(th.BasicUser.Email, "wrong") - CheckErrorMessage(t, resp, "api.user.check_user_password.invalid.app_error") + CheckErrorMessage(t, resp, "api.user.login.invalid_credentials") _, resp = th.Client.Login(th.BasicUser.Email, "wrong") - CheckErrorMessage(t, resp, "api.user.check_user_login_attempts.too_many.app_error") + CheckErrorMessage(t, resp, "api.user.login.invalid_credentials") _, resp = th.Client.Login(th.BasicUser.Email, "wrong") - CheckErrorMessage(t, resp, "api.user.check_user_login_attempts.too_many.app_error") + CheckErrorMessage(t, resp, "api.user.login.invalid_credentials") + + //Check if lock is active + _, resp = th.Client.Login(th.BasicUser.Email, th.BasicUser.Password) + CheckErrorMessage(t, resp, "api.user.login.invalid_credentials") // Fake user has MFA enabled if result := <-th.Server.Store.User().UpdateMfaActive(th.BasicUser2.Id, true); result.Err != nil { t.Fatal(result.Err) } _, resp = th.Client.LoginWithMFA(th.BasicUser2.Email, th.BasicUser2.Password, "000000") - CheckErrorMessage(t, resp, "api.user.check_user_mfa.bad_code.app_error") + CheckErrorMessage(t, resp, "api.user.login.invalid_credentials") _, resp = th.Client.LoginWithMFA(th.BasicUser2.Email, th.BasicUser2.Password, "000000") - CheckErrorMessage(t, resp, "api.user.check_user_mfa.bad_code.app_error") + CheckErrorMessage(t, resp, "api.user.login.invalid_credentials") _, resp = th.Client.LoginWithMFA(th.BasicUser2.Email, th.BasicUser2.Password, "000000") - CheckErrorMessage(t, resp, "api.user.check_user_mfa.bad_code.app_error") + CheckErrorMessage(t, resp, "api.user.login.invalid_credentials") _, resp = th.Client.LoginWithMFA(th.BasicUser2.Email, th.BasicUser2.Password, "000000") - CheckErrorMessage(t, resp, "api.user.check_user_login_attempts.too_many.app_error") + CheckErrorMessage(t, resp, "api.user.login.invalid_credentials") _, resp = th.Client.LoginWithMFA(th.BasicUser2.Email, th.BasicUser2.Password, "000000") - CheckErrorMessage(t, resp, "api.user.check_user_login_attempts.too_many.app_error") + CheckErrorMessage(t, resp, "api.user.login.invalid_credentials") + + // Fake user has MFA disabled + if result := <-th.Server.Store.User().UpdateMfaActive(th.BasicUser2.Id, false); result.Err != nil { + t.Fatal(result.Err) + } + + //Check if lock is active + _, resp = th.Client.Login(th.BasicUser2.Email, th.BasicUser2.Password) + CheckErrorMessage(t, resp, "api.user.login.invalid_credentials") }