Skip to content

Commit

Permalink
fix(acl): allow data deletion for non-reserved predicates (#8937)
Browse files Browse the repository at this point in the history
when data (non-reserved predicates) is added on UIDs that
belong to groot user or guardian group, it is allowed. But when the same
data is deleted, that is not allowed. This PR allows deletion of
non-reserved predicates on special UIDs.

Closes: https://dgraph.atlassian.net/browse/DGRAPHCORE-355
  • Loading branch information
jbhamra1 authored Aug 16, 2023
1 parent c74a3aa commit 8631dab
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 12 deletions.
4 changes: 2 additions & 2 deletions ee/acl/acl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2793,13 +2793,13 @@ func (asuite *AclTestSuite) TestDeleteGrootAndGuardiansUsingDelNQuadShouldFail()
// Try deleting groot user
_, err = gc.Mutate(mu)
require.Error(t, err, "Deleting groot user should have returned an error")
require.Contains(t, err.Error(), "Properties of guardians group and groot user cannot be deleted")
require.Contains(t, err.Error(), "properties of guardians group and groot user cannot be deleted")

mu = &api.Mutation{DelNquads: []byte(fmt.Sprintf("%s %s %s .", "<"+guardiansUid+">", "*", "*")), CommitNow: true}
// Try deleting guardians group
_, err = gc.Mutate(mu)
require.Error(t, err, "Deleting guardians group should have returned an error")
require.Contains(t, err.Error(), "Properties of guardians group and groot user cannot be deleted")
require.Contains(t, err.Error(), "properties of guardians group and groot user cannot be deleted")
}

func deleteGuardiansGroupAndGrootUserShouldFail(t *testing.T, hc *dgraphtest.HTTPClient) {
Expand Down
2 changes: 1 addition & 1 deletion query/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ func populateCluster() {
// In the query package, we test using hard coded UIDs so that we know what results
// to expect. We need to move the max assigned UID in zero to higher value than
// all the UIDs we are using during the tests.
x.Panic(dc.AssignUids(client, 65536))
x.Panic(dc.AssignUids(client.Dgraph, 65536))

setSchema(testSchema)
err := addTriplesToCluster(`
Expand Down
8 changes: 6 additions & 2 deletions query/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,23 @@
package query

import (
"context"
"testing"

"github.com/dgraph-io/dgraph/dgraphtest"
"github.com/dgraph-io/dgraph/testutil"
"github.com/dgraph-io/dgraph/x"
)

func TestMain(m *testing.M) {
dc = dgraphtest.NewComposeCluster()

var err error
client, err = testutil.DgraphClientWithGroot(testutil.SockAddr)
var cleanup func()
client, cleanup, err = dc.Client()
x.Panic(err)
defer cleanup()
x.Panic(client.LoginIntoNamespace(context.Background(), dgraphtest.DefaultUser,
dgraphtest.DefaultPassword, x.GalaxyNamespace))

populateCluster()
m.Run()
Expand Down
5 changes: 4 additions & 1 deletion query/mutation.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,9 @@ func checkIfDeletingAclOperation(ctx context.Context, edges []*pb.DirectedEdge)

isDeleteAclOperation := false
for _, edge := range edges {
if !x.IsReservedPredicate(edge.Attr) {
continue
}
// Disallow deleting of guardians group
if edge.Entity == guardianUid && edge.Op == pb.DirectedEdge_DEL {
isDeleteAclOperation = true
Expand All @@ -298,7 +301,7 @@ func checkIfDeletingAclOperation(ctx context.Context, edges []*pb.DirectedEdge)
}
}
if isDeleteAclOperation {
return errors.Errorf("Properties of guardians group and groot user cannot be deleted.")
return errors.Errorf("Reserved properties of guardians group and groot user cannot be deleted.")
}
return nil
}
47 changes: 46 additions & 1 deletion query/mutation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ package query

import (
"context"
"encoding/json"
"fmt"
"strconv"
"testing"
"time"

Expand All @@ -28,7 +31,7 @@ import (
"github.com/dgraph-io/dgo/v230/protos/api"
)

func TestReserverPredicateForMutation(t *testing.T) {
func TestReservedPredicateForMutation(t *testing.T) {
err := addTriplesToCluster(`_:x <dgraph.graphql.schema> "df"`)
require.Error(t, err, "Cannot mutate graphql reserved predicate dgraph.graphql.schema")
}
Expand Down Expand Up @@ -70,3 +73,45 @@ func TestAlteringReservedTypesAndPredicatesShouldFail(t *testing.T) {
require.Contains(t, err.Error(), "Can't store predicate `dgraph.name` as it is prefixed with "+
"`dgraph.` which is reserved as the namespace for dgraph's internal types/predicates.")
}

func TestUnreservedPredicateForDeletion(t *testing.T) {
grootUserQuery := `
{
grootUser(func:eq(dgraph.xid, "groot")){
uid
}
}`

// Structs to parse groot user query response
type userNode struct {
Uid string `json:"uid"`
}

type userQryResp struct {
GrootUser []userNode `json:"grootUser"`
}

resp, err := client.Query(grootUserQuery)
require.NoError(t, err, "groot user query failed")

var userResp userQryResp
if err := json.Unmarshal(resp.GetJson(), &userResp); err != nil {
t.Fatal("Couldn't unmarshal response from groot user query")
}
grootsUidStr := userResp.GrootUser[0].Uid
grootsUidInt, err := strconv.ParseUint(grootsUidStr, 0, 64)
require.NoError(t, err)
require.Greater(t, uint64(grootsUidInt), uint64(0))

const testSchema = `
type Star {
name
}
name : string @index(term, exact, trigram) @count @lang .
`
triple := fmt.Sprintf(`<%s> <dgraphcore_355> "Betelgeuse" .`, grootsUidStr)
setSchema(testSchema)
require.NoError(t, addTriplesToCluster(triple))
deleteTriplesInCluster(triple)
}
3 changes: 1 addition & 2 deletions query/query0_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (

"github.com/stretchr/testify/require"

"github.com/dgraph-io/dgo/v230"
"github.com/dgraph-io/dgraph/dgraphtest"
"github.com/dgraph-io/dgraph/dql"
)
Expand Down Expand Up @@ -3576,5 +3575,5 @@ func TestInvalidRegex(t *testing.T) {
}
}

var client *dgo.Dgraph
var client *dgraphtest.GrpcClient
var dc dgraphtest.Cluster
4 changes: 2 additions & 2 deletions query/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestMain(m *testing.M) {
x.Panic(dg.LoginIntoNamespace(context.Background(), dgraphtest.DefaultUser,
dgraphtest.DefaultPassword, x.GalaxyNamespace))

client = dg.Dgraph
client = dg
dc = c
populateCluster()
}
Expand All @@ -49,7 +49,7 @@ func TestMain(m *testing.M) {
x.Panic(dg.LoginIntoNamespace(context.Background(), dgraphtest.DefaultUser,
dgraphtest.DefaultPassword, x.GalaxyNamespace))

client = dg.Dgraph
client = dg
dc = c
return m.Run()
}
Expand Down
2 changes: 1 addition & 1 deletion systest/bgindex/count_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func TestCountIndex(t *testing.T) {
CommitNow: true,
DelNquads: []byte(fmt.Sprintf(`<%v> <value> "%v" .`, uid, ec-1)),
}); err != nil && (errors.Is(err, dgo.ErrAborted) ||
strings.Contains(err.Error(), "Properties of guardians group and groot user cannot be deleted")) {
strings.Contains(err.Error(), "properties of guardians group and groot user cannot be deleted")) {
return
} else if err != nil {
t.Fatalf("error in deletion :: %v\n", err)
Expand Down

0 comments on commit 8631dab

Please sign in to comment.