Skip to content

JSON policy with zero-argument method style call doesn't parse when converted to Cedar syntax #2116

@john-h-kastner-aws

Description

@john-h-kastner-aws

Before opening, please confirm:

Describe the bug

In this JSON policy there is a call to the offset extension function without any arguments provided. The policy parses without any errors.

{
  "effect": "permit",
  "principal": {
    "op": "==",
    "entity": { "type": "User", "id": "bob"}
  },
  "resource": {
    "op": "==",
    "entity": { "type": "User", "id": "bob"}
  },
  "action": {
    "op": "All"
  },
  "conditions": [
    {
      "kind": "when",
      "body": {
        "offset": []
      }
    }
  ]
}

We can convert it to the human-readable Cedar policy syntax (cedar convert-policy --direction json-to-cedar)

permit(
  principal == User::"bob",
  action,
  resource == User::"bob"
) when {
  offset()
};

The resulting policy has the expression offset(). This fails to parse because the offset extension function can only be called with the method-style syntax. There wasn't an error parsing the same policy from JSON because the JSON policy syntax doesn't differentiate between function and method-style calls.

The policy conversion function is aware of the call style, but it's not possible to emit a method-style call without at least one argument. Instead of erroring, we currently print this function style call instead

(Some(ast::CallStyle::MethodStyle), Some(receiver)) => {
maybe_with_parens(f, receiver, n)?;
write!(f, ".{}({})", fn_name, args.iter().skip(1).join(", "))
}
(_, _) => {
write!(f, "{}({})", fn_name, args.iter().join(", "))
}

We would like to have the property that the output of translate-policy is always a valid cedar policy, so we should detect the error when translating the policy instead of on parsing the resulting policy.

Options:

  1. (preferred) Detect the error when converting the JSON policy to Cedar syntax
  2. Detect the error when parsing the JSON policy. This option is a breaking change to the JSON policy format, so I prefer delaying the error until conversion. We might reasonably take this option if we believe the risk of breaking actual users is small enough, but I would rather not.
  3. Update the JSON and Cedar policy parsers to report an error whenever an extension function has the wrong number of arguments. This is a larger breaking change to both policy formats, so we won't do this now, but IMO we should consider doing it eventually.

Reproduction steps

// Put your code below this line.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working. This is as high priority issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions