Repository navigation
enhancement: Site Editor RBAC Integration with CMS Framework - #220
Merged
ViewFromTheBox merged 2 commits intoMar 23, 2026
Merged
ViewFromTheBox merged 2 commits into
ViewFromTheBox merged 2 commits into
Conversation
Add granular permission-based access control for the site editor with graceful degradation when the CMS framework is not installed. - Add 4 policy classes (Template, TemplatePart, Pattern, GlobalStyle) with applyFilters hooks for extensibility - Add CheckGateIfDefined middleware that skips authorization when gates are not registered (graceful degradation) - Add per-route permission middleware via configurable gates - Filter hub cards by user permissions - Register permissions with CMS framework when available - Register admin menu item with CMS framework when available - Add 7 permission slugs to config and 14 translation strings - Add 35 new tests across 4 test files Closes #215
📝 WalkthroughWalkthroughAdds configurable gate-based permissions to the Site Editor: new gate mappings in config, conditional gate middleware, policies for Template/TemplatePart/Pattern/GlobalStyle, service-provider registration (permissions, policies, admin menu), Livewire hub card permission filtering, helper, and tests for authorization and graceful degradation. Changes
Sequence Diagram(s)sequenceDiagram
actor Browser as User
participant Router as Router
participant Middleware as ve.gate (CheckGateIfDefined)
participant Gate as Gate
participant Handler as Route Handler / Controller
participant DB as Database
Browser->>Router: GET /site-editor/templates
Router->>Middleware: invoke ve.gate:visual-editor.manage-templates
Middleware->>Gate: Gate::has('visual-editor.manage-templates')?
alt Gate defined
Gate-->>Middleware: true
Middleware->>Middleware: ensure authenticated user
alt User authenticated
Middleware->>Gate: $user->can('visual-editor.manage-templates')?
alt allowed
Gate-->>Middleware: true
Middleware->>Handler: $next($request)
Handler->>DB: load templates
DB-->>Handler: templates
Handler-->>Browser: 200 OK
else denied
Middleware-->>Browser: 403 Forbidden
end
else unauthenticated
Middleware-->>Browser: 403 Forbidden
end
else Gate not defined
Gate-->>Middleware: false
Middleware->>Handler: $next($request)
Handler->>DB: load templates
DB-->>Handler: templates
Handler-->>Browser: 200 OK
end
sequenceDiagram
participant Hub as HubPage Component
participant Config as Config
participant Auth as Auth::user()
participant Gate as Gate
participant Filter as filterCardsByPermission()
Hub->>Config: load artisanpack.visual-editor.site_editor.gates
Config-->>Hub: gates array
Hub->>Hub: inject permission keys into default cards
Hub->>Filter: filterCardsByPermission(cards)
loop each card
alt card.permission empty
Filter->>Filter: include card
else has permission
Filter->>Gate: Gate::has(permission)?
alt gate undefined
Gate-->>Filter: false
Filter->>Filter: include card
else gate defined
Gate-->>Filter: true
Filter->>Auth: is user authenticated?
alt user exists
Filter->>Gate: $user->can(permission)?
alt allowed
Gate-->>Filter: true
Filter->>Filter: include card
else denied
Filter->>Filter: exclude card
end
else no user
Filter->>Filter: exclude card
end
end
end
end
Filter-->>Hub: filtered cards
Hub->>Hub: render hub
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
ViewFromTheBox
marked this pull request as ready for review
March 23, 2026 01:51
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
routes/web.php (1)
14-14: Unused import:Gatefacade is not used in this file.The
Gatefacade is imported but not referenced anywhere in the routes file. The authorization is handled via theve.gate:middleware string, not direct Gate calls.🧹 Remove unused import
-use Illuminate\Support\Facades\Gate; use Illuminate\Support\Facades\Route;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@routes/web.php` at line 14, Remove the unused Gate import by deleting the use statement for Illuminate\Support\Facades\Gate (the standalone "use Gate" import) from the top of the routes file since authorization is handled via middleware strings (e.g., "ve.gate:") and there are no direct Gate:: calls; this will eliminate the unused import warning and keep the file clean.tests/Feature/Authorization/SiteEditorAuthorizationTest.php (1)
157-185: Assert the concrete unauthenticated response.Lines 160-184 only verify “not 2xx”, so these tests also pass on 404/500 and won't catch a broken route or middleware regression. Please pin the expected behavior here instead, e.g. redirect-to-login vs 401/403.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Feature/Authorization/SiteEditorAuthorizationTest.php` around lines 157 - 185, The current tests for the unauthenticated access checks (tests that call route('visual-editor.site-editor'), route('visual-editor.global-styles'), route('visual-editor.templates'), route('visual-editor.template-parts'), and route('visual-editor.patterns')) only assert not-2xx; change each to assert the concrete expected unauthenticated behavior: either assert a redirect-to-login (expect response status 302 and Location header points to route('login') or use framework's assertRedirect mechanism) or, if your app returns a status like 401/403, assert that exact status (e.g., expect($response->status())->toBe(401) or 403). Update all five tests accordingly to verify the precise response instead of isSuccessful().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/helpers.php`:
- Around line 65-92: The docblock for veGateMiddleware is inaccurate: it claims
the middleware returns ['can:{ability}'] but the function actually returns
['ve.gate:' . $ability]; update the docblock text (description and example) to
reference 've.gate:{ability}' (or otherwise match the exact returned string) so
the documented return value and explanatory text align with the implementation
in veGateMiddleware.
In `@src/Livewire/SiteEditor/HubPage.php`:
- Around line 72-73: getCards() reads
artisanpack.visual-editor.site_editor.gates while mount() still uses
artisanpack.visual-editor.site_editor.permission, causing inconsistent checks;
update mount() to load the same gates array as getCards() and authorize using
the same access ability (e.g., $gates = (array)
config('artisanpack.visual-editor.site_editor.gates', []);
$this->authorize($gates['access'] ?? 'some_default') ), or alternatively change
getCards() to derive its ability from the single permission key — ensure both
mount() and getCards() reference the same config key and the same gate/ability
symbol so route middleware, card visibility and component fallback are
consistent.
- Around line 145-166: The current filterCardsByPermission uses
Gate::has($permission) to short-circuit and allow access when Gate::has returns
false, which skips authorization hooks from packages like Spatie; instead,
remove the Gate::has() branch and evaluate the permission against the actual
user context (use Gate::forUser($user)->allows($permission) or, when $user is
not null, $user->can($permission)); ensure filterCardsByPermission returns false
if $user is null and a permission is required, otherwise call
Gate::forUser($user)->allows($permission) (or $user->can($permission)) to decide
visibility.
In `@src/Policies/TemplateSiteEditorPolicy.php`:
- Around line 78-86: The update() logic in TemplateSiteEditorPolicy currently
allows users with only visual-editor.lock-content to update locked templates;
change it to first check the base update permission (the filtered
ve.template.update via veApplyFilters in update()) and return false if the user
lacks it, then if the template is locked additionally require
visual-editor.lock-content; specifically, in the update(Authenticatable $user,
Template $template) method compute $permission =
veApplyFilters('ve.template.update', 'visual-editor.manage-templates'), if
!$user->can($permission) return false, and only after that if
$template->is_locked return $user->can('visual-editor.lock-content'), otherwise
return true.
---
Nitpick comments:
In `@routes/web.php`:
- Line 14: Remove the unused Gate import by deleting the use statement for
Illuminate\Support\Facades\Gate (the standalone "use Gate" import) from the top
of the routes file since authorization is handled via middleware strings (e.g.,
"ve.gate:") and there are no direct Gate:: calls; this will eliminate the unused
import warning and keep the file clean.
In `@tests/Feature/Authorization/SiteEditorAuthorizationTest.php`:
- Around line 157-185: The current tests for the unauthenticated access checks
(tests that call route('visual-editor.site-editor'),
route('visual-editor.global-styles'), route('visual-editor.templates'),
route('visual-editor.template-parts'), and route('visual-editor.patterns')) only
assert not-2xx; change each to assert the concrete expected unauthenticated
behavior: either assert a redirect-to-login (expect response status 302 and
Location header points to route('login') or use framework's assertRedirect
mechanism) or, if your app returns a status like 401/403, assert that exact
status (e.g., expect($response->status())->toBe(401) or 403). Update all five
tests accordingly to verify the precise response instead of isSuccessful().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 89d3fc8e-9b16-41c7-a3ad-6709973db2ef
📒 Files selected for processing (15)
config/visual-editor.phpresources/lang/en/ve.phproutes/web.phpsrc/Http/Middleware/CheckGateIfDefined.phpsrc/Livewire/SiteEditor/HubPage.phpsrc/Policies/GlobalStylePolicy.phpsrc/Policies/PatternSiteEditorPolicy.phpsrc/Policies/TemplatePartSiteEditorPolicy.phpsrc/Policies/TemplateSiteEditorPolicy.phpsrc/VisualEditorServiceProvider.phpsrc/helpers.phptests/Feature/Authorization/GracefulDegradationTest.phptests/Feature/Authorization/SiteEditorAuthorizationTest.phptests/Unit/Policies/GlobalStylePolicyTest.phptests/Unit/Policies/TemplateSiteEditorPolicyTest.php
Comment on lines
+145
to
+166
| protected function filterCardsByPermission( array $cards ): array | ||
| { | ||
| /** @var Authenticatable|null $user */ | ||
| $user = Auth::user(); | ||
|
|
||
| return array_values( array_filter( $cards, function ( array $card ) use ( $user ): bool { | ||
| $permission = $card['permission'] ?? null; | ||
|
|
||
| if ( null === $permission || '' === $permission ) { | ||
| return true; | ||
| } | ||
|
|
||
| if ( ! Gate::has( $permission ) ) { | ||
| return true; | ||
| } | ||
|
|
||
| if ( null === $user ) { | ||
| return false; | ||
| } | ||
|
|
||
| return $user->can( $permission ); | ||
| } ) ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Laravel's authorization system, does Gate::has('ability') return true for abilities handled only via Gate::before callbacks (for example Spatie Permission or Bouncer), or only for abilities explicitly registered with Gate::define / policies?
💡 Result:
Gate::has('ability') returns true only for abilities that exist in the Gate’s internal $abilities map (i.e., abilities registered via Gate::define() / Gate::resource() / etc.). It does not consider Gate::before() callbacks (and it also won’t “discover” policy methods) when deciding whether an ability “has been defined”. [1]
So if an ability is handled only by a Gate::before callback (common in “dynamic permission” setups), Gate::has('that-ability') will be false unless something also calls Gate::define('that-ability', ...) (or equivalent) for it. [1] Gate::before() affects authorization evaluation, not whether the ability is registered/defined. [2]
Spatie’s docs, for example, show doing permission checks via Gate::before(...), which means Gate::has() won’t reflect those abilities unless you separately define them. [3]
Sources
[1] Laravel framework Gate::has() implementation (checks only $this->abilities[...]) (github.com)
[2] Laravel Authorization docs (Gate::before runs before other checks; its return value determines the check result) (laravel.com)
[3] Spatie laravel-permission docs showing permission logic via Gate::before(...) (spatie.be)
Citations:
- 1: https://github.com/laravel/framework/blob/12.x/src/Illuminate/Auth/Access/Gate.php
- 2: https://laravel.com/docs/authorization?utm_source=openai
- 3: https://spatie.be/docs/laravel-permission/v6/advanced-usage/custom-permission-check?utm_source=openai
🏁 Script executed:
# Verify the file exists and check the specific lines
if [ -f "src/Livewire/SiteEditor/HubPage.php" ]; then
echo "=== File exists. Checking lines 145-166 ==="
sed -n '145,166p' "src/Livewire/SiteEditor/HubPage.php"
else
echo "File not found at src/Livewire/SiteEditor/HubPage.php"
# Try to locate it
find . -name "HubPage.php" -type f 2>/dev/null | head -5
fiRepository: ArtisanPack-UI/visual-editor
Length of output: 627
🏁 Script executed:
# Check the referenced middleware file
if [ -f "src/Http/Middleware/CheckGateIfDefined.php" ]; then
echo "=== CheckGateIfDefined.php exists. Checking lines 35-58 ==="
sed -n '35,58p' "src/Http/Middleware/CheckGateIfDefined.php"
else
echo "File not found at src/Http/Middleware/CheckGateIfDefined.php"
# Try to locate it
find . -name "CheckGateIfDefined.php" -type f 2>/dev/null | head -5
fiRepository: ArtisanPack-UI/visual-editor
Length of output: 659
🏁 Script executed:
# Search for tests related to filterCardsByPermission
find . -name "*.php" -path "*/test*" -o -name "*Test.php" | xargs grep -l "filterCardsByPermission" 2>/dev/null || echo "No tests found for filterCardsByPermission"Repository: ArtisanPack-UI/visual-editor
Length of output: 114
🏁 Script executed:
# Search for Spatie Permission setup in tests
find . -name "*.php" -path "*/test*" | xargs grep -l "spatie.*permission\|Spatie.*Permission" 2>/dev/null | head -10 || echo "No Spatie Permission tests found"Repository: ArtisanPack-UI/visual-editor
Length of output: 54
🏁 Script executed:
# Check what permission systems are actually tested
grep -r "Gate::define\|Gate::before" --include="*.php" src/ | head -20Repository: ArtisanPack-UI/visual-editor
Length of output: 54
🏁 Script executed:
# Search for tests related to HubPage or CheckGateIfDefined
find . -name "*.php" -type f | xargs grep -l "HubPage\|CheckGateIfDefined" 2>/dev/null | grep -i testRepository: ArtisanPack-UI/visual-editor
Length of output: 170
🏁 Script executed:
# Check if there are gate definitions in test setup/bootstrap
find . -path "*/tests/*" -name "*.php" -type f | xargs grep -l "Gate::" 2>/dev/null | head -10Repository: ArtisanPack-UI/visual-editor
Length of output: 621
🏁 Script executed:
# Check HubPageTest.php - see what tests exist for filterCardsByPermission
cat -n ./tests/Feature/Livewire/HubPageTest.phpRepository: ArtisanPack-UI/visual-editor
Length of output: 6136
🏁 Script executed:
# Check GracefulDegradationTest.php
cat -n ./tests/Feature/Authorization/GracefulDegradationTest.phpRepository: ArtisanPack-UI/visual-editor
Length of output: 5661
🏁 Script executed:
# Check SiteEditorAuthorizationTest.php
wc -l ./tests/Feature/Authorization/SiteEditorAuthorizationTest.php && head -100 ./tests/Feature/Authorization/SiteEditorAuthorizationTest.phpRepository: ArtisanPack-UI/visual-editor
Length of output: 2815
Gate::has() skips authorization when using permission packages like Spatie Permission.
Lines 157 and 51 (middleware) both treat Gate::has($permission) returning false as "allow access", but Gate::has() only returns true for abilities explicitly registered via Gate::define() / policies. Permission packages like Spatie Permission and Bouncer handle authorization via Gate::before() callbacks without defining abilities, causing Gate::has() to return false. This allows unauthorized users to see cards and access routes.
The test suite confirms this: GracefulDegradationTest.php (lines 71–87) explicitly verifies that cards are shown and routes are accessible when gates are configured but not registered. All existing tests use only Gate::define(), so the Spatie Permission scenario (authorization via Gate::before() only) is never exercised.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Livewire/SiteEditor/HubPage.php` around lines 145 - 166, The current
filterCardsByPermission uses Gate::has($permission) to short-circuit and allow
access when Gate::has returns false, which skips authorization hooks from
packages like Spatie; instead, remove the Gate::has() branch and evaluate the
permission against the actual user context (use
Gate::forUser($user)->allows($permission) or, when $user is not null,
$user->can($permission)); ensure filterCardsByPermission returns false if $user
is null and a permission is required, otherwise call
Gate::forUser($user)->allows($permission) (or $user->can($permission)) to decide
visibility.
- Fix veGateMiddleware docblock to reference ve.gate: instead of can: - Make HubPage mount() derive access permission from gates config for consistency with getCards() - Require base update permission before lock-content check in TemplateSiteEditorPolicy and TemplatePartSiteEditorPolicy - Remove unused Gate import from routes/web.php - Add test for locked template update denied without manage-templates - Document Gate::has() trade-off for Spatie compatibility in filterCardsByPermission docblock
left a comment
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Policies/TemplatePartSiteEditorPolicy.php (1)
45-47: Consider centralizing filtered-permission checks and validating filter output type.The same pattern repeats in multiple methods, and a private helper can reduce duplication while failing closed on invalid filtered values.
♻️ Suggested refactor
class TemplatePartSiteEditorPolicy { + private function canWithFilteredPermission( Authenticatable $user, string $filter, string $default ): bool + { + $permission = veApplyFilters( $filter, $default ); + + if ( ! is_string( $permission ) || '' === trim( $permission ) ) { + return false; + } + + return $user->can( $permission ); + } + public function viewAny( Authenticatable $user ): bool { - $permission = veApplyFilters( 've.template-part.viewAny', 'visual-editor.manage-parts' ); - - return $user->can( $permission ); + return $this->canWithFilteredPermission( $user, 've.template-part.viewAny', 'visual-editor.manage-parts' ); } public function create( Authenticatable $user ): bool { - $permission = veApplyFilters( 've.template-part.create', 'visual-editor.manage-parts' ); - - return $user->can( $permission ); + return $this->canWithFilteredPermission( $user, 've.template-part.create', 'visual-editor.manage-parts' ); }Also applies to: 61-63, 80-83, 111-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Policies/TemplatePartSiteEditorPolicy.php` around lines 45 - 47, The repeated pattern in TemplatePartSiteEditorPolicy (methods using veApplyFilters like where $permission = veApplyFilters('ve.template-part.viewAny', 'visual-editor.manage-parts') and then return $user->can($permission)) should be replaced by a private helper (e.g., private function resolveFilteredPermission(string $filterName, string $default): string) that calls veApplyFilters, validates the result is a non-empty string, and returns the default when the filter returns an unexpected type (failing closed). Update each method (the ones around the existing veApplyFilters calls) to call resolveFilteredPermission('ve.template-part.viewAny','visual-editor.manage-parts') (and the other filter names) and then use $user->can($permission) as before. Ensure the helper throws or logs if non-string is returned and always returns a safe default permission string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Policies/TemplatePartSiteEditorPolicy.php`:
- Around line 45-47: The repeated pattern in TemplatePartSiteEditorPolicy
(methods using veApplyFilters like where $permission =
veApplyFilters('ve.template-part.viewAny', 'visual-editor.manage-parts') and
then return $user->can($permission)) should be replaced by a private helper
(e.g., private function resolveFilteredPermission(string $filterName, string
$default): string) that calls veApplyFilters, validates the result is a
non-empty string, and returns the default when the filter returns an unexpected
type (failing closed). Update each method (the ones around the existing
veApplyFilters calls) to call
resolveFilteredPermission('ve.template-part.viewAny','visual-editor.manage-parts')
(and the other filter names) and then use $user->can($permission) as before.
Ensure the helper throws or logs if non-string is returned and always returns a
safe default permission string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cc8689d6-420e-41a2-8ce1-70f198d1b83e
📒 Files selected for processing (6)
routes/web.phpsrc/Livewire/SiteEditor/HubPage.phpsrc/Policies/TemplatePartSiteEditorPolicy.phpsrc/Policies/TemplateSiteEditorPolicy.phpsrc/helpers.phptests/Unit/Policies/TemplateSiteEditorPolicyTest.php
🚧 Files skipped from review as they are similar to previous changes (4)
- routes/web.php
- src/helpers.php
- tests/Unit/Policies/TemplateSiteEditorPolicyTest.php
- src/Policies/TemplateSiteEditorPolicy.php
github-project-automation
Bot
moved this from In Review
to Done
in Visual Editor Package
This was referenced Mar 23, 2026
13 tasks done
This was referenced Apr 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Integrates the site editor with granular permission-based access control, with graceful degradation when the CMS framework is not installed.
veApplyFiltershooks for extensibilityCheckGateIfDefinedmiddleware that skips authorization when gates are not registered, enabling graceful degradationsite_editor.gatesconfigfunction_exists)Closes #215
Changes
New Files
src/Policies/TemplateSiteEditorPolicy.php— Template CRUD authorization (locked templates require lock-content permission)src/Policies/TemplatePartSiteEditorPolicy.php— Template part CRUD authorizationsrc/Policies/PatternSiteEditorPolicy.php— Pattern CRUD authorizationsrc/Policies/GlobalStylePolicy.php— Global style update/reset authorizationsrc/Http/Middleware/CheckGateIfDefined.php— Gate-aware middleware for graceful degradationModified Files
config/visual-editor.php— Addedsite_editor.gateswith 7 permission slugsroutes/web.php— Per-routeve.gate:middlewaresrc/VisualEditorServiceProvider.php— Register permissions, policies, middleware alias, admin menusrc/Livewire/SiteEditor/HubPage.php— Permission-based card filteringsrc/helpers.php— AddedveGateMiddleware()helperresources/lang/en/ve.php— 14 permission translation stringsTesting
tests/Unit/Policies/TemplateSiteEditorPolicyTest.php(9 tests)tests/Unit/Policies/GlobalStylePolicyTest.php(4 tests)tests/Feature/Authorization/SiteEditorAuthorizationTest.php(15 tests)tests/Feature/Authorization/GracefulDegradationTest.php(7 tests)Summary by CodeRabbit
New Features
Documentation
Tests