address comments

dgraph-io · Jun 12, 2023 · 423d358 · 423d358
1 parent ce9bca0
commit 423d358
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 31 deletions.
diff --git a/dgraphtest/acl.go b/dgraphtest/acl.go
@@ -79,7 +79,7 @@ func (hc *HTTPClient) GetCurrentUser() (string, error) {
 	params := GraphQLParams{Query: query}
 	resp, err := hc.RunGraphqlQuery(params, true)
 	if err != nil {
-		return string(resp), errors.Wrapf(err, "received response: %v", string(resp))
+		return "", errors.Wrapf(err, "received response: %v", string(resp))
 	}
 
 	var userResp struct {

diff --git a/dgraphtest/cluster.go b/dgraphtest/cluster.go
@@ -246,7 +246,7 @@ func (hc *HTTPClient) RunGraphqlQuery(params GraphQLParams, admin bool) ([]byte,
 		return nil, errors.Wrap(err, "error unmarshalling GQL response")
 	}
 	if len(gqlResp.Errors) > 0 {
-		return gqlResp.Data, errors.Wrapf(gqlResp.Errors, "error while running admin query")
+		return nil, errors.Wrapf(gqlResp.Errors, "error while running admin query, resp: %v", string(gqlResp.Data))
 	}
 	return gqlResp.Data, nil
 }

diff --git a/ee/acl/acl_integration_test.go b/ee/acl/acl_integration_test.go
@@ -37,9 +37,9 @@ func (suite *AclTestSuite) TestInvalidGetUser() {
 	require.NoError(t, err)
 	hc.HttpToken.AccessJwt = "invalid Token"
 	currentUser, err := hc.GetCurrentUser()
-	require.Equal(t, `{"getCurrentUser":null}`, currentUser)
 	require.Contains(t, err.Error(), "couldn't rewrite query getCurrentUser because "+
 		"unable to parse jwt token: token contains an invalid number of segments")
+	require.Equal(t, "", currentUser)
 }
 
 func (suite *AclTestSuite) TestPasswordReturn() {
@@ -362,7 +362,7 @@ func (suite *AclTestSuite) TestGuardianOnlyAccessForAdminEndpoints() {
 					require.Contains(t, err.Error(), tcase.guardianErr)
 				}
 
-				if tcase.guardianData != "" {
+				if tcase.guardianData != "" && err == nil {
 					require.JSONEq(t, tcase.guardianData, string(resp))
 				}
 			}

diff --git a/ee/acl/acl_test.go b/ee/acl/acl_test.go
@@ -112,9 +112,8 @@ func (suite *AclTestSuite) TestCreateAndDeleteUsers() {
 		dgraphtest.DefaultPassword, x.GalaxyNamespace))
 	user, err := hc.CreateUser(userid, userpassword)
 	require.Error(t, err)
-	require.Equal(t, "error while running admin query: couldn't rewrite mutation addUser because failed to "+
-		"rewrite mutation payload because id alice already exists for field name inside type User",
-		err.Error())
+	require.Contains(t, err.Error(), "couldn't rewrite mutation addUser because failed to rewrite "+
+		"mutation payload because id alice already exists for field name inside type User")
 	require.Equal(t, "", user)
 
 	// delete the user
@@ -446,16 +445,16 @@ func createGroupAndAcls(t *testing.T, group string, addUserToGroup bool, hc *dgr
 
 	rules := []dgraphtest.AclRule{
 		{
-			predicateToRead, Read.Code,
+			Predicate: predicateToRead, Permission: Read.Code,
 		},
 		{
-			queryAttr, Read.Code,
+			Predicate: queryAttr, Permission: Read.Code,
 		},
 		{
-			predicateToWrite, Write.Code,
+			Predicate: predicateToWrite, Permission: Write.Code,
 		},
 		{
-			predicateToAlter, Modify.Code,
+			Predicate: predicateToAlter, Permission: Modify.Code,
 		},
 	}
 
@@ -589,14 +588,17 @@ func (suite *AclTestSuite) TestUnauthorizedDeletion() {
 	nodeUID, ok := resp.Uids["a"]
 	require.True(t, ok)
 
-	require.NoError(t, hc.AddRulesToGroup(devGroup, []dgraphtest.AclRule{{unAuthPred, 0}}, true))
+	require.NoError(t, hc.AddRulesToGroup(devGroup, []dgraphtest.AclRule{{Predicate: unAuthPred, Permission: 0}}, true))
 	suite.Upgrade()
 	userClient, cleanup, err := suite.dc.Client()
 	require.NoError(t, err)
 	defer cleanup()
 	time.Sleep(defaultTimeToSleep)
 	require.NoError(t, userClient.LoginIntoNamespace(ctx, userid, userpassword, x.GalaxyNamespace))
-	mu = &api.Mutation{DelNquads: []byte(fmt.Sprintf("%s %s %s .", "<"+nodeUID+">", "<"+unAuthPred+">", "*")), CommitNow: true}
+	mu = &api.Mutation{
+		DelNquads: []byte(fmt.Sprintf("%s %s %s .", "<"+nodeUID+">", "<"+unAuthPred+">", "*")),
+		CommitNow: true,
+	}
 	_, err = userClient.Mutate(mu)
 
 	require.Error(t, err)
@@ -722,7 +724,8 @@ func (suite *AclTestSuite) TestQueryRemoveUnauthorizedPred() {
 	require.NoError(t, err)
 
 	// give read access of <name> to alice
-	require.NoError(t, hc.AddRulesToGroup(devGroup, []dgraphtest.AclRule{{"name", Read.Code}}, true))
+	require.NoError(t, hc.AddRulesToGroup(devGroup,
+		[]dgraphtest.AclRule{{Predicate: "name", Permission: Read.Code}}, true))
 	suite.Upgrade()
 	userClient, cleanup, err := suite.dc.Client()
 	// defer cleanup()
@@ -855,8 +858,8 @@ func (suite *AclTestSuite) TestExpandQueryWithACLPermissions() {
 	createdGroup, err = hc.CreateGroup(sreGroup)
 	require.NoError(t, err)
 	require.Equal(t, sreGroup, createdGroup)
-	require.NoError(t, hc.AddRulesToGroup(sreGroup,
-		[]dgraphtest.AclRule{{"age", Read.Code}, {"name", Write.Code}}, true))
+	require.NoError(t, hc.AddRulesToGroup(sreGroup, []dgraphtest.AclRule{{Predicate: "age", Permission: Read.Code},
+		{Predicate: "name", Permission: Write.Code}}, true))
 
 	require.NoError(t, hc.AddUserToGroup(userid, devGroup))
 
@@ -897,8 +900,8 @@ func (suite *AclTestSuite) TestExpandQueryWithACLPermissions() {
 	testutil.PollTillPassOrTimeout(t, userClient.Dgraph, query, `{}`, timeout)
 
 	// Give read access of <name>, write access of <age> to dev
-	require.NoError(t, hc.AddRulesToGroup(devGroup,
-		[]dgraphtest.AclRule{{"age", Write.Code}, {"name", Read.Code}}, true))
+	require.NoError(t, hc.AddRulesToGroup(devGroup, []dgraphtest.AclRule{{Predicate: "age", Permission: Write.Code},
+		{Predicate: "name", Permission: Read.Code}}, true))
 
 	testutil.PollTillPassOrTimeout(t, userClient.Dgraph, query,
 		`{"me":[{"name":"RandomGuy"},{"name":"RandomGuy2"}]}`, timeout)
@@ -917,8 +920,8 @@ func (suite *AclTestSuite) TestExpandQueryWithACLPermissions() {
 		dgraphtest.DefaultPassword, x.GalaxyNamespace))
 
 	// Give read access of <name> and <nickname>, write access of <age> to dev
-	require.NoError(t, hc.AddRulesToGroup(devGroup,
-		[]dgraphtest.AclRule{{"age", Write.Code}, {"name", Read.Code}, {"nickname", Read.Code}}, true))
+	require.NoError(t, hc.AddRulesToGroup(devGroup, []dgraphtest.AclRule{{Predicate: "age", Permission: Write.Code},
+		{Predicate: "name", Permission: Read.Code}, {Predicate: "nickname", Permission: Read.Code}}, true))
 
 	testutil.PollTillPassOrTimeout(t, userClient.Dgraph, query,
 		`{"me":[{"name":"RandomGuy","age":23, "nickname":"RG"},{"name":"RandomGuy2","age":25, "nickname":"RG2"}]}`,
@@ -988,8 +991,8 @@ func (suite *AclTestSuite) TestDeleteQueryWithACLPermissions() {
 	)
 
 	// Give Write Access to alice for name and age predicate
-	require.NoError(t, hc.AddRulesToGroup(devGroup,
-		[]dgraphtest.AclRule{{"name", Write.Code}, {"age", Write.Code}}, true))
+	require.NoError(t, hc.AddRulesToGroup(devGroup, []dgraphtest.AclRule{{Predicate: "name", Permission: Write.Code},
+		{Predicate: "age", Permission: Write.Code}}, true))
 	suite.Upgrade()
 	gc, _, err = suite.dc.Client()
 	require.NoError(t, err)
@@ -1019,8 +1022,8 @@ func (suite *AclTestSuite) TestDeleteQueryWithACLPermissions() {
 		string(resp.GetJson()))
 
 	// Give write access of <name> <dgraph.type> to dev
-	require.NoError(t, hc.AddRulesToGroup(devGroup,
-		[]dgraphtest.AclRule{{"name", Write.Code}, {"age", Write.Code}, {"dgraph.type", Write.Code}}, true))
+	require.NoError(t, hc.AddRulesToGroup(devGroup, []dgraphtest.AclRule{{Predicate: "name", Permission: Write.Code},
+		{Predicate: "age", Permission: Write.Code}, {Predicate: "dgraph.type", Permission: Write.Code}}, true))
 	time.Sleep(defaultTimeToSleep)
 
 	// delete S * * (user now has permission to name, age and dgraph.type)
@@ -1220,7 +1223,8 @@ func (suite *AclTestSuite) TestValQueryWithACLPermissions() {
 	}
 
 	// Give read access of <name> to dev
-	require.NoError(t, hc.AddRulesToGroup(devGroup, []dgraphtest.AclRule{{"name", Read.Code}}, true))
+	require.NoError(t, hc.AddRulesToGroup(devGroup,
+		[]dgraphtest.AclRule{{Predicate: "name", Permission: Read.Code}}, true))
 	time.Sleep(defaultTimeToSleep)
 
 	for _, tc := range tests {
@@ -1237,8 +1241,8 @@ func (suite *AclTestSuite) TestValQueryWithACLPermissions() {
 		dgraphtest.DefaultPassword, x.GalaxyNamespace))
 
 	// Give read access of <name> and <age> to dev
-	require.NoError(t, hc.AddRulesToGroup(devGroup,
-		[]dgraphtest.AclRule{{"name", Read.Code}, {"age", Read.Code}}, true))
+	require.NoError(t, hc.AddRulesToGroup(devGroup, []dgraphtest.AclRule{{Predicate: "name", Permission: Read.Code},
+		{Predicate: "age", Permission: Read.Code}}, true))
 	time.Sleep(defaultTimeToSleep)
 
 	for _, tc := range tests {
@@ -1374,7 +1378,8 @@ func (suite *AclTestSuite) TestAllPredsPermission() {
 		dgraphtest.DefaultPassword, x.GalaxyNamespace))
 
 	// Give read access of all predicates to dev
-	require.NoError(t, hc.AddRulesToGroup(devGroup, []dgraphtest.AclRule{{"dgraph.all", Read.Code}}, true))
+	require.NoError(t, hc.AddRulesToGroup(devGroup,
+		[]dgraphtest.AclRule{{Predicate: "dgraph.all", Permission: Read.Code}}, true))
 	time.Sleep(defaultTimeToSleep)
 
 	for _, tc := range tests {
@@ -1399,7 +1404,8 @@ func (suite *AclTestSuite) TestAllPredsPermission() {
 	require.Contains(t, err.Error(), "unauthorized to mutate")
 
 	// Give write access of all predicates to dev. Now mutation should succeed.
-	require.NoError(t, hc.AddRulesToGroup(devGroup, []dgraphtest.AclRule{{"dgraph.all", Write.Code | Read.Code}}, true))
+	require.NoError(t, hc.AddRulesToGroup(devGroup,
+		[]dgraphtest.AclRule{{Predicate: "dgraph.all", Permission: Write.Code | Read.Code}}, true))
 	time.Sleep(defaultTimeToSleep)
 
 	_, err = userClient.Mutate(mu)
@@ -1626,7 +1632,8 @@ func (suite *AclTestSuite) TestNonExistentGroup() {
 	require.NoError(t, err)
 	require.NoError(t, hc.LoginIntoNamespace(dgraphtest.DefaultUser,
 		dgraphtest.DefaultPassword, x.GalaxyNamespace))
-	require.NoError(t, hc.AddRulesToGroup(devGroup, []dgraphtest.AclRule{{"name", Read.Code}}, true))
+	require.NoError(t, hc.AddRulesToGroup(devGroup,
+		[]dgraphtest.AclRule{{Predicate: "name", Permission: Read.Code}}, true))
 }
 
 func (suite *AclTestSuite) TestQueryUserInfo() {
@@ -2405,7 +2412,8 @@ func (suite *AclTestSuite) TestAllowUIDAccess() {
 	require.NoError(t, err)
 
 	// give read access of <name> to alice
-	require.NoError(t, hc.AddRulesToGroup(devGroup, []dgraphtest.AclRule{{"name", Read.Code}}, true))
+	require.NoError(t, hc.AddRulesToGroup(devGroup,
+		[]dgraphtest.AclRule{{Predicate: "name", Permission: Read.Code}}, true))
 	suite.Upgrade()
 	userClient, cancel, err := suite.dc.Client()
 	require.NoError(t, err)