You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by ti...@apache.org on 2021/07/26 06:26:08 UTC
[servicecomb-service-center] branch master updated: Change: DON'T
check root exist if pass root token (#1104)
This is an automated email from the ASF dual-hosted git repository.
tianxiaoliang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-service-center.git
The following commit(s) were added to refs/heads/master by this push:
new 772b882 Change: DON'T check root exist if pass root token (#1104)
772b882 is described below
commit 772b882b2c2f5ce5054183ec202bdf07421043e9
Author: little-cui <su...@qq.com>
AuthorDate: Mon Jul 26 14:26:00 2021 +0800
Change: DON'T check root exist if pass root token (#1104)
* Change: DON'T check root exist if pass root token
* Change: user or pwd wrong return 404 when scope exclude service
* Fix: resolve comment
* Fix: resolve comment
* Fix: resolve comment
---
go.mod | 2 +-
go.sum | 2 +
server/plugin/auth/buildin/buildin.go | 75 ++++++++++++++++++++++----------
server/resource/v4/auth_resource_test.go | 2 +-
server/service/rbac/account_service.go | 6 +--
server/service/rbac/authr_plugin.go | 40 ++++++-----------
server/service/rbac/decision.go | 31 +++++++------
server/service/rbac/init.go | 13 ++++++
server/service/rbac/password.go | 4 +-
server/service/rbac/resource.go | 23 +++++++---
10 files changed, 120 insertions(+), 78 deletions(-)
diff --git a/go.mod b/go.mod
index 1f6e541..744aac8 100644
--- a/go.mod
+++ b/go.mod
@@ -16,7 +16,7 @@ require (
github.com/dgrijalva/jwt-go v3.2.0+incompatible
github.com/elithrar/simple-scrypt v1.3.0
github.com/ghodss/yaml v1.0.0
- github.com/go-chassis/cari v0.5.1-0.20210722135711-3b4cdd6dd9bf
+ github.com/go-chassis/cari v0.5.1-0.20210723060050-4a4f119d64ff
github.com/go-chassis/foundation v0.3.1-0.20210513015331-b54416b66bcd
github.com/go-chassis/go-archaius v1.5.1
github.com/go-chassis/go-chassis/v2 v2.2.1-0.20210630123055-6b4c31c5ad02
diff --git a/go.sum b/go.sum
index 58b963d..650f5b7 100644
--- a/go.sum
+++ b/go.sum
@@ -237,6 +237,8 @@ github.com/go-chassis/cari v0.5.0 h1:KMkHtVJuFeqd9NtlcJm27ZBJYhQja909+rgR5CaCs00
github.com/go-chassis/cari v0.5.0/go.mod h1:av/19fqwEP4eOC8unL/z67AAbFDwXUCko6SKa4Avrd8=
github.com/go-chassis/cari v0.5.1-0.20210722135711-3b4cdd6dd9bf h1:swMNKo+lr/v/xFOX2dzTL14BfpFS9+XBb9x2U4k6UUE=
github.com/go-chassis/cari v0.5.1-0.20210722135711-3b4cdd6dd9bf/go.mod h1:av/19fqwEP4eOC8unL/z67AAbFDwXUCko6SKa4Avrd8=
+github.com/go-chassis/cari v0.5.1-0.20210723060050-4a4f119d64ff h1:y3UteqAhvq6eAQ1PkHc5Gm9UGfy7k5VYt+JBRTBC8U8=
+github.com/go-chassis/cari v0.5.1-0.20210723060050-4a4f119d64ff/go.mod h1:av/19fqwEP4eOC8unL/z67AAbFDwXUCko6SKa4Avrd8=
github.com/go-chassis/foundation v0.2.2-0.20201210043510-9f6d3de40234/go.mod h1:2PjwqpVwYEVaAldl5A58a08viH8p27pNeYaiE3ZxOBA=
github.com/go-chassis/foundation v0.2.2/go.mod h1:2PjwqpVwYEVaAldl5A58a08viH8p27pNeYaiE3ZxOBA=
github.com/go-chassis/foundation v0.3.0/go.mod h1:2PjwqpVwYEVaAldl5A58a08viH8p27pNeYaiE3ZxOBA=
diff --git a/server/plugin/auth/buildin/buildin.go b/server/plugin/auth/buildin/buildin.go
index fc21896..de51cc5 100644
--- a/server/plugin/auth/buildin/buildin.go
+++ b/server/plugin/auth/buildin/buildin.go
@@ -18,7 +18,9 @@
package buildin
import (
+ "context"
"errors"
+ "fmt"
"net/http"
"strings"
@@ -34,6 +36,8 @@ import (
"github.com/go-chassis/go-chassis/v2/server/restful"
)
+var ErrNoRoles = errors.New("no role found in token")
+
func init() {
plugin.RegisterPlugin(plugin.Plugin{Kind: auth.AUTH, Name: "buildin", New: New})
}
@@ -60,56 +64,80 @@ func (ba *TokenAuthenticator) Identify(req *http.Request) error {
return nil
}
- claims, err := ba.VerifyToken(req)
+ account, err := ba.VerifyRequest(req)
if err != nil {
- 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", rbacmodel.ErrConvertErr)
- return rbacmodel.ErrConvertErr
- }
- account, err := rbacmodel.GetAccount(m)
+ // if account not exist should return auth failure
+ err = accountExist(req.Context(), account.Name)
if err != nil {
- log.Error("get account from token failed", err)
return err
}
- util.SetRequestContext(req, rbacsvc.CtxRequestClaims, m)
if !rbacsvc.MustCheckPerm(pattern) {
return nil
}
// user can change self password
- if isChangeSelfPassword(pattern, account, req) {
+ if isChangeSelfPassword(pattern, account.Name, req) {
return nil
}
+ matchedLabels, err := checkPerm(account.Roles, req)
+ if err != nil {
+ return err
+ }
+
+ util.SetRequestContext(req, authHandler.CtxResourceLabels, matchedLabels)
+ return nil
+}
+
+func (ba *TokenAuthenticator) VerifyRequest(req *http.Request) (*rbacmodel.Account, error) {
+ claims, err := ba.VerifyToken(req)
+ if err != nil {
+ log.Errorf(err, "verify request token failed, %s %s", req.Method, req.RequestURI)
+ return nil, err
+ }
+ m, ok := claims.(map[string]interface{})
+ if !ok {
+ log.Error("claims convert failed", rbacmodel.ErrConvert)
+ return nil, rbacmodel.ErrConvert
+ }
+ util.SetRequestContext(req, rbacsvc.CtxRequestClaims, m)
+ account, err := rbacmodel.GetAccount(m)
+ if err != nil {
+ log.Error("get account from token failed", err)
+ return nil, err
+ }
if len(account.Roles) == 0 {
log.Error("no role found in token", nil)
- return errors.New("no role found in token")
+ return nil, ErrNoRoles
}
+ return account, nil
+}
- project := req.URL.Query().Get(":project")
- allow, matchedLabels, err := checkPerm(account.Roles, project, req, pattern, req.Method)
+func accountExist(ctx context.Context, user string) error {
+ // if root should pass, cause of root initialization
+ if user == rbacsvc.RootName {
+ return nil
+ }
+ exist, err := rbacsvc.AccountExist(ctx, user)
if err != nil {
return err
}
- if !allow {
- return rbacmodel.NewError(rbacmodel.ErrNoPermission, "")
+ if !exist {
+ msg := fmt.Sprintf("account [%s] is deleted", user)
+ return rbacmodel.NewError(rbacmodel.ErrTokenOwnedAccountDeleted, msg)
}
-
- util.SetRequestContext(req, authHandler.CtxResourceLabels, matchedLabels)
return nil
}
-func isChangeSelfPassword(pattern string, a *rbacmodel.Account, req *http.Request) bool {
+func isChangeSelfPassword(pattern string, user string, req *http.Request) bool {
if pattern != rbacsvc.APIAccountPassword {
return false
}
- changerName := a.Name
+ changerName := user
targetName := req.URL.Query().Get(":name")
return changerName == targetName
}
@@ -140,16 +168,17 @@ func (ba *TokenAuthenticator) VerifyToken(req *http.Request) (interface{}, error
}
//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) {
+func checkPerm(roleList []string, req *http.Request) ([]map[string]string, error) {
hasAdmin, normalRoles := filterRoles(roleList)
if hasAdmin {
- return true, nil, nil
+ return nil, nil
}
//todo fast check for dev role
targetResource := FromRequest(req)
if targetResource == nil {
- return false, nil, errors.New("no valid resouce scope")
+ return nil, errors.New("no valid resouce scope")
}
//TODO add project
+ project := req.URL.Query().Get(":project")
return rbacsvc.Allow(req.Context(), project, normalRoles, targetResource)
}
diff --git a/server/resource/v4/auth_resource_test.go b/server/resource/v4/auth_resource_test.go
index 205c010..7a9ccf5 100644
--- a/server/resource/v4/auth_resource_test.go
+++ b/server/resource/v4/auth_resource_test.go
@@ -209,7 +209,7 @@ func TestAuthResource_DeleteAccount(t *testing.T) {
r2.Header.Set(restful.HeaderAuth, "Bearer "+devTo.TokenStr)
w2 := httptest.NewRecorder()
rest.GetRouter().ServeHTTP(w2, r2)
- assert.Equal(t, http.StatusUnauthorized, w2.Code)
+ assert.Equal(t, http.StatusForbidden, w2.Code)
})
t.Run("create your_account with admin role should be pass", func(t *testing.T) {
diff --git a/server/service/rbac/account_service.go b/server/service/rbac/account_service.go
index 13110c1..0a23a24 100644
--- a/server/service/rbac/account_service.go
+++ b/server/service/rbac/account_service.go
@@ -32,8 +32,7 @@ import (
"github.com/go-chassis/cari/rbac"
)
-//CreateAccount save 2 kv
-//1. account info
+//CreateAccount save account info
func CreateAccount(ctx context.Context, a *rbac.Account) error {
quotaErr := quota.Apply(ctx, quota.NewApplyQuotaResource(quota.TypeAccount,
util.ParseDomainProject(ctx), "", 1))
@@ -140,8 +139,7 @@ func DeleteAccount(ctx context.Context, name string) error {
return err
}
-//CreateAccount save 2 kv
-//1. account info
+//EditAccount save account info
func EditAccount(ctx context.Context, a *rbac.Account) error {
exist, err := datasource.GetAccountManager().AccountExist(ctx, a.Name)
if err != nil {
diff --git a/server/service/rbac/authr_plugin.go b/server/service/rbac/authr_plugin.go
index cfd8df5..b08f587 100644
--- a/server/service/rbac/authr_plugin.go
+++ b/server/service/rbac/authr_plugin.go
@@ -20,23 +20,17 @@ package rbac
import (
"context"
"crypto/rsa"
- "errors"
- "fmt"
+ "github.com/apache/servicecomb-service-center/pkg/log"
+ "github.com/apache/servicecomb-service-center/pkg/privacy"
+ "github.com/apache/servicecomb-service-center/pkg/util"
"github.com/dgrijalva/jwt-go"
"github.com/go-chassis/cari/pkg/errsvc"
"github.com/go-chassis/cari/rbac"
"github.com/go-chassis/go-chassis/v2/security/authr"
"github.com/go-chassis/go-chassis/v2/security/token"
-
- "github.com/apache/servicecomb-service-center/datasource"
- "github.com/apache/servicecomb-service-center/pkg/log"
- "github.com/apache/servicecomb-service-center/pkg/privacy"
- "github.com/apache/servicecomb-service-center/pkg/util"
)
-var ErrUnauthorized = errors.New("wrong user name or password")
-
//EmbeddedAuthenticator is sc default auth plugin, RBAC data is persisted in etcd
type EmbeddedAuthenticator struct {
}
@@ -50,7 +44,7 @@ func (a *EmbeddedAuthenticator) Login(ctx context.Context, user string, password
ip := util.GetIPFromContext(ctx)
if IsBanned(MakeBanKey(user, ip)) {
log.Warnf("ip [%s] is banned, account: %s", ip, user)
- return "", rbac.NewError(rbac.ErrAccountBlocked, "")
+ return "", ErrAccountBlocked
}
opt := &authr.LoginOptions{}
for _, o := range opts {
@@ -60,14 +54,14 @@ func (a *EmbeddedAuthenticator) Login(ctx context.Context, user string, password
if err != nil {
if errsvc.IsErrEqualCode(err, rbac.ErrAccountNotExist) {
TryLockAccount(MakeBanKey(user, ip))
- return "", rbac.NewError(rbac.ErrUserOrPwdWrong, "")
+ return "", UserOrPwdWrongError()
}
return "", err
}
same := privacy.SamePassword(account.Password, password)
if !same {
TryLockAccount(MakeBanKey(user, ip))
- return "", rbac.NewError(rbac.ErrUserOrPwdWrong, "")
+ return "", UserOrPwdWrongError()
}
secret, err := GetPrivateKey()
@@ -98,23 +92,10 @@ func (a *EmbeddedAuthenticator) Authenticate(ctx context.Context, tokenStr strin
claims, err := a.authToken(tokenStr, p)
if err != nil {
if a.isTokenExpiredError(err) {
- return nil, rbac.NewError(rbac.ErrTokenExpired, "")
+ return nil, ErrTokenExpired
}
return nil, err
}
- accountNameI := claims[rbac.ClaimsUser]
- n, ok := accountNameI.(string)
- if !ok {
- return nil, rbac.ErrConvert
- }
- exist, err := datasource.GetAccountManager().AccountExist(ctx, n)
- if err != nil {
- return nil, err
- }
- if !exist {
- msg := fmt.Sprintf("account [%s] is deleted", n)
- return nil, rbac.NewError(rbac.ErrTokenOwnedAccountDeleted, msg)
- }
return claims, nil
}
@@ -138,6 +119,13 @@ func (a *EmbeddedAuthenticator) authToken(tokenStr string, pub *rsa.PublicKey) (
})
}
+func UserOrPwdWrongError() error {
+ if AuthResource(ResourceService) {
+ return ErrUserOrPwdWrong
+ }
+ return ErrUserOrPwdWrongEx
+}
+
func init() {
authr.Install("default", newEmbeddedAuthenticator)
}
diff --git a/server/service/rbac/decision.go b/server/service/rbac/decision.go
index a344779..242d110 100644
--- a/server/service/rbac/decision.go
+++ b/server/service/rbac/decision.go
@@ -19,46 +19,49 @@ package rbac
import (
"context"
-
- "github.com/go-chassis/cari/rbac"
+ "fmt"
"github.com/apache/servicecomb-service-center/datasource"
"github.com/apache/servicecomb-service-center/pkg/log"
"github.com/apache/servicecomb-service-center/server/plugin/auth"
+ "github.com/go-chassis/cari/rbac"
)
-// return: allow, matched labels(empty if no label defined), error
+// Allow return: matched labels(empty if no label defined), error
func Allow(ctx context.Context, project string, roleList []string,
- targetResouce *auth.ResourceScope) (bool, []map[string]string, error) {
+ targetResource *auth.ResourceScope) ([]map[string]string, error) {
//TODO check project
allPerms, err := getPermsByRoles(ctx, roleList)
if err != nil {
log.Error("get role list errors", err)
- return false, nil, err
+ return nil, err
}
if len(allPerms) == 0 {
log.Warn("role list has no any permissions")
- return false, nil, nil
+ return nil, rbac.NewError(rbac.ErrNoPermission, "role has no any permissions")
}
- allow, labelList := GetLabel(allPerms, targetResouce.Type, targetResouce.Verb)
+ allow, labelList := GetLabel(allPerms, targetResource.Type, targetResource.Verb)
if !allow {
- return false, nil, nil
+ return nil, rbac.NewError(rbac.ErrNoPermission,
+ fmt.Sprintf("role has no permissions[%s:%s]", targetResource.Type, targetResource.Verb))
}
// allow, but no label found, means we can ignore the labels
if len(labelList) == 0 {
- return true, nil, nil
+ return nil, nil
}
// target resource needs no label, return without filter
- if len(targetResouce.Labels) == 0 {
- return true, labelList, nil
+ if len(targetResource.Labels) == 0 {
+ return labelList, nil
}
// allow, and labels found, filter the labels
- filteredLabelList := FilterLabel(targetResouce.Labels, labelList)
+ filteredLabelList := FilterLabel(targetResource.Labels, labelList)
// target resource label matches no label in permission, means not allow
if len(filteredLabelList) == 0 {
- return false, nil, nil
+ return nil, rbac.NewError(rbac.ErrNoPermission,
+ fmt.Sprintf("role has no permissions[%s:%s] for labels %v",
+ targetResource.Type, targetResource.Verb, targetResource.Labels))
}
- return true, filteredLabelList, nil
+ return filteredLabelList, nil
}
func FilterLabel(targetResourceLabel []map[string]string, permLabelList []map[string]string) []map[string]string {
diff --git a/server/service/rbac/init.go b/server/service/rbac/init.go
index f9ee4a9..55f22f6 100644
--- a/server/service/rbac/init.go
+++ b/server/service/rbac/init.go
@@ -26,9 +26,22 @@ import (
"github.com/apache/servicecomb-service-center/pkg/log"
)
+const ErrUserOrPwdWrongInHalfOpening int32 = 401302
+
+var (
+ ErrTokenExpired = rbac.NewError(rbac.ErrTokenExpired, "")
+ ErrAccountBlocked = rbac.NewError(rbac.ErrAccountBlocked, "")
+ ErrUserOrPwdWrong = rbac.NewError(rbac.ErrUserOrPwdWrong, "")
+ ErrUserOrPwdWrongEx = rbac.NewError(ErrUserOrPwdWrongInHalfOpening,
+ "User name or password is wrong, RBAC system is half opening")
+ ErrOldPwdWrong = rbac.NewError(rbac.ErrOldPwdWrong, "")
+)
+
var roleMap = map[string]*rbac.Role{}
func init() {
+ rbac.MustRegisterErr(ErrUserOrPwdWrongInHalfOpening, ErrUserOrPwdWrong.Error())
+
// Assign resources to admin role, admin role own all permissions
roleMap[rbac.RoleAdmin] = &rbac.Role{
Name: rbac.RoleAdmin,
diff --git a/server/service/rbac/password.go b/server/service/rbac/password.go
index d751057..ee8f5a0 100644
--- a/server/service/rbac/password.go
+++ b/server/service/rbac/password.go
@@ -75,7 +75,7 @@ func changePassword(ctx context.Context, name, currentPassword, pwd string) erro
ip := util.GetIPFromContext(ctx)
if IsBanned(MakeBanKey(name, ip)) {
log.Warnf("ip [%s] is banned, account: %s", ip, name)
- return rbac.NewError(rbac.ErrAccountBlocked, "")
+ return ErrAccountBlocked
}
if currentPassword == pwd {
return rbac.NewError(rbac.ErrNewPwdBad, ErrSamePassword.Error())
@@ -89,7 +89,7 @@ func changePassword(ctx context.Context, name, currentPassword, pwd string) erro
if !same {
log.Error("current password is wrong", nil)
TryLockAccount(MakeBanKey(name, ip))
- return rbac.NewError(rbac.ErrOldPwdWrong, "")
+ return ErrOldPwdWrong
}
return doChangePassword(ctx, old, pwd)
}
diff --git a/server/service/rbac/resource.go b/server/service/rbac/resource.go
index cb4c34d..7d89029 100644
--- a/server/service/rbac/resource.go
+++ b/server/service/rbac/resource.go
@@ -72,7 +72,7 @@ var (
APIServiceSchema = "/v4/:project/registry/microservices/:serviceId/schemas"
- AuthResources = map[string]struct{}{}
+ authResources = map[string]struct{}{}
whiteAPIList = mapset.NewSet()
)
@@ -109,22 +109,31 @@ func InitResourceMap() {
}
func initAuthResources() {
- scopes := strings.Split(config.GetString("rbac.scope", "*"), ",")
+ // scope MUST contain role and account resources
+ scopes := strings.Split(config.GetString("rbac.scope", "*")+",role,account", ",")
for _, scope := range scopes {
if scope == "*" {
- AuthResources = map[string]struct{}{}
+ authResources = map[string]struct{}{}
break
}
- AuthResources[scope] = struct{}{}
+ authResources[scope] = struct{}{}
}
- log.Info(fmt.Sprintf("init must auth resources: %v", AuthResources))
+ log.Info(fmt.Sprintf("init must auth resources: %v", authResources))
+}
+
+func AuthResource(resource string) bool {
+ if len(authResources) == 0 {
+ return true
+ }
+ _, ok := authResources[resource]
+ return ok
}
func MustAuth(apiPattern string) bool {
found := true
- if len(AuthResources) > 0 {
+ if len(authResources) > 0 {
resource := rbac.GetResource(apiPattern)
- _, found = AuthResources[resource]
+ _, found = authResources[resource]
}
if !found {
return false