-
Couldn't load subscription status.
- Fork 11
feat: add permission documentation by using a custom decorator #1355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis pull request implements enhanced authorization in the GraphQL API. A new directive, Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant GraphQLServer
participant SchemaTransformer
participant Resolver
Client->>GraphQLServer: Send GraphQL request
GraphQLServer->>SchemaTransformer: Apply `@usePermissions` transformation
SchemaTransformer->>GraphQLServer: Return updated schema
GraphQLServer->>Resolver: Execute resolver with permission checks
Resolver->>GraphQLServer: Validate permissions
GraphQLServer->>Client: Return response
Possibly related PRs
Poem
🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
api/src/unraid-api/graph/resolvers/array/array.mutations.resolver.ts (1)
1-110: 💡 Verification agent🧩 Analysis chain
Verify test coverage for permission changes
While the implementation changes look good, it would be beneficial to ensure there's adequate test coverage for these permission changes, especially since the PR mentions removing previous authorization logic and tests.
🏁 Script executed:
#!/bin/bash # Check for test files related to the new permission directive echo "Checking for test files related to the new permission directive..." fd "use-permissions.*\.spec" --type fileLength of output: 185
Attention: Insufficient Test Coverage for Permission Changes
It appears that no test files related to the new permission directive (matching the pattern
use-permissions.*.spec) were found. Please ensure that tests are added to verify that the permission changes in the resolver functions (e.g.,setState,addDiskToArray,removeDiskFromArray, etc.) are correctly enforced.
- Verify that permission-related behavior is covered in your tests.
- Add test cases to confirm that unauthorized requests are properly rejected and authorized requests pass as expected.
🧹 Nitpick comments (3)
api/src/unraid-api/graph/resolvers/vms/vms.resolver.ts (1)
39-48: Consider refactoring duplicate method implementationThe
domainanddomainsmethods have identical implementations. Consider refactoring to avoid code duplication.@ResolveField(() => [VmDomain]) public async domains(): Promise<Array<VmDomain>> { try { return await this.vmsService.getDomains(); } catch (error) { throw new Error( `Failed to retrieve VM domains: ${error instanceof Error ? error.message : 'Unknown error'}` ); } } @ResolveField(() => [VmDomain]) -public async domain(): Promise<Array<VmDomain>> { - try { - return await this.vmsService.getDomains(); - } catch (error) { - throw new Error( - `Failed to retrieve VM domains: ${error instanceof Error ? error.message : 'Unknown error'}` - ); - } +public async domain(): Promise<Array<VmDomain>> { + return this.domains(); }api/src/unraid-api/graph/directives/use-permissions.directive.ts (2)
17-43: Consider DRY approach for repeated enum creation
The logic for creatingAuthActionVerbEnumandAuthPossessionEnumis nearly identical, differing only in the source enum. You can refactor it into a reusable helper to avoid duplication and improve maintainability.Example helper function:
function buildGraphQLEnum(enumObj: Record<string, string | number>, name: string, description: string) { const values = Object.entries(enumObj) .filter(([key]) => isNaN(Number(key))) .reduce((acc, [key]) => { acc[key] = { value: key }; return acc; }, {} as Record<string, { value: string }>); return new GraphQLEnumType({ name, description, values }); }
85-122: Avoid using console.log for tracing
Line 90 logs a debug message withconsole.log, which is typically discouraged in production environments. Use a more robust logging strategy (e.g.,Loggerfrom NestJS) or remove it if it’s purely for troubleshooting.- console.log('usePermissionsDirective', usePermissionsDirective); + // console.log removed or replaced with a proper logger
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
api/generated-schema.graphql(2 hunks)api/src/unraid-api/graph/directives/auth.directive.spec.ts(0 hunks)api/src/unraid-api/graph/directives/auth.directive.ts(0 hunks)api/src/unraid-api/graph/directives/use-permissions.directive.ts(1 hunks)api/src/unraid-api/graph/graph.module.ts(2 hunks)api/src/unraid-api/graph/resolvers/api-key/api-key.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/array.mutations.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/array.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/parity.mutations.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/parity.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/cloud/cloud.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/config/config.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/connect/connect-settings.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/disks/disks.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/display/display.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/flash/flash.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/info.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/logs/logs.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/network/network.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/online/online.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/owner/owner.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/registration/registration.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/servers/server.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/vars/vars.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/vms/vms.mutations.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/vms/vms.resolver.ts(1 hunks)api/src/unraid-api/graph/services/services.resolver.ts(1 hunks)api/src/unraid-api/graph/shares/shares.resolver.ts(1 hunks)api/src/unraid-api/graph/user/user.resolver.ts(1 hunks)api/src/unraid-api/graph/utils/auth-enum.utils.ts(0 hunks)
💤 Files with no reviewable changes (3)
- api/src/unraid-api/graph/directives/auth.directive.spec.ts
- api/src/unraid-api/graph/utils/auth-enum.utils.ts
- api/src/unraid-api/graph/directives/auth.directive.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
api/src/unraid-api/graph/resolvers/display/display.resolver.ts (1)
web/composables/gql/graphql.ts (1)
Display(501-524)
api/src/unraid-api/graph/graph.module.ts (1)
api/src/unraid-api/graph/directives/use-permissions.directive.ts (2)
UsePermissionsDirective(46-64)usePermissionsSchemaTransformer(86-122)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Build Web App
- GitHub Check: Build API
- GitHub Check: Test API
- GitHub Check: Cloudflare Pages
🔇 Additional comments (36)
api/src/unraid-api/graph/resolvers/vars/vars.resolver.ts (1)
4-8: Import source updated to use local directiveThe import statements have been modified to source
AuthActionVerb,AuthPossession, andUsePermissionsfrom the local directive module instead of the externalnest-authzpackage. This change aligns with the PR objective of introducing a custom decorator for permission management.api/src/unraid-api/graph/resolvers/flash/flash.resolver.ts (1)
4-8: Import source updated to use local directiveThe import statements have been updated to source authorization entities from the local directive module. This maintains consistency with other resolver files while transitioning to the custom permission system.
api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.ts (1)
3-7: Import source updated to use local directiveThe import statements have been modified to use the local directive implementation instead of the external package. This change supports the new permission documentation feature while maintaining the same interface.
api/src/unraid-api/graph/resolvers/array/array.mutations.resolver.ts (1)
4-8: Import source updated to use local directiveThe import statements have been modified to source permission-related entities from the local directive. This change is consistent with the updates in other resolver files and supports the custom permission documentation feature.
api/src/unraid-api/graph/shares/shares.resolver.ts (1)
4-8: Import path updated for authorization componentsThe imports have been correctly updated to use the new directive from the local codebase instead of the external package. This aligns with the PR objective to use a custom decorator for unified permission management.
api/src/unraid-api/graph/resolvers/online/online.resolver.ts (1)
3-7: Import path updated for authorization componentsThe imports have been updated to use the local custom directive implementation instead of the external package, consistent with the PR objectives.
api/src/unraid-api/graph/resolvers/network/network.resolver.ts (1)
4-8: Import path updated for authorization componentsThe imports for
AuthActionVerb,AuthPossession, andUsePermissionshave been correctly updated to use the new custom directive implementation.api/src/unraid-api/graph/user/user.resolver.ts (1)
4-8: Import path updated for authorization componentsThe imports have been updated to use the custom directive implementation, maintaining consistent import patterns across all resolver files.
api/src/unraid-api/graph/resolvers/vms/vms.resolver.ts (1)
3-7: Import path updated for authorization componentsThe imports have been correctly updated to use the new custom directive, consistent with all other resolver files in this PR.
api/src/unraid-api/graph/resolvers/logs/logs.resolver.ts (1)
4-8: Updated Permissions Imports:
The import forAuthActionVerb,AuthPossession, andUsePermissionshas been updated to source from@app/unraid-api/graph/directives/use-permissions.directive.jsinstead ofnest-authz. This change aligns with the new permissions management approach introduced in this PR.api/src/unraid-api/graph/services/services.resolver.ts (1)
6-10: Consolidated Permissions Decorator Imports:
The permission decorators are now imported from the new module, ensuring consistency across the codebase. The usage with the@UsePermissionsdecorator on theservicesquery remains unchanged and correct.api/src/unraid-api/graph/resolvers/cloud/cloud.resolver.ts (1)
7-11: Consistent Import Update for Cloud Resolver:
The updated import ofAuthActionVerb,AuthPossession, andUsePermissionsfrom@app/unraid-api/graph/directives/use-permissions.directive.jsis correctly implemented. This update supports the new GraphQL directive for permissions without affecting existing business logic.api/src/unraid-api/graph/resolvers/vms/vms.mutations.resolver.ts (1)
3-7: Refined Permissions Imports in VM Mutations Resolver:
The import statement for the permission-related entities is correctly updated to the new source. The application of@UsePermissionsacross the mutation resolver methods is clear and aligns with the centralized permissions approach.api/src/unraid-api/graph/resolvers/api-key/api-key.resolver.ts (1)
5-9: Updated API Key Resolver Imports:
The change to importAuthActionVerb,AuthPossession, andUsePermissionsfrom@app/unraid-api/graph/directives/use-permissions.directive.jsis properly executed. This update keeps the permission management consistent and maintains the integrity of the resolver logic.api/src/unraid-api/graph/resolvers/connect/connect-settings.resolver.ts (1)
10-14: Import source updated to use new permission directiveThe import statement has been updated to use the local custom directive implementation instead of the external
nest-authzpackage. This aligns with the PR objective of introducing a custom decorator that integrates both authorization and GraphQL directive logic.api/src/unraid-api/graph/resolvers/registration/registration.resolver.ts (1)
7-11: Import source updated to use local permission directiveThe imports have been correctly updated to use the new local custom directive implementation, consistent with the pattern across other resolver files.
api/src/unraid-api/graph/resolvers/config/config.resolver.ts (1)
4-8: Import source updated for permission-related entitiesThe import source for authorization-related entities has been updated to use the new custom directive implementation, maintaining consistency with other resolver files while preserving existing functionality.
api/src/unraid-api/graph/resolvers/info/info.resolver.ts (1)
16-20: Updated import source for permission decoratorThe import statement has been updated to use the new custom directive implementation. This change is consistent with the modifications in other resolver files and supports the PR's goal of consolidating permission management.
api/src/unraid-api/graph/graph.module.ts (3)
12-15: New imports added for permission directive functionalityThe imports for
UsePermissionsDirectiveandusePermissionsSchemaTransformerfrom the custom directive file look good. This is consistent with the PR objective to introduce a custom decorator for permission management.
57-57: Added UsePermissionsDirective to schema build optionsGood addition of the
UsePermissionsDirectiveto the directives array withinbuildSchemaOptions. This properly registers the directive with the GraphQL schema.
59-59: Added schema transformer for permissionsThe
transformSchemaproperty is correctly set tousePermissionsSchemaTransformer, which will enhance field descriptions with permission information. This is a clean way to implement documentation of permission requirements.api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts (1)
3-7: Updated imports to use custom permissions directiveThe import has been correctly updated to use the custom permissions directive instead of the
nest-authzpackage. This change maintains the same interface while transitioning to the new implementation.api/src/unraid-api/graph/resolvers/disks/disks.resolver.ts (1)
3-7: Updated imports to use custom permissions directiveThe imports have been properly updated to use the custom permissions implementation instead of
nest-authz. This change is consistent with the other resolver files and supports the centralized permission management approach.api/src/unraid-api/graph/resolvers/array/parity.mutations.resolver.ts (1)
5-9: Updated imports to use custom permissions directiveThe import statements have been correctly updated to use the custom permissions directive. This change is in line with the rest of the codebase updates and maintains a consistent approach to permission management.
api/src/unraid-api/graph/resolvers/array/array.resolver.ts (1)
4-8: Import updated to use custom permissions directiveThe updated import statement correctly points to the new local implementation of the permissions directive instead of the external
nest-authzpackage. This change properly aligns with the PR objective of using a custom decorator for permissions management.api/src/unraid-api/graph/resolvers/owner/owner.resolver.ts (1)
5-9: Import updated to use custom permissions directiveThe updated import statement correctly points to the new local implementation of the permissions directive. This change is consistent with the project's move toward using a custom decorator for unified permission management.
api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts (1)
5-9: Import updated to use custom permissions directiveThe updated import statement correctly points to the new local implementation of the permissions directive. This change maintains the same permission structure while moving to the custom implementation.
api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts (1)
5-9: Import updated to use custom permissions directiveThe updated import statement correctly points to the new local implementation of the permissions directive. This change aligns with the PR objective of using a custom decorator for permissions management.
api/src/unraid-api/graph/resolvers/array/parity.resolver.ts (2)
1-1: Removed unused importsUnused imports
ArgsandMutationhave been removed from@nestjs/graphql, improving code cleanliness.
6-10: Updated permission imports to use custom directiveThe permission-related imports have been updated to use the new custom directive implementation from the project instead of the external
nest-authzpackage. This aligns with the PR objective of introducing a custom decorator for permissions management.api/src/unraid-api/graph/resolvers/servers/server.resolver.ts (1)
5-9: Updated permission imports to use custom directiveThe permission-related imports have been updated to use the new custom directive implementation instead of the external
nest-authzpackage, maintaining consistency with other resolvers in the codebase.api/generated-schema.graphql (2)
5-15: Added new usePermissions directiveA new GraphQL directive
@usePermissionshas been added to document and enforce required permissions for fields. This directive takes three parameters:
action: The required action verb (AuthActionVerb)resource: The resource being accessed (String)possession: The possession type required (AuthPossession)This implementation allows for more explicit permission documentation directly in the GraphQL schema.
1607-1620: Added authorization enumsTwo new enums have been added to support the permission system:
AuthActionVerb: Defines possible actions (CREATE, UPDATE, DELETE, READ)AuthPossession: Defines possession types (ANY, OWN, OWN_ANY)These enums provide type safety and structure for the permission system, enhancing maintainability and improving documentation of available permission options.
api/src/unraid-api/graph/resolvers/display/display.resolver.ts (2)
8-12: Updated permission imports to use custom directiveThe permission-related imports have been updated to use the new custom directive implementation instead of the external
nest-authzpackage, maintaining consistency with other resolvers in the codebase.
71-71: Improved decorator formattingThe
@Querydecorator has been moved to its own line, improving code readability without changing functionality.api/src/unraid-api/graph/directives/use-permissions.directive.ts (1)
66-83:❓ Verification inconclusive
Handle potential resource string injection and check enum casing
- If
permissions.resourcecan come from external or untrusted inputs, injecting quotes or other characters could disrupt the generated directive. Consider sanitizing or escaping the string to avoid schema parsing errors.- Verify that the uppercase usage of
permissions.action.toUpperCase()andpermissions.possession.toUpperCase()matches the enum keys declared inAuthActionVerbEnumandAuthPossessionEnum. If the originalnest-authzenums use lowercase keys, you may need to adjust this approach.Below is a possible fix for escaping quotes in
permissions.resource:Directive( - `@usePermissions(action: ${permissions.action.toUpperCase()}, resource: "${permissions.resource}", possession: ${permissions.possession.toUpperCase()})` + (() => { + const sanitizedResource = permissions.resource.replace(/"/g, '\\"'); + return `@usePermissions(action: ${permissions.action.toUpperCase()}, resource: "${sanitizedResource}", possession: ${permissions.possession.toUpperCase()})`; + })() )(target, propertyKey, descriptor);
Attention: Sanitize the resource string and verify enum casing
The code currently interpolatespermissions.resourcedirectly into the directive string. This could lead to injection issues if the value isn’t properly sanitized. Additionally, please confirm that applying.toUpperCase()to the enum properties (for bothactionandpossession) is consistent with the case expected byAuthActionVerbEnumandAuthPossessionEnum.
Sanitize Input: If
permissions.resourcemay contain user-controlled data, ensure that any quotes or special characters are escaped. For example:Directive( - `@usePermissions(action: ${permissions.action.toUpperCase()}, resource: "${permissions.resource}", possession: ${permissions.possession.toUpperCase()})` + (() => { + const sanitizedResource = permissions.resource.replace(/"/g, '\\"'); + return `@usePermissions(action: ${permissions.action.toUpperCase()}, resource: "${sanitizedResource}", possession: ${permissions.possession.toUpperCase()})`; + })() )(target, propertyKey, descriptor);Verify Enum Casing: Double-check that the uppercase conversion of the enum values is appropriate for your project's definitions. If the consuming logic or external services expect lowercase keys, adjust accordingly.
Please review these points and verify their impact on the rest of the codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
api/src/unraid-api/graph/directives/use-permissions.directive.ts (1)
87-123: Consider throwing an error for missing directive arguments.
Currently, a warning is logged ifaction,resource, orpossessionis missing. You might want to throw an error instead so that invalid schemas fail early rather than silently passing through.- console.warn( - `UsePermissions directive on ${typeName}.${fieldName} is missing required arguments.` - ); + throw new Error( + `UsePermissions directive on ${typeName}.${fieldName} is missing required arguments.` + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (2)
api/src/unraid-api/graph/directives/use-permissions.directive.ts(1 hunks)api/src/unraid-api/graph/resolvers/vms/vms.resolver.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/unraid-api/graph/resolvers/vms/vms.resolver.ts
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test API
- GitHub Check: Build Web App
- GitHub Check: Build API
- GitHub Check: Cloudflare Pages
🔇 Additional comments (4)
api/src/unraid-api/graph/directives/use-permissions.directive.ts (4)
13-14: Good practice to re-export essential types.
Re-exportingAuthActionVerbandAuthPossessionsimplifies imports for downstream modules.
47-66: Directive arguments look consistent with best practices.
The directive properly validates required arguments (action,resource,possession) at the schema level.
68-85: Check for potential string injection in directive usage.
You’re embedding unescapedpermissions.resourceinto a directive string. If these strings originate from user input, consider sanitizing or validating them to avoid GraphQL parse errors.
16-32:✅ Verification successful
Validate handling of numeric enum members.
The logic omits numeric enum keys by filtering them out. This ensures a tidy GraphQL enum, but make sure no numeric-based keys need to be included.Run the following script to check if
AuthActionVerborAuthPossessioncontain numeric keys that should not be ignored:
🏁 Script executed:
#!/bin/bash fd -t f | xargs rg "enum Auth(A|P)(ctionVerb|ossession)"Length of output: 160
Numeric Enum Filtering Verified
I confirmed that the generated schema (
api/generated-schema.graphql) defines bothAuthActionVerbandAuthPossessionwithout any numeric keys. The filtering logic inuse-permissions.directive.ts(lines 16-32) correctly omits numeric enum keys, and there’s no evidence that any necessary numeric-based keys are being erroneously removed.
api/src/unraid-api/graph/directives/use-permissions.directive.ts
Outdated
Show resolved
Hide resolved
* usePermissions applies both authz + graphQL directive logic to allow permissions and documentation in one place
072c4b1 to
8590471
Compare
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
api/src/unraid-api/graph/directives/use-permissions.directive.ts (1)
114-120: Slight simplification for string concatenationYou could simplify to something like:
fieldConfig.description = permissionDoc + (fieldConfig.description ?? '')This was previously mentioned in a past review; marking it as duplicate.
🧹 Nitpick comments (3)
api/src/unraid-api/graph/directives/use-permissions.directive.ts (3)
16-32:buildGraphQLEnumutility functionImplementation cleanly builds a new
GraphQLEnumTypeby filtering out numeric enum keys. This is a standard approach for string-based enums in TypeScript. Consider adding unit tests to validate that numeric keys are skipped and string keys are included.
68-85:UsePermissionsdecoratorCombining NestAuthz's
UsePermissionswith the GraphQL directive in one decorator is elegant. No immediate issues. Consider adding test coverage to ensure the decorator is applied and the directive is properly attached in the schema.
87-126: Schema transformer for permission documentationAppending permission details to the field description is a nice touch. Consider throwing an error — rather than only logging a warning — if required arguments (
action,resource,possession) are missing. This ensures that misconfigurations are caught early.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (35)
api/dev/Unraid.net/myservers.cfg(1 hunks)api/generated-schema.graphql(4 hunks)api/src/unraid-api/graph/directives/auth.directive.spec.ts(0 hunks)api/src/unraid-api/graph/directives/auth.directive.ts(0 hunks)api/src/unraid-api/graph/directives/use-permissions.directive.ts(1 hunks)api/src/unraid-api/graph/graph.module.ts(2 hunks)api/src/unraid-api/graph/resolvers/api-key/api-key.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/array.mutations.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/array.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/parity.mutations.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/array/parity.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/cloud/cloud.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/config/config.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/connect/connect-settings.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/disks/disks.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/display/display.resolver.ts(2 hunks)api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/flash/flash.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/info/info.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/logs/logs.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/network/network.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/online/online.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/owner/owner.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/registration/registration.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/servers/server.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/vars/vars.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/vms/vms.mutations.resolver.ts(1 hunks)api/src/unraid-api/graph/resolvers/vms/vms.resolver.ts(3 hunks)api/src/unraid-api/graph/services/services.resolver.ts(1 hunks)api/src/unraid-api/graph/shares/shares.resolver.ts(1 hunks)api/src/unraid-api/graph/user/user.resolver.ts(1 hunks)api/src/unraid-api/graph/utils/auth-enum.utils.ts(0 hunks)
💤 Files with no reviewable changes (3)
- api/src/unraid-api/graph/utils/auth-enum.utils.ts
- api/src/unraid-api/graph/directives/auth.directive.spec.ts
- api/src/unraid-api/graph/directives/auth.directive.ts
✅ Files skipped from review due to trivial changes (1)
- api/dev/Unraid.net/myservers.cfg
🚧 Files skipped from review as they are similar to previous changes (29)
- api/src/unraid-api/graph/resolvers/vars/vars.resolver.ts
- api/src/unraid-api/graph/resolvers/flash/flash.resolver.ts
- api/src/unraid-api/graph/resolvers/connect/connect-settings.resolver.ts
- api/src/unraid-api/graph/user/user.resolver.ts
- api/src/unraid-api/graph/resolvers/cloud/cloud.resolver.ts
- api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts
- api/src/unraid-api/graph/resolvers/online/online.resolver.ts
- api/src/unraid-api/graph/resolvers/array/array.mutations.resolver.ts
- api/src/unraid-api/graph/resolvers/info/info.resolver.ts
- api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.ts
- api/src/unraid-api/graph/resolvers/vms/vms.mutations.resolver.ts
- api/src/unraid-api/graph/resolvers/disks/disks.resolver.ts
- api/src/unraid-api/graph/graph.module.ts
- api/src/unraid-api/graph/resolvers/array/parity.mutations.resolver.ts
- api/src/unraid-api/graph/resolvers/config/config.resolver.ts
- api/src/unraid-api/graph/services/services.resolver.ts
- api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts
- api/src/unraid-api/graph/shares/shares.resolver.ts
- api/src/unraid-api/graph/resolvers/connect/connect.resolver.ts
- api/src/unraid-api/graph/resolvers/api-key/api-key.resolver.ts
- api/src/unraid-api/graph/resolvers/display/display.resolver.ts
- api/src/unraid-api/graph/resolvers/network/network.resolver.ts
- api/src/unraid-api/graph/resolvers/array/array.resolver.ts
- api/src/unraid-api/graph/resolvers/registration/registration.resolver.ts
- api/src/unraid-api/graph/resolvers/array/parity.resolver.ts
- api/src/unraid-api/graph/resolvers/servers/server.resolver.ts
- api/src/unraid-api/graph/resolvers/owner/owner.resolver.ts
- api/src/unraid-api/graph/resolvers/logs/logs.resolver.ts
- api/src/unraid-api/graph/resolvers/vms/vms.resolver.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (8)
api/generated-schema.graphql (5)
1427-1428: Additional query field looks goodNo issues spotted; the new query field
vms: Vms!with the accompanying docstring is suitably clear.
1438-1438: NewhealthfieldThe new
health: String!field is straightforward. Ensure a corresponding resolver or data source is implemented, if not already done.
1608-1609: No functional changes hereOnly a closing brace and blank line. No concerns to flag.
1610-1616: NewAuthActionVerbenum definitionEnum members (
CREATE,UPDATE,DELETE,READ) adequately cover the typical actions.
1618-1622: NewAuthPossessionenum definitionMembers (
ANY,OWN,OWN_ANY) look appropriate to define access possession.api/src/unraid-api/graph/directives/use-permissions.directive.ts (3)
1-14: Imports and re-export statements look goodThe chosen imports are appropriate. Re-exporting
AuthActionVerbandAuthPossessionfromnest-authzfor convenience is a tidy approach.
34-46: Creation ofAuthActionVerbEnumandAuthPossessionEnumUsing the same utility function ensures consistency. Nicely documented.
48-66: Definition ofUsePermissionsDirectiveDirective arguments comprehensively capture needed properties for permission checks. The field descriptions are helpful for clarity in the resulting schema.
* usePermissions applies both authz + graphQL directive logic to allow permissions and documentation in one place <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a new GraphQL permission directive that documents required permissions for API fields. - Added enums for defining action verbs (create, update, delete, read) and possession types (any, own, own any) to enable granular access control. - Added a new health field to the Query type for improved API health monitoring. - **Chores** - Consolidated permission handling by updating import sources and retiring legacy authorization tests and code, enhancing overall maintainability. - Updated configuration version in the API settings. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
🤖 I have created a release *beep* *boop* --- ## [4.7.0](v4.6.6...v4.7.0) (2025-04-24) ### Features * add basic docker network listing ([#1317](#1317)) ([c4fdff8](c4fdff8)) * add permission documentation by using a custom decorator ([#1355](#1355)) ([45ecab6](45ecab6)) * basic vm controls ([#1293](#1293)) ([bc3ca92](bc3ca92)) * code first graphql ([#1347](#1347)) ([f5724ab](f5724ab)) ### Bug Fixes * container names always null ([#1335](#1335)) ([8a5b238](8a5b238)) * **deps:** update all non-major dependencies ([#1337](#1337)) ([2345732](2345732)) * hide reboot notice for patch releases ([#1341](#1341)) ([4b57439](4b57439)) * move docker mutations to the mutations resolver ([#1333](#1333)) ([1bbe7d2](1bbe7d2)) * PR build issue ([457d338](457d338)) * remove some unused fields from the report object ([#1342](#1342)) ([cd323ac](cd323ac)) * sso unreliable if API outputs more than raw json ([#1353](#1353)) ([e65775f](e65775f)) * vms now can detect starting of libvirt and start local hypervisor ([#1356](#1356)) ([ad0f4c8](ad0f4c8)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit
New Features
Chores