[GitHub] [servicecomb-service-center] humingcheng commented on a change in pull request #1014: add rbac logic

[GitBox <gi...@apache.org>]

humingcheng commented on a change in pull request #1014:
URL: https://github.com/apache/servicecomb-service-center/pull/1014#discussion_r639775896



##########
File path: server/plugin/auth/buildin/buildin.go
##########
@@ -48,86 +49,113 @@ type TokenAuthenticator struct {
 }
 
 func (ba *TokenAuthenticator) Identify(req *http.Request) error {
-	if !rbacsvc.Enabled() {
+	if !rbac.Enabled() {
 		return nil
 	}
 	pattern, ok := req.Context().Value(rest.CtxMatchPattern).(string)
-	if ok && !mustAuth(pattern) {
-		return nil
-	}
-	v := req.Header.Get(restful.HeaderAuth)
-	if v == "" {
-		return rbac.ErrNoHeader
+	if !ok {
+		pattern = req.URL.Path
+		log.Warn("can not find api pattern")
 	}
-	s := strings.Split(v, " ")
-	if len(s) != 2 {
-		return rbac.ErrInvalidHeader
+
+	if !mustAuth(pattern) {
+		return nil
 	}
-	to := s[1]
 
-	claims, err := authr.Authenticate(req.Context(), to)
+	claims, err := ba.VerifyToken(req)
 	if err != nil {
-		log.Errorf(err, "authenticate request failed, %s %s", req.Method, req.RequestURI)
+		log.Errorf(err, "verify request token failed, %s %s", req.Method, req.RequestURI)
 		return err
 	}
+
 	m, ok := claims.(map[string]interface{})
 	if !ok {
-		log.Error("claims convert failed", rbac.ErrConvertErr)
-		return rbac.ErrConvertErr
+		log.Error("claims convert failed", rbacModel.ErrConvertErr)
+		return rbacModel.ErrConvertErr
 	}
-
-	roleList, err := rbac.GetRolesList(m)
+	account, err := rbacModel.GetAccount(m)
 	if err != nil {
-		log.Error("get role list failed", err)
+		log.Error("get account  failed", err)
 		return err
 	}
+	util.SetRequestContext(req, authHandler.CtxRequestClaims, m)
+	// user can change self password
+	if pattern == rbac.APIAccountPassword && account.Name == req.URL.Query().Get(":name") {
+		return nil
+	}
 
-	var apiPattern string
-	a := req.Context().Value(rest.CtxMatchPattern)
-	if a == nil { //handle exception
-		apiPattern = req.URL.Path
-		log.Warn("can not find api pattern")
-	} else {
-		apiPattern = a.(string)
+	if len(account.Roles) == 0 {
+		log.Error("no role found in token", nil)
+		return errors.New(errorsEx.MsgNoPerm)
 	}
 
 	project := req.URL.Query().Get(":project")
-	verbs := rbacsvc.MethodToVerbs[req.Method]
-	err = checkPerm(roleList, project, apiPattern, verbs)
+	allow, matchedLabels, err := checkPerm(account.Roles, project, req, pattern, req.Method)
 	if err != nil {
 		return err
 	}
-	req2 := req.WithContext(rbac.NewContext(req.Context(), m))
-	*req = *req2
+	if !allow {
+		return errors.New(errorsEx.MsgNoPerm)
+	}
+
+	util.SetRequestContext(req, authHandler.CtxResourceLabels, matchedLabels)
 	return nil
 }
 
-//this method decouple business code and perm checks
-func checkPerm(roleList []string, project, apiPattern, verbs string) error {
-	resource := rbac.GetResource(apiPattern)
-	if resource == "" {
-		//fast fail, no need to access role storage
-		return errors.New(errorsEx.MsgNoPerm)
+func filterRoles(roleList []string) (hasAdmin bool, normalRoles []string) {
+	for _, r := range roleList {
+		if r == rbacModel.RoleAdmin {
+			hasAdmin = true
+			return
+		}
+		normalRoles = append(normalRoles, r)
 	}
-	//TODO add verbs,project
-	allow, err := rbacsvc.Allow(context.TODO(), roleList, project, resource, verbs)
-	if err != nil {
-		log.Error("", err)
-		return errors.New(errorsEx.MsgRolePerm)
+	return
+}
+
+func (ba *TokenAuthenticator) VerifyToken(req *http.Request) (interface{}, error) {
+	v := req.Header.Get(restful.HeaderAuth)
+	if v == "" {
+		return nil, rbacModel.ErrNoHeader
 	}
-	if !allow {
-		return errors.New(errorsEx.MsgNoPerm)
+	s := strings.Split(v, " ")
+	if len(s) != 2 {
+		return nil, rbacModel.ErrInvalidHeader
 	}
-	return nil
+	to := s[1]
+
+	return authr.Authenticate(req.Context(), to)
+}
+
+//this method decouple business code and perm checks
+func checkPerm(roleList []string, project string, req *http.Request, apiPattern, method string) (bool, []map[string]string, error) {
+	hasAdmin, normalRoles := filterRoles(roleList)
+	if hasAdmin {
+		return true, nil, nil
+	}
+	//todo fast check for dev role
+	targetResource, ok := req.Context().Value(authHandler.CtxResourceScopes).(*auth.ResourceScope)
+	if !ok || targetResource == nil {
+		return false, nil, errors.New("no valid resouce scope")
+	}
+	//TODO add project
+	return rbac.Allow(context.TODO(), project, normalRoles, targetResource)
 }
 
 func mustAuth(pattern string) bool {
 	if util.IsVersionOrHealthPattern(pattern) {
 		return false
 	}
-	return rbac.MustAuth(pattern)
+	return rbacModel.MustAuth(pattern)
 }
 
 func (ba *TokenAuthenticator) ResourceScopes(r *http.Request) *auth.ResourceScope {
 	return FromRequest(r)
 }
+func AccountFromContext(ctx context.Context) (*rbacModel.Account, error) {
+	m, ok := ctx.Value(authHandler.CtxRequestClaims).(map[string]interface{})

Review comment:
       原来的语句，会导致req中的context指针被修改，sc里面其他地方就获取不到对应的context内容，所以暂时改成sc的方式。




-- 
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