Delete schema-based parsing for action attribtues #1194
Conversation
Action cannot be declared with attributes in the schema, so this code path was not reachable from the public API. If we add action attributes later, you now won't be able to use the schema-based parsing shortcuts when parsing action entity data, but you can still fully re-use action entities from the schema by not redefining them in the entity data. Signed-off-by: John Kastner <[email protected]>
Signed-off-by: John Kastner <[email protected]>
john-h-kastner-aws
force-pushed
the
jkastner/rm_schema_parsing_action_attr
branch
from
1b1ad56 to
4b8c350
Compare
| EntitySchemaInfo::Action(action) => { | ||
| // We'll do schema-based parsing assuming optimistically that | ||
| // the type in the JSON is the same as the type in the schema. | ||
| // (As of this writing, the schema doesn't actually tell us | ||
| // what type each action attribute is supposed to be) |
There was a problem hiding this comment.
I think this is a behavior change -- suppose you have an action attribute of type ip in the schema, then with this code you're allowed to pass just "192.168.0.1" for that attribute in the entity data (ie, just a string), whereas after this code is removed, you'll need the { __extn: { ... } } form.
however, this is probably inconsequential, because most users are probably not passing actions in their entity data if they are also passing the schema, since the schema contains the action info and we already automatically pull the proper action info from the schema when constructing entities.
There was a problem hiding this comment.
There is a behavior change for actions with attributes, but the schema doesn't allow that currently.
Signed-off-by: John Kastner <[email protected]>
john-h-kastner-aws
deleted the
jkastner/rm_schema_parsing_action_attr
branch
3 participants
Description of changes
Schema based parsing currently supports parsing action attributes according to their types, but the Cedar schema format does not support defining actions with their attributes or declaring the type of their attributes, so this code is not reachable by consumers of the Cedar library.
By deleting this code we can remove a caller of
schematype_of_partialvaluewhich is known to be buggy for set and records.Note that action entities are compared against the action entities from the schema when constructing an
Entitiesobject, so we still report an error if an action entity contains unexpected attributes.Issue #, if available
Checklist for requesting a review
The change in this PR is (choose one, and delete the other options):
cedar-policy(e.g., changes to the signature of an existing API).cedar-policy(e.g., addition of a new API).cedar-policy.cedar-policy-core,cedar-validator, etc.)I confirm that this PR (choose one, and delete the other options):
I confirm that
cedar-spec(choose one, and delete the other options):cedar-spec, and how you have tested that your updates are correct.)cedar-spec. (Post your PR anyways, and we'll discuss in the comments.)