feat: Support literal comparisons in authorization expressions#823
Conversation
🦋 Changeset detectedLatest commit: b36e385 The changes in this PR will be included in the next version bump. This PR includes changesets to release 21 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds support for literal value operands (string, number, boolean) in authorizer expression comparisons and strict inequality (!==), updates parsing, AST, validation (including literal-type checks), server codegen/query compilation, frontend validation wiring, and tests/changelog across lib, server, and web packages. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-validator.unit.test.ts (1)
679-695: Please add direct coverage for the new literal-validation branch.These assertions only verify that
fieldTypesis populated. A warning case likemodel.count === 1.5and a passing case likemodel.status === 'active'would exercisevalidateLiteralTypeCompatibility()itself instead of only its setup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/literal-comparisons-in-auth-expressions.md:
- Around line 1-6: The changeset frontmatter is missing the non-private package
'@baseplate-dev/project-builder-web'; update the YAML frontmatter in
.changeset/literal-comparisons-in-auth-expressions.md by adding the line
"'@baseplate-dev/project-builder-web': patch" alongside the existing entries so
the frontmatter lists '@baseplate-dev/project-builder-lib',
'@baseplate-dev/project-builder-server', and
'@baseplate-dev/project-builder-web' as patch releases.
In
`@packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-acorn-parser.ts`:
- Around line 203-226: convertFieldRefOrLiteral currently only treats node.type
=== 'Literal', so signed numeric literals parsed as UnaryExpression (e.g., {
type: 'UnaryExpression', operator: '-', argument: Literal }) are misclassified;
update convertFieldRefOrLiteral to detect node.type === 'UnaryExpression' with
operator '+' or '-' and an argument that is a numeric Literal, compute the
signed numeric value (apply operator to argument.value) and return a
LiteralValueNode with that value and the original node.start/node.end; otherwise
fall through to existing logic and keep throwing AuthorizerExpressionParseError
for unsupported cases.
In
`@packages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-validator.ts`:
- Around line 289-297: The validator groups 'int' with 'float'/'decimal' so an
expression like model.count === 1.5 passes; update the switch in
authorizer-expression-validator to treat 'int' separately: for fieldType ===
'int' return true only if literalJsType === 'number' AND the numeric value is an
integer (e.g., Number.isInteger or equivalent check on the literal value), while
keeping 'float' and 'decimal' accepting any numeric literal; use the existing
fieldType and literalJsType variables to locate and change the int case logic.
In
`@packages/project-builder-server/src/compiler/backend/query-filter-compiler.ts`:
- Around line 175-191: Add an explicit guard for model-vs-model comparisons
before the auth fallback: detect when both modelNode and otherNode have source
=== 'model' and throw a clear error (e.g., "model-to-model comparisons not
supported") instead of emitting ctx.auth.*; update the branch that currently
builds authExpr (`const authExpr = \`ctx.auth.\${otherNode.field}\``) to only
run when otherNode.source === 'auth', and keep the literalValue handling as-is;
reference modelNode, otherNode, operator and serializeLiteralValue when locating
the code to change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c8a61e52-efb9-4339-a9e3-4aa51085b59e
📒 Files selected for processing (13)
.changeset/literal-comparisons-in-auth-expressions.mdpackages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-acorn-parser.tspackages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-acorn-parser.unit.test.tspackages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-ast.tspackages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-parser.tspackages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-validator.tspackages/project-builder-lib/src/schema/models/authorizer/authorizer-expression-validator.unit.test.tspackages/project-builder-server/src/compiler/backend/authorizer-compiler.tspackages/project-builder-server/src/compiler/backend/authorizer-compiler.unit.test.tspackages/project-builder-server/src/compiler/backend/query-filter-compiler.tspackages/project-builder-server/src/compiler/backend/query-filter-compiler.unit.test.tspackages/project-builder-web/src/routes/data/models/edit.$key/-components/authorizer/authorizer-expression-linter.tspackages/project-builder-web/src/routes/data/models/edit.$key/-components/authorizer/model-authorizer-role-form.tsx
Summary by CodeRabbit
New Features
!==) alongside===for comparisonsBug Fixes / Improvements
Tests