From b95227688bbfb22c60e52dc89eb93ebdd7b3290b Mon Sep 17 00:00:00 2001 From: Torin Sandall Date: Wed, 17 Oct 2018 08:56:39 -0700 Subject: [PATCH] Add default decision configuration Previously, OPA would serve POST requests with an empty URL path by querying data.system.main and returning the generated value. In some cases, it's useful to be able to reconfigure OPA to use a different document to serve these kinds of requests. The same goes for the OPA authorization policy. These changes make the default decision and default authorization decision paths configurable. Signed-off-by: Torin Sandall --- docs/book/configuration.md | 6 +- docs/book/rest-api.md | 8 ++- runtime/runtime.go | 94 +++++++++++++++++++++++----- server/authorizer/authorizer.go | 18 +++--- server/authorizer/authorizer_test.go | 2 +- server/server.go | 62 +++++++++++------- server/server_test.go | 6 ++ 7 files changed, 145 insertions(+), 51 deletions(-) diff --git a/docs/book/configuration.md b/docs/book/configuration.md index 787c486cbb..47e8f07c9a 100644 --- a/docs/book/configuration.md +++ b/docs/book/configuration.md @@ -35,6 +35,8 @@ decision_logs: status: service: acmecorp + +default_decision: /http/example/authz/allow ``` ## Services @@ -56,6 +58,8 @@ multiple services. | Field | Type | Required | Description | | --- | --- | --- | --- | | `labels` | `object` | Yes | Set of key-value pairs that uniquely identify the OPA instance. Labels are included when OPA uploads decision logs and status information. | +| `default_decision` | `string` | No (default: `/system/main`) | Set path of default policy decision used to serve queries against OPA's base URL. | +| `default_authorization_decision` | `string` | No (default: `/system/authz/allow`) | Set path of default authorization decision for OPA's API. | ## Bundles @@ -82,4 +86,4 @@ multiple services. | `decision_logs.reporting.buffer_size_limit_bytes` | `int64` | No | Decision log buffer size limit in bytes. OPA will drop old events from the log if this limit is exceeded. By default, no limit is set. | | `decision_logs.reporting.upload_size_limit_bytes` | `int64` | No (default: `32768`) | Decision log upload size limit in bytes. OPA will chunk uploads to cap message body to this limit. | | `decision_logs.reporting.min_delay_seconds` | `int64` | No (default: `300`) | Minimum amount of time to wait between uploads. | -| `decision_logs.reporting.max_delay_seconds` | `int64` | No (default: `600`) | Maximum amount of time to wait between uploads. | +| `decision_logs.reporting.max_delay_seconds` | `int64` | No (default: `600`) | Maximum amount of time to wait between uploads. | \ No newline at end of file diff --git a/docs/book/rest-api.md b/docs/book/rest-api.md index 2773131717..3a81174823 100644 --- a/docs/book/rest-api.md +++ b/docs/book/rest-api.md @@ -1316,8 +1316,10 @@ Execute a simple query. OPA serves POST requests without a URL path by querying for the document at path `/data/system/main`. The content of that document defines the response -entirely. The policy example below shows how to define a rule that will produce a -value for the `/data/system/main` document. +entirely. The policy example below shows how to define a rule that will +produce a value for the `/data/system/main` document. You can configure OPA +to use a different URL path to serve these queries. See the [Configuration Reference](configuration.md) +for more information. The request message body is mapped to the [Input Document](/how-does-opa-work.md#the-input-document). @@ -1373,7 +1375,7 @@ Content-Type: application/json - **404** - not found - **500** - server error -If the `/data/system/main` document is undefined (e.g., because the administrator has not defined one) the server returns 404. +If the default decision (defaulting to `/system/main`) is undefined, the server returns 404. ### Execute an Ad-hoc Query diff --git a/runtime/runtime.go b/runtime/runtime.go index a07cfb597d..c47e773e38 100644 --- a/runtime/runtime.go +++ b/runtime/runtime.go @@ -14,6 +14,7 @@ import ( "io" "io/ioutil" "os" + "strings" "sync" "time" @@ -135,8 +136,10 @@ type Runtime struct { Store storage.Store Manager *plugins.Manager - info *ast.Term // runtime information provided to evaluation engine - decisionLogger func(context.Context, *server.Info) + info *ast.Term // runtime information provided to evaluation engine + defaultDecision ast.Ref + defaultAuthorizationDecision ast.Ref + decisionLogger func(context.Context, *server.Info) } // NewRuntime returns a new Runtime object initialized with params. @@ -176,14 +179,14 @@ func NewRuntime(ctx context.Context, params Params) (*Runtime, error) { return nil, errors.Wrapf(err, "storage error") } - m, plugins, err := initPlugins(params.ID, store, params.ConfigFile) + cfg, err := loadConfig(params.ID, store, params.ConfigFile) if err != nil { return nil, err } var decisionLogger func(context.Context, *server.Info) - if p, ok := plugins["decision_logs"]; ok { + if p, ok := cfg.Plugins["decision_logs"]; ok { decisionLogger = p.(*logs.Plugin).Log if params.DecisionIDFactory == nil { @@ -197,11 +200,13 @@ func NewRuntime(ctx context.Context, params Params) (*Runtime, error) { } rt := &Runtime{ - Store: store, - Manager: m, - Params: params, - info: info, - decisionLogger: decisionLogger, + Store: store, + Manager: cfg.Manager, + Params: params, + info: info, + defaultDecision: cfg.DefaultDecision, + defaultAuthorizationDecision: cfg.DefaultAuthorizationDecision, + decisionLogger: decisionLogger, } return rt, nil @@ -235,6 +240,8 @@ func (rt *Runtime) StartServer(ctx context.Context) { WithDecisionIDFactory(rt.Params.DecisionIDFactory). WithDecisionLogger(rt.decisionLogger). WithRuntime(rt.info). + WithDefaultDecision(rt.defaultDecision). + WithDefaultAuthorizationDecision(rt.defaultAuthorizationDecision). Init(ctx) if err != nil { @@ -476,11 +483,33 @@ func setupLogging(config LoggingConfig) { logrus.SetLevel(lvl) } +type loadedConfig struct { + Manager *plugins.Manager + Plugins map[string]plugins.Plugin + DefaultDecision ast.Ref + DefaultAuthorizationDecision ast.Ref +} + +type rawConfig struct { + DefaultDecision string `json:"default_decision"` + DefaultAuthorizationDecision string `json:"default_authorization_decision"` +} + +func parsePathToRef(s string) (ast.Ref, error) { + s = strings.Replace(strings.Trim(s, "/"), "/", ".", -1) + return ast.ParseRef("data." + s) +} + +const ( + defaultDecisionPath = "/system/main" + defaultAuthorizationDecisionPath = "/system/authz/allow" +) + // TODO(tsandall): revisit how plugins are wired up to the manager and how // everything is started and stopped. We could introduce a package-scoped // plugin registry that allows for (dynamic) init-time plugin registration. -func initPlugins(id string, store storage.Store, configFile string) (*plugins.Manager, map[string]plugins.Plugin, error) { +func loadConfig(id string, store storage.Store, configFile string) (*loadedConfig, error) { var bs []byte var err error @@ -488,20 +517,20 @@ func initPlugins(id string, store storage.Store, configFile string) (*plugins.Ma if configFile != "" { bs, err = ioutil.ReadFile(configFile) if err != nil { - return nil, nil, err + return nil, err } } m, err := plugins.New(bs, id, store) if err != nil { - return nil, nil, err + return nil, err } plugins := map[string]plugins.Plugin{} bundlePlugin, err := initBundlePlugin(m, bs) if err != nil { - return nil, nil, err + return nil, err } else if bundlePlugin != nil { plugins["bundle"] = bundlePlugin } @@ -509,7 +538,7 @@ func initPlugins(id string, store storage.Store, configFile string) (*plugins.Ma if bundlePlugin != nil { statusPlugin, err := initStatusPlugin(m, bs, bundlePlugin) if err != nil { - return nil, nil, err + return nil, err } else if statusPlugin != nil { plugins["status"] = statusPlugin } @@ -517,17 +546,48 @@ func initPlugins(id string, store storage.Store, configFile string) (*plugins.Ma decisionLogsPlugin, err := initDecisionLogsPlugin(m, bs) if err != nil { - return nil, nil, err + return nil, err } else if decisionLogsPlugin != nil { plugins["decision_logs"] = decisionLogsPlugin } err = initRegisteredPlugins(m, bs) if err != nil { - return nil, nil, err + return nil, err } - return m, plugins, nil + var raw rawConfig + + if err := util.Unmarshal(bs, &raw); err != nil { + return nil, err + } + + if raw.DefaultDecision == "" { + raw.DefaultDecision = defaultDecisionPath + } + + if raw.DefaultAuthorizationDecision == "" { + raw.DefaultAuthorizationDecision = defaultAuthorizationDecisionPath + } + + defaultDecision, err := parsePathToRef(raw.DefaultDecision) + if err != nil { + return nil, err + } + + defaultAuthorizationDecision, err := parsePathToRef(raw.DefaultAuthorizationDecision) + if err != nil { + return nil, err + } + + c := &loadedConfig{ + Manager: m, + Plugins: plugins, + DefaultDecision: defaultDecision, + DefaultAuthorizationDecision: defaultAuthorizationDecision, + } + + return c, nil } func initBundlePlugin(m *plugins.Manager, bs []byte) (*bundle.Plugin, error) { diff --git a/server/authorizer/authorizer.go b/server/authorizer/authorizer.go index 452e5988e2..7526d3fb77 100644 --- a/server/authorizer/authorizer.go +++ b/server/authorizer/authorizer.go @@ -7,9 +7,8 @@ package authorizer import ( "net/http" - "strings" - "net/url" + "strings" "github.com/open-policy-agent/opa/ast" "github.com/open-policy-agent/opa/rego" @@ -19,16 +18,13 @@ import ( "github.com/open-policy-agent/opa/storage" ) -// SystemAuthzPath is the path of the document that defines auth/z decisions for -// OPA itself. -const SystemAuthzPath = "data.system.authz.allow" - // Basic provides policy-based authorization over incoming requests. type Basic struct { inner http.Handler compiler func() *ast.Compiler store storage.Store runtime *ast.Term + decision string } // Runtime returns an argument that sets the runtime on the authorizer. @@ -38,6 +34,14 @@ func Runtime(term *ast.Term) func(*Basic) { } } +// Decision returns an argument that sets the path of the authorization decision +// to query. +func Decision(ref ast.Ref) func(*Basic) { + return func(b *Basic) { + b.decision = ref.String() + } +} + // NewBasic returns a new Basic object. func NewBasic(inner http.Handler, compiler func() *ast.Compiler, store storage.Store, opts ...func(*Basic)) http.Handler { b := &Basic{ @@ -62,7 +66,7 @@ func (h *Basic) ServeHTTP(w http.ResponseWriter, r *http.Request) { } rego := rego.New( - rego.Query(SystemAuthzPath), + rego.Query(h.decision), rego.Compiler(h.compiler()), rego.Store(h.store), rego.Input(input), diff --git a/server/authorizer/authorizer_test.go b/server/authorizer/authorizer_test.go index 7919258194..07dc96d6e1 100644 --- a/server/authorizer/authorizer_test.go +++ b/server/authorizer/authorizer_test.go @@ -163,7 +163,7 @@ func TestBasic(t *testing.T) { req = identifier.SetIdentity(req, tc.identity) } - NewBasic(&mockHandler{}, compiler, store).ServeHTTP(recorder, req) + NewBasic(&mockHandler{}, compiler, store, Decision(ast.MustParseRef("data.system.authz.allow"))).ServeHTTP(recorder, req) if recorder.Code != tc.expectedStatus { t.Fatalf("Expected status code %v but got: %v", tc.expectedStatus, recorder) diff --git a/server/server.go b/server/server.go index 63bbf29bce..aa90555404 100644 --- a/server/server.go +++ b/server/server.go @@ -16,14 +16,13 @@ import ( "net" "net/http" "net/http/httputil" + "net/url" "os" "strconv" "strings" "sync" "time" - "net/url" - "github.com/gorilla/mux" "github.com/open-policy-agent/opa/ast" "github.com/open-policy-agent/opa/metrics" @@ -74,8 +73,6 @@ const ( PromHandlerCatch = "catchall" ) -var systemMainPath = ast.MustParseRef("data.system.main") - // map of unsafe buitins var unsafeBuiltinsMap = map[string]bool{ast.HTTPSend.Name: true} @@ -83,22 +80,24 @@ var unsafeBuiltinsMap = map[string]bool{ast.HTTPSend.Name: true} type Server struct { Handler http.Handler - addrs []string - insecureAddr string - authentication AuthenticationScheme - authorization AuthorizationScheme - cert *tls.Certificate - mtx sync.RWMutex - partials map[string]rego.PartialResult - store storage.Store - manager *plugins.Manager - watcher *watch.Watcher - decisionIDFactory func() string - diagnostics Buffer - revision string - logger func(context.Context, *Info) - errLimit int - runtime *ast.Term + addrs []string + insecureAddr string + authentication AuthenticationScheme + authorization AuthorizationScheme + cert *tls.Certificate + mtx sync.RWMutex + partials map[string]rego.PartialResult + store storage.Store + manager *plugins.Manager + watcher *watch.Watcher + decisionIDFactory func() string + diagnostics Buffer + revision string + logger func(context.Context, *Info) + errLimit int + runtime *ast.Term + defaultDecision ast.Ref + defaultAuthorizationDecision ast.Ref } // Loop will contain all the calls from the server that we'll be listening on. @@ -187,7 +186,12 @@ func (s *Server) Init(ctx context.Context) (*Server, error) { // so that the latter can run first. switch s.authorization { case AuthorizationBasic: - s.Handler = authorizer.NewBasic(s.Handler, s.getCompiler, s.store, authorizer.Runtime(s.runtime)) + s.Handler = authorizer.NewBasic( + s.Handler, + s.getCompiler, + s.store, + authorizer.Runtime(s.runtime), + authorizer.Decision(s.defaultAuthorizationDecision)) } switch s.authentication { @@ -295,6 +299,20 @@ func (s *Server) WithRuntime(term *ast.Term) *Server { return s } +// WithDefaultDecision sets path of the policy decision to query to serve +// requests with an empty URL path. +func (s *Server) WithDefaultDecision(ref ast.Ref) *Server { + s.defaultDecision = ref + return s +} + +// WithDefaultAuthorizationDecision sets path of the policy decision to query to +// authorize requests to OPA itself. +func (s *Server) WithDefaultAuthorizationDecision(ref ast.Ref) *Server { + s.defaultAuthorizationDecision = ref + return s +} + // Listeners returns functions that listen and serve connections. func (s *Server) Listeners() ([]Loop, error) { loops := []Loop{} @@ -511,7 +529,7 @@ func (s *Server) migrateWatcher(txn storage.Transaction) { } func (s *Server) unversionedPost(w http.ResponseWriter, r *http.Request) { - s.v0QueryPath(w, r, systemMainPath) + s.v0QueryPath(w, r, s.defaultDecision) } func (s *Server) v0DataPost(w http.ResponseWriter, r *http.Request) { diff --git a/server/server_test.go b/server/server_test.go index e47db654d2..bef055baba 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -1787,6 +1787,8 @@ func TestDiagnostics(t *testing.T) { WithStore(f.server.store). WithManager(f.server.manager). WithDiagnosticsBuffer(NewBoundedBuffer(8)). + WithDefaultDecision(ast.MustParseRef("data.system.main")). + WithDefaultAuthorizationDecision(ast.MustParseRef("data.system.authz.allow")). Init(context.Background()) queriesOnly := `package system.diagnostics @@ -2427,6 +2429,8 @@ func TestAuthorization(t *testing.T) { WithStore(store). WithManager(m). WithAuthorization(AuthorizationBasic). + WithDefaultDecision(ast.MustParseRef("data.system.main")). + WithDefaultAuthorizationDecision(ast.MustParseRef("data.system.authz.allow")). Init(ctx) if err != nil { @@ -2617,6 +2621,8 @@ func newFixture(t *testing.T) *fixture { WithAddresses([]string{":8182"}). WithStore(store). WithManager(m). + WithDefaultDecision(ast.MustParseRef("data.system.main")). + WithDefaultAuthorizationDecision(ast.MustParseRef("data.system.authz.allow")). Init(ctx) if err != nil { panic(err)