Skip to content

Commit

Permalink
fixed #3206
Browse files Browse the repository at this point in the history
  • Loading branch information
Guite committed Mar 19, 2019
1 parent fa65ccf commit 64ea5cc
Show file tree
Hide file tree
Showing 17 changed files with 117 additions and 749 deletions.
1 change: 1 addition & 0 deletions CHANGELOG-3.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ CHANGELOG - ZIKULA 3.0.x
- `Zikula\UsersModule\ProfileModule\ProfileModuleInterface` requires a new method `getBundleName()` to be implemented.
- `Zikula\BlocksModule\AbstractBlockHandler` is not container aware anymore.
- `Zikula\ExtensionsModule\Entity\ExtensionEntity` has renamed `core_min` to `coreCompatibility` and removed `core_max` property (#3649).
- Removed all classes from the `Zikula\Core\Token` namespace. If you need custom csrf tokens use [isCsrfTokenValid()](https://symfony.com/doc/current/security/csrf.html#generating-and-checking-csrf-tokens-manually) instead (#3206).
- The `Zikula\Bundle\HookBundle\ServiceIdTrait` trait has been removed.
- Dropped vendors:
- Removed afarkas/html5shiv
Expand Down
10 changes: 0 additions & 10 deletions src/lib/Zikula/Bundle/CoreBundle/Resources/config/services.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
parameters:
zikula_core.internal.token.max_life: 3600

services:
_defaults:
autowire: true
Expand Down Expand Up @@ -84,13 +81,6 @@ services:
Michelf\:
resource: '../../../../../../vendor/michelf/php-markdown/Michelf/*'

Zikula\Core\Token\Storage: '@Zikula\Core\Token\Storage\SessionStorage'

Zikula\Core\Token\Generator:
arguments:
$secret: '_dummy'
$maxLifetime: '%zikula_core.internal.token.max_life%'

# public because BootstrapBundlesCommand accesses this using container
Zikula\Bundle\CoreBundle\Bundle\Helper\BootstrapHelper:
public: true
Expand Down
39 changes: 18 additions & 21 deletions src/lib/Zikula/Bundle/HookBundle/Controller/HookController.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
use Zikula\Bundle\HookBundle\Dispatcher\HookDispatcherInterface;
use Zikula\Common\Translator\TranslatorInterface;
use Zikula\Common\Translator\TranslatorTrait;
use Zikula\Core\Token\CsrfTokenHandler;
use Zikula\ExtensionsModule\Entity\ExtensionEntity;
use Zikula\ExtensionsModule\Entity\RepositoryInterface\ExtensionRepositoryInterface;
use Zikula\PermissionsModule\Api\ApiInterface\PermissionApiInterface;
Expand Down Expand Up @@ -281,7 +280,9 @@ public function toggleSubscribeAreaStatusAction(
HookDispatcherInterface $dispatcher
) {
$this->setTranslator($translator);
$this->checkAjaxToken();
if (!$this->checkAjaxToken($request)) {
throw new AccessDeniedException();
}

// get subscriberarea from POST
$subscriberArea = $request->request->get('subscriberarea', '');
Expand Down Expand Up @@ -364,7 +365,9 @@ public function changeProviderAreaOrderAction(
HookCollectorInterface $collector
) {
$this->setTranslator($this->get('translator.default'));
$this->checkAjaxToken();
if (!$this->checkAjaxToken($request)) {
throw new AccessDeniedException();
}

// get subscriberarea from POST
$subscriberarea = $request->request->get('subscriberarea', '');
Expand Down Expand Up @@ -393,38 +396,32 @@ public function changeProviderAreaOrderAction(
// set sorting
$this->get('hook_dispatcher')->setBindOrder($subscriberarea, $providerarea);

$ol_id = $request->request->get('ol_id', '');

return $this->json(['result' => true, 'ol_id' => $ol_id]);
return $this->json(['result' => true]);
}

/**
* Check the CSRF token.
* Checks will fall back to $token check if automatic checking fails
*
* @param CsrfTokenHandler $tokenHandler
* @param string $token Token, default null
*
* @throws AccessDeniedException If the CSFR token fails
* @throws \Exception if request is not an XmlHttpRequest
* @param Request $request
*
* @return void
* @return boolean
*/
private function checkAjaxToken(CsrfTokenHandler $tokenHandler, $token = null)
private function checkAjaxToken(Request $request)
{
$currentRequest = $this->get('request_stack')->getCurrentRequest();
if (!$currentRequest->isXmlHttpRequest()) {
throw new \Exception();
if (!$request->isXmlHttpRequest()) {
return false;
}

$sessionName = $this->container->getParameter('zikula.session.name');
$sessionId = $currentRequest->cookies->get($sessionName, null);
$sessionId = $request->cookies->get($sessionName, null);

if ($sessionId == $currentRequest->getSession()->getId()) {
return;
if ($sessionId != $request->getSession()->getId()) {
return false;
}

$tokenHandler->validate($token);
if (!$this->isCsrfTokenValid('hook-ui', $request->request->get('token'))) {
return false;
}
}

private function getExtensionsCapableOf(HookCollectorInterface $collector, ExtensionRepositoryInterface $extensionRepository, $type)
Expand Down
20 changes: 9 additions & 11 deletions src/lib/Zikula/Bundle/HookBundle/Resources/public/js/hookui.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,13 @@ var cloneDraggedItem = true;
* @return none;
*/
subscriberAreaToggle = function(sarea, parea) {
var pars = {
subscriberarea: sarea,
providerarea: parea
};

$.ajax({
url: Routing.generate('zikula_hook_hook_togglesubscribeareastatus'),
data: pars
data: {
token: $('#csrfToken').data('token'),
subscriberarea: sarea,
providerarea: parea
}
}).done(function(data) {
if (data.action == 'bind') {
if (!appendItemBeforeResponse) {
Expand Down Expand Up @@ -286,16 +285,15 @@ var cloneDraggedItem = true;
providersAreas += '&providerarea[]=' + $($(this).attr('id') + '_a').val();
});

var pars = 'ol_id=' + listId +
'&subscriberarea=' + subscriberArea +
providersAreas;
var parameters = 'subscriberarea=' + subscriberArea + providersAreas;
parameters += '&token=' + $('#csrfToken').data('token');

$.ajax({
url: Routing.generate('zikula_hook_hook_changeproviderareaorder'),
data: pars
data: parameters
}).done(function(data) {
// update new sort order
recolorListElements(data.ol_id, $('#' + data.ol_id).down(0).attr('id'));
recolorListElements(listId, $('#' + listId).down(0).attr('id'));
}).fail(function(result) {
alert(result.status + ': ' + result.statusText);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
{{ pageSetVar('title', __('Hooks')) }}
</h3>

<div id="csrfToken" class="hidden" data-token="{{ csrf_token('hook-ui') }}"></div>

<div class="alert alert-info">{{ __('Drag items from the right column to the left column to connect the hooks. Only areas with the same category can be connected.') }}</div>
{% if showBothPanels %}
<div role="tabpanel">
Expand Down
122 changes: 0 additions & 122 deletions src/lib/Zikula/Core/Token/CsrfTokenHandler.php

This file was deleted.

Loading

1 comment on commit 64ea5cc

@craigh
Copy link
Member

@craigh craigh commented on 64ea5cc Mar 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very nice 👍

Please sign in to comment.