[MM-15767] Mask errors in login flow only explicitly (#11051)
* Explicit list of errors that should be masked for login flow * Fix unit test * fix test #2 * Use of whitelist of passed through errors; Rework error messages
Этот коммит содержится в:
Daniel Schalla
коммит произвёл
GitHub
GitHub
родитель
41774b671a
Коммит
79fb20bc1a
55
api4/user.go
55
api4/user.go
@@ -1242,13 +1242,56 @@ func sendPasswordReset(c *Context, w http.ResponseWriter, r *http.Request) {
|
||||
func login(c *Context, w http.ResponseWriter, r *http.Request) {
|
||||
// Translate all login errors to generic. MFA error being an exception, since it's required for the login flow itself
|
||||
defer func() {
|
||||
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)
|
||||
if c.Err == nil {
|
||||
return
|
||||
}
|
||||
|
||||
unmaskedErrors := []string{
|
||||
"mfa.validate_token.authenticate.app_error",
|
||||
"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.client_side_cert.certificate.app_error",
|
||||
"api.user.login.inactive.app_error",
|
||||
"api.user.login.not_verified.app_error",
|
||||
}
|
||||
|
||||
maskError := true
|
||||
|
||||
for _, unmaskedError := range unmaskedErrors {
|
||||
if c.Err.Id == unmaskedError {
|
||||
maskError = false
|
||||
}
|
||||
}
|
||||
|
||||
if !maskError {
|
||||
return
|
||||
}
|
||||
|
||||
config := c.App.Config()
|
||||
enableUsername := *config.EmailSettings.EnableSignInWithUsername
|
||||
enableEmail := *config.EmailSettings.EnableSignInWithEmail
|
||||
samlEnabled := *config.SamlSettings.Enable
|
||||
gitlabEnabled := *config.GetSSOService("gitlab").Enable
|
||||
googleEnabled := *config.GetSSOService("google").Enable
|
||||
office365Enabled := *config.GetSSOService("office365").Enable
|
||||
|
||||
if samlEnabled || gitlabEnabled || googleEnabled || office365Enabled {
|
||||
c.Err = model.NewAppError("login", "api.user.login.invalid_credentials_sso", nil, "", http.StatusUnauthorized)
|
||||
return
|
||||
}
|
||||
|
||||
if enableUsername && !enableEmail {
|
||||
c.Err = model.NewAppError("login", "api.user.login.invalid_credentials_username", nil, "", http.StatusUnauthorized)
|
||||
return
|
||||
}
|
||||
|
||||
if !enableUsername && enableEmail {
|
||||
c.Err = model.NewAppError("login", "api.user.login.invalid_credentials_email", nil, "", http.StatusUnauthorized)
|
||||
return
|
||||
}
|
||||
|
||||
c.Err = model.NewAppError("login", "api.user.login.invalid_credentials_email_username", nil, "", http.StatusUnauthorized)
|
||||
}()
|
||||
|
||||
props := model.MapFromJson(r.Body)
|
||||
|
||||
@@ -2673,7 +2673,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, "api.user.login.invalid_credentials")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials_email_username")
|
||||
})
|
||||
|
||||
t.Run("valid login", func(t *testing.T) {
|
||||
@@ -4121,6 +4121,61 @@ func TestGetUserTermsOfService(t *testing.T) {
|
||||
assert.NotEmpty(t, userTermsOfService.CreateAt)
|
||||
}
|
||||
|
||||
func TestLoginErrorMessage(t *testing.T) {
|
||||
th := Setup().InitBasic()
|
||||
defer th.TearDown()
|
||||
|
||||
_, resp := th.Client.Logout()
|
||||
CheckNoError(t, resp)
|
||||
|
||||
|
||||
// Email and Username enabled
|
||||
th.App.UpdateConfig(func(cfg *model.Config) {
|
||||
*cfg.EmailSettings.EnableSignInWithEmail = true
|
||||
*cfg.EmailSettings.EnableSignInWithUsername = true
|
||||
})
|
||||
_, resp = th.Client.Login(th.BasicUser.Email, "wrong")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials_email_username")
|
||||
|
||||
// Email enabled
|
||||
th.App.UpdateConfig(func(cfg *model.Config) {
|
||||
*cfg.EmailSettings.EnableSignInWithEmail = true
|
||||
*cfg.EmailSettings.EnableSignInWithUsername = false
|
||||
})
|
||||
_, resp = th.Client.Login(th.BasicUser.Email, "wrong")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials_email")
|
||||
|
||||
// Username enabled
|
||||
th.App.UpdateConfig(func(cfg *model.Config) {
|
||||
*cfg.EmailSettings.EnableSignInWithEmail = false
|
||||
*cfg.EmailSettings.EnableSignInWithUsername = true
|
||||
})
|
||||
_, resp = th.Client.Login(th.BasicUser.Email, "wrong")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials_username")
|
||||
|
||||
// SAML/SSO enabled
|
||||
th.App.UpdateConfig(func(cfg *model.Config) {
|
||||
*cfg.SamlSettings.Enable = true
|
||||
*cfg.SamlSettings.Verify = false
|
||||
*cfg.SamlSettings.Encrypt = false
|
||||
*cfg.SamlSettings.IdpUrl = "https://localhost/adfs/ls"
|
||||
*cfg.SamlSettings.IdpDescriptorUrl = "https://localhost/adfs/services/trust"
|
||||
*cfg.SamlSettings.AssertionConsumerServiceURL = "https://localhost/login/sso/saml"
|
||||
*cfg.SamlSettings.IdpCertificateFile = app.SamlIdpCertificateName
|
||||
*cfg.SamlSettings.PrivateKeyFile = app.SamlPrivateKeyName
|
||||
*cfg.SamlSettings.PublicCertificateFile = app.SamlPublicCertificateName
|
||||
*cfg.SamlSettings.EmailAttribute = "Email"
|
||||
*cfg.SamlSettings.UsernameAttribute = "Username"
|
||||
*cfg.SamlSettings.FirstNameAttribute = "FirstName"
|
||||
*cfg.SamlSettings.LastNameAttribute = "LastName"
|
||||
*cfg.SamlSettings.NicknameAttribute = ""
|
||||
*cfg.SamlSettings.PositionAttribute = ""
|
||||
*cfg.SamlSettings.LocaleAttribute = ""
|
||||
})
|
||||
_, resp = th.Client.Login(th.BasicUser.Email, "wrong")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials_sso")
|
||||
}
|
||||
|
||||
func TestLoginLockout(t *testing.T) {
|
||||
th := Setup().InitBasic()
|
||||
defer th.TearDown()
|
||||
@@ -4132,34 +4187,34 @@ 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.login.invalid_credentials")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials_email_username")
|
||||
_, resp = th.Client.Login(th.BasicUser.Email, "wrong")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials_email_username")
|
||||
_, resp = th.Client.Login(th.BasicUser.Email, "wrong")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials_email_username")
|
||||
_, resp = th.Client.Login(th.BasicUser.Email, "wrong")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials_email_username")
|
||||
_, resp = th.Client.Login(th.BasicUser.Email, "wrong")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials_email_username")
|
||||
|
||||
//Check if lock is active
|
||||
_, resp = th.Client.Login(th.BasicUser.Email, th.BasicUser.Password)
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials_email_username")
|
||||
|
||||
// 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.login.invalid_credentials")
|
||||
CheckErrorMessage(t, resp, "api.user.check_user_mfa.bad_code.app_error")
|
||||
_, resp = th.Client.LoginWithMFA(th.BasicUser2.Email, th.BasicUser2.Password, "000000")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials")
|
||||
CheckErrorMessage(t, resp, "api.user.check_user_mfa.bad_code.app_error")
|
||||
_, resp = th.Client.LoginWithMFA(th.BasicUser2.Email, th.BasicUser2.Password, "000000")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials")
|
||||
CheckErrorMessage(t, resp, "api.user.check_user_mfa.bad_code.app_error")
|
||||
_, resp = th.Client.LoginWithMFA(th.BasicUser2.Email, th.BasicUser2.Password, "000000")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials_email_username")
|
||||
_, resp = th.Client.LoginWithMFA(th.BasicUser2.Email, th.BasicUser2.Password, "000000")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials_email_username")
|
||||
|
||||
// Fake user has MFA disabled
|
||||
if result := <-th.Server.Store.User().UpdateMfaActive(th.BasicUser2.Id, false); result.Err != nil {
|
||||
@@ -4168,5 +4223,5 @@ func TestLoginLockout(t *testing.T) {
|
||||
|
||||
//Check if lock is active
|
||||
_, resp = th.Client.Login(th.BasicUser2.Email, th.BasicUser2.Password)
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials")
|
||||
CheckErrorMessage(t, resp, "api.user.login.invalid_credentials_email_username")
|
||||
}
|
||||
|
||||
Ссылка в новой задаче
Block a user