From 7d28a91c1da189f0957754626539269323fa3dfd Mon Sep 17 00:00:00 2001 From: JanardhanSharma Date: Thu, 1 Aug 2024 10:41:10 +0200 Subject: [PATCH] Facilitate OPA decision correlation with business flows (#3160) Signed-off-by: JanardhanSharma --- docs/tutorials/auth.md | 8 +++ filters/openpolicyagent/evaluation.go | 34 ++++++++++- .../opaauthorizerequest_test.go | 56 +++++++++++++++++-- filters/openpolicyagent/openpolicyagent.go | 6 +- .../openpolicyagent/openpolicyagent_test.go | 18 ++++-- 5 files changed, 108 insertions(+), 14 deletions(-) diff --git a/docs/tutorials/auth.md b/docs/tutorials/auth.md index 14bf04e1f7..b8db9b7135 100644 --- a/docs/tutorials/auth.md +++ b/docs/tutorials/auth.md @@ -466,6 +466,14 @@ The second argument is parsed as YAML, cannot be nested and values need to be st In Rego this can be used like this `input.attributes.contextExtensions["com.mycompany.myprop"] == "my value"` +### Decision ID in Policies + +Each evaluation yields a distinct decision, identifiable by its unique decision ID. +This decision ID can be located within the input at: +`input.attributes.metadataContext.filterMetadata.open_policy_agent.decision_id` +Typical use cases are either propagation of the decision ID to downstream systems or returning it as part of the response. As an example this can allow to trouble shoot deny requests by looking up details using the full decision in a control plane. + + ### Quick Start Rego Playground A quick way without setting up Backend APIs is to use the [Rego Playground](https://play.openpolicyagent.org/). diff --git a/filters/openpolicyagent/evaluation.go b/filters/openpolicyagent/evaluation.go index 03f56b450a..2f38958617 100644 --- a/filters/openpolicyagent/evaluation.go +++ b/filters/openpolicyagent/evaluation.go @@ -3,8 +3,7 @@ package openpolicyagent import ( "context" "fmt" - "time" - + ext_authz_v3_core "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" ext_authz_v3 "github.com/envoyproxy/go-control-plane/envoy/service/auth/v3" "github.com/open-policy-agent/opa-envoy-plugin/envoyauth" "github.com/open-policy-agent/opa-envoy-plugin/opa/decisionlog" @@ -13,6 +12,8 @@ import ( "github.com/open-policy-agent/opa/server" "github.com/open-policy-agent/opa/topdown" "github.com/opentracing/opentracing-go" + pbstruct "google.golang.org/protobuf/types/known/structpb" + "time" ) func (opa *OpenPolicyAgentInstance) Eval(ctx context.Context, req *ext_authz_v3.CheckRequest) (*envoyauth.EvalResult, error) { @@ -23,6 +24,12 @@ func (opa *OpenPolicyAgentInstance) Eval(ctx context.Context, req *ext_authz_v3. return nil, err } + err = setDecisionIdInRequest(req, decisionId) + if err != nil { + opa.Logger().WithFields(map[string]interface{}{"err": err}).Error("Unable to set decision ID in Request.") + return nil, err + } + result, stopeval, err := envoyauth.NewEvalResult(withDecisionID(decisionId)) if err != nil { opa.Logger().WithFields(map[string]interface{}{"err": err}).Error("Unable to generate new result with decision ID.") @@ -71,6 +78,29 @@ func (opa *OpenPolicyAgentInstance) Eval(ctx context.Context, req *ext_authz_v3. return result, nil } +func setDecisionIdInRequest(req *ext_authz_v3.CheckRequest, decisionId string) error { + if req.Attributes.MetadataContext == nil { + req.Attributes.MetadataContext = &ext_authz_v3_core.Metadata{ + FilterMetadata: map[string]*pbstruct.Struct{}, + } + } + + filterMeta, err := FormOpenPolicyAgentMetaDataObject(decisionId) + if err != nil { + return err + } + req.Attributes.MetadataContext.FilterMetadata["open_policy_agent"] = filterMeta + return nil +} + +func FormOpenPolicyAgentMetaDataObject(decisionId string) (*pbstruct.Struct, error) { + + innerFields := make(map[string]interface{}) + innerFields["decision_id"] = decisionId + + return pbstruct.NewStruct(innerFields) +} + func (opa *OpenPolicyAgentInstance) logDecision(ctx context.Context, input interface{}, result *envoyauth.EvalResult, err error) error { info := &server.Info{ Timestamp: time.Now(), diff --git a/filters/openpolicyagent/opaauthorizerequest/opaauthorizerequest_test.go b/filters/openpolicyagent/opaauthorizerequest/opaauthorizerequest_test.go index 1e48c63c8f..ab7616086c 100644 --- a/filters/openpolicyagent/opaauthorizerequest/opaauthorizerequest_test.go +++ b/filters/openpolicyagent/opaauthorizerequest/opaauthorizerequest_test.go @@ -255,6 +255,21 @@ func TestAuthorizeRequestFilter(t *testing.T) { backendHeaders: make(http.Header), removeHeaders: make(http.Header), }, + { + msg: "Decision id in request header", + filterName: "opaAuthorizeRequest", + bundleName: "somebundle.tar.gz", + regoQuery: "envoy/authz/allow_object_decision_id_in_header", + requestMethod: "POST", + body: `{ "target_id" : "123456" }`, + requestHeaders: map[string][]string{"content-type": {"application/json"}}, + requestPath: "/allow/structured", + expectedStatus: http.StatusOK, + expectedBody: "Welcome!", + expectedHeaders: map[string][]string{"Decision-Id": {"some-random-decision-id-generated-during-evaluation"}}, + backendHeaders: make(http.Header), + removeHeaders: make(http.Header), + }, { msg: "Invalid UTF-8 in Path", filterName: "opaAuthorizeRequest", @@ -345,8 +360,21 @@ func TestAuthorizeRequestFilter(t *testing.T) { allow_body { input.parsed_body.target_id == "123456" - } - `, + } + + decision_id := input.attributes.metadataContext.filterMetadata.open_policy_agent.decision_id + + allow_object_decision_id_in_header = response { + input.parsed_path = ["allow", "structured"] + decision_id + response := { + "allowed": true, + "response_headers_to_add": { + "decision-id": decision_id + } + } + } + `, }), ) @@ -374,10 +402,23 @@ func TestAuthorizeRequestFilter(t *testing.T) { } }`, opaControlPlane.URL(), ti.regoQuery)) + envoyMetaDataConfig := []byte(`{ + "filter_metadata": { + "envoy.filters.http.header_to_metadata": { + "policy_type": "ingress" + } + } + }`) + + opts := make([]func(*openpolicyagent.OpenPolicyAgentInstanceConfig) error, 0) + opts = append(opts, + openpolicyagent.WithConfigTemplate(config), + openpolicyagent.WithEnvoyMetadataBytes(envoyMetaDataConfig)) + opaFactory := openpolicyagent.NewOpenPolicyAgentRegistry(openpolicyagent.WithTracer(&tracingtest.Tracer{})) - ftSpec := NewOpaAuthorizeRequestSpec(opaFactory, openpolicyagent.WithConfigTemplate(config)) + ftSpec := NewOpaAuthorizeRequestSpec(opaFactory, opts...) fr.Register(ftSpec) - ftSpec = NewOpaAuthorizeRequestWithBodySpec(opaFactory, openpolicyagent.WithConfigTemplate(config)) + ftSpec = NewOpaAuthorizeRequestWithBodySpec(opaFactory, opts...) fr.Register(ftSpec) r := eskip.MustParse(fmt.Sprintf(`* -> %s("%s", "%s") %s -> "%s"`, ti.filterName, ti.bundleName, ti.contextExtensions, ti.extraeskip, clientServer.URL)) @@ -436,7 +477,12 @@ func isHeadersPresent(t *testing.T, expectedHeaders http.Header, headers http.He return false } - assert.ElementsMatch(t, expectedValues, actualValues) + // since decision id is randomly generated we are just checking for not nil + if headerName == "Decision-Id" { + assert.NotNil(t, actualValues) + } else { + assert.ElementsMatch(t, expectedValues, actualValues) + } } return true } diff --git a/filters/openpolicyagent/openpolicyagent.go b/filters/openpolicyagent/openpolicyagent.go index 3144c6c2ea..04a8845f63 100644 --- a/filters/openpolicyagent/openpolicyagent.go +++ b/filters/openpolicyagent/openpolicyagent.go @@ -5,6 +5,7 @@ import ( "context" "errors" "fmt" + "google.golang.org/protobuf/proto" "io" "net/http" "os" @@ -205,7 +206,10 @@ func WithStartupTimeout(timeout time.Duration) func(*OpenPolicyAgentInstanceConf } func (cfg *OpenPolicyAgentInstanceConfig) GetEnvoyMetadata() *ext_authz_v3_core.Metadata { - return cfg.envoyMetadata + if cfg.envoyMetadata != nil { + return proto.Clone(cfg.envoyMetadata).(*ext_authz_v3_core.Metadata) + } + return nil } func NewOpenPolicyAgentConfig(opts ...func(*OpenPolicyAgentInstanceConfig) error) (*OpenPolicyAgentInstanceConfig, error) { diff --git a/filters/openpolicyagent/openpolicyagent_test.go b/filters/openpolicyagent/openpolicyagent_test.go index 117e8522bb..6e15c859eb 100644 --- a/filters/openpolicyagent/openpolicyagent_test.go +++ b/filters/openpolicyagent/openpolicyagent_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "fmt" + pbstruct "google.golang.org/protobuf/types/known/structpb" "io" "net/http" "os" @@ -29,7 +30,6 @@ import ( "github.com/zalando/skipper/routing" "github.com/zalando/skipper/tracing/tracingtest" "google.golang.org/protobuf/encoding/protojson" - _struct "google.golang.org/protobuf/types/known/structpb" ) type MockOpenPolicyAgentFilter struct { @@ -63,7 +63,7 @@ func TestInterpolateTemplate(t *testing.T) { func TestLoadEnvoyMetadata(t *testing.T) { cfg := &OpenPolicyAgentInstanceConfig{} - WithEnvoyMetadataBytes([]byte(` + _ = WithEnvoyMetadataBytes([]byte(` { "filter_metadata": { "envoy.filters.http.header_to_metadata": { @@ -74,11 +74,11 @@ func TestLoadEnvoyMetadata(t *testing.T) { `))(cfg) expectedBytes, err := protojson.Marshal(&ext_authz_v3_core.Metadata{ - FilterMetadata: map[string]*_struct.Struct{ + FilterMetadata: map[string]*pbstruct.Struct{ "envoy.filters.http.header_to_metadata": { - Fields: map[string]*_struct.Value{ + Fields: map[string]*pbstruct.Value{ "policy_type": { - Kind: &_struct.Value_StringValue{StringValue: "ingress"}, + Kind: &pbstruct.Value_StringValue{StringValue: "ingress"}, }, }, }, @@ -411,7 +411,13 @@ func TestEval(t *testing.T) { span := tracer.StartSpan("open-policy-agent") ctx := opentracing.ContextWithSpan(context.Background(), span) - result, err := inst.Eval(ctx, &authv3.CheckRequest{}) + result, err := inst.Eval(ctx, &authv3.CheckRequest{ + Attributes: &authv3.AttributeContext{ + Request: nil, + ContextExtensions: nil, + MetadataContext: nil, + }, + }) assert.NoError(t, err) allowed, err := result.IsAllowed()