From 592bd6fa35961fd492b58e1f570e2f26622b6857 Mon Sep 17 00:00:00 2001 From: minhaj-shakeel Date: Fri, 30 Jul 2021 18:48:56 +0530 Subject: [PATCH] cherry pick: release/v21.03: The Acl cache should be updated on restart and restore. (#7964) --- dgraph/cmd/alpha/run.go | 5 +- edgraph/access.go | 15 ++- edgraph/access_ee.go | 90 +++++++------ edgraph/server.go | 4 +- graphql/admin/restore.go | 2 +- systest/acl/restore/acl-secret | 1 + systest/acl/restore/acl_restore_test.go | 124 ++++++++++++++++++ .../dgraph.20210730.124449.146/r21-g1.backup | Bin 0 -> 1828 bytes .../acl/restore/data/backups/manifest.json | 33 +++++ systest/acl/restore/docker-compose.yml | 50 +++++++ systest/online-restore/online_restore_test.go | 1 - {edgraph => worker}/acl_cache.go | 56 ++++++-- {edgraph => worker}/acl_cache_test.go | 16 +-- worker/online_restore_ee.go | 1 + 14 files changed, 329 insertions(+), 69 deletions(-) create mode 100644 systest/acl/restore/acl-secret create mode 100644 systest/acl/restore/acl_restore_test.go create mode 100644 systest/acl/restore/data/backups/dgraph.20210730.124449.146/r21-g1.backup create mode 100644 systest/acl/restore/data/backups/manifest.json create mode 100644 systest/acl/restore/docker-compose.yml rename {edgraph => worker}/acl_cache.go (80%) rename {edgraph => worker}/acl_cache_test.go (78%) diff --git a/dgraph/cmd/alpha/run.go b/dgraph/cmd/alpha/run.go index 58d8cc812e0..54e8d42fd52 100644 --- a/dgraph/cmd/alpha/run.go +++ b/dgraph/cmd/alpha/run.go @@ -814,8 +814,9 @@ func run() { // initialization of the admin account can only be done after raft nodes are running // and health check passes - edgraph.ResetAcl(updaters) - edgraph.RefreshAcls(updaters) + edgraph.InitializeAcl(updaters) + edgraph.RefreshACLs(updaters.Ctx()) + edgraph.SubscribeForAclUpdates(updaters) }() // Graphql subscribes to alpha to get schema updates. We need to close that before we diff --git a/edgraph/access.go b/edgraph/access.go index 88bb5b985b8..3c1ad80a680 100644 --- a/edgraph/access.go +++ b/edgraph/access.go @@ -42,17 +42,26 @@ func (s *Server) Login(ctx context.Context, } // ResetAcl is an empty method since ACL is only supported in the enterprise version. -func ResetAcl(closer *z.Closer) { +func InitializeAcl(closer *z.Closer) { // do nothing } -// ResetAcls is an empty method since ACL is only supported in the enterprise version. -func RefreshAcls(closer *z.Closer) { +func upsertGuardianAndGroot(closer *z.Closer, ns uint64) { + // do nothing +} + +// SubscribeForAclUpdates is an empty method since ACL is only supported in the enterprise version. +func SubscribeForAclUpdates(closer *z.Closer) { // do nothing <-closer.HasBeenClosed() closer.Done() } +// RefreshACLs is an empty method since ACL is only supported in the enterprise version. +func RefreshACLs(ctx context.Context) { + return +} + func authorizeAlter(ctx context.Context, op *api.Operation) error { return nil } diff --git a/edgraph/access_ee.go b/edgraph/access_ee.go index dff3eb26dd8..42128193d68 100644 --- a/edgraph/access_ee.go +++ b/edgraph/access_ee.go @@ -312,8 +312,43 @@ func authorizeUser(ctx context.Context, userid string, password string) ( return user, nil } -// RefreshAcls queries for the ACL triples and refreshes the ACLs accordingly. -func RefreshAcls(closer *z.Closer) { +func refreshAclCache(ctx context.Context, ns, refreshTs uint64) error { + req := &Request{ + req: &api.Request{ + Query: queryAcls, + ReadOnly: true, + StartTs: refreshTs, + }, + doAuth: NoAuthorize, + } + + ctx = x.AttachNamespace(ctx, ns) + queryResp, err := (&Server{}).doQuery(ctx, req) + if err != nil { + return errors.Errorf("unable to retrieve acls: %v", err) + } + groups, err := acl.UnmarshalGroups(queryResp.GetJson(), "allAcls") + if err != nil { + return err + } + + worker.AclCachePtr.Update(ns, groups) + glog.V(2).Infof("Updated the ACL cache for namespace: %#x", ns) + return nil + +} + +func RefreshACLs(ctx context.Context) { + for ns := range schema.State().Namespaces() { + if err := refreshAclCache(ctx, ns, 0); err != nil { + glog.Errorf("Error while retrieving acls for namespace %#x: %v", ns, err) + } + } + worker.AclCachePtr.Set() +} + +// SubscribeForAclUpdates subscribes for ACL predicates and updates the acl cache. +func SubscribeForAclUpdates(closer *z.Closer) { defer func() { glog.Infoln("RefreshAcls closed") closer.Done() @@ -323,38 +358,13 @@ func RefreshAcls(closer *z.Closer) { return } - // retrieve the full data set of ACLs from the corresponding alpha server, and update the - // aclCachePtr var maxRefreshTs uint64 retrieveAcls := func(ns uint64, refreshTs uint64) error { if refreshTs <= maxRefreshTs { return nil } maxRefreshTs = refreshTs - - glog.V(3).Infof("Refreshing ACLs") - req := &Request{ - req: &api.Request{ - Query: queryAcls, - ReadOnly: true, - StartTs: refreshTs, - }, - doAuth: NoAuthorize, - } - - ctx := x.AttachNamespace(closer.Ctx(), ns) - queryResp, err := (&Server{}).doQuery(ctx, req) - if err != nil { - return errors.Errorf("unable to retrieve acls: %v", err) - } - groups, err := acl.UnmarshalGroups(queryResp.GetJson(), "allAcls") - if err != nil { - return err - } - - aclCachePtr.update(ns, groups) - glog.V(3).Infof("Updated the ACL cache") - return nil + return refreshAclCache(closer.Ctx(), ns, refreshTs) } closer.AddRunning(1) @@ -402,10 +412,10 @@ var aclPrefixes = [][]byte{ x.PredicatePrefix(x.GalaxyAttr("dgraph.xid")), } -// clears the aclCachePtr and upserts the Groot account. -func ResetAcl(closer *z.Closer) { +// upserts the Groot account. +func InitializeAcl(closer *z.Closer) { defer func() { - glog.Infof("ResetAcl closed") + glog.Infof("InitializeAcl closed") closer.Done() }() @@ -611,12 +621,16 @@ func authorizePreds(ctx context.Context, userData *userData, preds []string, if err != nil { return nil, errors.Wrapf(err, "While authorizing preds") } + if !worker.AclCachePtr.Loaded() { + RefreshACLs(ctx) + } + userId := userData.userId groupIds := userData.groupIds blockedPreds := make(map[string]struct{}) for _, pred := range preds { nsPred := x.NamespaceAttr(ns, pred) - if err := aclCachePtr.authorizePredicate(groupIds, nsPred, aclOp); err != nil { + if err := worker.AclCachePtr.AuthorizePredicate(groupIds, nsPred, aclOp); err != nil { logAccess(&accessEntry{ userId: userId, groups: groupIds, @@ -628,22 +642,20 @@ func authorizePreds(ctx context.Context, userData *userData, preds []string, blockedPreds[pred] = struct{}{} } } - aclCachePtr.RLock() - allowedPreds := make([]string, len(aclCachePtr.userPredPerms[userId])) // User can have multiple permission for same predicate, add predicate + allowedPreds := make([]string, len(worker.AclCachePtr.GetUserPredPerms(userId))) // only if the acl.Op is covered in the set of permissions for the user - for predicate, perm := range aclCachePtr.userPredPerms[userId] { + for predicate, perm := range worker.AclCachePtr.GetUserPredPerms(userId) { if (perm & aclOp.Code) > 0 { allowedPreds = append(allowedPreds, predicate) } } - aclCachePtr.RUnlock() return &authPredResult{allowed: allowedPreds, blocked: blockedPreds}, nil } // authorizeAlter parses the Schema in the operation and authorizes the operation -// using the aclCachePtr. It will return error if any one of the predicates specified in alter -// are not authorized. +// using the worker.AclCachePtr. It will return error if any one of the predicates +// specified in alter are not authorized. func authorizeAlter(ctx context.Context, op *api.Operation) error { if len(worker.Config.HmacSecret) == 0 { // the user has not turned on the acl feature @@ -768,7 +780,7 @@ func isAclPredMutation(nquads []*api.NQuad) bool { return false } -// authorizeMutation authorizes the mutation using the aclCachePtr. It will return permission +// authorizeMutation authorizes the mutation using the worker.AclCachePtr. It will return permission // denied error if any one of the predicates in mutation(set or delete) is unauthorized. // At this stage, namespace is not attached in the predicates. func authorizeMutation(ctx context.Context, gmu *gql.Mutation) error { diff --git a/edgraph/server.go b/edgraph/server.go index 7cf906fd174..2d8415208a6 100644 --- a/edgraph/server.go +++ b/edgraph/server.go @@ -409,7 +409,7 @@ func (s *Server) Alter(ctx context.Context, op *api.Operation) (*api.Payload, er // reset their in-memory GraphQL schema _, err = UpdateGQLSchema(ctx, "", "") // recreate the admin account after a drop all operation - ResetAcl(nil) + InitializeAcl(nil) return empty, err } @@ -447,7 +447,7 @@ func (s *Server) Alter(ctx context.Context, op *api.Operation) (*api.Payload, er // just reinsert the GraphQL schema, no need to alter dgraph schema as this was drop_data _, err = UpdateGQLSchema(ctx, graphQLSchema, "") // recreate the admin account after a drop data operation - ResetAcl(nil) + InitializeAcl(nil) return empty, err } diff --git a/graphql/admin/restore.go b/graphql/admin/restore.go index 4dfb2f6b738..1496653c25a 100644 --- a/graphql/admin/restore.go +++ b/graphql/admin/restore.go @@ -84,7 +84,7 @@ func resolveRestore(ctx context.Context, m schema.Mutation) (*resolve.Resolved, go func() { wg.Wait() - edgraph.ResetAcl(nil) + edgraph.InitializeAcl(nil) }() return resolve.DataResult( diff --git a/systest/acl/restore/acl-secret b/systest/acl/restore/acl-secret new file mode 100644 index 00000000000..f02b5b99605 --- /dev/null +++ b/systest/acl/restore/acl-secret @@ -0,0 +1 @@ +12345678901234567890123456789012 \ No newline at end of file diff --git a/systest/acl/restore/acl_restore_test.go b/systest/acl/restore/acl_restore_test.go new file mode 100644 index 00000000000..fb7710fee01 --- /dev/null +++ b/systest/acl/restore/acl_restore_test.go @@ -0,0 +1,124 @@ +package main + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io/ioutil" + "net/http" + "testing" + + "github.com/dgraph-io/dgo/v210" + "github.com/dgraph-io/dgo/v210/protos/api" + "github.com/dgraph-io/dgraph/graphql/e2e/common" + "github.com/dgraph-io/dgraph/testutil" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" +) + +// disableDraining disables draining mode before each test for increased reliability. +func disableDraining(t *testing.T) { + drainRequest := `mutation draining { + draining(enable: false) { + response { + code + message + } + } + }` + + params := testutil.GraphQLParams{ + Query: drainRequest, + } + b, err := json.Marshal(params) + require.NoError(t, err) + + token := testutil.Login(t, &testutil.LoginParams{UserID: "groot", Passwd: "password", Namespace: 0}) + + client := &http.Client{} + req, err := http.NewRequest("POST", testutil.AdminUrl(), bytes.NewBuffer(b)) + require.Nil(t, err) + req.Header.Add("content-type", "application/json") + req.Header.Add("X-Dgraph-AccessToken", token.AccessJwt) + + resp, err := client.Do(req) + require.NoError(t, err) + buf, err := ioutil.ReadAll(resp.Body) + fmt.Println(string(buf)) + require.NoError(t, err) + require.Contains(t, string(buf), "draining mode has been set to false") +} + +func sendRestoreRequest(t *testing.T, location, backupId string, backupNum int) { + if location == "" { + location = "/data/backup2" + } + params := &testutil.GraphQLParams{ + Query: `mutation restore($location: String!, $backupId: String, $backupNum: Int) { + restore(input: {location: $location, backupId: $backupId, backupNum: $backupNum}) { + code + message + } + }`, + Variables: map[string]interface{}{ + "location": location, + "backupId": backupId, + "backupNum": backupNum, + }, + } + + token := testutil.Login(t, &testutil.LoginParams{UserID: "groot", Passwd: "password", Namespace: 0}) + resp := testutil.MakeGQLRequestWithAccessJwt(t, params, token.AccessJwt) + resp.RequireNoGraphQLErrors(t) + + var restoreResp struct { + Restore struct { + Code string + Message string + RestoreId int + } + } + require.NoError(t, json.Unmarshal(resp.Data, &restoreResp)) + require.Equal(t, restoreResp.Restore.Code, "Success") +} + +func TestAclCacheRestore(t *testing.T) { + disableDraining(t) + conn, err := grpc.Dial(testutil.SockAddr, grpc.WithInsecure()) + require.NoError(t, err) + dg := dgo.NewDgraphClient(api.NewDgraphClient(conn)) + dg.Login(context.Background(), "groot", "password") + + sendRestoreRequest(t, "/backups", "vibrant_euclid5", 1) + testutil.WaitForRestore(t, dg) + + token := testutil.Login(t, + &testutil.LoginParams{UserID: "alice1", Passwd: "password", Namespace: 0}) + params := &common.GraphQLParams{ + Query: `query{ + queryPerson{ + name + age + } + }`, + + Headers: make(http.Header), + } + params.Headers.Set("X-Dgraph-AccessToken", token.AccessJwt) + + resp := params.ExecuteAsPost(t, common.GraphqlURL) + require.Nil(t, resp.Errors) + + expected := ` + { + "queryPerson": [ + { + "name": "MinhajSh", + "age": 20 + } + ] + } + ` + require.JSONEq(t, expected, string(resp.Data)) +} diff --git a/systest/acl/restore/data/backups/dgraph.20210730.124449.146/r21-g1.backup b/systest/acl/restore/data/backups/dgraph.20210730.124449.146/r21-g1.backup new file mode 100644 index 0000000000000000000000000000000000000000..c9fc5de70812c532d9c91a1ccd2356b428f95c60 GIT binary patch literal 1828 zcmX9!8lk1tmM)W$EXk&6n~>5@X^GTG-Lz%ZwrEYYt=YHGI;lTeT21PvDLZWc z-uImMygJYGyyrQOO#tPDn$l*$Zie6fZ1Bx}7e599S|D*EVMIwPtCvefzq*)}dHPA} z6dsERQ*0A_0gyQt`Yg&F(JVe9Iy)XeMhytNEGc9lf`=!kgw)`|qONK=|9XBwMzn)A zJ5n9OVpY`_@@lax3&u1?8aJEUMf|@7&d79a111#6`YK+ zz(~Ec(GinP>T-TQ8x-TXMynl;1FlVRLAUcQFh$&`shC|mrI)JO>KNEsNj>JMa}4{; zoN!*E1|KpiPoNTZ6MLi$coS!Q4|#w?9`p`wCk+t~@Pz9w97UCk;G?6dn)TPOs_>YXpVUDDP@y=kXvY}-e@E+~-WHLi<0f7$-sqi=m}ukH;<#xpR;(Qeu*gm?e>o!^)f?^4O10n2nb ztBcoT4X#)I`ukh2{%h;jyT-I~0?x$KFle9wdQ1p5zh-@S-dmwHVmfx8Us)EDJy;Wb z!tUOC7am;x^QDRs0U?%ajff%R2ZKCgxKX%Se7IYmKB!WYcSejW{chprA09sc?`xMn zuA(@$q`1UBfbqqBh}{YAgYw|?HUiV-ZDh=Zw-L%5re@>yru+2Xk=w&RxXTtTnd)*@ z{IKV!X7iEzH@^JG*Wdot+jm>H!8Y>|06s{A%|m3Sg#TG|D24aw|4G3I3)oLw!|lM= za-H`&KSTjApn&o>93sbt0|3+-P(B#{sfF@BZsR@8Fk5@o9g}aQt#+e zG(I1go6tsAX4lm4@?=unitUBBKxx1F44@Y_ZB?eafAPl47LJYU_qxz`pK4?6QQOO% zs0|E>1OQ^mt;1Cl3qF$1GHMZ}oo`H>-#;N4Xg~E0T!p&GF|d{AbJ>DgSlGG}`4~oC zlLm<&bBWIY@3W_$x9UO?y%al#hNTi|#OTR&V5i6FamTm8qY`lCi}>?IxHozx;+m}J z`QjoGm>Wb~Dh(+yjsc1X4*+V;aKtC!S*EnN-q2#w3UOY z;xtH@`l-t?9pcRT3`jl1gSp)7jM;T{zVa!AA~GUfq!yb( zudoL7!vhD?AVkTU^`xBSW(!C^6UA;btH`v2R2o`){YBBtPtni1PDs- zBqTyH$fF$xPdk{2h7}n4MY>-~k_J|sw!YA=om;79^$prVn&(~wvBj8?P34>#>J58W zAQno4aK3ttA&sUX(gJyc(LGLBw~-Vf*xVc;7CJ;~1bJnMr**Tp3(1jvp4vA0#Cps$ zMOS4)Rv9to>AT_qPtQj0a@K~gUPe4^UL_n_XHdULY}g!cWQ|lJTi+`5sTaUc>xK9i zz}po(V4PpeAWWV`@hza^0u