Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/authz/authorizers/cedar/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
63 changes: 63 additions & 0 deletions pkg/authz/authorizers/cedar/core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: `
Expand Down
8 changes: 6 additions & 2 deletions pkg/authz/authorizers/cedar/entity.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
63 changes: 63 additions & 0 deletions pkg/authz/authorizers/cedar/entity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading