From 7834db23478815da4894543b05d2e9527cfee8d7 Mon Sep 17 00:00:00 2001 From: Samir Patel <48686912+54mir@users.noreply.github.com> Date: Mon, 3 Jan 2022 16:24:14 -0600 Subject: [PATCH] change write call detection --- http/handler.go | 43 +++++++++++++++++------------------ http/handler_internal_test.go | 32 +++++++++++++++++++++++--- 2 files changed, 50 insertions(+), 25 deletions(-) diff --git a/http/handler.go b/http/handler.go index 33375ad5a..43cd9dda5 100644 --- a/http/handler.go +++ b/http/handler.go @@ -284,8 +284,13 @@ const ( contextKeyQueryRequest contextKeyQuery = iota contextKeyQueryError contextKeyGroupMembership + contextKeyPermission ) +func GetContextKeyPermission() contextKeyQuery { + return contextKeyPermission +} + // addQueryContext puts the results of handler.readQueryRequest into the Context for use by // both other middleware and any handlers. func (h *Handler) addQueryContext(next http.Handler) http.Handler { @@ -550,23 +555,15 @@ func (h *Handler) chkAuthN(handler http.HandlerFunc) http.HandlerFunc { func (h *Handler) chkAuthZ(handler http.HandlerFunc, perm authz.Permission) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { if h.auth != nil { - fmt.Println("a") groups, err := h.auth.Authenticate(w, r) if err != nil { http.Error(w, errors.Wrap(err, "authenticating").Error(), http.StatusBadRequest) return } - indexName, ok := mux.Vars(r)["index"] - if !ok { - indexName = "" - } - if h.permissions == nil { panic("authentication is turned on without authorization permissions set") } - p, err := h.permissions.GetPermissions(groups, indexName) - // err is being checked later, after logging uinfo := h.auth.GetUserInfo(w, r) @@ -577,29 +574,32 @@ func (h *Handler) chkAuthZ(handler http.HandlerFunc, perm authz.Permission) http if req, ok := queryRequest.(*pilosa.QueryRequest); ok { queryString = req.Query } - writeWords := []string{"store", "set", "clear", "clearrow"} - q := strings.ToLower(queryString) - for _, w := range writeWords { - if strings.Contains(q, w) { - perm = authz.Write - } + + q, _ := pql.ParseString(fmt.Sprintf(queryString)) + if q.WriteCallN() > 0 { + perm = authz.Write } queryString = strings.Replace(queryString, "\n", "", -1) if r.Method == "POST" { - h.querylogger.Infof("User ID: %s, User Name: %s, Endpoint: %s, Index: %s, Query: %s, Err: %v", uinfo.UserID, uinfo.UserName, r.URL.Path, indexName, queryString, err) - } - if err != nil || !p.Satisfies(perm) { - w.Header().Add("Content-Type", "text/plain") - w.WriteHeader(http.StatusForbidden) - return + h.querylogger.Infof("User ID: %s, User Name: %s, Endpoint: %s, Index: %s, Query: %s, Err: %v", uinfo.UserID, uinfo.UserName, r.URL.Path, "indexName", queryString, err) } ctx := context.WithValue(r.Context(), contextKeyGroupMembership, groups) + indexName, ok := mux.Vars(r)["index"] + if ok { + p, err := h.permissions.GetPermissions(groups, indexName) + ctx = context.WithValue(r.Context(), contextKeyPermission, p) + if err != nil || !p.Satisfies(perm) { + w.Header().Add("Content-Type", "text/plain") + w.WriteHeader(http.StatusForbidden) + return + } + } + handler.ServeHTTP(w, r.WithContext(ctx)) } else { - fmt.Println("z") handler.ServeHTTP(w, r) } @@ -960,7 +960,6 @@ var DoPerQueryProfiling = false // handlePostQuery handles /query requests. func (h *Handler) handlePostQuery(w http.ResponseWriter, r *http.Request) { // Read previouly parsed request from context - fmt.Println("hi") qreq := r.Context().Value(contextKeyQueryRequest) qerr := r.Context().Value(contextKeyQueryError) req, ok := qreq.(*pilosa.QueryRequest) diff --git a/http/handler_internal_test.go b/http/handler_internal_test.go index 2bcc02f97..6dd92a955 100644 --- a/http/handler_internal_test.go +++ b/http/handler_internal_test.go @@ -5,7 +5,6 @@ import ( "bytes" "encoding/hex" "encoding/json" - "fmt" "io/ioutil" gohttp "net/http" "net/http/httptest" @@ -19,6 +18,7 @@ import ( "github.com/gorilla/securecookie" pilosa "github.com/molecula/featurebase/v2" "github.com/molecula/featurebase/v2/authn" + "github.com/stretchr/testify/assert" "github.com/molecula/featurebase/v2/authz" "github.com/molecula/featurebase/v2/logger" @@ -595,6 +595,22 @@ admin: "ac97c9e2-346b-42a2-b6da-18bcb61a32fe"` path: "/index/{index}/query", kind: "middleware", cookie: expiredCookie, + handler: func(w gohttp.ResponseWriter, r *gohttp.Request) { + f := h.chkAuthN(h.handlePostQuery) + f(w, r) + }, + fn: func(w *httptest.ResponseRecorder, data []byte) { + if w.Result().StatusCode != 307 { + t.Errorf("expected http code 307, got: %+v", w.Result().StatusCode) + } + + }, + }, + { + name: "MW-ExpiredAuth2", + path: "/index/{index}/query", + kind: "middleware", + cookie: expiredCookie, handler: func(w gohttp.ResponseWriter, r *gohttp.Request) { f := h.chkAuthZ(h.handlePostQuery, authz.Admin) f(w, r) @@ -606,6 +622,18 @@ admin: "ac97c9e2-346b-42a2-b6da-18bcb61a32fe"` }, }, + { + name: "MW-NoPermissions", + path: "/index/{index}/query", + kind: "middleware", + cookie: validCookie, + handler: func(w gohttp.ResponseWriter, r *gohttp.Request) { + h := h + f := h.chkAuthZ(h.handlePostQuery, authz.Write) + assert.Panics(t, func() { f(w, r) }, "expected panic") + }, + fn: func(w *httptest.ResponseRecorder, data []byte) {}, + }, { name: "MW-NoIndexNoAdmin", path: "/index/{index}/query", @@ -619,7 +647,6 @@ admin: "ac97c9e2-346b-42a2-b6da-18bcb61a32fe"` t.Errorf("Error: %s", err) } h.permissions = &p - fmt.Printf("%+v\n", h) f := h.chkAuthZ(h.handlePostQuery, authz.Write) f(w, r) }, @@ -674,7 +701,6 @@ admin: "ac97c9e2-346b-42a2-b6da-18bcb61a32fe"` } test.handler(w, r) - fmt.Println("hey") data, err := readResponse(w) if err != nil { t.Errorf("expected no errors reading response, got: %+v", err)