ppl: pass contextual information through policy (#2612)

* ppl: pass contextual information through policy

* maybe fix nginx

* fix nginx

* pr comments

* go mod tidy

authorize/evaluator/evaluator.go

@@ -16,15 +16,12 @@ import (
 	"github.com/pomerium/pomerium/internal/telemetry/trace"
 	"github.com/pomerium/pomerium/internal/urlutil"
 	"github.com/pomerium/pomerium/pkg/cryptutil"
+	"github.com/pomerium/pomerium/pkg/policy/criteria"
 )
 
 // notFoundOutput is what's returned if a route isn't found for a policy.
 var notFoundOutput = &Result{
-	Allow: false,
-	Deny: &Denial{
-		Status:  http.StatusNotFound,
-		Message: "route not found",
-	},
+	Deny:    NewRuleResult(true, criteria.ReasonRouteNotFound),
 	Headers: make(http.Header),
 }
 
@@ -50,8 +47,8 @@ type RequestSession struct {
 
 // Result is the result of evaluation.
 type Result struct {
-	Allow   bool
-	Deny    *Denial
+	Allow   RuleResult
+	Deny    RuleResult
 	Headers http.Header
 
 	DataBrokerServerVersion, DataBrokerRecordVersion uint64

authorize/evaluator/evaluator_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/pomerium/pomerium/pkg/grpc/directory"
 	"github.com/pomerium/pomerium/pkg/grpc/session"
 	"github.com/pomerium/pomerium/pkg/grpc/user"
+	"github.com/pomerium/pomerium/pkg/policy/criteria"
 	"github.com/pomerium/pomerium/pkg/protoutil"
 )
 
@@ -98,7 +99,7 @@ func TestEvaluator(t *testing.T) {
 				Policy: &policies[0],
 			})
 			require.NoError(t, err)
-			assert.Equal(t, &Denial{Status: 495, Message: "invalid client certificate"}, res.Deny)
+			assert.Equal(t, NewRuleResult(true, criteria.ReasonInvalidClientCertificate), res.Deny)
 		})
 		t.Run("valid", func(t *testing.T) {
 			res, err := eval(t, options, nil, &Request{
@@ -108,7 +109,7 @@ func TestEvaluator(t *testing.T) {
 				},
 			})
 			require.NoError(t, err)
-			assert.Nil(t, res.Deny)
+			assert.False(t, res.Deny.Value)
 		})
 	})
 	t.Run("identity_headers", func(t *testing.T) {
@@ -186,7 +187,7 @@ func TestEvaluator(t *testing.T) {
 				},
 			})
 			require.NoError(t, err)
-			assert.True(t, res.Allow)
+			assert.True(t, res.Allow.Value)
 		})
 		t.Run("allowed sub", func(t *testing.T) {
 			res, err := eval(t, options, []proto.Message{
@@ -210,7 +211,7 @@ func TestEvaluator(t *testing.T) {
 				},
 			})
 			require.NoError(t, err)
-			assert.True(t, res.Allow)
+			assert.True(t, res.Allow.Value)
 		})
 		t.Run("denied", func(t *testing.T) {
 			res, err := eval(t, options, []proto.Message{
@@ -234,7 +235,7 @@ func TestEvaluator(t *testing.T) {
 				},
 			})
 			require.NoError(t, err)
-			assert.False(t, res.Allow)
+			assert.False(t, res.Allow.Value)
 		})
 	})
 	t.Run("impersonate email", func(t *testing.T) {
@@ -265,7 +266,7 @@ func TestEvaluator(t *testing.T) {
 				},
 			})
 			require.NoError(t, err)
-			assert.True(t, res.Allow)
+			assert.True(t, res.Allow.Value)
 		})
 	})
 	t.Run("user_id", func(t *testing.T) {
@@ -290,7 +291,7 @@ func TestEvaluator(t *testing.T) {
 			},
 		})
 		require.NoError(t, err)
-		assert.True(t, res.Allow)
+		assert.True(t, res.Allow.Value)
 	})
 	t.Run("domain", func(t *testing.T) {
 		res, err := eval(t, options, []proto.Message{
@@ -314,7 +315,7 @@ func TestEvaluator(t *testing.T) {
 			},
 		})
 		require.NoError(t, err)
-		assert.True(t, res.Allow)
+		assert.True(t, res.Allow.Value)
 	})
 	t.Run("impersonate domain", func(t *testing.T) {
 		res, err := eval(t, options, []proto.Message{
@@ -343,7 +344,7 @@ func TestEvaluator(t *testing.T) {
 			},
 		})
 		require.NoError(t, err)
-		assert.True(t, res.Allow)
+		assert.True(t, res.Allow.Value)
 	})
 	t.Run("groups", func(t *testing.T) {
 		res, err := eval(t, options, []proto.Message{
@@ -376,7 +377,7 @@ func TestEvaluator(t *testing.T) {
 			},
 		})
 		require.NoError(t, err)
-		assert.True(t, res.Allow)
+		assert.True(t, res.Allow.Value)
 	})
 	t.Run("any authenticated user", func(t *testing.T) {
 		res, err := eval(t, options, []proto.Message{
@@ -399,7 +400,7 @@ func TestEvaluator(t *testing.T) {
 			},
 		})
 		require.NoError(t, err)
-		assert.True(t, res.Allow)
+		assert.True(t, res.Allow.Value)
 	})
 	t.Run("carry over assertion header", func(t *testing.T) {
 		tcs := []struct {

authorize/evaluator/policy_evaluator.go

@@ -3,8 +3,6 @@ package evaluator
 import (
 	"context"
 	"fmt"
-	"net/http"
-	"strconv"
 	"strings"
 
 	"github.com/open-policy-agent/opa/rego"
@@ -15,6 +13,7 @@ import (
 	"github.com/pomerium/pomerium/internal/telemetry/trace"
 	"github.com/pomerium/pomerium/pkg/cryptutil"
 	"github.com/pomerium/pomerium/pkg/policy"
+	"github.com/pomerium/pomerium/pkg/policy/criteria"
 )
 
 // PolicyRequest is the input to policy evaluation.
@@ -26,29 +25,49 @@ type PolicyRequest struct {
 
 // PolicyResponse is the result of evaluating a policy.
 type PolicyResponse struct {
-	Allow bool
-	Deny  *Denial
+	Allow, Deny RuleResult
 }
 
-// Merge merges another PolicyResponse into this PolicyResponse. Access is allowed if either is allowed. Access is denied if
-// either is denied. (and denials take precedence)
-func (res *PolicyResponse) Merge(other *PolicyResponse) *PolicyResponse {
-	merged := &PolicyResponse{
-		Allow: res.Allow || other.Allow,
-		Deny:  res.Deny,
+// A RuleResult is the result of evaluating a rule.
+type RuleResult struct {
+	Value   bool
+	Reasons criteria.Reasons
+}
+
+// NewRuleResult creates a new RuleResult.
+func NewRuleResult(value bool, reasons ...criteria.Reason) RuleResult {
+	return RuleResult{
+		Value:   value,
+		Reasons: criteria.NewReasons(reasons...),
 	}
-	if other.Deny != nil {
-		merged.Deny = other.Deny
+}
+
+// MergeRuleResultsWithOr merges all the results using `or`.
+func MergeRuleResultsWithOr(results ...RuleResult) (merged RuleResult) {
+	var trueResults, falseResults []RuleResult
+	for _, result := range results {
+		if result.Value {
+			trueResults = append(trueResults, result)
+		} else {
+			falseResults = append(falseResults, result)
+		}
 	}
+
+	if len(trueResults) > 0 {
+		merged.Value = true
+		for _, result := range trueResults {
+			merged.Reasons = merged.Reasons.Union(result.Reasons)
+		}
+	} else {
+		merged.Value = false
+		for _, result := range falseResults {
+			merged.Reasons = merged.Reasons.Union(result.Reasons)
+		}
+	}
+
 	return merged
 }
 
-// A Denial indicates the request should be denied (even if otherwise allowed).
-type Denial struct {
-	Status  int
-	Message string
-}
-
 type policyQuery struct {
 	rego.PreparedEvalQuery
 	checksum string
@@ -133,7 +152,8 @@ func (e *PolicyEvaluator) Evaluate(ctx context.Context, req *PolicyRequest) (*Po
 		if err != nil {
 			return nil, err
 		}
-		res = res.Merge(o)
+		res.Allow = MergeRuleResultsWithOr(res.Allow, o.Allow)
+		res.Deny = MergeRuleResultsWithOr(res.Deny, o.Deny)
 	}
 	return res, nil
 }
@@ -153,67 +173,45 @@ func (e *PolicyEvaluator) evaluateQuery(ctx context.Context, req *PolicyRequest,
 	}
 
 	res := &PolicyResponse{
-		Allow: e.getAllow(rs[0].Bindings),
-		Deny:  e.getDeny(ctx, rs[0].Bindings),
+		Allow: e.getRuleResult("allow", rs[0].Bindings),
+		Deny:  e.getRuleResult("deny", rs[0].Bindings),
 	}
 	return res, nil
 }
 
-// getAllow gets the allow var. It expects a boolean.
-func (e *PolicyEvaluator) getAllow(vars rego.Vars) bool {
+// getRuleResult gets the rule result var. It expects a boolean or [boolean, []string].
+func (e *PolicyEvaluator) getRuleResult(name string, vars rego.Vars) (result RuleResult) {
+	result = NewRuleResult(false)
+
 	m, ok := vars["result"].(map[string]interface{})
 	if !ok {
-		return false
+		return result
 	}
 
-	allow, ok := m["allow"].(bool)
-	if !ok {
-		return false
-	}
-
-	return allow
-}
-
-// getDeny gets the deny var. It expects an (http status code, message) pair.
-func (e *PolicyEvaluator) getDeny(ctx context.Context, vars rego.Vars) *Denial {
-	m, ok := vars["result"].(map[string]interface{})
-	if !ok {
-		return nil
-	}
-
-	var status int
-	var reason string
-	switch t := m["deny"].(type) {
+	switch t := m[name].(type) {
 	case bool:
-		if t {
-			status = http.StatusForbidden
-			reason = ""
-		} else {
-			return nil
-		}
+		result.Value = t
 	case []interface{}:
 		switch len(t) {
-		case 0:
-			return nil
 		case 2:
-			var err error
-			status, err = strconv.Atoi(fmt.Sprint(t[0]))
-			if err != nil {
-				log.Error(ctx).Err(err).Msg("invalid type in deny")
-				return nil
+			// fill in the reasons
+			v, ok := t[1].([]interface{})
+			if !ok {
+				return result
 			}
-			reason = fmt.Sprint(t[1])
-		default:
-			log.Error(ctx).Interface("deny", t).Msg("invalid size in deny")
-			return nil
-
+			for _, vv := range v {
+				result.Reasons.Add(criteria.Reason(fmt.Sprint(vv)))
+			}
+			fallthrough
+		case 1:
+			// fill in the value
+			v, ok := t[0].(bool)
+			if !ok {
+				return result
+			}
+			result.Value = v
 		}
-	default:
-		return nil
 	}
 
-	return &Denial{
-		Status:  status,
-		Message: reason,
-	}
+	return result
 }

authorize/evaluator/policy_evaluator_test.go

@@ -3,7 +3,6 @@ package evaluator
 import (
 	"context"
 	"math"
-	"net/http"
 	"strings"
 	"testing"
 
@@ -16,6 +15,7 @@ import (
 	"github.com/pomerium/pomerium/pkg/grpc/session"
 	"github.com/pomerium/pomerium/pkg/grpc/user"
 	"github.com/pomerium/pomerium/pkg/policy"
+	"github.com/pomerium/pomerium/pkg/policy/criteria"
 )
 
 func TestPolicyEvaluator(t *testing.T) {
@@ -70,7 +70,8 @@ func TestPolicyEvaluator(t *testing.T) {
 			})
 		require.NoError(t, err)
 		assert.Equal(t, &PolicyResponse{
-			Allow: true,
+			Allow: NewRuleResult(true, criteria.ReasonEmailOK),
+			Deny:  NewRuleResult(false, criteria.ReasonValidClientCertificateOrNoneRequired),
 		}, output)
 	})
 	t.Run("invalid cert", func(t *testing.T) {
@@ -85,11 +86,8 @@ func TestPolicyEvaluator(t *testing.T) {
 			})
 		require.NoError(t, err)
 		assert.Equal(t, &PolicyResponse{
-			Allow: true,
-			Deny: &Denial{
-				Status:  495,
-				Message: "invalid client certificate",
-			},
+			Allow: NewRuleResult(true, criteria.ReasonEmailOK),
+			Deny:  NewRuleResult(true, criteria.ReasonInvalidClientCertificate),
 		}, output)
 	})
 	t.Run("forbidden", func(t *testing.T) {
@@ -104,7 +102,8 @@ func TestPolicyEvaluator(t *testing.T) {
 			})
 		require.NoError(t, err)
 		assert.Equal(t, &PolicyResponse{
-			Allow: false,
+			Allow: NewRuleResult(false, criteria.ReasonEmailUnauthorized, criteria.ReasonNonPomeriumRoute, criteria.ReasonUserUnauthorized),
+			Deny:  NewRuleResult(false, criteria.ReasonValidClientCertificateOrNoneRequired),
 		}, output)
 	})
 	t.Run("ppl", func(t *testing.T) {
@@ -133,7 +132,8 @@ func TestPolicyEvaluator(t *testing.T) {
 				})
 			require.NoError(t, err)
 			assert.Equal(t, &PolicyResponse{
-				Allow: true,
+				Allow: NewRuleResult(true, criteria.ReasonAccept),
+				Deny:  NewRuleResult(false, criteria.ReasonValidClientCertificateOrNoneRequired),
 			}, output)
 		})
 		t.Run("deny", func(t *testing.T) {
@@ -161,9 +161,8 @@ func TestPolicyEvaluator(t *testing.T) {
 				})
 			require.NoError(t, err)
 			assert.Equal(t, &PolicyResponse{
-				Deny: &Denial{
-					Status: http.StatusForbidden,
-				},
+				Allow: NewRuleResult(false, criteria.ReasonNonPomeriumRoute),
+				Deny:  NewRuleResult(true, criteria.ReasonAccept),
 			}, output)
 		})
 		t.Run("client certificate", func(t *testing.T) {
@@ -192,10 +191,8 @@ func TestPolicyEvaluator(t *testing.T) {
 				})
 			require.NoError(t, err)
 			assert.Equal(t, &PolicyResponse{
-				Deny: &Denial{
-					Status:  495,
-					Message: "invalid client certificate",
-				},
+				Allow: NewRuleResult(false, criteria.ReasonNonPomeriumRoute),
+				Deny:  NewRuleResult(true, criteria.ReasonAccept, criteria.ReasonInvalidClientCertificate),
 			}, output)
 		})
 	})