fix(acl): fix duplicate groot user creation (#51)

This PR adds the unique directive to the 'dgraph.xid' predicate. Prior to this change, users could
create duplicate users leading to misconfiguration of ACL.

check_upgrade/check_upgrade.go

@@ -0,0 +1,210 @@
+/*
+ * Copyright 2024 Dgraph Labs, Inc. and Contributors
+ *
+ * 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 checkupgrade
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"os"
+
+	"github.com/pkg/errors"
+	"github.com/spf13/cobra"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/credentials/insecure"
+
+	"github.com/dgraph-io/dgo/v230"
+	"github.com/dgraph-io/dgo/v230/protos/api"
+	"github.com/dgraph-io/dgraph/dgraphtest"
+	"github.com/dgraph-io/dgraph/x"
+)
+
+var (
+	CheckUpgrade x.SubCommand
+)
+
+const (
+	alphaGrpc = "grpc_port"
+	alphaHttp = "http_port"
+)
+
+type commandInput struct {
+	alphaGrpc string
+	alphaHttp string
+}
+
+type ACLNode struct {
+	UID        string   `json:"uid"`
+	DgraphXID  string   `json:"dgraph.xid"`
+	DgraphType []string `json:"dgraph.type"`
+}
+
+func setupClients(alphaGrpc, alphaHttp string) (*dgo.Dgraph, *dgraphtest.HTTPClient, error) {
+	d, err := grpc.Dial(alphaGrpc, grpc.WithTransportCredentials(insecure.NewCredentials()))
+	if err != nil {
+		return nil, nil, errors.Wrapf(err, "while dialing gRPC server")
+	}
+
+	return dgo.NewDgraphClient(api.NewDgraphClient(d)), dgraphtest.GetHttpClient(alphaHttp), nil
+}
+
+func findDuplicateNodes(aclNodes []ACLNode) []map[string][]string {
+	du := make(map[string][]string)
+	dg := make(map[string][]string)
+	dug := make(map[string][]string)
+
+	var duplicates []map[string][]string
+
+	for i, node1 := range aclNodes {
+		for j, node2 := range aclNodes {
+			if i != j && node1.DgraphXID == node2.DgraphXID {
+				if node1.DgraphType[0] == "dgraph.type.User" && node1.DgraphType[0] == node2.DgraphType[0] {
+					du[node1.DgraphXID] = []string{node1.UID, node2.UID}
+					break
+				} else if node1.DgraphType[0] == "dgraph.type.Group" && node1.DgraphType[0] == node2.DgraphType[0] {
+					dg[node1.DgraphXID] = []string{node1.UID, node2.UID}
+					break
+				} else {
+					dug[node1.DgraphXID] = []string{node1.UID, node2.UID}
+					break
+				}
+			}
+		}
+	}
+
+	duplicates = append(duplicates, du, dg, dug)
+
+	return duplicates
+}
+
+func queryACLNodes(ctx context.Context, dg *dgo.Dgraph) ([]map[string][]string, error) {
+	query := `{ 
+		nodes(func: has(dgraph.xid)) {
+			       uid
+			       dgraph.xid
+                   dgraph.type
+		        }
+	}`
+
+	resp, err := dg.NewTxn().Query(ctx, query)
+	if err != nil {
+		return nil, errors.Wrapf(err, "while querying dgraph for duplicate nodes")
+	}
+
+	type Nodes struct {
+		Nodes []ACLNode `json:"nodes"`
+	}
+	var result Nodes
+	if err := json.Unmarshal([]byte(resp.Json), &result); err != nil {
+		return nil, errors.Wrapf(err, "while unmarshalling response: %v", string(resp.Json))
+
+	}
+
+	return findDuplicateNodes(result.Nodes), nil
+}
+
+func printDuplicates(entityType string, ns uint64, nodesmap map[string][]string) {
+	if len(nodesmap) > 0 {
+		fmt.Printf("Found duplicate %ss in namespace: #%v\n", entityType, ns)
+		for key, node := range nodesmap {
+			fmt.Printf("dgraph.xid %v , Uids: %v\n", key, node)
+		}
+		fmt.Println("")
+	}
+}
+
+func init() {
+	CheckUpgrade.Cmd = &cobra.Command{
+		Use:   "checkupgrade",
+		Short: "Run the checkupgrade tool",
+		Long:  "The checkupgrade tool is used to check for duplicate dgraph.xid's in the Dgraph database before upgrade.",
+		Run: func(cmd *cobra.Command, args []string) {
+			run()
+		},
+		Annotations: map[string]string{"group": "tool"},
+	}
+	CheckUpgrade.Cmd.SetHelpTemplate(x.NonRootTemplate)
+	flag := CheckUpgrade.Cmd.Flags()
+	flag.String(alphaGrpc, "127.0.0.1:9080",
+		"Dgraph Alpha gRPC server address")
+
+	flag.String(alphaHttp, "http://127.0.0.1:8080", "Draph Alpha HTTP(S) endpoint.")
+}
+
+func run() {
+	if err := checkUpgrade(); err != nil {
+		fmt.Fprintln(os.Stderr, err)
+	}
+}
+
+func checkUpgrade() error {
+	fmt.Println("Running check-upgrade tool")
+
+	cmdInput := parseInput()
+
+	gc, hc, err := setupClients(cmdInput.alphaGrpc, cmdInput.alphaHttp)
+	if err != nil {
+		return errors.Wrapf(err, "while setting up clients")
+	}
+
+	hc.LoginIntoNamespace(dgraphtest.DefaultUser, dgraphtest.DefaultPassword, x.GalaxyNamespace)
+	if err != nil {
+		return errors.Wrapf(err, "while logging into namespace: %v", x.GalaxyNamespace)
+	}
+
+	namespaces, err := hc.ListNamespaces()
+	if err != nil {
+		return err
+	}
+
+	ctx := context.Background()
+	for _, ns := range namespaces {
+		if err := gc.LoginIntoNamespace(ctx, dgraphtest.DefaultUser, dgraphtest.DefaultPassword, ns); err != nil {
+			return errors.Wrapf(err, "while logging into namespace: %v", ns)
+		}
+
+		duplicates, err := queryACLNodes(ctx, gc)
+		if err != nil {
+			return err
+		}
+
+		printDuplicates("user", ns, duplicates[0])
+		// example output:
+		//	Found duplicate users in namespace: #0
+		// dgraph.xid user1 , Uids: [0x4 0x3]
+		printDuplicates("group", ns, duplicates[1])
+		// Found duplicate groups in namespace: #1
+		// dgraph.xid group1 , Uids: [0x2714 0x2711]
+		printDuplicates("groups and user", ns, duplicates[2])
+		// Found duplicate groups and users in namespace: #0
+		// dgraph.xid userGroup1 , Uids: [0x7532 0x7531]
+	}
+
+	fmt.Println("To delete duplicate nodes use following mutation: ")
+	deleteMut := `
+	{
+		delete {
+			<UID> * * .
+		}
+	}`
+	fmt.Fprint(os.Stderr, deleteMut)
+
+	return nil
+}
+func parseInput() *commandInput {
+	return &commandInput{alphaGrpc: CheckUpgrade.Conf.GetString(alphaGrpc), alphaHttp: CheckUpgrade.Conf.GetString(alphaHttp)}
+}

dgraph/cmd/root.go

@@ -32,6 +32,7 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 
+	checkupgrade "github.com/dgraph-io/dgraph/check_upgrade"
 	"github.com/dgraph-io/dgraph/dgraph/cmd/alpha"
 	"github.com/dgraph-io/dgraph/dgraph/cmd/bulk"
 	"github.com/dgraph-io/dgraph/dgraph/cmd/cert"
@@ -84,6 +85,7 @@ var rootConf = viper.New()
 var subcommands = []*x.SubCommand{
 	&bulk.Bulk, &cert.Cert, &conv.Conv, &live.Live, &alpha.Alpha, &zero.Zero, &version.Version,
 	&debug.Debug, &migrate.Migrate, &debuginfo.DebugInfo, &upgrade.Upgrade, &decrypt.Decrypt, &increment.Increment,
+	&checkupgrade.CheckUpgrade,
 }
 
 func initCmds() {

dgraphtest/cluster.go

@@ -751,3 +751,12 @@ func IsHigherVersion(higher, lower string) (bool, error) {
 
 	return true, nil
 }
+
+func GetHttpClient(alphaUrl string) *HTTPClient {
+	return &HTTPClient{
+		adminURL:   alphaUrl + "/admin",
+		graphqlURL: alphaUrl + "/graphql",
+		dqlURL:     alphaUrl + "/query",
+		HttpToken:  &HttpToken{},
+	}
+}

dgraphtest/ee.go

@@ -123,7 +123,7 @@ func (hc *HTTPClient) CreateGroup(name string) (string, error) {
 	}
 	resp, err := hc.RunGraphqlQuery(params, true)
 	if err != nil {
-		return "", nil
+		return "", err
 	}
 	type Response struct {
 		AddGroup struct {
@@ -453,3 +453,27 @@ func (hc *HTTPClient) DeleteNamespace(nsID uint64) (uint64, error) {
 	}
 	return 0, errors.New(result.DeleteNamespace.Message)
 }
+
+func (hc *HTTPClient) ListNamespaces() ([]uint64, error) {
+	const listNss = `{ state {
+		namespaces
+	  }
+	}`
+
+	params := GraphQLParams{Query: listNss}
+	resp, err := hc.RunGraphqlQuery(params, true)
+	if err != nil {
+		return nil, err
+	}
+
+	var result struct {
+		State struct {
+			Namespaces []uint64 `json:"namespaces"`
+		} `json:"state"`
+	}
+	if err := json.Unmarshal(resp, &result); err != nil {
+		return nil, errors.Wrap(err, "error unmarshalling response")
+	}
+
+	return result.State.Namespaces, nil
+}

ee/acl/acl_integration_test.go

@@ -412,6 +412,7 @@ func (asuite *AclTestSuite) TestWrongPermission() {
 	defer cleanup()
 	require.NoError(t, gc.LoginIntoNamespace(ctx, dgraphtest.DefaultUser,
 		dgraphtest.DefaultPassword, x.GalaxyNamespace))
+	require.NoError(t, gc.DropAll())
 
 	mu := &api.Mutation{SetNquads: []byte(`
 	_:dev <dgraph.type> "dgraph.type.Group" .
@@ -437,3 +438,20 @@ func (asuite *AclTestSuite) TestWrongPermission() {
 	require.Error(t, err, "Setting permission to -1 should have returned error")
 	require.Contains(t, err.Error(), "Value for this predicate should be between 0 and 7")
 }
+
+func (asuite *AclTestSuite) TestACLDuplicateGrootUser() {
+	t := asuite.T()
+	gc, cleanup, err := asuite.dc.Client()
+	require.NoError(t, err)
+	defer cleanup()
+	require.NoError(t, gc.LoginIntoNamespace(context.Background(),
+		dgraphtest.DefaultUser, dgraphtest.DefaultPassword, x.GalaxyNamespace))
+
+	rdfs := `_:a <dgraph.xid> "groot" .
+	         _:a <dgraph.type> "dgraph.type.User"  .`
+
+	mu := &api.Mutation{SetNquads: []byte(rdfs), CommitNow: true}
+	_, err = gc.Mutate(mu)
+	require.Error(t, err)
+	require.ErrorContains(t, err, "could not insert duplicate value [groot] for predicate [dgraph.xid]")
+}