Use Policies rather then overriding can*() functions
#1837
Conversation
📝 WalkthroughWalkthroughThis PR refactors authorization by removing Changes
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (14)
app/Policies/Server/ServerPolicies.php (1)
9-14: Consider adding explicit policy methods for standard CRUD operations.While the
before()method handles permission-based authorization, the policy relies on theDefaultPoliciestrait for standard CRUD operations (viewAny,view,create,update,delete). According to the PR objectives, explicit policy methods with response objects are preferred over relying on fallback behavior.Consider adding explicit methods like:
public function viewAny(User $user): bool { return true; // or your specific logic } public function view(User $user, Server $server): bool { return true; // handled by before() in most cases } public function create(User $user): bool { // Define explicit create authorization return $user->canCreateServers(); } // Additional methods as neededThis would make the authorization behavior more explicit and align with the broader refactoring goals.
app/Policies/Admin/MountPolicy.php (1)
14-28: Harden before() signature to avoid arg count/type pitfalls.Gate may call before($user, $ability, ...$args) with zero or a class-string. A required third param can trigger an ArgumentCountError. Make it nullable with a default and guard type.
Apply:
- public function before(User $user, string $ability, string|Mount $mount): ?bool + public function before(User $user, string $ability, string|Mount|null $mount = null): ?bool { - // For "viewAny" the $mount param is the class name - if (is_string($mount)) { + // For "viewAny" the $mount param is the class name; some calls may pass no subject. + if (! $mount instanceof Mount) { return null; } - foreach ($mount->nodes as $node) { + foreach ($mount->nodes as $node) { if (!$user->canTarget($node)) { return false; } } return null; }Optional:
$mount->loadMissing('nodes');before the loop to reduce repeated lazy load.app/Policies/Admin/DefaultPolicies.php (1)
10-44: Consider returning policy Response objects for richer feedback.Current bool returns work, but swapping to Response enables messages/reasons and aligns with the PR’s “policy response objects” goal.
Example:
+use Illuminate\Auth\Access\Response; @@ - public function viewAny(User $user): bool + public function viewAny(User $user): bool|\Illuminate\Auth\Access\Response { - return $user->can('viewList ' . $this->modelName); + return $user->can('viewList ' . $this->modelName) + ? Response::allow() + : Response::deny(); }Repeat similarly for view/create/update/delete/replicate if you want consistent messaging. Otherwise, leaving bools is fine.
app/Filament/Server/Resources/Schedules/ScheduleResource.php (1)
338-339: Use getEditAuthorizationResponse() instead of canEdit() for consistency with policy responses.Aligns the UI logic with the new authorization flow and avoids the legacy can* API.
Apply:
- ViewAction::make() - ->hidden(fn ($record) => static::canEdit($record)), + ViewAction::make() + ->hidden(fn (Schedule $record) => static::getEditAuthorizationResponse($record)->allowed()),app/Policies/Server/UserPolicy.php (3)
23-26: Silence PHPMD UnusedFormalParameter for $record or consume itEither suppress or mark it consumed; keeps signatures intact for future use.
Option A — suppression:
public static function edit(Model $record): bool { + /** @noinspection PhpUnusedParameterInspection */ + /** @SuppressWarnings(PHPMD.UnusedFormalParameter) */ return user()?->can(Permission::ACTION_USER_UPDATE, Filament::getTenant()) ?? false; } @@ public static function delete(Model $record): bool { + /** @noinspection PhpUnusedParameterInspection */ + /** @SuppressWarnings(PHPMD.UnusedFormalParameter) */ return user()?->can(Permission::ACTION_USER_DELETE, Filament::getTenant()) ?? false; }Option B — explicitly consume:
- public static function edit(Model $record): bool + public static function edit(Model $record): bool { + // parameter currently unused but kept for signature parity + unset($record); return user()?->can(Permission::ACTION_USER_UPDATE, Filament::getTenant()) ?? false; }Also applies to: 28-31
23-31: Provide update() alias for broader compatibilityMany callers/Gate conventions use update(). Add a thin alias calling edit().
public static function edit(Model $record): bool { return user()?->can(Permission::ACTION_USER_UPDATE, Filament::getTenant()) ?? false; } + + public static function update(Model $record): bool + { + return self::edit($record); + }
13-31: Consider returning Laravel policy Responses to carry reasonsIf PR goal targets richer responses, these methods can return Response::allow()/deny() instead of bools, or let Resource get*AuthorizationResponse() wrap reasons. Confirm intended design.
app/Policies/Server/SchedulePolicy.php (2)
23-26: Suppress or consume $record to appease PHPMDKeep signature parity; silence the warning or unset the param.
public static function edit(Model $record): bool { + /** @noinspection PhpUnusedParameterInspection */ + /** @SuppressWarnings(PHPMD.UnusedFormalParameter) */ return user()?->can(Permission::ACTION_SCHEDULE_UPDATE, Filament::getTenant()) ?? false; } @@ public static function delete(Model $record): bool { + /** @noinspection PhpUnusedParameterInspection */ + /** @SuppressWarnings(PHPMD.UnusedFormalParameter) */ return user()?->can(Permission::ACTION_SCHEDULE_DELETE, Filament::getTenant()) ?? false; }Also applies to: 28-31
23-31: Add update() alias and optional view() for parity
- Provide update() alias for compatibility.
- Optionally add view() mirroring read permission to match DatabasePolicy.
public static function edit(Model $record): bool { return user()?->can(Permission::ACTION_SCHEDULE_UPDATE, Filament::getTenant()) ?? false; } + + public static function update(Model $record): bool + { + return self::edit($record); + } + + public static function view(Model $record): bool + { + return user()?->can(Permission::ACTION_SCHEDULE_READ, Filament::getTenant()) ?? false; + }app/Policies/Server/AllocationPolicy.php (2)
23-26: Suppress or consume $record parameterSilence PHPMD while preserving signatures.
public static function edit(Model $record): bool { + /** @noinspection PhpUnusedParameterInspection */ + /** @SuppressWarnings(PHPMD.UnusedFormalParameter) */ return user()?->can(Permission::ACTION_ALLOCATION_UPDATE, Filament::getTenant()) ?? false; } @@ public static function delete(Model $record): bool { + /** @noinspection PhpUnusedParameterInspection */ + /** @SuppressWarnings(PHPMD.UnusedFormalParameter) */ return user()?->can(Permission::ACTION_ALLOCATION_DELETE, Filament::getTenant()) ?? false; }Also applies to: 28-31
23-31: Add update() alias and optional view()Improves compatibility and parity across policies.
public static function edit(Model $record): bool { return user()?->can(Permission::ACTION_ALLOCATION_UPDATE, Filament::getTenant()) ?? false; } + + public static function update(Model $record): bool + { + return self::edit($record); + } + + public static function view(Model $record): bool + { + return user()?->can(Permission::ACTION_ALLOCATION_READ, Filament::getTenant()) ?? false; + }app/Policies/Server/DatabasePolicy.php (3)
18-21: Suppress or consume $record to resolve PHPMD warningsSame approach as others to avoid UnusedFormalParameter.
public static function view(Model $record): bool { + /** @noinspection PhpUnusedParameterInspection */ + /** @SuppressWarnings(PHPMD.UnusedFormalParameter) */ return user()?->can(Permission::ACTION_DATABASE_READ, Filament::getTenant()) ?? false; } @@ public static function edit(Model $record): bool { + /** @noinspection PhpUnusedParameterInspection */ + /** @SuppressWarnings(PHPMD.UnusedFormalParameter) */ return user()?->can(Permission::ACTION_DATABASE_UPDATE, Filament::getTenant()) ?? false; } @@ public static function delete(Model $record): bool { + /** @noinspection PhpUnusedParameterInspection */ + /** @SuppressWarnings(PHPMD.UnusedFormalParameter) */ return user()?->can(Permission::ACTION_DATABASE_DELETE, Filament::getTenant()) ?? false; }Also applies to: 28-31, 33-36
28-31: Add update() alias to complement edit()Aligns with common ability naming.
public static function edit(Model $record): bool { return user()?->can(Permission::ACTION_DATABASE_UPDATE, Filament::getTenant()) ?? false; } + + public static function update(Model $record): bool + { + return self::edit($record); + }
11-11: Optional: constant instead of mutable propertyIf $modelName is constant metadata, prefer a class constant for clarity and immutability.
- protected string $modelName = 'database'; + public const MODEL_NAME = 'database';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
app/Filament/Server/Resources/Activities/ActivityResource.php(0 hunks)app/Filament/Server/Resources/Allocations/AllocationResource.php(0 hunks)app/Filament/Server/Resources/Backups/BackupResource.php(0 hunks)app/Filament/Server/Resources/Databases/DatabaseResource.php(0 hunks)app/Filament/Server/Resources/Files/FileResource.php(0 hunks)app/Filament/Server/Resources/Schedules/ScheduleResource.php(1 hunks)app/Filament/Server/Resources/Users/UserResource.php(0 hunks)app/Policies/Admin/ApiKeyPolicy.php(1 hunks)app/Policies/Admin/DatabaseHostPolicy.php(1 hunks)app/Policies/Admin/DefaultPolicies.php(1 hunks)app/Policies/Admin/EggPolicy.php(1 hunks)app/Policies/Admin/MountPolicy.php(1 hunks)app/Policies/Admin/NodePolicy.php(1 hunks)app/Policies/Admin/RolePolicy.php(1 hunks)app/Policies/Admin/UserPolicy.php(1 hunks)app/Policies/Admin/WebhookConfigurationPolicy.php(1 hunks)app/Policies/DatabasePolicy.php(0 hunks)app/Policies/Server/ActivityLogPolicy.php(1 hunks)app/Policies/Server/AllocationPolicy.php(1 hunks)app/Policies/Server/BackupPolicy.php(1 hunks)app/Policies/Server/DatabasePolicy.php(1 hunks)app/Policies/Server/DefaultPolicies.php(1 hunks)app/Policies/Server/FilePolicy.php(1 hunks)app/Policies/Server/SchedulePolicy.php(1 hunks)app/Policies/Server/ServerPolicies.php(1 hunks)app/Policies/Server/UserPolicy.php(1 hunks)app/Providers/AppServiceProvider.php(2 hunks)
💤 Files with no reviewable changes (7)
- app/Filament/Server/Resources/Activities/ActivityResource.php
- app/Filament/Server/Resources/Files/FileResource.php
- app/Policies/DatabasePolicy.php
- app/Filament/Server/Resources/Users/UserResource.php
- app/Filament/Server/Resources/Backups/BackupResource.php
- app/Filament/Server/Resources/Databases/DatabaseResource.php
- app/Filament/Server/Resources/Allocations/AllocationResource.php
🧰 Additional context used
🧬 Code graph analysis (8)
app/Policies/Server/DatabasePolicy.php (3)
app/Models/Permission.php (1)
Permission(11-221)app/Models/Database.php (1)
Database(29-195)app/Policies/Server/AllocationPolicy.php (4)
viewAny(13-16)create(18-21)edit(23-26)delete(28-31)
app/Policies/Server/SchedulePolicy.php (4)
app/Models/Permission.php (1)
Permission(11-221)app/Policies/Server/ActivityLogPolicy.php (1)
viewAny(12-15)app/Policies/Server/AllocationPolicy.php (4)
viewAny(13-16)create(18-21)edit(23-26)delete(28-31)app/Policies/Server/DatabasePolicy.php (4)
viewAny(13-16)create(23-26)edit(28-31)delete(33-36)
app/Policies/Server/BackupPolicy.php (3)
app/Models/Permission.php (1)
Permission(11-221)app/Policies/Server/ActivityLogPolicy.php (1)
viewAny(12-15)app/Policies/Server/AllocationPolicy.php (3)
viewAny(13-16)create(18-21)delete(28-31)
app/Policies/Server/ActivityLogPolicy.php (3)
app/Models/Permission.php (1)
Permission(11-221)app/Policies/Server/AllocationPolicy.php (1)
viewAny(13-16)app/Policies/Server/DatabasePolicy.php (2)
viewAny(13-16)view(18-21)
app/Policies/Server/UserPolicy.php (3)
app/Models/Permission.php (1)
Permission(11-221)app/Policies/Server/ActivityLogPolicy.php (1)
viewAny(12-15)app/Policies/Server/AllocationPolicy.php (4)
viewAny(13-16)create(18-21)edit(23-26)delete(28-31)
app/Policies/Server/FilePolicy.php (2)
app/Models/Permission.php (1)
Permission(11-221)app/Policies/Server/AllocationPolicy.php (4)
viewAny(13-16)create(18-21)edit(23-26)delete(28-31)
app/Policies/Server/AllocationPolicy.php (7)
app/Models/Permission.php (1)
Permission(11-221)app/Policies/Server/ActivityLogPolicy.php (1)
viewAny(12-15)app/Policies/Server/BackupPolicy.php (3)
viewAny(13-16)create(18-21)delete(23-26)app/Policies/Server/DatabasePolicy.php (4)
viewAny(13-16)create(23-26)edit(28-31)delete(33-36)app/Policies/Server/FilePolicy.php (4)
viewAny(13-16)create(18-21)edit(23-26)delete(28-31)app/Policies/Server/SchedulePolicy.php (4)
viewAny(13-16)create(18-21)edit(23-26)delete(28-31)app/Policies/Server/UserPolicy.php (4)
viewAny(13-16)create(18-21)edit(23-26)delete(28-31)
app/Providers/AppServiceProvider.php (1)
app/Models/User.php (2)
User(94-492)isRootAdmin(379-382)
🪛 PHPMD (2.15.0)
app/Policies/Server/DatabasePolicy.php
18-18: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
28-28: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
33-33: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
app/Policies/Server/SchedulePolicy.php
23-23: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
28-28: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
app/Policies/Server/BackupPolicy.php
23-23: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
app/Policies/Server/UserPolicy.php
23-23: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
28-28: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
app/Policies/Server/FilePolicy.php
23-23: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
28-28: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
app/Policies/Server/AllocationPolicy.php
23-23: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
28-28: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
app/Providers/AppServiceProvider.php
173-173: Avoid unused parameters such as '$ability'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (14)
app/Policies/Server/ServerPolicies.php (3)
3-3: LGTM! Good namespace organization.Moving server policies to the
App\Policies\Servernamespace improves code organization and aligns with the broader restructuring effort.
18-45: Authorization logic is well-structured.The
before()method correctly implements the authorization hierarchy:
- Owners receive full permissions
- Subusers are authorized based on their permission array
- Node targeting is enforced as a hard requirement
- Defers to default policy methods when none of the special cases apply
Note: When a subuser exists but lacks the specific permission, the method falls through to the node check and then returns
null. This delegates the final decision to individual policy methods (or the DefaultPolicies trait), which appears intentional for the authorization flow.
11-11: The DefaultPolicies trait correctly implements deny-by-default behavior for undefined policy methods.The trait's
__call()method intercepts any undefined method calls and returnsvoid(implicitly falsy), which causes Laravel's authorization to deny access. This is the intended "hack" to prevent unintended authorization bypasses. The implementation is secure and no changes are needed.app/Policies/Admin/RolePolicy.php (1)
3-3: Namespace move looks good.No behavioral change; aligns with Admin-scoped policies.
app/Policies/Admin/ApiKeyPolicy.php (1)
3-3: LGTM on namespace shift.Consistent with Admin policy layout.
app/Policies/Admin/EggPolicy.php (1)
3-3: OK to move under Admin namespace.Matches the new policy resolution scheme.
app/Policies/Admin/WebhookConfigurationPolicy.php (1)
3-3: Namespace adjustment approved.Consistent with the Admin policy grouping.
app/Policies/Admin/NodePolicy.php (1)
3-3: LGTM! Clean namespace refactor.The namespace change organizes admin policies into a dedicated subdirectory, aligning with the broader policy reorganization in this PR.
app/Policies/Admin/DatabaseHostPolicy.php (1)
3-3: LGTM! Consistent namespace refactor.The namespace change aligns with the policy reorganization pattern applied across all admin policies.
app/Policies/Admin/UserPolicy.php (1)
3-3: LGTM! Namespace refactor.Consistent with the policy reorganization moving admin policies to
App\Policies\Admin.app/Providers/AppServiceProvider.php (2)
173-173: LGTM! Cleaner arrow function syntax.The refactor to an arrow function is more concise while preserving the same logic (root admins bypass all checks).
Note: The static analysis warning about unused
$abilityis a false positive—Laravel'sGate::beforesignature requires this parameter.
175-188: LGTM! Dynamic policy resolution aligns with namespace refactors.The dynamic policy resolution based on the active Filament panel enables per-panel authorization logic. The implementation properly checks for class existence and skips when the panel ID is 'App', which complements the namespace reorganization of policies into
AdminandServersubdirectories.app/Policies/Server/BackupPolicy.php (1)
13-26: LGTM! Consistent server policy implementation.The policy methods correctly check permissions against the Filament tenant context, following the same pattern as other server policies in this PR.
Note: The static analysis warning about unused
$recordis likely a false positive—the parameter may be required for Laravel's policy method resolution or reserved for future use.app/Policies/Server/FilePolicy.php (1)
13-31: LGTM! Consistent file policy implementation.The policy methods correctly delegate to permission checks with appropriate
ACTION_FILE_*constants, following the same pattern asAllocationPolicyand other server policies.Note: The static analysis warnings about unused
$recordparameters are likely false positives—these parameters may be required by Laravel's policy method resolution or reserved for future enhancements.
rmartinoscar
force-pushed
the
chore/policies
branch
from
9127b9e to
42ce06e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (7)
app/Policies/Server/UserPolicy.php (2)
11-11: Consider removing unused$modelNameproperty.The
$modelNameproperty is not referenced in this policy class. Unless used by reflection (which doesn't appear to be the case), it should be removed.Apply this diff if unused:
- protected string $modelName = 'user'; - public function viewAny(): bool
13-31: Add null coalescing to prevent type errors when user is unauthenticated.The nullsafe operator
user()?->can(...)returnsnullwhenuser()isnull, violating theboolreturn type and potentially causing a fatal type error.Apply this diff:
public function viewAny(): bool { - return user()?->can(Permission::ACTION_USER_READ, Filament::getTenant()); + return user()?->can(Permission::ACTION_USER_READ, Filament::getTenant()) ?? false; } public function create(): bool { - return user()?->can(Permission::ACTION_USER_CREATE, Filament::getTenant()); + return user()?->can(Permission::ACTION_USER_CREATE, Filament::getTenant()) ?? false; } public function edit(Model $record): bool { - return user()?->can(Permission::ACTION_USER_UPDATE, Filament::getTenant()); + return user()?->can(Permission::ACTION_USER_UPDATE, Filament::getTenant()) ?? false; } public function delete(Model $record): bool { - return user()?->can(Permission::ACTION_USER_DELETE, Filament::getTenant()); + return user()?->can(Permission::ACTION_USER_DELETE, Filament::getTenant()) ?? false; }app/Filament/Server/Resources/Schedules/ScheduleResource.php (1)
338-339: Fix inverted authorization logic for ViewAction.The
ViewActionis currently hidden when edit is allowed, which is backwards. It should be hidden when edit is not allowed.Apply this diff:
ViewAction::make() - ->hidden(fn ($record) => static::getEditAuthorizationResponse($record)->allowed()), + ->hidden(fn ($record) => !static::getEditAuthorizationResponse($record)->allowed()),app/Policies/Server/SchedulePolicy.php (1)
13-31: Coalesce null to false to satisfy bool return type.All four methods use
user()?->can(...)which returnsnullwhen the user is not authenticated, violating theboolreturn type declaration. This will cause a type error in strict mode.Apply this diff to coalesce to
false:public function viewAny(): bool { - return user()?->can(Permission::ACTION_SCHEDULE_READ, Filament::getTenant()); + return user()?->can(Permission::ACTION_SCHEDULE_READ, Filament::getTenant()) ?? false; } public function create(): bool { - return user()?->can(Permission::ACTION_SCHEDULE_CREATE, Filament::getTenant()); + return user()?->can(Permission::ACTION_SCHEDULE_CREATE, Filament::getTenant()) ?? false; } public function edit(Model $record): bool { - return user()?->can(Permission::ACTION_SCHEDULE_UPDATE, Filament::getTenant()); + return user()?->can(Permission::ACTION_SCHEDULE_UPDATE, Filament::getTenant()) ?? false; } public function delete(Model $record): bool { - return user()?->can(Permission::ACTION_SCHEDULE_DELETE, Filament::getTenant()); + return user()?->can(Permission::ACTION_SCHEDULE_DELETE, Filament::getTenant()) ?? false; }app/Policies/Server/AllocationPolicy.php (1)
13-31: Coalesce null to false to satisfy bool return type.All four methods use
user()?->can(...)which returnsnullwhen the user is not authenticated, violating theboolreturn type declaration.Apply this diff:
public function viewAny(): bool { - return user()?->can(Permission::ACTION_ALLOCATION_READ, Filament::getTenant()); + return user()?->can(Permission::ACTION_ALLOCATION_READ, Filament::getTenant()) ?? false; } public function create(): bool { - return user()?->can(Permission::ACTION_ALLOCATION_CREATE, Filament::getTenant()); + return user()?->can(Permission::ACTION_ALLOCATION_CREATE, Filament::getTenant()) ?? false; } public function edit(Model $record): bool { - return user()?->can(Permission::ACTION_ALLOCATION_UPDATE, Filament::getTenant()); + return user()?->can(Permission::ACTION_ALLOCATION_UPDATE, Filament::getTenant()) ?? false; } public function delete(Model $record): bool { - return user()?->can(Permission::ACTION_ALLOCATION_DELETE, Filament::getTenant()); + return user()?->can(Permission::ACTION_ALLOCATION_DELETE, Filament::getTenant()) ?? false; }app/Policies/Server/ActivityLogPolicy.php (1)
13-21: Coalesce null to false to satisfy bool return type.Both methods use
user()?->can(...)which returnsnullwhen the user is not authenticated, violating theboolreturn type declaration.Apply this diff:
public function viewAny(): bool { - return user()?->can(Permission::ACTION_ACTIVITY_READ, Filament::getTenant()); + return user()?->can(Permission::ACTION_ACTIVITY_READ, Filament::getTenant()) ?? false; } public function view(Model $model): bool { - return user()?->can(Permission::ACTION_ACTIVITY_READ, Filament::getTenant()); + return user()?->can(Permission::ACTION_ACTIVITY_READ, Filament::getTenant()) ?? false; }app/Policies/Server/DatabasePolicy.php (1)
13-36: Coalesce null to false to satisfy bool return type.All five methods use
user()?->can(...)which returnsnullwhen the user is not authenticated, violating theboolreturn type declaration.Apply this diff:
public function viewAny(): bool { - return user()?->can(Permission::ACTION_DATABASE_READ, Filament::getTenant()); + return user()?->can(Permission::ACTION_DATABASE_READ, Filament::getTenant()) ?? false; } public function view(Model $record): bool { - return user()?->can(Permission::ACTION_DATABASE_READ, Filament::getTenant()); + return user()?->can(Permission::ACTION_DATABASE_READ, Filament::getTenant()) ?? false; } public function create(): bool { - return user()?->can(Permission::ACTION_DATABASE_CREATE, Filament::getTenant()); + return user()?->can(Permission::ACTION_DATABASE_CREATE, Filament::getTenant()) ?? false; } public function edit(Model $record): bool { - return user()?->can(Permission::ACTION_DATABASE_UPDATE, Filament::getTenant()); + return user()?->can(Permission::ACTION_DATABASE_UPDATE, Filament::getTenant()) ?? false; } public function delete(Model $record): bool { - return user()?->can(Permission::ACTION_DATABASE_DELETE, Filament::getTenant()); + return user()?->can(Permission::ACTION_DATABASE_DELETE, Filament::getTenant()) ?? false; }
🧹 Nitpick comments (6)
app/Policies/Server/BackupPolicy.php (1)
11-11: Consider removing unused$modelNameproperty.The
$modelNameproperty is not referenced anywhere in this policy class. Unless it's used by reflection or a parent trait (which doesn't appear to be the case), it should be removed.Apply this diff if unused:
- protected string $modelName = 'backup'; - public function viewAny(): boolapp/Policies/Server/FilePolicy.php (1)
11-11: Consider removing unused$modelNameproperty.The
$modelNameproperty is not referenced in this policy class. Unless used by reflection (which doesn't appear to be the case), it should be removed.Apply this diff if unused:
- protected string $modelName = 'file'; - public function viewAny(): boolapp/Policies/Server/SchedulePolicy.php (1)
23-26: Unused$recordparameter is acceptable for Laravel policy contract.The static analysis tool flags the
$recordparameter as unused in bothedit()anddelete()methods. This is acceptable because:
- Laravel policy methods follow standard signatures that include the model parameter
- The permission checks are tenant-scoped via
Filament::getTenant()rather than record-scoped- Maintaining the parameter preserves consistency with Laravel's policy contract
Also applies to: 28-31
app/Policies/Server/AllocationPolicy.php (1)
23-26: Unused$recordparameter is acceptable for Laravel policy contract.The
$recordparameter inedit()anddelete()follows Laravel's standard policy signature. Since permission checks are tenant-scoped viaFilament::getTenant(), the parameter remains unused but maintains API consistency.Also applies to: 28-31
app/Policies/Server/ActivityLogPolicy.php (1)
18-21: Unused$modelparameter is acceptable for Laravel policy contract.The
$modelparameter inview()follows Laravel's standard policy signature. Since the permission check is tenant-scoped, the parameter remains unused but maintains API consistency.app/Policies/Server/DatabasePolicy.php (1)
18-21: Unused$recordparameter is acceptable for Laravel policy contract.The
$recordparameter inview(),edit(), anddelete()methods follows Laravel's standard policy signature. Since permission checks are tenant-scoped viaFilament::getTenant(), the parameters remain unused but maintain API consistency.Also applies to: 28-31, 33-36
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
app/Filament/Admin/Resources/DatabaseHosts/DatabaseHostResource.php(1 hunks)app/Filament/Admin/Resources/Mounts/MountResource.php(1 hunks)app/Filament/Admin/Resources/Roles/RoleResource.php(1 hunks)app/Filament/Admin/Resources/Users/UserResource.php(1 hunks)app/Filament/Admin/Resources/Webhooks/WebhookResource.php(1 hunks)app/Filament/Server/Resources/Activities/ActivityResource.php(0 hunks)app/Filament/Server/Resources/Allocations/AllocationResource.php(0 hunks)app/Filament/Server/Resources/Backups/BackupResource.php(0 hunks)app/Filament/Server/Resources/Databases/DatabaseResource.php(0 hunks)app/Filament/Server/Resources/Files/FileResource.php(0 hunks)app/Filament/Server/Resources/Schedules/ScheduleResource.php(1 hunks)app/Filament/Server/Resources/Users/UserResource.php(0 hunks)app/Policies/Admin/ApiKeyPolicy.php(1 hunks)app/Policies/Admin/DatabaseHostPolicy.php(1 hunks)app/Policies/Admin/DefaultPolicies.php(1 hunks)app/Policies/Admin/EggPolicy.php(1 hunks)app/Policies/Admin/MountPolicy.php(1 hunks)app/Policies/Admin/NodePolicy.php(1 hunks)app/Policies/Admin/RolePolicy.php(1 hunks)app/Policies/Admin/UserPolicy.php(1 hunks)app/Policies/Admin/WebhookConfigurationPolicy.php(1 hunks)app/Policies/DatabasePolicy.php(0 hunks)app/Policies/Server/ActivityLogPolicy.php(1 hunks)app/Policies/Server/AllocationPolicy.php(1 hunks)app/Policies/Server/BackupPolicy.php(1 hunks)app/Policies/Server/DatabasePolicy.php(1 hunks)app/Policies/Server/DefaultPolicies.php(1 hunks)app/Policies/Server/FilePolicy.php(1 hunks)app/Policies/Server/SchedulePolicy.php(1 hunks)app/Policies/Server/ServerPolicies.php(1 hunks)app/Policies/Server/UserPolicy.php(1 hunks)app/Providers/AppServiceProvider.php(2 hunks)
💤 Files with no reviewable changes (7)
- app/Filament/Server/Resources/Users/UserResource.php
- app/Filament/Server/Resources/Activities/ActivityResource.php
- app/Filament/Server/Resources/Backups/BackupResource.php
- app/Filament/Server/Resources/Databases/DatabaseResource.php
- app/Filament/Server/Resources/Files/FileResource.php
- app/Filament/Server/Resources/Allocations/AllocationResource.php
- app/Policies/DatabasePolicy.php
🚧 Files skipped from review as they are similar to previous changes (5)
- app/Policies/Admin/DatabaseHostPolicy.php
- app/Policies/Admin/EggPolicy.php
- app/Policies/Admin/ApiKeyPolicy.php
- app/Policies/Admin/WebhookConfigurationPolicy.php
- app/Policies/Admin/DefaultPolicies.php
🧰 Additional context used
🧬 Code graph analysis (8)
app/Policies/Server/AllocationPolicy.php (2)
app/Models/Permission.php (1)
Permission(11-221)app/Policies/Server/DatabasePolicy.php (4)
viewAny(13-16)create(23-26)edit(28-31)delete(33-36)
app/Providers/AppServiceProvider.php (5)
app/Policies/Admin/DatabaseHostPolicy.php (1)
before(14-28)app/Policies/Admin/MountPolicy.php (1)
before(14-28)app/Policies/Admin/NodePolicy.php (1)
before(14-26)app/Policies/Server/ServerPolicies.php (1)
before(18-45)app/Models/User.php (2)
User(94-492)isRootAdmin(379-382)
app/Policies/Server/ActivityLogPolicy.php (3)
app/Models/Permission.php (1)
Permission(11-221)app/Policies/Server/AllocationPolicy.php (1)
viewAny(13-16)app/Policies/Server/DatabasePolicy.php (2)
viewAny(13-16)view(18-21)
app/Policies/Server/UserPolicy.php (2)
app/Models/Permission.php (1)
Permission(11-221)app/Policies/Server/AllocationPolicy.php (4)
viewAny(13-16)create(18-21)edit(23-26)delete(28-31)
app/Policies/Server/FilePolicy.php (3)
app/Models/Permission.php (1)
Permission(11-221)app/Policies/Server/ActivityLogPolicy.php (1)
viewAny(13-16)app/Policies/Server/AllocationPolicy.php (4)
viewAny(13-16)create(18-21)edit(23-26)delete(28-31)
app/Policies/Server/BackupPolicy.php (5)
app/Models/Permission.php (1)
Permission(11-221)app/Policies/Server/ActivityLogPolicy.php (1)
viewAny(13-16)app/Policies/Server/AllocationPolicy.php (3)
viewAny(13-16)create(18-21)delete(28-31)app/Policies/Server/DatabasePolicy.php (3)
viewAny(13-16)create(23-26)delete(33-36)app/Policies/Server/FilePolicy.php (3)
viewAny(13-16)create(18-21)delete(28-31)
app/Policies/Server/DatabasePolicy.php (4)
app/Models/Permission.php (1)
Permission(11-221)app/Policies/Server/ActivityLogPolicy.php (2)
viewAny(13-16)view(18-21)app/Policies/Server/AllocationPolicy.php (4)
viewAny(13-16)create(18-21)edit(23-26)delete(28-31)app/Policies/Server/FilePolicy.php (4)
viewAny(13-16)create(18-21)edit(23-26)delete(28-31)
app/Policies/Server/SchedulePolicy.php (2)
app/Models/Permission.php (1)
Permission(11-221)app/Policies/Server/AllocationPolicy.php (4)
viewAny(13-16)create(18-21)edit(23-26)delete(28-31)
🪛 PHPMD (2.15.0)
app/Policies/Server/AllocationPolicy.php
23-23: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
28-28: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
app/Providers/AppServiceProvider.php
173-173: Avoid unused parameters such as '$ability'. (undefined)
(UnusedFormalParameter)
app/Policies/Server/ActivityLogPolicy.php
18-18: Avoid unused parameters such as '$model'. (undefined)
(UnusedFormalParameter)
app/Policies/Server/UserPolicy.php
23-23: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
28-28: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
app/Policies/Server/FilePolicy.php
23-23: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
28-28: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
app/Policies/Server/BackupPolicy.php
23-23: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
app/Policies/Server/DatabasePolicy.php
18-18: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
28-28: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
33-33: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
app/Policies/Server/SchedulePolicy.php
23-23: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
28-28: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (14)
app/Filament/Admin/Resources/DatabaseHosts/DatabaseHostResource.php (1)
95-95: LGTM! Authorization response migration implemented correctly.The ViewAction visibility now uses
getEditAuthorizationResponse($record)->allowed()instead of the deprecatedcanEdit($record)method, aligning with the PR's objective to adopt policy response objects for authorization.app/Filament/Admin/Resources/Users/UserResource.php (1)
133-133: LGTM! Consistent with authorization response pattern.ViewAction visibility correctly migrated to use authorization response method.
app/Policies/Admin/RolePolicy.php (1)
3-3: LGTM! Policy namespace reorganization.The namespace change from
App\PoliciestoApp\Policies\Adminis part of a broader policy reorganization to separate admin and server policies. No behavioral changes.app/Filament/Admin/Resources/Webhooks/WebhookResource.php (1)
101-101: LGTM! Authorization response pattern applied correctly.app/Policies/Admin/NodePolicy.php (1)
3-3: LGTM! Namespace refactor consistent with policy reorganization.app/Policies/Admin/MountPolicy.php (1)
3-3: LGTM! Policy namespace reorganization applied consistently.app/Filament/Admin/Resources/Roles/RoleResource.php (1)
101-101: LGTM! Authorization response migration implemented correctly.app/Filament/Admin/Resources/Mounts/MountResource.php (1)
98-98: LGTM! Authorization response pattern applied consistently.Excellent work on this PR! All resource files consistently migrate from
canEdit()togetEditAuthorizationResponse()->allowed(), and all policy namespaces are properly reorganized. The changes align perfectly with the PR objectives.app/Policies/Server/ServerPolicies.php (2)
3-3: LGTM! Namespace refactored to align with Admin/Server policy structure.The move to
App\Policies\Serveris consistent with the new policy organization approach.
11-11: LGTM! DefaultPolicies trait ensures before() is always invoked.The trait provides a
__callfallback so Laravel's authorization system will callbefore()even when no matching policy method exists.app/Policies/Server/DefaultPolicies.php (1)
5-18: LGTM! Trait correctly implements __call workaround.The
__callmethod signature is correct witharray $arguments, and the no-op implementation appropriately ensures Laravel invokesbefore()for all authorization checks, even when specific policy methods don't exist.app/Policies/Admin/UserPolicy.php (1)
3-3: LGTM! Namespace refactored to Admin policy structure.The move to
App\Policies\Adminaligns with the new policy organization separating Admin and Server contexts.app/Providers/AppServiceProvider.php (2)
173-173: LGTM! Simplified Gate::before to arrow function.The arrow function is more concise and correctly returns
truefor root admins, allowing other users to proceed through normal authorization.Note: The
$abilityparameter is required by theGate::beforesignature even though it's unused here.
175-188: LGTM! Dynamic policy resolution enables panel-specific policies.The
Gate::guessPolicyNamesUsingcallback correctly maps models to panel-specific policy classes (App\Policies\Admin\*orApp\Policies\Server\*), supporting the new authorization structure.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
app/Policies/Server/ServerPolicy.php (1)
47-50: Type change is correct; minor docblock inaccuracy.The change from
mixedtoarrayfor$argumentsis accurate since PHP's__callalways receives an array of arguments.The docblock annotation
array<string, mixed>is slightly inaccurate—__callreceives a numerically indexed array, not string-keyed. Consider:- * @param array<string, mixed> $arguments + * @param list<mixed> $argumentsapp/Policies/Server/BackupPolicy.php (2)
17-20: Consider whether record-level authorization is needed.The
$recordparameter inview()anddelete()is unused—both methods check only tenant-level permissions. If backup authorization is intentionally tenant-scoped (any user with tenant permission can view/delete any backup), the current implementation is correct but the unused parameter could be confusing.If record-specific checks are needed (e.g., backup ownership, status constraints), incorporate
$recordinto the authorization logic. Otherwise, the code is acceptable as-is for tenant-scoped authorization.Also applies to: 27-30
12-30: Optional: Use response objects for clearer authorization feedback.While returning
boolis valid, the PR objectives mention adopting policy response objects. Consider returningIlluminate\Auth\Access\Responseinstances to provide custom messages when authorization fails:use Illuminate\Auth\Access\Response; public function create(User $user): Response { return $user->can(Permission::ACTION_BACKUP_CREATE, Filament::getTenant()) ? Response::allow() : Response::deny('You need backup:create permission.'); }This improves user experience by explaining why access was denied, though it's not required for functionality.
app/Policies/Server/DatabasePolicy.php (2)
17-20: Unused$recordparameters are expected for policy signatures; consider silencing PHPMDThe
$recordarguments are unused but required for the policy method contract so Gate/Filament can resolve them correctly. I would keep them as-is and either:
- Configure PHPMD to ignore this rule for policy classes, or
- Add a
@SuppressWarnings("unused")(or equivalent) on these methods if your tooling supports it.Also applies to: 27-30, 32-35
27-30: Confirmeditvsupdateability namingLaravel’s default policy method for modifications is
update. If your Gate/Filament wiring intentionally calls aneditability here, this is fine; otherwise, consider renaming toupdateor adding anupdate()method that delegates toedit()so both entry points are covered.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
app/Filament/Admin/Resources/DatabaseHosts/DatabaseHostResource.php(1 hunks)app/Filament/Admin/Resources/Mounts/MountResource.php(1 hunks)app/Filament/Admin/Resources/Roles/RoleResource.php(1 hunks)app/Filament/Admin/Resources/Users/UserResource.php(1 hunks)app/Filament/Admin/Resources/Webhooks/WebhookResource.php(1 hunks)app/Filament/Server/Resources/Allocations/AllocationResource.php(0 hunks)app/Filament/Server/Resources/Backups/BackupResource.php(0 hunks)app/Filament/Server/Resources/Databases/DatabaseResource.php(0 hunks)app/Filament/Server/Resources/Schedules/ScheduleResource.php(1 hunks)app/Filament/Server/Resources/Users/UserResource.php(0 hunks)app/Policies/Admin/ApiKeyPolicy.php(1 hunks)app/Policies/Admin/DatabaseHostPolicy.php(1 hunks)app/Policies/Admin/ServerPolicy.php(1 hunks)app/Policies/Server/ActivityLogPolicy.php(1 hunks)app/Policies/Server/AllocationPolicy.php(1 hunks)app/Policies/Server/BackupPolicy.php(1 hunks)app/Policies/Server/DatabasePolicy.php(1 hunks)app/Policies/Server/FilePolicy.php(1 hunks)app/Policies/Server/SchedulePolicy.php(1 hunks)app/Policies/Server/ServerPolicy.php(2 hunks)app/Policies/Server/UserPolicy.php(1 hunks)app/Providers/AppServiceProvider.php(2 hunks)
💤 Files with no reviewable changes (4)
- app/Filament/Server/Resources/Users/UserResource.php
- app/Filament/Server/Resources/Allocations/AllocationResource.php
- app/Filament/Server/Resources/Databases/DatabaseResource.php
- app/Filament/Server/Resources/Backups/BackupResource.php
🚧 Files skipped from review as they are similar to previous changes (4)
- app/Filament/Admin/Resources/DatabaseHosts/DatabaseHostResource.php
- app/Policies/Admin/DatabaseHostPolicy.php
- app/Filament/Admin/Resources/Users/UserResource.php
- app/Filament/Admin/Resources/Webhooks/WebhookResource.php
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-11-06T10:11:54.301Z
Learnt from: Boy132
Repo: pelican-dev/panel PR: 1866
File: app/Policies/PluginPolicy.php:7-7
Timestamp: 2025-11-06T10:11:54.301Z
Learning: In the codebase, the `DefaultPolicies` trait is located in the `App\Policies` namespace (file: app/Policies/DefaultPolicies.php). Policy classes in the same namespace can use this trait without requiring an import statement.
Applied to files:
app/Policies/Admin/ApiKeyPolicy.phpapp/Policies/Admin/ServerPolicy.phpapp/Policies/Server/DatabasePolicy.phpapp/Policies/Server/FilePolicy.phpapp/Policies/Server/UserPolicy.phpapp/Policies/Server/ServerPolicy.php
📚 Learning: 2025-09-16T19:30:21.443Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1708
File: app/Filament/App/Resources/Servers/Pages/ListServers.php:115-116
Timestamp: 2025-09-16T19:30:21.443Z
Learning: The Filament `recordActions` method accepts both arrays and ActionGroup instances (signature: `array | ActionGroup $actions`), so ActionGroup can be passed directly without wrapping in an array.
Applied to files:
app/Filament/Admin/Resources/Mounts/MountResource.phpapp/Filament/Server/Resources/Schedules/ScheduleResource.php
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, Wizard Step components use the Filament\Schemas namespace (Filament\Schemas\Components\Wizard\Step), not Filament\Forms.
Applied to files:
app/Filament/Server/Resources/Schedules/ScheduleResource.php
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, the Forms component Placeholder was deprecated and removed. Use TextEntry from Filament\Infolists\Components\TextEntry in forms instead, binding values with ->state(). For HTML content, use ->html().
Applied to files:
app/Filament/Server/Resources/Schedules/ScheduleResource.php
🧬 Code graph analysis (7)
app/Policies/Admin/ServerPolicy.php (1)
app/Policies/Server/ServerPolicy.php (1)
ServerPolicy(9-54)
app/Providers/AppServiceProvider.php (1)
app/Models/User.php (2)
User(94-491)isRootAdmin(378-381)
app/Policies/Server/DatabasePolicy.php (4)
app/Models/Permission.php (1)
Permission(11-221)app/Policies/Server/ActivityLogPolicy.php (2)
viewAny(12-15)view(17-20)app/Policies/Server/BackupPolicy.php (4)
viewAny(12-15)view(17-20)create(22-25)delete(27-30)app/Policies/Admin/DefaultPolicies.php (4)
viewAny(10-13)view(15-18)create(20-23)delete(30-33)
app/Policies/Server/FilePolicy.php (9)
app/Models/Permission.php (1)
Permission(11-221)app/Policies/Server/ActivityLogPolicy.php (2)
viewAny(12-15)view(17-20)app/Policies/Server/AllocationPolicy.php (5)
viewAny(12-15)view(17-20)create(22-25)edit(27-30)delete(32-35)app/Policies/Server/BackupPolicy.php (4)
viewAny(12-15)view(17-20)create(22-25)delete(27-30)app/Policies/Server/DatabasePolicy.php (5)
viewAny(12-15)view(17-20)create(22-25)edit(27-30)delete(32-35)app/Policies/Server/SchedulePolicy.php (5)
viewAny(12-15)view(17-20)create(22-25)edit(27-30)delete(32-35)app/Policies/Server/UserPolicy.php (5)
viewAny(12-15)view(17-20)create(22-25)edit(27-30)delete(32-35)app/Policies/Admin/DefaultPolicies.php (4)
viewAny(10-13)view(15-18)create(20-23)delete(30-33)app/Policies/Admin/UserPolicy.php (1)
delete(22-25)
app/Policies/Server/ActivityLogPolicy.php (4)
app/Models/Permission.php (1)
Permission(11-221)app/Policies/Server/BackupPolicy.php (2)
viewAny(12-15)view(17-20)app/Policies/Server/UserPolicy.php (2)
viewAny(12-15)view(17-20)app/Policies/Admin/DefaultPolicies.php (2)
viewAny(10-13)view(15-18)
app/Policies/Server/SchedulePolicy.php (8)
app/Policies/Server/ActivityLogPolicy.php (2)
viewAny(12-15)view(17-20)app/Policies/Server/AllocationPolicy.php (5)
viewAny(12-15)view(17-20)create(22-25)edit(27-30)delete(32-35)app/Policies/Server/BackupPolicy.php (4)
viewAny(12-15)view(17-20)create(22-25)delete(27-30)app/Policies/Server/DatabasePolicy.php (5)
viewAny(12-15)view(17-20)create(22-25)edit(27-30)delete(32-35)app/Policies/Server/FilePolicy.php (5)
viewAny(12-15)view(17-20)create(22-25)edit(27-30)delete(32-35)app/Policies/Server/UserPolicy.php (5)
viewAny(12-15)view(17-20)create(22-25)edit(27-30)delete(32-35)app/Policies/Admin/DefaultPolicies.php (4)
viewAny(10-13)view(15-18)create(20-23)delete(30-33)app/Policies/Admin/UserPolicy.php (1)
delete(22-25)
app/Policies/Server/AllocationPolicy.php (7)
app/Policies/Server/ActivityLogPolicy.php (2)
viewAny(12-15)view(17-20)app/Policies/Server/BackupPolicy.php (4)
viewAny(12-15)view(17-20)create(22-25)delete(27-30)app/Policies/Server/DatabasePolicy.php (5)
viewAny(12-15)view(17-20)create(22-25)edit(27-30)delete(32-35)app/Policies/Server/FilePolicy.php (5)
viewAny(12-15)view(17-20)create(22-25)edit(27-30)delete(32-35)app/Policies/Server/UserPolicy.php (5)
viewAny(12-15)view(17-20)create(22-25)edit(27-30)delete(32-35)app/Policies/Admin/DefaultPolicies.php (4)
viewAny(10-13)view(15-18)create(20-23)delete(30-33)app/Policies/Admin/UserPolicy.php (1)
delete(22-25)
🪛 PHPMD (2.15.0)
app/Policies/Server/BackupPolicy.php
17-17: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
27-27: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
app/Providers/AppServiceProvider.php
110-110: Avoid unused parameters such as '$ability'. (undefined)
(UnusedFormalParameter)
app/Policies/Server/DatabasePolicy.php
17-17: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
27-27: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
32-32: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
app/Policies/Server/FilePolicy.php
17-17: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
27-27: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
32-32: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
app/Policies/Server/UserPolicy.php
17-17: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
27-27: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
32-32: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
app/Policies/Server/ActivityLogPolicy.php
17-17: Avoid unused parameters such as '$model'. (undefined)
(UnusedFormalParameter)
app/Policies/Server/SchedulePolicy.php
17-17: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
27-27: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
32-32: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
app/Policies/Server/AllocationPolicy.php
17-17: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
27-27: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
32-32: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (13)
app/Policies/Admin/ServerPolicy.php (1)
3-9: ServerPolicy scaffolding looks consistent with Admin policy patternThe Admin
ServerPolicycleanly delegates toDefaultPolicieswith an appropriate$modelName = 'server', which matches the expected lightweight wrapper pattern for these Admin policies. No issues from this file in isolation.app/Policies/Server/ServerPolicy.php (1)
3-3: Namespace reorganization looks good.The move from
App\PoliciestoApp\Policies\Serveraligns with the broader refactoring to separate Admin and Server policy contexts.app/Policies/Server/BackupPolicy.php (1)
12-30: Previous null-safety concern has been resolved.The past review comment about adding null coalescing is no longer applicable. The current implementation correctly type-hints
User $useras a required parameter in all methods, eliminating the possibility of null user values.app/Filament/Admin/Resources/Mounts/MountResource.php (1)
97-98: Consistent authorization response implementation.The change correctly mirrors the pattern in
RoleResource.php, usinggetEditAuthorizationResponse($record)->allowed()instead ofcanEdit($record). The same verification regarding Filament version support applies here.app/Filament/Admin/Resources/Roles/RoleResource.php (1)
99-100: VerifygetEditAuthorizationResponse()support in Filament v4.0 documentation or release notes.The method is actively used across multiple Resource files in the codebase (RoleResource, ScheduleResource, UserResource, MountResource, WebhookResource, DatabaseHostResource) with consistent patterns, suggesting it is an intentional implementation. However, the method could not be located in the codebase itself, indicating it should be inherited from the Filament Resource base class. Since the sandbox environment prevents inspection of vendor code, please confirm that
getEditAuthorizationResponse()is a supported method in Filament v4.0 by checking the official Filament documentation or release notes.app/Providers/AppServiceProvider.php (1)
110-125: LGTM - Dynamic policy resolution is well-structured.The
Gate::beforecorrectly grants root admins full access. TheguessPolicyNamesUsingclosure properly resolves panel-scoped policies dynamically. The implicitnullreturn when no class exists (or panel is 'App') is the expected behavior for Laravel's policy guesser fallback.Note: The
$abilityparameter inGate::beforeis required by the signature but intentionally unused here since root admins bypass all ability checks.app/Policies/Server/UserPolicy.php (1)
10-36: LGTM - Policy implementation follows established patterns.The policy correctly delegates permission checks to Laravel's authorization layer with tenant context. The
$recordparameters inview,edit, anddeleteare required by Laravel's policy method signatures even though the current authorization logic uses tenant-based permissions rather than record-based checks.app/Policies/Server/SchedulePolicy.php (1)
10-36: LGTM - Consistent with other Server policies.The
SchedulePolicycorrectly implements the tenant-based permission pattern usingPermission::ACTION_SCHEDULE_*constants. Structure and signatures align with sibling policies (UserPolicy,DatabasePolicy, etc.).app/Policies/Server/FilePolicy.php (1)
10-36: LGTM - Good granular permission separation.The
FilePolicyappropriately distinguishes between listing files (ACTION_FILE_READ) and reading file contents (ACTION_FILE_READ_CONTENT), providing more granular access control than the other resource policies.app/Policies/Server/AllocationPolicy.php (1)
10-36: LGTM - Follows established server policy pattern.The
AllocationPolicycorrectly implements tenant-scoped permission checks usingPermission::ACTION_ALLOCATION_*constants, consistent with sibling policies.app/Policies/Server/ActivityLogPolicy.php (1)
10-21: LGTM - Appropriately scoped for read-only resource.The
ActivityLogPolicycorrectly provides onlyviewAnyandviewmethods since activity logs are an immutable audit trail that users cannot create, modify, or delete.app/Policies/Admin/ApiKeyPolicy.php (1)
7-7: No action required. TheDefaultPoliciestrait is located in the same namespace (App\Policies\Admin) asApiKeyPolicy, so using the unqualified trait name is correct and does not require an import statement.app/Policies/Server/DatabasePolicy.php (1)
10-35: Policy implementation matches existing Server permission patternSignatures, return types, and use of
Permission::ACTION_DATABASE_*withFilament::getTenant()are consistent with the other Server policies and with the PR’s move to centralized policies. Nothing blocking here.
Closes #1831
Made this a Draft cause Filament doesn't seem to call them everywhere atm.