diff --git a/docs/content/security.md b/docs/content/security.md index aa8b3c1fa6..8c0c7e4c90 100644 --- a/docs/content/security.md +++ b/docs/content/security.md @@ -191,7 +191,31 @@ policy: # characters following a hyphen are uppercase. The rest are lowercase. # If the header key contains space or invalid header field bytes, # no conversion is performed. - "headers": {"...": [...]} + "headers": {"...": [...]}, + + # Request message body if present for applicable APIs. + # + # Example Request: + # + # POST v1/data HTTP/1.1 + # Content-Type: application/json + # + # {"input": {"action": "trade", "stock": "ACME"}} + # + # Example input.body Value: + # + # {"input": {"action": "trade", "stock": "ACME"}} + # + # Example body check: + # + # input.body.input.stock == "ACME" + # + # The 'body' field is provided for the following APIs: + # + # * POST v1/data + # * POST v0/data + # * POST / + "body": ..., } ``` diff --git a/server/authorizer/authorizer.go b/server/authorizer/authorizer.go index 7d0ae4f801..e47f6565d3 100644 --- a/server/authorizer/authorizer.go +++ b/server/authorizer/authorizer.go @@ -6,6 +6,8 @@ package authorizer import ( + "context" + "io/ioutil" "net/http" "net/url" "strings" @@ -16,6 +18,7 @@ import ( "github.com/open-policy-agent/opa/server/types" "github.com/open-policy-agent/opa/server/writer" "github.com/open-policy-agent/opa/storage" + "github.com/open-policy-agent/opa/util" ) // Basic provides policy-based authorization over incoming requests. @@ -59,7 +62,9 @@ func NewBasic(inner http.Handler, compiler func() *ast.Compiler, store storage.S func (h *Basic) ServeHTTP(w http.ResponseWriter, r *http.Request) { - input, err := makeInput(r) + // TODO(tsandall): Pass AST value as input instead of Go value to avoid unnecessary + // conversions. + r, input, err := makeInput(r) if err != nil { writer.ErrorString(w, http.StatusBadRequest, types.CodeInvalidParameter, err) return @@ -97,15 +102,25 @@ func (h *Basic) ServeHTTP(w http.ResponseWriter, r *http.Request) { writer.Error(w, http.StatusUnauthorized, types.NewErrorV1(types.CodeUnauthorized, types.MsgUnauthorizedError)) } -func makeInput(r *http.Request) (interface{}, error) { +func makeInput(r *http.Request) (*http.Request, interface{}, error) { + path, err := parsePath(r.URL.Path) if err != nil { - return nil, err + return r, nil, err } method := strings.ToUpper(r.Method) query := r.URL.Query() + var rawBody []byte + + if expectBody(r.Method, path) { + rawBody, err = readBody(r) + if err != nil { + return r, nil, err + } + } + input := map[string]interface{}{ "path": path, "method": method, @@ -113,12 +128,66 @@ func makeInput(r *http.Request) (interface{}, error) { "headers": r.Header, } + if len(rawBody) > 0 { + var body interface{} + if expectYAML(r) { + if err := util.Unmarshal(rawBody, &body); err != nil { + return r, nil, err + } + } else if err := util.UnmarshalJSON(rawBody, &body); err != nil { + return r, nil, err + } + + // We cache the parsed body on the context so the server does not have + // to parse the input document twice. + input["body"] = body + ctx := SetBodyOnContext(r.Context(), body) + r = r.WithContext(ctx) + } + identity, ok := identifier.Identity(r) if ok { input["identity"] = identity } - return input, nil + return r, input, nil +} + +var dataAPIVersions = map[string]bool{ + "v0": true, + "v1": true, +} + +func expectBody(method string, path []interface{}) bool { + if method == http.MethodPost { + if len(path) == 1 { + s := path[0].(string) + return s == "" + } else if len(path) >= 2 { + s1 := path[0].(string) + s2 := path[1].(string) + return dataAPIVersions[s1] && s2 == "data" + } + } + return false +} + +func expectYAML(r *http.Request) bool { + // NOTE(tsandall): This check comes from the server's HTTP handler code. The docs + // are a bit more strict, but the authorizer should be consistent w/ the original + // server handler implementation. + return strings.Contains(r.Header.Get("Content-Type"), "yaml") +} + +func readBody(r *http.Request) ([]byte, error) { + + bs, err := ioutil.ReadAll(r.Body) + + if err != nil { + return nil, err + } + + return bs, nil } func parsePath(path string) ([]interface{}, error) { @@ -139,3 +208,29 @@ func parsePath(path string) ([]interface{}, error) { } return sl, nil } + +type authorizerCachedBody struct { + parsed interface{} +} + +type authorizerCachedBodyKey string + +const ctxkey authorizerCachedBodyKey = "authorizerCachedBodyKey" + +// SetBodyOnContext adds the parsed input value to the context. This function is only +// exposed for test purposes. +func SetBodyOnContext(ctx context.Context, x interface{}) context.Context { + return context.WithValue(ctx, ctxkey, authorizerCachedBody{ + parsed: x, + }) +} + +// GetBodyOnContext returns the parsed input from the request context if it exists. +// The authorizer saves the parsed input on the context when it runs. +func GetBodyOnContext(ctx context.Context) (interface{}, bool) { + input, ok := ctx.Value(ctxkey).(authorizerCachedBody) + if !ok { + return nil, false + } + return input.parsed, true +} diff --git a/server/authorizer/authorizer_test.go b/server/authorizer/authorizer_test.go index b45cda4ad5..e33df72ed4 100644 --- a/server/authorizer/authorizer_test.go +++ b/server/authorizer/authorizer_test.go @@ -5,6 +5,7 @@ package authorizer import ( + "bytes" "encoding/json" "net/http" "reflect" @@ -251,7 +252,7 @@ func TestMakeInput(t *testing.T) { req = identifier.SetIdentity(req, "bob") - result, err := makeInput(req) + req, result, err := makeInput(req) if err != nil { panic(err) } @@ -275,3 +276,123 @@ func TestMakeInput(t *testing.T) { } } + +func TestMakeInputWithBody(t *testing.T) { + + reqs := []struct { + method string + path string + headers map[string]string + body string + useYAML bool + assertBodyExists bool + assertBodyDoesNotExist bool + }{ + { + method: "POST", + path: "/", + body: `{"foo": "bar"}`, + assertBodyExists: true, + }, + { + method: "POST", + path: "/", + body: `foo: bar`, + useYAML: true, + assertBodyExists: true, + }, + { + method: "POST", + path: "/v0/data", + body: `{"foo": "bar"}`, + assertBodyExists: true, + }, + { + method: "POST", + path: "/v1/data", + body: `{"foo": "bar"}`, + assertBodyExists: true, + }, + { + method: "PUT", + path: "/v1/data", + body: `{"foo": "bar"}`, + assertBodyDoesNotExist: true, + }, + { + method: "PATCH", + path: "/v1/data", + body: `{"foo": "bar"}`, + assertBodyDoesNotExist: true, + }, + { + method: "GET", + path: "/v1/data", + assertBodyDoesNotExist: true, + }, + { + method: "PUT", + path: "/v1/policies/test", + body: "package test\np = 7", + assertBodyDoesNotExist: true, + }, + } + + for _, tc := range reqs { + + t.Run(tc.method+"_"+tc.path, func(t *testing.T) { + + req, err := http.NewRequest(tc.method, "http://localhost:8181"+tc.path, bytes.NewBufferString(tc.body)) + if err != nil { + t.Fatal(err) + } + + if tc.useYAML { + req.Header.Set("Content-Type", "application/x-yaml") + } + + req, input, err := makeInput(req) + if err != nil { + t.Fatal(err) + } + + if tc.assertBodyExists { + + var want interface{} + + if tc.useYAML { + if err := util.Unmarshal([]byte(tc.body), &want); err != nil { + t.Fatal(err) + } + } else { + want = util.MustUnmarshalJSON([]byte(tc.body)) + } + + body := input.(map[string]interface{})["body"] + + if !reflect.DeepEqual(body, want) { + t.Fatalf("expected parsed bodies to be equal but got %v and want %v", body, want) + } + + body, ok := GetBodyOnContext(req.Context()) + if !ok || !reflect.DeepEqual(body, want) { + t.Fatalf("expected parsed body to be cached on context but got %v and want %v", body, want) + } + } + + if tc.assertBodyDoesNotExist { + _, ok := input.(map[string]interface{})["body"] + if ok { + t.Fatal("expected no parsed body in input") + } + _, ok = GetBodyOnContext(req.Context()) + if ok { + t.Fatal("expected no parsed body to be cached on context") + } + } + + }) + + } + +} diff --git a/server/server.go b/server/server.go index 4f1c459eb6..ca59d5e278 100644 --- a/server/server.go +++ b/server/server.go @@ -2313,14 +2313,22 @@ func getExplain(p []string, zero types.ExplainModeV1) types.ExplainModeV1 { } func readInputV0(r *http.Request) (ast.Value, error) { + + parsed, ok := authorizer.GetBodyOnContext(r.Context()) + if ok { + return ast.InterfaceToValue(parsed) + } + bs, err := ioutil.ReadAll(r.Body) if err != nil { return nil, err } + bs = bytes.TrimSpace(bs) if len(bs) == 0 { return nil, nil } + var x interface{} if strings.Contains(r.Header.Get("Content-Type"), "yaml") { @@ -2344,6 +2352,16 @@ func readInputGetV1(str string) (ast.Value, error) { func readInputPostV1(r *http.Request) (ast.Value, error) { + parsed, ok := authorizer.GetBodyOnContext(r.Context()) + if ok { + if obj, ok := parsed.(map[string]interface{}); ok { + if input, ok := obj["input"]; ok { + return ast.InterfaceToValue(input) + } + } + return nil, nil + } + bs, err := ioutil.ReadAll(r.Body) if err != nil { diff --git a/server/server_test.go b/server/server_test.go index 367b03a398..2c7e548a7b 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -5,6 +5,7 @@ package server import ( + "bytes" "context" "encoding/json" "fmt" @@ -23,6 +24,7 @@ import ( "github.com/open-policy-agent/opa/metrics" "github.com/open-policy-agent/opa/plugins" pluginBundle "github.com/open-policy-agent/opa/plugins/bundle" + "github.com/open-policy-agent/opa/server/authorizer" "github.com/open-policy-agent/opa/server/identifier" "github.com/open-policy-agent/opa/server/types" "github.com/open-policy-agent/opa/storage" @@ -2782,6 +2784,32 @@ func TestAuthorization(t *testing.T) { // Try bob again. server.Handler.ServeHTTP(recorder, req1) validateAuthorizedRequest(t, server, req1, http.StatusUnauthorized) + + // Try to query for "data" as alice (allowed) + req3, err := http.NewRequest(http.MethodPost, "http://localhost:8182/v1/data", bytes.NewBufferString(`{"input": {"foo": "bar"}}`)) + if err != nil { + panic(err) + } + + req3 = identifier.SetIdentity(req3, "alice") + recorder = httptest.NewRecorder() + server.Handler.ServeHTTP(recorder, req3) + if recorder.Code != http.StatusOK { + t.Fatal("expected successful response for data") + } + + // Try to query for "data" as bob (denied) + req4, err := http.NewRequest(http.MethodPost, "http://localhost:8182/v1/data", bytes.NewBufferString(`{"input": {"foo": "bar"}}`)) + if err != nil { + panic(err) + } + + req4 = identifier.SetIdentity(req4, "bob") + recorder = httptest.NewRecorder() + server.Handler.ServeHTTP(recorder, req4) + if recorder.Code != http.StatusUnauthorized { + t.Fatal("expected unauthorized response for data") + } } func validateAuthorizedRequest(t *testing.T, s *Server, req *http.Request, exp int) { @@ -2804,6 +2832,48 @@ func validateAuthorizedRequest(t *testing.T, s *Server, req *http.Request, exp i } } +func TestServerUsesAuthorizerParsedBody(t *testing.T) { + + // Construct a request w/ a different message body (this should never happen.) + req, err := http.NewRequest(http.MethodPost, "http://localhost:8182/v1/data/test/echo", bytes.NewBufferString(`{"foo": "bad"}`)) + if err != nil { + t.Fatal(err) + } + + // Set the authorizer's parsed input to the expected message body. + ctx := authorizer.SetBodyOnContext(req.Context(), map[string]interface{}{ + "input": map[string]interface{}{ + "foo": "good", + }, + }) + + // Check that v1 reader function behaves correctly. + inp, err := readInputPostV1(req.WithContext(ctx)) + if err != nil { + t.Fatal(err) + } + + exp := ast.MustParseTerm(`{"foo": "good"}`) + + if exp.Value.Compare(inp) != 0 { + t.Fatalf("expected %v but got %v", exp, inp) + } + + // Check that v0 reader function behaves correctly. + ctx = authorizer.SetBodyOnContext(req.Context(), map[string]interface{}{ + "foo": "good", + }) + + inp, err = readInputV0(req.WithContext(ctx)) + if err != nil { + t.Fatal(err) + } + + if exp.Value.Compare(inp) != 0 { + t.Fatalf("expected %v but got %v", exp, inp) + } +} + func TestServerReloadTrigger(t *testing.T) { f := newFixture(t) store := f.server.store