[BREAKING] Change default behavior to block operations in ACL

contrib/integration/testtxn/main_test.go

@@ -35,7 +35,6 @@ import (
 	"github.com/dgraph-io/dgraph/x"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"google.golang.org/grpc"
 )
 
 type state struct {
@@ -214,37 +213,26 @@ func TestTxnRead5(t *testing.T) {
 
 	require.NoError(t, txn.Commit(context.Background()))
 	q := fmt.Sprintf(`{ me(func: uid(%s)) { name }}`, uid)
-	// We don't supply startTs, it should be fetched from zero by dgraph alpha.
-	req := api.Request{
-		Query: q,
-	}
-
-	conn, err := grpc.Dial(addr, grpc.WithInsecure())
-	if err != nil {
-		log.Fatal(err)
-	}
-	dc := api.NewDgraphClient(conn)
 
-	resp, err := dc.Query(context.Background(), &req)
+	txn = s.dg.NewReadOnlyTxn()
+	resp, err := txn.Query(context.Background(), q)
 	if err != nil {
 		log.Fatalf("Error while running query: %v\n", err)
 	}
 	x.AssertTrue(bytes.Equal(resp.Json, []byte("{\"me\":[{\"name\":\"Manish\"}]}")))
 	x.AssertTrue(resp.Txn.StartTs > 0)
 
-	mu = &api.Mutation{}
+	mu = &api.Mutation{CommitNow: true}
 	mu.SetJson = []byte(fmt.Sprintf("{\"uid\": \"%s\", \"name\": \"Manish2\"}", uid))
 
-	muReq := api.Request{
-		Mutations: []*api.Mutation{mu},
-		CommitNow: true,
-	}
-	res, err := dc.Query(context.Background(), &muReq)
+	txn = s.dg.NewTxn()
+	res, err := txn.Mutate(context.Background(), mu)
 	if err != nil {
 		log.Fatalf("Error while running mutation: %v\n", err)
 	}
 	x.AssertTrue(res.Txn.StartTs > 0)
-	resp, err = dc.Query(context.Background(), &req)
+	txn = s.dg.NewReadOnlyTxn()
+	resp, err = txn.Query(context.Background(), q)
 	if err != nil {
 		log.Fatalf("Error while running query: %v\n", err)
 	}

dgraph/cmd/alpha/http_test.go

@@ -21,6 +21,7 @@ import (
 	"compress/gzip"
 	"encoding/json"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
@@ -49,6 +50,52 @@ type params struct {
 	Variables map[string]string `json:"variables"`
 }
 
+// runGzipWithRetry makes request gzip compressed request. If access token is expired,
+// it will try to refresh access token.
+func runGzipWithRetry(contentType, url string, buf io.Reader, gzReq, gzResp bool) (
+	*http.Response, error) {
+
+	client := &http.Client{}
+	numRetries := 2
+
+	var resp *http.Response
+	var err error
+	for i := 0; i < numRetries; i++ {
+		req, err := http.NewRequest("POST", url, buf)
+		if err != nil {
+			return nil, err
+		}
+		req.Header.Add("Content-Type", contentType)
+		req.Header.Set("X-Dgraph-AccessToken", grootAccessJwt)
+
+		if gzReq {
+			req.Header.Set("Content-Encoding", "gzip")
+		}
+
+		if gzResp {
+			req.Header.Set("Accept-Encoding", "gzip")
+		}
+
+		resp, err = client.Do(req)
+		if err != nil && strings.Contains(err.Error(), "Token is expired") {
+			grootAccessJwt, grootRefreshJwt, err = testutil.HttpLogin(&testutil.LoginParams{
+				Endpoint:   addr + "/login",
+				RefreshJwt: grootRefreshJwt,
+			})
+
+			if err != nil {
+				return nil, err
+			}
+			continue
+		} else if err != nil {
+			return nil, err
+		}
+		break
+	}
+
+	return resp, err
+}
+
 func queryWithGz(queryText, contentType, debug, timeout string, gzReq, gzResp bool) (
 	string, *http.Response, error) {
 
@@ -72,31 +119,17 @@ func queryWithGz(queryText, contentType, debug, timeout string, gzReq, gzResp bo
 		buf = bytes.NewBufferString(queryText)
 	}
 
-	req, err := http.NewRequest("POST", url, buf)
+	resp, err := runGzipWithRetry(contentType, url, buf, gzReq, gzResp)
 	if err != nil {
 		return "", nil, err
 	}
-	req.Header.Add("Content-Type", contentType)
-
-	if gzReq {
-		req.Header.Set("Content-Encoding", "gzip")
-	}
-
-	if gzResp {
-		req.Header.Set("Accept-Encoding", "gzip")
-	}
 
-	client := &http.Client{}
-	resp, err := client.Do(req)
+	defer resp.Body.Close()
+	rd := resp.Body
 	if err != nil {
 		return "", nil, err
 	}
-	if status := resp.StatusCode; status != http.StatusOK {
-		return "", nil, errors.Errorf("Unexpected status code: %v", status)
-	}
 
-	defer resp.Body.Close()
-	rd := resp.Body
 	if gzResp {
 		if strings.Contains(resp.Header.Get("Content-Encoding"), "gzip") {
 			rd, err = gzip.NewReader(rd)

dgraph/cmd/live/load-json/load_test.go

@@ -103,7 +103,7 @@ func TestLiveLoadJSONFileEmpty(t *testing.T) {
 		{"echo", "[]"},
 		{os.ExpandEnv("$GOPATH/bin/dgraph"), "live",
 			"--schema", testDataDir + "/family.schema", "--files", "/dev/stdin",
-			"--alpha", alphaService, "--zero", zeroService},
+			"--alpha", alphaService, "--zero", zeroService, "-u", "groot", "-p", "password"},
 	}
 	err := testutil.Pipeline(pipeline)
 	require.NoError(t, err, "live loading JSON file ran successfully")
@@ -115,7 +115,7 @@ func TestLiveLoadJSONFile(t *testing.T) {
 	pipeline := [][]string{
 		{os.ExpandEnv("$GOPATH/bin/dgraph"), "live",
 			"--schema", testDataDir + "/family.schema", "--files", testDataDir + "/family.json",
-			"--alpha", alphaService, "--zero", zeroService},
+			"--alpha", alphaService, "--zero", zeroService, "-u", "groot", "-p", "password"},
 	}
 	err := testutil.Pipeline(pipeline)
 	require.NoError(t, err, "live loading JSON file exited with error")
@@ -130,7 +130,7 @@ func TestLiveLoadJSONCompressedStream(t *testing.T) {
 		{"gzip", "-c", testDataDir + "/family.json"},
 		{os.ExpandEnv("$GOPATH/bin/dgraph"), "live",
 			"--schema", testDataDir + "/family.schema", "--files", "/dev/stdin",
-			"--alpha", alphaService, "--zero", zeroService},
+			"--alpha", alphaService, "--zero", zeroService, "-u", "groot", "-p", "password"},
 	}
 	err := testutil.Pipeline(pipeline)
 	require.NoError(t, err, "live loading JSON stream exited with error")
@@ -151,7 +151,7 @@ func TestLiveLoadJSONMultipleFiles(t *testing.T) {
 	pipeline := [][]string{
 		{os.ExpandEnv("$GOPATH/bin/dgraph"), "live",
 			"--schema", testDataDir + "/family.schema", "--files", fileList,
-			"--alpha", alphaService, "--zero", zeroService},
+			"--alpha", alphaService, "--zero", zeroService, "-u", "groot", "-p", "password"},
 	}
 	err := testutil.Pipeline(pipeline)
 	require.NoError(t, err, "live loading multiple JSON files exited with error")

dgraph/cmd/live/load-uids/load_test.go

@@ -122,7 +122,7 @@ func TestLiveLoadJsonUidKeep(t *testing.T) {
 	pipeline := [][]string{
 		{os.ExpandEnv("$GOPATH/bin/dgraph"), "live",
 			"--schema", testDataDir + "/family.schema", "--files", testDataDir + "/family.json",
-			"--alpha", alphaService, "--zero", zeroService},
+			"--alpha", alphaService, "--zero", zeroService, "-u", "groot", "-p", "password"},
 	}
 	err := testutil.Pipeline(pipeline)
 	require.NoError(t, err, "live loading JSON file exited with error")
@@ -136,7 +136,7 @@ func TestLiveLoadJsonUidDiscard(t *testing.T) {
 	pipeline := [][]string{
 		{os.ExpandEnv("$GOPATH/bin/dgraph"), "live", "--new_uids",
 			"--schema", testDataDir + "/family.schema", "--files", testDataDir + "/family.json",
-			"--alpha", alphaService, "--zero", zeroService},
+			"--alpha", alphaService, "--zero", zeroService, "-u", "groot", "-p", "password"},
 	}
 	err := testutil.Pipeline(pipeline)
 	require.NoError(t, err, "live loading JSON file exited with error")
@@ -150,7 +150,7 @@ func TestLiveLoadRdfUidKeep(t *testing.T) {
 	pipeline := [][]string{
 		{os.ExpandEnv("$GOPATH/bin/dgraph"), "live",
 			"--schema", testDataDir + "/family.schema", "--files", testDataDir + "/family.rdf",
-			"--alpha", alphaService, "--zero", zeroService},
+			"--alpha", alphaService, "--zero", zeroService, "-u", "groot", "-p", "password"},
 	}
 	err := testutil.Pipeline(pipeline)
 	require.NoError(t, err, "live loading JSON file exited with error")
@@ -164,7 +164,7 @@ func TestLiveLoadRdfUidDiscard(t *testing.T) {
 	pipeline := [][]string{
 		{os.ExpandEnv("$GOPATH/bin/dgraph"), "live", "--new_uids",
 			"--schema", testDataDir + "/family.schema", "--files", testDataDir + "/family.rdf",
-			"--alpha", alphaService, "--zero", zeroService},
+			"--alpha", alphaService, "--zero", zeroService, "-u", "groot", "-p", "password"},
 	}
 	err := testutil.Pipeline(pipeline)
 	require.NoError(t, err, "live loading JSON file exited with error")

edgraph/access_ee.go

@@ -70,6 +70,7 @@ func (s *Server) Login(ctx context.Context,
 		glog.Errorf(errMsg)
 		return nil, errors.Errorf(errMsg)
 	}
+	glog.Infof("%s logged in successfully", user.UserID)
 
 	resp := &api.Response{}
 	accessJwt, err := getAccessJwt(user.UserID, user.Groups)

edgraph/acl_cache.go

@@ -86,24 +86,17 @@ func (cache *aclCache) authorizePredicate(groups []string, predicate string,
 	predPerms := aclCachePtr.predPerms
 	aclCachePtr.RUnlock()
 
-	var singlePredMatch bool
 	if groupPerms, found := predPerms[predicate]; found {
-		singlePredMatch = true
 		if hasRequiredAccess(groupPerms, groups, operation) {
 			return nil
 		}
 	}
 
-	if singlePredMatch {
-		// there is an ACL rule defined that can match the predicate
-		// and the operation has not been allowed
-		return errors.Errorf("unauthorized to do %s on predicate %s",
-			operation.Name, predicate)
-	}
-
 	// no rule has been defined that can match the predicate
-	// by default we follow the fail open approach and allow the operation
-	return nil
+	// by default we block operation
+	return errors.Errorf("unauthorized to do %s on predicate %s",
+		operation.Name, predicate)
+
 }
 
 // hasRequiredAccess checks if any group in the passed in groups is allowed to perform the operation

edgraph/acl_cache_test.go

@@ -28,8 +28,8 @@ func TestAclCache(t *testing.T) {
 	var emptyGroups []string
 	group := "dev"
 	predicate := "friend"
-	require.NoError(t, aclCachePtr.authorizePredicate(emptyGroups, predicate, acl.Read),
-		"the anonymous user should have access when the acl cache is empty")
+	require.Error(t, aclCachePtr.authorizePredicate(emptyGroups, predicate, acl.Read),
+		"the anonymous user should not have access when the acl cache is empty")
 
 	acls := []acl.Acl{
 		{
@@ -54,6 +54,6 @@ func TestAclCache(t *testing.T) {
 	// update the cache with empty acl list in order to clear the cache
 	aclCachePtr.update([]acl.Group{})
 	// the anonymous user should have access again
-	require.NoError(t, aclCachePtr.authorizePredicate(emptyGroups, predicate, acl.Read),
-		"the anonymous user should have access when the acl cache is empty")
+	require.Error(t, aclCachePtr.authorizePredicate(emptyGroups, predicate, acl.Read),
+		"the anonymous user should not have access when the acl cache is empty")
 }

ee/acl/acl_curl_test.go

@@ -44,14 +44,15 @@ func TestCurlAuthorization(t *testing.T) {
 	})
 	require.NoError(t, err, "login failed")
 
-	// test fail open with the accessJwt
+	// No ACL rules are specified, so everything should fail.
 	queryArgs := func(jwt string) []string {
 		return []string{"-H", fmt.Sprintf("X-Dgraph-AccessToken:%s", jwt),
 			"-H", "Content-Type: application/graphql+-",
 			"-d", query, curlQueryEndpoint}
 	}
 	testutil.VerifyCurlCmd(t, queryArgs(accessJwt), &testutil.CurlFailureConfig{
-		ShouldFail: false,
+		ShouldFail:   true,
+		DgraphErrMsg: "PermissionDenied",
 	})
 
 	mutateArgs := func(jwt string) []string {
@@ -64,15 +65,17 @@ func TestCurlAuthorization(t *testing.T) {
 	}
 
 	testutil.VerifyCurlCmd(t, mutateArgs(accessJwt), &testutil.CurlFailureConfig{
-		ShouldFail: false,
+		ShouldFail:   true,
+		DgraphErrMsg: "PermissionDenied",
 	})
 
 	alterArgs := func(jwt string) []string {
 		return []string{"-H", fmt.Sprintf("X-Dgraph-AccessToken:%s", jwt),
 			"-d", fmt.Sprintf(`%s: int .`, predicateToAlter), curlAlterEndpoint}
 	}
 	testutil.VerifyCurlCmd(t, alterArgs(accessJwt), &testutil.CurlFailureConfig{
-		ShouldFail: false,
+		ShouldFail:   true,
+		DgraphErrMsg: "PermissionDenied",
 	})
 
 	// sleep long enough (longer than 10s, the access JWT TTL defined in the docker-compose.yml
@@ -98,10 +101,6 @@ func TestCurlAuthorization(t *testing.T) {
 		RefreshJwt: refreshJwt,
 	})
 	require.NoError(t, err, fmt.Sprintf("login through refresh token failed: %v", err))
-	// verify that the query works again with the new access jwt
-	testutil.VerifyCurlCmd(t, queryArgs(accessJwt), &testutil.CurlFailureConfig{
-		ShouldFail: false,
-	})
 
 	createGroupAndAcls(t, unusedGroup, false)
 	// wait for 6 seconds to ensure the new acl have reached all acl caches

ee/acl/acl_test.go

@@ -135,11 +135,11 @@ func testAuthorization(t *testing.T, dg *dgo.Dgraph) {
 		t.Fatalf("unable to login using the account %v", userid)
 	}
 
-	// initially the query, mutate and alter operations should all succeed
-	// when there are no rules defined on the predicates (the fail open approach)
-	queryPredicateWithUserAccount(t, dg, false)
-	mutatePredicateWithUserAccount(t, dg, false)
-	alterPredicateWithUserAccount(t, dg, false)
+	// initially the query, mutate and alter operations should all fail
+	// when there are no rules defined on the predicates
+	queryPredicateWithUserAccount(t, dg, true)
+	mutatePredicateWithUserAccount(t, dg, true)
+	alterPredicateWithUserAccount(t, dg, true)
 	createGroupAndAcls(t, unusedGroup, false)
 	// wait for 6 seconds to ensure the new acl have reached all acl caches
 	glog.Infof("Sleeping for 6 seconds for acl caches to be refreshed")
@@ -377,11 +377,13 @@ func TestPredicatePermission(t *testing.T) {
 	err = dg.Login(ctx, userid, userpassword)
 	require.NoError(t, err, "Logging in with the current password should have succeeded")
 
-	// The operations should be allowed when no rule is defined (the fail open approach).
-	queryPredicateWithUserAccount(t, dg, false)
+	// Schema query is allowed to all logged in users.
 	querySchemaWithUserAccount(t, dg, false)
-	mutatePredicateWithUserAccount(t, dg, false)
-	alterPredicateWithUserAccount(t, dg, false)
+
+	// The operations should be blocked when no rule is defined.
+	queryPredicateWithUserAccount(t, dg, true)
+	mutatePredicateWithUserAccount(t, dg, true)
+	alterPredicateWithUserAccount(t, dg, true)
 	createGroupAndAcls(t, unusedGroup, false)
 
 	// Wait for 6 seconds to ensure the new acl have reached all acl caches.
@@ -405,11 +407,11 @@ func TestAccessWithoutLoggingIn(t *testing.T) {
 	require.NoError(t, err)
 
 	// Without logging in, the anonymous user should be evaluated as if the user does not
-	// belong to any group, and access should be granted if there is no ACL rule defined
-	// for a predicate (fail open).
-	queryPredicateWithUserAccount(t, dg, false)
-	mutatePredicateWithUserAccount(t, dg, false)
-	alterPredicateWithUserAccount(t, dg, false)
+	// belong to any group, and access should not be granted if there is no ACL rule defined
+	// for a predicate.
+	queryPredicateWithUserAccount(t, dg, true)
+	mutatePredicateWithUserAccount(t, dg, true)
+	alterPredicateWithUserAccount(t, dg, true)
 
 	// Schema queries should fail if the user has not logged in.
 	querySchemaWithUserAccount(t, dg, true)

systest/bulk_live_fixture_test.go

@@ -161,6 +161,7 @@ func (s *suite) setup(schemaFile, rdfFile string) {
 			"--schema", schemaFile,
 			"--alpha", testutil.SockAddr,
 			"--zero", testutil.SockAddrZero,
+			"-u", "groot", "-p", "password",
 		)
 		liveCmd.Dir = liveDir
 		if out, err := liveCmd.Output(); err != nil {