From a4831bc82a0b156eb25fdddeea7f855c5bac332f Mon Sep 17 00:00:00 2001 From: Islam Aleiv Date: Wed, 8 May 2024 12:42:47 +0200 Subject: [PATCH 1/2] Enable sec. indexes for ACP, add tests --- internal/db/collection_index.go | 6 - internal/db/errors.go | 1 - tests/integration/acp/index/create_test.go | 92 +---- tests/integration/acp/index/fixture.go | 87 +++++ tests/integration/acp/index/query_test.go | 185 ++++++++++ .../acp/index/query_with_relation_test.go | 319 ++++++++++++++++++ 6 files changed, 601 insertions(+), 89 deletions(-) create mode 100644 tests/integration/acp/index/fixture.go create mode 100644 tests/integration/acp/index/query_test.go create mode 100644 tests/integration/acp/index/query_with_relation_test.go diff --git a/internal/db/collection_index.go b/internal/db/collection_index.go index a5db0a96d3..14f9a1b805 100644 --- a/internal/db/collection_index.go +++ b/internal/db/collection_index.go @@ -256,12 +256,6 @@ func (c *collection) createIndex( ctx context.Context, desc client.IndexDescription, ) (CollectionIndex, error) { - // Don't allow creating index on a permissioned collection, until following is implemented. - // TODO-ACP: ACP <> INDEX https://github.com/sourcenetwork/defradb/issues/2365 - if c.Description().Policy.HasValue() { - return nil, ErrCanNotCreateIndexOnCollectionWithPolicy - } - if desc.Name != "" && !schema.IsValidIndexName(desc.Name) { return nil, schema.NewErrIndexWithInvalidName("!") } diff --git a/internal/db/errors.go b/internal/db/errors.go index f917ee9724..fcb4baf13f 100644 --- a/internal/db/errors.go +++ b/internal/db/errors.go @@ -97,7 +97,6 @@ const ( var ( ErrFailedToGetCollection = errors.New(errFailedToGetCollection) - ErrCanNotCreateIndexOnCollectionWithPolicy = errors.New("can not create index on a collection with a policy") ErrSubscriptionsNotAllowed = errors.New("server does not accept subscriptions") ErrInvalidFilter = errors.New("invalid filter") ErrCollectionAlreadyExists = errors.New(errCollectionAlreadyExists) diff --git a/tests/integration/acp/index/create_test.go b/tests/integration/acp/index/create_test.go index f2c9b193a7..9c440e25e2 100644 --- a/tests/integration/acp/index/create_test.go +++ b/tests/integration/acp/index/create_test.go @@ -17,46 +17,14 @@ import ( acpUtils "github.com/sourcenetwork/defradb/tests/integration/acp" ) -// This test documents that we don't allow creating indexes on collections that have policy -// until the following is implemented: -// TODO-ACP: ACP <> P2P https://github.com/sourcenetwork/defradb/issues/2365 -func TestACP_IndexCreateWithSeparateRequest_OnCollectionWithPolicy_ReturnError(t *testing.T) { +func TestACP_IndexCreateWithSeparateRequest_OnCollectionWithPolicy_NoError(t *testing.T) { test := testUtils.TestCase{ - Description: "Test acp, with creating new index using separate request on permissioned collection, error", + Description: "Test acp, with creating new index using separate request on permissioned collection, no error", Actions: []any{ testUtils.AddPolicy{ - - Identity: acpUtils.Actor1Identity, - - Policy: ` - description: a test policy which marks a collection in a database as a resource - - actor: - name: actor - - resources: - users: - permissions: - read: - expr: owner + reader - write: - expr: owner - - relations: - owner: - types: - - actor - reader: - types: - - actor - admin: - manages: - - reader - types: - - actor - `, - + Identity: acpUtils.Actor1Identity, + Policy: userPolicy, ExpectedPolicyID: "53980e762616fcffbe76307995895e862f87ef3f21d509325d1dc772a770b001", }, @@ -74,12 +42,8 @@ func TestACP_IndexCreateWithSeparateRequest_OnCollectionWithPolicy_ReturnError(t testUtils.CreateIndex{ CollectionID: 0, - - IndexName: "some_index", - - FieldName: "name", - - ExpectedError: "can not create index on a collection with a policy", + IndexName: "some_index", + FieldName: "name", }, testUtils.Request{ @@ -99,46 +63,14 @@ func TestACP_IndexCreateWithSeparateRequest_OnCollectionWithPolicy_ReturnError(t testUtils.ExecuteTestCase(t, test) } -// This test documents that we don't allow creating indexes on collections that have policy -// until the following is implemented: -// TODO-ACP: ACP <> P2P https://github.com/sourcenetwork/defradb/issues/2365 -func TestACP_IndexCreateWithDirective_OnCollectionWithPolicy_ReturnError(t *testing.T) { +func TestACP_IndexCreateWithDirective_OnCollectionWithPolicy_NoError(t *testing.T) { test := testUtils.TestCase{ - Description: "Test acp, with creating new index using directive on permissioned collection, error", + Description: "Test acp, with creating new index using directive on permissioned collection, no error", Actions: []any{ testUtils.AddPolicy{ - - Identity: acpUtils.Actor1Identity, - - Policy: ` - description: a test policy which marks a collection in a database as a resource - - actor: - name: actor - - resources: - users: - permissions: - read: - expr: owner + reader - write: - expr: owner - - relations: - owner: - types: - - actor - reader: - types: - - actor - admin: - manages: - - reader - types: - - actor - `, - + Identity: acpUtils.Actor1Identity, + Policy: userPolicy, ExpectedPolicyID: "53980e762616fcffbe76307995895e862f87ef3f21d509325d1dc772a770b001", }, @@ -152,8 +84,6 @@ func TestACP_IndexCreateWithDirective_OnCollectionWithPolicy_ReturnError(t *test age: Int } `, - - ExpectedError: "can not create index on a collection with a policy", }, testUtils.Request{ @@ -164,8 +94,6 @@ func TestACP_IndexCreateWithDirective_OnCollectionWithPolicy_ReturnError(t *test age } }`, - - ExpectedError: `Cannot query field "Users" on type "Query"`, }, }, } diff --git a/tests/integration/acp/index/fixture.go b/tests/integration/acp/index/fixture.go new file mode 100644 index 0000000000..af2f401496 --- /dev/null +++ b/tests/integration/acp/index/fixture.go @@ -0,0 +1,87 @@ +// Copyright 2024 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package test_acp_index + +const userPolicy = ` +description: a test policy which marks a collection in a database as a resource + +actor: + name: actor + +resources: + users: + permissions: + read: + expr: owner + reader + write: + expr: owner + + relations: + owner: + types: + - actor + reader: + types: + - actor + admin: + manages: + - reader + types: + - actor +` + +const bookAuthorPolicy = ` +description: a test policy which marks a collection in a database as a resource + +actor: + name: actor + +resources: + author: + permissions: + read: + expr: owner + reader + write: + expr: owner + + relations: + owner: + types: + - actor + reader: + types: + - actor + admin: + manages: + - reader + types: + - actor + + book: + permissions: + read: + expr: owner + reader + write: + expr: owner + + relations: + owner: + types: + - actor + reader: + types: + - actor + admin: + manages: + - reader + types: + - actor +` diff --git a/tests/integration/acp/index/query_test.go b/tests/integration/acp/index/query_test.go new file mode 100644 index 0000000000..a7c09cd9e9 --- /dev/null +++ b/tests/integration/acp/index/query_test.go @@ -0,0 +1,185 @@ +// Copyright 2024 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package test_acp_index + +import ( + "testing" + + testUtils "github.com/sourcenetwork/defradb/tests/integration" + acpUtils "github.com/sourcenetwork/defradb/tests/integration/acp" +) + +func TestACPWithIndex_UponQueryingPrivateDocWithoutIdentity_ShouldNotFetch(t *testing.T) { + test := testUtils.TestCase{ + Description: "Test acp, querying private doc without identity should not fetch", + Actions: []any{ + testUtils.AddPolicy{ + Identity: acpUtils.Actor1Identity, + Policy: userPolicy, + ExpectedPolicyID: "53980e762616fcffbe76307995895e862f87ef3f21d509325d1dc772a770b001", + }, + testUtils.SchemaUpdate{ + Schema: ` + type Users @policy( + id: "53980e762616fcffbe76307995895e862f87ef3f21d509325d1dc772a770b001", + resource: "users" + ) { + name: String @index + age: Int + } + `, + }, + testUtils.CreateDoc{ + Doc: ` + { + "name": "Shahzad" + } + `, + }, + testUtils.CreateDoc{ + Identity: acpUtils.Actor1Identity, + Doc: ` + { + "name": "Islam" + } + `, + }, + testUtils.Request{ + Request: ` + query { + Users { + name + } + }`, + Results: []map[string]any{ + { + "name": "Shahzad", + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} + +func TestACPWithIndex_UponQueryingPrivateDocWithIdentity_ShouldFetch(t *testing.T) { + test := testUtils.TestCase{ + Description: "Test acp, querying private doc with identity should fetch", + Actions: []any{ + testUtils.AddPolicy{ + Identity: acpUtils.Actor1Identity, + Policy: userPolicy, + ExpectedPolicyID: "53980e762616fcffbe76307995895e862f87ef3f21d509325d1dc772a770b001", + }, + testUtils.SchemaUpdate{ + Schema: ` + type Users @policy( + id: "53980e762616fcffbe76307995895e862f87ef3f21d509325d1dc772a770b001", + resource: "users" + ) { + name: String @index + age: Int + } + `, + }, + testUtils.CreateDoc{ + Doc: ` + { + "name": "Shahzad" + } + `, + }, + testUtils.CreateDoc{ + Identity: acpUtils.Actor1Identity, + Doc: ` + { + "name": "Islam" + } + `, + }, + testUtils.Request{ + Identity: acpUtils.Actor1Identity, + Request: ` + query { + Users { + name + } + }`, + Results: []map[string]any{ + { + "name": "Islam", + }, + { + "name": "Shahzad", + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} + +func TestACPWithIndex_UponQueryingPrivateDocWithWrongIdentity_ShouldNotFetch(t *testing.T) { + test := testUtils.TestCase{ + Description: "Test acp, querying private doc with wrong identity should not fetch", + Actions: []any{ + testUtils.AddPolicy{ + Identity: acpUtils.Actor1Identity, + Policy: userPolicy, + ExpectedPolicyID: "53980e762616fcffbe76307995895e862f87ef3f21d509325d1dc772a770b001", + }, + testUtils.SchemaUpdate{ + Schema: ` + type Users @policy( + id: "53980e762616fcffbe76307995895e862f87ef3f21d509325d1dc772a770b001", + resource: "users" + ) { + name: String @index + age: Int + } + `, + }, + testUtils.CreateDoc{ + Doc: ` + { + "name": "Shahzad" + } + `, + }, + testUtils.CreateDoc{ + Identity: acpUtils.Actor1Identity, + Doc: ` + { + "name": "Islam" + } + `, + }, + testUtils.Request{ + Identity: acpUtils.Actor2Identity, + Request: ` + query { + Users { + name + } + }`, + Results: []map[string]any{ + { + "name": "Shahzad", + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} diff --git a/tests/integration/acp/index/query_with_relation_test.go b/tests/integration/acp/index/query_with_relation_test.go new file mode 100644 index 0000000000..614aaa6e84 --- /dev/null +++ b/tests/integration/acp/index/query_with_relation_test.go @@ -0,0 +1,319 @@ +// Copyright 2024 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package test_acp_index + +import ( + "testing" + + testUtils "github.com/sourcenetwork/defradb/tests/integration" + acpUtils "github.com/sourcenetwork/defradb/tests/integration/acp" +) + +func createAuthorBooksSchemaWithPolicyAndCreateDocs() []any { + return []any{ + testUtils.AddPolicy{ + Identity: acpUtils.Actor1Identity, + Policy: bookAuthorPolicy, + ExpectedPolicyID: "68a4e64d5034b8a0565a90cd36483de0d61e0ea2450cf57c1fa8d27cbbf17c2c", + }, + testUtils.SchemaUpdate{ + Schema: ` + type Author @policy( + id: "68a4e64d5034b8a0565a90cd36483de0d61e0ea2450cf57c1fa8d27cbbf17c2c", + resource: "author" + ) { + name: String + age: Int @index + verified: Boolean + published: [Book] + } + + type Book @policy( + id: "68a4e64d5034b8a0565a90cd36483de0d61e0ea2450cf57c1fa8d27cbbf17c2c", + resource: "author" + ) { + name: String + rating: Float @index + author: Author + }`, + }, + testUtils.CreateDoc{ + CollectionID: 0, + // bae-41598f0c-19bc-5da6-813b-e80f14a10df3 + Doc: `{ + "name": "John Grisham", + "age": 65, + "verified": true + }`, + }, + testUtils.CreateDoc{ + Identity: acpUtils.Actor1Identity, + CollectionID: 0, + // bae-b769708d-f552-5c3d-a402-ccfd7ac7fb04 + Doc: `{ + "name": "Cornelia Funke", + "age": 62, + "verified": false + }`, + }, + testUtils.CreateDoc{ + CollectionID: 1, + Doc: `{ + "name": "Painted House", + "rating": 4.9, + "author_id": "bae-41598f0c-19bc-5da6-813b-e80f14a10df3" + }`, + }, + testUtils.CreateDoc{ + Identity: acpUtils.Actor1Identity, + CollectionID: 1, + Doc: `{ + "name": "A Time for Mercy", + "rating": 4.5, + "author_id": "bae-41598f0c-19bc-5da6-813b-e80f14a10df3" + }`, + }, + testUtils.CreateDoc{ + Identity: acpUtils.Actor1Identity, + CollectionID: 1, + Doc: `{ + "name": "Theif Lord", + "rating": 4.8, + "author_id": "bae-b769708d-f552-5c3d-a402-ccfd7ac7fb04" + }`, + }, + } +} + +func TestACPWithIndex_UponQueryingPrivateOneToManyRelatedDocWithoutIdentity_ShouldNotFetch(t *testing.T) { + test := testUtils.TestCase{ + Description: "Test ACP with index: upon querying private (one-to-many) related doc without identity should not fetch", + Actions: []any{ + createAuthorBooksSchemaWithPolicyAndCreateDocs(), + testUtils.Request{ + Request: ` + query { + Author(filter: { + published: {rating: {_gt: 3}} + }) { + name + published { + name + } + } + }`, + Results: []map[string]any{ + { + "name": "John Grisham", + "published": []map[string]any{ + { + "name": "Painted House", + }, + }, + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} + +func TestACPWithIndex_UponQueryingPrivateOneToManyRelatedDocWithIdentity_ShouldFetch(t *testing.T) { + test := testUtils.TestCase{ + Description: "Test ACP with index: upon querying private (one-to-many) related doc with identity should fetch", + Actions: []any{ + createAuthorBooksSchemaWithPolicyAndCreateDocs(), + testUtils.Request{ + Identity: acpUtils.Actor1Identity, + Request: ` + query { + Author(filter: { + published: {rating: {_gt: 3}} + }) { + name + published { + name + } + } + }`, + Results: []map[string]any{ + { + "name": "John Grisham", + "published": []map[string]any{ + { + "name": "Painted House", + }, + { + "name": "A Time for Mercy", + }, + }, + }, + { + "name": "Cornelia Funke", + "published": []map[string]any{ + { + "name": "Theif Lord", + }, + }, + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} + +func TestACPWithIndex_UponQueryingPrivateOneToManyRelatedDocWithWrongIdentity_ShouldNotFetch(t *testing.T) { + test := testUtils.TestCase{ + Description: "Test ACP with index: upon querying private (one-to-many) related doc with wrong identity should not fetch", + Actions: []any{ + createAuthorBooksSchemaWithPolicyAndCreateDocs(), + testUtils.Request{ + Identity: acpUtils.Actor2Identity, + Request: ` + query { + Author(filter: { + published: {rating: {_gt: 3}} + }) { + name + published { + name + } + } + }`, + Results: []map[string]any{ + { + "name": "John Grisham", + "published": []map[string]any{ + { + "name": "Painted House", + }, + }, + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} + +func TestACPWithIndex_UponQueryingPrivateManyToOneRelatedDocWithoutIdentity_ShouldNotFetch(t *testing.T) { + test := testUtils.TestCase{ + Description: "Test ACP with index: upon querying private (many-to-one) related doc without identity should not fetch", + Actions: []any{ + createAuthorBooksSchemaWithPolicyAndCreateDocs(), + testUtils.Request{ + Request: ` + query { + Book(filter: { + author: {age: {_gt: 60}} + }) { + name + author { + name + } + } + }`, + Results: []map[string]any{ + { + "name": "Painted House", + "author": map[string]any{ + "name": "John Grisham", + }, + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} + +func TestACPWithIndex_UponQueryingPrivateManyToOneRelatedDocWithIdentity_ShouldFetch(t *testing.T) { + test := testUtils.TestCase{ + Description: "Test ACP with index: upon querying private (many-to-one) related doc with identity should fetch", + Actions: []any{ + createAuthorBooksSchemaWithPolicyAndCreateDocs(), + testUtils.Request{ + Identity: acpUtils.Actor1Identity, + Request: ` + query { + Book(filter: { + author: {age: {_gt: 60}} + }) { + name + author { + name + } + } + }`, + Results: []map[string]any{ + { + "name": "Theif Lord", + "author": map[string]any{ + "name": "Cornelia Funke", + }, + }, + { + "name": "Painted House", + "author": map[string]any{ + "name": "John Grisham", + }, + }, + { + "name": "A Time for Mercy", + "author": map[string]any{ + "name": "John Grisham", + }, + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} + +func TestACPWithIndex_UponQueryingPrivateManyToOneRelatedDocWithWrongIdentity_ShouldNotFetch(t *testing.T) { + test := testUtils.TestCase{ + Description: "Test ACP with index: upon querying private (many-to-one) related doc without identity should not fetch", + Actions: []any{ + createAuthorBooksSchemaWithPolicyAndCreateDocs(), + testUtils.Request{ + Identity: acpUtils.Actor2Identity, + Request: ` + query { + Book(filter: { + author: {age: {_gt: 60}} + }) { + name + author { + name + } + } + }`, + Results: []map[string]any{ + { + "name": "Painted House", + "author": map[string]any{ + "name": "John Grisham", + }, + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} From 3fa87a7b8991b29cf0a5b4d10645bfa3b4ffef26 Mon Sep 17 00:00:00 2001 From: Islam Aleiv Date: Thu, 9 May 2024 08:21:06 +0200 Subject: [PATCH 2/2] Added comments to policies --- tests/integration/acp/index/fixture.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/integration/acp/index/fixture.go b/tests/integration/acp/index/fixture.go index af2f401496..49c76e8b22 100644 --- a/tests/integration/acp/index/fixture.go +++ b/tests/integration/acp/index/fixture.go @@ -10,6 +10,7 @@ package test_acp_index +// policy id: "53980e762616fcffbe76307995895e862f87ef3f21d509325d1dc772a770b001" const userPolicy = ` description: a test policy which marks a collection in a database as a resource @@ -38,6 +39,7 @@ resources: - actor ` +// policy id: "68a4e64d5034b8a0565a90cd36483de0d61e0ea2450cf57c1fa8d27cbbf17c2c" const bookAuthorPolicy = ` description: a test policy which marks a collection in a database as a resource