From 423d3580ad88711ffb1278184f9f9a8848055f3f Mon Sep 17 00:00:00 2001 From: Aman Mangal Date: Wed, 7 Jun 2023 13:31:10 -0400 Subject: [PATCH] address comments --- dgraphtest/acl.go | 2 +- dgraphtest/cluster.go | 2 +- ee/acl/acl_integration_test.go | 4 +-- ee/acl/acl_test.go | 62 +++++++++++++++++++--------------- 4 files changed, 39 insertions(+), 31 deletions(-) diff --git a/dgraphtest/acl.go b/dgraphtest/acl.go index 04a747ee0d2..23ca22282be 100644 --- 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 index 5b7b18f17a3..86dea875065 100644 --- 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 index b1019f0eb81..50dc068e145 100644 --- 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 index d1cd80c3f5c..eff2aa1fbe6 100644 --- 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 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 , write access of 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 and , write access of 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 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 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 and 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 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)