Merge pull request #10013 from heww/permission-checking-improvement

perf(rbac): add permission evaluator to improve performance
This commit is contained in:
Daniel Jiang 2019-11-29 11:23:56 +08:00 committed by GitHub
commit 798059aed5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 525 additions and 354 deletions

View File

@ -17,17 +17,20 @@ package rbac
import (
"errors"
"fmt"
"math/rand"
"regexp"
"strings"
"sync"
"time"
"github.com/casbin/casbin"
"github.com/casbin/casbin/model"
"github.com/casbin/casbin/persist"
"github.com/casbin/casbin/util"
"github.com/goharbor/harbor/src/common/utils/log"
)
var (
errNotImplemented = errors.New("Not implemented")
errNotImplemented = errors.New("not implemented")
)
// Syntax for models see https://casbin.org/docs/en/syntax-for-models
@ -53,12 +56,59 @@ e = some(where (p.eft == allow)) && !some(where (p.eft == deny))
m = g(r.sub, p.sub) && keyMatch2(r.obj, p.obj) && (r.act == p.act || p.act == '*')
`
// keyMatch2 determines whether key1 matches the pattern of key2, its behavior most likely the builtin KeyMatch2
// except that the match of ("/project/1/robot", "/project/1") will return false
func keyMatch2(key1 string, key2 string) bool {
key2 = strings.Replace(key2, "/*", "/.*", -1)
type regexpStore struct {
entries sync.Map
}
func (s *regexpStore) Get(key string, build func(string) *regexp.Regexp) *regexp.Regexp {
value, ok := s.entries.Load(key)
if !ok {
value = build(key)
s.entries.Store(key, value)
}
return value.(*regexp.Regexp)
}
func (s *regexpStore) Purge() {
var keys []interface{}
s.entries.Range(func(key, value interface{}) bool {
keys = append(keys, key)
return true
})
for _, key := range keys {
s.entries.Delete(key)
}
}
var (
store = &regexpStore{}
)
func init() {
startRegexpStorePurging(store, time.Hour*24)
}
func startRegexpStorePurging(s *regexpStore, intervalDuration time.Duration) {
go func() {
rand.Seed(time.Now().Unix())
jitter := time.Duration(rand.Int()%60) * time.Minute
log.Debugf("Starting regexp store purge in %s", jitter)
time.Sleep(jitter)
for {
s.Purge()
log.Debugf("Starting regexp store purge in %s", intervalDuration)
time.Sleep(intervalDuration)
}
}()
}
func keyMatch2Build(key2 string) *regexp.Regexp {
re := regexp.MustCompile(`(.*):[^/]+(.*)`)
key2 = strings.Replace(key2, "/*", "/.*", -1)
for {
if !strings.Contains(key2, "/:") {
break
@ -67,7 +117,13 @@ func keyMatch2(key1 string, key2 string) bool {
key2 = re.ReplaceAllString(key2, "$1[^/]+$2")
}
return util.RegexMatch(key1, "^"+key2+"$")
return regexp.MustCompile("^" + key2 + "$")
}
// keyMatch2 determines whether key1 matches the pattern of key2, its behavior most likely the builtin KeyMatch2
// except that the match of ("/project/1/robot", "/project/1") will return false
func keyMatch2(key1 string, key2 string) bool {
return store.Get(key2, keyMatch2Build).MatchString(key1)
}
func keyMatch2Func(args ...interface{}) (interface{}, error) {

View File

@ -16,6 +16,8 @@ package rbac
import (
"testing"
"github.com/stretchr/testify/assert"
)
func Test_keyMatch2(t *testing.T) {
@ -57,3 +59,26 @@ func Test_keyMatch2(t *testing.T) {
})
}
}
func TestRegexpStore(t *testing.T) {
assert := assert.New(t)
s := &regexpStore{}
sLen := func() int {
var l int
s.entries.Range(func(key, value interface{}) bool {
l++
return true
})
return l
}
r1 := s.Get("key1", keyMatch2Build)
r2 := s.Get("key1", keyMatch2Build)
assert.Equal(r1, r2)
s.Purge()
assert.Equal(0, sLen())
}

View File

@ -0,0 +1,83 @@
// Copyright Project Harbor Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package rbac
import (
"fmt"
"sync"
"github.com/casbin/casbin"
)
// Evaluator the permissioin evaluator
type Evaluator interface {
// HasPermission returns true when user has action permission for the resource
HasPermission(resource Resource, action Action) bool
}
type userEvaluator struct {
user User
enforcer *casbin.Enforcer
once sync.Once
}
func (e *userEvaluator) HasPermission(resource Resource, action Action) bool {
e.once.Do(func() {
e.enforcer = enforcerForUser(e.user)
})
return e.enforcer.Enforce(e.user.GetUserName(), resource.String(), action.String())
}
// NewUserEvaluator returns Evaluator for the rbac user
func NewUserEvaluator(user User) Evaluator {
return &userEvaluator{
user: user,
}
}
type namespaceEvaluator struct {
factory func(Namespace) Evaluator
namespaceKind string
cache sync.Map
}
func (e *namespaceEvaluator) HasPermission(resource Resource, action Action) bool {
ns, err := resource.GetNamespace()
if err == nil && ns.Kind() == e.namespaceKind {
var evaluator Evaluator
key := fmt.Sprintf("%s:%v", ns.Kind(), ns.Identity())
value, ok := e.cache.Load(key)
if !ok {
evaluator = e.factory(ns)
e.cache.Store(key, evaluator)
} else {
evaluator, _ = value.(Evaluator) // maybe value is nil
}
return evaluator != nil && evaluator.HasPermission(resource, action)
}
return false
}
// NewNamespaceEvaluator returns permission evaluator for which support namespace
func NewNamespaceEvaluator(namespaceKind string, factory func(Namespace) Evaluator) Evaluator {
return &namespaceEvaluator{
namespaceKind: namespaceKind,
factory: factory,
}
}

View File

@ -0,0 +1,285 @@
// Copyright Project Harbor Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package rbac
import (
"testing"
)
type role struct {
RoleName string
}
func (r *role) GetRoleName() string {
return r.RoleName
}
func (r *role) GetPolicies() []*Policy {
return []*Policy{
{Resource: "/project", Action: "create"},
{Resource: "/project", Action: "update"},
}
}
type userWithRoles struct {
Username string
RoleName string
BaseUser
}
func (u *userWithRoles) GetUserName() string {
return u.Username
}
func (u *userWithRoles) GetRoles() []Role {
return []Role{
&role{RoleName: u.RoleName},
}
}
type userWithoutRoles struct {
Username string
UserPolicies []*Policy
BaseUser
}
func (u *userWithoutRoles) GetUserName() string {
return u.Username
}
func (u *userWithoutRoles) GetPolicies() []*Policy {
return u.UserPolicies
}
// HasPermission ...
func HasPermission(user User, resource Resource, action Action) bool {
return NewUserEvaluator(user).HasPermission(resource, action)
}
func TestHasPermissionUserWithRoles(t *testing.T) {
type args struct {
user User
resource Resource
action Action
}
tests := []struct {
name string
args args
want bool
}{
{
name: "project create for project admin",
args: args{
&userWithRoles{Username: "project admin", RoleName: "projectAdmin"},
"/project",
"create",
},
want: true,
},
{
name: "project update for project admin",
args: args{
&userWithRoles{Username: "project admin", RoleName: "projectAdmin"},
"/project",
"update",
},
want: true,
},
{
name: "project delete for project admin",
args: args{
&userWithRoles{Username: "project admin", RoleName: "projectAdmin"},
"/project",
"delete",
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := HasPermission(tt.args.user, tt.args.resource, tt.args.action); got != tt.want {
t.Errorf("HasPermission() = %v, want %v", got, tt.want)
}
})
}
}
func TestHasPermissionUsernameEmpty(t *testing.T) {
type args struct {
user User
resource Resource
action Action
}
tests := []struct {
name string
args args
want bool
}{
{
name: "project create for user without roles",
args: args{
&userWithoutRoles{Username: "", UserPolicies: []*Policy{{Resource: "/project", Action: "create"}}},
"/project",
"create",
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := HasPermission(tt.args.user, tt.args.resource, tt.args.action); got != tt.want {
t.Errorf("HasPermission() = %v, want %v", got, tt.want)
}
})
}
}
func TestHasPermissionRoleNameEmpty(t *testing.T) {
type args struct {
user User
resource Resource
action Action
}
tests := []struct {
name string
args args
want bool
}{
{
name: "project create for project admin",
args: args{
&userWithRoles{Username: "project admin", RoleName: ""},
"/project",
"create",
},
want: false,
},
{
name: "project update for project admin",
args: args{
&userWithRoles{Username: "project admin", RoleName: ""},
"/project",
"update",
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := HasPermission(tt.args.user, tt.args.resource, tt.args.action); got != tt.want {
t.Errorf("HasPermission() = %v, want %v", got, tt.want)
}
})
}
}
func TestHasPermissionResourceKeyMatch(t *testing.T) {
type args struct {
user User
resource Resource
action Action
}
tests := []struct {
name string
args args
want bool
}{
{
name: "project member create for resource key match",
args: args{
&userWithoutRoles{Username: "user1", UserPolicies: []*Policy{{Resource: "/project/1/*", Action: "*"}}},
"/project/1/member",
"create",
},
want: true,
},
{
name: "project member create for resource key match",
args: args{
&userWithoutRoles{Username: "user1", UserPolicies: []*Policy{{Resource: "/project/:id/*", Action: "*"}}},
"/project/1/member",
"create",
},
want: true,
},
{
name: "project repository create test for resource key match",
args: args{
&userWithoutRoles{Username: "user1", UserPolicies: []*Policy{{Resource: "/project/1/*", Action: "create"}}},
"/project/1/repository",
"create",
},
want: true,
},
{
name: "project repository delete test for resource key match",
args: args{
&userWithoutRoles{Username: "user1", UserPolicies: []*Policy{{Resource: "/project/1/*", Action: "create"}}},
"/project/1/repository",
"delete",
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := HasPermission(tt.args.user, tt.args.resource, tt.args.action); got != tt.want {
t.Errorf("HasPermission() = %v, want %v", got, tt.want)
}
})
}
}
func TestHasPermissionPolicyDeny(t *testing.T) {
type args struct {
user User
resource Resource
action Action
}
tests := []struct {
name string
args args
want bool
}{
{
name: "project member create for resource deny",
args: args{
&userWithoutRoles{
Username: "user1",
UserPolicies: []*Policy{
{Resource: "/project/1/*", Action: "*"},
{Resource: "/project/1/member", Action: "create", Effect: EffectDeny},
},
},
"/project/1/member",
"create",
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := HasPermission(tt.args.user, tt.args.resource, tt.args.action); got != tt.want {
t.Errorf("HasPermission() = %v, want %v", got, tt.want)
}
})
}
}

View File

@ -146,8 +146,3 @@ func (u *BaseUser) GetUserName() string {
func (u *BaseUser) GetPolicies() []*Policy {
return nil
}
// HasPermission returns whether the user has action permission on resource
func HasPermission(user User, resource Resource, action Action) bool {
return enforcerForUser(user).Enforce(user.GetUserName(), resource.String(), action.String())
}

View File

@ -19,308 +19,6 @@ import (
"testing"
)
type role struct {
RoleName string
}
func (r *role) GetRoleName() string {
return r.RoleName
}
func (r *role) GetPolicies() []*Policy {
return []*Policy{
{Resource: "/project", Action: "create"},
{Resource: "/project", Action: "update"},
}
}
type userWithRoles struct {
Username string
RoleName string
BaseUser
}
func (u *userWithRoles) GetUserName() string {
return u.Username
}
func (u *userWithRoles) GetRoles() []Role {
return []Role{
&role{RoleName: u.RoleName},
}
}
type userWithoutRoles struct {
Username string
UserPolicies []*Policy
BaseUser
}
func (u *userWithoutRoles) GetUserName() string {
return u.Username
}
func (u *userWithoutRoles) GetPolicies() []*Policy {
return u.UserPolicies
}
func TestHasPermissionUserWithRoles(t *testing.T) {
type args struct {
user User
resource Resource
action Action
}
tests := []struct {
name string
args args
want bool
}{
{
name: "project create for project admin",
args: args{
&userWithRoles{Username: "project admin", RoleName: "projectAdmin"},
"/project",
"create",
},
want: true,
},
{
name: "project update for project admin",
args: args{
&userWithRoles{Username: "project admin", RoleName: "projectAdmin"},
"/project",
"update",
},
want: true,
},
{
name: "project delete for project admin",
args: args{
&userWithRoles{Username: "project admin", RoleName: "projectAdmin"},
"/project",
"delete",
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := HasPermission(tt.args.user, tt.args.resource, tt.args.action); got != tt.want {
t.Errorf("HasPermission() = %v, want %v", got, tt.want)
}
})
}
}
func TestHasPermissionUserWithoutRoles(t *testing.T) {
type args struct {
user User
resource Resource
action Action
}
tests := []struct {
name string
args args
want bool
}{
{
name: "project create for user without roles",
args: args{
&userWithoutRoles{Username: "user1", UserPolicies: []*Policy{{Resource: "/project", Action: "create"}}},
"/project",
"create",
},
want: true,
},
{
name: "project delete test for user without roles",
args: args{
&userWithoutRoles{Username: "user1", UserPolicies: []*Policy{{Resource: "/project", Action: "create"}}},
"/project",
"delete",
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := HasPermission(tt.args.user, tt.args.resource, tt.args.action); got != tt.want {
t.Errorf("HasPermission() = %v, want %v", got, tt.want)
}
})
}
}
func TestHasPermissionUsernameEmpty(t *testing.T) {
type args struct {
user User
resource Resource
action Action
}
tests := []struct {
name string
args args
want bool
}{
{
name: "project create for user without roles",
args: args{
&userWithoutRoles{Username: "", UserPolicies: []*Policy{{Resource: "/project", Action: "create"}}},
"/project",
"create",
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := HasPermission(tt.args.user, tt.args.resource, tt.args.action); got != tt.want {
t.Errorf("HasPermission() = %v, want %v", got, tt.want)
}
})
}
}
func TestHasPermissionRoleNameEmpty(t *testing.T) {
type args struct {
user User
resource Resource
action Action
}
tests := []struct {
name string
args args
want bool
}{
{
name: "project create for project admin",
args: args{
&userWithRoles{Username: "project admin", RoleName: ""},
"/project",
"create",
},
want: false,
},
{
name: "project update for project admin",
args: args{
&userWithRoles{Username: "project admin", RoleName: ""},
"/project",
"update",
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := HasPermission(tt.args.user, tt.args.resource, tt.args.action); got != tt.want {
t.Errorf("HasPermission() = %v, want %v", got, tt.want)
}
})
}
}
func TestHasPermissionResourceKeyMatch(t *testing.T) {
type args struct {
user User
resource Resource
action Action
}
tests := []struct {
name string
args args
want bool
}{
{
name: "project member create for resource key match",
args: args{
&userWithoutRoles{Username: "user1", UserPolicies: []*Policy{{Resource: "/project/1/*", Action: "*"}}},
"/project/1/member",
"create",
},
want: true,
},
{
name: "project member create for resource key match",
args: args{
&userWithoutRoles{Username: "user1", UserPolicies: []*Policy{{Resource: "/project/:id/*", Action: "*"}}},
"/project/1/member",
"create",
},
want: true,
},
{
name: "project repository create test for resource key match",
args: args{
&userWithoutRoles{Username: "user1", UserPolicies: []*Policy{{Resource: "/project/1/*", Action: "create"}}},
"/project/1/repository",
"create",
},
want: true,
},
{
name: "project repository delete test for resource key match",
args: args{
&userWithoutRoles{Username: "user1", UserPolicies: []*Policy{{Resource: "/project/1/*", Action: "create"}}},
"/project/1/repository",
"delete",
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := HasPermission(tt.args.user, tt.args.resource, tt.args.action); got != tt.want {
t.Errorf("HasPermission() = %v, want %v", got, tt.want)
}
})
}
}
func TestHasPermissionPolicyDeny(t *testing.T) {
type args struct {
user User
resource Resource
action Action
}
tests := []struct {
name string
args args
want bool
}{
{
name: "project member create for resource deny",
args: args{
&userWithoutRoles{
Username: "user1",
UserPolicies: []*Policy{
{Resource: "/project/1/*", Action: "*"},
{Resource: "/project/1/member", Action: "create", Effect: EffectDeny},
},
},
"/project/1/member",
"create",
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := HasPermission(tt.args.user, tt.args.resource, tt.args.action); got != tt.want {
t.Errorf("HasPermission() = %v, want %v", got, tt.want)
}
})
}
}
func TestResource_Subresource(t *testing.T) {
type args struct {
resources []Resource

View File

@ -15,18 +15,23 @@
package admiral
import (
"sync"
"github.com/goharbor/harbor/src/common/models"
"github.com/goharbor/harbor/src/common/rbac"
"github.com/goharbor/harbor/src/common/rbac/project"
"github.com/goharbor/harbor/src/common/security/admiral/authcontext"
"github.com/goharbor/harbor/src/common/utils/log"
"github.com/goharbor/harbor/src/core/promgr"
)
// SecurityContext implements security.Context interface based on
// auth context and project manager
type SecurityContext struct {
ctx *authcontext.AuthContext
pm promgr.ProjectManager
ctx *authcontext.AuthContext
pm promgr.ProjectManager
evaluator rbac.Evaluator
once sync.Once
}
// NewSecurityContext ...
@ -71,19 +76,24 @@ func (s *SecurityContext) IsSolutionUser() bool {
// Can returns whether the user can do action on resource
func (s *SecurityContext) Can(action rbac.Action, resource rbac.Resource) bool {
ns, err := resource.GetNamespace()
if err == nil {
switch ns.Kind() {
case "project":
s.once.Do(func() {
s.evaluator = rbac.NewNamespaceEvaluator("project", func(ns rbac.Namespace) rbac.Evaluator {
projectID := ns.Identity().(int64)
isPublicProject, _ := s.pm.IsPublic(projectID)
projectNamespace := rbac.NewProjectNamespace(projectID, isPublicProject)
user := project.NewUser(s, projectNamespace, s.GetProjectRoles(projectID)...)
return rbac.HasPermission(user, resource, action)
}
}
proj, err := s.pm.Get(projectID)
if err != nil {
log.Errorf("failed to get project %d, error: %v", projectID, err)
return nil
}
if proj == nil {
return nil
}
return false
user := project.NewUser(s, rbac.NewProjectNamespace(projectID, proj.IsPublic()), s.GetProjectRoles(projectID)...)
return rbac.NewUserEvaluator(user)
})
})
return s.evaluator != nil && s.evaluator.HasPermission(resource, action)
}
// GetMyProjects ...

View File

@ -15,6 +15,8 @@
package local
import (
"sync"
"github.com/goharbor/harbor/src/common"
"github.com/goharbor/harbor/src/common/dao"
"github.com/goharbor/harbor/src/common/models"
@ -26,8 +28,10 @@ import (
// SecurityContext implements security.Context interface based on database
type SecurityContext struct {
user *models.User
pm promgr.ProjectManager
user *models.User
pm promgr.ProjectManager
evaluator rbac.Evaluator
once sync.Once
}
// NewSecurityContext ...
@ -68,19 +72,24 @@ func (s *SecurityContext) IsSolutionUser() bool {
// Can returns whether the user can do action on resource
func (s *SecurityContext) Can(action rbac.Action, resource rbac.Resource) bool {
ns, err := resource.GetNamespace()
if err == nil {
switch ns.Kind() {
case "project":
s.once.Do(func() {
s.evaluator = rbac.NewNamespaceEvaluator("project", func(ns rbac.Namespace) rbac.Evaluator {
projectID := ns.Identity().(int64)
isPublicProject, _ := s.pm.IsPublic(projectID)
projectNamespace := rbac.NewProjectNamespace(projectID, isPublicProject)
user := project.NewUser(s, projectNamespace, s.GetProjectRoles(projectID)...)
return rbac.HasPermission(user, resource, action)
}
}
proj, err := s.pm.Get(projectID)
if err != nil {
log.Errorf("failed to get project %d, error: %v", projectID, err)
return nil
}
if proj == nil {
return nil
}
return false
user := project.NewUser(s, rbac.NewProjectNamespace(projectID, proj.IsPublic()), s.GetProjectRoles(projectID)...)
return rbac.NewUserEvaluator(user)
})
})
return s.evaluator != nil && s.evaluator.HasPermission(resource, action)
}
// GetProjectRoles ...

View File

@ -15,17 +15,22 @@
package robot
import (
"sync"
"github.com/goharbor/harbor/src/common/models"
"github.com/goharbor/harbor/src/common/rbac"
"github.com/goharbor/harbor/src/common/utils/log"
"github.com/goharbor/harbor/src/core/promgr"
"github.com/goharbor/harbor/src/pkg/robot/model"
)
// SecurityContext implements security.Context interface based on database
type SecurityContext struct {
robot *model.Robot
pm promgr.ProjectManager
policy []*rbac.Policy
robot *model.Robot
pm promgr.ProjectManager
policy []*rbac.Policy
evaluator rbac.Evaluator
once sync.Once
}
// NewSecurityContext ...
@ -73,17 +78,22 @@ func (s *SecurityContext) GetProjectRoles(projectIDOrName interface{}) []int {
// Can returns whether the robot can do action on resource
func (s *SecurityContext) Can(action rbac.Action, resource rbac.Resource) bool {
ns, err := resource.GetNamespace()
if err == nil {
switch ns.Kind() {
case "project":
s.once.Do(func() {
s.evaluator = rbac.NewNamespaceEvaluator("project", func(ns rbac.Namespace) rbac.Evaluator {
projectID := ns.Identity().(int64)
isPublicProject, _ := s.pm.IsPublic(projectID)
projectNamespace := rbac.NewProjectNamespace(projectID, isPublicProject)
robot := NewRobot(s.GetUsername(), projectNamespace, s.policy)
return rbac.HasPermission(robot, resource, action)
}
}
proj, err := s.pm.Get(projectID)
if err != nil {
log.Errorf("failed to get project %d, error: %v", projectID, err)
return nil
}
if proj == nil {
return nil
}
return false
robot := NewRobot(s.GetUsername(), rbac.NewProjectNamespace(projectID, proj.IsPublic()), s.policy)
return rbac.NewUserEvaluator(robot)
})
})
return s.evaluator != nil && s.evaluator.HasPermission(resource, action)
}