From b8bb95498c2d6d00ced9a663c7b4be7a8b10a722 Mon Sep 17 00:00:00 2001 From: Harshil Goel Date: Tue, 30 Jul 2024 15:58:06 +0530 Subject: [PATCH 1/2] fix(core): Fix namespace used by unique query generator --- edgraph/server.go | 35 ++++++++++++++++++++++++++-------- ee/acl/acl_integration_test.go | 33 +++++++++++++++++++++++++++++++- 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/edgraph/server.go b/edgraph/server.go index 8cefed6f15c..f59d65c7ff3 100644 --- a/edgraph/server.go +++ b/edgraph/server.go @@ -1344,7 +1344,7 @@ func (s *Server) doQuery(ctx context.Context, req *Request) (resp *api.Response, graphql: isGraphQL, gqlField: req.gqlField, } - if rerr = parseRequest(qc); rerr != nil { + if rerr = parseRequest(ctx, qc); rerr != nil { return } @@ -1565,7 +1565,7 @@ func processQuery(ctx context.Context, qc *queryContext) (*api.Response, error) } // parseRequest parses the incoming request -func parseRequest(qc *queryContext) error { +func parseRequest(ctx context.Context, qc *queryContext) error { start := time.Now() defer func() { qc.latency.Parsing = time.Since(start) @@ -1585,7 +1585,7 @@ func parseRequest(qc *queryContext) error { qc.gmuList = append(qc.gmuList, gmu) } - if err := addQueryIfUnique(qc); err != nil { + if err := addQueryIfUnique(ctx, qc); err != nil { return err } @@ -1698,19 +1698,38 @@ func verifyUnique(qc *queryContext, qr query.Request) error { } // addQueryIfUnique adds dummy queries in the request for checking whether predicate is unique in the db -func addQueryIfUnique(qc *queryContext) error { +func addQueryIfUnique(qctx context.Context, qc *queryContext) error { if len(qc.gmuList) == 0 { return nil } - ctx := context.WithValue(context.Background(), schema.IsWrite, false) + ctx := context.WithValue(qctx, schema.IsWrite, false) + namespace, err := x.ExtractNamespace(ctx) + if err != nil { + // It's okay to ignore this here. If namespace is not set, it could mean either there is no + // authorization or it's trying to be bypassed. So the namespace is either 0 or the mutation would fail. + glog.Errorf("Error while extracting namespace, assuming default %s", err) + namespace = 0 + } + isGalaxyQuery := x.IsGalaxyOperation(ctx) + qc.uniqueVars = map[uint64]uniquePredMeta{} var buildQuery strings.Builder for gmuIndex, gmu := range qc.gmuList { for rdfIndex, pred := range gmu.Set { - predSchema, _ := schema.State().Get(ctx, x.NamespaceAttr(pred.Namespace, pred.Predicate)) - if !predSchema.Unique { - continue + if isGalaxyQuery { + // The caller should make sure that the directed edges contain the namespace we want + // to insert into. + namespace = pred.Namespace + } + if pred.Predicate != "dgraph.xid" { + // [TODO] Don't check if it's dgraph.xid. It's a bug as this node might not be aware + // of the schema for the given predicate. This is a bug issue for dgraph.xid hence + // we are bypassing it manually until the bug is fixed. + predSchema, ok := schema.State().Get(ctx, x.NamespaceAttr(namespace, pred.Predicate)) + if !ok || !predSchema.Unique { + continue + } } var predicateName string if pred.Lang != "" { diff --git a/ee/acl/acl_integration_test.go b/ee/acl/acl_integration_test.go index e8232479e9f..8c60aab9c66 100644 --- a/ee/acl/acl_integration_test.go +++ b/ee/acl/acl_integration_test.go @@ -439,6 +439,37 @@ func (asuite *AclTestSuite) TestWrongPermission() { require.Contains(t, err.Error(), "Value for this predicate should be between 0 and 7") } +func (asuite *AclTestSuite) TestACLNamespaceEdge() { + t := asuite.T() + gc, cleanup, err := asuite.dc.Client() + require.NoError(t, err) + defer cleanup() + require.NoError(t, gc.LoginIntoNamespace(context.Background(), + dgraphapi.DefaultUser, dgraphapi.DefaultPassword, x.GalaxyNamespace)) + + json := ` + { + "set": [ + { + "dgraph.xid": "groot", + "dgraph.password": "password", + "dgraph.type": "dgraph.type.User", + "dgraph.user.group": { + "dgraph.xid": "guardians", + "dgraph.type": "dgraph.type.Group", + "namespace": 1 + }, + "namespace": 1 + } + ] +}` + + mu := &api.Mutation{SetJson: []byte(json), CommitNow: true} + _, err = gc.Mutate(mu) + require.Error(t, err) + require.ErrorContains(t, err, "could not insert duplicate value") // Could be gaurdian or groot +} + func (asuite *AclTestSuite) TestACLDuplicateGrootUser() { t := asuite.T() gc, cleanup, err := asuite.dc.Client() @@ -451,7 +482,7 @@ func (asuite *AclTestSuite) TestACLDuplicateGrootUser() { _:a "dgraph.type.User" .` mu := &api.Mutation{SetNquads: []byte(rdfs), CommitNow: true} - _, err = gc.Mutate(mu) + _, err := gc.Mutate(mu) require.Error(t, err) require.ErrorContains(t, err, "could not insert duplicate value [groot] for predicate [dgraph.xid]") } From 05962583062016457d28b943bfb73dc19091c7c2 Mon Sep 17 00:00:00 2001 From: Harshil Goel Date: Tue, 30 Jul 2024 23:43:32 +0530 Subject: [PATCH 2/2] fixed : in test --- ee/acl/acl_integration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/acl/acl_integration_test.go b/ee/acl/acl_integration_test.go index 8c60aab9c66..eaf360cf218 100644 --- a/ee/acl/acl_integration_test.go +++ b/ee/acl/acl_integration_test.go @@ -482,7 +482,7 @@ func (asuite *AclTestSuite) TestACLDuplicateGrootUser() { _:a "dgraph.type.User" .` mu := &api.Mutation{SetNquads: []byte(rdfs), CommitNow: true} - _, err := gc.Mutate(mu) + _, err = gc.Mutate(mu) require.Error(t, err) require.ErrorContains(t, err, "could not insert duplicate value [groot] for predicate [dgraph.xid]") }