Skip to content

Commit

Permalink
fix(Auth): fix Poor-man's auth for admin operations (#6660)
Browse files Browse the repository at this point in the history
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`
  • Loading branch information
abhimanyusinghgaur committed Oct 8, 2020
1 parent 8fd7279 commit 4fc328d
Show file tree
Hide file tree
Showing 11 changed files with 301 additions and 24 deletions.
5 changes: 5 additions & 0 deletions dgraph/cmd/alpha/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand Down
6 changes: 2 additions & 4 deletions dgraph/cmd/alpha/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions dgraph/cmd/alpha/login_ee.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down
2 changes: 1 addition & 1 deletion dgraph/cmd/alpha/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.")

Expand Down
2 changes: 1 addition & 1 deletion edgraph/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
183 changes: 183 additions & 0 deletions graphql/e2e/admin_auth/admin_auth_test.go
Original file line number Diff line number Diff line change
@@ -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{},
}
}
73 changes: 73 additions & 0 deletions graphql/e2e/admin_auth/docker-compose.yml
Original file line number Diff line number Diff line change
@@ -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
Loading

0 comments on commit 4fc328d

Please sign in to comment.