cedar-policy / cedar

Implementation of the Cedar Policy Language
https://www.cedarpolicy.com
Apache License 2.0
832 stars 72 forks source link

Suggested replacement for unrecognized action entity misses obvious fix #166

Open mwhicks1 opened 1 year ago

mwhicks1 commented 1 year ago

Before opening, please confirm:

Bug Category

Schemas and Validation

Describe the bug

The actions section of the schema lists entity IDs, whereas the entityTypes section lists entity types. If a user mistakenly includes the Action entity type (and any namespace) as part of their action ID, the error messages are confusing.

Expected behavior

Ideally, the error message can identify the user's mistake more clearly, and do better at suggesting a fix. The suggestion would need to account for this particular mistake of including part of the type/namespace in the action ID.

Reproduction steps

In tinytodo's tinytodo.cedarschema.json, modify the actions element "createList" to instead be "Action::CreateList". When validating polices.cedar we get the error message

Validation error on policy policy0: Unrecognized action id Action::"CreateList", did you mean Action::"GetList"?

This is confusing! With this change, we are defining an action entity Action::"Action::CreateList". This is not what the user intended, because they failed to appreciate the distinction between entity IDs and entity types.

Code Snippet

No response

Log output

No response

Additional configuration

No response

Operating System

No response

Additional information and screenshots

No response

adpaco-aws commented 1 week ago

I haven't been able to reproduce this issue yet.

First, the JSON file tinytodo.cedarschema.json doesn't exist anymore, and has been replaced by the non-JSON schema since then. It's possible to retrieve the file going to the 3.0 branch: release/3.0.x/tinytodo.cedarschema.json. My fork now includes the issue-166 branch which uses the JSON schema, but results in the following parsing error:

>>>   2024-09-09T23:33:18.014524Z ERROR tiny_todo_server: Failed to load entities, policies, or schema: Error Parsing Human-readable Schema: error parsing schema: unexpected token `{`
adpaco-aws commented 1 week ago

@john-h-kastner-aws pointed out it's possible to reproduce it using quotes on the non-JSON schema. This is what the error looks like now:

>>>   2024-09-10T00:00:15.977378Z ERROR tiny_todo_server: Failed to load entities, policies, or schema: Validation Failed: for policy `policy0`, unrecognized action `Action::"CreateList"`
for policy `policy0`, unable to find an applicable action given the policy scope constraints
john-h-kastner-aws commented 1 week ago

When looking at these error messages, you should be aware that tiny-todo might not be printing all available information (cedar-policy/cedar-examples#97). The CLI will do better job of it, but we've previously noted that it may be dropping source information for some schema parsing errors (#948). Both of these should be pretty simple fixes if you want to knock them out.

john-h-kastner-aws commented 1 week ago

Also, while looking at this code, it might make sense resolve #246, #332, and #380 at the same time since these all deal with the suggested replacement mechanism.