From 4fc328d7c4fcd3264f7bcf2330a01cd6882cbd72 Mon Sep 17 00:00:00 2001 From: Abhimanyu Singh Gaur <12651351+abhimanyusinghgaur@users.noreply.github.com> Date: Thu, 8 Oct 2020 14:55:24 +0530 Subject: [PATCH] fix(Auth): fix Poor-man's auth for admin operations (#6660) Fixes DGRAPH-2419 Fixes [Discuss Issue](https://discuss.dgraph.io/t/acl-login-will-fail-if-auth-token-enabled-in-v20-07-0/10044) This PR fixes Poor-man's auth for following endpoints: * `/login` * `/admin` --- dgraph/cmd/alpha/admin.go | 5 + dgraph/cmd/alpha/http.go | 6 +- dgraph/cmd/alpha/login_ee.go | 2 + dgraph/cmd/alpha/run.go | 2 +- edgraph/server.go | 2 +- graphql/e2e/admin_auth/admin_auth_test.go | 183 ++++++++++++++++++++++ graphql/e2e/admin_auth/docker-compose.yml | 73 +++++++++ graphql/e2e/common/common.go | 30 ++-- graphql/e2e/common/query.go | 2 +- graphql/web/http.go | 5 +- x/x.go | 15 ++ 11 files changed, 301 insertions(+), 24 deletions(-) create mode 100644 graphql/e2e/admin_auth/admin_auth_test.go create mode 100644 graphql/e2e/admin_auth/docker-compose.yml diff --git a/dgraph/cmd/alpha/admin.go b/dgraph/cmd/alpha/admin.go index 1a35e7756e0..d9c41213e5a 100644 --- a/dgraph/cmd/alpha/admin.go +++ b/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 } diff --git a/dgraph/cmd/alpha/http.go b/dgraph/cmd/alpha/http.go index 99ecf16d9a1..d7f5c5bcddf 100644 --- a/dgraph/cmd/alpha/http.go +++ b/dgraph/cmd/alpha/http.go @@ -589,10 +589,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 { diff --git a/dgraph/cmd/alpha/login_ee.go b/dgraph/cmd/alpha/login_ee.go index ee205fd82cf..e126e630767 100644 --- a/dgraph/cmd/alpha/login_ee.go +++ b/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{} diff --git a/dgraph/cmd/alpha/run.go b/dgraph/cmd/alpha/run.go index ea98d97c07a..ba6838ab0f3 100644 --- a/dgraph/cmd/alpha/run.go +++ b/dgraph/cmd/alpha/run.go @@ -141,7 +141,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.") diff --git a/edgraph/server.go b/edgraph/server.go index 2e4cbfea668..ca33154eb76 100644 --- a/edgraph/server.go +++ b/edgraph/server.go @@ -1312,7 +1312,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) diff --git a/graphql/e2e/admin_auth/admin_auth_test.go b/graphql/e2e/admin_auth/admin_auth_test.go new file mode 100644 index 00000000000..769052a8fdf --- /dev/null +++ b/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{}, + } +} diff --git a/graphql/e2e/admin_auth/docker-compose.yml b/graphql/e2e/admin_auth/docker-compose.yml new file mode 100644 index 00000000000..6ddc8597926 --- /dev/null +++ b/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 diff --git a/graphql/e2e/common/common.go b/graphql/e2e/common/common.go index 453fe620adc..9fea8068086 100644 --- a/graphql/e2e/common/common.go +++ b/graphql/e2e/common/common.go @@ -445,12 +445,12 @@ func gzipCompressionHeader(t *testing.T) { }`, } - req, err := queryCountry.createGQLPost(GraphqlURL) + req, err := queryCountry.CreateGQLPost(GraphqlURL) require.NoError(t, err) req.Header.Set("Content-Encoding", "gzip") - resData, err := runGQLRequest(req) + resData, err := RunGQLRequest(req) require.NoError(t, err) var result *GraphQLResponse @@ -472,11 +472,11 @@ func gzipCompressionNoHeader(t *testing.T) { gzipEncoding: true, } - req, err := queryCountry.createGQLPost(GraphqlURL) + req, err := queryCountry.CreateGQLPost(GraphqlURL) require.NoError(t, err) req.Header.Del("Content-Encoding") - resData, err := runGQLRequest(req) + resData, err := RunGQLRequest(req) require.NoError(t, err) var result *GraphQLResponse @@ -515,7 +515,7 @@ func (params *GraphQLParams) Execute(t require.TestingT, req *http.Request) *Gra for h := range params.Headers { req.Header.Set(h, params.Headers.Get(h)) } - res, err := runGQLRequest(req) + res, err := RunGQLRequest(req) require.NoError(t, err) var result *GraphQLResponse @@ -533,7 +533,7 @@ func (params *GraphQLParams) Execute(t require.TestingT, req *http.Request) *Gra // ExecuteAsPost builds a HTTP POST request from the GraphQL input structure // and executes the request to url. func (params *GraphQLParams) ExecuteAsPost(t require.TestingT, url string) *GraphQLResponse { - req, err := params.createGQLPost(url) + req, err := params.CreateGQLPost(url) require.NoError(t, err) return params.Execute(t, req) @@ -614,7 +614,7 @@ func (params *GraphQLParams) buildPostRequest(url string, body []byte, contentTy return req, nil } -func (params *GraphQLParams) createGQLPost(url string) (*http.Request, error) { +func (params *GraphQLParams) CreateGQLPost(url string) (*http.Request, error) { body, err := json.Marshal(params) if err != nil { return nil, err @@ -627,8 +627,8 @@ func (params *GraphQLParams) createApplicationGQLPost(url string) (*http.Request return params.buildPostRequest(url, []byte(params.Query), "application/graphql") } -// runGQLRequest runs a HTTP GraphQL request and returns the data or any errors. -func runGQLRequest(req *http.Request) ([]byte, error) { +// RunGQLRequest runs a HTTP GraphQL request and returns the data or any errors. +func RunGQLRequest(req *http.Request) ([]byte, error) { client := &http.Client{Timeout: 50 * time.Second} resp, err := client.Do(req) if err != nil { @@ -714,7 +714,7 @@ func allCountriesAdded() ([]*country, error) { } req.Header.Set("Content-Type", "application/json") - resp, err := runGQLRequest(req) + resp, err := RunGQLRequest(req) if err != nil { return nil, errors.Wrap(err, "error running GraphQL query") } @@ -757,12 +757,12 @@ func hasCurrentGraphQLSchema(url string) (bool, error) { schemaQry := &GraphQLParams{ Query: `query { getGQLSchema { schema } }`, } - req, err := schemaQry.createGQLPost(url) + req, err := schemaQry.CreateGQLPost(url) if err != nil { return false, errors.Wrap(err, "while creating gql post") } - res, err := runGQLRequest(req) + res, err := RunGQLRequest(req) if err != nil { return false, errors.Wrap(err, "error running GraphQL query") } @@ -806,12 +806,12 @@ func addSchema(url, schema string) error { }`, Variables: map[string]interface{}{"sch": schema}, } - req, err := add.createGQLPost(url) + req, err := add.CreateGQLPost(url) if err != nil { return errors.Wrap(err, "error creating GraphQL query") } - resp, err := runGQLRequest(req) + resp, err := RunGQLRequest(req) if err != nil { return errors.Wrap(err, "error running GraphQL query") } @@ -849,7 +849,7 @@ func addSchemaThroughAdminSchemaEndpt(url, schema string) error { return errors.Wrap(err, "error running GraphQL query") } - resp, err := runGQLRequest(req) + resp, err := RunGQLRequest(req) if err != nil { return errors.Wrap(err, "error running GraphQL query") } diff --git a/graphql/e2e/common/query.go b/graphql/e2e/common/query.go index b2f1f489c63..527c797e322 100644 --- a/graphql/e2e/common/query.go +++ b/graphql/e2e/common/query.go @@ -72,7 +72,7 @@ func touchedUidsHeader(t *testing.T) { } }`, } - req, err := query.createGQLPost(GraphqlURL) + req, err := query.CreateGQLPost(GraphqlURL) require.NoError(t, err) client := http.Client{Timeout: 10 * time.Second} diff --git a/graphql/web/http.go b/graphql/web/http.go index 4f2433adebe..522546f0992 100644 --- a/graphql/web/http.go +++ b/graphql/web/http.go @@ -200,11 +200,12 @@ func (gh *graphqlHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { x.Panic(errors.New("graphqlHandler not initialised")) } + // Pass in GraphQL @auth information ctx = authorization.AttachAuthorizationJwt(ctx, r) + // Pass in PoorMan's auth, ACL and IP information if present. ctx = x.AttachAccessJwt(ctx, r) - // Add remote addr as peer info so that the remote address can be logged - // inside Server.Login ctx = x.AttachRemoteIP(ctx, r) + ctx = x.AttachAuthToken(ctx, r) var res *schema.Response gqlReq, err := getRequest(ctx, r) diff --git a/x/x.go b/x/x.go index 1185ce50dd5..ee61f804464 100644 --- a/x/x.go +++ b/x/x.go @@ -320,6 +320,7 @@ func (gqlErr *GqlError) WithPath(path []interface{}) *GqlError { // SetStatus sets the error code, message and the newly assigned uids // in the http response. func SetStatus(w http.ResponseWriter, code, msg string) { + w.Header().Set("Content-Type", "application/json") var qr queryRes ext := make(map[string]interface{}) ext["code"] = code @@ -411,6 +412,20 @@ func ParseRequest(w http.ResponseWriter, r *http.Request, data interface{}) bool return true } +// AttachAuthToken adds any incoming PoorMan's auth header data into the grpc context metadata +func AttachAuthToken(ctx context.Context, r *http.Request) context.Context { + if authToken := r.Header.Get("X-Dgraph-AuthToken"); authToken != "" { + md, ok := metadata.FromIncomingContext(ctx) + if !ok { + md = metadata.New(nil) + } + + md.Append("auth-token", authToken) + ctx = metadata.NewIncomingContext(ctx, md) + } + return ctx +} + // AttachAccessJwt adds any incoming JWT header data into the grpc context metadata func AttachAccessJwt(ctx context.Context, r *http.Request) context.Context { if accessJwt := r.Header.Get("X-Dgraph-AccessToken"); accessJwt != "" {