diff --git a/fulfillment-service/internal/database/dao/generic_dao_list.go b/fulfillment-service/internal/database/dao/generic_dao_list.go index ae15406eef..7c479d1f6c 100644 --- a/fulfillment-service/internal/database/dao/generic_dao_list.go +++ b/fulfillment-service/internal/database/dao/generic_dao_list.go @@ -91,7 +91,9 @@ func (r *ListRequest[O]) do(ctx context.Context) (response *ListResponse[O], err if r.sql.filter.Len() > 0 { r.sql.filter.WriteString(` and `) } + r.sql.filter.WriteString(`(`) r.sql.filter.WriteString(filter) + r.sql.filter.WriteString(`)`) } // Calculate the order clause: diff --git a/fulfillment-service/internal/database/dao/generic_dao_tenant_visibility_test.go b/fulfillment-service/internal/database/dao/generic_dao_tenant_visibility_test.go index 0d63274d90..8f8509d81b 100644 --- a/fulfillment-service/internal/database/dao/generic_dao_tenant_visibility_test.go +++ b/fulfillment-service/internal/database/dao/generic_dao_tenant_visibility_test.go @@ -351,6 +351,62 @@ var _ = Describe("Tenant visibility", func() { Expect(getResponse.GetObject().GetMyString()).To(Equal("updated")) }) + It("Isolates tenant rows when filter has a top-level OR", func() { + // Create an unrestricted tenancy to seed objects in different tenants: + tenancyAll := auth.NewMockTenancyLogic(ctrl) + tenancyAll.EXPECT().DetermineVisibleTenants(gomock.Any()). + Return(collections.NewUniversalSet[string](), nil). + AnyTimes() + daoAll, err := NewGenericDAO[*testsv1.Object](). + SetLogger(logger). + SetTenancyLogic(tenancyAll). + Build() + Expect(err).ToNot(HaveOccurred()) + + // Create an object in tenant-a (will be visible): + _, err = daoAll.Create(). + SetObject(testsv1.Object_builder{ + Metadata: testsv1.Metadata_builder{ + Tenant: "tenant-a", + Name: "shared-name", + }.Build(), + }.Build()). + Do(ctx) + Expect(err).ToNot(HaveOccurred()) + + // Create an object in tenant-b with the same name (should be invisible to tenant-a): + _, err = daoAll.Create(). + SetObject(testsv1.Object_builder{ + Metadata: testsv1.Metadata_builder{ + Tenant: "tenant-b", + Name: "shared-name", + }.Build(), + }.Build()). + Do(ctx) + Expect(err).ToNot(HaveOccurred()) + + // Create a restricted DAO that can only see tenant-a: + tenancyRestricted := auth.NewMockTenancyLogic(ctrl) + tenancyRestricted.EXPECT().DetermineVisibleTenants(gomock.Any()). + Return(collections.NewSet("tenant-a"), nil). + AnyTimes() + daoRestricted, err := NewGenericDAO[*testsv1.Object](). + SetLogger(logger). + SetTenancyLogic(tenancyRestricted). + Build() + Expect(err).ToNot(HaveOccurred()) + + // List with a top-level OR filter (id-or-name pattern). The tenancy clause + // must apply to the entire filter, not just the first branch: + listResponse, err := daoRestricted.List(). + SetFilter(`this.id == "nonexistent" || this.metadata.name == "shared-name"`). + Do(ctx) + Expect(err).ToNot(HaveOccurred()) + Expect(listResponse.GetTotal()).To(Equal(int32(1))) + Expect(listResponse.GetItems()).To(HaveLen(1)) + Expect(listResponse.GetItems()[0].GetMetadata().GetTenant()).To(Equal("tenant-a")) + }) + It("Rejects update of an object belonging to an invisible tenant as not found", func() { // Create a tenancy logic that makes all tenants visible, used to create the object: tenancyA := auth.NewMockTenancyLogic(ctrl)