feat: replace OPA deny-reasons with boolean allow rule

README.md

@@ -143,16 +143,16 @@ s3 {
 
 ## Authorization (OPA)
 
-Cachew uses [Open Policy Agent](https://www.openpolicyagent.org/) for request authorization. The default policy allows all methods from `127.0.0.1` and `GET`/`HEAD` from elsewhere.
+Cachew uses [Open Policy Agent](https://www.openpolicyagent.org/) for request authorization. The default policy allows GET/HEAD from any source and all methods from `127.0.0.1`.
 
-Policies must be in `package cachew.authz` and define a `deny` rule set. If the set is empty, the request is allowed; otherwise the reasons are returned to the client.
+Policies must be in `package cachew.authz` and define an `allow` rule. If `allow` is true the request proceeds; otherwise it is rejected with 403.
 
 ```hcl
 opa {
   policy = <<EOF
     package cachew.authz
-    deny contains "unauthenticated" if not input.headers["authorization"]
-    deny contains "writes not allowed" if input.method == "PUT"
+    default allow := false
+    allow if input.headers["authorization"]
   EOF
 }
 ```
@@ -239,7 +239,8 @@ log {
 opa {
   policy = <<EOF
     package cachew.authz
-    deny contains "not localhost" if not startswith(input.remote_addr, "127.0.0.1:")
+    default allow := false
+    allow if startswith(input.remote_addr, "127.0.0.1:")
   EOF
 }
 

internal/opa/opa.go

@@ -6,7 +6,6 @@ import (
 	"encoding/json"
 	"net/http"
 	"os"
-	"sort"
 	"strings"
 
 	"github.com/alecthomas/errors"
@@ -15,11 +14,12 @@ import (
 	"github.com/block/cachew/internal/logging"
 )
 
-// DefaultPolicy allows only GET and HEAD requests from localhost.
+// DefaultPolicy allows GET and HEAD from any source, and all methods from localhost.
 const DefaultPolicy = `package cachew.authz
 
-deny contains "method not allowed" if not input.method in {"GET", "HEAD"}
-deny contains "remote address not allowed" if not startswith(input.remote_addr, "127.0.0.1:")
+default allow := false
+allow if input.method in {"GET", "HEAD"}
+allow if startswith(input.remote_addr, "127.0.0.1:")
 `
 
 // Config for OPA policy evaluation. If neither Policy nor PolicyFile is set,
@@ -32,10 +32,8 @@ type Config struct {
 }
 
 // Middleware returns an http.Handler that evaluates OPA policy before delegating to next.
-// The policy must define a set "deny" rule under package cachew.authz whose elements
-// are human-readable reason strings (e.g. `deny contains "unauthenticated" if ...`).
-// If the deny set is empty, the request is allowed. Otherwise it is rejected and
-// the reasons are included in the response body and server logs.
+// The policy must define a boolean "allow" rule under package cachew.authz.
+// If allow is true the request proceeds; otherwise it is rejected with 403.
 func Middleware(ctx context.Context, cfg Config, next http.Handler) (http.Handler, error) {
 	policy, err := loadPolicy(cfg)
 	if err != nil {
@@ -47,24 +45,24 @@ func Middleware(ctx context.Context, cfg Config, next http.Handler) (http.Handle
 		return nil, err
 	}
 
-	prepared, err := prepareQuery(ctx, "data.cachew.authz.deny", policy, dataOpts)
+	prepared, err := prepareQuery(ctx, "data.cachew.authz.allow", policy, dataOpts)
 	if err != nil {
-		return nil, errors.Errorf("compile OPA deny query: %w", err)
+		return nil, errors.Errorf("compile OPA allow query: %w", err)
 	}
 
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		input := buildInput(r)
 		logger := logging.FromContext(r.Context())
 
-		reasons, err := evalDeny(r.Context(), prepared, input)
+		allowed, err := evalAllow(r.Context(), prepared, input)
 		if err != nil {
 			logger.Error("OPA evaluation failed", "error", err)
 			http.Error(w, "policy evaluation error", http.StatusInternalServerError)
 			return
 		}
-		if len(reasons) > 0 {
-			logger.Warn("OPA denied request", "method", r.Method, "path", r.URL.Path, "remote_addr", r.RemoteAddr, "reasons", reasons)
-			http.Error(w, "forbidden: "+strings.Join(reasons, "; "), http.StatusForbidden)
+		if !allowed {
+			logger.Warn("OPA denied request", "method", r.Method, "path", r.URL.Path, "remote_addr", r.RemoteAddr)
+			http.Error(w, "forbidden", http.StatusForbidden)
 			return
 		}
 
@@ -96,33 +94,17 @@ func dataOptions(cfg Config) ([]func(*rego.Rego), error) {
 	return []func(*rego.Rego){rego.Data(opaData)}, nil
 }
 
-// evalDeny evaluates the prepared deny query and returns any denial reason strings.
-// If the policy produces no deny reasons, nil is returned.
-func evalDeny(ctx context.Context, prepared rego.PreparedEvalQuery, input map[string]any) ([]string, error) {
+// evalAllow evaluates the prepared allow query and returns whether the request is permitted.
+func evalAllow(ctx context.Context, prepared rego.PreparedEvalQuery, input map[string]any) (bool, error) {
 	results, err := prepared.Eval(ctx, rego.EvalInput(input))
 	if err != nil {
-		return nil, errors.Errorf("evaluate deny query: %w", err)
+		return false, errors.Errorf("evaluate allow query: %w", err)
 	}
 	if len(results) == 0 || len(results[0].Expressions) == 0 {
-		return nil, nil
-	}
-	val := results[0].Expressions[0].Value
-	// OPA represents sets as []interface{} in the Go bindings.
-	set, isSet := val.([]any)
-	if !isSet {
-		return nil, nil
-	}
-	if len(set) == 0 {
-		return nil, nil
-	}
-	reasons := make([]string, 0, len(set))
-	for _, v := range set {
-		if s, isString := v.(string); isString {
-			reasons = append(reasons, s)
-		}
+		return false, nil
 	}
-	sort.Strings(reasons) // deterministic order for logging/testing
-	return reasons, nil
+	allowed, ok := results[0].Expressions[0].Value.(bool)
+	return ok && allowed, nil
 }
 
 func loadPolicy(cfg Config) (string, error) {

internal/opa/opa_test.go

@@ -23,13 +23,15 @@ func TestMiddlewareDefaultPolicy(t *testing.T) {
 	tests := []struct {
 		Name           string
 		Method         string
+		RemoteAddr     string
 		ExpectedStatus int
 	}{
-		{"GETAllowed", http.MethodGet, http.StatusOK},
-		{"HEADAllowed", http.MethodHead, http.StatusOK},
-		{"POSTDenied", http.MethodPost, http.StatusForbidden},
-		{"PUTDenied", http.MethodPut, http.StatusForbidden},
-		{"DELETEDenied", http.MethodDelete, http.StatusForbidden},
+		{"GETFromAnywhere", http.MethodGet, "10.0.0.1:9999", http.StatusOK},
+		{"HEADFromAnywhere", http.MethodHead, "10.0.0.1:9999", http.StatusOK},
+		{"POSTFromLocalhost", http.MethodPost, "127.0.0.1:12345", http.StatusOK},
+		{"PUTFromLocalhost", http.MethodPut, "127.0.0.1:12345", http.StatusOK},
+		{"POSTFromRemote", http.MethodPost, "10.0.0.1:9999", http.StatusForbidden},
+		{"DELETEFromRemote", http.MethodDelete, "10.0.0.1:9999", http.StatusForbidden},
 	}
 
 	next := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})
@@ -39,33 +41,18 @@ func TestMiddlewareDefaultPolicy(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.Name, func(t *testing.T) {
 			r := newRequest(test.Method, "/some/path")
-			// Default policy requires localhost; httptest uses 192.0.2.1, so
-			// non-localhost requests that are also non-GET/HEAD get two reasons.
-			// Override RemoteAddr so we only test the method rule.
-			r.RemoteAddr = "127.0.0.1:12345"
+			r.RemoteAddr = test.RemoteAddr
 			w := httptest.NewRecorder()
 			handler.ServeHTTP(w, r)
 			assert.Equal(t, test.ExpectedStatus, w.Code)
 		})
 	}
 }
 
-func TestMiddlewareDefaultPolicyDeniesNonLocalhost(t *testing.T) {
-	next := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})
-	handler, err := opa.Middleware(t.Context(), opa.Config{}, next)
-	assert.NoError(t, err)
-
-	r := newRequest(http.MethodGet, "/some/path")
-	r.RemoteAddr = "10.0.0.1:9999"
-	w := httptest.NewRecorder()
-	handler.ServeHTTP(w, r)
-	assert.Equal(t, http.StatusForbidden, w.Code)
-	assert.Contains(t, w.Body.String(), "remote address not allowed")
-}
-
 func TestMiddlewareInlinePolicy(t *testing.T) {
 	policy := `package cachew.authz
-deny contains "only POST allowed" if input.method != "POST"
+default allow := false
+allow if input.method == "POST"
 `
 	next := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})
 	handler, err := opa.Middleware(t.Context(), opa.Config{Policy: policy}, next)
@@ -91,7 +78,8 @@ deny contains "only POST allowed" if input.method != "POST"
 
 func TestMiddlewarePolicyFile(t *testing.T) {
 	policy := `package cachew.authz
-deny contains "private path" if input.path[0] != "public"
+default allow := false
+allow if input.path[0] == "public"
 `
 	dir := t.TempDir()
 	path := filepath.Join(dir, "policy.rego")
@@ -121,10 +109,9 @@ deny contains "private path" if input.path[0] != "public"
 
 func TestMiddlewarePathBasedPolicy(t *testing.T) {
 	policy := `package cachew.authz
-deny contains "path not allowed" if {
-	not input.path[0] == "api"
-	not input.path[0] == "_liveness"
-}
+default allow := false
+allow if input.path[0] == "api"
+allow if input.path[0] == "_liveness"
 `
 	next := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})
 	handler, err := opa.Middleware(t.Context(), opa.Config{Policy: policy}, next)
@@ -151,7 +138,8 @@ deny contains "path not allowed" if {
 
 func TestMiddlewareInlineData(t *testing.T) {
 	policy := `package cachew.authz
-deny contains "method not in allowed set" if not data.allowed_methods[input.method]
+default allow := false
+allow if data.allowed_methods[input.method]
 `
 	next := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})
 	handler, err := opa.Middleware(t.Context(), opa.Config{
@@ -193,7 +181,8 @@ func TestMiddlewareInlineDataInvalidJSON(t *testing.T) {
 
 func TestMiddlewareDataFile(t *testing.T) {
 	policy := `package cachew.authz
-deny contains "method not in allowed set" if not data.allowed_methods[input.method]
+default allow := false
+allow if data.allowed_methods[input.method]
 `
 	dataJSON := `{"allowed_methods": {"POST": true, "PUT": true}}`
 
@@ -261,80 +250,28 @@ func TestMiddlewareInvalidPolicy(t *testing.T) {
 	assert.Error(t, err)
 }
 
-func TestMiddlewareDenyReasons(t *testing.T) {
+func TestMiddlewareHeaderBasedPolicy(t *testing.T) {
 	policy := `package cachew.authz
-deny contains "writes are not allowed" if input.method == "PUT"
-deny contains "deletes are not allowed" if input.method == "DELETE"
+default allow := false
+allow if input.headers["authorization"]
 `
 	next := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})
 	handler, err := opa.Middleware(t.Context(), opa.Config{Policy: policy}, next)
 	assert.NoError(t, err)
 
-	tests := []struct {
-		Name           string
-		Method         string
-		ExpectedStatus int
-		ExpectedBody   string
-	}{
-		{"GETAllowed", http.MethodGet, http.StatusOK, ""},
-		{"PUTDenied", http.MethodPut, http.StatusForbidden, "forbidden: writes are not allowed\n"},
-		{"DELETEDenied", http.MethodDelete, http.StatusForbidden, "forbidden: deletes are not allowed\n"},
-	}
-	for _, test := range tests {
-		t.Run(test.Name, func(t *testing.T) {
-			r := newRequest(test.Method, "/")
-			w := httptest.NewRecorder()
-			handler.ServeHTTP(w, r)
-			assert.Equal(t, test.ExpectedStatus, w.Code)
-			if test.ExpectedBody != "" {
-				assert.Equal(t, test.ExpectedBody, w.Body.String())
-			}
-		})
-	}
-}
-
-func TestMiddlewareDenyUnauthenticated(t *testing.T) {
-	policy := `package cachew.authz
-deny contains "unauthenticated" if not input.headers["authorization"]
-`
-	next := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})
-	handler, err := opa.Middleware(t.Context(), opa.Config{Policy: policy}, next)
-	assert.NoError(t, err)
-
-	// Without Authorization header: denied.
 	r := newRequest(http.MethodGet, "/")
 	w := httptest.NewRecorder()
 	handler.ServeHTTP(w, r)
 	assert.Equal(t, http.StatusForbidden, w.Code)
-	assert.Equal(t, "forbidden: unauthenticated\n", w.Body.String())
 
-	// With Authorization header: allowed.
 	r = newRequest(http.MethodGet, "/")
 	r.Header.Set("Authorization", "Bearer token")
 	w = httptest.NewRecorder()
 	handler.ServeHTTP(w, r)
 	assert.Equal(t, http.StatusOK, w.Code)
 }
 
-func TestMiddlewareDenyMultipleReasons(t *testing.T) {
-	policy := `package cachew.authz
-deny contains "reason-a" if input.method == "POST"
-deny contains "reason-b" if input.method == "POST"
-`
-	next := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})
-	handler, err := opa.Middleware(t.Context(), opa.Config{Policy: policy}, next)
-	assert.NoError(t, err)
-
-	r := newRequest(http.MethodPost, "/")
-	w := httptest.NewRecorder()
-	handler.ServeHTTP(w, r)
-	assert.Equal(t, http.StatusForbidden, w.Code)
-	// Reasons are sorted deterministically.
-	assert.Equal(t, "forbidden: reason-a; reason-b\n", w.Body.String())
-}
-
-func TestMiddlewareEmptyDenyAllowsAll(t *testing.T) {
-	// A policy with no deny rules allows everything.
+func TestMiddlewareEmptyPolicyDeniesAll(t *testing.T) {
 	policy := `package cachew.authz
 `
 	next := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})
@@ -345,6 +282,6 @@ func TestMiddlewareEmptyDenyAllowsAll(t *testing.T) {
 		r := newRequest(method, "/any/path")
 		w := httptest.NewRecorder()
 		handler.ServeHTTP(w, r)
-		assert.Equal(t, http.StatusOK, w.Code)
+		assert.Equal(t, http.StatusForbidden, w.Code)
 	}
 }