From f760b1b317eaa9f74ee034a73615b3edb348d297 Mon Sep 17 00:00:00 2001 From: Peter Dodosch Date: Tue, 21 Mar 2023 14:09:32 +0100 Subject: [PATCH 1/6] chore: add todo --- Admin/FormAdmin.php | 1 + 1 file changed, 1 insertion(+) diff --git a/Admin/FormAdmin.php b/Admin/FormAdmin.php index e94dc251..4810a3ef 100644 --- a/Admin/FormAdmin.php +++ b/Admin/FormAdmin.php @@ -76,6 +76,7 @@ public function configureNavigationItems(NavigationItemCollection $navigationIte public function configureViews(ViewCollection $viewCollection): void { + // Todo: add security $formLocales = \array_values( \array_map( function(Localization $localization) { From 17eb25405f3e51231972aebe5fd4e93497212f12 Mon Sep 17 00:00:00 2001 From: Peter Dodosch Date: Tue, 21 Mar 2023 14:55:55 +0100 Subject: [PATCH 2/6] Add security checks to FormAdmin --- Admin/FormAdmin.php | 149 ++++++++++++++++++++++++-------------------- 1 file changed, 82 insertions(+), 67 deletions(-) diff --git a/Admin/FormAdmin.php b/Admin/FormAdmin.php index 4810a3ef..4c4f62c8 100644 --- a/Admin/FormAdmin.php +++ b/Admin/FormAdmin.php @@ -85,79 +85,94 @@ function(Localization $localization) { $this->webspaceManager->getAllLocalizations() ) ); - $formToolbarActions = [ - new ToolbarAction('sulu_admin.save'), - new ToolbarAction('sulu_admin.delete'), - new DropdownToolbarAction( + + $formToolbarActions = []; + $listToolbarActions = []; + $dataListToolbarActions = []; + + if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::EDIT)) { + $formToolbarActions[] = new ToolbarAction('sulu_admin.save'); + } + if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::DELETE)) { + $formToolbarActions[] = new ToolbarAction('sulu_admin.delete'); + } + if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::ADD)) { + $formToolbarActions[] = new DropdownToolbarAction( 'sulu_admin.edit', 'su-pen', [ new ToolbarAction('sulu_admin.copy'), ] - ), - ]; - $listToolbarActions = [ - new ToolbarAction('sulu_admin.add'), - new ToolbarAction('sulu_admin.delete'), - ]; - $dataListToolbarActions = [ - new ToolbarAction('sulu_admin.delete'), - new ToolbarAction('sulu_admin.export'), - ]; + ); + } + if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::ADD)) { + $listToolbarActions[] = new ToolbarAction('sulu_admin.add'); + } + if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::DELETE)) { + $listToolbarActions[] = new ToolbarAction('sulu_admin.delete'); + } + if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::VIEW)) { + $dataListToolbarActions[] = new ToolbarAction('sulu_admin.export'); + } + if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::DELETE)) { + $dataListToolbarActions[] = new ToolbarAction('sulu_admin.delete'); + } - $viewCollection->add( - $this->viewBuilderFactory->createListViewBuilder(static::LIST_VIEW, '/forms/:locale') - ->setResourceKey(Form::RESOURCE_KEY) - ->setListKey('forms') - ->setTitle('sulu_form.forms') - ->addListAdapters(['table']) - ->addLocales($formLocales) - ->setDefaultLocale($formLocales[0]) - ->setAddView(static::ADD_FORM_VIEW) - ->setEditView(static::EDIT_FORM_VIEW) - ->enableSearching() - ->addToolbarActions($listToolbarActions) - ); - $viewCollection->add( - $this->viewBuilderFactory->createResourceTabViewBuilder(static::ADD_FORM_VIEW, '/forms/:locale/add') - ->setResourceKey(Form::RESOURCE_KEY) - ->addLocales($formLocales) - ->setBackView(static::LIST_VIEW) - ); - $viewCollection->add( - $this->viewBuilderFactory->createFormViewBuilder(static::ADD_FORM_DETAILS_VIEW, '/details') - ->setResourceKey(Form::RESOURCE_KEY) - ->setFormKey('form_details') - ->setTabTitle('sulu_form.general') - ->setEditView(static::EDIT_FORM_VIEW) - ->addToolbarActions($formToolbarActions) - ->setParent(static::ADD_FORM_VIEW) - ); - $viewCollection->add( - $this->viewBuilderFactory->createResourceTabViewBuilder(static::EDIT_FORM_VIEW, '/forms/:locale/:id') - ->setResourceKey(Form::RESOURCE_KEY) - ->addLocales($formLocales) - ->setBackView(static::LIST_VIEW) - ); - $viewCollection->add( - $this->viewBuilderFactory->createFormViewBuilder(static::EDIT_FORM_DETAILS_VIEW, '/details') - ->setResourceKey(Form::RESOURCE_KEY) - ->setFormKey('form_details') - ->setTabTitle('sulu_form.general') - ->addToolbarActions($formToolbarActions) - ->setParent(static::EDIT_FORM_VIEW) - ); - $viewCollection->add( - $this->viewBuilderFactory->createListViewBuilder(static::LIST_VIEW_DATA, '/data') - ->setResourceKey('dynamic_forms') - ->setListKey('form_data') - ->setTabTitle('sulu_form.data') - ->addListAdapters(['table']) - ->addRouterAttributesToListRequest(['id' => 'form']) - ->addRouterAttributesToListMetadata(['id' => 'id']) - ->addToolbarActions($dataListToolbarActions) - ->setParent(static::EDIT_FORM_VIEW) - ); + if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::EDIT)) { + $viewCollection->add( + $this->viewBuilderFactory->createListViewBuilder(static::LIST_VIEW, '/forms/:locale') + ->setResourceKey(Form::RESOURCE_KEY) + ->setListKey('forms') + ->setTitle('sulu_form.forms') + ->addListAdapters(['table']) + ->addLocales($formLocales) + ->setDefaultLocale($formLocales[0]) + ->setAddView(static::ADD_FORM_VIEW) + ->setEditView(static::EDIT_FORM_VIEW) + ->enableSearching() + ->addToolbarActions($listToolbarActions) + ); + $viewCollection->add( + $this->viewBuilderFactory->createResourceTabViewBuilder(static::ADD_FORM_VIEW, '/forms/:locale/add') + ->setResourceKey(Form::RESOURCE_KEY) + ->addLocales($formLocales) + ->setBackView(static::LIST_VIEW) + ); + $viewCollection->add( + $this->viewBuilderFactory->createFormViewBuilder(static::ADD_FORM_DETAILS_VIEW, '/details') + ->setResourceKey(Form::RESOURCE_KEY) + ->setFormKey('form_details') + ->setTabTitle('sulu_form.general') + ->setEditView(static::EDIT_FORM_VIEW) + ->addToolbarActions($formToolbarActions) + ->setParent(static::ADD_FORM_VIEW) + ); + $viewCollection->add( + $this->viewBuilderFactory->createResourceTabViewBuilder(static::EDIT_FORM_VIEW, '/forms/:locale/:id') + ->setResourceKey(Form::RESOURCE_KEY) + ->addLocales($formLocales) + ->setBackView(static::LIST_VIEW) + ); + $viewCollection->add( + $this->viewBuilderFactory->createFormViewBuilder(static::EDIT_FORM_DETAILS_VIEW, '/details') + ->setResourceKey(Form::RESOURCE_KEY) + ->setFormKey('form_details') + ->setTabTitle('sulu_form.general') + ->addToolbarActions($formToolbarActions) + ->setParent(static::EDIT_FORM_VIEW) + ); + $viewCollection->add( + $this->viewBuilderFactory->createListViewBuilder(static::LIST_VIEW_DATA, '/data') + ->setResourceKey('dynamic_forms') + ->setListKey('form_data') + ->setTabTitle('sulu_form.data') + ->addListAdapters(['table']) + ->addRouterAttributesToListRequest(['id' => 'form']) + ->addRouterAttributesToListMetadata(['id' => 'id']) + ->addToolbarActions($dataListToolbarActions) + ->setParent(static::EDIT_FORM_VIEW) + ); + } } public function getSecurityContexts() From a5d0edbef01ae0fbfe89ce4be18df1e207a7fbec Mon Sep 17 00:00:00 2001 From: Peter Dodosch Date: Tue, 21 Mar 2023 15:15:46 +0100 Subject: [PATCH 3/6] Add security check to form list --- Admin/FormAdmin.php | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/Admin/FormAdmin.php b/Admin/FormAdmin.php index 4c4f62c8..799a346d 100644 --- a/Admin/FormAdmin.php +++ b/Admin/FormAdmin.php @@ -76,7 +76,6 @@ public function configureNavigationItems(NavigationItemCollection $navigationIte public function configureViews(ViewCollection $viewCollection): void { - // Todo: add security $formLocales = \array_values( \array_map( function(Localization $localization) { @@ -117,21 +116,26 @@ function(Localization $localization) { if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::DELETE)) { $dataListToolbarActions[] = new ToolbarAction('sulu_admin.delete'); } - + if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::VIEW)) { + $listViewBuilder = $this->viewBuilderFactory->createListViewBuilder(static::LIST_VIEW, '/forms/:locale') + ->setResourceKey(Form::RESOURCE_KEY) + ->setListKey('forms') + ->setTitle('sulu_form.forms') + ->addListAdapters(['table']) + ->addLocales($formLocales) + ->setDefaultLocale($formLocales[0]) + ->enableSearching() + ->addToolbarActions($listToolbarActions) + ; + if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::EDIT)) { + $listViewBuilder->setEditView(static::EDIT_FORM_VIEW); + } + if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::ADD)) { + $listViewBuilder->setAddView(static::ADD_FORM_VIEW); + } + $viewCollection->add($listViewBuilder); + } if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::EDIT)) { - $viewCollection->add( - $this->viewBuilderFactory->createListViewBuilder(static::LIST_VIEW, '/forms/:locale') - ->setResourceKey(Form::RESOURCE_KEY) - ->setListKey('forms') - ->setTitle('sulu_form.forms') - ->addListAdapters(['table']) - ->addLocales($formLocales) - ->setDefaultLocale($formLocales[0]) - ->setAddView(static::ADD_FORM_VIEW) - ->setEditView(static::EDIT_FORM_VIEW) - ->enableSearching() - ->addToolbarActions($listToolbarActions) - ); $viewCollection->add( $this->viewBuilderFactory->createResourceTabViewBuilder(static::ADD_FORM_VIEW, '/forms/:locale/add') ->setResourceKey(Form::RESOURCE_KEY) From b9663d36ed66b48999e2da98af268e315d4dac72 Mon Sep 17 00:00:00 2001 From: Alexander Schranz Date: Thu, 23 Mar 2023 00:23:07 +0100 Subject: [PATCH 4/6] Fix add without edit permissions in FormAdmin --- Admin/FormAdmin.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Admin/FormAdmin.php b/Admin/FormAdmin.php index 799a346d..d63d89e9 100644 --- a/Admin/FormAdmin.php +++ b/Admin/FormAdmin.php @@ -64,7 +64,7 @@ public function __construct( public function configureNavigationItems(NavigationItemCollection $navigationItemCollection): void { - if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::VIEW)) { + if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::EDIT)) { $navigationItem = new NavigationItem('sulu_form.forms'); $navigationItem->setIcon('su-magic'); $navigationItem->setPosition(10); @@ -135,7 +135,7 @@ function(Localization $localization) { } $viewCollection->add($listViewBuilder); } - if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::EDIT)) { + if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::ADD)) { $viewCollection->add( $this->viewBuilderFactory->createResourceTabViewBuilder(static::ADD_FORM_VIEW, '/forms/:locale/add') ->setResourceKey(Form::RESOURCE_KEY) @@ -151,6 +151,8 @@ function(Localization $localization) { ->addToolbarActions($formToolbarActions) ->setParent(static::ADD_FORM_VIEW) ); + } + if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::EDIT)) { $viewCollection->add( $this->viewBuilderFactory->createResourceTabViewBuilder(static::EDIT_FORM_VIEW, '/forms/:locale/:id') ->setResourceKey(Form::RESOURCE_KEY) From 05799bae0a124818c3c3a16f1b31560a433f5aec Mon Sep 17 00:00:00 2001 From: Alexander Schranz Date: Thu, 23 Mar 2023 00:32:36 +0100 Subject: [PATCH 5/6] Split add and edit FormToolbarActions --- Admin/FormAdmin.php | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/Admin/FormAdmin.php b/Admin/FormAdmin.php index d63d89e9..8391b5a4 100644 --- a/Admin/FormAdmin.php +++ b/Admin/FormAdmin.php @@ -85,18 +85,22 @@ function(Localization $localization) { ) ); - $formToolbarActions = []; + $addFormToolbarActions = []; + $editFormToolbarActions = []; $listToolbarActions = []; $dataListToolbarActions = []; + if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::ADD)) { + $addFormToolbarActions[] = new ToolbarAction('sulu_admin.save'); + } if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::EDIT)) { - $formToolbarActions[] = new ToolbarAction('sulu_admin.save'); + $editFormToolbarActions[] = new ToolbarAction('sulu_admin.save'); } if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::DELETE)) { - $formToolbarActions[] = new ToolbarAction('sulu_admin.delete'); + $editFormToolbarActions[] = new ToolbarAction('sulu_admin.delete'); } if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::ADD)) { - $formToolbarActions[] = new DropdownToolbarAction( + $editFormToolbarActions[] = new DropdownToolbarAction( 'sulu_admin.edit', 'su-pen', [ @@ -127,7 +131,7 @@ function(Localization $localization) { ->enableSearching() ->addToolbarActions($listToolbarActions) ; - if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::EDIT)) { + if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::VIEW)) { $listViewBuilder->setEditView(static::EDIT_FORM_VIEW); } if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::ADD)) { @@ -148,11 +152,11 @@ function(Localization $localization) { ->setFormKey('form_details') ->setTabTitle('sulu_form.general') ->setEditView(static::EDIT_FORM_VIEW) - ->addToolbarActions($formToolbarActions) + ->addToolbarActions($addFormToolbarActions) ->setParent(static::ADD_FORM_VIEW) ); } - if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::EDIT)) { + if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::VIEW)) { $viewCollection->add( $this->viewBuilderFactory->createResourceTabViewBuilder(static::EDIT_FORM_VIEW, '/forms/:locale/:id') ->setResourceKey(Form::RESOURCE_KEY) @@ -164,7 +168,7 @@ function(Localization $localization) { ->setResourceKey(Form::RESOURCE_KEY) ->setFormKey('form_details') ->setTabTitle('sulu_form.general') - ->addToolbarActions($formToolbarActions) + ->addToolbarActions($editFormToolbarActions) ->setParent(static::EDIT_FORM_VIEW) ); $viewCollection->add( From 4136525079fe632f66a5932a9f8a9c2e571b6fe0 Mon Sep 17 00:00:00 2001 From: Alexander Schranz Date: Thu, 23 Mar 2023 00:35:37 +0100 Subject: [PATCH 6/6] Fix unnecessary check for EditView detail should be accessible --- Admin/FormAdmin.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Admin/FormAdmin.php b/Admin/FormAdmin.php index 8391b5a4..a5cba21e 100644 --- a/Admin/FormAdmin.php +++ b/Admin/FormAdmin.php @@ -130,10 +130,8 @@ function(Localization $localization) { ->setDefaultLocale($formLocales[0]) ->enableSearching() ->addToolbarActions($listToolbarActions) + ->setEditView(static::EDIT_FORM_VIEW) ; - if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::VIEW)) { - $listViewBuilder->setEditView(static::EDIT_FORM_VIEW); - } if ($this->securityChecker->hasPermission(static::SECURITY_CONTEXT, PermissionTypes::ADD)) { $listViewBuilder->setAddView(static::ADD_FORM_VIEW); }