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 2021/05/22 02:55:47 UTC
[GitHub] [servicecomb-service-center] humingcheng opened a new pull request #1000: Realize all rbac APIs.
humingcheng opened a new pull request #1000:
URL: https://github.com/apache/servicecomb-service-center/pull/1000
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).
- [ ] Never comment source code, delete it.
- [ ] UT should has "context, subject, expected result" result as test case name, when you call t.Run().
---
--
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] humingcheng commented on a change in pull request #1000: Realize all rbac APIs.
Posted by GitBox <gi...@apache.org>.
humingcheng commented on a change in pull request #1000:
URL: https://github.com/apache/servicecomb-service-center/pull/1000#discussion_r637371508
##########
File path: server/service/rbac/dao/account_dao.go
##########
@@ -33,6 +33,33 @@ func CreateAccount(ctx context.Context, a *rbacmodel.Account) error {
return datasource.Instance().CreateAccount(ctx, a)
}
+// UpdateAccount updates an account's info, except the password
+func UpdateAccount(ctx context.Context, name string, a *rbacmodel.Account) error {
+ // todo params validation
+ exist, err := datasource.Instance().AccountExist(ctx, name)
+ if err != nil {
+ log.Errorf(err, "check account [%s] exit failed", name)
+ return err
+ }
+ if !exist {
+ return datasource.ErrAccountNotExist
+ }
+ oldAccount, err := GetAccount(ctx, name)
+ if err != nil {
+ log.Errorf(err, "get account [%s] failed", name)
+ return err
+ }
+ oldAccount.Roles = a.Roles
+ oldAccount.Status = a.Status
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] little-cui commented on a change in pull request #1000: Realize all rbac APIs.
Posted by GitBox <gi...@apache.org>.
little-cui commented on a change in pull request #1000:
URL: https://github.com/apache/servicecomb-service-center/pull/1000#discussion_r637345316
##########
File path: server/service/rbac/dao/account_dao.go
##########
@@ -33,6 +33,33 @@ func CreateAccount(ctx context.Context, a *rbacmodel.Account) error {
return datasource.Instance().CreateAccount(ctx, a)
}
+// UpdateAccount updates an account's info, except the password
+func UpdateAccount(ctx context.Context, name string, a *rbacmodel.Account) error {
Review comment:
增加ut
##########
File path: server/service/rbac/dao/account_dao.go
##########
@@ -33,6 +33,33 @@ func CreateAccount(ctx context.Context, a *rbacmodel.Account) error {
return datasource.Instance().CreateAccount(ctx, a)
}
+// UpdateAccount updates an account's info, except the password
+func UpdateAccount(ctx context.Context, name string, a *rbacmodel.Account) error {
+ // todo params validation
+ exist, err := datasource.Instance().AccountExist(ctx, name)
+ if err != nil {
+ log.Errorf(err, "check account [%s] exit failed", name)
+ return err
+ }
+ if !exist {
+ return datasource.ErrAccountNotExist
+ }
+ oldAccount, err := GetAccount(ctx, name)
+ if err != nil {
+ log.Errorf(err, "get account [%s] failed", name)
+ return err
+ }
+ oldAccount.Roles = a.Roles
+ oldAccount.Status = a.Status
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] humingcheng commented on a change in pull request #1000: Realize all rbac APIs.
Posted by GitBox <gi...@apache.org>.
humingcheng commented on a change in pull request #1000:
URL: https://github.com/apache/servicecomb-service-center/pull/1000#discussion_r637371444
##########
File path: server/service/rbac/dao/account_dao.go
##########
@@ -33,6 +33,33 @@ func CreateAccount(ctx context.Context, a *rbacmodel.Account) error {
return datasource.Instance().CreateAccount(ctx, a)
}
+// UpdateAccount updates an account's info, except the password
+func UpdateAccount(ctx context.Context, name string, a *rbacmodel.Account) error {
+ // todo params validation
+ exist, err := datasource.Instance().AccountExist(ctx, name)
+ if err != nil {
+ log.Errorf(err, "check account [%s] exit failed", name)
+ return err
+ }
+ if !exist {
+ return datasource.ErrAccountNotExist
+ }
+ oldAccount, err := GetAccount(ctx, name)
+ if err != nil {
+ log.Errorf(err, "get account [%s] failed", name)
+ return err
+ }
+ oldAccount.Roles = a.Roles
Review comment:
Done
--
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] little-cui commented on a change in pull request #1000: Realize all rbac APIs.
Posted by GitBox <gi...@apache.org>.
little-cui commented on a change in pull request #1000:
URL: https://github.com/apache/servicecomb-service-center/pull/1000#discussion_r637344984
##########
File path: server/resource/v4/auth_resource.go
##########
@@ -93,6 +94,27 @@ func (r *AuthResource) DeleteAccount(w http.ResponseWriter, req *http.Request) {
}
w.WriteHeader(http.StatusNoContent)
}
+func (r *AuthResource) UpdateAccount(w http.ResponseWriter, req *http.Request) {
+ body, err := ioutil.ReadAll(req.Body)
+ if err != nil {
+ log.Error("read body err", err)
+ controller.WriteError(w, discovery.ErrInternal, err.Error())
+ return
+ }
+ a := &rbac.Account{}
+ if err = json.Unmarshal(body, a); err != nil {
+ log.Error("json err", err)
+ controller.WriteError(w, discovery.ErrInvalidParams, err.Error())
+ return
+ }
+ name := req.URL.Query().Get(":name")
+ err = dao.UpdateAccount(context.TODO(), name, a)
+ if err != nil {
+ log.Error(errorsEx.MsgOperateAccountFailed, err)
+ controller.WriteError(w, discovery.ErrInternal, errorsEx.MsgOperateAccountFailed)
+ return
+ }
Review comment:
成功场景没有写response status，导致链接阻塞
--
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 #1000: Realize all rbac APIs.
Posted by GitBox <gi...@apache.org>.
tianxiaoliang merged pull request #1000:
URL: https://github.com/apache/servicecomb-service-center/pull/1000
--
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] humingcheng commented on a change in pull request #1000: Realize all rbac APIs.
Posted by GitBox <gi...@apache.org>.
humingcheng commented on a change in pull request #1000:
URL: https://github.com/apache/servicecomb-service-center/pull/1000#discussion_r637371349
##########
File path: server/service/rbac/dao/account_dao.go
##########
@@ -33,6 +33,33 @@ func CreateAccount(ctx context.Context, a *rbacmodel.Account) error {
return datasource.Instance().CreateAccount(ctx, a)
}
+// UpdateAccount updates an account's info, except the password
+func UpdateAccount(ctx context.Context, name string, a *rbacmodel.Account) error {
+ // todo params validation
+ exist, err := datasource.Instance().AccountExist(ctx, name)
+ if err != nil {
+ log.Errorf(err, "check account [%s] exit failed", name)
+ return err
+ }
+ if !exist {
+ return datasource.ErrAccountNotExist
+ }
+ oldAccount, err := GetAccount(ctx, name)
+ if err != nil {
+ log.Errorf(err, "get account [%s] failed", name)
+ return err
+ }
+ oldAccount.Roles = a.Roles
+ oldAccount.Status = a.Status
+ err = datasource.Instance().UpdateAccount(ctx, name, oldAccount)
+ if err != nil {
+ log.Errorf(err, "can not edit account info")
+ return err
+ }
+ log.Info("account is edit")
Review comment:
Done
--
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] humingcheng commented on a change in pull request #1000: Realize all rbac APIs.
Posted by GitBox <gi...@apache.org>.
humingcheng commented on a change in pull request #1000:
URL: https://github.com/apache/servicecomb-service-center/pull/1000#discussion_r637371322
##########
File path: server/service/rbac/dao/account_dao.go
##########
@@ -33,6 +33,33 @@ func CreateAccount(ctx context.Context, a *rbacmodel.Account) error {
return datasource.Instance().CreateAccount(ctx, a)
}
+// UpdateAccount updates an account's info, except the password
+func UpdateAccount(ctx context.Context, name string, a *rbacmodel.Account) error {
Review comment:
done
--
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 #1000: Realize all rbac APIs.
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #1000:
URL: https://github.com/apache/servicecomb-service-center/pull/1000#discussion_r637369390
##########
File path: server/service/rbac/dao/account_dao.go
##########
@@ -36,6 +37,9 @@ func CreateAccount(ctx context.Context, a *rbacmodel.Account) error {
// UpdateAccount updates an account's info, except the password
func UpdateAccount(ctx context.Context, name string, a *rbacmodel.Account) error {
// todo params validation
+ if len(a.Roles) == 0 {
Review comment:
不 ，实际上客户端可以只更新status，不传roles，也可以只更新roles不更新status，所以是在59行那里判断，59行判断不为0就设置roles
--
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] humingcheng commented on a change in pull request #1000: Realize all rbac APIs.
Posted by GitBox <gi...@apache.org>.
humingcheng commented on a change in pull request #1000:
URL: https://github.com/apache/servicecomb-service-center/pull/1000#discussion_r637371423
##########
File path: server/service/rbac/dao/account_dao.go
##########
@@ -36,6 +37,9 @@ func CreateAccount(ctx context.Context, a *rbacmodel.Account) error {
// UpdateAccount updates an account's info, except the password
func UpdateAccount(ctx context.Context, name string, a *rbacmodel.Account) error {
// todo params validation
+ if len(a.Roles) == 0 {
Review comment:
不允许同时为空，其他情况有值则更新。
##########
File path: server/service/rbac/dao/role_dao.go
##########
@@ -50,17 +50,23 @@ func DeleteRole(ctx context.Context, name string) (bool, error) {
return datasource.Instance().DeleteRole(ctx, name)
}
-func EditRole(ctx context.Context, a *rbac.Role) error {
- exist, err := datasource.Instance().RoleExist(ctx, a.Name)
+func EditRole(ctx context.Context, name string, a *rbac.Role) error {
+ exist, err := datasource.Instance().RoleExist(ctx, name)
if err != nil {
- log.Errorf(err, "can not edit account info")
+ log.Errorf(err, "check role [%s] exist failed", name)
return err
}
if !exist {
return datasource.ErrRoleCanNotEdit
}
+ oldRole, err := GetRole(ctx, name)
+ if err != nil {
+ log.Errorf(err, "get role [%s] failed", name)
+ return err
+ }
+ oldRole.Perms = a.Perms
- err = datasource.Instance().UpdateRole(ctx, a.Name, a)
+ err = datasource.Instance().UpdateRole(ctx, name, oldRole)
Review comment:
Done.
--
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 #1000: Realize all rbac APIs.
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #1000:
URL: https://github.com/apache/servicecomb-service-center/pull/1000#discussion_r637345732
##########
File path: server/service/rbac/dao/account_dao.go
##########
@@ -33,6 +33,33 @@ func CreateAccount(ctx context.Context, a *rbacmodel.Account) error {
return datasource.Instance().CreateAccount(ctx, a)
}
+// UpdateAccount updates an account's info, except the password
+func UpdateAccount(ctx context.Context, name string, a *rbacmodel.Account) error {
+ // todo params validation
+ exist, err := datasource.Instance().AccountExist(ctx, name)
+ if err != nil {
+ log.Errorf(err, "check account [%s] exit failed", name)
+ return err
+ }
+ if !exist {
+ return datasource.ErrAccountNotExist
+ }
+ oldAccount, err := GetAccount(ctx, name)
+ if err != nil {
+ log.Errorf(err, "get account [%s] failed", name)
+ return err
+ }
+ oldAccount.Roles = a.Roles
+ oldAccount.Status = a.Status
+ err = datasource.Instance().UpdateAccount(ctx, name, oldAccount)
+ if err != nil {
+ log.Errorf(err, "can not edit account info")
+ return err
+ }
+ log.Info("account is edit")
Review comment:
审计日志要打印账号id
##########
File path: server/service/rbac/dao/account_dao.go
##########
@@ -33,6 +33,33 @@ func CreateAccount(ctx context.Context, a *rbacmodel.Account) error {
return datasource.Instance().CreateAccount(ctx, a)
}
+// UpdateAccount updates an account's info, except the password
+func UpdateAccount(ctx context.Context, name string, a *rbacmodel.Account) error {
+ // todo params validation
+ exist, err := datasource.Instance().AccountExist(ctx, name)
+ if err != nil {
+ log.Errorf(err, "check account [%s] exit failed", name)
+ return err
+ }
+ if !exist {
+ return datasource.ErrAccountNotExist
+ }
+ oldAccount, err := GetAccount(ctx, name)
+ if err != nil {
+ log.Errorf(err, "get account [%s] failed", name)
+ return err
+ }
+ oldAccount.Roles = a.Roles
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] tianxiaoliang commented on a change in pull request #1000: Realize all rbac APIs.
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #1000:
URL: https://github.com/apache/servicecomb-service-center/pull/1000#discussion_r637345981
##########
File path: server/service/rbac/dao/role_dao.go
##########
@@ -50,17 +50,23 @@ func DeleteRole(ctx context.Context, name string) (bool, error) {
return datasource.Instance().DeleteRole(ctx, name)
}
-func EditRole(ctx context.Context, a *rbac.Role) error {
- exist, err := datasource.Instance().RoleExist(ctx, a.Name)
+func EditRole(ctx context.Context, name string, a *rbac.Role) error {
+ exist, err := datasource.Instance().RoleExist(ctx, name)
if err != nil {
- log.Errorf(err, "can not edit account info")
+ log.Errorf(err, "check role [%s] exist failed", name)
return err
}
if !exist {
return datasource.ErrRoleCanNotEdit
}
+ oldRole, err := GetRole(ctx, name)
+ if err != nil {
+ log.Errorf(err, "get role [%s] failed", name)
+ return err
+ }
+ oldRole.Perms = a.Perms
- err = datasource.Instance().UpdateRole(ctx, a.Name, a)
+ err = datasource.Instance().UpdateRole(ctx, name, oldRole)
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] tianxiaoliang commented on a change in pull request #1000: Realize all rbac APIs.
Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on a change in pull request #1000:
URL: https://github.com/apache/servicecomb-service-center/pull/1000#discussion_r637369390
##########
File path: server/service/rbac/dao/account_dao.go
##########
@@ -36,6 +37,9 @@ func CreateAccount(ctx context.Context, a *rbacmodel.Account) error {
// UpdateAccount updates an account's info, except the password
func UpdateAccount(ctx context.Context, name string, a *rbacmodel.Account) error {
// todo params validation
+ if len(a.Roles) == 0 {
Review comment:
不 ，实际上客户端可以只更新status，不传roles，所以是在59行那里判断，59行判断不为0就设置roles
--
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] humingcheng commented on a change in pull request #1000: Realize all rbac APIs.
Posted by GitBox <gi...@apache.org>.
humingcheng commented on a change in pull request #1000:
URL: https://github.com/apache/servicecomb-service-center/pull/1000#discussion_r637371303
##########
File path: server/resource/v4/auth_resource.go
##########
@@ -93,6 +94,27 @@ func (r *AuthResource) DeleteAccount(w http.ResponseWriter, req *http.Request) {
}
w.WriteHeader(http.StatusNoContent)
}
+func (r *AuthResource) UpdateAccount(w http.ResponseWriter, req *http.Request) {
+ body, err := ioutil.ReadAll(req.Body)
+ if err != nil {
+ log.Error("read body err", err)
+ controller.WriteError(w, discovery.ErrInternal, err.Error())
+ return
+ }
+ a := &rbac.Account{}
+ if err = json.Unmarshal(body, a); err != nil {
+ log.Error("json err", err)
+ controller.WriteError(w, discovery.ErrInvalidParams, err.Error())
+ return
+ }
+ name := req.URL.Query().Get(":name")
+ err = dao.UpdateAccount(context.TODO(), name, a)
+ if err != nil {
+ log.Error(errorsEx.MsgOperateAccountFailed, err)
+ controller.WriteError(w, discovery.ErrInternal, errorsEx.MsgOperateAccountFailed)
+ return
+ }
Review comment:
done
--
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