From 1c325f747dc505495369461be3231ad33ece8a3d Mon Sep 17 00:00:00 2001 From: rainzm Date: Wed, 27 Jan 2021 19:30:24 +0800 Subject: [PATCH] feat(keystone): more specific error return during authentication add error class UserNotFound, UserLocked, UserDisabled and WrongPassword --- pkg/httperrors/consts.go | 13 +++++++++---- pkg/httperrors/errors.go | 8 ++++++++ pkg/keystone/driver/ldap/ldap.go | 6 ++++++ pkg/keystone/models/users.go | 6 ++++-- pkg/keystone/tokens/auth.go | 24 ++++++++++++------------ pkg/keystone/tokens/handlers.go | 2 +- 6 files changed, 40 insertions(+), 19 deletions(-) diff --git a/pkg/httperrors/consts.go b/pkg/httperrors/consts.go index 762a380e87..e2845c79ba 100644 --- a/pkg/httperrors/consts.go +++ b/pkg/httperrors/consts.go @@ -30,7 +30,6 @@ const ( ErrSpecNotFound = errors.Error("SpecNotFoundError") ErrActionNotFound = errors.Error("ActionNotFoundError") ErrTenantNotFound = errors.Error("TenantNotFoundError") - ErrUserNotFound = errors.Error("UserNotFoundError") ErrServerStatus = errors.Error("ServerStatusError") ErrInvalidStatus = errors.ErrInvalidStatus @@ -84,12 +83,15 @@ const ( ErrTooManyAttempts = errors.Error("TooManyFailedAttempts") ErrTooManyRequests = errors.Error("TooManyRequests") - ErrUserLocked = errors.Error("User Locked") - ErrUserDisabled = errors.Error("User Disabled") - ErrUnsupportedProtocol = errors.Error("UnsupportedProtocol") ErrPolicyDefinition = errors.Error("PolicyDefinitionError") + + ErrUserNotFound = errors.Error("UserNotFound") + ErrUserLocked = errors.Error("UserLocked") + ErrUserDisabled = errors.Error("UserDisabled") + ErrWrongPassword = errors.Error("WrongPassword") + ErrIncorrectUsernameOrPassword = errors.Error("IncorrectUsernameOrPassword") ) var ( @@ -166,6 +168,9 @@ var ( ErrUserLocked: 423, ErrUserDisabled: 423, + ErrWrongPassword: 401, + ErrIncorrectUsernameOrPassword: 401, + ErrPolicyDefinition: 409, } ) diff --git a/pkg/httperrors/errors.go b/pkg/httperrors/errors.go index b621514913..96e7774b28 100644 --- a/pkg/httperrors/errors.go +++ b/pkg/httperrors/errors.go @@ -207,3 +207,11 @@ func NewUnclassifiedError(msg string, params ...interface{}) *httputils.JSONClie func NewTooLargeEntityError(msg string, params ...interface{}) *httputils.JSONClientError { return httputils.NewJsonClientError(httpErrorCode[ErrTooLarge], string(ErrTooLarge), msg, params...) } + +func NewJsonClientError(err errors.Error, msg string, params ...interface{}) *httputils.JSONClientError { + code, ok := httpErrorCode[err] + if !ok { + code = 500 + } + return httputils.NewJsonClientError(code, string(err), msg, params...) +} diff --git a/pkg/keystone/driver/ldap/ldap.go b/pkg/keystone/driver/ldap/ldap.go index 60cca8f21d..c0f354dc4c 100644 --- a/pkg/keystone/driver/ldap/ldap.go +++ b/pkg/keystone/driver/ldap/ldap.go @@ -294,6 +294,12 @@ func (self *SLDAPDriver) Authenticate(ctx context.Context, ident mcclient.SAuthe ) if err != nil { log.Errorf("LDAP AUTH error: %s", err) + if errors.Cause(err) == ldaputils.ErrUserNotFound { + return nil, httperrors.ErrUserNotFound + } + if errors.Cause(err) == ldaputils.ErrUserBadCredential { + return nil, httperrors.ErrWrongPassword + } return nil, errors.Wrap(err, "Authenticate error") } diff --git a/pkg/keystone/models/users.go b/pkg/keystone/models/users.go index 32f1a33ae8..5aa7bd5677 100644 --- a/pkg/keystone/models/users.go +++ b/pkg/keystone/models/users.go @@ -17,7 +17,6 @@ package models import ( "context" "database/sql" - "fmt" "time" "yunion.io/x/jsonutils" @@ -282,6 +281,9 @@ func (manager *SUserManager) FetchUserExtended(userId, userName, domainId, domai extUser := api.SUserExtended{} err := q.First(&extUser) if err != nil { + if err == sql.ErrNoRows { + return nil, httperrors.ErrUserNotFound + } return nil, errors.Wrap(err, "query") } @@ -312,7 +314,7 @@ func localUserVerifyPassword(user *api.SUserExtended, passwd string) error { if err == nil { return nil } - return errors.Error(fmt.Sprintf("invalid password: %v", err)) + return httperrors.ErrWrongPassword } // 用户列表 diff --git a/pkg/keystone/tokens/auth.go b/pkg/keystone/tokens/auth.go index 24152e1052..2f58e5f2a1 100644 --- a/pkg/keystone/tokens/auth.go +++ b/pkg/keystone/tokens/auth.go @@ -83,7 +83,7 @@ func authUserByIdentity(ctx context.Context, ident mcclient.SAuthenticationIdent return nil, sqlchemy.ErrDuplicateEntry } else if usrCnt == 0 { log.Errorf("find no user with name %s", ident.Password.User.Name) - return nil, sqlchemy.ErrEmptyQuery + return nil, httperrors.ErrUserNotFound } else { // userCnt == 1 usr := models.SUser{} @@ -99,20 +99,11 @@ func authUserByIdentity(ctx context.Context, ident mcclient.SAuthenticationIdent usrExt, err := models.UserManager.FetchUserExtended(ident.Password.User.Id, ident.Password.User.Name, ident.Password.User.Domain.Id, ident.Password.User.Domain.Name) - if err != nil && err != sql.ErrNoRows { + if err != nil && errors.Cause(err) != httperrors.ErrUserNotFound { return nil, errors.Wrap(err, "UserManager.FetchUserExtended") } - // checn enable - if !usrExt.Enabled { - if usrExt.IsLocal && usrExt.LocalFailedAuthCount > o.Options.PasswordErrorLockCount { - // user locked - return nil, errors.Wrap(httperrors.ErrUserLocked, "please contact the administrator") - } - // user disabled - return nil, errors.Wrap(httperrors.ErrUserLocked, "please contact the administrator") - } - if err == sql.ErrNoRows { + if err != nil { // no such user locally, query domain idp domain, err := models.DomainManager.FetchDomain(ident.Password.User.Domain.Id, ident.Password.User.Domain.Name) if err != nil { @@ -124,6 +115,15 @@ func authUserByIdentity(ctx context.Context, ident mcclient.SAuthenticationIdent } idpId = mapping.IdpId } else { + // check enable + if !usrExt.Enabled { + if usrExt.IsLocal && usrExt.LocalFailedAuthCount > o.Options.PasswordErrorLockCount { + // user locked + return nil, httperrors.ErrUserLocked + } + // user disabled + return nil, httperrors.ErrUserLocked + } // user exists, query user's idp idps, err := models.IdentityProviderManager.FetchIdentityProvidersByUserId(usrExt.Id, api.PASSWORD_PROTECTED_IDPS) if err != nil { diff --git a/pkg/keystone/tokens/handlers.go b/pkg/keystone/tokens/handlers.go index b2103f9441..70c089584d 100644 --- a/pkg/keystone/tokens/handlers.go +++ b/pkg/keystone/tokens/handlers.go @@ -89,7 +89,7 @@ func authenticateTokensV3(ctx context.Context, w http.ResponseWriter, r *http.Re switch errors.Cause(err) { case sqlchemy.ErrDuplicateEntry: httperrors.ConflictError(ctx, w, "duplicate username") - case httperrors.ErrTooManyAttempts, httperrors.ErrUserNotFound, httperrors.ErrUserDisabled, httperrors.ErrUserLocked: + case httperrors.ErrTooManyAttempts, httperrors.ErrUserNotFound, httperrors.ErrUserDisabled, httperrors.ErrUserLocked, httperrors.ErrWrongPassword: httperrors.GeneralServerError(ctx, w, err) default: httperrors.UnauthorizedError(ctx, w, "unauthorized %s", err)