-
Notifications
You must be signed in to change notification settings - Fork 33
feat(ci): Enhance authorization bdd test coverage #3298
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
base: main
Are you sure you want to change the base?
Changes from all commits
7094768
51273df
5d1dfd1
3441f0d
194bf75
3ca7554
26740fd
6278ba4
d26824c
3af77a2
fc917bf
d2fed66
2d17070
e03b0e5
7ef6a6e
6ee91c4
6a859db
eb7dd89
23a877a
1fa8804
15b110f
4743bc1
ca09e51
a9915b3
49eec3f
287c07b
3f4723e
fb9554b
63342df
5e8b6bc
5078bd2
f1bfac9
1a48887
44af78f
0f57432
58abc34
fbc75db
5889055
5eb0db3
ef57189
8688af4
81b6662
cf10241
57b785c
1f10165
9cb258e
1384ee5
5510d3c
7373b32
79e3a29
f026dbe
7612d4e
c89d3e4
6eb73e0
a6fe877
dcd6aea
4adac6f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,11 +2,15 @@ package claims | |
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "fmt" | ||
| "log" | ||
| "log/slog" | ||
| "strconv" | ||
| "strings" | ||
|
|
||
| "connectrpc.com/connect" | ||
| "github.com/go-viper/mapstructure/v2" | ||
| "github.com/lestrrat-go/jwx/v2/jwt" | ||
| "github.com/opentdf/platform/protocol/go/entity" | ||
| entityresolutionV2 "github.com/opentdf/platform/protocol/go/entityresolution/v2" | ||
|
|
@@ -22,17 +26,30 @@ import ( | |
|
|
||
| type EntityResolutionServiceV2 struct { | ||
| entityresolutionV2.UnimplementedEntityResolutionServiceServer | ||
| logger *logger.Logger | ||
| logger *logger.Logger | ||
| allowDirectEntitlements bool | ||
| trace.Tracer | ||
| } | ||
|
|
||
| func RegisterClaimsERS(_ config.ServiceConfig, logger *logger.Logger) (EntityResolutionServiceV2, serviceregistry.HandlerServer) { | ||
| claimsSVC := EntityResolutionServiceV2{logger: logger} | ||
| type Config struct { | ||
| AllowDirectEntitlements bool `mapstructure:"allow_direct_entitlements" json:"allow_direct_entitlements" default:"false"` | ||
| } | ||
|
|
||
| func RegisterClaimsERS(cfg config.ServiceConfig, logger *logger.Logger) (EntityResolutionServiceV2, serviceregistry.HandlerServer) { | ||
| var inputConfig Config | ||
| if err := mapstructure.Decode(cfg, &inputConfig); err != nil { | ||
| logger.Error("failed to decode claims entity resolution configuration", slog.Any("error", err)) | ||
| log.Fatalf("Failed to decode claims entity resolution configuration: %v", err) | ||
| } | ||
| claimsSVC := EntityResolutionServiceV2{ | ||
| logger: logger, | ||
| allowDirectEntitlements: inputConfig.AllowDirectEntitlements, | ||
| } | ||
| return claimsSVC, nil | ||
| } | ||
|
|
||
| func (s EntityResolutionServiceV2) ResolveEntities(ctx context.Context, req *connect.Request[entityresolutionV2.ResolveEntitiesRequest]) (*connect.Response[entityresolutionV2.ResolveEntitiesResponse], error) { | ||
| resp, err := EntityResolution(ctx, req.Msg, s.logger) | ||
| resp, err := EntityResolution(ctx, req.Msg, s.logger, s.allowDirectEntitlements) | ||
| return connect.NewResponse(&resp), err | ||
| } | ||
|
|
||
|
|
@@ -63,13 +80,14 @@ func CreateEntityChainsFromTokens( | |
| } | ||
|
|
||
| func EntityResolution(_ context.Context, | ||
| req *entityresolutionV2.ResolveEntitiesRequest, logger *logger.Logger, | ||
| req *entityresolutionV2.ResolveEntitiesRequest, logger *logger.Logger, allowDirectEntitlements bool, | ||
| ) (entityresolutionV2.ResolveEntitiesResponse, error) { | ||
| payload := req.GetEntities() | ||
| var resolvedEntities []*entityresolutionV2.EntityRepresentation | ||
|
|
||
| for idx, ident := range payload { | ||
| entityStruct := &structpb.Struct{} | ||
| var directEntitlements []*entityresolutionV2.DirectEntitlement | ||
| switch ident.GetEntityType().(type) { | ||
| case *entity.Entity_Claims: | ||
| claims := ident.GetClaims() | ||
|
|
@@ -79,6 +97,13 @@ func EntityResolution(_ context.Context, | |
| return entityresolutionV2.ResolveEntitiesResponse{}, connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("error unpacking anypb.Any to structpb.Struct: %w", err)) | ||
| } | ||
| } | ||
| if allowDirectEntitlements { | ||
| var err error | ||
| directEntitlements, err = parseDirectEntitlementsFromClaims(entityStruct) | ||
| if err != nil { | ||
| return entityresolutionV2.ResolveEntitiesResponse{}, connect.NewError(connect.CodeInvalidArgument, err) | ||
| } | ||
| } | ||
| default: | ||
| retrievedStruct, err := entityToStructPb(ident) | ||
| if err != nil { | ||
|
|
@@ -95,8 +120,9 @@ func EntityResolution(_ context.Context, | |
| resolvedEntities = append( | ||
| resolvedEntities, | ||
| &entityresolutionV2.EntityRepresentation{ | ||
| OriginalId: originialID, | ||
| AdditionalProps: []*structpb.Struct{entityStruct}, | ||
| OriginalId: originialID, | ||
| AdditionalProps: []*structpb.Struct{entityStruct}, | ||
| DirectEntitlements: directEntitlements, | ||
| }, | ||
| ) | ||
| } | ||
|
|
@@ -164,3 +190,109 @@ func entityToStructPb(ident *entity.Entity) (*structpb.Struct, error) { | |
| } | ||
| return &entityStruct, nil | ||
| } | ||
|
|
||
| func parseDirectEntitlementsFromClaims(entityStruct *structpb.Struct) ([]*entityresolutionV2.DirectEntitlement, error) { | ||
| if entityStruct == nil { | ||
| return nil, nil | ||
| } | ||
| claims := entityStruct.AsMap() | ||
| rawEntitlements, ok := claims["direct_entitlements"] | ||
| if !ok { | ||
| rawEntitlements, ok = claims["directEntitlements"] | ||
| } | ||
| if !ok { | ||
| return nil, nil | ||
| } | ||
|
|
||
| entitlementList, entitlementsOK := rawEntitlements.([]interface{}) | ||
| if !entitlementsOK { | ||
| return nil, errors.New("direct_entitlements must be an array") | ||
| } | ||
|
|
||
| out := make([]*entityresolutionV2.DirectEntitlement, 0, len(entitlementList)) | ||
| for idx, entry := range entitlementList { | ||
| entryMap, entryOK := entry.(map[string]interface{}) | ||
| if !entryOK { | ||
| return nil, fmt.Errorf("direct_entitlements[%d] must be an object", idx) | ||
| } | ||
|
|
||
| fqn, err := parseDirectEntitlementFQN(entryMap) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("direct_entitlements[%d] %w", idx, err) | ||
| } | ||
|
|
||
| rawActions, actionsOK := entryMap["actions"] | ||
| if !actionsOK { | ||
| return nil, fmt.Errorf("direct_entitlements[%d] missing actions", idx) | ||
| } | ||
| actions, err := parseDirectEntitlementActions(rawActions) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("direct_entitlements[%d] invalid actions: %w", idx, err) | ||
| } | ||
|
|
||
| out = append(out, &entityresolutionV2.DirectEntitlement{ | ||
| AttributeValueFqn: fqn, | ||
| Actions: actions, | ||
| }) | ||
| } | ||
|
|
||
| return out, nil | ||
| } | ||
|
|
||
| func parseDirectEntitlementFQN(entry map[string]interface{}) (string, error) { | ||
| if raw, ok := entry["attribute_value_fqn"]; ok { | ||
| if fqn, fqnOK := raw.(string); fqnOK { | ||
| fqn = strings.TrimSpace(fqn) | ||
| if fqn != "" { | ||
| return fqn, nil | ||
| } | ||
| } | ||
| } | ||
| if raw, ok := entry["attributeValueFqn"]; ok { | ||
| if fqn, fqnOK := raw.(string); fqnOK { | ||
| fqn = strings.TrimSpace(fqn) | ||
| if fqn != "" { | ||
| return fqn, nil | ||
| } | ||
| } | ||
| } | ||
| return "", errors.New("missing attribute_value_fqn") | ||
| } | ||
|
|
||
| func parseDirectEntitlementActions(raw interface{}) ([]string, error) { | ||
| actions := make([]string, 0) | ||
| switch typed := raw.(type) { | ||
| case []interface{}: | ||
| for _, action := range typed { | ||
| actionStr, ok := action.(string) | ||
| if !ok { | ||
| return nil, errors.New("action must be a string") | ||
| } | ||
| actionStr = strings.TrimSpace(strings.ToLower(actionStr)) | ||
| if actionStr != "" { | ||
| actions = append(actions, actionStr) | ||
| } | ||
| } | ||
| case []string: | ||
| for _, action := range typed { | ||
| action = strings.TrimSpace(strings.ToLower(action)) | ||
| if action != "" { | ||
| actions = append(actions, action) | ||
| } | ||
| } | ||
| case string: | ||
| for _, action := range strings.Split(typed, ",") { | ||
| action = strings.TrimSpace(strings.ToLower(action)) | ||
| if action != "" { | ||
| actions = append(actions, action) | ||
| } | ||
| } | ||
| default: | ||
| return nil, errors.New("actions must be an array or string") | ||
| } | ||
|
|
||
| if len(actions) == 0 { | ||
| return nil, errors.New("no actions provided") | ||
| } | ||
| return actions, nil | ||
| } | ||
|
Comment on lines
+262
to
+298
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Minor: the
🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,7 +22,7 @@ func Test_ClientResolveEntity(t *testing.T) { | |
| req := entityresolutionV2.ResolveEntitiesRequest{} | ||
| req.Entities = validBody | ||
|
|
||
| resp, reserr := claims.EntityResolution(t.Context(), &req, logger.CreateTestLogger()) | ||
| resp, reserr := claims.EntityResolution(t.Context(), &req, logger.CreateTestLogger(), false) | ||
|
|
||
| require.NoError(t, reserr) | ||
|
|
||
|
|
@@ -44,7 +44,7 @@ func Test_EmailResolveEntity(t *testing.T) { | |
| req := entityresolutionV2.ResolveEntitiesRequest{} | ||
| req.Entities = validBody | ||
|
|
||
| resp, reserr := claims.EntityResolution(t.Context(), &req, logger.CreateTestLogger()) | ||
| resp, reserr := claims.EntityResolution(t.Context(), &req, logger.CreateTestLogger(), false) | ||
|
|
||
| require.NoError(t, reserr) | ||
|
|
||
|
|
@@ -78,7 +78,7 @@ func Test_ClaimsResolveEntity(t *testing.T) { | |
| req := entityresolutionV2.ResolveEntitiesRequest{} | ||
| req.Entities = validBody | ||
|
|
||
| resp, reserr := claims.EntityResolution(t.Context(), &req, logger.CreateTestLogger()) | ||
| resp, reserr := claims.EntityResolution(t.Context(), &req, logger.CreateTestLogger(), false) | ||
|
|
||
| require.NoError(t, reserr) | ||
|
|
||
|
|
@@ -93,6 +93,65 @@ func Test_ClaimsResolveEntity(t *testing.T) { | |
| assert.EqualValues(t, 42, propMap["baz"]) | ||
| } | ||
|
|
||
| func Test_ClaimsResolveEntityDirectEntitlements(t *testing.T) { | ||
| customclaims := map[string]interface{}{ | ||
| "direct_entitlements": []interface{}{ | ||
| map[string]interface{}{ | ||
| "attribute_value_fqn": "https://example.com/attr/department/value/eng", | ||
| "actions": []interface{}{"read", "update"}, | ||
| }, | ||
| }, | ||
| } | ||
| structClaims, err := structpb.NewStruct(customclaims) | ||
| require.NoError(t, err) | ||
|
|
||
| anyClaims, err := anypb.New(structClaims) | ||
| require.NoError(t, err) | ||
|
|
||
| var validBody []*entity.Entity | ||
| validBody = append(validBody, &entity.Entity{EphemeralId: "1234", EntityType: &entity.Entity_Claims{Claims: anyClaims}}) | ||
|
|
||
| req := entityresolutionV2.ResolveEntitiesRequest{Entities: validBody} | ||
|
|
||
| resp, reserr := claims.EntityResolution(t.Context(), &req, logger.CreateTestLogger(), true) | ||
| require.NoError(t, reserr) | ||
|
|
||
| entityRepresentations := resp.GetEntityRepresentations() | ||
| require.Len(t, entityRepresentations, 1) | ||
|
|
||
| entitlements := entityRepresentations[0].GetDirectEntitlements() | ||
| require.Len(t, entitlements, 1) | ||
| assert.Equal(t, "https://example.com/attr/department/value/eng", entitlements[0].GetAttributeValueFqn()) | ||
| assert.ElementsMatch(t, []string{"read", "update"}, entitlements[0].GetActions()) | ||
| } | ||
|
|
||
| func Test_ClaimsResolveEntityDirectEntitlementsDisabled(t *testing.T) { | ||
| customclaims := map[string]interface{}{ | ||
| "direct_entitlements": []interface{}{ | ||
| map[string]interface{}{ | ||
| "attribute_value_fqn": "https://example.com/attr/department/value/eng", | ||
| "actions": []interface{}{"read"}, | ||
| }, | ||
| }, | ||
| } | ||
| structClaims, err := structpb.NewStruct(customclaims) | ||
| require.NoError(t, err) | ||
|
|
||
| anyClaims, err := anypb.New(structClaims) | ||
| require.NoError(t, err) | ||
|
|
||
| req := entityresolutionV2.ResolveEntitiesRequest{Entities: []*entity.Entity{ | ||
| {EphemeralId: "1234", EntityType: &entity.Entity_Claims{Claims: anyClaims}}, | ||
| }} | ||
|
|
||
| resp, reserr := claims.EntityResolution(t.Context(), &req, logger.CreateTestLogger(), false) | ||
| require.NoError(t, reserr) | ||
|
|
||
| entityRepresentations := resp.GetEntityRepresentations() | ||
| require.Len(t, entityRepresentations, 1) | ||
| assert.Empty(t, entityRepresentations[0].GetDirectEntitlements()) | ||
| } | ||
|
Comment on lines
+96
to
+153
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial LGTM — good positive/negative coverage for the new flag. The two new tests tightly mirror the enabled ( Optional follow-up (nice-to-have, not blocking): consider a third case that exercises malformed/partial 🤖 Prompt for AI Agents |
||
|
|
||
| func Test_JWTToEntityChainClaims(t *testing.T) { | ||
| validBody := []*entity.Token{{Jwt: samplejwt}} | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| authEndpoint: &authEndpoint http://{{ .hostname }}:{{.kcPort }}/auth | ||
| issuerEndpoint: &issuerEndpoint http://{{ .hostname }}:{{.kcPort }}/auth/realms/{{.authRealm}} | ||
| tokenEndpoint: &tokenEndpoint http://{{ .hostname }}:{{.kcPort }}/auth/realms/{{.authRealm}}/protocol/openid-connect/token | ||
| entityResolutionServiceUrl: &entityResolutionServiceUrl https://{{ .hostname }}:{{.platformPort }}/entityresolution/resolve | ||
| platformEndpoint: &platformEndpoint https://{{.hostname }}:{{.platformPort }} | ||
| authRealm: &authRealm {{.authRealm}} | ||
| mode: all | ||
| logger: | ||
| level: debug | ||
| type: text | ||
| output: stdout | ||
| server: | ||
| port: {{.platformPort}} | ||
| auth: | ||
| enabled: true | ||
| enforceDPoP: false | ||
| audience: *platformEndpoint | ||
| issuer: *issuerEndpoint | ||
| policy: | ||
| extension: | | ||
| g, opentdf-admin, role:admin | ||
| g, opentdf-standard, role:standard | ||
| db: | ||
| host: {{ .pgHost }} | ||
| port: {{ .pgPort }} | ||
| database: {{ .pgDatabase }} | ||
| user: postgres | ||
| password: changeme | ||
| schema: otdf | ||
| services: | ||
| authorization: | ||
| allow_direct_entitlements: true | ||
| kas: | ||
| keyring: | ||
| - kid: e1 | ||
| alg: ec:secp256r1 | ||
| - kid: r1 | ||
| alg: rsa:2048 | ||
| entityresolution: | ||
| mode: claims | ||
| allow_direct_entitlements: true | ||
| shared: | ||
| clientId: otdf-shared | ||
| clientSecret: secret | ||
| authClientId: otdf-shared-auth | ||
| serviceHostName: shared | ||
| platformEndpoint: *platformEndpoint | ||
| platformAuthEndpoint: *authEndpoint | ||
| platformAuthRealm: *authRealm | ||
| tokenEndpoint: *tokenEndpoint | ||
| # ...other service configs as needed... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
Repository: opentdf/platform
Length of output: 12178
Replace
log.Fatalfwithpanicfor consistent config decode error handling.The current
log.Fatalfcall bypasses deferred cleanup in the server boot path and double-logs (slogError+ stdliblog.Fatalf). Registry-aware ERS constructors likeRegisterMultiStrategyERSV2usepanic(fmt.Sprintf(...))instead, which allows the service registry to catch and handle startup errors gracefully. Align this with that pattern.🤖 Prompt for AI Agents