diff --git a/ProcessMaker/Managers/MenuManager.php b/ProcessMaker/Managers/MenuManager.php index 77b541ab99..e37ef6b786 100644 --- a/ProcessMaker/Managers/MenuManager.php +++ b/ProcessMaker/Managers/MenuManager.php @@ -4,9 +4,19 @@ use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\View; +use Lavary\Menu\Collection; class MenuManager extends \Lavary\Menu\Menu { + /** + * Clear menu builders so the next request does not inherit items from a prior user. + */ + public function reset(): void + { + $this->menu = []; + $this->collection = new Collection(); + } + /** * Create a new menu builder instance. * diff --git a/ProcessMaker/Octane/ResetRequestState.php b/ProcessMaker/Octane/ResetRequestState.php index 45071e97ad..397075271e 100644 --- a/ProcessMaker/Octane/ResetRequestState.php +++ b/ProcessMaker/Octane/ResetRequestState.php @@ -4,7 +4,9 @@ namespace ProcessMaker\Octane; +use Lavary\Menu\Menu; use ProcessMaker\Listeners\HandleRedirectListener; +use ProcessMaker\Managers\MenuManager; use ProcessMaker\Providers\ProcessMakerServiceProvider; final class ResetRequestState @@ -13,5 +15,10 @@ public function handle(): void { ProcessMakerServiceProvider::beginRequestTiming(); HandleRedirectListener::reset(); + + $menuManager = app(Menu::class); + if ($menuManager instanceof MenuManager) { + $menuManager->reset(); + } } } diff --git a/config/octane.php b/config/octane.php index 9e221adfcd..471f22cd10 100644 --- a/config/octane.php +++ b/config/octane.php @@ -135,10 +135,8 @@ ProcessMaker\Models\AnonymousUser::class, ProcessMaker\ImportExport\Extension::class, ProcessMaker\ImportExport\SignalHelper::class, - ProcessMaker\Managers\MenuManager::class, ProcessMaker\Managers\PackageManager::class, ProcessMaker\Managers\IndexManager::class, - ProcessMaker\Managers\ScreenBuilderManager::class, ProcessMaker\Managers\ScriptBuilderManager::class, ProcessMaker\Managers\DockerManager::class, ProcessMaker\Managers\GlobalScriptsManager::class, diff --git a/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php b/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php index 7895d8519f..7adfd3a993 100644 --- a/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php +++ b/tests/unit/ProcessMaker/Octane/ResetRequestStateTest.php @@ -5,12 +5,17 @@ namespace Tests\Unit\ProcessMaker\Octane; use Illuminate\Http\Request; +use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Event; use Laravel\Octane\Events\RequestTerminated; +use Lavary\Menu\Facade as Menu; +use Lavary\Menu\Menu as MenuContract; use ProcessMaker\Events\RedirectToEvent; +use ProcessMaker\Http\Middleware\GenerateMenus; use ProcessMaker\Listeners\HandleRedirectListener; use ProcessMaker\Models\ProcessRequest; +use ProcessMaker\Models\User; use ProcessMaker\Octane\ResetRequestState; use ProcessMaker\Providers\ProcessMakerServiceProvider; use Symfony\Component\HttpFoundation\Response; @@ -63,6 +68,65 @@ public function test_octane_request_termination_automatically_resets_request_sta Event::assertNotDispatched(RedirectToEvent::class); } + + public function test_generate_menus_does_not_leak_admin_items_to_sso_user_after_octane_reset(): void + { + $admin = User::factory()->create(['is_administrator' => true]); + $ssoUser = User::factory()->create(['is_administrator' => false]); + + Auth::login($admin); + $this->runGenerateMenus(); + + $this->assertTrue($this->menuHasItemTitle('sidebar_admin', __('Users'))); + + $listener = new ResetRequestState(); + $listener->handle(); + $this->app->forgetInstance(MenuContract::class); + + Auth::login($ssoUser); + $this->runGenerateMenus(); + + $this->assertFalse($this->menuHasItemTitle('topnav', __('Admin'))); + $this->assertFalse($this->menuHasItemTitle('sidebar_admin', __('Users'))); + } + + private function runGenerateMenus(): void + { + $middleware = app(GenerateMenus::class); + $middleware->handle(Request::create('/'), fn () => response('ok')); + } + + private function menuHasItemTitle(string $menuName, string $title): bool + { + $builder = Menu::get($menuName); + + if ($builder === null) { + return false; + } + + foreach ($builder->all() as $item) { + if ($this->itemHasTitle($item, $title)) { + return true; + } + } + + return false; + } + + private function itemHasTitle($item, string $title): bool + { + if ($item->title === $title) { + return true; + } + + foreach ($item->children() as $child) { + if ($this->itemHasTitle($child, $title)) { + return true; + } + } + + return false; + } } final class RedirectStateProbe extends HandleRedirectListener