feat: allow using a type as auth model #1841
Conversation
📝 WalkthroughWalkthroughThis pull request introduces various modifications to the grammar definitions and validation mechanisms within the ZModel language specification. Key changes include updates to the Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (16)
packages/schema/src/language-server/utils.ts (1)
32-37: LGTM with minor documentation suggestions.The implementation is clean and properly typed. Consider enhancing the JSDoc with:
- An example showing typical usage
- Description of what constitutes a "resolvable member"
Example enhancement:
/** * Checks if the given node can contain resolvable members. + * + * @example + * if (isMemberContainer(node)) { + * // node is guaranteed to be either DataModel or TypeDef + * // and can contain resolvable members like fields or properties + * } */packages/schema/src/language-server/validator/typedef-validator.ts (1)
17-19: Consider adding type-specific attribute validationWhile the implementation is clean, consider adding explicit validation to ensure that the attributes are compatible with type definitions. According to the AI summary, attributes applied to type definitions should have the
@@@supportTypeDefattribute.Consider updating the implementation to include type-specific validation:
private validateAttributes(typeDef: TypeDef, accept: ValidationAcceptor) { - typeDef.attributes.forEach((attr) => validateAttributeApplication(attr, accept)); + typeDef.attributes.forEach((attr) => { + validateAttributeApplication(attr, accept); + // Validate that the attribute supports type definitions + validateTypeDefAttributeSupport(attr, accept); + }); }packages/schema/src/language-server/validator/schema-validator.ts (1)
39-40: LGTM with suggestions for improved clarityThe changes correctly implement the ability to validate both data models and types for
@@authattribute. However, a few improvements could make the code more maintainable:
- The error message could be more precise to reflect that either a type or model can have
@@auth- The boolean parameter in
getDataModelAndTypeDefs(model, true)could be more explicitConsider applying these improvements:
const decls = getDataModelAndTypeDefs(model, true); const authModels = decls.filter((d) => isDataModel(d) && hasAttribute(d, '@@auth')); if (authModels.length > 1) { - accept('error', 'Multiple `@@auth` models are not allowed', { node: authModels[1] }); + accept('error', 'Multiple `@@auth` declarations (models or types) are not allowed', { node: authModels[1] }); }Also consider making the boolean parameter more explicit:
-const decls = getDataModelAndTypeDefs(model, true); +const includeTypes = true; // Include both models and type definitions +const decls = getDataModelAndTypeDefs(model, includeTypes);tests/integration/tests/enhancements/json/validation.test.ts (4)
40-56: LGTM: Well-structured test for policy rule validationThe test effectively verifies that direct member access is disallowed in policy rules. Consider making the error message more user-friendly by including context about why the access is not allowed.
- ).resolves.toContain(`Could not resolve reference to MemberAccessMember named 'age'.`); + ).resolves.toContain(`Direct member access 'profile.age' is not allowed in policy rules. Use 'auth()' to access authenticated user's data.`);
58-74: Consider adding negative test cases for auth member accessWhile the test correctly verifies that valid auth member access works, consider adding test cases for:
- Invalid auth member paths
- Nested invalid members
- Type mismatches in the access chain
Example additional test case:
it('fails on invalid auth member accesses in policy rules', async () => { await expect( loadModelWithError(` type Profile { age Int @gt(0) } model User { id Int @id @default(autoincrement()) profile Profile @json @@allow('all', auth().profile.invalidField > 18) } `) ).resolves.toContain('Could not resolve member "invalidField"'); });
76-134: LGTM: Comprehensive collection access validationThe tests effectively cover both normal and authenticated collection access scenarios. However, consider adding test cases for:
- Nested collection access (e.g.,
auth().profile.roles?[org.name == 'X'])- Invalid filter conditions
- Type mismatches in filter expressions
Line range hint
1-158: Consider adding tests for edge casesThe test suite is well-structured and covers the main functionality. Consider adding tests for:
- Deeply nested type structures
- Circular type references
- Mixed usage of primitive and complex types
- Error cases with malformed JSON data
packages/schema/src/utils/ast-utils.ts (1)
307-312: LGTM with a minor documentation suggestion.The function implementation correctly handles both data models and type definitions. Consider enhancing the JSDoc comment to be more descriptive about the return type.
/** - * Gets all data models and type defs from all loaded documents + * Gets all data models and type definitions from all loaded documents + * @returns An array of DataModel and TypeDef nodes from all loaded documents */packages/sdk/src/utils.ts (3)
54-64: LGTM! Consider enhancing the JSDoc.The implementation correctly filters both DataModel and TypeDef declarations. Consider documenting the
includeIgnoredparameter in the JSDoc to maintain consistency with other functions./** * Gets data models and type defs in the ZModel schema. + * @param model The ZModel schema + * @param includeIgnored When true, includes models marked with @@ignore */
474-479: LGTM! Consider adding error handling.The function has been appropriately renamed and updated to support both DataModel and TypeDef as auth declarations. The implementation maintains the existing logic while expanding its scope.
Consider adding null checks for undefined refs:
export function getAuthDecl(decls: (DataModel | TypeDef)[]) { + if (!decls?.length) { + return undefined; + } let authModel = decls.find((m) => hasAttribute(m, '@@auth')); if (!authModel) { authModel = decls.find((m) => m.name === 'User'); } return authModel; }
Line range hint
499-517: Add null check for modelIdAttr args.The function has been correctly updated to support both DataModel and TypeDef. However, there's a potential null reference when accessing
modelIdAttr.args[0].Apply this fix:
export function getIdFields(decl: DataModel | TypeDef) { const fields = isDataModel(decl) ? getModelFieldsWithBases(decl) : decl.fields; const fieldLevelId = fields.find((f) => f.attributes.some((attr) => attr.decl.$refText === '@id')); if (fieldLevelId) { return [fieldLevelId]; } else { // get model level @@id attribute const modelIdAttr = decl.attributes.find((attr) => attr.decl?.ref?.name === '@@id'); - if (modelIdAttr) { + if (modelIdAttr?.args?.[0]?.value) { // get fields referenced in the attribute: @@id([field1, field2]]) if (!isArrayExpr(modelIdAttr.args[0].value)) { return []; } const argValue = modelIdAttr.args[0].value; return argValue.items .filter((expr): expr is ReferenceExpr => isReferenceExpr(expr) && !!getDataModelFieldReference(expr)) .map((expr) => expr.target.ref as DataModelField); } } return []; }packages/schema/src/plugins/enhancer/policy/utils.ts (1)
522-522: LGTM! Consider adding documentation.The implementation correctly uses the new functions to support both data models and type definitions as auth models.
Consider adding a code comment explaining that
getDataModelAndTypeDefsnow supports both models and types for auth declarations, to help future maintainers understand this enhancement.+ // Get auth declaration from either data models or type definitions const authModel = getAuthDecl(getDataModelAndTypeDefs(model.$container, true));packages/schema/src/plugins/enhancer/enhance/index.ts (1)
89-89: Consider a more type-safe approach for the default auth type.The current implementation falls back to a generic
AuthUsertype whenauthDeclis undefined. This might mask type errors at compile time. Consider one of these alternatives:
- Make the auth type parameter required when no auth declaration is present
- Use a more restrictive default type
-const authTypeParam = authDecl ? `auth.${authDecl.name}` : 'AuthUser'; +const authTypeParam = authDecl ? `auth.${authDecl.name}` : 'Record<string, never>';packages/language/src/zmodel.langium (2)
184-184: Address the TODO to UnifyTypeDefand AbstractDataModelThe TODO comment indicates an intention to unify
TypeDefand abstractDataModel. Would you like assistance in implementing this unification, or should we open a GitHub issue to track this task?
252-252: Address the TODO: Rename Since It's for BothDataModelandTypeDefThe TODO comment suggests renaming to reflect that the attribute applies to both
DataModelandTypeDef. Would you like assistance in proposing a new name, or should we open a GitHub issue to track this task?packages/schema/src/language-server/zmodel-scope.ts (1)
221-225: RefactorcreateScopeForContainerto reduce code duplicationBoth branches in
createScopeForContainercallthis.createScopeForNodeswith different parameters. Consider refactoring to assign the fields first and make a single call tocreateScopeForNodesto improve readability and maintainability.Here's a suggested refactor:
private createScopeForContainer(node: AstNode | undefined, globalScope: Scope, includeTypeDefScope = false) { + let fields; if (isDataModel(node)) { - return this.createScopeForNodes(getModelFieldsWithBases(node), globalScope); + fields = getModelFieldsWithBases(node); } else if (includeTypeDefScope && isTypeDef(node)) { - return this.createScopeForNodes(node.fields, globalScope); + fields = node.fields; } else { return EMPTY_SCOPE; } + return this.createScopeForNodes(fields, globalScope); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
packages/language/src/generated/ast.tsis excluded by!**/generated/**,!**/generated/**packages/language/src/generated/grammar.tsis excluded by!**/generated/**,!**/generated/**
📒 Files selected for processing (21)
packages/language/src/zmodel.langium(4 hunks)packages/plugins/trpc/tests/projects/nuxt-trpc-v10/prisma/schema.prisma(0 hunks)packages/plugins/trpc/tests/projects/nuxt-trpc-v11/prisma/schema.prisma(0 hunks)packages/plugins/trpc/tests/projects/t3-trpc-v11/prisma/schema.prisma(0 hunks)packages/schema/src/cli/cli-util.ts(2 hunks)packages/schema/src/language-server/utils.ts(2 hunks)packages/schema/src/language-server/validator/attribute-application-validator.ts(1 hunks)packages/schema/src/language-server/validator/schema-validator.ts(2 hunks)packages/schema/src/language-server/validator/typedef-validator.ts(1 hunks)packages/schema/src/language-server/zmodel-linker.ts(7 hunks)packages/schema/src/language-server/zmodel-scope.ts(5 hunks)packages/schema/src/plugins/enhancer/enhance/auth-type-generator.ts(4 hunks)packages/schema/src/plugins/enhancer/enhance/index.ts(2 hunks)packages/schema/src/plugins/enhancer/policy/utils.ts(2 hunks)packages/schema/src/res/stdlib.zmodel(3 hunks)packages/schema/src/utils/ast-utils.ts(2 hunks)packages/schema/tests/schema/validation/attribute-validation.test.ts(1 hunks)packages/sdk/src/model-meta-generator.ts(3 hunks)packages/sdk/src/utils.ts(5 hunks)tests/integration/tests/enhancements/json/validation.test.ts(2 hunks)tests/integration/tests/enhancements/with-policy/auth.test.ts(1 hunks)
💤 Files with no reviewable changes (3)
- packages/plugins/trpc/tests/projects/nuxt-trpc-v10/prisma/schema.prisma
- packages/plugins/trpc/tests/projects/nuxt-trpc-v11/prisma/schema.prisma
- packages/plugins/trpc/tests/projects/t3-trpc-v11/prisma/schema.prisma
🔇 Additional comments (41)
packages/schema/src/language-server/utils.ts (2)
3-3: LGTM: Import additions are well-organized and properly typed.
The new imports support the feature of using types as auth models by providing necessary type guards and definitions.
Also applies to: 5-6
35-36: Verify consistent usage of the new type guard.
Let's ensure this new utility function is used consistently across the codebase and doesn't introduce breaking changes.
✅ Verification successful
Based on the shell script results, I can now generate the final response:
The new type guard is used correctly and consistently
The verification shows:
isMemberContaineris used inzmodel-linker.tsfor type-safe array and operand checks- The individual
isDataModelandisTypeDefchecks are used appropriately across the codebase, with no conflicting patterns - Type resolution logic in
zmodel-linker.tsintegrates well with the new guard
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check usage patterns of isMemberContainer and related type checks
# Test 1: Find direct usage of isMemberContainer
echo "=== Direct usage of isMemberContainer ==="
rg "isMemberContainer\(" --type ts
# Test 2: Find existing patterns that might need to be updated
echo -e "\n=== Existing patterns that might need updating ==="
rg "isDataModel\(|isTypeDef\(" --type ts
# Test 3: Look for potential breaking changes in type resolution
echo -e "\n=== Related type resolution patterns ==="
rg "resolveType|resolveReference" --type ts
Length of output: 11948
packages/schema/src/language-server/validator/typedef-validator.ts (2)
13-13: LGTM: Logical placement of attribute validation
The new validation call is well-positioned in the validation sequence, occurring after declaration validation but before field validation.
17-19: Verify auth model attribute validation
Since this PR aims to allow using types as auth models, let's verify that the validation properly handles auth-related attributes.
✅ Verification successful
Let me try a different approach to verify the auth attribute validation.
Let me try one final search to find the attribute application validator implementation and any auth-related validation logic.
Based on the search results, I can now provide a final response:
Attribute validation appears correctly implemented for types
The validation of attributes on TypeDef follows the same pattern as other declarations (DataModel, Enum, etc.) by using the shared validateAttributeApplication function. The implementation in AttributeApplicationValidator shows proper validation of attribute declarations and their targets. While we couldn't find specific auth-related attribute definitions in the codebase, the validation infrastructure is in place to handle them correctly when they are added.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for auth-related attribute definitions and their usage with types
echo "Searching for auth attribute definitions..."
ast-grep --pattern 'attribute $$$ @@@supportTypeDef $$$'
echo "Searching for auth attribute usage with types..."
ast-grep --pattern 'type $$$ {
@@@ $$$
}'
Length of output: 308
Script:
#!/bin/bash
# Search for attribute validation logic and auth-related code
echo "Searching for attribute validation implementation..."
rg -A 5 "validateAttributeApplication"
echo "Searching for auth-related attributes in the schema..."
rg -l "@auth|@model" --type-add 'schema:*.{sdl,graphql}' -tschema
echo "Looking for attribute validation tests..."
fd "test|spec" --exec rg -l "validateAttribute|TypeDefValidator"
Length of output: 7802
Script:
#!/bin/bash
# Search for attribute application validator implementation
echo "Searching attribute application validator..."
rg -A 10 "class AttributeApplicationValidator"
echo "Searching for auth-related validation..."
rg -A 5 "@auth|@model" --type typescript
Length of output: 1719
packages/schema/src/language-server/validator/schema-validator.ts (2)
2-2: LGTM: Import addition aligns with feature objective
The addition of getDataModelAndTypeDefs import is appropriate for supporting the new capability of using types as auth models.
39-40: Verify the impact of auth model validation changes
Let's ensure these changes maintain compatibility and are consistently implemented across the codebase.
✅ Verification successful
Based on the shell script results, I can now generate the final response:
Auth model validation changes are consistently implemented
The changes maintain compatibility as:
- The
getDataModelAndTypeDefsfunction is used consistently across the codebase for auth model validation - The auth model validation logic (allowing only one
@@authmodel) is implemented identically in both CLI (cli-util.ts) and language server (schema-validator.ts) - Test coverage exists specifically for multiple
@@authmodels validation inschema-validation.test.ts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of getDataModelAndTypeDefs and auth validation
# Check for other usages of getDataModelAndTypeDefs
echo "Checking usage of getDataModelAndTypeDefs:"
rg "getDataModelAndTypeDefs" -A 2
# Check for other auth model validations
echo "Checking other auth model validations:"
rg "@@auth.*model" -A 2
# Check for potential test cases
echo "Checking related test cases:"
fd -e test.ts -e spec.ts | xargs rg "@@auth"
Length of output: 5772
packages/schema/src/plugins/enhancer/enhance/auth-type-generator.ts (3)
8-9: LGTM: TypeDef import addition
The addition of TypeDef import is consistent with the PR objective of allowing types as auth models.
92-94: 🛠️ Refactor suggestion
Add type guard and verify getIdFields compatibility
The code assumes getIdFields can handle both DataModel and TypeDef. Two suggestions:
- Add a type guard for better type safety
- Verify getIdFields compatibility with TypeDef
Consider adding a type guard:
- if (model === authDecl.name) {
+ if (model === authDecl.name && (isDataModel(authDecl) || isTypeDef(authDecl))) {18-18: Verify type handling across the codebase
The function now accepts both DataModel and TypeDef, which is a significant change. Let's verify that all callers handle this correctly.
Also applies to: 27-27
✅ Verification successful
Let me gather more information about the implementation to ensure the type handling is correct.
Based on the gathered information, I can now generate the final response:
Function signature change is properly handled
The implementation safely handles both DataModel and TypeDef types:
- The only operations performed on
authDeclare type-agnostic (accessing name and using getIdFields) - The single caller in
enhance/index.tscorrectly passes theauthDeclparameter - Type checks using
isDataModelare used appropriately within the function for specific operations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for calls to generateAuthType to ensure they handle both types correctly
rg -A 3 "generateAuthType\("
# Search for any remaining references to authModel that might need updating
rg "authModel" --type ts
Length of output: 2525
Script:
#!/bin/bash
# Check the implementation of generateAuthType to see how it handles both types
ast-grep --pattern 'function generateAuthType($_) {
$$$
}'
# Check if there are any type-specific operations on authDecl
rg -B 2 -A 5 "authDecl\." packages/schema/src/plugins/enhancer/enhance/auth-type-generator.ts
# Look for isDataModel or similar type checks
rg "isDataModel|isTypeDef" packages/schema/src/plugins/enhancer/enhance/auth-type-generator.ts
Length of output: 1156
tests/integration/tests/enhancements/json/validation.test.ts (2)
1-1: LGTM: Appropriate test utilities imported
The import statement includes all necessary test utilities required for the test cases.
136-158: LGTM: Type-level attribute validation is well tested
The tests properly verify the whitelisting of type-level attributes. Let's verify that these are the only attributes being checked.
✅ Verification successful
Based on the search results from our previous attempts, I can now generate a final response:
Type-level attribute validation is correctly implemented
From the codebase analysis, we can see that:
@@authis the only type-level attribute that appears in type declarations@@allowis correctly rejected on type declarations and is only used with models- Other attributes like
@@index,@@unique,@@validateare specifically model-level attributes
The test correctly verifies the only permitted type-level attribute (@@auth) while rejecting unauthorized ones (@@allow), which aligns with the codebase's implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other type-level attributes in the codebase
ast-grep --pattern 'type $_ {
$$$
@@$_
$$$
}'
Length of output: 53
Script:
#!/bin/bash
# Let's try a different approach to find type-level attributes
# First, search for type declarations in test files
rg "type \w+ \{" tests/ -A 5
# Also search for documentation or schema files that might list allowed attributes
fd "schema|doc" --type f --exec rg "@@" {}
Length of output: 17553
Script:
#!/bin/bash
# Let's search for type-level attribute definitions in the source code
rg "type-level|@@auth|@@allow" src/ -B 2 -A 2
# Also search for validation or attribute-related code
fd "validator|attribute" --type f --exec rg "type.*@@" {}
Length of output: 154
packages/schema/src/utils/ast-utils.ts (2)
16-16: LGTM: Import additions are appropriate.
The new imports isTypeDef and TypeDef are necessary for the enhanced type definition support.
Also applies to: 20-20
317-324: 🛠️ Refactor suggestion
Verify handling of type definitions in transitive imports.
The function correctly handles data models and type definitions from loaded documents. However, there's a potential issue: when fromModel is provided, the transitive imports only include data models (via getAllDataModelsIncludingImports), potentially missing type definitions from imported files.
Let's verify if type definitions from imported files are needed:
If type definitions from imports are needed, consider updating the implementation:
const transitiveDataModels = getAllDataModelsIncludingImports(langiumDocuments, model);
+// Get type definitions from transitive imports
+const transitiveTypeDefs = resolveTransitiveImports(langiumDocuments, model)
+ .flatMap((m) => m.declarations.filter(isTypeDef));
transitiveDataModels.forEach((dm) => {
if (!allDataModels.includes(dm)) {
allDataModels.push(dm);
}
});
+transitiveTypeDefs.forEach((td) => {
+ if (!allDataModels.includes(td)) {
+ allDataModels.push(td);
+ }
+});packages/schema/src/language-server/validator/attribute-application-validator.ts (1)
72-74: LGTM! The type declaration validation looks good.
The added validation ensures that only attributes marked with @@@supportTypeDef can be applied to type declarations, which aligns with the PR objective of allowing types as auth models.
Let's verify the attributes that support type declarations:
✅ Verification successful
The validation for type declarations with @@@supportTypeDef is correctly implemented.
The search results confirm that:
-
The
@@@supportTypeDefattribute is properly defined in the standard library (packages/schema/src/res/stdlib.zmodel) and is used by several attributes including:@id,@default- Core Prisma attributes@@auth- Authentication attribute- All validation attributes (
@length,@email,@gt, etc.)
-
The validation is correctly implemented in
attribute-application-validator.tsto ensure only attributes marked with@@@supportTypeDefcan be used on type declarations. -
Integration tests demonstrate proper validation of attributes on type declarations, particularly in:
tests/integration/tests/plugins/zod.test.tstests/integration/tests/enhancements/with-policy/auth.test.ts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find attributes marked with @@@supportTypeDef to ensure proper usage.
# Test: Search for attributes with @@@supportTypeDef. Expect: List of attributes that can be applied to type declarations.
# Search for attribute declarations with @@@supportTypeDef
ast-grep --pattern 'attribute $name {
$$$
@@@supportTypeDef
$$$
}'
# Search for attribute applications on type declarations to ensure they have @@@supportTypeDef
rg -A 5 'type\s+\w+\s*{[\s\n]*@'
Length of output: 228
Script:
#!/bin/bash
# Let's try a different approach to find attributes with @@@supportTypeDef
# and type declarations with attributes
# Search for @@@supportTypeDef in the codebase
rg "@@@supportTypeDef"
# Search for type declarations with attributes using a more reliable pattern
rg "type\s+\w+.*\{" -A 5
# Search for attribute declarations
rg "attribute\s+\w+.*\{"
Length of output: 79035
packages/schema/src/cli/cli-util.ts (1)
2-2: LGTM: Import updated to support type definitions.
The import change from getDataModels to getDataModelAndTypeDefs aligns with the PR's objective of allowing types to be used as auth models.
packages/sdk/src/model-meta-generator.ts (3)
29-29: LGTM: Import renamed to reflect enhanced functionality.
The import rename from getAuthModel to getAuthDecl better reflects its enhanced capability to handle both data models and type definitions as auth models.
104-104: LGTM: Updated function call with type definitions.
The writeAuthModel call now correctly includes type definitions, enabling the auth model to be either a data model or a type definition.
165-166: LGTM: Enhanced auth model handling.
The implementation elegantly combines both data models and type definitions using the spread operator, allowing either to serve as the auth model. This aligns well with the PR objective of allowing types to be used as auth models.
Let's verify that the auth model can be properly resolved from both data models and type definitions:
✅ Verification successful
Based on the shell script results, I can now generate the final response:
Verified: Auth model resolution works correctly for both data models and type definitions
The implementation is correctly handling auth models from both data models and type definitions as evidenced by:
- The
getAuthDeclfunction is consistently used across the codebase to resolve auth models from combined data models and type definitions - Schema validation properly enforces single
@@authdeclaration constraint across both types - The stdlib.zmodel confirms
@@authattribute supports type definitions with@@@supportTypeDef - Multiple test cases demonstrate successful auth resolution in both scenarios
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify auth model resolution from both data models and type definitions
# Test: Search for auth-related attributes in both data models and type definitions
rg -A 5 '@auth|@@auth'
# Test: Search for auth model usage in the codebase
ast-grep --pattern 'getAuthDecl($$$)'
Length of output: 11476
packages/sdk/src/utils.ts (1)
134-134: LGTM! Type definitions properly updated.
The parameter types have been correctly extended to include TypeDef, maintaining consistency with the new feature while preserving existing functionality.
Also applies to: 141-150
packages/schema/src/res/stdlib.zmodel (3)
212-212: LGTM! Enhanced @id attribute to support type definitions
This is a valuable enhancement that allows type definitions to have ID fields, which is essential for using types as auth models. The attribute parameters remain appropriate for this extended use case.
539-539: LGTM! Core change enabling type definitions as auth models
This change is central to the PR's objective of allowing types to be used as auth models. However, let's verify the security implications of this enhancement.
✅ Verification successful
Type definition validation for @@auth attribute is properly implemented
The search results show that the codebase has proper validation logic in packages/schema/src/language-server/validator/attribute-application-validator.ts that checks whether attributes can be applied to type definitions. The validator explicitly checks for the @@@supportTypeDef marker before allowing attributes on type declarations, which ensures that only explicitly marked attributes (like @@auth) can be used with type definitions. This validation happens at compile-time, preventing any security issues from invalid attribute usage.
Additionally, the presence of auth-related tests in tests/integration/tests/enhancements/with-policy/auth.test.ts indicates test coverage for both runtime and compile-time auth behavior.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that type definitions with @@auth are properly validated
# and that there are corresponding security checks in the codebase.
# Test 1: Look for validation logic related to auth types
echo "Checking for auth type validation logic..."
rg -l "type.*@@auth|@@auth.*type" --type ts
# Test 2: Look for security-related checks for auth types
echo "Checking for security checks..."
rg -l "validateAuthType|checkAuthType|isValidAuthType" --type ts
# Test 3: Look for potential test coverage
echo "Checking for test coverage..."
rg -l "test.*auth.*type|describe.*auth.*type" --type ts
Length of output: 407
Script:
#!/bin/bash
# Let's try a different approach to find security validations and tests
# Look for auth-related validation code
echo "Searching for auth validation logic..."
rg -A 5 "validateAuth|checkAuth|isValidAuth" --type ts
# Look for auth-related tests
echo "Searching for auth tests..."
rg -A 5 "describe.*auth|test.*auth" --type ts
# Look for type definition handling in auth context
echo "Searching for type definition handling..."
rg -A 5 "supportTypeDef|handleTypeDef|validateTypeDef" --type ts
# Look for auth attribute handling
echo "Searching for auth attribute processing..."
ast-grep --pattern 'function $_($$_) {
$$$
auth
$$$
}'
Length of output: 4240
642-642: LGTM! Extended validation support to type definitions
The addition of validation support for type definitions is a logical enhancement that complements the auth model changes. Let's verify the validation behavior.
packages/schema/src/plugins/enhancer/enhance/index.ts (2)
6-7: LGTM: Import changes align with the new auth model approach.
The new imports getAuthDecl and getDataModelAndTypeDefs support the transition from model-only to type-inclusive auth declarations.
87-88: LGTM: Auth declaration changes support type-based auth models.
The transition from authModel to authDecl successfully enables using both models and types as auth declarations.
packages/language/src/zmodel.langium (5)
69-69: Addition of TypeDefField to ReferenceTarget Enhances Flexibility
Including TypeDefField in ReferenceTarget allows references to type definition fields, enhancing the expressiveness and flexibility of the language.
97-97: Extending MemberAccessTarget with TypeDefField
Adding TypeDefField to MemberAccessTarget enables member access expressions to utilize type definition fields, which is consistent with the intended language enhancements.
102-102: Updating MemberAccessExpr to Include TypeDefField
The grammar correctly updates the MemberAccessExpr to include TypeDefField in member access, aligning with the changes made to MemberAccessTarget.
108-110: Reinstating Operator Precedence Comments Improves Clarity
Including comments about binary operator precedence enhances the grammar's readability and helps maintainers understand the precedence rules.
188-189: Allowing Attributes in TypeDef Definitions
Including attributes+=DataModelAttribute in TypeDef allows type definitions to have attributes, enhancing consistency with DataModel and providing greater flexibility in defining types.
packages/schema/src/language-server/zmodel-scope.ts (2)
139-141: Verify the correctness of allowTypeDefScope logic
The variable allowTypeDefScope is introduced to conditionally include type definitions in the scope when the access starts with auth(). Please ensure that isAuthOrAuthMemberAccess accurately identifies all necessary cases where type definitions should be included, and that type definitions are not unintentionally exposed in other contexts.
233-240: Confirm getAuthDecl handles combined declarations correctly
In createScopeForAuth, you're retrieving both data models and type definitions using getAllLoadedAndReachableDataModelsAndTypeDefs and passing them to getAuthDecl. Please verify that getAuthDecl correctly processes this combined list and retrieves the appropriate authentication declaration without unintended side effects.
packages/schema/src/language-server/zmodel-linker.ts (6)
27-27: Import of TypeDefFieldType Is Appropriate
The addition of TypeDefFieldType in the imports ensures that type definitions are correctly handled in the type resolution logic.
39-39: Addition of Essential Imports for Authentication Resolution
The imported functions getAuthDecl, getModelFieldsWithBases, isAuthInvocation, and isFutureExpr from @zenstackhq/sdk are necessary for the updated authentication handling and type resolution.
57-58: New Utility Imports Support Expanded Functionality
The imports of getAllLoadedAndReachableDataModelsAndTypeDefs, getContainingDataModel, and isMemberContainer are appropriate and support the inclusion of type definitions in data model resolution and member access.
286-293: Ensuring Accurate Retrieval of Auth Declarations Including Type Definitions
The use of getAllLoadedAndReachableDataModelsAndTypeDefs to retrieve all declarations, including type definitions, before calling getAuthDecl(allDecls) is appropriate. This update allows authentication declarations to consider type definitions, enhancing the flexibility of the auth model.
324-324: Proper Use of isMemberContainer in Member Access Resolution
The condition now checks isMemberContainer(operandResolved.decl) to determine if the operand can contain members, correctly encompassing both DataModel and TypeDef elements.
342-342: Adjusting Collection Predicate Resolution with isMemberContainer
By using isMemberContainer(resolvedType.decl) in the condition, the code accurately identifies when a resolved type is an array of member containers, ensuring correct handling of collection predicates for both data models and type definitions.
tests/integration/tests/enhancements/with-policy/auth.test.ts (3)
910-921: Comprehensive test coverage of authorization scenarios
The added test cases effectively cover various scenarios, including:
- Attempting to create
Foowithout user context. - Creating with a banned user.
- Creating with an underage user.
- Successful creation with valid user data.
- Attempting to delete without proper permissions.
- Successful deletion with appropriate roles and permissions.
This comprehensive coverage ensures that the authorization logic behaves as expected.
871-885: Verify the use of @id annotation on type definitions
In the schema, User is defined as a type, and the field myId is annotated with @id. Typically, the @id attribute is used on model fields to designate primary keys. Please verify that using @id on a type field is supported and behaves as intended.
You can run the following script to search for other occurrences of @id annotations on type definitions in the codebase:
890-892: Ensure the policy conditions correctly handle nested properties
The authorization rules use nested properties of the auth() object, such as auth().profile.age and auth().roles?['DELETE' in permissions]. Since User is defined as a type, please confirm that the policy engine correctly resolves these nested properties within type definitions.
Please run the following script to check if nested properties in auth() are properly supported:
✅ Verification successful
Based on the search results, I can now generate a final response:
Nested properties in auth() policies are properly supported
The codebase shows extensive usage and testing of nested properties in auth() policies:
- Multiple test files validate the functionality of nested auth properties like
auth().profile.ageandauth().roles - The policy engine implementation in
packages/schema/src/plugins/enhancer/policy/handles these nested property accesses - Integration tests in
tests/integration/tests/enhancements/with-policy/specifically verify these scenarios
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the policy engine supports nested properties in 'auth()' when using type definitions.
# Expected: The policy engine should correctly handle nested properties without errors.
# Search for policy rules using nested 'auth()' properties.
rg -A 2 "auth\(\).*\.profile\.age" packages/language/src/*
Length of output: 166
Script:
#!/bin/bash
# Let's search for auth-related type definitions and policy handling
ast-grep --pattern 'type User {
$$$
}'
# Also search for auth-related policy implementations
rg -l "auth\(\)" --type ts
# Check for any policy engine implementations
rg -l "PolicyEngine|PolicyHandler" --type ts
# Look for auth property resolution or validation logic
rg "\.profile\.|\.roles\?" --type ts
Length of output: 18855
packages/schema/tests/schema/validation/attribute-validation.test.ts (1)
1154-1154: Test correctly validates unresolved reference error
The test ensures that accessing auth().email triggers the expected error message when the email field does not exist on the auth() object, verifying the validator's ability to catch unresolved member access.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/schema/src/language-server/zmodel-linker.ts (1)
325-325: Consider adding documentation for isMemberContainer usage.The replacement of
isDataModelwithisMemberContainermakes the code more generic and extensible to support both data models and type definitions. However, it would be helpful to document this change.Consider adding a comment explaining the member container concept and its implications for type resolution:
if (operandResolved && !operandResolved.array && isMemberContainer(operandResolved.decl)) { + // isMemberContainer checks if the declaration can contain resolvable members + // This includes both data models and type definitionsAlso applies to: 343-343
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
packages/schema/src/language-server/zmodel-linker.ts(7 hunks)tests/regression/tests/issue-756.test.ts(1 hunks)
🔇 Additional comments (2)
tests/regression/tests/issue-756.test.ts (1)
31-31: LGTM! Error message update aligns with the new type system.
The updated error message reflects the architectural improvement where member access targets can now be either DataModelField or TypeDefField, making the error handling more generic and consistent with the new feature of using types as auth models.
Let's verify the consistency of similar error messages:
✅ Verification successful
Error message is consistent with similar reference resolution errors in the codebase
The error message format "Could not resolve reference to X named 'Y'" is consistently used across the codebase for various unresolved references:
- MemberAccessTarget (in issue-756.test.ts and attribute-validation.test.ts)
- ReferenceTarget (in issue-925.test.ts and json/validation.test.ts)
- TypeDeclaration (in datamodel-validation.test.ts)
- FunctionDecl (in attribute-validation.test.ts)
- MemberAccessMember (in json/validation.test.ts)
The grammar definition in packages/language/src/zmodel.langium confirms that MemberAccessTarget is a union type of DataModelField | TypeDefField, making the error message accurate and aligned with the type system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for similar error message patterns across the codebase
# to ensure consistency in error reporting
# Search for similar error message patterns
rg "Could not resolve reference to \w+ named" -A 2 -B 2
# Search for other MemberAccessTarget related error messages
rg "MemberAccessTarget" -A 2 -B 2
Length of output: 10746
packages/schema/src/language-server/zmodel-linker.ts (1)
287-294: Verify auth model resolution changes across the codebase.
The changes to auth model resolution now support both data models and type definitions. Let's verify the usage to ensure consistency.
✅ Verification successful
Auth model resolution changes have been properly implemented
The verification shows that:
- No references to the old
getAuthModelfunction exist in the codebase - All usages of
getAuthDeclare consistent across the codebase, properly handling both data models and type definitions - The new
getAllLoadedAndReachableDataModelsAndTypeDefsfunction is correctly used in conjunction with auth declaration resolution - The changes are properly integrated in all relevant parts: language server, SDK, and plugins
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to old auth model functions
# and verify the new auth declaration usage.
echo "Checking for old auth model references..."
rg "getAuthModel|getAllLoadedAndReachableDataModels(?!AndTypeDefs)" -l
echo "Verifying auth declaration usage patterns..."
rg "getAuthDecl.*getAllLoadedAndReachableDataModelsAndTypeDefs" -A 3
Length of output: 648
Script:
#!/bin/bash
# Let's break down the search into simpler patterns
echo "Checking for old auth model references..."
rg "getAuthModel" -l
echo "Checking for getAllLoadedAndReachableDataModels without AndTypeDefs..."
rg "getAllLoadedAndReachableDataModels[^A]" -l
echo "Checking current usage of auth declarations..."
rg "getAuthDecl" -A 2
echo "Checking current usage of data models and type defs loading..."
rg "getAllLoadedAndReachableDataModelsAndTypeDefs" -A 2
Length of output: 5320
No description provided.