cherry-pick v20.07: fix(Auth): fix Poor-man's auth for admin operations (#6660)

dgraph/cmd/alpha/admin.go

@@ -45,6 +45,10 @@ func hasPoormansAuth(r *http.Request) bool {
 func allowedMethodsHandler(allowedMethods allowedMethods, next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if _, ok := allowedMethods[r.Method]; !ok {
+			x.AddCorsHeaders(w)
+			if r.Method == http.MethodOptions {
+				return
+			}
 			x.SetStatus(w, x.ErrorInvalidMethod, "Invalid method")
 			w.WriteHeader(http.StatusMethodNotAllowed)
 			return
@@ -59,6 +63,7 @@ func allowedMethodsHandler(allowedMethods allowedMethods, next http.Handler) htt
 func adminAuthHandler(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if !hasPoormansAuth(r) {
+			x.AddCorsHeaders(w)
 			x.SetStatus(w, x.ErrorUnauthorized, "Invalid X-Dgraph-AuthToken")
 			return
 		}

dgraph/cmd/alpha/http.go

@@ -563,10 +563,8 @@ func alterHandler(w http.ResponseWriter, r *http.Request) {
 		glog.Infof("The alter request is forwarded by %s\n", fwd)
 	}
 
-	md := metadata.New(nil)
-	// Pass in an auth token, if present.
-	md.Append("auth-token", r.Header.Get("X-Dgraph-AuthToken"))
-	ctx := metadata.NewIncomingContext(context.Background(), md)
+	// Pass in PoorMan's auth, ACL and IP information if present.
+	ctx := x.AttachAuthToken(context.Background(), r)
 	ctx = x.AttachAccessJwt(ctx, r)
 	ctx = x.AttachRemoteIP(ctx, r)
 	if _, err := (&edgraph.Server{}).Alter(ctx, op); err != nil {

dgraph/cmd/alpha/login_ee.go

@@ -34,7 +34,9 @@ func loginHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Pass in PoorMan's auth, IP information if present.
 	ctx := x.AttachRemoteIP(context.Background(), r)
+	ctx = x.AttachAuthToken(ctx, r)
 
 	body := readRequest(w, r)
 	loginReq := api.LoginRequest{}

dgraph/cmd/alpha/run.go

@@ -161,7 +161,7 @@ they form a Raft group and provide synchronous replication.
 		"Commits to disk will give up after these number of retries to prevent locking the worker"+
 			" in a failed state. Use -1 to retry infinitely.")
 	flag.String("auth_token", "",
-		"If set, all Alter requests to Dgraph would need to have this token."+
+		"If set, all Admin requests to Dgraph would need to have this token."+
 			" The token can be passed as follows: For HTTP requests, in X-Dgraph-AuthToken header."+
 			" For Grpc, in auth-token key in the context.")
 	flag.Bool("enable_sentry", true, "Turn on/off sending events to Sentry. (default on)")

edgraph/server.go

@@ -1262,7 +1262,7 @@ func isMutationAllowed(ctx context.Context) bool {
 	return true
 }
 
-var errNoAuth = errors.Errorf("No Auth Token found. Token needed for Alter operations.")
+var errNoAuth = errors.Errorf("No Auth Token found. Token needed for Admin operations.")
 
 func hasAdminAuth(ctx context.Context, tag string) (net.Addr, error) {
 	ipAddr, err := x.HasWhitelistedIP(ctx)

graphql/e2e/admin_auth/admin_auth_test.go

@@ -0,0 +1,183 @@
+/*
+ * Copyright 2020 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 admin_auth
+
+import (
+	"encoding/json"
+	"net/http"
+	"testing"
+
+	"github.com/dgraph-io/dgraph/x"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/dgraph-io/dgraph/graphql/e2e/common"
+)
+
+const (
+	poorManAdminURL        = "http://localhost:8180/admin"
+	poorManWithAclAdminURL = "http://localhost:8280/admin"
+
+	authTokenHeader = "X-Dgraph-AuthToken"
+	authToken       = "itIsSecret"
+	wrongAuthToken  = "wrongToken"
+
+	accessJwtHeader = "X-Dgraph-AccessToken"
+)
+
+func TestLoginWithPoorManAuth(t *testing.T) {
+	// without X-Dgraph-AuthToken should give error
+	params := getGrootLoginParams()
+	assertAuthTokenError(t, poorManWithAclAdminURL, params)
+
+	// setting a wrong value for the token should still give error
+	params.Headers.Set(authTokenHeader, wrongAuthToken)
+	assertAuthTokenError(t, poorManWithAclAdminURL, params)
+
+	// setting correct value for the token should not give any GraphQL error
+	params.Headers.Set(authTokenHeader, authToken)
+	common.RequireNoGQLErrors(t, params.ExecuteAsPost(t, poorManWithAclAdminURL))
+}
+
+func TestAdminOnlyPoorManAuth(t *testing.T) {
+	// without X-Dgraph-AuthToken should give error
+	params := getUpdateGqlSchemaParams()
+	assertAuthTokenError(t, poorManAdminURL, params)
+
+	// setting a wrong value for the token should still give error
+	params.Headers.Set(authTokenHeader, wrongAuthToken)
+	assertAuthTokenError(t, poorManAdminURL, params)
+
+	// setting correct value for the token should not give any GraphQL error
+	params.Headers.Set(authTokenHeader, authToken)
+	common.RequireNoGQLErrors(t, params.ExecuteAsPost(t, poorManAdminURL))
+}
+
+func TestAdminPoorManWithAcl(t *testing.T) {
+	// without auth token and access JWT headers, should give auth token related error
+	params := getUpdateGqlSchemaParams()
+	assertAuthTokenError(t, poorManWithAclAdminURL, params)
+
+	// setting a wrong value for the auth token should still give auth token related error
+	params.Headers.Set(authTokenHeader, wrongAuthToken)
+	assertAuthTokenError(t, poorManWithAclAdminURL, params)
+
+	// setting correct value for the auth token should now give ACL related GraphQL error
+	params.Headers.Set(authTokenHeader, authToken)
+	assertMissingAclError(t, params)
+
+	// setting wrong value for the access JWT should still give ACL related GraphQL error
+	params.Headers.Set(accessJwtHeader, wrongAuthToken)
+	assertBadAclError(t, params)
+
+	// setting correct value for both tokens should not give errors
+	accessJwt, _ := grootLogin(t)
+	params.Headers.Set(accessJwtHeader, accessJwt)
+	common.RequireNoGQLErrors(t, params.ExecuteAsPost(t, poorManWithAclAdminURL))
+}
+
+func assertAuthTokenError(t *testing.T, url string, params *common.GraphQLParams) {
+	req, err := params.CreateGQLPost(url)
+	require.NoError(t, err)
+
+	resp, err := common.RunGQLRequest(req)
+	require.NoError(t, err)
+	require.JSONEq(t, `{
+		"errors":[{
+			"message":"Invalid X-Dgraph-AuthToken",
+			"extensions":{"code":"ErrorUnauthorized"}
+		}]
+	}`, string(resp))
+}
+
+func assertMissingAclError(t *testing.T, params *common.GraphQLParams) {
+	resp := params.ExecuteAsPost(t, poorManWithAclAdminURL)
+	require.Equal(t, x.GqlErrorList{{
+		Message: "resolving updateGQLSchema failed because rpc error: code = PermissionDenied desc = no accessJwt available",
+		Locations: []x.Location{{
+			Line:   2,
+			Column: 4,
+		}},
+	}}, resp.Errors)
+}
+
+func assertBadAclError(t *testing.T, params *common.GraphQLParams) {
+	resp := params.ExecuteAsPost(t, poorManWithAclAdminURL)
+	require.Equal(t, x.GqlErrorList{{
+		Message: "resolving updateGQLSchema failed because rpc error: code = Unauthenticated desc = unable to parse jwt token:token contains an invalid number of segments",
+		Locations: []x.Location{{
+			Line:   2,
+			Column: 4,
+		}},
+	}}, resp.Errors)
+}
+
+func grootLogin(t *testing.T) (string, string) {
+	loginParams := getGrootLoginParams()
+	loginParams.Headers.Set(authTokenHeader, authToken)
+	resp := loginParams.ExecuteAsPost(t, poorManWithAclAdminURL)
+	common.RequireNoGQLErrors(t, resp)
+
+	var loginResp struct {
+		Login struct {
+			Response struct {
+				AccessJWT  string
+				RefreshJWT string
+			}
+		}
+	}
+	require.NoError(t, json.Unmarshal(resp.Data, &loginResp))
+
+	return loginResp.Login.Response.AccessJWT, loginResp.Login.Response.RefreshJWT
+}
+
+func getGrootLoginParams() *common.GraphQLParams {
+	return &common.GraphQLParams{
+		Query: `mutation login($userId: String, $password: String, $refreshToken: String) {
+			login(userId: $userId, password: $password, refreshToken: $refreshToken) {
+				response {
+					accessJWT
+					refreshJWT
+				}
+			}
+		}`,
+		Variables: map[string]interface{}{
+			"userId":       x.GrootId,
+			"password":     "password",
+			"refreshToken": "",
+		},
+		Headers: http.Header{},
+	}
+}
+
+func getUpdateGqlSchemaParams() *common.GraphQLParams {
+	schema := `type Person {
+		id: ID!
+		name: String!
+	}`
+	return &common.GraphQLParams{
+		Query: `mutation updateGQLSchema($sch: String!) {
+			updateGQLSchema(input: { set: { schema: $sch }}) {
+				gqlSchema {
+					schema
+				}
+			}
+		}`,
+		Variables: map[string]interface{}{"sch": schema},
+		Headers:   http.Header{},
+	}
+}

graphql/e2e/admin_auth/docker-compose.yml

@@ -0,0 +1,73 @@
+version: "3.5"
+services:
+  zero:
+    image: dgraph/dgraph:latest
+    container_name: zero1
+    working_dir: /data/zero1
+    ports:
+      - 5180:5180
+      - 6180:6180
+    labels:
+      cluster: test
+      service: zero1
+    volumes:
+      - type: bind
+        source: $GOPATH/bin
+        target: /gobin
+        read_only: true
+    command: /gobin/dgraph zero -o 100 --logtostderr -v=2 --bindall --expose_trace --profile_mode block --block_rate 10 --my=zero1:5180
+
+  alpha:
+    image: dgraph/dgraph:latest
+    container_name: alpha1
+    working_dir: /data/alpha1
+    volumes:
+      - type: bind
+        source: $GOPATH/bin
+        target: /gobin
+        read_only: true
+    ports:
+      - 8180:8180
+      - 9180:9180
+    labels:
+      cluster: test
+      service: alpha1
+    command: /gobin/dgraph alpha --zero=zero1:5180 -o 100 --expose_trace --trace 1.0 --profile_mode block --block_rate 10 --logtostderr -v=2 --whitelist 10.0.0.0/8,172.16.0.0/12,192.168.0.0/16 --my=alpha1:7180 --auth_token itIsSecret
+
+  zeroAdmin:
+    image: dgraph/dgraph:latest
+    container_name: zeroAdmin
+    working_dir: /data/zeroAdmin
+    ports:
+      - 5280:5280
+      - 6280:6280
+    labels:
+      cluster: admintest
+      service: zeroAdmin
+    volumes:
+      - type: bind
+        source: $GOPATH/bin
+        target: /gobin
+        read_only: true
+    command: /gobin/dgraph zero -o 200 --logtostderr -v=2 --bindall --expose_trace --profile_mode block --block_rate 10 --my=zeroAdmin:5280
+
+  alphaAdmin:
+    image: dgraph/dgraph:latest
+    container_name: alphaAdmin
+    working_dir: /data/alphaAdmin
+    volumes:
+      - type: bind
+        source: $GOPATH/bin
+        target: /gobin
+        read_only: true
+      - type: bind
+        source: ../../../ee/acl/hmac-secret
+        target: /dgraph-acl/hmac-secret
+        read_only: true
+    ports:
+      - 8280:8280
+      - 9280:9280
+    labels:
+      cluster: admintest
+      service: alphaAdmin
+    command: /gobin/dgraph alpha --zero=zeroAdmin:5280 -o 200 --expose_trace --trace 1.0 --profile_mode block --block_rate 10 --logtostderr -v=2 --whitelist 10.0.0.0/8,172.16.0.0/12,192.168.0.0/16 --my=alphaAdmin:7280 --acl_secret_file /dgraph-acl/hmac-secret --acl_access_ttl 3s --auth_token itIsSecret