Merge pull request #1293 in YUNIONIO/onecloud from ~QIUJIAN/onecloud:hotfix/qj-rbac-simplify-condition to release/2.6.0

* commit '78faed91a4ee0ebda6608f55bc6ae924195df0e3':
  fix: SyncOnce should invalidate policy cache
  fix: refine policy match conditions
  minor updates
  fix: simplify rbac policy condition to match projects and roles
This commit is contained in:
邱剑
2019-03-21 11:31:11 +08:00
7 changed files with 233 additions and 95 deletions
+6 -1
View File
@@ -253,7 +253,7 @@ func init() {
}
req.Add(jsonutils.NewArray(data...), key)
}
fmt.Println(req.String())
fmt.Println("Request:", req.String())
var token mcclient.TokenCredential
if len(args.User) > 0 {
@@ -273,6 +273,11 @@ func init() {
}
printObject(result)
fmt.Println("userCred:", token)
m := policy.PolicyManager.MatchedPolicies(false, token)
fmt.Println("matched policies:", m)
m = policy.PolicyManager.MatchedPolicies(true, token)
fmt.Println("matched admin policies:", m)
return nil
})
}
+21 -13
View File
@@ -16,7 +16,6 @@ import (
"yunion.io/x/onecloud/pkg/mcclient"
"yunion.io/x/onecloud/pkg/mcclient/auth"
"yunion.io/x/onecloud/pkg/mcclient/modules"
"yunion.io/x/onecloud/pkg/util/conditionparser"
"yunion.io/x/onecloud/pkg/util/hashcache"
"yunion.io/x/onecloud/pkg/util/rbacutils"
)
@@ -148,6 +147,7 @@ func (manager *SPolicyManager) SyncOnce() error {
manager.adminPolicies = adminPolicies
manager.lastSync = time.Now()
manager.cache.Invalidate()
return nil
}
@@ -231,27 +231,21 @@ func (manager *SPolicyManager) allowWithoutCache(isAdmin bool, userCred mcclient
log.Warningf("no policies fetched")
return rbacutils.Deny
}
var userCredJson jsonutils.JSONObject
if userCred != nil {
userCredJson = userCred.ToJson()
} else {
userCredJson = jsonutils.NewDict()
}
currentPriv := rbacutils.Deny
for _, p := range policies {
result := p.Allow(userCredJson, service, resource, action, extra...)
result := p.Allow(userCred, service, resource, action, extra...)
if currentPriv.StricterThan(result) {
currentPriv = result
}
}
if !isAdmin && manager.defaultPolicy != nil {
result := manager.defaultPolicy.Allow(userCredJson, service, resource, action, extra...)
result := manager.defaultPolicy.Allow(userCred, service, resource, action, extra...)
if currentPriv.StricterThan(result) {
currentPriv = result
}
}
if consts.IsRbacDebug() {
log.Debugf("[RBAC: %v] %s %s %s %#v permission %s userCred: %s", isAdmin, service, resource, action, extra, currentPriv, userCredJson)
log.Debugf("[RBAC: %v] %s %s %s %#v permission %s userCred: %s", isAdmin, service, resource, action, extra, currentPriv, userCred)
}
return unifyRbacResult(isAdmin, currentPriv)
}
@@ -365,12 +359,26 @@ func (manager *SPolicyManager) IsAdminCapable(userCred mcclient.TokenCredential)
return true
}
userCredJson := userCred.ToJson()
for _, p := range manager.adminPolicies {
match, _ := conditionparser.Eval(p.Condition, userCredJson)
if match {
if p.Match(userCred) {
return true
}
}
return false
}
func (manager *SPolicyManager) MatchedPolicies(isAdmin bool, userCred mcclient.TokenCredential) []string {
var policies map[string]rbacutils.SRbacPolicy
if isAdmin {
policies = manager.adminPolicies
} else {
policies = manager.policies
}
ret := make([]string, 0)
for k, p := range policies {
if p.Match(userCred) {
ret = append(ret, k)
}
}
return ret
}
+10
View File
@@ -4,6 +4,7 @@ import (
"yunion.io/x/jsonutils"
"yunion.io/x/onecloud/pkg/mcclient"
"yunion.io/x/onecloud/pkg/util/rbacutils"
)
type SPolicyManager struct {
@@ -17,7 +18,16 @@ func policyReadFilter(session *mcclient.ClientSession, s jsonutils.JSONObject, q
ret := ss.CopyIncludes("id", "type")
blobStr, _ := ss.GetString("blob")
if len(blobStr) > 0 {
policy := rbacutils.SRbacPolicy{}
blobJson, _ := jsonutils.ParseString(blobStr)
err := policy.Decode(blobJson)
if err != nil {
return nil, err
}
blobJson, err = policy.Encode()
if err != nil {
return nil, err
}
var format string
if query != nil {
format, _ = query.GetString("format")
+19
View File
@@ -8,12 +8,22 @@ import (
"time"
)
var initTime = time.Time{}
type cacheNode struct {
key string
expire time.Time
value interface{}
}
func (node *cacheNode) reset() {
if !node.expire.IsZero() {
node.key = ""
node.expire = initTime
node.value = nil
}
}
type Cache struct {
table []cacheNode
lock *sync.Mutex
@@ -101,3 +111,12 @@ func (c *Cache) AtomicSet(key string, val interface{}) {
defer c.lock.Unlock()
c.Set(key, val)
}
func (c *Cache) Invalidate() {
c.lock.Lock()
defer c.lock.Unlock()
for i := range c.table {
c.table[i].reset()
}
}
+11
View File
@@ -46,4 +46,15 @@ func TestCache(t *testing.T) {
if v != nil {
t.Errorf("key 123 shoud expire")
}
c.Set("123", 1234)
v = c.Get("123")
if v == nil || v.(int) != 1234 {
t.Error("Key 123 not found")
}
c.Invalidate()
v = c.Get("123")
if v != nil {
t.Error("Key 123 should not found")
}
}
+79 -12
View File
@@ -2,11 +2,11 @@ package rbacutils
import (
"fmt"
"regexp"
"yunion.io/x/jsonutils"
"yunion.io/x/log"
"yunion.io/x/onecloud/pkg/util/conditionparser"
"yunion.io/x/onecloud/pkg/mcclient"
)
type TRbacResult string
@@ -45,6 +45,8 @@ type SRbacPolicy struct {
Condition string
IsAdmin bool
Rules []SRbacRule
Projects []string
Roles []string
}
type SRbacRule struct {
@@ -188,9 +190,48 @@ func CompactRules(rules []SRbacRule) []SRbacRule {
return output
}
var (
tenantEqualsPattern = regexp.MustCompile(`tenant\s*==\s*['"]?(\w+)['"]?`)
roleContainsPattern = regexp.MustCompile(`roles.contains\(['"]?(\w+)['"]?\)`)
)
func searchMatchStrings(pattern *regexp.Regexp, condstr string) []string {
ret := make([]string, 0)
matches := pattern.FindAllStringSubmatch(condstr, -1)
for _, match := range matches {
ret = append(ret, match[1])
}
return ret
}
func searchMatchTenants(condstr string) []string {
return searchMatchStrings(tenantEqualsPattern, condstr)
}
func searchMatchRoles(condstr string) []string {
return searchMatchStrings(roleContainsPattern, condstr)
}
func (policy *SRbacPolicy) Decode(policyJson jsonutils.JSONObject) error {
policy.Condition, _ = policyJson.GetString("condition")
policy.IsAdmin = jsonutils.QueryBoolean(policyJson, "is_admin", false)
if policyJson.Contains("projects") {
projectJson, _ := policyJson.GetArray("projects")
policy.Projects = jsonutils.JSONArray2StringArray(projectJson)
}
if policyJson.Contains("roles") {
roleJson, _ := policyJson.GetArray("roles")
policy.Roles = jsonutils.JSONArray2StringArray(roleJson)
}
if len(policy.Projects) == 0 && len(policy.Roles) == 0 && len(policy.Condition) > 0 {
// XXX hack
// for smooth transtion from condition to projects&roles
policy.Projects = searchMatchTenants(policy.Condition)
policy.Roles = searchMatchRoles(policy.Condition)
}
// empty condition, no longer use this field
policy.Condition = ""
ruleJson, err := policyJson.Get("policy")
if err != nil {
@@ -288,7 +329,7 @@ func (rule *SRbacRule) toStringArray() []string {
strArr = append(strArr, rule.Extra...)
}
i := len(strArr) - 1
for i >= 0 && (len(strArr[i]) == 0 || strArr[i] == WILD_MATCH) {
for i > 0 && (len(strArr[i]) == 0 || strArr[i] == WILD_MATCH) {
i -= 1
}
return strArr[0 : i+1]
@@ -346,12 +387,18 @@ func (policy *SRbacPolicy) Encode() (jsonutils.JSONObject, error) {
}
ret := jsonutils.NewDict()
ret.Add(jsonutils.NewString(policy.Condition), "condition")
// ret.Add(jsonutils.NewString(policy.Condition), "condition")
if policy.IsAdmin {
ret.Add(jsonutils.JSONTrue, "is_admin")
} else {
ret.Add(jsonutils.JSONFalse, "is_admin")
}
if len(policy.Projects) > 0 {
ret.Add(jsonutils.NewStringArray(policy.Projects), "projects")
}
if len(policy.Roles) > 0 {
ret.Add(jsonutils.NewStringArray(policy.Roles), "roles")
}
ret.Add(rules, "policy")
return ret, nil
}
@@ -369,17 +416,37 @@ func (policy *SRbacPolicy) Explain(request [][]string) [][]string {
return output
}
func (policy *SRbacPolicy) Allow(userCred jsonutils.JSONObject, service, resource, action string, extra ...string) TRbacResult {
if len(policy.Condition) > 0 {
match, err := conditionparser.Eval(policy.Condition, userCred)
if err != nil {
log.Errorf("eval condition %s fail %s", policy.Condition, err)
return Deny
func contains(s1 []string, s string) bool {
for i := range s1 {
if s1[i] == s {
return true
}
if !match {
return Deny
}
return false
}
func intersect(s1 []string, s2 []string) bool {
for i := range s1 {
for j := range s2 {
if s1[i] == s2[j] {
return true
}
}
}
return false
}
func (policy *SRbacPolicy) Match(userCred mcclient.TokenCredential) bool {
if (len(policy.Projects) == 0 || (userCred != nil && contains(policy.Projects, userCred.GetProjectName()))) && (len(policy.Roles) == 0 || (userCred != nil && intersect(policy.Roles, userCred.GetRoles()))) {
return true
}
return false
}
func (policy *SRbacPolicy) Allow(userCred mcclient.TokenCredential, service, resource, action string, extra ...string) TRbacResult {
if !policy.Match(userCred) {
return Deny
}
rule := policy.GetMatchRule(service, resource, action, extra...)
if rule == nil {
return Deny
+87 -69
View File
@@ -4,6 +4,8 @@ import (
"testing"
"yunion.io/x/jsonutils"
"yunion.io/x/onecloud/pkg/mcclient"
)
func TestSRabcRule_Match(t *testing.T) {
@@ -179,60 +181,6 @@ func TestSRabcPolicy_Encode(t *testing.T) {
t.Logf("%s", policyStr1)
}
func TestSRbacPolicy_Allow(t *testing.T) {
userCredStr := `{"domain":"Default","domain_id":"default","expires":"2018-10-28T05:33:54.000000Z","roles":"admin,teamleader","tenant":"system","tenant_id":"5d65667d112e47249ae66dbd7bc07030","token":"gAAAAABb0_jC2Qz2PpB00-pieLi4exKXq4O3QrvoqerpqoSxbp9pOLLdNWaAg0cPcd8eAjkiPhSo7VWQAVodoxnad95LdNbUf_1It8R_wXVDtO20caB7oLcas1oQt8b1cG0a7qagauP0iWVSW_dq_e92rD5Hd3SHn3Lw6ycrp_eHLskz_8EbPiI","user":"sysadmin","user_id":"dddf386b6ff24572b2e6a771d768495e"}`
userCred, err := jsonutils.ParseString(userCredStr)
if err != nil {
t.Errorf("parse json fail %s", err)
return
}
cases := []struct {
policy string
ops []string
want TRbacResult
}{
{
`{
"condition": "tenant==\"system\" && roles.contains(\"admin\")",
"is_admin": true,
"policy": {
"*": "allow"
}
}`,
[]string{"compute", "servers", "list"},
Allow,
},
{
`{"is_admin":"false","policy":{"*":{"*":{"*":"allow","delete":"deny"}}}}`,
[]string{"compute", "servers", "delete"},
Deny,
},
}
for _, c := range cases {
policyJson, err := jsonutils.ParseString(c.policy)
if err != nil {
t.Errorf("fail to parse json string %s", err)
return
}
policy := SRbacPolicy{}
err = policy.Decode(policyJson)
if err != nil {
t.Errorf("decode error %s", err)
return
}
isAllow := policy.Allow(userCred, c.ops[0], c.ops[1], c.ops[2])
if isAllow != c.want {
t.Errorf("%s %#v expect %#v, but get %#v", policyJson.String(), c.ops, c.want, isAllow)
return
}
}
}
func TestSRabcPolicy_Explain(t *testing.T) {
policyStr := `{
"condition": "usercred.project != \"system\" && usercred.roles==\"projectowner\"",
@@ -281,24 +229,94 @@ func TestSRabcPolicy_Explain(t *testing.T) {
t.Logf("%#v", output)
}
func TestTRbacResult_IsHigherPrivilege(t *testing.T) {
cases := []struct {
t1 TRbacResult
t2 TRbacResult
want bool
}{
{Allow, Allow, false},
{Allow, OwnerAllow, true},
{Deny, Deny, false},
{Deny, OwnerAllow, false},
{Deny, Allow, false},
{OwnerAllow, Allow, false},
}
func TestConditionParser(t *testing.T) {
condition := `tenant=="system" && roles.contains("admin")`
tenants := searchMatchTenants(condition)
t.Logf("%s", tenants)
roles := searchMatchRoles(condition)
t.Logf("%s", roles)
}
func TestSRbacPolicyMatch(t *testing.T) {
cases := []struct {
policy SRbacPolicy
userCred mcclient.TokenCredential
want bool
}{
{
SRbacPolicy{},
&mcclient.SSimpleToken{},
true,
},
{
SRbacPolicy{},
nil,
true,
},
{
SRbacPolicy{
Projects: []string{"system"},
},
&mcclient.SSimpleToken{
Project: "system",
},
true,
},
{
SRbacPolicy{
Projects: []string{"system"},
},
&mcclient.SSimpleToken{
Project: "demo",
},
false,
},
{
SRbacPolicy{
Projects: []string{"system"},
Roles: []string{"admin"},
},
&mcclient.SSimpleToken{
Project: "system",
Roles: "admin",
},
true,
},
{
SRbacPolicy{
Projects: []string{"system"},
Roles: []string{"admin"},
},
&mcclient.SSimpleToken{
Project: "system",
Roles: "admin,_member_",
},
true,
},
{
SRbacPolicy{
Projects: []string{"system"},
Roles: []string{"admin"},
},
&mcclient.SSimpleToken{
Project: "system",
Roles: "_member_",
},
false,
},
{
SRbacPolicy{
Projects: []string{"system"},
Roles: []string{"admin"},
},
nil,
false,
},
}
for _, c := range cases {
got := c.t1.IsHigherPrivilege(c.t2)
got := c.policy.Match(c.userCred)
if got != c.want {
t.Errorf("%s IsHigherPrivilege %s want %v got %v", c.t1, c.t2, c.want, got)
t.Errorf("%#v %#v got %v want %v", c.policy, c.userCred, got, c.want)
}
}
}