feat(schema): accept empty principal/resource lists in human schema#2311
Draft
olehbozhok wants to merge 1 commit intocedar-policy:mainfrom
Draft
feat(schema): accept empty principal/resource lists in human schema#2311olehbozhok wants to merge 1 commit intocedar-policy:mainfrom
olehbozhok wants to merge 1 commit intocedar-policy:mainfrom
Conversation
Align the human-readable schema parser with RFC 55 semantics, which already defines an empty principal/resource list as "the action applies to no entities of that kind" and is honored by the JSON parser and the validator. Before this change, the human parser alone rejected `principal: []` / `resource: []` with "is `[]`, which is invalid", breaking JSON-to-human round-tripping and forcing downstream users of partial authorization (e.g. multi-issuer flows calling `is_authorized_partial`) to declare a sentinel entity solely to pacify the parser. Changes: - `to_json_schema.rs`: accept an empty `entity_tys` and lower it to `principal_types: vec![]` / `resource_types: vec![]`. - `err.rs`: drop the `MissingOrEmpty::Empty` variant and the `empty_principal` / `empty_resource` constructors. `NoPrincipalOrResource` now covers only the still-rejected "clause omitted" case. Help text updated to point users at `principal: []` when that is the intent. - `fmt.rs`: when `principal_types` or `resource_types` is empty, emit the explicit `[]` form rather than suppressing the entire `appliesTo` block, restoring round-trippability for JSON schemas that use empty lists. - `test.rs`: invert `empty_principal` / `empty_resource` rejection assertions into acceptance assertions; add a `both_empty` test; update `print_actions` to assert the new formatter output. Omitting the `principal` / `resource` clause entirely remains a parse error, per RFC 55. No change to JSON schema parsing, validator, evaluator, or authorization APIs. No currently-valid schema changes meaning; no currently-valid schema becomes invalid. Reference implementation for cedar-policy/rfcs#113. Signed-off-by: Oleh <6554798+olehbozhok@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of changes
Align the human-readable schema parser with RFC 55 semantics, which already define an empty
principal/resourcelist as "the action applies to no entities of that kind" and are honored by the JSON parser and the validator. Before this change, the human parser alone rejectedprincipal: []/resource: []with"is '[]', which is invalid", breaking JSON ⇄ human round-tripping and forcing downstream users of partial authorization (e.g. multi-issuer flows callingis_authorized_partial) to declare a sentinel entity solely to pacify the parser.What changes:
to_json_schema.rs: accept an emptyentity_tysand lower it toprincipal_types: vec![]/resource_types: vec![].err.rs: drop theMissingOrEmpty::Emptyvariant and theempty_principal/empty_resourceconstructors.NoPrincipalOrResourcenow covers only the still-rejected "clause omitted" case. Help text updated to suggestprincipal: []when that is the author's intent.fmt.rs: whenprincipal_typesorresource_typesis empty, emit the explicit[]form rather than suppressing the entireappliesToblock. Restores round-trippability for JSON schemas that use empty lists.test.rs: invertempty_principal/empty_resourcerejection assertions into acceptance assertions; add aboth_emptytest; updateprint_actionsto assert the new formatter output.Scope guarantees:
principal/resourceclause entirely remains a parse error, per RFC 55.This is the reference implementation for cedar-policy/rfcs#113. Please do not merge until that RFC is accepted; I'm opening this PR as a draft so maintainers have code to discuss alongside the RFC.
Issue #, if available
Related: #1335 (improved the error message for this case but did not revisit the rejection), #351 (predecessor discussion,
requires-RFC).Checklist for requesting a review
The change in this PR is:
cedar-policy-core,cedar-validator, etc.)Note: marked as the closest-fitting option because this is a human-schema parser change in
cedar-policy-core, with nocedar-policyRust API change. It is a language-level behavior change (a schema form that used to fail to parse now parses), so I have marked the CHANGELOG entry with(*)per the repo convention.I confirm that this PR:
I confirm that
cedar-spec:cedar-spec.The formal model in
cedar-speccovers authorization; the human-schema parser is outside that scope, but I'd like confirmation from maintainers whether the Lean formalization of the schema surface syntax needs updating.I confirm that
docs.cedarpolicy.com:cedar-docs. PRs should be targeted at astaging-X.Ybranch, notmain.)Once this PR and RFC #113 are accepted, I will open a companion PR in
cedar-docsupdating the schema-syntax reference to document the acceptedprincipal: []/resource: []form and the motivating partial-authorization use case.