fix: expand policy tags with org node tags (#19416)

Co-authored-by: Qiu Jian <qiujian@yunionyun.com>

pkg/apis/identity/policy.go

@@ -26,7 +26,7 @@ type SOrganizationNodeInfo struct {
 	FullLabel    string
 	OrgId        string
 	Organization string
-	Tags         []tagutils.STag
+	Tags         tagutils.TTagSet
 	Type         TOrgType
 }
 

pkg/cloudcommon/db/db_dispatcher.go

@@ -587,7 +587,7 @@ func ListItems(manager IModelManager, ctx context.Context, userCred mcclient.Tok
 		// query senders are responsible for clear up other constraint
 		// like setting "pendinge_delete" to "all"
 		queryDelete, _ := query.GetString("delete")
-		if queryDelete == "all" && userCred.HasSystemAdminPrivilege() {
+		if queryDelete == "all" && policy.PolicyManager.Allow(rbacscope.ScopeSystem, userCred, consts.GetServiceType(), manager.KeywordPlural(), policy.PolicyActionList).Result.IsAllow() {
 			useRawQuery = true
 		}
 	}

pkg/cloudcommon/policy/policy.go

@@ -30,6 +30,7 @@ import (
 	"yunion.io/x/pkg/util/rbacscope"
 
 	"yunion.io/x/onecloud/pkg/apis"
+	identity_api "yunion.io/x/onecloud/pkg/apis/identity"
 	"yunion.io/x/onecloud/pkg/appsrv"
 	"yunion.io/x/onecloud/pkg/cloudcommon/consts"
 	"yunion.io/x/onecloud/pkg/httperrors"
@@ -92,11 +93,34 @@ type sPolicyData struct {
 	Policy        jsonutils.JSONObject `json:"policy"`
 	DomainTags    tagutils.TTagSet     `json:"domain_tags"`
 	ProjectTags   tagutils.TTagSet     `json:"project_tags"`
-	ResourceTags  tagutils.TTagSet     `json:"resource_tags"`
+	ObjectTags    tagutils.TTagSet     `json:"resource_tags"`
+
+	OrgNodes []identity_api.SOrganizationNodeInfo `json:"org_nodes"`
 }
 
 func (data sPolicyData) getPolicy() (*rbacutils.SPolicy, error) {
-	return rbacutils.DecodePolicyData(data.DomainTags, data.ProjectTags, data.ResourceTags, data.Policy)
+	var domainTags, projectTags, objectTags tagutils.TTagSetList
+	if len(data.DomainTags) > 0 {
+		domainTags = domainTags.Append(data.DomainTags)
+	}
+	if len(data.ProjectTags) > 0 {
+		projectTags = projectTags.Append(data.ProjectTags)
+	}
+	if len(data.ObjectTags) > 0 {
+		objectTags = objectTags.Append(data.ObjectTags)
+	}
+	for i := range data.OrgNodes {
+		orgNode := data.OrgNodes[i]
+		switch orgNode.Type {
+		case identity_api.OrgTypeDomain:
+			domainTags = domainTags.Append(orgNode.Tags)
+		case identity_api.OrgTypeProject:
+			projectTags = projectTags.Append(orgNode.Tags)
+		case identity_api.OrgTypeObject:
+			objectTags = objectTags.Append(orgNode.Tags)
+		}
+	}
+	return rbacutils.DecodePolicyData(domainTags, projectTags, objectTags, data.Policy)
 }
 
 func (manager *SPolicyManager) init(refreshInterval time.Duration, workerCount int) {
@@ -464,9 +488,9 @@ func explainPolicyInternal(userCred mcclient.TokenCredential, policyReq jsonutil
 			result = rbacutils.PolicyDeny
 			if match != nil {
 				result.Result = match.Rule.Result
-				result.DomainTags = tagutils.TTagSetList{match.DomainTags}
-				result.ProjectTags = tagutils.TTagSetList{match.ProjectTags}
-				result.ObjectTags = tagutils.TTagSetList{match.ObjectTags}
+				result.DomainTags = match.DomainTags
+				result.ProjectTags = match.ProjectTags
+				result.ObjectTags = match.ObjectTags
 			}
 		}
 	}

pkg/keystone/models/policies.go

@@ -296,9 +296,11 @@ func (manager *SPolicyManager) ValidateCreateData(
 		input.Name = input.Type
 	}
 
+	var domainTags, projectTags, objectTags tagutils.TTagSetList
+
 	if len(input.OrgNodeId) > 0 {
 		for i := range input.OrgNodeId {
-			orgNode, err := OrganizationNodeManager.FetchById(input.OrgNodeId[i])
+			orgNodeObj, err := OrganizationNodeManager.FetchById(input.OrgNodeId[i])
 			if err != nil {
 				if errors.Cause(err) == sql.ErrNoRows {
 					return input, httperrors.NewResourceNotFoundError2(OrganizationNodeManager.Keyword(), input.OrgNodeId[i])
@@ -306,11 +308,34 @@ func (manager *SPolicyManager) ValidateCreateData(
 					return input, errors.Wrap(err, "OrganizationNodeManager.FetchById")
 				}
 			}
-			input.OrgNodeId[i] = orgNode.GetId()
+			orgNode := orgNodeObj.(*SOrganizationNode)
+			input.OrgNodeId[i] = orgNode.Id
+			org, err := orgNode.GetOrganization()
+			if err != nil {
+				return input, errors.Wrap(err, "orgNode.GetOrganization")
+			}
+			switch org.Type {
+			case api.OrgTypeDomain:
+				domainTags = domainTags.Append(orgNode.GetTagSet(org))
+			case api.OrgTypeProject:
+				projectTags = projectTags.Append(orgNode.GetTagSet(org))
+			case api.OrgTypeObject:
+				objectTags = objectTags.Append(orgNode.GetTagSet(org))
+			}
 		}
 	}
 
-	policy, err := rbacutils.DecodePolicyData(input.DomainTags, input.ProjectTags, input.ObjectTags, input.Blob)
+	if len(input.DomainTags) > 0 {
+		domainTags = domainTags.Append(input.DomainTags)
+	}
+	if len(input.ProjectTags) > 0 {
+		projectTags = projectTags.Append(input.ProjectTags)
+	}
+	if len(input.ObjectTags) > 0 {
+		objectTags = objectTags.Append(input.ObjectTags)
+	}
+
+	policy, err := rbacutils.DecodePolicyData(domainTags, projectTags, objectTags, input.Blob)
 	if err != nil {
 		return input, httperrors.NewInputParameterError("fail to decode policy data")
 	}
@@ -374,14 +399,24 @@ func (policy *SPolicy) ValidateUpdateData(ctx context.Context, userCred mcclient
 		}
 	}
 
+	var tagChanged bool
+
 	switch input.TagUpdatePolicy {
 	case api.TAG_UPDATE_POLICY_REMOVE:
-		input.DomainTags = policy.DomainTags.Remove(input.DomainTags...)
-		input.ProjectTags = policy.ProjectTags.Remove(input.ProjectTags...)
-		input.ObjectTags = policy.ObjectTags.Remove(input.ObjectTags...)
+		var domainChanged, projectChanged, objectChanged bool
+		input.DomainTags, domainChanged = policy.DomainTags.Remove(input.DomainTags...)
+		input.ProjectTags, projectChanged = policy.ProjectTags.Remove(input.ProjectTags...)
+		input.ObjectTags, objectChanged = policy.ObjectTags.Remove(input.ObjectTags...)
+		if domainChanged || projectChanged || objectChanged {
+			tagChanged = true
+		}
 	case api.TAG_UPDATE_POLICY_REPLACE:
 		// do nothing
+		tagChanged = true
 	default:
+		if len(input.DomainTags) > 0 || len(input.ProjectTags) > 0 || len(input.ObjectTags) > 0 {
+			tagChanged = true
+		}
 		input.DomainTags = policy.DomainTags.Append(input.DomainTags...)
 		input.ProjectTags = policy.ProjectTags.Append(input.ProjectTags...)
 		input.ObjectTags = policy.ObjectTags.Append(input.ObjectTags...)
@@ -406,6 +441,8 @@ func (policy *SPolicy) ValidateUpdateData(ctx context.Context, userCred mcclient
 		for i := range policy.OrgNodeId {
 			if !utils.IsInArray(policy.OrgNodeId[i], input.OrgNodeId) {
 				nodeIds = append(nodeIds, policy.OrgNodeId[i])
+			} else {
+				tagChanged = true
 			}
 		}
 		input.OrgNodeId = nodeIds
@@ -419,8 +456,38 @@ func (policy *SPolicy) ValidateUpdateData(ctx context.Context, userCred mcclient
 		}
 	}
 
-	if input.Blob != nil {
-		p, err := rbacutils.DecodePolicyData(input.DomainTags, input.ProjectTags, input.ObjectTags, input.Blob)
+	if input.Blob != nil || tagChanged {
+		var domainTags, projectTags, objectTags tagutils.TTagSetList
+		if len(input.DomainTags) > 0 {
+			domainTags = domainTags.Append(input.DomainTags)
+		}
+		if len(input.ProjectTags) > 0 {
+			projectTags = projectTags.Append(input.ProjectTags)
+		}
+		if len(input.ObjectTags) > 0 {
+			objectTags = objectTags.Append(input.DomainTags)
+		}
+		for i := range input.OrgNodeId {
+			orgNodeObj, err := OrganizationNodeManager.FetchById(input.OrgNodeId[i])
+			if err != nil {
+				return input, errors.Wrapf(err, "OrganizationNodeManager.FetchById %s", input.OrgNodeId[i])
+			}
+			orgNode := orgNodeObj.(*SOrganizationNode)
+			org, err := orgNode.GetOrganization()
+			if err != nil {
+				return input, errors.Wrap(err, "GetOrganization")
+			}
+			switch org.Type {
+			case api.OrgTypeDomain:
+				domainTags = domainTags.Append(orgNode.GetTagSet(org))
+			case api.OrgTypeProject:
+				projectTags = projectTags.Append(orgNode.GetTagSet(org))
+			case api.OrgTypeObject:
+				objectTags = objectTags.Append(orgNode.GetTagSet(org))
+			}
+		}
+
+		p, err := rbacutils.DecodePolicyData(domainTags, projectTags, objectTags, input.Blob)
 		if err != nil {
 			return input, httperrors.NewInputParameterError("fail to decode policy data")
 		}
@@ -653,7 +720,39 @@ func (policy *SPolicy) GetSharedDomains() []string {
 }
 
 func (policy *SPolicy) getPolicy() (*rbacutils.SPolicy, error) {
-	pc, err := rbacutils.DecodePolicyData(policy.DomainTags, policy.ProjectTags, policy.ObjectTags, policy.Blob)
+	var domainTags, projectTags, objectTags tagutils.TTagSetList
+	if len(policy.DomainTags) > 0 {
+		domainTags = domainTags.Append(policy.DomainTags)
+	}
+	if len(policy.ProjectTags) > 0 {
+		projectTags = projectTags.Append(policy.ProjectTags)
+	}
+	if len(policy.ObjectTags) > 0 {
+		objectTags = objectTags.Append(policy.ObjectTags)
+	}
+	var errs []error
+	for i := range policy.OrgNodeId {
+		orgNodeObj, err := OrganizationNodeManager.FetchById(policy.OrgNodeId[i])
+		if err != nil {
+			errs = append(errs, err)
+		} else {
+			orgNode := orgNodeObj.(*SOrganizationNode)
+			org, err := orgNode.GetOrganization()
+			if err != nil {
+				errs = append(errs, err)
+			} else {
+				switch org.Type {
+				case api.OrgTypeDomain:
+					domainTags = domainTags.Append(orgNode.GetTagSet(org))
+				case api.OrgTypeProject:
+					projectTags = projectTags.Append(orgNode.GetTagSet(org))
+				case api.OrgTypeObject:
+					objectTags = objectTags.Append(orgNode.GetTagSet(org))
+				}
+			}
+		}
+	}
+	pc, err := rbacutils.DecodePolicyData(domainTags, projectTags, objectTags, policy.Blob)
 	if err != nil {
 		return nil, errors.Wrap(err, "Decode")
 	}

pkg/util/rbacutils/policytags.go

@@ -26,11 +26,11 @@ type SPolicy struct {
 	// policy rules
 	Rules TPolicy
 	// tags for domains
-	DomainTags tagutils.TTagSet
+	DomainTags tagutils.TTagSetList
 	// tags for projects
-	ProjectTags tagutils.TTagSet
+	ProjectTags tagutils.TTagSetList
 	// tags for resources
-	ObjectTags tagutils.TTagSet
+	ObjectTags tagutils.TTagSetList
 }
 
 func (policy SPolicy) GetMatchRule(service string, resource string, action string, extra ...string) *SPolicyMatch {
@@ -47,10 +47,10 @@ func (policy SPolicy) GetMatchRule(service string, resource string, action strin
 }
 
 func DecodePolicy(policyJson jsonutils.JSONObject) (*SPolicy, error) {
-	tags := []tagutils.TTagSet{
-		make(tagutils.TTagSet, 0), // domain
-		make(tagutils.TTagSet, 0), // project
-		make(tagutils.TTagSet, 0), // resource
+	tags := []tagutils.TTagSetList{
+		tagutils.TTagSetList{}, // domain
+		tagutils.TTagSetList{}, // project
+		tagutils.TTagSetList{}, // resource
 	}
 	for i, key := range []string{
 		DomainTagsKey,
@@ -60,7 +60,13 @@ func DecodePolicy(policyJson jsonutils.JSONObject) (*SPolicy, error) {
 		if policyJson.Contains(key) {
 			err := policyJson.Unmarshal(&tags[i], key)
 			if err != nil {
-				return nil, errors.Wrapf(err, "Unmarshal %s", key)
+				tmpTagSet := make(tagutils.TTagSet, 0)
+				err2 := policyJson.Unmarshal(&tmpTagSet, key)
+				if err2 == nil {
+					tags[i] = tags[i].Append(tmpTagSet)
+				} else {
+					return nil, errors.Wrapf(errors.NewAggregate([]error{err, err2}), "Unmarshal %s", key)
+				}
 			}
 		}
 	}
@@ -76,7 +82,7 @@ func DecodePolicy(policyJson jsonutils.JSONObject) (*SPolicy, error) {
 	}, nil
 }
 
-func DecodePolicyData(domainTags, projectTags, objectTags tagutils.TTagSet, input jsonutils.JSONObject) (*SPolicy, error) {
+func DecodePolicyData(domainTags, projectTags, objectTags tagutils.TTagSetList, input jsonutils.JSONObject) (*SPolicy, error) {
 	rules, err := DecodeRawPolicyData(input)
 	if err != nil {
 		return nil, errors.Wrap(err, "decodePolicyData")
@@ -114,13 +120,13 @@ func (policy1 SPolicy) Contains(policy2 SPolicy) bool {
 	if !policy1.Rules.Contains(policy2.Rules) {
 		return false
 	}
-	if !policy1.DomainTags.Contains(policy2.DomainTags) {
+	if !policy1.DomainTags.ContainsAll(policy2.DomainTags) {
 		return false
 	}
-	if !policy1.ProjectTags.Contains(policy2.ProjectTags) {
+	if !policy1.ProjectTags.ContainsAll(policy2.ProjectTags) {
 		return false
 	}
-	if !policy1.ObjectTags.Contains(policy2.ObjectTags) {
+	if !policy1.ObjectTags.ContainsAll(policy2.ObjectTags) {
 		return false
 	}
 	return true

pkg/util/rbacutils/policytags_test.go

@@ -47,10 +47,12 @@ func TestSPolicy_Contains(t *testing.T) {
 					},
 				},
 				DomainTags: nil,
-				ProjectTags: tagutils.TTagSet{
-					tagutils.STag{
-						Key:   "project",
-						Value: "a",
+				ProjectTags: tagutils.TTagSetList{
+					tagutils.TTagSet{
+						tagutils.STag{
+							Key:   "project",
+							Value: "a",
+						},
 					},
 				},
 				ObjectTags: nil,
@@ -80,14 +82,16 @@ func TestSPolicy_Contains(t *testing.T) {
 					},
 				},
 				DomainTags: nil,
-				ProjectTags: tagutils.TTagSet{
-					tagutils.STag{
-						Key:   "project",
-						Value: "a",
-					},
-					tagutils.STag{
-						Key:   "env",
-						Value: "test",
+				ProjectTags: tagutils.TTagSetList{
+					tagutils.TTagSet{
+						tagutils.STag{
+							Key:   "project",
+							Value: "a",
+						},
+						tagutils.STag{
+							Key:   "env",
+							Value: "test",
+						},
 					},
 				},
 				ObjectTags: nil,
@@ -108,10 +112,12 @@ func TestSPolicy_Contains(t *testing.T) {
 					},
 				},
 				DomainTags: nil,
-				ProjectTags: tagutils.TTagSet{
-					tagutils.STag{
-						Key:   "project",
-						Value: "a",
+				ProjectTags: tagutils.TTagSetList{
+					tagutils.TTagSet{
+						tagutils.STag{
+							Key:   "project",
+							Value: "a",
+						},
 					},
 				},
 				ObjectTags: nil,

pkg/util/rbacutils/results.go

@@ -24,9 +24,9 @@ import (
 
 type SPolicyMatch struct {
 	Rule        SRbacRule
-	DomainTags  tagutils.TTagSet
-	ProjectTags tagutils.TTagSet
-	ObjectTags  tagutils.TTagSet
+	DomainTags  tagutils.TTagSetList
+	ProjectTags tagutils.TTagSetList
+	ObjectTags  tagutils.TTagSetList
 }
 
 type SPolicyResult struct {
@@ -55,9 +55,9 @@ func (matches TPolicyMatches) GetResult() SPolicyResult {
 	for _, match := range matches {
 		if match.Rule.Result == Allow {
 			result.Result = Allow
-			result.DomainTags = append(result.DomainTags, match.DomainTags)
-			result.ProjectTags = append(result.ProjectTags, match.ProjectTags)
-			result.ObjectTags = append(result.ObjectTags, match.ObjectTags)
+			result.DomainTags = result.DomainTags.AppendAll(match.DomainTags)
+			result.ProjectTags = result.ProjectTags.AppendAll(match.ProjectTags)
+			result.ObjectTags = result.ObjectTags.AppendAll(match.ObjectTags)
 			if len(match.DomainTags) == 0 {
 				isWideDomainTag = true
 			}

pkg/util/tagutils/tagset.go

@@ -108,10 +108,11 @@ func (ts TTagSet) add(e STag) TTagSet {
 	return ts
 }
 
-func (ts TTagSet) Remove(ele ...STag) TTagSet {
+func (ts TTagSet) Remove(ele ...STag) (TTagSet, bool) {
 	if len(ts) == 0 {
-		return ts
+		return ts, false
 	}
+	changed := false
 	for _, e := range ele {
 		if len(e.Value) == 0 {
 			e.Value = AnyValue
@@ -120,12 +121,13 @@ func (ts TTagSet) Remove(ele ...STag) TTagSet {
 		if !find {
 			continue
 		}
+		changed = true
 		if pos < len(ts)-1 {
 			copy(ts[pos:], ts[pos+1:])
 		}
 		ts = ts[:len(ts)-1]
 	}
-	return ts
+	return ts, changed
 }
 
 func (a TTagSet) Len() int      { return len(a) }

pkg/util/tagutils/tagsetlist.go

@@ -50,21 +50,11 @@ func (t1 TTagSetList) Contains(t2 TTagSet) bool {
 //	tagsetlist t1 contains tagsetlist t2 means any tag set of t2 is
 //	contained by one of the tag set of t1
 func (t1 TTagSetList) ContainsAll(t2 TTagSetList) bool {
-	if len(t2) == 0 {
+	if len(t1) == 0 {
 		return true
 	}
 	for _, ts2 := range t2 {
-		if len(t1) == 0 {
-			return false
-		}
-		contained := false
-		for _, ts1 := range t1 {
-			if ts1.Contains(ts2) {
-				contained = true
-				break
-			}
-		}
-		if !contained {
+		if !t1.Contains(ts2) {
 			return false
 		}
 	}
@@ -89,6 +79,13 @@ func (tsl TTagSetList) Append(t TTagSet) TTagSetList {
 	return ret
 }
 
+func (tsl TTagSetList) AppendAll(tsl2 TTagSetList) TTagSetList {
+	for i := range tsl2 {
+		tsl = tsl.Append(tsl2[i])
+	}
+	return tsl
+}
+
 func (tsl TTagSetList) Intersect(t TTagSet) TTagSetList {
 	ret := TTagSetList{}
 	for i := 0; i < len(tsl); i++ {