From c5223f9131573abe2579e9ddb72f1d8c29f5f17f Mon Sep 17 00:00:00 2001 From: rainzm Date: Tue, 19 Jan 2021 17:17:50 +0800 Subject: [PATCH] feat(keystone,apigateway): return a readable message when the user is locked or disabled --- pkg/apigateway/handler/auth.go | 2 +- pkg/apis/identity/usrext.go | 11 +++-- pkg/httperrors/consts.go | 6 +++ pkg/keystone/models/users.go | 1 + pkg/keystone/tokens/auth.go | 88 ++++++++++++++++----------------- pkg/keystone/tokens/handlers.go | 2 +- 6 files changed, 57 insertions(+), 53 deletions(-) diff --git a/pkg/apigateway/handler/auth.go b/pkg/apigateway/handler/auth.go index 9d23347396..3039300e90 100644 --- a/pkg/apigateway/handler/auth.go +++ b/pkg/apigateway/handler/auth.go @@ -309,7 +309,7 @@ func (h *AuthHandlers) doCredentialLogin(ctx context.Context, req *http.Request, if err != nil { switch httperr := err.(type) { case *httputils.JSONClientError: - if httperr.Code == 409 || httperr.Code == 429 { + if httperr.Code == 409 || httperr.Code == 429 || httperr.Code == 423 { return nil, err } } diff --git a/pkg/apis/identity/usrext.go b/pkg/apis/identity/usrext.go index 74cfb2a7fc..380df74dcf 100644 --- a/pkg/apis/identity/usrext.go +++ b/pkg/apis/identity/usrext.go @@ -31,11 +31,12 @@ type SUserExtended struct { Email string Mobile string - LocalId int - LocalName string - DomainName string - DomainEnabled bool - IsLocal bool + LocalId int + LocalName string + LocalFailedAuthCount int + DomainName string + DomainEnabled bool + IsLocal bool // IdpId string // IdpName string } diff --git a/pkg/httperrors/consts.go b/pkg/httperrors/consts.go index 4ef9fa42b9..762a380e87 100644 --- a/pkg/httperrors/consts.go +++ b/pkg/httperrors/consts.go @@ -84,6 +84,9 @@ 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") @@ -160,6 +163,9 @@ var ( ErrTooManyAttempts: 429, ErrTooManyRequests: 429, + ErrUserLocked: 423, + ErrUserDisabled: 423, + ErrPolicyDefinition: 409, } ) diff --git a/pkg/keystone/models/users.go b/pkg/keystone/models/users.go index 798a1ec35f..f6803610c4 100644 --- a/pkg/keystone/models/users.go +++ b/pkg/keystone/models/users.go @@ -254,6 +254,7 @@ func (manager *SUserManager) FetchUserExtended(userId, userName, domainId, domai users.Field("is_system_account"), localUsers.Field("id", "local_id"), localUsers.Field("name", "local_name"), + localUsers.Field("failed_auth_count", "local_failed_auth_count"), domains.Field("name", "domain_name"), domains.Field("enabled", "domain_enabled"), // idmappings.Field("domain_id", "idp_id"), diff --git a/pkg/keystone/tokens/auth.go b/pkg/keystone/tokens/auth.go index 89bd49abb5..24152e1052 100644 --- a/pkg/keystone/tokens/auth.go +++ b/pkg/keystone/tokens/auth.go @@ -19,7 +19,6 @@ import ( "database/sql" "time" - "yunion.io/x/jsonutils" "yunion.io/x/log" "yunion.io/x/pkg/errors" "yunion.io/x/pkg/utils" @@ -30,6 +29,7 @@ import ( "yunion.io/x/onecloud/pkg/keystone/driver" "yunion.io/x/onecloud/pkg/keystone/models" "yunion.io/x/onecloud/pkg/keystone/options" + o "yunion.io/x/onecloud/pkg/keystone/options" "yunion.io/x/onecloud/pkg/keystone/saml" "yunion.io/x/onecloud/pkg/mcclient" "yunion.io/x/onecloud/pkg/util/s3auth" @@ -73,7 +73,7 @@ func authUserByIdentity(ctx context.Context, ident mcclient.SAuthenticationIdent } if len(ident.Password.User.Name) > 0 && len(ident.Password.User.Id) == 0 && len(ident.Password.User.Domain.Id) == 0 && len(ident.Password.User.Domain.Name) == 0 { // no user domain specified, try to find user domain - q := models.UserManager.Query().Equals("name", ident.Password.User.Name).IsTrue("enabled") + q := models.UserManager.Query().Equals("name", ident.Password.User.Name) usrCnt, err := q.CountWithError() if err != nil { return nil, errors.Wrap(err, "Query user by name") @@ -93,51 +93,48 @@ func authUserByIdentity(ctx context.Context, ident mcclient.SAuthenticationIdent return nil, errors.Wrap(err, "Query user") } ident.Password.User.Domain.Id = usr.DomainId - idps, err := models.IdentityProviderManager.FetchIdentityProvidersByUserId(usr.Id, api.PASSWORD_PROTECTED_IDPS) - if err != nil { - return nil, errors.Wrap(err, "IdentityProviderManager.FetchIdentityProvidersByUserId") - } - log.Debugf("user %s idps: %s", ident.Password.User.Name, jsonutils.Marshal(idps)) - if len(idps) == 0 { - idpId = api.DEFAULT_IDP_ID - } else if len(idps) == 1 { - idpId = idps[0].Id - } else { - log.Errorf("find %d password idps for user %s", len(idps), ident.Password.User.Name) - return nil, sqlchemy.ErrDuplicateEntry - } - } - } else { - 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 { - return nil, errors.Wrap(err, "UserManager.FetchUserExtended") + ident.Password.User.Id = usr.Id } + } - if err == sql.ErrNoRows { - // 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 { - return nil, errors.Wrap(err, "DomainManager.FetchDomain") - } - mapping, err := models.IdmappingManager.FetchFirstEntity(domain.Id, api.IdMappingEntityDomain) - if err != nil { - return nil, errors.Wrap(err, "IdmappingManager.FetchEntity") - } - idpId = mapping.IdpId + 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 { + 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 { + // 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 { + return nil, errors.Wrap(err, "DomainManager.FetchDomain") + } + mapping, err := models.IdmappingManager.FetchFirstEntity(domain.Id, api.IdMappingEntityDomain) + if err != nil { + return nil, errors.Wrap(err, "IdmappingManager.FetchEntity") + } + idpId = mapping.IdpId + } else { + // user exists, query user's idp + idps, err := models.IdentityProviderManager.FetchIdentityProvidersByUserId(usrExt.Id, api.PASSWORD_PROTECTED_IDPS) + if err != nil { + return nil, errors.Wrap(err, "IdentityProviderManager.FetchIdentityProvidersByUserId") + } + if len(idps) == 0 { + idpId = api.DEFAULT_IDP_ID + } else if len(idps) == 1 { + idpId = idps[0].Id } else { - // user exists, query user's idp - idps, err := models.IdentityProviderManager.FetchIdentityProvidersByUserId(usrExt.Id, api.PASSWORD_PROTECTED_IDPS) - if err != nil { - return nil, errors.Wrap(err, "IdentityProviderManager.FetchIdentityProvidersByUserId") - } - if len(idps) == 0 { - idpId = api.DEFAULT_IDP_ID - } else if len(idps) == 1 { - idpId = idps[0].Id - } else { - return nil, sqlchemy.ErrDuplicateEntry - } + return nil, sqlchemy.ErrDuplicateEntry } } @@ -177,7 +174,6 @@ func authUserByIdentity(ctx context.Context, ident mcclient.SAuthenticationIdent if idp.Status == api.IdentityDriverStatusDisconnected { idp.MarkConnected(ctx, models.GetDefaultAdminCred()) } - return usr, nil } @@ -429,7 +425,7 @@ func AuthenticateV3(ctx context.Context, input mcclient.SAuthenticationInputV3) // auth by other methods, e.g. password , etc... user, err = authUserByIdentityV3(ctx, input) if err != nil { - return nil, errors.Wrap(err, "authUserByIdentityV3") + return nil, err } } diff --git a/pkg/keystone/tokens/handlers.go b/pkg/keystone/tokens/handlers.go index 9137820307..b2103f9441 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: + case httperrors.ErrTooManyAttempts, httperrors.ErrUserNotFound, httperrors.ErrUserDisabled, httperrors.ErrUserLocked: httperrors.GeneralServerError(ctx, w, err) default: httperrors.UnauthorizedError(ctx, w, "unauthorized %s", err)