-
-
Notifications
You must be signed in to change notification settings - Fork 4k
[5.x] Feature: Strict authorization in CanAuthorizeAccess #18879
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
[5.x] Feature: Strict authorization in CanAuthorizeAccess #18879
Conversation
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.
Pull request overview
This PR extends Filament's strict authorization mode to enforce authorization checks on custom pages. Currently, when strictAuthorization() is enabled, pages using the default canAccess() implementation can be accessed without explicit permission definitions. This change makes the authorization flow stricter by throwing a LogicException when strict mode is enabled and a page hasn't overridden the canAccess() method.
Key Changes:
- Modified the default
canAccess()method inCanAuthorizeAccesstrait to check if strict authorization mode is enabled - When enabled, throws a
LogicExceptioninstead of returningtrueby default - Aligns page authorization behavior with the existing strict authorization pattern used for resources in
helpers.php
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| return true; | ||
| return Filament::isAuthorizationStrict() | ||
| ? throw new LogicException(sprintf('Strict authorization mode is enabled, but [canAccess()] method in [%s] class is not defined.', static::class)) |
Copilot
AI
Jan 6, 2026
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.
The error message states that "canAccess() method... is not defined" which is technically incorrect - the method IS defined in the CanAuthorizeAccess trait. Consider rewording to be more accurate, such as "Strict authorization mode is enabled, but the default canAccess() implementation is being used in class [%s]. Please override canAccess() to define custom authorization logic." This would better communicate to developers what action they need to take.
| ? throw new LogicException(sprintf('Strict authorization mode is enabled, but [canAccess()] method in [%s] class is not defined.', static::class)) | |
| ? throw new LogicException(sprintf('Strict authorization mode is enabled, but the default canAccess() implementation is being used in class [%s]. Please override canAccess() to define custom authorization logic.', static::class)) |
| return Filament::isAuthorizationStrict() | ||
| ? throw new LogicException(sprintf('Strict authorization mode is enabled, but [canAccess()] method in [%s] class is not defined.', static::class)) | ||
| : true; |
Copilot
AI
Jan 6, 2026
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.
This change will break existing pages that don't override the canAccess() method when strict authorization is enabled. For example, the Dashboard class (packages/panels/src/Pages/Dashboard.php) and custom pages like Settings in tests don't override canAccess(). When strictAuthorization() is enabled, accessing these pages will throw a LogicException instead of allowing access. This is a significant breaking change that should be carefully considered. If the intent is to require all pages to define canAccess() when strict mode is enabled, this needs clear documentation and migration guidance.
| return Filament::isAuthorizationStrict() | ||
| ? throw new LogicException(sprintf('Strict authorization mode is enabled, but [canAccess()] method in [%s] class is not defined.', static::class)) | ||
| : true; |
Copilot
AI
Jan 6, 2026
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.
The use of 'throw' in a ternary operator expression is unconventional and may reduce code readability. While valid in PHP, consider using a traditional if-else structure for better clarity and maintainability. This pattern is inconsistent with the approach used in helpers.php (lines 43-53) where a traditional if statement is used for similar strict authorization checks.
| return Filament::isAuthorizationStrict() | |
| ? throw new LogicException(sprintf('Strict authorization mode is enabled, but [canAccess()] method in [%s] class is not defined.', static::class)) | |
| : true; | |
| if (Filament::isAuthorizationStrict()) { | |
| throw new LogicException(sprintf('Strict authorization mode is enabled, but [canAccess()] method in [%s] class is not defined.', static::class)); | |
| } | |
| return true; |
|
Thanks, I think this is probably too far for this feature for now though. |
Description
Actually, this is just an idea to make authorization stricter in v5. While developing the authorization flow in my app, I noticed that many custom pages and dashboards are accessible even when a user has zero permissions, even if
strictAuthorization()is enabled.As a developer, I would expect the system to notify me when some permissions are not defined, instead of silently allowing uncontrolled access to resources.
I'm not sure whether modifying
CanAuthorizeAccesswouldn't be too broad. I also considered addingcanAccessdirectly toPageor even to theDashboardpage to prevent too many breaking changes, I will be grateful for your opinion here.Functional changes
composer cscommand.