-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add authn for graphql and http admin endpoints #5162
Changes from 5 commits
91fc1ca
c78becc
563ecde
1dc12d1
7006028
603fd25
5629249
deeb2bb
08772df
4009e29
9ea03fd
262b944
9af6def
1b64682
46330f9
3497cbb
fa2b651
1a471ba
0b92b7f
076e68b
636999f
0d75284
75a4d8b
b6e6863
4f513fb
ed81244
1d80239
3677c15
aa4e3cc
8fe2324
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,54 +25,83 @@ import ( | |
"net/http" | ||
"strconv" | ||
|
||
"github.com/dgraph-io/dgraph/edgraph" | ||
"github.com/dgraph-io/dgraph/posting" | ||
"github.com/dgraph-io/dgraph/worker" | ||
"github.com/dgraph-io/dgraph/x" | ||
"github.com/golang/glog" | ||
) | ||
|
||
func checkIpIsWhitelisted(w http.ResponseWriter, r *http.Request) bool { | ||
ip, _, err := net.SplitHostPort(r.RemoteAddr) | ||
if err != nil || (!ipInIPWhitelistRanges(ip) && !net.ParseIP(ip).IsLoopback()) { | ||
x.SetStatus(w, x.ErrorUnauthorized, fmt.Sprintf("Request from IP: %v", ip)) | ||
return false | ||
} | ||
return true | ||
} | ||
|
||
func checkPoormansAcl(w http.ResponseWriter, r *http.Request) bool { | ||
if worker.Config.AuthToken != "" && worker.Config.AuthToken != r.Header.Get( | ||
"X-Dgraph-AuthToken") { | ||
x.SetStatus(w, x.ErrorUnauthorized, "Invalid X-Dgraph-AuthToken") | ||
return false | ||
} | ||
return true | ||
} | ||
|
||
// handlerInit does some standard checks. Returns false if something is wrong. | ||
func handlerInit(w http.ResponseWriter, r *http.Request, allowedMethods map[string]bool) bool { | ||
func handlerInit(w http.ResponseWriter, r *http.Request, allowedMethods map[string]bool, | ||
allowOnlyGuardians bool) bool { | ||
if _, ok := allowedMethods[r.Method]; !ok { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can't we move these allowedMethods type checks into some sort of router or something? I know we do use httprouter, why aren't 405s moved over there? |
||
x.SetStatus(w, x.ErrorInvalidMethod, "Invalid method") | ||
w.WriteHeader(http.StatusMethodNotAllowed) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't this a bit strange? Middlewares should either do the |
||
return false | ||
} | ||
|
||
ip, _, err := net.SplitHostPort(r.RemoteAddr) | ||
if err != nil || (!ipInIPWhitelistRanges(ip) && !net.ParseIP(ip).IsLoopback()) { | ||
x.SetStatus(w, x.ErrorUnauthorized, fmt.Sprintf("Request from IP: %v", ip)) | ||
if !checkIpIsWhitelisted(w, r) || !checkPoormansAcl(w, r) { | ||
return false | ||
} | ||
|
||
if allowOnlyGuardians { | ||
err := edgraph.AuthorizeGuardians(x.AttachAccessJwt(context.Background(), r)) | ||
if err != nil { | ||
x.SetStatus(w, x.ErrorUnauthorized, err.Error()) | ||
return false | ||
} | ||
} | ||
return true | ||
} | ||
|
||
func drainingHandler(w http.ResponseWriter, r *http.Request) { | ||
switch r.Method { | ||
case http.MethodPut, http.MethodPost: | ||
enableStr := r.URL.Query().Get("enable") | ||
if !handlerInit(w, r, map[string]bool{ | ||
http.MethodPut: true, | ||
http.MethodPost: true, | ||
}, true) { | ||
return | ||
} | ||
|
||
enable, err := strconv.ParseBool(enableStr) | ||
if err != nil { | ||
x.SetStatus(w, x.ErrorInvalidRequest, | ||
"Found invalid value for the enable parameter") | ||
return | ||
} | ||
enableStr := r.URL.Query().Get("enable") | ||
|
||
x.UpdateDrainingMode(enable) | ||
_, err = w.Write([]byte(fmt.Sprintf(`{"code": "Success",`+ | ||
`"message": "draining mode has been set to %v"}`, enable))) | ||
if err != nil { | ||
glog.Errorf("Failed to write response: %v", err) | ||
} | ||
default: | ||
w.WriteHeader(http.StatusMethodNotAllowed) | ||
enable, err := strconv.ParseBool(enableStr) | ||
if err != nil { | ||
x.SetStatus(w, x.ErrorInvalidRequest, | ||
"Found invalid value for the enable parameter") | ||
return | ||
} | ||
|
||
x.UpdateDrainingMode(enable) | ||
_, err = w.Write([]byte(fmt.Sprintf(`{"code": "Success",`+ | ||
`"message": "draining mode has been set to %v"}`, enable))) | ||
if err != nil { | ||
glog.Errorf("Failed to write response: %v", err) | ||
} | ||
} | ||
|
||
func shutDownHandler(w http.ResponseWriter, r *http.Request) { | ||
if !handlerInit(w, r, map[string]bool{ | ||
http.MethodGet: true, | ||
}) { | ||
}, true) { | ||
return | ||
} | ||
|
||
|
@@ -84,7 +113,7 @@ func shutDownHandler(w http.ResponseWriter, r *http.Request) { | |
func exportHandler(w http.ResponseWriter, r *http.Request) { | ||
if !handlerInit(w, r, map[string]bool{ | ||
http.MethodGet: true, | ||
}) { | ||
}, true) { | ||
return | ||
} | ||
if err := r.ParseForm(); err != nil { | ||
|
@@ -114,13 +143,18 @@ func exportHandler(w http.ResponseWriter, r *http.Request) { | |
} | ||
|
||
func memoryLimitHandler(w http.ResponseWriter, r *http.Request) { | ||
if !handlerInit(w, r, map[string]bool{ | ||
http.MethodGet: true, | ||
http.MethodPut: true, | ||
}, true) { | ||
return | ||
} | ||
|
||
switch r.Method { | ||
case http.MethodGet: | ||
memoryLimitGetHandler(w, r) | ||
case http.MethodPut: | ||
memoryLimitPutHandler(w, r) | ||
default: | ||
w.WriteHeader(http.StatusMethodNotAllowed) | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we be clubbing allowedMethods and allowOnlyGuardians into a type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, can we rename handlerInit to something like checkAuth?