-
Notifications
You must be signed in to change notification settings - Fork 24
feat(authz): add protovalidate interceptor for Casbin #2986
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
alkalescent
wants to merge
13
commits into
main
Choose a base branch
from
DSPX-2187-protovalidate
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 3 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
52a390c
protovalidate interceptor
alkalescent 24353fb
improve whitelisting
alkalescent b12e954
casbin integration
alkalescent 4aa9aa3
auth fail check
alkalescent cfb921d
comment
alkalescent 839558c
reuse validator
alkalescent d7b1481
convert to suite
alkalescent 5a57161
move enforcement test
alkalescent 17e77d6
lint
alkalescent e16bcf7
comments
alkalescent 72f0c45
lint
alkalescent af130b9
rename var
alkalescent 9debc04
reduce fx complexity by creating helper fxs
alkalescent File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| package auth | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
|
|
||
| "buf.build/go/protovalidate" | ||
| "connectrpc.com/connect" | ||
| "github.com/lestrrat-go/jwx/v2/jwt" | ||
| "google.golang.org/protobuf/proto" | ||
| "google.golang.org/protobuf/reflect/protoreflect" | ||
| ) | ||
|
|
||
| // ProtoAttrMapper extracts selected proto fields and converts them to | ||
| // casbin-request attributes. Enforces whitelist-only access to ensure | ||
| // ONLY configured fields are available to authorization policies. | ||
| type ProtoAttrMapper struct { | ||
| // Allowed fields to extract and expose to Casbin (whitelist-only) | ||
| Allowed []string | ||
| // RequiredFields that must exist on the request (subset of Allowed) | ||
| RequiredFields []string | ||
| // Validate controls whether to run protovalidate on the incoming message | ||
| Validate bool | ||
| } | ||
|
|
||
| // Interceptor returns a ConnectRPC unary interceptor that validates the | ||
| // request protobuf using protovalidate and attaches a map[string]string of | ||
| // attributes to the context for downstream enforcement. | ||
| func (p *ProtoAttrMapper) Interceptor(e *Enforcer) connect.UnaryInterceptorFunc { | ||
| interceptor := func(next connect.UnaryFunc) connect.UnaryFunc { | ||
| return connect.UnaryFunc(func(ctx context.Context, req connect.AnyRequest) (connect.AnyResponse, error) { | ||
| // Validate proto message using protovalidate if available | ||
| if any := req.Any(); any != nil { | ||
| if m, ok := any.(proto.Message); ok { | ||
| if p.Validate { | ||
| v, err := protovalidate.New() | ||
| if err == nil { | ||
| if err := v.Validate(m); err != nil { | ||
| return nil, connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("protovalidate failed: %w", err)) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Build attributes map - whitelist-only extraction | ||
| attrs := map[string]string{} | ||
| mr := m.ProtoReflect() | ||
| for _, allow := range p.Allowed { | ||
| if val, ok := lookupProtoFieldString(mr, allow); ok { | ||
| attrs[allow] = val | ||
| } | ||
| } | ||
|
|
||
| // Validate required fields are present | ||
| for _, required := range p.RequiredFields { | ||
| if _, exists := attrs[required]; !exists { | ||
| return nil, connect.NewError( | ||
| connect.CodeInvalidArgument, | ||
| fmt.Errorf("required field %q is missing or invalid", required), | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| // Attach attrs to context for downstream (store under key "casbin_attrs") | ||
| // SECURITY: Only whitelisted fields are in this map - no other request | ||
| // fields are accessible to Casbin policy evaluation | ||
| ctx = context.WithValue(ctx, casbinContextKey("casbin_attrs"), attrs) | ||
|
|
||
| // Optionally perform synchronous enforcement: derive resource/action | ||
| if e != nil { | ||
| if tk, ok := ctx.Value(tokenContextKey{}).(jwt.Token); ok { | ||
| res := req.Spec().Procedure | ||
| act := req.Spec().Procedure | ||
| _, _ = e.Enforce(tk, res, act) | ||
alkalescent marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
| } | ||
| } | ||
| return next(ctx, req) | ||
| }) | ||
| } | ||
| return connect.UnaryInterceptorFunc(interceptor) | ||
| } | ||
|
|
||
| // helper to lookup a dot-separated path on a protoreflect.Message and | ||
| // return its string value if present. | ||
alkalescent marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| func lookupProtoFieldString(m protoreflect.Message, path string) (string, bool) { | ||
| // Only support single-level fields for now to keep simple | ||
| fld := m.Descriptor().Fields().ByName(protoreflect.Name(path)) | ||
| if fld == nil { | ||
| return "", false | ||
| } | ||
| v := m.Get(fld) | ||
| if !v.IsValid() { | ||
| return "", false | ||
| } | ||
| // Convert scalar to string if possible | ||
| switch fld.Kind() { | ||
|
Check failure on line 97 in service/internal/auth/protovalidate_interceptor.go
|
||
| case protoreflect.StringKind: | ||
| s := v.String() | ||
| // Treat empty strings as missing for required field validation | ||
| if s == "" { | ||
| return "", false | ||
| } | ||
| return s, true | ||
| case protoreflect.Int32Kind, protoreflect.Int64Kind: | ||
| return fmt.Sprintf("%d", v.Int()), true | ||
| case protoreflect.Uint32Kind, protoreflect.Uint64Kind: | ||
| return fmt.Sprintf("%d", v.Uint()), true | ||
| case protoreflect.BoolKind: | ||
| return fmt.Sprintf("%t", v.Bool()), true | ||
| default: | ||
| return "", false | ||
| } | ||
| } | ||
|
|
||
| // context keys | ||
| type casbinContextKey string | ||
| type tokenContextKey struct{} | ||
|
|
||
| // GetCasbinAttrsFromContext retrieves the extracted proto attributes from the context. | ||
| // Returns the attributes map and true if present, or nil and false if not found. | ||
| func GetCasbinAttrsFromContext(ctx context.Context) (map[string]string, bool) { | ||
| v := ctx.Value(casbinContextKey("casbin_attrs")) | ||
| if v == nil { | ||
| return nil, false | ||
| } | ||
| attrs, ok := v.(map[string]string) | ||
| return attrs, ok | ||
| } | ||
229 changes: 229 additions & 0 deletions
229
service/internal/auth/protovalidate_interceptor_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,229 @@ | ||
| package auth | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| "connectrpc.com/connect" | ||
| "github.com/opentdf/platform/protocol/go/common" | ||
| "github.com/opentdf/platform/service/logger" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func Test_ProtoAttrMapper_Interceptor(t *testing.T) { | ||
| mapper := &ProtoAttrMapper{Allowed: []string{"name", "id"}, Validate: false} | ||
|
|
||
| // create a simple proto message from policy namespace that has string fields | ||
| msg := &common.IdNameIdentifier{ | ||
| Id: "abc", | ||
| Name: "example", | ||
| } | ||
|
|
||
| // create a no-op next handler that checks context for attrs | ||
| next := func(ctx context.Context, req connect.AnyRequest) (connect.AnyResponse, error) { | ||
| v := ctx.Value(casbinContextKey("casbin_attrs")) | ||
| require.NotNil(t, v) | ||
| m, ok := v.(map[string]string) | ||
| require.True(t, ok) | ||
| require.Equal(t, "example", m["name"]) | ||
| require.Equal(t, "abc", m["id"]) | ||
| return connect.NewResponse[any](nil), nil | ||
| } | ||
|
|
||
| interceptor := mapper.Interceptor(nil) | ||
| wrapped := interceptor(next) | ||
|
|
||
| // Build a connect request wrapper | ||
| req := connect.NewRequest(msg) | ||
| _, err := wrapped(context.Background(), req) | ||
| require.NoError(t, err) | ||
| } | ||
|
|
||
| func Test_ProtoAttrMapper_RequiredFields(t *testing.T) { | ||
| t.Run("missing required field should fail", func(t *testing.T) { | ||
| mapper := &ProtoAttrMapper{ | ||
| Allowed: []string{"name", "id"}, | ||
| RequiredFields: []string{"name", "id"}, | ||
| Validate: false, | ||
| } | ||
|
|
||
| // Message missing 'name' field (empty string) | ||
| msg := &common.IdNameIdentifier{ | ||
| Id: "abc", | ||
| Name: "", // empty/missing | ||
| } | ||
|
|
||
| next := func(ctx context.Context, req connect.AnyRequest) (connect.AnyResponse, error) { | ||
| t.Fatal("should not reach next handler") | ||
| return connect.NewResponse[any](nil), nil | ||
| } | ||
|
|
||
| interceptor := mapper.Interceptor(nil) | ||
| wrapped := interceptor(next) | ||
|
|
||
| req := connect.NewRequest(msg) | ||
| _, err := wrapped(context.Background(), req) | ||
| require.Error(t, err) | ||
| require.Contains(t, err.Error(), "required field") | ||
| require.Contains(t, err.Error(), "name") | ||
| }) | ||
|
|
||
| t.Run("all required fields present should succeed", func(t *testing.T) { | ||
| mapper := &ProtoAttrMapper{ | ||
| Allowed: []string{"name", "id"}, | ||
| RequiredFields: []string{"name"}, | ||
| Validate: false, | ||
| } | ||
|
|
||
| msg := &common.IdNameIdentifier{ | ||
| Id: "abc", | ||
| Name: "example", | ||
| } | ||
|
|
||
| next := func(ctx context.Context, req connect.AnyRequest) (connect.AnyResponse, error) { | ||
| v := ctx.Value(casbinContextKey("casbin_attrs")) | ||
| require.NotNil(t, v) | ||
| return connect.NewResponse[any](nil), nil | ||
| } | ||
|
|
||
| interceptor := mapper.Interceptor(nil) | ||
| wrapped := interceptor(next) | ||
|
|
||
| req := connect.NewRequest(msg) | ||
| _, err := wrapped(context.Background(), req) | ||
| require.NoError(t, err) | ||
| }) | ||
| } | ||
|
|
||
| func Test_ProtoAttrMapper_WhitelistOnly(t *testing.T) { | ||
| t.Run("only whitelisted fields should be in attrs", func(t *testing.T) { | ||
| // Only allow 'name', not 'id' | ||
| mapper := &ProtoAttrMapper{ | ||
| Allowed: []string{"name"}, | ||
| Validate: false, | ||
| } | ||
|
|
||
| msg := &common.IdNameIdentifier{ | ||
| Id: "secret-id-should-not-be-exposed", | ||
| Name: "example", | ||
| } | ||
|
|
||
| next := func(ctx context.Context, req connect.AnyRequest) (connect.AnyResponse, error) { | ||
| v := ctx.Value(casbinContextKey("casbin_attrs")) | ||
| require.NotNil(t, v) | ||
| m, ok := v.(map[string]string) | ||
| require.True(t, ok) | ||
|
|
||
| // SECURITY TEST: only 'name' should be present | ||
| require.Equal(t, "example", m["name"]) | ||
| require.NotContains(t, m, "id", "id should NOT be in attrs - security violation") | ||
| require.Len(t, m, 1, "only whitelisted fields should be present") | ||
| return connect.NewResponse[any](nil), nil | ||
| } | ||
|
|
||
| interceptor := mapper.Interceptor(nil) | ||
| wrapped := interceptor(next) | ||
|
|
||
| req := connect.NewRequest(msg) | ||
| _, err := wrapped(context.Background(), req) | ||
| require.NoError(t, err) | ||
| }) | ||
| } | ||
|
|
||
| func Test_ProtoAttrMapper_EnforcementIntegration(t *testing.T) { | ||
| t.Run("enforcement with attribute-based policy", func(t *testing.T) { | ||
| // Create a Casbin enforcer with an attribute-aware model | ||
| modelConf := ` | ||
| [request_definition] | ||
| r = sub, res, act, owner | ||
|
|
||
| [policy_definition] | ||
| p = sub, res, act, owner, eft | ||
|
|
||
| [role_definition] | ||
| g = _, _ | ||
|
|
||
| [policy_effect] | ||
| e = some(where (p.eft == allow)) && !some(where (p.eft == deny)) | ||
|
|
||
| [matchers] | ||
| m = g(r.sub, p.sub) && keyMatch(r.res, p.res) && keyMatch(r.act, p.act) && (p.owner == "*" || r.owner == p.owner) | ||
| ` | ||
|
|
||
| policyCSV := ` | ||
| p, role:admin, /policy/*, read, *, allow | ||
| p, role:user, /policy/*, read, user123, allow | ||
| g, admin-user, role:admin | ||
| g, regular-user, role:user | ||
| ` | ||
| loggerInstance, err := logger.NewLogger(logger.Config{ | ||
| Level: "error", | ||
| Output: "stdout", | ||
| Type: "json", | ||
| }) | ||
| require.NoError(t, err) | ||
| require.NotNil(t, loggerInstance) | ||
|
|
||
| casbinCfg := CasbinConfig{ | ||
| PolicyConfig: PolicyConfig{ | ||
| Model: modelConf, | ||
| Csv: policyCSV, | ||
| }, | ||
| } | ||
| enforcer, err := NewCasbinEnforcer(casbinCfg, loggerInstance) | ||
| require.NoError(t, err) | ||
| require.NotNil(t, enforcer) | ||
|
|
||
| // Test 1: Admin can access any resource | ||
| allowed, err := enforcer.Enforcer.Enforce("role:admin", "/policy/attributes", "read", "*") | ||
| require.NoError(t, err) | ||
| require.True(t, allowed, "admin should have access") | ||
|
|
||
| // Test 2: User can only access their own resources | ||
| allowed, err = enforcer.Enforcer.Enforce("role:user", "/policy/attributes", "read", "user123") | ||
| require.NoError(t, err) | ||
| require.True(t, allowed, "user should have access to their own resource") | ||
|
|
||
| // Test 3: User cannot access other user's resources | ||
| allowed, err = enforcer.Enforcer.Enforce("role:user", "/policy/attributes", "read", "user456") | ||
| require.NoError(t, err) | ||
| require.False(t, allowed, "user should NOT have access to other user's resource") | ||
|
|
||
| t.Log("Attribute-based enforcement working correctly") | ||
| }) | ||
alkalescent marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| t.Run("interceptor extracts attrs for enforcement", func(t *testing.T) { | ||
| mapper := &ProtoAttrMapper{ | ||
| Allowed: []string{"name", "id"}, | ||
| RequiredFields: []string{"id"}, | ||
| Validate: false, | ||
| } | ||
|
|
||
| msg := &common.IdNameIdentifier{ | ||
| Id: "user123", | ||
| Name: "test-resource", | ||
| } | ||
|
|
||
| next := func(ctx context.Context, req connect.AnyRequest) (connect.AnyResponse, error) { | ||
| v := ctx.Value(casbinContextKey("casbin_attrs")) | ||
| require.NotNil(t, v) | ||
| attrs, ok := v.(map[string]string) | ||
| require.True(t, ok) | ||
|
|
||
| // Verify extracted attributes are ready for enforcement | ||
| require.Equal(t, "user123", attrs["id"]) | ||
| require.Equal(t, "test-resource", attrs["name"]) | ||
|
|
||
| // These attrs can now be passed to Casbin Enforce with extended signature | ||
| // e.g., enforcer.Enforce(subject, resource, action, attrs["id"]) | ||
| return connect.NewResponse[any](nil), nil | ||
| } | ||
|
|
||
| interceptor := mapper.Interceptor(nil) | ||
| wrapped := interceptor(next) | ||
|
|
||
| req := connect.NewRequest(msg) | ||
| _, err := wrapped(context.Background(), req) | ||
| require.NoError(t, err) | ||
| }) | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.