From 3b4ea1f02e17f3d837d9e41a1f7f7a8c1b876416 Mon Sep 17 00:00:00 2001
From: Qiu Jian <qiujian@yunionyun.com>
Date: Thu, 14 Nov 2019 12:00:36 +0800
Subject: [PATCH] fix: check password complexity even if password length is
 zero

---
 cmd/climc/shell/users.go         | 46 ++++++++++++++++----------------
 pkg/keystone/models/passwords.go | 13 +++++++--
 pkg/keystone/models/users.go     | 13 ++++-----
 3 files changed, 41 insertions(+), 31 deletions(-)

diff --git a/cmd/climc/shell/users.go b/cmd/climc/shell/users.go
index fef2fe0f2a..60a6535eac 100644
--- a/cmd/climc/shell/users.go
+++ b/cmd/climc/shell/users.go
@@ -149,15 +149,15 @@ func init() {
 	})
 
 	type UserCreateOptions struct {
-		NAME        string `help:"Name of the new user"`
-		Domain      string `help:"Domain"`
-		Desc        string `help:"Description"`
-		Password    string `help:"Password"`
-		Displayname string `help:"Displayname"`
-		Email       string `help:"Email"`
-		Mobile      string `help:"Mobile"`
-		Enabled     bool   `help:"Enabled"`
-		Disabled    bool   `help:"Disabled"`
+		NAME        string  `help:"Name of the new user"`
+		Domain      string  `help:"Domain"`
+		Desc        string  `help:"Description"`
+		Password    *string `help:"Password"`
+		Displayname string  `help:"Displayname"`
+		Email       string  `help:"Email"`
+		Mobile      string  `help:"Mobile"`
+		Enabled     bool    `help:"Enabled"`
+		Disabled    bool    `help:"Disabled"`
 
 		// DefaultProject string `help:"Default project"`
 		SystemAccount bool `help:"is a system account?"`
@@ -174,8 +174,8 @@ func init() {
 			}
 			params.Add(jsonutils.NewString(domainId), "domain_id")
 		}
-		if len(args.Password) > 0 {
-			params.Add(jsonutils.NewString(args.Password), "password")
+		if args.Password != nil {
+			params.Add(jsonutils.NewString(*args.Password), "password")
 		}
 		if len(args.Displayname) > 0 {
 			params.Add(jsonutils.NewString(args.Displayname), "displayname")
@@ -222,16 +222,16 @@ func init() {
 	})
 
 	type UserUpdateOptions struct {
-		ID          string `help:"ID or name of the user"`
-		Domain      string `help:"Domain"`
-		Name        string `help:"New name of the user"`
-		Password    string `help:"New password"`
-		Desc        string `help:"Description"`
-		Displayname string `help:"Displayname"`
-		Email       string `help:"Email"`
-		Mobile      string `help:"Mobile"`
-		Enabled     bool   `help:"Enabled"`
-		Disabled    bool   `help:"Disabled"`
+		ID          string  `help:"ID or name of the user"`
+		Domain      string  `help:"Domain"`
+		Name        string  `help:"New name of the user"`
+		Password    *string `help:"New password"`
+		Desc        string  `help:"Description"`
+		Displayname string  `help:"Displayname"`
+		Email       string  `help:"Email"`
+		Mobile      string  `help:"Mobile"`
+		Enabled     bool    `help:"Enabled"`
+		Disabled    bool    `help:"Disabled"`
 
 		SystemAccount    bool `help:"Turn on is_system_account"`
 		NotSystemAccount bool `help:"Turn off is_system_account"`
@@ -262,8 +262,8 @@ func init() {
 		if len(args.Name) > 0 {
 			params.Add(jsonutils.NewString(args.Name), "name")
 		}
-		if len(args.Password) > 0 {
-			params.Add(jsonutils.NewString(args.Password), "password")
+		if args.Password != nil {
+			params.Add(jsonutils.NewString(*args.Password), "password")
 		}
 		if len(args.Displayname) > 0 {
 			params.Add(jsonutils.NewString(args.Displayname), "displayname")
diff --git a/pkg/keystone/models/passwords.go b/pkg/keystone/models/passwords.go
index f10db6d32c..528b0941a4 100644
--- a/pkg/keystone/models/passwords.go
+++ b/pkg/keystone/models/passwords.go
@@ -24,6 +24,7 @@ import (
 	"yunion.io/x/sqlchemy"
 
 	"yunion.io/x/onecloud/pkg/cloudcommon/db"
+	"yunion.io/x/onecloud/pkg/httperrors"
 	o "yunion.io/x/onecloud/pkg/keystone/options"
 	"yunion.io/x/onecloud/pkg/util/seclib2"
 )
@@ -110,9 +111,17 @@ func (manager *SPasswordManager) fetchByLocaluserId(localUserId int) ([]SPasswor
 	return passes, nil
 }
 
-func (manager *SPasswordManager) validatePassword(localUserId int, password string) error {
+func validatePasswordComplexity(password string) error {
 	if o.Options.PasswordMinimalLength > 0 && len(password) < o.Options.PasswordMinimalLength {
-		return errors.Error("too simple password")
+		return errors.Wrap(httperrors.ErrWeakPassword, "too simple password")
+	}
+	return nil
+}
+
+func (manager *SPasswordManager) validatePassword(localUserId int, password string) error {
+	err := validatePasswordComplexity(password)
+	if err != nil {
+		return errors.Wrap(err, "validatePasswordComplexity")
 	}
 	if o.Options.PasswordUniqueHistoryCheck > 0 {
 		shaPass := shaPassword(password)
diff --git a/pkg/keystone/models/users.go b/pkg/keystone/models/users.go
index 183b8c3cd8..29afb53604 100644
--- a/pkg/keystone/models/users.go
+++ b/pkg/keystone/models/users.go
@@ -389,10 +389,11 @@ func (manager *SUserManager) FilterByHiddenSystemAttributes(q *sqlchemy.SQuery,
 }
 
 func (manager *SUserManager) ValidateCreateData(ctx context.Context, userCred mcclient.TokenCredential, ownerId mcclient.IIdentityProvider, query jsonutils.JSONObject, data *jsonutils.JSONDict) (*jsonutils.JSONDict, error) {
-	passwd, _ := data.GetString("password")
-	if len(passwd) > 0 {
-		if o.Options.PasswordMinimalLength > 0 && len(passwd) < o.Options.PasswordMinimalLength {
-			return nil, errors.Error("too simple password")
+	if data.Contains("password") {
+		passwd, _ := data.GetString("password")
+		err := validatePasswordComplexity(passwd)
+		if err != nil {
+			return nil, errors.Wrap(err, "validatePasswordComplexity")
 		}
 	}
 	return manager.SEnabledIdentityBaseResourceManager.ValidateCreateData(ctx, userCred, ownerId, query, data)
@@ -418,8 +419,8 @@ func (user *SUser) ValidateUpdateData(ctx context.Context, userCred mcclient.Tok
 			}
 		}
 	}
-	passwd, _ := data.GetString("password")
-	if len(passwd) > 0 {
+	if data.Contains("password") {
+		passwd, _ := data.GetString("password")
 		usrExt, err := UserManager.FetchUserExtended(user.Id, "", "", "")
 		if err != nil {
 			return nil, errors.Wrap(err, "UserManager.FetchUserExtended")