diff --git a/pkg/authz/authorizers/cedar/core.go b/pkg/authz/authorizers/cedar/core.go index a42cdd6ed3..7c3504d73b 100644 --- a/pkg/authz/authorizers/cedar/core.go +++ b/pkg/authz/authorizers/cedar/core.go @@ -637,6 +637,7 @@ func (a *Authorizer) authorizeResourceRead( // Create attributes for the entities attributes := mergeContexts(map[string]interface{}{ + "name": resourceURI, "uri": resourceURI, "operation": "read", "feature": "resource", diff --git a/pkg/authz/authorizers/cedar/core_test.go b/pkg/authz/authorizers/cedar/core_test.go index e39e16cd1a..a0c7aa0a5c 100644 --- a/pkg/authz/authorizers/cedar/core_test.go +++ b/pkg/authz/authorizers/cedar/core_test.go @@ -269,6 +269,69 @@ func TestAuthorizeWithJWTClaims(t *testing.T) { arguments: nil, expectAuthorized: true, }, + { + name: "Resource entity exposes name attribute for Cedar schema", + policy: ` + permit( + principal, + action == Action::"read_resource", + resource + ) + when { + resource.name == "sensitive_data" + }; + `, + claims: jwt.MapClaims{ + "sub": "user123", + }, + feature: authorizers.MCPFeatureResource, + operation: authorizers.MCPOperationRead, + resourceID: "sensitive_data", + arguments: nil, + expectAuthorized: true, + }, + { + name: "Resource entity retains uri attribute for backward compat", + policy: ` + permit( + principal, + action == Action::"read_resource", + resource + ) + when { + resource.uri == "sensitive_data" + }; + `, + claims: jwt.MapClaims{ + "sub": "user123", + }, + feature: authorizers.MCPFeatureResource, + operation: authorizers.MCPOperationRead, + resourceID: "sensitive_data", + arguments: nil, + expectAuthorized: true, + }, + { + name: "Resource name and uri attributes carry the same value", + policy: ` + permit( + principal, + action == Action::"read_resource", + resource + ) + when { + resource.name == resource.uri + }; + `, + claims: jwt.MapClaims{ + "sub": "user123", + }, + feature: authorizers.MCPFeatureResource, + operation: authorizers.MCPOperationRead, + resourceID: "sensitive_data", + arguments: nil, + expectAuthorized: true, + }, { name: "User can get prompt", policy: ` diff --git a/pkg/authz/authorizers/cedar/entity.go b/pkg/authz/authorizers/cedar/entity.go index a495f154f3..ee7307f1ee 100644 --- a/pkg/authz/authorizers/cedar/entity.go +++ b/pkg/authz/authorizers/cedar/entity.go @@ -79,11 +79,15 @@ func (*EntityFactory) CreateResourceEntity( ) (cedar.EntityUID, cedar.Entity) { uid := cedar.NewEntityUID(cedar.EntityType(resourceType), cedar.String(resourceID)) - // Ensure name attribute is set + // Ensure name attribute is set — but don't overwrite if the caller + // already provided one (e.g. authorizeResourceRead sets name to the + // original URI before sanitization). if attributes == nil { attributes = make(map[string]interface{}) } - attributes["name"] = resourceID + if _, exists := attributes["name"]; !exists { + attributes["name"] = resourceID + } attrs := convertMapToCedarRecord(attributes) diff --git a/pkg/authz/authorizers/cedar/entity_test.go b/pkg/authz/authorizers/cedar/entity_test.go index 8569c3d125..c971627aa6 100644 --- a/pkg/authz/authorizers/cedar/entity_test.go +++ b/pkg/authz/authorizers/cedar/entity_test.go @@ -161,6 +161,69 @@ func TestCreateResourceEntity_Parents(t *testing.T) { } } +// TestCreateResourceEntity_NamePreservation is a regression test for the bug +// where CreateResourceEntity unconditionally overwrote attributes["name"] with +// resourceID (the sanitized entity UID). authorizeResourceRead sets name to the +// original, unsanitized URI before calling CreateResourceEntity — the caller's +// value must survive. When no name is provided, resourceID is used as fallback. +func TestCreateResourceEntity_NamePreservation(t *testing.T) { + t.Parallel() + + factory := NewEntityFactory() + + tests := []struct { + name string + resourceID string + attributes map[string]interface{} + wantName string + }{ + { + name: "caller_name_preserved", + resourceID: "sanitized-resource-id", + attributes: map[string]interface{}{ + "name": "original/unsanitized:uri", + }, + wantName: "original/unsanitized:uri", + }, + { + name: "uri_with_special_characters_preserved", + resourceID: "https___example_com_api_v1_resource_id=42", + attributes: map[string]interface{}{ + "name": "https://example.com/api/v1/resource?id=42", + }, + wantName: "https://example.com/api/v1/resource?id=42", + }, + { + name: "fallback_to_resourceID_when_name_absent", + resourceID: "weather", + attributes: map[string]interface{}{ + "description": "Weather tool", + }, + wantName: "weather", + }, + { + name: "fallback_to_resourceID_when_attributes_nil", + resourceID: "weather", + attributes: nil, + wantName: "weather", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + _, entity := factory.CreateResourceEntity( + "Resource", tt.resourceID, tt.attributes, + ) + + nameVal, found := entity.Attributes.Get(cedar.String("name")) + require.True(t, found, "name attribute must always be set") + assert.Equal(t, tt.wantName, string(nameVal.(cedar.String))) + }) + } +} + // TestCreateEntitiesForRequest_GroupsAsParents verifies that // CreateEntitiesForRequest sets THVGroup parent UIDs on the principal but // does NOT insert separate THVGroup entities into the entity map (fixing