You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by GitBox <gi...@apache.org> on 2020/06/28 07:27:59 UTC

[GitHub] [servicecomb-service-center] tianxiaoliang opened a new pull request #652: able to change password

tianxiaoliang opened a new pull request #652:
URL: https://github.com/apache/servicecomb-service-center/pull/652


   Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SCB) filed for the change (usually before you start working on it).  Trivial changes like typos do not require a JIRA issue.  Your pull request should address just this issue, without pulling in other changes.
    - [ ] Each commit in the pull request should have a meaningful subject line and body.
    - [ ] Format the pull request title like `[SCB-XXX] Fixes bug in ApproximateQuantiles`, where you replace `SCB-XXX` with the appropriate JIRA issue.
    - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
    - [ ] Run `go build` `go test` `go fmt` `go vet` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
    - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   ---
   
   新增:
   - 密码可修改
   - role字段
   变更:
   - 私钥不再入库,从文件系统初始化到内存
   - root账户名不允许自定义,只允许定义初始密码
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [servicecomb-service-center] tianxiaoliang merged pull request #652: able to change password

Posted by GitBox <gi...@apache.org>.
tianxiaoliang merged pull request #652:
URL: https://github.com/apache/servicecomb-service-center/pull/652


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [servicecomb-service-center] coveralls edited a comment on pull request #652: able to change password

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #652:
URL: https://github.com/apache/servicecomb-service-center/pull/652#issuecomment-650715528


   
   [![Coverage Status](https://coveralls.io/builds/31732295/badge)](https://coveralls.io/builds/31732295)
   
   Coverage decreased (-0.005%) to 61.091% when pulling **b4f32d996427c896cf6fda4ee800b283f2809be2 on tianxiaoliang:dev** into **bd88a191b2177631c0c51202462718727e9b5748 on apache:master**.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [servicecomb-service-center] coveralls commented on pull request #652: able to change password

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #652:
URL: https://github.com/apache/servicecomb-service-center/pull/652#issuecomment-650715528


   
   [![Coverage Status](https://coveralls.io/builds/31730346/badge)](https://coveralls.io/builds/31730346)
   
   Coverage increased (+0.01%) to 61.107% when pulling **964cb021a068a0bee4a6e9bb2115c0596448991c on tianxiaoliang:dev** into **bd88a191b2177631c0c51202462718727e9b5748 on apache:master**.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [servicecomb-service-center] coveralls edited a comment on pull request #652: able to change password

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #652:
URL: https://github.com/apache/servicecomb-service-center/pull/652#issuecomment-650715528


   
   [![Coverage Status](https://coveralls.io/builds/31732353/badge)](https://coveralls.io/builds/31732353)
   
   Coverage increased (+0.04%) to 61.132% when pulling **c95c137b9fa5f411724d2c7879aeb4cdda3dd2ad on tianxiaoliang:dev** into **bd88a191b2177631c0c51202462718727e9b5748 on apache:master**.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [servicecomb-service-center] aseTo2016 commented on a change in pull request #652: able to change password

Posted by GitBox <gi...@apache.org>.
aseTo2016 commented on a change in pull request #652:
URL: https://github.com/apache/servicecomb-service-center/pull/652#discussion_r446645941



##########
File path: server/service/rbac/rbac.go
##########
@@ -49,43 +55,52 @@ func Init() {
 	if err != nil {
 		log.Fatal("can not enable auth module", err)
 	}
-	admin := archaius.GetString(InitRoot, "")
-	if admin == "" {
-		log.Fatal("can not enable rbac, root is empty", nil)
-		return
-	}
-	accountExist, err := dao.AccountExist(context.Background(), admin)
+	accountExist, err := dao.AccountExist(context.Background(), RootName)
 	if err != nil {
 		log.Fatal("can not enable auth module", err)
 	}
 	if !accountExist {
-		initFirstTime(admin)
+		initFirstTime(RootName)
 	}
-	overrideSecretKey()
+	readPrivateKey()
 	readPublicKey()
 	log.Info("rbac is enabled")
 }
 
+//readPublicKey read key to memory
+func readPrivateKey() {
+	pf := beego.AppConfig.String("rbac_rsa_private_key_file")
+	// 打开文件
+	data, err := ioutil.ReadFile(pf)
+	if err != nil {
+		log.Fatal("can not read private key", err)
+		return
+	}
+	archaius.Set("rbac_private_key", string(data))
+	log.Info("read private key success %s")
+}
+
 //readPublicKey read key to memory
 func readPublicKey() {
-	pf := beego.AppConfig.String("rbac_rsa_pub_key_file")
+	pf := beego.AppConfig.String("rbac_rsa_public_key_file")
 	// 打开文件
-	fp, err := os.Open(pf)
+	f, err := os.Open(pf)
 	if err != nil {
 		log.Fatal("can not find public key", err)
 		return
 	}
-	defer fp.Close()
+	defer f.Close()
 	buf := make([]byte, 1024)
 	for {
 		// 循环读取文件
-		_, err := fp.Read(buf)
+		_, err := f.Read(buf)

Review comment:
       go提供了这个方法,没有必要自己写一个




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [servicecomb-service-center] aseTo2016 commented on a change in pull request #652: able to change password

Posted by GitBox <gi...@apache.org>.
aseTo2016 commented on a change in pull request #652:
URL: https://github.com/apache/servicecomb-service-center/pull/652#discussion_r446638138



##########
File path: server/rest/controller/v4/auth_resource.go
##########
@@ -37,9 +37,55 @@ type AuthResource struct {
 func (r *AuthResource) URLPatterns() []rest.Route {
 	return []rest.Route{
 		{http.MethodPost, "/v4/token", r.Login},
+		{http.MethodPut, "/v4/account-password", r.ChangePassword},
 	}
 }
+func (r *AuthResource) ChangePassword(w http.ResponseWriter, req *http.Request) {
+	body, err := ioutil.ReadAll(req.Body)
+	if err != nil {
+		log.Error("read body err", err)
+		controller.WriteError(w, scerror.ErrInternal, err.Error())
+		return
+	}
+	a := &model.Account{}
+	if err = json.Unmarshal(body, a); err != nil {
+		log.Error("json err", err)
+		controller.WriteError(w, scerror.ErrInvalidParams, err.Error())
+		return
+	}
+	if a.Password == "" {
+		controller.WriteError(w, scerror.ErrInvalidParams, "new password is empty")
+		return
+	}
+	claims := req.Context().Value("accountInfo")
+	m, ok := claims.(map[string]interface{})
+	if !ok {
+		log.Errorf(err, "claims convert failed: %T", claims)
+		controller.WriteError(w, scerror.ErrInvalidParams, "wrong account info")
+		return
+	}
+	accountNameI := m[rbac.ClaimsUser]
+	changer, ok := accountNameI.(string)
+	if !ok {
+		log.Error("can not convert claim", nil)
+		controller.WriteError(w, scerror.ErrInvalidParams, err.Error())

Review comment:
       err为nil,打印会panic

##########
File path: server/handler/auth/auth.go
##########
@@ -63,15 +64,22 @@ func (h *Handler) Handle(i *chain.Invocation) {
 	}
 	to := s[1]
 	//TODO rbac
-	_, err := authr.Authenticate(i.Context(), to)
-	if err == nil {
-		log.Info("user access")
-		i.Next()
+	claims, err := authr.Authenticate(i.Context(), to)
+	if err != nil {
+		log.Errorf(err, "authenticate request failed, %s %s", req.Method, req.RequestURI)
+		controller.WriteError(w, scerr.ErrUnauthorized, err.Error())
+		i.Fail(nil)
 		return
 	}
-	log.Errorf(err, "authenticate request failed, %s %s", req.Method, req.RequestURI)
-	controller.WriteError(w, scerr.ErrUnauthorized, err.Error())
-	i.Fail(nil)
+	log.Info("user access")
+	req2 := req.WithContext(context.WithValue(req.Context(), "accountInfo", claims))
+
+	*req = *req2
+	log.Infof("%s", req.Context())

Review comment:
       context是否需要打印?是否有敏感信息

##########
File path: server/service/rbac/rbac.go
##########
@@ -49,43 +55,52 @@ func Init() {
 	if err != nil {
 		log.Fatal("can not enable auth module", err)
 	}
-	admin := archaius.GetString(InitRoot, "")
-	if admin == "" {
-		log.Fatal("can not enable rbac, root is empty", nil)
-		return
-	}
-	accountExist, err := dao.AccountExist(context.Background(), admin)
+	accountExist, err := dao.AccountExist(context.Background(), RootName)
 	if err != nil {
 		log.Fatal("can not enable auth module", err)
 	}
 	if !accountExist {
-		initFirstTime(admin)
+		initFirstTime(RootName)
 	}
-	overrideSecretKey()
+	readPrivateKey()
 	readPublicKey()
 	log.Info("rbac is enabled")
 }
 
+//readPublicKey read key to memory
+func readPrivateKey() {
+	pf := beego.AppConfig.String("rbac_rsa_private_key_file")
+	// 打开文件
+	data, err := ioutil.ReadFile(pf)
+	if err != nil {
+		log.Fatal("can not read private key", err)
+		return
+	}
+	archaius.Set("rbac_private_key", string(data))
+	log.Info("read private key success %s")

Review comment:
       %s后面没有值

##########
File path: server/service/rbac/rbac.go
##########
@@ -49,43 +55,52 @@ func Init() {
 	if err != nil {
 		log.Fatal("can not enable auth module", err)
 	}
-	admin := archaius.GetString(InitRoot, "")
-	if admin == "" {
-		log.Fatal("can not enable rbac, root is empty", nil)
-		return
-	}
-	accountExist, err := dao.AccountExist(context.Background(), admin)
+	accountExist, err := dao.AccountExist(context.Background(), RootName)
 	if err != nil {
 		log.Fatal("can not enable auth module", err)
 	}
 	if !accountExist {
-		initFirstTime(admin)
+		initFirstTime(RootName)
 	}
-	overrideSecretKey()
+	readPrivateKey()
 	readPublicKey()
 	log.Info("rbac is enabled")
 }
 
+//readPublicKey read key to memory
+func readPrivateKey() {
+	pf := beego.AppConfig.String("rbac_rsa_private_key_file")
+	// 打开文件
+	data, err := ioutil.ReadFile(pf)
+	if err != nil {
+		log.Fatal("can not read private key", err)
+		return
+	}
+	archaius.Set("rbac_private_key", string(data))
+	log.Info("read private key success %s")
+}
+
 //readPublicKey read key to memory
 func readPublicKey() {
-	pf := beego.AppConfig.String("rbac_rsa_pub_key_file")
+	pf := beego.AppConfig.String("rbac_rsa_public_key_file")
 	// 打开文件
-	fp, err := os.Open(pf)
+	f, err := os.Open(pf)
 	if err != nil {
 		log.Fatal("can not find public key", err)
 		return
 	}
-	defer fp.Close()
+	defer f.Close()
 	buf := make([]byte, 1024)
 	for {
 		// 循环读取文件
-		_, err := fp.Read(buf)
+		_, err := f.Read(buf)

Review comment:
       建议使用ioutil.ReadFile()

##########
File path: server/service/rbac/rbac.go
##########
@@ -49,43 +55,52 @@ func Init() {
 	if err != nil {
 		log.Fatal("can not enable auth module", err)
 	}
-	admin := archaius.GetString(InitRoot, "")
-	if admin == "" {
-		log.Fatal("can not enable rbac, root is empty", nil)
-		return
-	}
-	accountExist, err := dao.AccountExist(context.Background(), admin)
+	accountExist, err := dao.AccountExist(context.Background(), RootName)
 	if err != nil {
 		log.Fatal("can not enable auth module", err)
 	}
 	if !accountExist {
-		initFirstTime(admin)
+		initFirstTime(RootName)
 	}
-	overrideSecretKey()
+	readPrivateKey()
 	readPublicKey()
 	log.Info("rbac is enabled")
 }
 
+//readPublicKey read key to memory
+func readPrivateKey() {
+	pf := beego.AppConfig.String("rbac_rsa_private_key_file")
+	// 打开文件
+	data, err := ioutil.ReadFile(pf)
+	if err != nil {
+		log.Fatal("can not read private key", err)
+		return
+	}
+	archaius.Set("rbac_private_key", string(data))
+	log.Info("read private key success %s")
+}
+
 //readPublicKey read key to memory
 func readPublicKey() {
-	pf := beego.AppConfig.String("rbac_rsa_pub_key_file")
+	pf := beego.AppConfig.String("rbac_rsa_public_key_file")

Review comment:
       rbac_rsa_public_key_file有两个地方引用,建议写成const

##########
File path: server/service/rbac/rbac.go
##########
@@ -158,3 +158,52 @@ func GetPrivateKey(ctx context.Context) (*rsa.PrivateKey, error) {
 	}
 	return p, nil
 }
+
+func ChangePassword(ctx context.Context, changerRole, changerName string, a *model.Account) error {
+	if a.Name != "" {
+		if changerRole != model.RoleAdmin { //need to check password mismatch. but admin role can change any user password without supply current password
+			log.Error("can not change other account pwd", nil)
+			return ErrInputChangeAccount
+		} else {

Review comment:
       可以删除else,写成return changePasswordForcibly(ctx, a.Name, a.Password)

##########
File path: server/rest/controller/v4/auth_resource.go
##########
@@ -37,9 +37,55 @@ type AuthResource struct {
 func (r *AuthResource) URLPatterns() []rest.Route {
 	return []rest.Route{
 		{http.MethodPost, "/v4/token", r.Login},
+		{http.MethodPut, "/v4/account-password", r.ChangePassword},
 	}
 }
+func (r *AuthResource) ChangePassword(w http.ResponseWriter, req *http.Request) {
+	body, err := ioutil.ReadAll(req.Body)
+	if err != nil {
+		log.Error("read body err", err)
+		controller.WriteError(w, scerror.ErrInternal, err.Error())
+		return
+	}
+	a := &model.Account{}
+	if err = json.Unmarshal(body, a); err != nil {
+		log.Error("json err", err)
+		controller.WriteError(w, scerror.ErrInvalidParams, err.Error())
+		return
+	}
+	if a.Password == "" {
+		controller.WriteError(w, scerror.ErrInvalidParams, "new password is empty")
+		return
+	}
+	claims := req.Context().Value("accountInfo")
+	m, ok := claims.(map[string]interface{})
+	if !ok {
+		log.Errorf(err, "claims convert failed: %T", claims)
+		controller.WriteError(w, scerror.ErrInvalidParams, "wrong account info")
+		return
+	}
+	accountNameI := m[rbac.ClaimsUser]
+	changer, ok := accountNameI.(string)
+	if !ok {
+		log.Error("can not convert claim", nil)
+		controller.WriteError(w, scerror.ErrInvalidParams, err.Error())
+		return
+	}
+	roleI := m[rbac.ClaimsRole]
+	role, ok := roleI.(string)
+	if !ok {
+		log.Error("can not convert claim", nil)
+		controller.WriteError(w, scerror.ErrInvalidParams, err.Error())

Review comment:
       同上




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [servicecomb-service-center] coveralls edited a comment on pull request #652: able to change password

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #652:
URL: https://github.com/apache/servicecomb-service-center/pull/652#issuecomment-650715528


   
   [![Coverage Status](https://coveralls.io/builds/31732010/badge)](https://coveralls.io/builds/31732010)
   
   Coverage decreased (-0.07%) to 61.027% when pulling **20111471c3b94c578673a0fb0b084acd889a6245 on tianxiaoliang:dev** into **bd88a191b2177631c0c51202462718727e9b5748 on apache:master**.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [servicecomb-service-center] tianxiaoliang commented on a change in pull request #652: able to change password

Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #652:
URL: https://github.com/apache/servicecomb-service-center/pull/652#discussion_r446641160



##########
File path: server/service/rbac/rbac.go
##########
@@ -49,43 +55,52 @@ func Init() {
 	if err != nil {
 		log.Fatal("can not enable auth module", err)
 	}
-	admin := archaius.GetString(InitRoot, "")
-	if admin == "" {
-		log.Fatal("can not enable rbac, root is empty", nil)
-		return
-	}
-	accountExist, err := dao.AccountExist(context.Background(), admin)
+	accountExist, err := dao.AccountExist(context.Background(), RootName)
 	if err != nil {
 		log.Fatal("can not enable auth module", err)
 	}
 	if !accountExist {
-		initFirstTime(admin)
+		initFirstTime(RootName)
 	}
-	overrideSecretKey()
+	readPrivateKey()
 	readPublicKey()
 	log.Info("rbac is enabled")
 }
 
+//readPublicKey read key to memory
+func readPrivateKey() {
+	pf := beego.AppConfig.String("rbac_rsa_private_key_file")
+	// 打开文件
+	data, err := ioutil.ReadFile(pf)
+	if err != nil {
+		log.Fatal("can not read private key", err)
+		return
+	}
+	archaius.Set("rbac_private_key", string(data))
+	log.Info("read private key success %s")
+}
+
 //readPublicKey read key to memory
 func readPublicKey() {
-	pf := beego.AppConfig.String("rbac_rsa_pub_key_file")
+	pf := beego.AppConfig.String("rbac_rsa_public_key_file")
 	// 打开文件
-	fp, err := os.Open(pf)
+	f, err := os.Open(pf)
 	if err != nil {
 		log.Fatal("can not find public key", err)
 		return
 	}
-	defer fp.Close()
+	defer f.Close()
 	buf := make([]byte, 1024)
 	for {
 		// 循环读取文件
-		_, err := fp.Read(buf)
+		_, err := f.Read(buf)

Review comment:
       这个理由是?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org