fix(cloudcommon): fall back list query scope to the resource default view (#25587)

When admin or scope query parameters do not map to a concrete view, use the resource's own scope and keep owner filters applied.

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

pkg/cloudcommon/db/domain.go

@@ -57,16 +57,6 @@ func (manager *SDomainizedResourceBaseManager) ResourceScope() rbacscope.TRbacSc
 func (manager *SDomainizedResourceBaseManager) FilterByOwner(ctx context.Context, q *sqlchemy.SQuery, man FilterByOwnerProvider, userCred mcclient.TokenCredential, owner mcclient.IIdentityProvider, scope rbacscope.TRbacScope) *sqlchemy.SQuery {
 	if owner != nil {
 		switch scope {
-		case rbacscope.ScopeProject, rbacscope.ScopeDomain:
-			q = q.Equals("domain_id", owner.GetProjectDomainId())
-			if userCred != nil {
-				result := policy.PolicyManager.Allow(scope, userCred, consts.GetServiceType(), man.KeywordPlural(), policy.PolicyActionList)
-				if !result.ObjectTags.IsEmpty() {
-					policyTagFilters := tagutils.STagFilters{}
-					policyTagFilters.AddFilters(result.ObjectTags)
-					q = ObjectIdQueryWithTagFilters(ctx, q, "id", man.Keyword(), policyTagFilters)
-				}
-			}
 		case rbacscope.ScopeSystem:
 			if userCred != nil {
 				result := policy.PolicyManager.Allow(scope, userCred, consts.GetServiceType(), man.KeywordPlural(), policy.PolicyActionList)
@@ -81,6 +71,18 @@ func (manager *SDomainizedResourceBaseManager) FilterByOwner(ctx context.Context
 					q = ObjectIdQueryWithTagFilters(ctx, q, "id", man.Keyword(), policyTagFilters)
 				}
 			}
+		default:
+			// domain view, also used when the requested scope has no
+			// dedicated case
+			q = q.Equals("domain_id", owner.GetProjectDomainId())
+			if userCred != nil {
+				result := policy.PolicyManager.Allow(scope, userCred, consts.GetServiceType(), man.KeywordPlural(), policy.PolicyActionList)
+				if !result.ObjectTags.IsEmpty() {
+					policyTagFilters := tagutils.STagFilters{}
+					policyTagFilters.AddFilters(result.ObjectTags)
+					q = ObjectIdQueryWithTagFilters(ctx, q, "id", man.Keyword(), policyTagFilters)
+				}
+			}
 		}
 	}
 	return q

pkg/cloudcommon/db/fetch.go

@@ -437,25 +437,9 @@ func FetchCheckQueryOwnerScope(
 	} else {
 		ownerId = userCred
 		reqScopeStr, _ := data.GetString("scope")
-		if len(reqScopeStr) > 0 {
-			if reqScopeStr == "max" || reqScopeStr == "maxallowed" {
-				queryScope = allowScope
-			} else {
-				queryScope = rbacscope.String2Scope(reqScopeStr)
-			}
-		} else if data.Contains("admin") {
-			isAdmin := jsonutils.QueryBoolean(data, "admin", false)
-			if isAdmin && allowScope.HigherThan(rbacscope.ScopeProject) {
-				queryScope = allowScope
-			}
-		} else if action == policy.PolicyActionGet {
-			queryScope = allowScope
-		} else {
-			queryScope = resScope
-		}
-		// if resScope.HigherThan(queryScope) {
-		// 	queryScope = resScope
-		// }
+		hasAdmin := data.Contains("admin")
+		isAdmin := hasAdmin && jsonutils.QueryBoolean(data, "admin", false)
+		queryScope = resolveQueryScope(reqScopeStr, hasAdmin, isAdmin, allowScope, resScope, action)
 		requireScope = queryScope
 	}
 	if doCheckRbac && (requireScope.HigherThan(allowScope) || policyTagFilters.Result.IsDeny()) {
@@ -466,6 +450,49 @@ func FetchCheckQueryOwnerScope(
 	return ownerId, queryScope, nil, policyTagFilters
 }
 
+// resolveQueryScope computes the scope at which a list/get query is filtered
+// when the request carries no explicit owner (no project_id/domain_id/user_id
+// filter).
+//
+// Semantics:
+//   - an explicit scope=system/domain/project selects that view; the RBAC
+//     caller check (requireScope) enforces the caller is allowed that scope.
+//     Querying a resource below its natural scope (e.g. project scope on a
+//     domain-scoped resource, resolved by custom FilterByOwner overrides) is
+//     a supported feature, so no blanket clamping to resScope is done here.
+//   - scope=max/maxallowed selects the widest scope the caller is allowed.
+//   - admin=true selects the widest allowed scope for callers above project
+//     scope; project-level callers fall back to the default resource view.
+//   - an unset (empty) or none scope, and a user scope on a non user-scoped
+//     resource, fall back to the natural scope of the resource.
+func resolveQueryScope(reqScopeStr string, hasAdmin, isAdmin bool, allowScope, resScope rbacscope.TRbacScope, action string) rbacscope.TRbacScope {
+	var queryScope rbacscope.TRbacScope
+	if len(reqScopeStr) > 0 {
+		if reqScopeStr == "max" || reqScopeStr == "maxallowed" {
+			queryScope = allowScope
+		} else {
+			queryScope = rbacscope.String2Scope(reqScopeStr)
+		}
+	} else if hasAdmin {
+		if isAdmin && allowScope.HigherThan(rbacscope.ScopeProject) {
+			queryScope = allowScope
+		} else {
+			// a project-level caller asking for the admin view (or sending
+			// admin=false) falls back to the default view of the resource
+			queryScope = resScope
+		}
+	} else if action == policy.PolicyActionGet {
+		queryScope = allowScope
+	} else {
+		queryScope = resScope
+	}
+	if queryScope == "" || queryScope == rbacscope.ScopeNone ||
+		(queryScope == rbacscope.ScopeUser && resScope != rbacscope.ScopeUser) {
+		queryScope = resScope
+	}
+	return queryScope
+}
+
 func mapKeys(idMap map[string]string) []string {
 	keys := make([]string, len(idMap))
 	idx := 0

pkg/cloudcommon/db/fetch_test.go

@@ -0,0 +1,98 @@
+// Copyright 2019 Yunion
+//
+// 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 db
+
+import (
+	"testing"
+
+	"yunion.io/x/pkg/util/rbacscope"
+
+	"yunion.io/x/onecloud/pkg/cloudcommon/policy"
+)
+
+func TestResolveQueryScopeFallback(t *testing.T) {
+	cases := []struct {
+		name       string
+		reqScope   string
+		hasAdmin   bool
+		isAdmin    bool
+		allowScope rbacscope.TRbacScope
+		resScope   rbacscope.TRbacScope
+	}{
+		{"admin_true_project_caller", "", true, true, rbacscope.ScopeProject, rbacscope.ScopeProject},
+		{"admin_true_project_caller_domain_resource", "", true, true, rbacscope.ScopeProject, rbacscope.ScopeDomain},
+		{"admin_true_user_caller", "", true, true, rbacscope.ScopeUser, rbacscope.ScopeProject},
+		{"admin_false_project_caller", "", true, false, rbacscope.ScopeProject, rbacscope.ScopeProject},
+		{"admin_false_system_caller", "", true, false, rbacscope.ScopeSystem, rbacscope.ScopeProject},
+		{"scope_user_on_project_resource", "user", false, false, rbacscope.ScopeProject, rbacscope.ScopeProject},
+		{"scope_user_on_domain_resource", "user", false, false, rbacscope.ScopeDomain, rbacscope.ScopeDomain},
+		{"scope_max_no_permission", "max", false, false, rbacscope.ScopeNone, rbacscope.ScopeProject},
+		{"scope_max_no_permission_domain_resource", "max", false, false, rbacscope.ScopeNone, rbacscope.ScopeDomain},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			got := resolveQueryScope(c.reqScope, c.hasAdmin, c.isAdmin, c.allowScope, c.resScope, policy.PolicyActionList)
+			if got == "" || got == rbacscope.ScopeNone {
+				t.Fatalf("queryScope = %q, want a real scope (never unset/none)", got)
+			}
+			if got == rbacscope.ScopeUser && c.resScope != rbacscope.ScopeUser {
+				t.Fatalf("queryScope = user on a %s-scoped resource, want the resource scope", c.resScope)
+			}
+			if got != c.resScope {
+				t.Fatalf("queryScope = %q, want fallback to resource scope %q", got, c.resScope)
+			}
+		})
+	}
+}
+
+// the supported explicit views and fallbacks keep working
+func TestResolveQueryScopeSemantics(t *testing.T) {
+	cases := []struct {
+		name       string
+		reqScope   string
+		hasAdmin   bool
+		isAdmin    bool
+		allowScope rbacscope.TRbacScope
+		resScope   rbacscope.TRbacScope
+		action     string
+		want       rbacscope.TRbacScope
+	}{
+		// explicit views
+		{"explicit_system", "system", false, false, rbacscope.ScopeSystem, rbacscope.ScopeProject, policy.PolicyActionList, rbacscope.ScopeSystem},
+		{"explicit_domain", "domain", false, false, rbacscope.ScopeDomain, rbacscope.ScopeProject, policy.PolicyActionList, rbacscope.ScopeDomain},
+		{"explicit_project_on_domain_resource", "project", false, false, rbacscope.ScopeProject, rbacscope.ScopeDomain, policy.PolicyActionList, rbacscope.ScopeProject},
+		{"explicit_user_on_user_resource", "user", false, false, rbacscope.ScopeUser, rbacscope.ScopeUser, policy.PolicyActionList, rbacscope.ScopeUser},
+		{"unknown_scope_string_defaults_to_project", "garbage", false, false, rbacscope.ScopeProject, rbacscope.ScopeProject, policy.PolicyActionList, rbacscope.ScopeProject},
+		// max = widest allowed scope
+		{"max_for_system_caller", "max", false, false, rbacscope.ScopeSystem, rbacscope.ScopeProject, policy.PolicyActionList, rbacscope.ScopeSystem},
+		{"max_for_project_caller", "max", false, false, rbacscope.ScopeProject, rbacscope.ScopeProject, policy.PolicyActionList, rbacscope.ScopeProject},
+		// admin view for callers above project scope
+		{"admin_true_system_caller", "", true, true, rbacscope.ScopeSystem, rbacscope.ScopeProject, policy.PolicyActionList, rbacscope.ScopeSystem},
+		{"admin_true_domain_caller", "", true, true, rbacscope.ScopeDomain, rbacscope.ScopeProject, policy.PolicyActionList, rbacscope.ScopeDomain},
+		// default views
+		{"no_params_project_resource", "", false, false, rbacscope.ScopeProject, rbacscope.ScopeProject, policy.PolicyActionList, rbacscope.ScopeProject},
+		{"no_params_domain_resource", "", false, false, rbacscope.ScopeDomain, rbacscope.ScopeDomain, policy.PolicyActionList, rbacscope.ScopeDomain},
+		{"no_params_user_resource", "", false, false, rbacscope.ScopeUser, rbacscope.ScopeUser, policy.PolicyActionList, rbacscope.ScopeUser},
+		{"get_action_uses_allow_scope", "", false, false, rbacscope.ScopeProject, rbacscope.ScopeProject, policy.PolicyActionGet, rbacscope.ScopeProject},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			got := resolveQueryScope(c.reqScope, c.hasAdmin, c.isAdmin, c.allowScope, c.resScope, c.action)
+			if got != c.want {
+				t.Fatalf("queryScope = %q, want %q", got, c.want)
+			}
+		})
+	}
+}

pkg/cloudcommon/db/metadata.go

@@ -426,6 +426,8 @@ func (manager *SMetadataManager) ListItemFilter(ctx context.Context, q *sqlchemy
 		}
 		if len(conditions) > 0 {
 			q = q.Filter(sqlchemy.OR(conditions...))
+		} else {
+			q = q.Equals("obj_id", "")
 		}
 	}
 

pkg/cloudcommon/db/opslog.go

@@ -366,6 +366,9 @@ func (manager *SOpsLogManager) LogSyncUpdate(m IModel, uds sqlchemy.UpdateDiffs,
 
 func (self *SOpsLogManager) FilterByOwner(ctx context.Context, q *sqlchemy.SQuery, man FilterByOwnerProvider, userCred mcclient.TokenCredential, ownerId mcclient.IIdentityProvider, scope rbacscope.TRbacScope) *sqlchemy.SQuery {
 	if ownerId != nil {
+		if scope == "" || scope == rbacscope.ScopeNone {
+			scope = rbacscope.ScopeProject
+		}
 		switch scope {
 		case rbacscope.ScopeUser:
 			if len(ownerId.GetUserId()) > 0 {

pkg/cloudcommon/db/project.go

@@ -55,16 +55,6 @@ func (model *SProjectizedResourceBase) GetOwnerId() mcclient.IIdentityProvider {
 func (manager *SProjectizedResourceBaseManager) FilterByOwner(ctx context.Context, q *sqlchemy.SQuery, man FilterByOwnerProvider, userCred mcclient.TokenCredential, owner mcclient.IIdentityProvider, scope rbacscope.TRbacScope) *sqlchemy.SQuery {
 	if owner != nil {
 		switch scope {
-		case rbacscope.ScopeProject:
-			q = q.Equals("tenant_id", owner.GetProjectId())
-			if userCred != nil {
-				result := policy.PolicyManager.Allow(scope, userCred, consts.GetServiceType(), man.KeywordPlural(), policy.PolicyActionList)
-				if !result.ObjectTags.IsEmpty() {
-					policyTagFilters := tagutils.STagFilters{}
-					policyTagFilters.AddFilters(result.ObjectTags)
-					q = ObjectIdQueryWithTagFiltersOptimized(ctx, q, "id", man.Keyword(), policyTagFilters)
-				}
-			}
 		case rbacscope.ScopeDomain:
 			q = q.Equals("domain_id", owner.GetProjectDomainId())
 			if userCred != nil {
@@ -99,6 +89,18 @@ func (manager *SProjectizedResourceBaseManager) FilterByOwner(ctx context.Contex
 					q = ObjectIdQueryWithTagFiltersOptimized(ctx, q, "id", man.Keyword(), policyTagFilters)
 				}
 			}
+		default:
+			// project view, also used when the requested scope has no
+			// dedicated case
+			q = q.Equals("tenant_id", owner.GetProjectId())
+			if userCred != nil {
+				result := policy.PolicyManager.Allow(scope, userCred, consts.GetServiceType(), man.KeywordPlural(), policy.PolicyActionList)
+				if !result.ObjectTags.IsEmpty() {
+					policyTagFilters := tagutils.STagFilters{}
+					policyTagFilters.AddFilters(result.ObjectTags)
+					q = ObjectIdQueryWithTagFiltersOptimized(ctx, q, "id", man.Keyword(), policyTagFilters)
+				}
+			}
 		}
 	}
 	return q

pkg/cloudcommon/db/scoperesource.go

@@ -117,7 +117,7 @@ func (m *SScopedResourceBaseManager) FilterByOwner(ctx context.Context, q *sqlch
 	switch scope {
 	case rbacscope.ScopeDomain:
 		q = q.Equals("domain_id", owner.GetProjectDomainId())
-	case rbacscope.ScopeProject:
+	default:
 		q = q.Equals("tenant_id", owner.GetProjectId())
 	}
 	return q

pkg/cloudcommon/db/sharablebase.go

@@ -217,6 +217,24 @@ func SharableManagerValidateCreateData(
 func SharableManagerFilterByOwner(ctx context.Context, manager IStandaloneModelManager, q *sqlchemy.SQuery, userCred mcclient.TokenCredential, owner mcclient.IIdentityProvider, scope rbacscope.TRbacScope) *sqlchemy.SQuery {
 	if owner != nil {
 		resScope := manager.ResourceScope()
+		// map unrecognized scopes to the resource's default view
+		if scope != rbacscope.ScopeSystem && scope != rbacscope.ScopeDomain &&
+			scope != rbacscope.ScopeProject &&
+			!(scope == rbacscope.ScopeUser && resScope == rbacscope.ScopeUser) {
+			switch resScope {
+			case rbacscope.ScopeUser:
+				scope = rbacscope.ScopeUser
+			case rbacscope.ScopeDomain:
+				scope = rbacscope.ScopeDomain
+			default:
+				scope = rbacscope.ScopeProject
+			}
+		}
+		// domain-level owner has no project id: use the domain view
+		if scope == rbacscope.ScopeProject && len(owner.GetProjectId()) == 0 &&
+			len(owner.GetProjectDomainId()) > 0 {
+			scope = rbacscope.ScopeDomain
+		}
 		if resScope == rbacscope.ScopeUser {
 			targetProjectId := owner.GetProjectId()
 			if len(targetProjectId) == 0 && userCred != nil {