diff --git a/docs/authz.md b/docs/authz.md index c20f19e111..45a2bba0ad 100644 --- a/docs/authz.md +++ b/docs/authz.md @@ -192,9 +192,17 @@ In the context of MCP servers, the following entities are used: - Examples: - `Tool::"weather"`: The weather tool - `Prompt::"greeting"`: The greeting prompt - - `Resource::"data"`: The data resource + - `Resource::"data"`: A short resource name + - `Resource::"file:///etc/passwd"`: An MCP resource URI (exact URI is the Cedar entity ID) - `FeatureType::"tool"`: The tool feature type (used for list operations) + For `read_resource`, the Cedar entity ID is the **exact resource URI** (for example + `Resource::"file:///ok"` or `Resource::"mcp://srv/config:admin"`). Do not rewrite + characters such as `/`, `:`, or `?` into underscores; policies must name the URI as + the client and server see it. In Cedar source the ID is a double-quoted string + literal, so almost every URI character is ordinary, but `"` and `\` must be escaped + (for example `Resource::"file://C:\\share\\data"`). + #### Example policies Here are some example policies for common scenarios: @@ -221,7 +229,17 @@ This policy allows any client to get the greeting prompt. permit(principal, action == Action::"read_resource", resource == Resource::"data"); ``` -This policy allows any client to read the data resource. +This policy allows any client to read the resource whose entity ID is `data`. + +For URI-shaped MCP resources, name the exact URI: + +```plain +permit( + principal, + action == Action::"read_resource", + resource == Resource::"file:///etc/passwd" +); +``` ##### List operations diff --git a/pkg/authz/authorizers/cedar/core.go b/pkg/authz/authorizers/cedar/core.go index d6198e71fe..e370653628 100644 --- a/pkg/authz/authorizers/cedar/core.go +++ b/pkg/authz/authorizers/cedar/core.go @@ -1084,10 +1084,14 @@ func (a *Authorizer) authorizeResourceRead( // Action is to read a resource action := "Action::read_resource" - // Resource is the resource being accessed - // Use the URI as the resource ID, but sanitize it for Cedar - sanitizedURI := sanitizeURIForCedar(resourceURI) - resource := fmt.Sprintf("Resource::%s", sanitizedURI) + // Resource is the resource being accessed. Use the exact URI as the + // entity ID: entities are built programmatically via cedar.NewEntityUID, + // which accepts any string on the request side, so no character rewriting + // is needed. (Policy authors still write the ID as a Cedar string literal, + // so " and \ need escaping in policy source; see docs/authz.md.) A lossy + // mapping would let distinct URIs collide onto one entity ID, and a + // policy grant on one URI would then silently cover every colliding URI. + resource := fmt.Sprintf("Resource::%s", resourceURI) // Create attributes for the entities attributes := mergeContexts(map[string]interface{}{ @@ -1161,25 +1165,6 @@ func parseCedarEntityID(entityID string) (string, string, error) { return parts[0], parts[1], nil } -// sanitizeURIForCedar sanitizes a URI for use in Cedar policies. -// Cedar entity IDs have restrictions on characters, so we need to sanitize the URI. -func sanitizeURIForCedar(uri string) string { - // Replace characters that are not allowed in Cedar entity IDs - // This is a simple implementation - you may need to enhance it based on your needs - replacer := strings.NewReplacer( - ":", "_", - "/", "_", - "\\", "_", - "?", "_", - "&", "_", - "=", "_", - "#", "_", - " ", "_", - ".", "_", - ) - return replacer.Replace(uri) -} - // AuthorizeWithJWTClaims demonstrates how to use JWT claims with the Cedar authorization middleware. // This method: // 1. Extracts JWT claims from the context diff --git a/pkg/authz/authorizers/cedar/core_test.go b/pkg/authz/authorizers/cedar/core_test.go index 936e50fc90..bca48b8788 100644 --- a/pkg/authz/authorizers/cedar/core_test.go +++ b/pkg/authz/authorizers/cedar/core_test.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "encoding/json" + "fmt" "log/slog" "maps" "sync" @@ -3347,38 +3348,61 @@ func TestParseCedarEntityID(t *testing.T) { } } -// TestSanitizeURIForCedar tests the sanitizeURIForCedar helper function. -func TestSanitizeURIForCedar(t *testing.T) { +// TestAuthorizeResourceReadEntityIDsCollisionFree verifies that resource URIs +// become Cedar entity IDs verbatim. Distinct URIs that a lossy sanitization +// would map onto the same entity ID must stay distinct, so a policy grant on +// one URI never authorizes a colliding URI. +func TestAuthorizeResourceReadEntityIDsCollisionFree(t *testing.T) { t.Parallel() tests := []struct { - name string - input string - want string + name string + grantedURI string + collidingURI string }{ - {name: "empty_string", input: "", want: ""}, - {name: "already_clean", input: "simple_resource", want: "simple_resource"}, - {name: "colon", input: "a:b", want: "a_b"}, - {name: "forward_slash", input: "a/b", want: "a_b"}, - {name: "backslash", input: `a\b`, want: "a_b"}, - {name: "question_mark", input: "a?b", want: "a_b"}, - {name: "ampersand", input: "a&b", want: "a_b"}, - {name: "equals", input: "a=b", want: "a_b"}, - {name: "hash", input: "a#b", want: "a_b"}, - {name: "space", input: "a b", want: "a_b"}, - {name: "dot", input: "a.b", want: "a_b"}, - { - name: "complex_uri", - input: "https://api.example.com/v1/data?key=val&other=123#fragment", - want: "https___api_example_com_v1_data_key_val_other_123_fragment", + { + name: "file_uri_colon_and_slash_run", + grantedURI: "file:///etc/passwd", + collidingURI: "file://_etc/passwd", + }, + { + name: "mcp_uri_colon_vs_slash", + grantedURI: "mcp://srv/config:admin", + collidingURI: "mcp://srv/config/admin", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - got := sanitizeURIForCedar(tt.input) - assert.Equal(t, tt.want, got) + + authorizer, err := NewCedarAuthorizer(ConfigOptions{ + Policies: []string{ + fmt.Sprintf(`permit(principal, action == Action::"read_resource", resource == Resource::"%s");`, tt.grantedURI), + }, + EntitiesJSON: `[]`, + }, "") + require.NoError(t, err, "Failed to create Cedar authorizer") + + identity := &auth.Identity{PrincipalInfo: auth.PrincipalInfo{ + Subject: "test-user", + Claims: jwt.MapClaims{"sub": "user123"}, + }} + claimsCtx := auth.WithIdentity(context.Background(), identity) + + // The exact URI named in the policy is authorized. + authorized, err := authorizer.AuthorizeWithJWTClaims( + claimsCtx, authorizers.MCPFeatureResource, authorizers.MCPOperationRead, tt.grantedURI, nil) + require.NoError(t, err) + assert.True(t, authorized, "exact URI named in the policy should be authorized") + + // The colliding URI, which mapped to the same entity ID under the + // previous lossy sanitization, must not be authorized. + authorized, err = authorizer.AuthorizeWithJWTClaims( + claimsCtx, authorizers.MCPFeatureResource, authorizers.MCPOperationRead, tt.collidingURI, nil) + require.NoError(t, err) + assert.False(t, authorized, + "colliding URI %q must not be authorized by a grant on %q", tt.collidingURI, tt.grantedURI) }) } }